Re: [PATCH 01/25] ARM: tegra: Clean up external memory controller nodes

Dmitry Osipenko
2021-12-10T15:15:33Z

10.12.2021 15:17, Thierry Reding пишет:
> On Thu, Dec 09, 2021 at 11:01:10PM +0300, Dmitry Osipenko wrote:
>> 09.12.2021 20:33, Thierry Reding пишет:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> The external memory controller should be sorted after the memory
>>> controller to keep the ordering by unit-address intact.
>>
>> "sorted after the memory controller, I don't understand what this means.
>> Please clarify.
>
> In device tree we sort nodes by unit-address. In these files the
> external memory controller device tree nodes (which have a higher unit-
> address than the memory controller device tree nodes) was listed before
> the memory controller device tree node. This fixes the order by sorting
> the nodes correctly.
>
> Guess "sort after" doesn't really exist, but I thought it'd be clear
> enough in the context to avoid using so many words. Guess I was wrong.

Now I see what you did, thanks.

>>> While at it,
>>> rename the emc-timings and timing nodes to avoid including the RAM code
>>> and clock frequency in their names. There is no requirement to do this,
>>> so we can use simple enumerations instead.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>> arch/arm/boot/dts/tegra124-apalis-emc.dtsi | 483 +++--
>>> .../arm/boot/dts/tegra124-jetson-tk1-emc.dtsi | 699 +++---
>>> arch/arm/boot/dts/tegra124-nyan-big-emc.dtsi | 1918 +++++++++--------
>>> .../arm/boot/dts/tegra124-nyan-blaze-emc.dtsi | 639 +++---
>>> 4 files changed, 1900 insertions(+), 1839 deletions(-)
>>
>> It's very suspicious that you changed only T124. This all doesn't look
>> good to me.
>>
>> Please either explain it all or drop this patch.
>
> Well, on other SoCs the order of the MC vs. EMC was correct, so I didn't
> have to touch those and therefore didn't fix up the numbering while at
> it. What exactly is suspicous about that?
>
> I'll go and update all of those files as well if that makes you happy.

You renamed only the T124 timings here. This is similar to another patch
where you partially removed the reg-shift, which is incorrect to do.

If you'll split this patch into two and rename all the timings
consistently in a separate patch, then I'll be happy. Please note that
you'll also have to change the new DTs if you're going to take them into
5.17.