Hi!

There's some locking weirdness, and few missing comments in lp5523
driver.

Now, this is untested patch from my reverse-engineering. I hope I
understood things right...

In particular, there's unbalanced unlock in
lp5523_update_program_memory, and lp5523_update_program_memory needs
to be protected by the lock.

Comments? Does someone maintain this?

Thanks,
                                                                Pavel
Signed-off-by: Pavel Machek <pa...@ucw.cz>


diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
index c00f55e4..e8b83e9 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -34,7 +34,15 @@
 
 #include "leds-lp55xx-common.h"
 
-#define LP5523_PROGRAM_LENGTH          32
+#define LP5523_PROGRAM_LENGTH          32      /* bytes */
+/* Memory is used like this:
+   0x00 engine 1 program
+   0x10 engine 2 program
+   0x20 engine 3 program
+   0x30 engine 1 muxing info
+   0x40 engine 2 muxing info
+   0x50 engine 3 muxing info
+   ...and offsets are hard-coded all around :-( */
 #define LP5523_MAX_LEDS                        9
 
 /* Registers */
@@ -265,20 +273,25 @@ static int lp5523_init_program_engine(struct lp55xx_chip 
*chip)
        /* one pattern per engine setting LED MUX start and stop addresses */
        static const u8 pattern[][LP5523_PROGRAM_LENGTH] =  {
                { 0x9c, 0x30, 0x9c, 0xb0, 0x9d, 0x80, 0xd8, 0x00, 0},
+               /* 9c30 -- mux_map_start(0x30)
+                  9cb0 -- mux_ld_end(0x50)
+                  9d80 -- mux_sel???(0x00)
+                  d800 -- invalid?? */
                { 0x9c, 0x40, 0x9c, 0xc0, 0x9d, 0x80, 0xd8, 0x00, 0},
                { 0x9c, 0x50, 0x9c, 0xd0, 0x9d, 0x80, 0xd8, 0x00, 0},
        };
 
-       /* hardcode 32 bytes of memory for each engine from program memory */
+       /* hardcode LP5523_PROGRAM_LENGTH bytes of memory for each
+          engine from program memory */
        ret = lp55xx_write(chip, LP5523_REG_CH1_PROG_START, 0x00);
        if (ret)
                return ret;
 
-       ret = lp55xx_write(chip, LP5523_REG_CH2_PROG_START, 0x10);
+       ret = lp55xx_write(chip, LP5523_REG_CH2_PROG_START, 
LP5523_PROGRAM_LENGTH / 2);
        if (ret)
                return ret;
 
-       ret = lp55xx_write(chip, LP5523_REG_CH3_PROG_START, 0x20);
+       ret = lp55xx_write(chip, LP5523_REG_CH3_PROG_START, 
LP5523_PROGRAM_LENGTH);
        if (ret)
                return ret;
 
@@ -346,10 +359,8 @@ static int lp5523_update_program_memory(struct lp55xx_chip 
*chip,
 
        for (i = 0; i < LP5523_PROGRAM_LENGTH; i++) {
                ret = lp55xx_write(chip, LP5523_REG_PROG_MEM + i, pattern[i]);
-               if (ret) {
-                       mutex_unlock(&chip->lock);
+               if (ret)
                        return -EINVAL;
-               }
        }
 
        return size;
@@ -551,15 +562,17 @@ static ssize_t store_engine_load(struct device *dev,
 {
        struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
        struct lp55xx_chip *chip = led->chip;
+       ssize_t res;
 
        mutex_lock(&chip->lock);
 
        chip->engine_idx = nr;
        lp5523_load_engine_and_select_page(chip);
 
+       res = lp5523_update_program_memory(chip, buf, len);
        mutex_unlock(&chip->lock);
 
-       return lp5523_update_program_memory(chip, buf, len);
+       return res;
 }
 store_load(1)
 store_load(2)
diff --git a/drivers/leds/leds-lp8501.c b/drivers/leds/leds-lp8501.c
index 8d55a780..ae5c6fe 100644
--- a/drivers/leds/leds-lp8501.c
+++ b/drivers/leds/leds-lp8501.c
@@ -262,7 +262,7 @@ static void lp8501_firmware_loaded(struct lp55xx_chip *chip)
        }
 
        /*
-        * Program momery sequence
+        * Program memory sequence
         *  1) set engine mode to "LOAD"
         *  2) write firmware data into program memory
         */

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to