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