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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 19 Dec 2023 16:56:01 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Jan Kiszka <jan.kiszka@...mens.com>, Nishanth Menon <nm@...com>,
 Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>,
 Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Bao Cheng Su <baocheng.su@...mens.com>,
 Chao Zeng <chao.zeng@...mens.com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, Li Hua Qian <huaqian.li@...mens.com>
Subject: Re: [PATCH 4/4] dts: iot2050: Support IOT2050-SM variant

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@...2050-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@...2050-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