Hi Tom,
Am Mittwoch, 17. Februar 2021, 03:21:25 CET schrieb 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@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".
Ok ... then I'guess I'll not stand in the way ;-) .
Though we're in the megabyte range with CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR being 0x4000 * 512 . But I guess with current emmc sizes that might not matter too much.
But should there be some sort of warning when the CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR doesn't match the expected value? Because for example rk3399-puma and rk3368-lion historical use that 0x200 instead of 0x4000 block offset and I think at least their default firmware also expects u-boot to not reach that far into the emmc.
Heiko