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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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