Re: [PATCH v2 2/3] arm64: dts: ti: Introduce J742S2 SoC family

[Andrew Davis <afd@ti.com>]

On 7/31/24 9:58 AM, Manorit Chawdhry wrote:
> Hi Andrew,
> 
> On 09:37-20240731, Andrew Davis wrote:
>> On 7/31/24 8:57 AM, Manorit Chawdhry wrote:
>>> Hi Nishanth,
>>>
>>> On 06:06-20240731, Nishanth Menon wrote:
>>>> On 09:49-20240731, Manorit Chawdhry wrote:
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include "k3-j784s4.dtsi"
>>>>>>> +
>>>>>>> +/ {
>>>>>>> +	model = "Texas Instruments K3 J742S2 SoC";
>>>>>>> +	compatible = "ti,j742s2";
>>>>>>> +
>>>>>>> +	cpus {
>>>>>>> +		cpu-map {
>>>>>>> +			/delete-node/ cluster1;
>>>>>>> +		};
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	/delete-node/ cpu4;
>>>>>>> +	/delete-node/ cpu5;
>>>>>>> +	/delete-node/ cpu6;
>>>>>>> +	/delete-node/ cpu7;
>>>>>>
>>>>>> I suggest refactoring by renaming the dtsi files as common and split out
>>>>>> j784s4 similar to j722s/am62p rather than using /delete-node/
>>>>>>
>>>>>
>>>>> I don't mind the suggestion Nishanth if there is a reason behind it.
>>>>> Could you tell why we should not be using /delete-node/?
>>>>>
>>>>
>>>> Maintenance, readability and sustenance are the reasons. This is a
>>>> optimized die. It will end up having it's own changes in property
>>>> and integration details. While reuse is necessary, modifying the
>>>> properties with overrides and /delete-nodes/ creates maintenance
>>>> challenges down the road. We already went down this road with am62p
>>>> reuse with j722s, and eventually determined split and reuse is the
>>>> best option. See [1] for additional guidance.
>>>>
>>>>
>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n189
>>>
>>> Thank you for giving some reasoning, would do the needful!
>>>
>>
>> This refactor will require some interesting naming for the
>> common SoC files. Based on your name for the EVM, I'm guessing
>> you will go with
> 
> One other reason I was trying to avoid that and going with
> /delete-node/. For such a small delta change tbh, this churn doesn't
> feel worth the effort to me and is just gonna create confusion.
> 
> EVM one was required as Rob did raise an interesting point and we did
> require a soc file that wasn't existing with the previous patchset but
> now for deleting just 4 cpus and 1 dsp, am gonna have to rename all the
> files, change the hierarchical structure, add all the cpus again with
> some weird naming for the file as don't know if some other soc is gonna
> come up in future so don't wanna clutter the file names as well with
> j784s4-j742s2-j7xxx.dtsi which is just gonna create another set of mess
> in future.
> 

Which is why I would suggesting getting the name picked and agreed on
here before you start doing the renames (renames for .dtsi files are not
a problem, only the final .dtb names seem to require stability as the
bootloader tend to load them by name, and those are not changing)

What is wrong with just k3-j784s4-common.dtsi? All future spins of
this base device can include from this file. Every spin doesn't need
to be in the common file's name.

Andrew

> Regards,
> Manorit
> 
>>
>> k3-j784s4-common.dtsi
>>
>> included from the real k3-j784s4.dtsi and the new k3-j742s2.dtsi?
>>
>> Too bad the Jacinto SoC names don't use a hierarchical naming. :(
>>
>> J7<family><part><spin><etc>..
>>
>> Andrew
>>
>>> Regards,
>>> Manorit
>>>
>>>>
>>>> -- 
>>>> Regards,
>>>> Nishanth Menon
>>>> Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D
>>>