From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Stuebner Date: Wed, 17 Feb 2021 14:08:55 +0100 Subject: [PATCH] odroid-go2: remove setting SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR In-Reply-To: <20210217022125.GX10169@bill-the-cat> References: <20210213155901.10247-1-royger@FreeBSD.org> <5600009.N7aMVyhfb1@diego> <20210217022125.GX10169@bill-the-cat> Message-ID: <7479695.6fTUFtlzNn@diego> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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? > > > > > > > > > > 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