Hi Alex, On Samstag, 24. Oktober 2020 08:42:38 CEST Alexander Dahl wrote: > 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@lin > > aro.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. Exactly, that I understand well. > > > 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? > That's from the i read from the documentation as well. How is user space supposed to get function and/or color from the led? I don't see it exposed in user space - apart from the directory name (label) in the leds-gpio example. This is what I get in sysfs for the multicolor led with the lpg driver: brightness device -> ../../../fc4cf000.spmi:pm8941@1:lpg max_brightness multi_index multi_intensity power subsystem -> ../../../../../../../../../class/leds trigger uevent > > 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? > > Based on Pavel's reply my RGB LED should be _ID_RGB. It would be great if this comment would be clarified because honestly still don't know what "arbitrary color" should mean; and add an example to the _ID_MULTI one that it only should be used for "strange stuff like Blue-UV combination". > > 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. > Even in the doc the directory name is "multicolor:status" exposing color & function.. Curiously when setting the (deprecated) property "label" to "rgb:status" the directory name in sysfs becomes rgb:status - so is it a bug that this doesn't happen automatically? Feels like it at least. I've looked into the code yesterday a bit and found that "struct led_init_data *init_data" in led_classdev_register_ext is always passed as NULL for multicolor leds from devm_led_classdev_multicolor_register which seems to cause this to happen. > 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 Regards Luca
