Messages in this thread Patch in this message | | | Date | Fri, 8 Nov 2013 14:15:48 +0900 | From | Milo Kim <> | Subject | Re: [PATCH 00/10] leds: lp5521,5523: restore device attributes for running LED patterns |
| |
Hi Pali,
Sorry for the late reply.
On 10/26/2013 03:21 AM, Pali Rohár wrote: > On Friday 25 October 2013 19:10:07 Bryan Wu wrote: >> On Fri, Oct 25, 2013 at 9:38 AM, Pali Rohár > <pali.rohar@gmail.com> wrote: >>> On Tuesday 13 August 2013 23:04:14 Bryan Wu wrote: >>>> On Thu, Aug 8, 2013 at 12:59 AM, Milo Kim > <woogyom.kim@gmail.com> wrote: >>>>> This patch-set resolves the application conflict by >>>>> restoring sysfs files. >>>>> >>>>> For LP5521 >>>>> >>>>> engine1/2/3_mode >>>>> engine1/2/3_load >>>>> >>>>> For LP5523 >>>>> >>>>> engine1/2/3_mode >>>>> engine1/2/3_load >>>>> engine1/2/3_leds >>>>> >>>>> Those attributes are accessed when LED pattern is run by >>>>> custom application. Those were removed when LED pattern >>>>> interface was changed to generic firmware interface. >>>>> Please refer to commits below. >>>>> >>>>> git commit 9ce7cb170f97f83a78dc948bf7d25690f15e1328 >>>>> (leds-lp5521: use generic firmware interface) >>>>> >>>>> git commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0 >>>>> (leds-lp5523: use generic firmware interface) >>>>> >>>>> Necessary attributes are restored in this patch-set. >>>>> >>>>> (Other changes) >>>>> New data structure is added for handling values from/to >>>>> an application. Few code fixes for reducing writing I2C >>>>> commands. >>>>> Add LP55xx common macros for code refactoring. >>>>> Documentation updates. >>>>> >>>>> You can also pull from the location below >>>>> This branch is based on 'for-next' of linux-leds. >>>>> >>>>> https://github.com/milokim/lp55xx.git >>>>> resolve-missing-sysfs >>>> >>>> Thanks, I've already merged the whole patchset in my -devel >>>> branch [1]. >>>> >>>> Pali, could you please help to test it on your hardware? >>>> Just grab my -devel branch and build then run. >>>> >>>> Thanks, >>>> -Bryan >>> >>> Hi, I see that all your patches are part of 3.12-rc5 kernel. >>> >>> Now I tested this example led program: >>> # Clearing LED-state to be sure >>> echo "disabled" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine1_mode echo >>> "disabled" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine2_mode echo 0 >>> > /sys/class/leds/lp5523:r/brightness >>> echo 0 > /sys/class/leds/lp5523:g/brightness >>> echo 0 > /sys/class/leds/lp5523:b/brightness >>> >>> # Setting yellow light pattern and running it >>> echo "load" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine1_mode echo >>> "000001100" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine1_leds echo >>> "9d804000427f0d7f7f007f0042000000" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine1_load echo >>> "load" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine2_mode echo >>> "000000000" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine2_leds echo >>> "9d800000" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine2_load echo >>> "run" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine2_mode echo >>> "run" > >>> /sys/class/i2c-adapter/i2c-2/2-0032/engine1_mode echo >>> 20 > /sys/class/leds/lp5523:r/led_current >>> echo 2 > /sys/class/leds/lp5523:g/led_current >>> echo 0 > /sys/class/leds/lp5523:b/led_current
In the latest LP5523 driver, I just found wrong operation mode setting in case of multiple engine used. (please find a patch below)
>>> >>> All sysfs entries exists and every echo returned 0. >>> >>> But led does not start blinking that yellow ligh pattern. >>> So it not working on 3.12-rc5 kernel :-( >> >> OK, great! Do you still remember which kernel version works on >> you system? Milo, do you have time to take a look? I bet it's >> a regression somewhere. >> >> Thanks, >> -Bryan > > I do not know which version. Now I tried pattern example from > Documentation/leds/leds-lp55xx.txt which using new API. > > echo 2 > /sys/class/i2c-adapter/i2c-2/2-0032/select_engine > echo 1 > /sys/class/firmware/lp5523/loading > echo "9d80400004ff05ff437f0000" > /sys/class/firmware/lp5523/data > echo 0 > /sys/class/firmware/lp5523/loading > echo 1 > /sys/class/i2c-adapter/i2c-2/2-0032/run_engine > > But it failed at second command. In directory /sys/class/firmware/ > I have only one file with name timeout. Nothing more, no lp5523 > folder.
I can't reproduce this issue but I found some duplicated mutex problem. But in my case, I could found /sys/class/firmware/lp5523 directory. Is any permission issue? Anyway, I've added fixed mutex code below.
Could you check this patch below? If it's resolved, then I'll send official patch-set.
-------------------- 8< ---------------- 8< ---------------------------
From dad27e0d834a5635232116569aaa1fa471ac46bf Mon Sep 17 00:00:00 2001 From: Milo Kim <milo.kim@ti.com> Date: Fri, 8 Nov 2013 13:43:33 +0900 Subject: [PATCH] leds: lp5523: Fix multiple engine usage bug and remove duplicated mutex
Fix multiple engine usage bug: Whenever the engine is loaded by the user-application, the operation mode is reset first. But it has a problem in case of multiple engine used because previous engine settings are cleared. The driver should update not whole 8bits but each engine bits by masking.
Remove duplicate mutex: It can be a problem when a pattern is loaded via the firmware interface. LP55xx common driver has already locked the mutex in 'lp55xx_firmware_loaded()'. So it should be deleted.
Signed-off-by: Milo Kim <milo.kim@ti.com> --- drivers/leds/leds-lp5523.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c index fe3bcbb..c00f55e 100644 --- a/drivers/leds/leds-lp5523.c +++ b/drivers/leds/leds-lp5523.c @@ -188,7 +188,15 @@ static void lp5523_load_engine_and_select_page(struct lp55xx_chip *chip)
static void lp5523_stop_engine(struct lp55xx_chip *chip) { - lp55xx_write(chip, LP5523_REG_OP_MODE, 0); + enum lp55xx_engine_index idx = chip->engine_idx; + u8 mask[] = { + [LP55XX_ENGINE_1] = LP5523_MODE_ENG1_M, + [LP55XX_ENGINE_2] = LP5523_MODE_ENG2_M, + [LP55XX_ENGINE_3] = LP5523_MODE_ENG3_M, + }; + + lp55xx_update_bits(chip, LP5523_REG_OP_MODE, mask[idx], 0); + lp5523_wait_opmode_done(); }
@@ -336,8 +344,6 @@ static int lp5523_update_program_memory(struct lp55xx_chip *chip, if (i % 2) goto err;
- mutex_lock(&chip->lock); - for (i = 0; i < LP5523_PROGRAM_LENGTH; i++) { ret = lp55xx_write(chip, LP5523_REG_PROG_MEM + i, pattern[i]); if (ret) { @@ -346,8 +352,6 @@ static int lp5523_update_program_memory(struct lp55xx_chip *chip, } }
- mutex_unlock(&chip->lock); - return size;
err: -- 1.7.9.5 -------------------- 8< ---------------- 8< --------------------------- Best regards, Milo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |