[PATCH] HID: hid-sony: Prevent LED overrun

[PATCH] HID: hid-sony: Prevent LED overrun

[Simon Wood]

If the value written to led1 is larger than 1 (for SixAxis or Intec controllers),
it can effect other leds on the device.

For example:
# echo 3 > /sys/class/leds/0003\:054C\:0268.0013\:\:sony1/brightness

turns on both led1 and led2, led2 does not then behave as expected through it's
own interface.

Patch limits the LEDs 'value' to the 'max brightness', thus preventing bug.

Tested with SixAxis DS3, DS4 and Intec (3rd party) controllers, via USB connection.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-sony.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 31e9d25..385fa1f 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1296,6 +1296,9 @@ static void sony_led_set_brightness(struct led_classdev *led,
 			drv_data->led_delay_on[n] ||
 			drv_data->led_delay_off[n]))) {
 
+			if (value > led->max_brightness)
+				value = led->max_brightness;
+
 			drv_data->led_state[n] = value;
 
 			/* Setting the brightness stops the blinking */
-- 
1.9.1

Re: [PATCH] HID: hid-sony: Prevent LED overrun

[Frank Praznik]

> If the value written to led1 is larger than 1 (for SixAxis or Intec
> controllers),
> it can effect other leds on the device.
> For example:
> # echo 3 > /sys/class/leds/0003\:054C\:0268.0013\:\:sony1/brightness
>
> turns on both led1 and led2, led2 does not then behave as expected through it's
> own interface.
>
> Patch limits the LEDs 'value' to the 'max brightness', thus preventing bug.
>
> Tested with SixAxis DS3, DS4 and Intec (3rd party) controllers, via USB
> connection.
>
> Signed-off-by: Simon Wood <si...@mungewell.org>
> ---
>   drivers/hid/hid-sony.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 31e9d25..385fa1f 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -1296,6 +1296,9 @@ static void sony_led_set_brightness(struct led_classdev
> *led,
>                          drv_data->led_delay_on[n] ||
>                          drv_data->led_delay_off[n]))) {
>   
> +                       if (value > led->max_brightness)
> +                               value = led->max_brightness;
> +
>                          drv_data->led_state[n] = value;
>   
>                          /* Setting the brightness stops the blinking */
> -- 
> 1.9.1

Hi Simon,

I think this was a bug in the LED system itself fixed by this commit:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/leds/leds.h?id=56d06fdee534124f79b51ff92232373b783cddc2

In kernel 3.19 the brightness values are clamped correctly and I can't 
replicate your bug.

Regards,
Frank

Re: [PATCH] HID: hid-sony: Prevent LED overrun

[simon]

> I think this was a bug in the LED system itself fixed by this commit:
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/leds/leds.h?id=56d06fdee534124f79b51ff92232373b783cddc2
>
> In kernel 3.19 the brightness values are clamped correctly and I can't
> replicate your bug.

I guess it's possible, I am running 3.19.0rc5. I'll build 3.19 and see if
it's showing the bug.

Simon

Re: [PATCH] HID: hid-sony: Prevent LED overrun

[simon]

> On Feb 18, 2015 12:32 AM, <simon@mungewell.org> wrote:
>>
>>
>> > I think this was a bug in the LED system itself fixed by this commit:
>> >
>> >
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/leds/leds.h?id=56d06fdee534124f79b51ff92232373b783cddc2
>> >
>> > In kernel 3.19 the brightness values are clamped correctly and I can't
>> > replicate your bug.
>>
>> I guess it's possible, I am running 3.19.0rc5. I'll build 3.19 and see
>> if it's showing the bug.
>>
>
> Ah, my mistake.  That fix wasn't pulled in until 3.20 so you'll need to
> grab the latest working version to try it.
>

Well I did a 'git pull' and left it building overnight, and only just saw
this email...

A quick test this morning shows that the bug does not occur with the new
build, so it looks like my patch is not required after all (and fixing in
LED subsystem makes far more sense).

I am, however, confused about what I pulled and built. It definately had
the patch (I looked in drivers/leds/leds.h before building) but built as
3.19.... guess I'll just have a confused expression for the rest of the
day.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/drivers/leds/leds.h

Simon.