Commit 3499495205a676d85fcc2f3c28e35ec9b43c47e3
Committed by
Kyle McMartin
1 parent
ba1f188cae
[PARISC] Use work queue in LED/LCD driver instead of tasklet.
2.6.12-rc1-pa6 use work queue in LED/LCD driver instead of tasklet. Main advantage is it allows use of msleep() in the led_LCD_driver to "atomically" perform two MMIO writes (CMD, then DATA). Lead to nice cleanup of the main led_work_func() and led_LCD_driver(). Kudos to David for being persistent. From: David Pye <dmp@davidmpye.dyndns.org> Signed-off-by: Grant Grundler <grundler@parisc-linux.org> Signed-off-by: Kyle McMartin <kyle@parisc-linux.org>
Showing 3 changed files with 112 additions and 124 deletions Side-by-side Diff
arch/parisc/kernel/time.c
... | ... | @@ -89,14 +89,6 @@ |
89 | 89 | } |
90 | 90 | } |
91 | 91 | |
92 | -#ifdef CONFIG_CHASSIS_LCD_LED | |
93 | - /* Only schedule the led tasklet on cpu 0, and only if it | |
94 | - * is enabled. | |
95 | - */ | |
96 | - if (cpu == 0 && !atomic_read(&led_tasklet.count)) | |
97 | - tasklet_schedule(&led_tasklet); | |
98 | -#endif | |
99 | - | |
100 | 92 | /* check soft power switch status */ |
101 | 93 | if (cpu == 0 && !atomic_read(&power_tasklet.count)) |
102 | 94 | tasklet_schedule(&power_tasklet); |
drivers/parisc/led.c
... | ... | @@ -18,6 +18,9 @@ |
18 | 18 | * Changes: |
19 | 19 | * - Audit copy_from_user in led_proc_write. |
20 | 20 | * Daniele Bellucci <bellucda@tiscali.it> |
21 | + * - Switch from using a tasklet to a work queue, so the led_LCD_driver | |
22 | + * can sleep. | |
23 | + * David Pye <dmp@davidmpye.dyndns.org> | |
21 | 24 | */ |
22 | 25 | |
23 | 26 | #include <linux/config.h> |
... | ... | @@ -37,6 +40,7 @@ |
37 | 40 | #include <linux/proc_fs.h> |
38 | 41 | #include <linux/ctype.h> |
39 | 42 | #include <linux/blkdev.h> |
43 | +#include <linux/workqueue.h> | |
40 | 44 | #include <linux/rcupdate.h> |
41 | 45 | #include <asm/io.h> |
42 | 46 | #include <asm/processor.h> |
43 | 47 | |
44 | 48 | |
45 | 49 | |
... | ... | @@ -47,25 +51,30 @@ |
47 | 51 | #include <asm/uaccess.h> |
48 | 52 | |
49 | 53 | /* The control of the LEDs and LCDs on PARISC-machines have to be done |
50 | - completely in software. The necessary calculations are done in a tasklet | |
51 | - which is scheduled at every timer interrupt and since the calculations | |
52 | - may consume relatively much CPU-time some of the calculations can be | |
54 | + completely in software. The necessary calculations are done in a work queue | |
55 | + task which is scheduled regularly, and since the calculations may consume a | |
56 | + relatively large amount of CPU time, some of the calculations can be | |
53 | 57 | turned off with the following variables (controlled via procfs) */ |
54 | 58 | |
55 | 59 | static int led_type = -1; |
56 | -static int led_heartbeat = 1; | |
57 | -static int led_diskio = 1; | |
58 | -static int led_lanrxtx = 1; | |
60 | +static unsigned char lastleds; /* LED state from most recent update */ | |
61 | +static unsigned int led_heartbeat = 1; | |
62 | +static unsigned int led_diskio = 1; | |
63 | +static unsigned int led_lanrxtx = 1; | |
59 | 64 | static char lcd_text[32]; |
60 | 65 | static char lcd_text_default[32]; |
61 | 66 | |
67 | + | |
68 | +static struct workqueue_struct *led_wq; | |
69 | +static void led_work_func(void *); | |
70 | +static DECLARE_WORK(led_task, led_work_func, NULL); | |
71 | + | |
62 | 72 | #if 0 |
63 | 73 | #define DPRINTK(x) printk x |
64 | 74 | #else |
65 | 75 | #define DPRINTK(x) |
66 | 76 | #endif |
67 | 77 | |
68 | - | |
69 | 78 | struct lcd_block { |
70 | 79 | unsigned char command; /* stores the command byte */ |
71 | 80 | unsigned char on; /* value for turning LED on */ |
72 | 81 | |
73 | 82 | |
... | ... | @@ -116,12 +125,27 @@ |
116 | 125 | #define LCD_DATA_REG lcd_info.lcd_data_reg_addr |
117 | 126 | #define LED_DATA_REG lcd_info.lcd_cmd_reg_addr /* LASI & ASP only */ |
118 | 127 | |
128 | +#define LED_HASLCD 1 | |
129 | +#define LED_NOLCD 0 | |
119 | 130 | |
131 | +/* The workqueue must be created at init-time */ | |
132 | +static int start_task(void) | |
133 | +{ | |
134 | + /* Display the default text now */ | |
135 | + if (led_type == LED_HASLCD) lcd_print( lcd_text_default ); | |
136 | + | |
137 | + /* Create the work queue and queue the LED task */ | |
138 | + led_wq = create_singlethread_workqueue("led_wq"); | |
139 | + queue_work(led_wq, &led_task); | |
140 | + | |
141 | + return 0; | |
142 | +} | |
143 | + | |
144 | +device_initcall(start_task); | |
145 | + | |
120 | 146 | /* ptr to LCD/LED-specific function */ |
121 | 147 | static void (*led_func_ptr) (unsigned char); |
122 | 148 | |
123 | -#define LED_HASLCD 1 | |
124 | -#define LED_NOLCD 0 | |
125 | 149 | #ifdef CONFIG_PROC_FS |
126 | 150 | static int led_proc_read(char *page, char **start, off_t off, int count, |
127 | 151 | int *eof, void *data) |
128 | 152 | |
129 | 153 | |
130 | 154 | |
... | ... | @@ -286,52 +310,35 @@ |
286 | 310 | /* |
287 | 311 | ** |
288 | 312 | ** led_LCD_driver() |
289 | - ** | |
290 | - ** The logic of the LCD driver is, that we write at every scheduled call | |
291 | - ** only to one of LCD_CMD_REG _or_ LCD_DATA_REG - registers. | |
292 | - ** That way we don't need to let this tasklet busywait for min_cmd_delay | |
293 | - ** milliseconds. | |
294 | - ** | |
295 | - ** TODO: check the value of "min_cmd_delay" against the value of HZ. | |
296 | 313 | ** |
297 | 314 | */ |
298 | 315 | static void led_LCD_driver(unsigned char leds) |
299 | 316 | { |
300 | - static int last_index; /* 0:heartbeat, 1:disk, 2:lan_in, 3:lan_out */ | |
301 | - static int last_was_cmd;/* 0: CMD was written last, 1: DATA was last */ | |
302 | - struct lcd_block *block_ptr; | |
303 | - int value; | |
317 | + static int i; | |
318 | + static unsigned char mask[4] = { LED_HEARTBEAT, LED_DISK_IO, | |
319 | + LED_LAN_RCV, LED_LAN_TX }; | |
320 | + | |
321 | + static struct lcd_block * blockp[4] = { | |
322 | + &lcd_info.heartbeat, | |
323 | + &lcd_info.disk_io, | |
324 | + &lcd_info.lan_rcv, | |
325 | + &lcd_info.lan_tx | |
326 | + }; | |
304 | 327 | |
305 | - switch (last_index) { | |
306 | - case 0: block_ptr = &lcd_info.heartbeat; | |
307 | - value = leds & LED_HEARTBEAT; | |
308 | - break; | |
309 | - case 1: block_ptr = &lcd_info.disk_io; | |
310 | - value = leds & LED_DISK_IO; | |
311 | - break; | |
312 | - case 2: block_ptr = &lcd_info.lan_rcv; | |
313 | - value = leds & LED_LAN_RCV; | |
314 | - break; | |
315 | - case 3: block_ptr = &lcd_info.lan_tx; | |
316 | - value = leds & LED_LAN_TX; | |
317 | - break; | |
318 | - default: /* should never happen: */ | |
319 | - return; | |
320 | - } | |
321 | - | |
322 | - if (last_was_cmd) { | |
323 | - /* write the value to the LCD data port */ | |
324 | - gsc_writeb( value ? block_ptr->on : block_ptr->off, LCD_DATA_REG ); | |
325 | - } else { | |
326 | - /* write the command-byte to the LCD command register */ | |
327 | - gsc_writeb( block_ptr->command, LCD_CMD_REG ); | |
328 | - } | |
328 | + /* Convert min_cmd_delay to milliseconds */ | |
329 | + unsigned int msec_cmd_delay = 1 + (lcd_info.min_cmd_delay / 1000); | |
329 | 330 | |
330 | - /* now update the vars for the next interrupt iteration */ | |
331 | - if (++last_was_cmd == 2) { /* switch between cmd & data */ | |
332 | - last_was_cmd = 0; | |
333 | - if (++last_index == 4) | |
334 | - last_index = 0; /* switch back to heartbeat index */ | |
331 | + for (i=0; i<4; ++i) | |
332 | + { | |
333 | + if ((leds & mask[i]) != (lastleds & mask[i])) | |
334 | + { | |
335 | + gsc_writeb( blockp[i]->command, LCD_CMD_REG ); | |
336 | + msleep(msec_cmd_delay); | |
337 | + | |
338 | + gsc_writeb( leds & mask[i] ? blockp[i]->on : | |
339 | + blockp[i]->off, LCD_DATA_REG ); | |
340 | + msleep(msec_cmd_delay); | |
341 | + } | |
335 | 342 | } |
336 | 343 | } |
337 | 344 | |
... | ... | @@ -356,7 +363,7 @@ |
356 | 363 | |
357 | 364 | rx_total = tx_total = 0; |
358 | 365 | |
359 | - /* we are running as tasklet, so locking dev_base | |
366 | + /* we are running as a workqueue task, so locking dev_base | |
360 | 367 | * for reading should be OK */ |
361 | 368 | read_lock(&dev_base_lock); |
362 | 369 | rcu_read_lock(); |
... | ... | @@ -405,7 +412,7 @@ |
405 | 412 | static unsigned long last_pgpgin, last_pgpgout; |
406 | 413 | struct page_state pgstat; |
407 | 414 | int changed; |
408 | - | |
415 | + | |
409 | 416 | get_full_page_state(&pgstat); /* get no of sectors in & out */ |
410 | 417 | |
411 | 418 | /* Just use a very simple calculation here. Do not care about overflow, |
412 | 419 | |
413 | 420 | |
414 | 421 | |
415 | 422 | |
416 | 423 | |
417 | 424 | |
418 | 425 | |
419 | 426 | |
420 | 427 | |
421 | 428 | |
422 | 429 | |
423 | 430 | |
424 | 431 | |
425 | 432 | |
426 | 433 | |
427 | 434 | |
... | ... | @@ -413,87 +420,71 @@ |
413 | 420 | changed = (pgstat.pgpgin != last_pgpgin) || (pgstat.pgpgout != last_pgpgout); |
414 | 421 | last_pgpgin = pgstat.pgpgin; |
415 | 422 | last_pgpgout = pgstat.pgpgout; |
416 | - | |
423 | + | |
417 | 424 | return (changed ? LED_DISK_IO : 0); |
418 | 425 | } |
419 | 426 | |
420 | 427 | |
421 | 428 | |
422 | 429 | /* |
423 | - ** led_tasklet_func() | |
430 | + ** led_work_func() | |
424 | 431 | ** |
425 | - ** is scheduled at every timer interrupt from time.c and | |
426 | - ** updates the chassis LCD/LED | |
432 | + ** manages when and which chassis LCD/LED gets updated | |
427 | 433 | |
428 | 434 | TODO: |
429 | 435 | - display load average (older machines like 715/64 have 4 "free" LED's for that) |
430 | 436 | - optimizations |
431 | 437 | */ |
432 | 438 | |
433 | -#define HEARTBEAT_LEN (HZ*6/100) | |
434 | -#define HEARTBEAT_2ND_RANGE_START (HZ*22/100) | |
439 | +#define HEARTBEAT_LEN (HZ*10/100) | |
440 | +#define HEARTBEAT_2ND_RANGE_START (HZ*28/100) | |
435 | 441 | #define HEARTBEAT_2ND_RANGE_END (HEARTBEAT_2ND_RANGE_START + HEARTBEAT_LEN) |
436 | 442 | |
437 | -#define NORMALIZED_COUNT(count) (count/(HZ/100)) | |
443 | +#define LED_UPDATE_INTERVAL (1 + (HZ*19/1000)) | |
438 | 444 | |
439 | -static void led_tasklet_func(unsigned long unused) | |
445 | +static void led_work_func (void *unused) | |
440 | 446 | { |
441 | - static unsigned char lastleds; | |
442 | - unsigned char currentleds; /* stores current value of the LEDs */ | |
443 | - static unsigned long count; /* static incremented value, not wrapped */ | |
447 | + static unsigned long last_jiffies; | |
444 | 448 | static unsigned long count_HZ; /* counter in range 0..HZ */ |
449 | + unsigned char currentleds = 0; /* stores current value of the LEDs */ | |
445 | 450 | |
446 | 451 | /* exit if not initialized */ |
447 | 452 | if (!led_func_ptr) |
448 | 453 | return; |
449 | 454 | |
450 | - /* increment the local counters */ | |
451 | - ++count; | |
452 | - if (++count_HZ == HZ) | |
455 | + /* increment the heartbeat timekeeper */ | |
456 | + count_HZ += jiffies - last_jiffies; | |
457 | + last_jiffies = jiffies; | |
458 | + if (count_HZ >= HZ) | |
453 | 459 | count_HZ = 0; |
454 | 460 | |
455 | - currentleds = lastleds; | |
456 | - | |
457 | - if (led_heartbeat) | |
461 | + if (likely(led_heartbeat)) | |
458 | 462 | { |
459 | - /* flash heartbeat-LED like a real heart (2 x short then a long delay) */ | |
460 | - if (count_HZ<HEARTBEAT_LEN || | |
461 | - (count_HZ>=HEARTBEAT_2ND_RANGE_START && count_HZ<HEARTBEAT_2ND_RANGE_END)) | |
462 | - currentleds |= LED_HEARTBEAT; | |
463 | - else | |
464 | - currentleds &= ~LED_HEARTBEAT; | |
463 | + /* flash heartbeat-LED like a real heart | |
464 | + * (2 x short then a long delay) | |
465 | + */ | |
466 | + if (count_HZ < HEARTBEAT_LEN || | |
467 | + (count_HZ >= HEARTBEAT_2ND_RANGE_START && | |
468 | + count_HZ < HEARTBEAT_2ND_RANGE_END)) | |
469 | + currentleds |= LED_HEARTBEAT; | |
465 | 470 | } |
466 | 471 | |
467 | - /* look for network activity and flash LEDs respectively */ | |
468 | - if (led_lanrxtx && ((NORMALIZED_COUNT(count)+(8/2)) & 7) == 0) | |
469 | - { | |
470 | - currentleds &= ~(LED_LAN_RCV | LED_LAN_TX); | |
471 | - currentleds |= led_get_net_activity(); | |
472 | - } | |
472 | + if (likely(led_lanrxtx)) currentleds |= led_get_net_activity(); | |
473 | + if (likely(led_diskio)) currentleds |= led_get_diskio_activity(); | |
473 | 474 | |
474 | - /* avoid to calculate diskio-stats at same irq as netio-stats */ | |
475 | - if (led_diskio && (NORMALIZED_COUNT(count) & 7) == 0) | |
476 | - { | |
477 | - currentleds &= ~LED_DISK_IO; | |
478 | - currentleds |= led_get_diskio_activity(); | |
479 | - } | |
480 | - | |
481 | 475 | /* blink all LEDs twice a second if we got an Oops (HPMC) */ |
482 | - if (oops_in_progress) { | |
476 | + if (unlikely(oops_in_progress)) | |
483 | 477 | currentleds = (count_HZ<=(HZ/2)) ? 0 : 0xff; |
478 | + | |
479 | + if (currentleds != lastleds) | |
480 | + { | |
481 | + led_func_ptr(currentleds); /* Update the LCD/LEDs */ | |
482 | + lastleds = currentleds; | |
484 | 483 | } |
485 | - | |
486 | - /* update the LCD/LEDs */ | |
487 | - if (currentleds != lastleds) { | |
488 | - led_func_ptr(currentleds); | |
489 | - lastleds = currentleds; | |
490 | - } | |
484 | + | |
485 | + queue_delayed_work(led_wq, &led_task, LED_UPDATE_INTERVAL); | |
491 | 486 | } |
492 | 487 | |
493 | -/* main led tasklet struct (scheduled from time.c) */ | |
494 | -DECLARE_TASKLET_DISABLED(led_tasklet, led_tasklet_func, 0); | |
495 | - | |
496 | - | |
497 | 488 | /* |
498 | 489 | ** led_halt() |
499 | 490 | ** |
... | ... | @@ -522,9 +513,13 @@ |
522 | 513 | default: return NOTIFY_DONE; |
523 | 514 | } |
524 | 515 | |
525 | - /* completely stop the LED/LCD tasklet */ | |
526 | - tasklet_disable(&led_tasklet); | |
527 | - | |
516 | + /* Cancel the work item and delete the queue */ | |
517 | + if (led_wq) { | |
518 | + cancel_rearming_delayed_workqueue(led_wq, &led_task); | |
519 | + destroy_workqueue(led_wq); | |
520 | + led_wq = NULL; | |
521 | + } | |
522 | + | |
528 | 523 | if (lcd_info.model == DISPLAY_MODEL_LCD) |
529 | 524 | lcd_print(txt); |
530 | 525 | else |
... | ... | @@ -559,7 +554,6 @@ |
559 | 554 | printk(KERN_INFO "LCD display at %lx,%lx registered\n", |
560 | 555 | LCD_CMD_REG , LCD_DATA_REG); |
561 | 556 | led_func_ptr = led_LCD_driver; |
562 | - lcd_print( lcd_text_default ); | |
563 | 557 | led_type = LED_HASLCD; |
564 | 558 | break; |
565 | 559 | |
... | ... | @@ -589,9 +583,11 @@ |
589 | 583 | initialized++; |
590 | 584 | register_reboot_notifier(&led_notifier); |
591 | 585 | |
592 | - /* start the led tasklet for the first time */ | |
593 | - tasklet_enable(&led_tasklet); | |
594 | - | |
586 | + /* Ensure the work is queued */ | |
587 | + if (led_wq) { | |
588 | + queue_work(led_wq, &led_task); | |
589 | + } | |
590 | + | |
595 | 591 | return 0; |
596 | 592 | } |
597 | 593 | |
... | ... | @@ -626,8 +622,8 @@ |
626 | 622 | ** lcd_print() |
627 | 623 | ** |
628 | 624 | ** Displays the given string on the LCD-Display of newer machines. |
629 | - ** lcd_print() disables the timer-based led tasklet during its | |
630 | - ** execution and enables it afterwards again. | |
625 | + ** lcd_print() disables/enables the timer-based led work queue to | |
626 | + ** avoid a race condition while writing the CMD/DATA register pair. | |
631 | 627 | ** |
632 | 628 | */ |
633 | 629 | int lcd_print( char *str ) |
634 | 630 | |
... | ... | @@ -637,12 +633,13 @@ |
637 | 633 | if (!led_func_ptr || lcd_info.model != DISPLAY_MODEL_LCD) |
638 | 634 | return 0; |
639 | 635 | |
640 | - /* temporarily disable the led tasklet */ | |
641 | - tasklet_disable(&led_tasklet); | |
636 | + /* temporarily disable the led work task */ | |
637 | + if (led_wq) | |
638 | + cancel_rearming_delayed_workqueue(led_wq, &led_task); | |
642 | 639 | |
643 | 640 | /* copy display string to buffer for procfs */ |
644 | 641 | strlcpy(lcd_text, str, sizeof(lcd_text)); |
645 | - | |
642 | + | |
646 | 643 | /* Set LCD Cursor to 1st character */ |
647 | 644 | gsc_writeb(lcd_info.reset_cmd1, LCD_CMD_REG); |
648 | 645 | udelay(lcd_info.min_cmd_delay); |
... | ... | @@ -656,8 +653,10 @@ |
656 | 653 | udelay(lcd_info.min_cmd_delay); |
657 | 654 | } |
658 | 655 | |
659 | - /* re-enable the led tasklet */ | |
660 | - tasklet_enable(&led_tasklet); | |
656 | + /* re-queue the work */ | |
657 | + if (led_wq) { | |
658 | + queue_work(led_wq, &led_task); | |
659 | + } | |
661 | 660 | |
662 | 661 | return lcd_info.lcd_width; |
663 | 662 | } |
include/asm-parisc/led.h
... | ... | @@ -23,9 +23,6 @@ |
23 | 23 | |
24 | 24 | #define LED_CMD_REG_NONE 0 /* NULL == no addr for the cmd register */ |
25 | 25 | |
26 | -/* led tasklet struct */ | |
27 | -extern struct tasklet_struct led_tasklet; | |
28 | - | |
29 | 26 | /* register_led_driver() */ |
30 | 27 | int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long data_reg); |
31 | 28 |