Re: [PATCH v10] leds: USB: HID: Add support for MSI GT683R led panels

From: Johan Hovold
Date: Tue Jun 24 2014 - 10:50:46 EST


On Tue, Jun 24, 2014 at 03:10:43PM +0200, Bjørn Mork wrote:
> Johan Hovold <johan@xxxxxxxxxx> writes:
>
> > This is a non-standard attribute of this particular laptop. It has three
> > individual LEDs that can be enabled separately (using standard LED class
> > attributes), but they will all three be in the same "mode" (which here
> > apparently means that they can be fully on, vary with the volume(?!), or
> > pulse synchronously when enabled).
> >
> > If we were to implement this mode attribute as a class attribute,
> > changing the mode of of one LED would also change the mode of the other
> > two devices (LEDs).
>
> Document this behaviour and it becomes a feature.

Yeah, you're right. That is probably the best way to handle this.

Janne, could you send two separate patches on top of v9 that fixes the
use-before-initialisation (reported by Oliver) and moves the led_mode
attribute from the parent HID device to the led-class devices (thereby
fixing the race reported by Greg)?

As the "led_"-prefix will then be redundant, you should probably rename
the attribute as "msi_mode" or similar (just "mode" might be too
generic).

Remember to update the ABI documentation as well, including mentioning
the fact that changing the mode of one LED will update the mode of its
two sibling devices as well (as suggested by Bjørn).

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/