On 3/22/24 07:53, Michal Simek wrote:
On 3/21/24 17:20, Sean Anderson wrote:
On 3/20/24 07:18, Michal Simek wrote:
if (reg >> BOOT_MODE_ALT_SHIFT) condition rules out alternative jtag boot mode which is 0. When 0 was used origin(HW) boot mode was used instead. That's why directly fill reg variable with requested boot mode and don't let code to read value back. "else" part of code remain unchanged.
Signed-off-by: Michal Simek michal.simek@amd.com
arch/arm/mach-zynqmp/spl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c index 5af735aa5cef..979ff3aef6c2 100644 --- a/arch/arm/mach-zynqmp/spl.c +++ b/arch/arm/mach-zynqmp/spl.c @@ -91,13 +91,14 @@ u32 spl_boot_device(void) #if defined(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED) /* Change default boot mode at run-time */ + reg = CONFIG_SPL_ZYNQMP_ALT_BOOTMODE; writel(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE << BOOT_MODE_ALT_SHIFT, &crlapb_base->boot_mode); -#endif
+#else reg = readl(&crlapb_base->boot_mode); if (reg >> BOOT_MODE_ALT_SHIFT) reg >>= BOOT_MODE_ALT_SHIFT; +#endif bootmode = reg & BOOT_MODES_MASK;
Looks fine; can we change this to
if (IS_ENABLED(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED)) { ... } else { ... }
Issue is that CONFIG_SPL_ZYNQMP_ALT_BOOTMODE symbol is not defined and depends on CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED. It means you get compilation error. That symbol can be setup and then what you have above can work. Is it worth? TBH I don't have preference. Please take a look at patch below. (And if v1 is fine then at least there should be added depends on SPL_ZYNQMP_ALT_BOOTMODE_ENABLED to SPL_ZYNQMP_ALT_BOOTMODE which is missing there now).
Thanks, Michal
diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig index eee34380f0a0..75d3ec916a66 100644 --- a/arch/arm/mach-zynqmp/Kconfig +++ b/arch/arm/mach-zynqmp/Kconfig @@ -145,7 +145,7 @@ config ZYNQ_SDHCI_MAX_FREQ
config SPL_ZYNQMP_ALT_BOOTMODE hex - default 0x0 if JTAG_MODE + default 0x0 default 0x1 if QSPI_MODE_24BIT default 0x2 if QSPI_MODE_32BIT default 0x3 if SD_MODE diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c index 979ff3aef6c2..bbbf684ae496 100644 --- a/arch/arm/mach-zynqmp/spl.c +++ b/arch/arm/mach-zynqmp/spl.c @@ -89,16 +89,16 @@ u32 spl_boot_device(void) u32 reg = 0; u8 bootmode;
-#if defined(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED) - /* Change default boot mode at run-time */ - reg = CONFIG_SPL_ZYNQMP_ALT_BOOTMODE; - writel(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE << BOOT_MODE_ALT_SHIFT, - &crlapb_base->boot_mode); -#else - reg = readl(&crlapb_base->boot_mode); - if (reg >> BOOT_MODE_ALT_SHIFT) - reg >>= BOOT_MODE_ALT_SHIFT; -#endif + if (IS_ENABLED(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED)) { + /* Change default boot mode at run-time */ + reg = CONFIG_SPL_ZYNQMP_ALT_BOOTMODE; + writel(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE << BOOT_MODE_ALT_SHIFT, + &crlapb_base->boot_mode); + } else { + reg = readl(&crlapb_base->boot_mode); + if (reg >> BOOT_MODE_ALT_SHIFT) + reg >>= BOOT_MODE_ALT_SHIFT; + }
bootmode = reg & BOOT_MODES_MASK;
Reviewed-by: Sean Anderson sean.anderson@linux.dev