Re: Clarification regarding multicolor leds

[Alexander Dahl <post@lespocky.de>]

[-- Attachment #1: Type: text/plain, Size: 3840 bytes --]

Hello Luca,

On Sat, Oct 24, 2020 at 12:48:42AM +0200, Luca Weiss wrote:
> I'm currently experimenting with the qcom lpg[0] which is a driver for the rgb 
> notification led found on e.g. Snapdragon 801 devices (and many more), 
> specifically my example is about the Fairphone 2 (msm8974-fairphone-fp2).

Great to hear someone is interested in mainline support for Fairphone.
I just bought a used FP2 on ebay. :-)

> 
> [0] https://lore.kernel.org/lkml/20201021201224.3430546-1-bjorn.andersson@linaro.org/
> 
> My dts is looking like the following (abbreviated):
> 
>     [in lpg node]
>     multi-led {
>         color = <LED_COLOR_ID_MULTI>;
>         function = LED_FUNCTION_STATUS;
>         ....
>     };
> 
> I'm comparing this to the PinePhone where the leds are defined as follows:
> 
>     [in gpio-leds node]
>     blue {
>         function = LED_FUNCTION_INDICATOR;
>         color = <LED_COLOR_ID_BLUE>;
>     };
>     
>     green {
>         function = LED_FUNCTION_INDICATOR;
>         color = <LED_COLOR_ID_GREEN>;
>     };
>     
>     red {
>         function = LED_FUNCTION_INDICATOR;
>         color = <LED_COLOR_ID_RED>;
>     };
> 
> (sidenote: the LED_FUNCTION_INDICATOR should probably also be 
> LED_FUNCTION_STATUS there; the dts was made before the better descriptions for 
> the defines have been added)
> 
> This gets the following directories created in /sys/class/leds/:
> 
>     blue:indicator
>     green:indicator
>     red:indicator

That's right.  From Linux point of view these behave like three
independent LEDs.  It's fully up to userspace to handle this.

> 
> But with the multicolor led on the Fairphone 2 only a directory with the name 
> of "multi-led" gets created which I would have expected to be 
> "multicolor:status" instead.

Obviously it's named after the node label.  If I read
Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
correctly, that's how it is supposed to be named?

> 
> 
> What's further confusing me is that include/dt-bindings/leds/common.h states 
> (reformatted for clarity):
> 
> /* For multicolor LEDs */
> #define LED_COLOR_ID_MULTI	8
> 
> /* For multicolor LEDs that can do arbitrary color, so this would include RGBW 
> and similar */
> #define LED_COLOR_ID_RGB	9
> 
> It sounds like these comments are the wrong way around as it says that RGB 
> does RGBW & others while MULTI is normal RGB?
> 
> I have also found this commit[1] while browsing through lkml which seems to 
> validate my suspicions that _ID_RGB should be used normally? This commit seems 
> have been applied early September but hasn't been merged in the 5.10 merge 
> window?
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git/
> commit/?h=for-next&id=3d93edc77515c6f51fa9bbbe2185e2ec32bad024
> 
> But drivers/leds/led-core.c also states "We want to label LEDs that can 
> produce full range of colors as RGB, not multicolor" - not sure what "full 
> range" means here.
> 
> Thanks for reading through my long email and I'd appreciate any clarification 
> on the situation!

I just read
https://www.kernel.org/doc/html/latest/leds/leds-class-multicolor.html
and apart from formatting and inter-doc-link issues due to not used
markup features, I can understand how it's supposed to be used from
userspace.

However, multicolor is still quite new, maybe drivers/leds/TODO gives
some hints?  It seems to me some open issues are already known. ;-)

HTH & Greets
Alex

-- 
/"\ ASCII RIBBON | »With the first link, the chain is forged. The first
\ / CAMPAIGN     | speech censured, the first thought forbidden, the
 X  AGAINST      | first freedom denied, chains us all irrevocably.«
/ \ HTML MAIL    | (Jean-Luc Picard, quoting Judge Aaron Satie)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]