Re: [PATCH v1 2/3] arm64: dts: ti: Add Aquila AM69 Support

[Vignesh Raghavendra <vigneshr@ti.com>]

On 10/11/25 15:17, Francesco Dolcini wrote:
> Hi Vignesh,
> 
> On Mon, Nov 10, 2025 at 10:06:58AM +0530, Vignesh Raghavendra wrote:
>> On 06/11/25 15:49, Francesco Dolcini wrote:
>>>>> Yes, known. Is this an issue?
>>>>>
>>>>> This node must be disabled, no matter what is present in any included
>>>>> dtsi file, it's a deliberate decision.
>>>>>
>>>>> This dtsi file describes a SoM, the used pins/functions are defined on
>>>>> the pinout, but this node cannot be enabled unless the SoM is mated with
>>>>> a carrier board that is exposing it.
>>>> Same as my point above, you shouldn't enable nodes that are not used
>>>> or have anything attached. The SoM only has some edge connectors so
>>>> it should not be enabled at the SoM level, that we seem to agree, but
>>>> the carrier board doesn't connect those lines to anything either. They
>>>> just run to a pin header with nothing attached, how is that header
>>>> any different than the pins on the edge of the SoM?
>>> You are commenting something unrelated here, or I am not understanding
>>> you.
>>>
>>> You commented that the status = "disabled" is redundant. We both agree
>>> that this node needs to be disabled in the SoM dtsi, and it is already
>>> like that.
>>>
>>> I would prefer to keep the redundant "disabled", because I see value on
>>> not having to rely on what is done on any included dtsi, where the
>>> original node is defined. 
>>
>>
>> One can always reverse compile the DTB to see if a node is enabled or not.
> 
> Sure. My reason for having it explicitly disabled here is not to have to
> depend on anything that is happening on the included dtsi on this
> regard, given that we really want those disabled in the SoM.
> 
> See for example https://lore.kernel.org/all/20251015111344.3639415-1-s-vadapalli@ti.com/
> where you can see that our verdin am62 was not impacted by this change
> at all. 
> 
> To me this is just a benefit, without any drawback.
> 
>>> I see this as a common pattern in multiple
>>> dts/dtsi file and is what I would prefer to have (and I do not see any
>>> kind of maintenance  overhead on having it nor this being in conflict
>>> with dts-coding-style.rst).
>>
>> I cannot seem to find any precedence to such a pattern (adding status =
>> "disabled" for nodes that are already disabled at SoC level dtsi.) Could
>> you point me to some?
> 
> Just a couple of examples, you can easily find more if needed
> 
>   k3-am62-verdin.dtsi:&main_spi1
>   nxp/imx/imx6qdl-phytec-phycore-som.dtsi:&fec
> 
> 
>>> Vignesh, Nishanth, what is your expectation on this redundant
>>> `status = "disabled"` property?
>>> 	
>>
>> Assuming such pattern exists, please add a note in the commit message in
>> the next version.
> 
> With that said, it's a small thing, if you prefer me to change it just
> let me know and I'll send a v2 with this changed.
> 
> Vignesh: do you prefer a v2 with a mention in the commit message on the
> `disabled` node, or that I just get rid of those? Anything else that you
> spotted on this patch and that should be changed?
> 

Please send v2 with line added in commit message.

PS: I fully except someone to send a patch cleaning up all the double
disables without knowing the background :)


> Francesco
> 

-- 
Regards
Vignesh
https://ti.com/opensource