[PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode

[Michal Simek]

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 at 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;