leds-lp5523: Broken commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0

leds-lp5523: Broken commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1070 bytes --]

Hello,

git commit "leds-lp5523: use generic firmware interface" 
db6eaf8388a413a5ee1b4547ce78506b9c6456b0 introduced in kernel 
3.10 changed user space API for modifing lp5523 led patterns via 
/sys.

Before this commit there were sysfs attributes engineX_mode, 
engineX_load and engineX_leds (for every engine X).

Now (after commit) there are sysfs attributes: select_engine, 
loading and data (see: 10c06d178df11b0b2b746321a80ea14241997127 
for description). Old sysfs attributes were removed.

So commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0 totally broke 
all existing userspace applications which use lp5523 leds.

This also broke application MCE which is used on Nokia N900 phone 
for changing LED patterns in lp5523 led.

I think that kernel api which is used by userspace applications 
(like attributes in /sys) should not be changed in way that 
totally broke existing applications.

Can you fix this api problem? (maybe adding old sysfs attributes 
which would call new api function...)

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

RE: leds-lp5523: Broken commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0

[Kim, Milo]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1407 bytes --]

> Hello,
> 
> git commit "leds-lp5523: use generic firmware interface"
> db6eaf8388a413a5ee1b4547ce78506b9c6456b0 introduced in kernel
> 3.10 changed user space API for modifing lp5523 led patterns via /sys.
> 
> Before this commit there were sysfs attributes engineX_mode, engineX_load and
> engineX_leds (for every engine X).
> 
> Now (after commit) there are sysfs attributes: select_engine, loading and
> data (see: 10c06d178df11b0b2b746321a80ea14241997127
> for description). Old sysfs attributes were removed.
> 
> So commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0 totally broke all existing
> userspace applications which use lp5523 leds.
> 
> This also broke application MCE which is used on Nokia N900 phone for
> changing LED patterns in lp5523 led.

First of all, I'm really sorry for this conflict.

> 
> I think that kernel api which is used by userspace applications (like
> attributes in /sys) should not be changed in way that totally broke existing
> applications.
> 
> Can you fix this api problem? (maybe adding old sysfs attributes which would
> call new api function...)

I'll recover old sysfs attributes and implement them by using new functions.
Additionally, I'll do same works in LP5521 driver also.

Best Regards,
Milo
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: leds-lp5523: Broken commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1483 bytes --]

On Monday 05 August 2013 09:26:46 Kim, Milo wrote:
> > Hello,
> > 
> > git commit "leds-lp5523: use generic firmware interface"
> > db6eaf8388a413a5ee1b4547ce78506b9c6456b0 introduced in
> > kernel 3.10 changed user space API for modifing lp5523 led
> > patterns via /sys.
> > 
> > Before this commit there were sysfs attributes engineX_mode,
> > engineX_load and engineX_leds (for every engine X).
> > 
> > Now (after commit) there are sysfs attributes:
> > select_engine, loading and data (see:
> > 10c06d178df11b0b2b746321a80ea14241997127 for description).
> > Old sysfs attributes were removed.
> > 
> > So commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0 totally
> > broke all existing userspace applications which use lp5523
> > leds.
> > 
> > This also broke application MCE which is used on Nokia N900
> > phone for changing LED patterns in lp5523 led.
> 
> First of all, I'm really sorry for this conflict.
> 
> > I think that kernel api which is used by userspace
> > applications (like attributes in /sys) should not be
> > changed in way that totally broke existing applications.
> > 
> > Can you fix this api problem? (maybe adding old sysfs
> > attributes which would call new api function...)
> 
> I'll recover old sysfs attributes and implement them by using
> new functions. Additionally, I'll do same works in LP5521
> driver also.
> 
> Best Regards,
> Milo

Hi, thank you very much!

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: leds-lp5523: Broken commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0

[Bryan Wu]

On Mon, Aug 5, 2013 at 12:31 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Monday 05 August 2013 09:26:46 Kim, Milo wrote:
>> > Hello,
>> >
>> > git commit "leds-lp5523: use generic firmware interface"
>> > db6eaf8388a413a5ee1b4547ce78506b9c6456b0 introduced in
>> > kernel 3.10 changed user space API for modifing lp5523 led
>> > patterns via /sys.
>> >
>> > Before this commit there were sysfs attributes engineX_mode,
>> > engineX_load and engineX_leds (for every engine X).
>> >
>> > Now (after commit) there are sysfs attributes:
>> > select_engine, loading and data (see:
>> > 10c06d178df11b0b2b746321a80ea14241997127 for description).
>> > Old sysfs attributes were removed.
>> >
>> > So commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0 totally
>> > broke all existing userspace applications which use lp5523
>> > leds.
>> >
>> > This also broke application MCE which is used on Nokia N900
>> > phone for changing LED patterns in lp5523 led.
>>
>> First of all, I'm really sorry for this conflict.
>>
>> > I think that kernel api which is used by userspace
>> > applications (like attributes in /sys) should not be
>> > changed in way that totally broke existing applications.
>> >
>> > Can you fix this api problem? (maybe adding old sysfs
>> > attributes which would call new api function...)
>>
>> I'll recover old sysfs attributes and implement them by using
>> new functions. Additionally, I'll do same works in LP5521
>> driver also.
>>
>> Best Regards,
>> Milo
>
> Hi, thank you very much!
>

Also my bad, I should noticed that and keep those old APIs at that moment.
Thanks for reporting this. I think Milo is going to fix it and I
probably pull this fix into 3.11 cycle instead of next merge window.

-Bryan