[PATCH] odroid-go2: remove setting SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR

[Tom Rini]

On Wed, Feb 17, 2021 at 02:42:21AM +0100, Heiko Stuebner wrote:
> Hi Tom,
> 
> Am Dienstag, 16. Februar 2021, 15:26:52 CET schrieb Tom Rini:
> > On Sat, Feb 13, 2021 at 11:45:50PM +0100, Heiko Stuebner wrote:
> > > Hi Roger,
> > > 
> > > Am Samstag, 13. Februar 2021, 16:59:01 CET schrieb Roger Pau Monne:
> > > > From: Roger Pau Monné <royger at FreeBSD.org>
> > > > 
> > > > Using a non-default SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR setting makes the
> > > > resulting u-boot-rockchip.bin unbootable, as it gets stuck after SPL.
> > > > Removing the setting from the defconfig allows U-Boot to load
> > > > successfully.
> > > 
> > > Hmm, I'd disagree slightly.
> > > 
> > > In the rockchip-common.h the CONFIG_SPL_PAD_TO is defined as
> > > 
> > > /* ((CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR - 64) * 512) */
> > > #define CONFIG_SPL_PAD_TO		8355840
> > > 
> > > so it's a static value but based on the MMCSD_RAW_MODE... config option.
> > > 
> > > So instead of mandating one specific MMCSD_RAW_MODE... value
> > > that CONFIG_SPL_PAD_TO should be defined based on the the
> > > actual config value of CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
> > > and not some static number that then gets enforced for all boards.
> > 
> > So, what does CONFIG_SPL_PAD_TO actually mean, in this case?  And
> > SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR ?  What I'm getting at is that we
> > generally have some required to be fixed (by the SoC/ROM/etc) locations
> > some parts of our SPL/TPL/U-Boot need to be at and then the rest of the
> > values are (supposed to be) well and carefully chosen offsets and not
> > changed around.  So with the above comment in the code to explain where
> > 8355840 came from, it also shouldn't and nor should
> > CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR be changed without a compelling
> > reason.
> 
> Normally Rockchip platforms have two loader binaries:
> 
> - idbLoader.img (tpl + spl, or only spl), loving at offset 64 of a sd-card
>   This is mandated by the bootrom
> - u-boot.itb (u-boot, atf, etc) living at SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
>   As SPL will load from the location specified in the config, this location
>   can be set depending on emmc/sd-card/whatever needs.
> 
> It looks like recently a new binary creating method was added that creates
> a u-boot-rockchip.bin combining these somewhat automatically:
> 
> idbLoader.img
> + SPL_PAD_TO
> + u-boot.itb
> 
> So that only that binary needs to be flashed to the boot medium
> instead of two.
> 
> So the SPL_PAD_TO essentially would mandate one specific
> SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR for every board.
> 
> 
> For the odroid-go2 itself it doesn't really matter I guess, but there are
> other boards with different requirements, so mandating one specific place
> for the main uboot for all boards that will ever exist seems a bit counter-
> intuitive to me.

I would say that yes, it's quite intentional that all boards for a given
SoC (or SoC family) would use the same value for
SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR and NOT leave it up to be per-board.
It should be a matter of kilobytes being potentially wasted which is
(often or most likely) worth sacrificing in the name of consistency and
ease of future use / development.  In other cases this ends up being
something around "ROM will only load something of $X size, round that up
a little bit, place U-Boot there, as it's the next thing to load".

-- 
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/20210216/9148e40f/attachment.sig>