Re: [PATCH 4/4] dts: iot2050: Support IOT2050-SM variant

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

On 19/12/2023 16:48, Jan Kiszka wrote:
> On 19.12.23 16:42, Krzysztof Kozlowski wrote:
>> On 19/12/2023 16:40, Jan Kiszka wrote:
>>> On 19.12.23 16:39, Krzysztof Kozlowski wrote:
>>>> On 19/12/2023 16:37, Jan Kiszka wrote:
>>>>>>>>
>>>>>>>> You have label for that... Somehow all these nodes are half-baked,
>>>>>>>> without all the expected properties and now you call node name as ABI.
>>>>>>>> The node name is not the ABI.
>>>>>>>
>>>>>>> Well, existing userspace uses those names, and adding the properties
>>>>>>> would break that interface. Now, does Linux do that?
>>>>>>
>>>>>> I don't think you understood the concept. There is no change for
>>>>>> userspace. Same interface, same names. No ABI break.
>>>>>
>>>>> I do understand the impact very well:
>>>>> open("/sys/class/leds/user-led1-red") has to work for all the variants,
>>>>> consistently and backward-compatible for userspace.
>>>>
>>>> And it will. The name is the same.
>>>
>>> Nope, it's not - I tried that already :)
>>>
>>> root@iot2050-debian:~# ls -l /sys/class/leds/
>>> total 0
>>> lrwxrwxrwx 1 root root 0 Dec 19 09:49 green:indicator -> ../../devices/platform/leds/leds/green:indicator
>>
>> And how does your DTS look like?
>>
>> Because I also tried and it is exactly the same.
>>
> 
> I played with
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> index 402afa4bc1b6..a791444eeb93 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> @@ -10,6 +10,7 @@
>   */
>  
>  #include "k3-am654.dtsi"
> +#include <dt-bindings/leds/common.h>
>  #include <dt-bindings/phy/phy.h>
>  #include <dt-bindings/net/ti-dp83867.h>
>  
> @@ -84,27 +85,39 @@ leds {
>  		pinctrl-0 = <&leds_pins_default>;
>  
>  		status-led-red {
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_STATUS;
>  			gpios = <&wkup_gpio0 32 GPIO_ACTIVE_HIGH>;
>  			panic-indicator;

And where is the label property?

Please read my message again:

>> You:
>> patch, but the node names are now part of the kernel ABI. Changing
them would break existing userland.
> Me:
> You mean label. Why node names became the ABI? Which interface exposes
them?


>> You:
>> root@iot2050-debian:~# ls -l /sys/class/leds/
> Me:
> I replied too fast previous and did not include answer here:
> You have label for that...

So again: The stable ABI is fulfilled by using label property. Not the
Devicetree "label" phandle in front of the node, but the dedicated property.

Best regards,
Krzysztof