Re: [PATCH v1 04/10] ARM: tegra: Add reg property to Tegra20 EMC table device-tree nodes

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2974 bytes --]

On Mon, May 31, 2021 at 11:45:19PM +0300, Dmitry Osipenko wrote:
> 31.05.2021 12:14, Thierry Reding пишет:
> > On Mon, May 10, 2021 at 11:25:54PM +0300, Dmitry Osipenko wrote:
> >> The reg property is now specified for the emc-tables nodes in the Tegra20
> >> device-tree binding. Add reg property to the EMC table device-tree nodes
> >> of Tegra20 board device-trees in order to silence dt_binding_check warning
> >> about the missing property.
> >>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >>  arch/arm/boot/dts/tegra20-acer-a500-picasso.dts | 4 ++++
> >>  arch/arm/boot/dts/tegra20-paz00.dts             | 1 +
> >>  2 files changed, 5 insertions(+)
> > 
> > In retrospect we should've just used "reg" in the first place rather
> > than adding the custom "nvidia,ram-code". It's a bit redundant to have
> > both of them with the same value. I wonder if we should deprecate the
> > use of "nvidia,ram-code" and at least make the code look at the "reg"
> > property first and only fall back to "nvidia,ram-code" if "reg" does
> > not exist. We probably won't ever be able to get rid of the fallback
> > for backwards-compatibility reasons, but at least that would make the
> > intent a bit clearer.
> 
> This may be not doable. We have Asus TF101 which doesn't use RAM code
> for the memory identification, instead it uses LPDDR chip info [1]. I
> will send the LPDDR patches later on.
> 
> [1]
> https://github.com/grate-driver/linux/blob/master/arch/arm/boot/dts/tegra20-asus-tf101.dts#L1115

That DTS defines both "jedec,lpddr-manufacturer-id" and "reg" with the
same value, so we could simply use "reg" there. If you plan to support
the JEDEC properties, we'll have to add code for that anyway, so there
is no downside to first trying "reg". And we may not even need to add
support for any of those JEDEC properties if we can just use the "reg"
standard property in the first place.

> The TF101 support mostly in a completed state now, we still need to try
> to figure out why upstream kernel doesn't work using a stock Android
> bootloader, so far bootloader replacement to u-boot is required.

I think it's fine to merge support upstream if there is some sort of
bootloader that it can run with. If that bootloader is open-source like
U-Boot, the better, but I don't think we need to set the bar as high as
being able to boot with any available bootloader. There are all sorts
of reasons why the Android stock bootloader may cause things not to work
and there's probably no way to get it fixed anyway. It's similarly
possible that the kernel may need some outlandish quirk to accomodate
for that breakage and we may not want, or be able, to upstream such
quirks anyway.

So if you want to pursue making upstream Linux work with the stock
Android bootloader, that's a fine goal and I won't object, but it's not
a requirement that I will insist on before merging DTS files.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]