| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <527C7384.1040103@ti.com>
Date: Fri, 8 Nov 2013 14:15:48 +0900
From: Milo Kim <milo.kim@...com>
To: Pali Rohár <pali.rohar@...il.com>
CC: Bryan Wu <cooloney@...il.com>, Milo Kim <woogyom.kim@...il.com>,
Linux LED Subsystem <linux-leds@...r.kernel.org>,
lkml <linux-kernel@...r.kernel.org>
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@...il.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@...il.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@...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@...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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists