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

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

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?

> 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?

> 
>>
>>> +
>>> +&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?
> 

Best regards,
Krzysztof