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

[Tom Rini]

On Thu, Mar 28, 2024 at 04:58:53PM -0500, Chris Morgan wrote:
> On Thu, Mar 28, 2024 at 04:02:09PM -0400, Tom Rini wrote:
> > 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.
> 
> Any thought/benefit of trying to force the ATAGS into a bloblist after the
> fact (instead of expecting it to be passed that way)?  Otherwise yes I'll
> work on simplified ATAGS parsing for the memory info and constrain it to
> the rk3588 specific board code/wrap it around ROCKCHIP_TPL present.

I don't think it's worth the code size to convert the ATAGS to bloblist
at this time. In the future when we have rockchip platforms that pass
bloblist natively at that point it might make sense to convert the ATAGS
instead.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240328/0d5d0575/attachment.sig>