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] [thread-next>] [day] [month] [year] [list]

Date: Tue, 19 Dec 2023 10:03:59 +0100
From: Jan Kiszka <jan.kiszka@...mens.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
 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.23 09:48, Krzysztof Kozlowski wrote:
> On 19/12/2023 09:22, Jan Kiszka wrote:
>>>
>>>> +			gpios = <&wkup_gpio0 53 GPIO_ACTIVE_HIGH>;
>>>
>>> Ditto
>>>
>>
>> This is adjusting the existing LED nodes in k3-am65-iot2050-common.dtsi,
>> not introducing new ones. We can add the color properties in a separate
> 
> 
> Then why aren't you overriding by phandle/label?
> 

We could do that as well if we added labels first (they don't exist so 
far). Not seeing any difference, though.

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

root@...2050-debian:~# ls -l /sys/class/leds/
total 0
lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc0:: -> ../../devices/platform/bus@...000/4fa0000.mmc/leds/mmc0::
lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc1:: -> ../../devices/platform/bus@...000/4f80000.mmc/leds/mmc1::
lrwxrwxrwx 1 root root 0 Dec 14 21:12 status-led-green -> ../../devices/platform/leds/leds/status-led-green
lrwxrwxrwx 1 root root 0 Dec 19 08:55 status-led-red -> ../../devices/platform/leds/leds/status-led-red
lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-green -> ../../devices/platform/leds/leds/user-led1-green
lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-red -> ../../devices/platform/leds/leds/user-led1-red
lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-green -> ../../devices/platform/leds/leds/user-led2-green
lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-red -> ../../devices/platform/leds/leds/user-led2-red

>>>> +
>>>> +&dwc3_0 {
>>>> +	assigned-clock-parents = <&k3_clks 151 4>,  /* set REF_CLK to 20MHz i.e. PER0_PLL/48 */
>>>> +				 <&k3_clks 151 9>;  /* set PIPE3_TXB_CLK to CLK_12M_RC/256 (for HS only) */
>>>> +	/delete-property/ phys;
>>>> +	/delete-property/ phy-names;
>>>
>>> If your board need to remove phys from the SoC node, something is wrong.
>>> Either your board or SoC.
>>>
>>> Any removal of properties in DTS is weird and unexpected. It deserves
>>> comments.
>>
>> This goes along disabling USB3 which is by default enabled via
>> k3-am65-iot2050-common-pg2.dtsi
> 
> Isn't this mistake? Common part enables only these pieces which are
> working in common hardware SoM. If your common part of hardware, which
> DTSI should represent, has USB3 then why is it being disabled here? If
> common hardware design does not have USB3, then why is it being enabled
> in DTSI?

It's a trade-off between adding yet another dtsi for those widely 
common bits vs. adjusting the differences of only one variant from 
that. We do the same for the Display Port so far.

Jan

-- 
Siemens AG, Technology
Linux Expert Center