Re: [PATCH 5/5] ARM: dts: qcom: msm8974-oneplus-bacon: Add notification LED

Krzysztof Kozlowski
2023-01-27T08:27:16Z

On 26/01/2023 21:41, Luca Weiss wrote:
> On Donnerstag, 26. Jänner 2023 10:41:24 CET Krzysztof Kozlowski wrote:
>> On 25/01/2023 21:51, Luca Weiss wrote:
>>> On Montag, 23. Jänner 2023 18:18:16 CET Krzysztof Kozlowski wrote:
>>>> On 22/01/2023 17:48, Luca Weiss wrote:
>>>>> Add the node describing the sn3193 that's used to provide notification
>>>>> LED.
>>>>>
>>>>> Unfortunately the driver currently supports neither multicolor API nor
>>>>> using the properties function & color, so we use label instead.
>>>>>
>>>>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
>>>>> ---
>>>>>
>>>>> .../arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts | 28
>>>>> ++++++++++++++++++++++ 1 file changed, 28 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts
>>>>> b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts index
>>>>> ffb486ceb6a6..a672c45d7070 100644
>>>>> --- a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts
>>>>> +++ b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts
>>>>> @@ -114,6 +114,34 @@ led@0 {
>>>>>
>>>>> default-brightness = <80>;
>>>>>
>>>>> };
>>>>>
>>>>> };
>>>>>
>>>>> +
>>>>> + led-controller@68 {
>>>>> + compatible = "si-en,sn3193";
>>>>> + reg = <0x68>;
>>>>> +
>>>>> + shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>;
>>>>> +
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> +
>>>>> + led@1 {
>>>>> + reg = <1>;
>>>>> + label = "red:status";
>>>>
>>>> These should be colors and functions.
>>>
>>> Hi Krzysztof,
>>>
>>> please check the commit message for this. I tried using it but it didn't
>>> work, the driver in general might need an overhaul at some point...
>>>
>>>> Unfortunately the driver currently supports neither multicolor API nor
>>>> using the properties function & color, so we use label instead.
>>>
>>> Or maybe I messed up but pretty sure this was the case when I wrote the
>>> dts.
>> I don't understand how driver is related here - it's not the driver's
>> property, but LED core. If the driver "supports" label, then it supports
>> color and status.
>
> The leds-is31fl319x.c driver uses devm_led_classdev_register but the function
> where 'function' and 'color' is parsed is led_parse_fwnode_props which is
> called from led_compose_name which is called from led_classdev_register_ext
> (or with the devm_ prefix), and not called from the register method without
> _ext suffix.

No, devm_led_classdev_register calls devm_led_classdev_register_ext
which calls led_classdev_register_ext which calls led_compose_name (if
initdata is set). The point is that we lack initdata, so indeed color
and function cannot be used.

>
> So in other words, the led driver would need changes to use the new
> registration API to be able to use 'function' and 'color' instead of 'label'.
>
> Regards
> Luca
>
>>
>> Best regards,
>> Krzysztof
>
>
>
>

Best regards,
Krzysztof