[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>

1 files changed, 112 insertions, 113 deletions

diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c
index 286902298e33..95bd07b8b61b 100644
--- a/drivers/parisc/led.c
+++ b/drivers/parisc/led.c
@@ -18,6 +18,9 @@
  * Changes:
  *      - Audit copy_from_user in led_proc_write.
  *                                Daniele Bellucci <bellucda@tiscali.it>
+ *      - Switch from using a tasklet to a work queue, so the led_LCD_driver
+ *              can sleep.
+ *                                David Pye <dmp@davidmpye.dyndns.org>
  */
 
 #include <linux/config.h>
@@ -37,6 +40,7 @@
 #include <linux/proc_fs.h>
 #include <linux/ctype.h>
 #include <linux/blkdev.h>
+#include <linux/workqueue.h>
 #include <linux/rcupdate.h>
 #include <asm/io.h>
 #include <asm/processor.h>
@@ -47,25 +51,30 @@
 #include <asm/uaccess.h>
 
 /* The control of the LEDs and LCDs on PARISC-machines have to be done 
-   completely in software. The necessary calculations are done in a tasklet
-   which is scheduled at every timer interrupt and since the calculations 
-   may consume relatively much CPU-time some of the calculations can be 
+   completely in software. The necessary calculations are done in a work queue
+   task which is scheduled regularly, and since the calculations may consume a 
+   relatively large amount of CPU time, some of the calculations can be 
    turned off with the following variables (controlled via procfs) */
 
 static int led_type = -1;
-static int led_heartbeat = 1;
-static int led_diskio = 1;
-static int led_lanrxtx = 1;
+static unsigned char lastleds;  /* LED state from most recent update */
+static unsigned int led_heartbeat = 1;
+static unsigned int led_diskio = 1;
+static unsigned int led_lanrxtx = 1;
 static char lcd_text[32];
 static char lcd_text_default[32];
 
+
+static struct workqueue_struct *led_wq;
+static void led_work_func(void *);
+static DECLARE_WORK(led_task, led_work_func, NULL);
+
 #if 0
 #define DPRINTK(x)      printk x
 #else
 #define DPRINTK(x)
 #endif
 
-
 struct lcd_block {
         unsigned char command;  /* stores the command byte      */
         unsigned char on;       /* value for turning LED on     */
@@ -116,12 +125,27 @@ lcd_info __attribute__((aligned(8))) =
 #define LCD_DATA_REG    lcd_info.lcd_data_reg_addr       
 #define LED_DATA_REG    lcd_info.lcd_cmd_reg_addr       /* LASI & ASP only */
 
+#define LED_HASLCD 1
+#define LED_NOLCD  0
+
+/* The workqueue must be created at init-time */
+static int start_task(void) 
+{       
+        /* Display the default text now */
+        if (led_type == LED_HASLCD) lcd_print( lcd_text_default );
+
+        /* Create the work queue and queue the LED task */
+        led_wq = create_singlethread_workqueue("led_wq");       
+        queue_work(led_wq, &led_task);
+
+        return 0;
+}
+
+device_initcall(start_task);
 
 /* ptr to LCD/LED-specific function */
 static void (*led_func_ptr) (unsigned char);
 
-#define LED_HASLCD 1
-#define LED_NOLCD  0
 #ifdef CONFIG_PROC_FS
 static int led_proc_read(char *page, char **start, off_t off, int count, 
         int *eof, void *data)
@@ -286,52 +310,35 @@ static void led_LASI_driver(unsigned char leds)
 /*
    ** 
    ** led_LCD_driver()
-   ** 
-   ** The logic of the LCD driver is, that we write at every scheduled call
-   ** only to one of LCD_CMD_REG _or_ LCD_DATA_REG - registers.
-   ** That way we don't need to let this tasklet busywait for min_cmd_delay
-   ** milliseconds.
-   **
-   ** TODO: check the value of "min_cmd_delay" against the value of HZ.
    **   
  */
 static void led_LCD_driver(unsigned char leds)
 {
-        static int last_index;  /* 0:heartbeat, 1:disk, 2:lan_in, 3:lan_out */
-        static int last_was_cmd;/* 0: CMD was written last, 1: DATA was last */
-        struct lcd_block *block_ptr;
-        int value;
-
-        switch (last_index) {
-            case 0:     block_ptr = &lcd_info.heartbeat;
-                        value = leds & LED_HEARTBEAT;
-                        break;
-            case 1:     block_ptr = &lcd_info.disk_io;
-                        value = leds & LED_DISK_IO;
-                        break;                                  
-            case 2:     block_ptr = &lcd_info.lan_rcv;
-                        value = leds & LED_LAN_RCV;
-                        break;                                  
-            case 3:     block_ptr = &lcd_info.lan_tx;
-                        value = leds & LED_LAN_TX;
-                        break;
-            default:    /* should never happen: */
-                        return;
-        }
-
-        if (last_was_cmd) {
-            /* write the value to the LCD data port */
-            gsc_writeb( value ? block_ptr->on : block_ptr->off, LCD_DATA_REG );
-        } else {
-            /* write the command-byte to the LCD command register */
-            gsc_writeb( block_ptr->command, LCD_CMD_REG );
-        }    
+        static int i;
+        static unsigned char mask[4] = { LED_HEARTBEAT, LED_DISK_IO,
+                LED_LAN_RCV, LED_LAN_TX };
         
-        /* now update the vars for the next interrupt iteration */ 
-        if (++last_was_cmd == 2) { /* switch between cmd & data */
-            last_was_cmd = 0;
-            if (++last_index == 4) 
-                last_index = 0;  /* switch back to heartbeat index */
+        static struct lcd_block * blockp[4] = {
+                &lcd_info.heartbeat,
+                &lcd_info.disk_io,
+                &lcd_info.lan_rcv,
+                &lcd_info.lan_tx
+        };
+
+        /* Convert min_cmd_delay to milliseconds */
+        unsigned int msec_cmd_delay = 1 + (lcd_info.min_cmd_delay / 1000);
+        
+        for (i=0; i<4; ++i) 
+        {
+                if ((leds & mask[i]) != (lastleds & mask[i])) 
+                {
+                        gsc_writeb( blockp[i]->command, LCD_CMD_REG );
+                        msleep(msec_cmd_delay);
+                        
+                        gsc_writeb( leds & mask[i] ? blockp[i]->on : 
+                                        blockp[i]->off, LCD_DATA_REG );
+                        msleep(msec_cmd_delay);
+                }
         }
 }
 
@@ -356,7 +363,7 @@ static __inline__ int led_get_net_activity(void)
 
         rx_total = tx_total = 0;
         
-        /* we are running as tasklet, so locking dev_base 
+        /* we are running as a workqueue task, so locking dev_base 
          * for reading should be OK */
         read_lock(&dev_base_lock);
         rcu_read_lock();
@@ -405,7 +412,7 @@ static __inline__ int led_get_diskio_activity(void)
         static unsigned long last_pgpgin, last_pgpgout;
         struct page_state pgstat;
         int changed;
-        
+
         get_full_page_state(&pgstat); /* get no of sectors in & out */
 
         /* Just use a very simple calculation here. Do not care about overflow,
@@ -413,86 +420,70 @@ static __inline__ int led_get_diskio_activity(void)
         changed = (pgstat.pgpgin != last_pgpgin) || (pgstat.pgpgout != last_pgpgout);
         last_pgpgin  = pgstat.pgpgin;
         last_pgpgout = pgstat.pgpgout;
-        
+
         return (changed ? LED_DISK_IO : 0);
 }
 
 
 
 /*
-   ** led_tasklet_func()
+   ** led_work_func()
    ** 
-   ** is scheduled at every timer interrupt from time.c and
-   ** updates the chassis LCD/LED 
+   ** manages when and which chassis LCD/LED gets updated
 
     TODO:
     - display load average (older machines like 715/64 have 4 "free" LED's for that)
     - optimizations
  */
 
-#define HEARTBEAT_LEN (HZ*6/100)
-#define HEARTBEAT_2ND_RANGE_START (HZ*22/100)
+#define HEARTBEAT_LEN (HZ*10/100)
+#define HEARTBEAT_2ND_RANGE_START (HZ*28/100)
 #define HEARTBEAT_2ND_RANGE_END   (HEARTBEAT_2ND_RANGE_START + HEARTBEAT_LEN)
 
-#define NORMALIZED_COUNT(count) (count/(HZ/100))
+#define LED_UPDATE_INTERVAL (1 + (HZ*19/1000))
 
-static void led_tasklet_func(unsigned long unused)
+static void led_work_func (void *unused)
 {
-        static unsigned char lastleds;
-        unsigned char currentleds; /* stores current value of the LEDs */
-        static unsigned long count; /* static incremented value, not wrapped */
+        static unsigned long last_jiffies;
         static unsigned long count_HZ; /* counter in range 0..HZ */
+        unsigned char currentleds = 0; /* stores current value of the LEDs */
 
         /* exit if not initialized */
         if (!led_func_ptr)
             return;
 
-        /* increment the local counters */
-        ++count;
-        if (++count_HZ == HZ)
+        /* increment the heartbeat timekeeper */
+        count_HZ += jiffies - last_jiffies;
+        last_jiffies = jiffies;
+        if (count_HZ >= HZ)
             count_HZ = 0;
 
-        currentleds = lastleds;
-
-        if (led_heartbeat)
-        {
-                /* flash heartbeat-LED like a real heart (2 x short then a long delay) */
-                if (count_HZ<HEARTBEAT_LEN || 
-                    (count_HZ>=HEARTBEAT_2ND_RANGE_START && count_HZ<HEARTBEAT_2ND_RANGE_END)) 
-                    currentleds |= LED_HEARTBEAT;
-                else
-                    currentleds &= ~LED_HEARTBEAT;
-        }
-
-        /* look for network activity and flash LEDs respectively */
-        if (led_lanrxtx && ((NORMALIZED_COUNT(count)+(8/2)) & 7) == 0)
+        if (likely(led_heartbeat))
         {
-                currentleds &= ~(LED_LAN_RCV | LED_LAN_TX);
-                currentleds |= led_get_net_activity();
+                /* flash heartbeat-LED like a real heart
+                 * (2 x short then a long delay)
+                 */
+                if (count_HZ < HEARTBEAT_LEN || 
+                                (count_HZ >= HEARTBEAT_2ND_RANGE_START &&
+                                count_HZ < HEARTBEAT_2ND_RANGE_END)) 
+                        currentleds |= LED_HEARTBEAT;
         }
 
-        /* avoid to calculate diskio-stats at same irq  as netio-stats */
-        if (led_diskio && (NORMALIZED_COUNT(count) & 7) == 0)
-        {
-                currentleds &= ~LED_DISK_IO;
-                currentleds |= led_get_diskio_activity();
-        }
+        if (likely(led_lanrxtx))  currentleds |= led_get_net_activity();
+        if (likely(led_diskio))   currentleds |= led_get_diskio_activity();
 
         /* blink all LEDs twice a second if we got an Oops (HPMC) */
-        if (oops_in_progress) {
+        if (unlikely(oops_in_progress)) 
                 currentleds = (count_HZ<=(HZ/2)) ? 0 : 0xff;
-        }
-        
-        /* update the LCD/LEDs */
-        if (currentleds != lastleds) {
-            led_func_ptr(currentleds);
-            lastleds = currentleds;
-        }
-}
 
-/* main led tasklet struct (scheduled from time.c) */
-DECLARE_TASKLET_DISABLED(led_tasklet, led_tasklet_func, 0);
+        if (currentleds != lastleds)
+        {
+                led_func_ptr(currentleds);      /* Update the LCD/LEDs */
+                lastleds = currentleds;
+        }
 
+        queue_delayed_work(led_wq, &led_task, LED_UPDATE_INTERVAL);
+}
 
 /*
    ** led_halt()
@@ -522,9 +513,13 @@ static int led_halt(struct notifier_block *nb, unsigned long event, void *buf)
         default:                return NOTIFY_DONE;
         }
         
-        /* completely stop the LED/LCD tasklet */
-        tasklet_disable(&led_tasklet);
-
+        /* Cancel the work item and delete the queue */
+        if (led_wq) {
+                cancel_rearming_delayed_workqueue(led_wq, &led_task);
+                destroy_workqueue(led_wq);
+                led_wq = NULL;
+        }
+ 
         if (lcd_info.model == DISPLAY_MODEL_LCD)
                 lcd_print(txt);
         else
@@ -559,7 +554,6 @@ int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long d
                 printk(KERN_INFO "LCD display at %lx,%lx registered\n", 
                         LCD_CMD_REG , LCD_DATA_REG);
                 led_func_ptr = led_LCD_driver;
-                lcd_print( lcd_text_default );
                 led_type = LED_HASLCD;
                 break;
 
@@ -589,9 +583,11 @@ int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long d
         initialized++;
         register_reboot_notifier(&led_notifier);
 
-        /* start the led tasklet for the first time */
-        tasklet_enable(&led_tasklet);
-        
+        /* Ensure the work is queued */
+        if (led_wq) {
+                queue_work(led_wq, &led_task);
+        }
+
         return 0;
 }
 
@@ -626,8 +622,8 @@ void __init register_led_regions(void)
    ** lcd_print()
    ** 
    ** Displays the given string on the LCD-Display of newer machines.
-   ** lcd_print() disables the timer-based led tasklet during its 
-   ** execution and enables it afterwards again.
+   ** lcd_print() disables/enables the timer-based led work queue to
+   ** avoid a race condition while writing the CMD/DATA register pair.
    **
  */
 int lcd_print( char *str )
@@ -637,12 +633,13 @@ int lcd_print( char *str )
         if (!led_func_ptr || lcd_info.model != DISPLAY_MODEL_LCD)
             return 0;
         
-        /* temporarily disable the led tasklet */
-        tasklet_disable(&led_tasklet);
+        /* temporarily disable the led work task */
+        if (led_wq)
+                cancel_rearming_delayed_workqueue(led_wq, &led_task);
 
         /* copy display string to buffer for procfs */
         strlcpy(lcd_text, str, sizeof(lcd_text));
-        
+
         /* Set LCD Cursor to 1st character */
         gsc_writeb(lcd_info.reset_cmd1, LCD_CMD_REG);
         udelay(lcd_info.min_cmd_delay);
@@ -656,8 +653,10 @@ int lcd_print( char *str )
             udelay(lcd_info.min_cmd_delay);
         }
         
-        /* re-enable the led tasklet */
-        tasklet_enable(&led_tasklet);
+        /* re-queue the work */
+        if (led_wq) {
+                queue_work(led_wq, &led_task);
+        }
 
         return lcd_info.lcd_width;
 }