Re: [PATCH 1/2] rockchip: rk3588: Add support for ATAG parsing

[Tom Rini <trini@konsulko.com>]

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

On Wed, Mar 27, 2024 at 10:03:11AM -0500, Chris Morgan wrote:
> On Wed, Mar 27, 2024 at 04:21:49PM +0200, Eugen Hristev wrote:
> > On 3/27/24 15:32, Chris Morgan wrote:
> > > On Wed, Mar 27, 2024 at 06:32:06PM +0800, Kever Yang wrote:
> > >> Hi Chris,
> > >>
> > >>     The ATAGS is used for passing parameter from bootloader to kernel at
> > >> first, which has been replaced by DTB now for ARM platform.
> > >>
> > >>     And Rockchip using ATAGs for passing parameter like dram memory
> > >> size/board uart in different boot process like DRAM init binary/ TPL/SPL to
> > >> U-Boot since 2018.
> > >>
> > >>     And almost at the same time, Simon add bloblist for mainline U-Boot
> > >> which for similar purpose.
> > >>
> > >>     So I'm not sure if this ATAGS should be accept in mainline U-Boot or
> > >> not, even for rockchip platform only seems some kind of regression for this
> > >> feature support.
> > >>
> > >>
> > >> Hi Simon and Tom,
> > >>
> > >>      Could you help to give some suggestion for this?
> > >>
> > > 
> > > I really meant to do this as an "RFC", so I apologize in advance for
> > > possibly causing more work in treating this as a full-fledged patch.
> > > 
> > > The problem I'm trying to solve is that I've got 2 boards, a Rock 5B
> > > as well as an Indiedroid Nova both with 16GB of RAM. I noticed that
> > > without the memory holes the Rock 5B defined in my Indiedroid it would
> > > also fail to boot. I've got 2 other boards as well with less than 16GB
> > > of RAM which seem to work fine without any holes (a 4GB Indiedroid Nova
> > > prototype and a GameForce Ace with 12GB of RAM).
> > 
> > Hi,
> > 
> > When I initially added these holes in the memory, I tried to ask Rockchip what are
> > the holes for. I didn't get any answer, however the patches to reserve the holes
> > were accepted.
> > If we could get more information about why the holes are there, if that area is
> > specific to something, or that it's fixed in a per-SoC basis, we could reserve it
> > by hardcoding in the Linux DT, without the need for ATAGs.
> > Without real information, we cannot be sure that for other variants of the SoC or
> > some other bootrom configuration, the holes will not change/move.
> > 
> > Eugen
> 
> It's not *just* about the holes, but also making sure we use the full
> extent of the RAM. For example on my 4GB Indiedroid Nova the existing
> logic ignores about 256MB of RAM that's otherwise present. When we
> use the existing code the RAM map reported by Linux is as follows:
> 
> [    0.000000] Early memory node ranges
> [    0.000000]   node   0: [mem 0x0000000000200000-0x00000000efffffff]
> 
> Whereas when we do ATAGS parsing to set the banks we get the following:
> 
> [    0.000000] Early memory node ranges
> [    0.000000]   node   0: [mem 0x0000000000200000-0x00000000efffffff]
> [    0.000000]   node   0: [mem 0x00000001f0000000-0x00000001ffffffff]
> 
> So basically the issue is two-fold... one we need to create the memory
> holes when necessary, and two the current method of defining memory
> banks leaves some RAM unallocated. Parsing the ATAGS does both of these,
> I'm just curious if it's the RIGHT way or there's something else I'm
> missing...

It's unfortunate that all of this is in the wild, from the Rockchip
side. The best answer is that the platform should be passing all of this
along via bloblist. I would hope that future platforms will do that. For
now yes, I guess we need to constrain this ATAG parsing to rockchip as
much as we can and use it.

-- 
Tom

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