* [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode
@ 2024-03-20 11:18 Michal Simek
2024-03-21 16:20 ` Sean Anderson
2024-03-25 14:15 ` Michal Simek
0 siblings, 2 replies; 5+ messages in thread
From: Michal Simek @ 2024-03-20 11:18 UTC (permalink / raw)
To: u-boot, git; +Cc: Sean Anderson, Tom Rini
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;
--
2.36.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode
2024-03-20 11:18 [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode Michal Simek
@ 2024-03-21 16:20 ` Sean Anderson
2024-03-22 11:53 ` Michal Simek
2024-03-25 14:15 ` Michal Simek
1 sibling, 1 reply; 5+ messages in thread
From: Sean Anderson @ 2024-03-21 16:20 UTC (permalink / raw)
To: Michal Simek, u-boot, git; +Cc: Tom Rini
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 {
...
}
?
--Sean
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode
2024-03-21 16:20 ` Sean Anderson
@ 2024-03-22 11:53 ` Michal Simek
2024-03-22 16:38 ` Sean Anderson
0 siblings, 1 reply; 5+ messages in thread
From: Michal Simek @ 2024-03-22 11:53 UTC (permalink / raw)
To: Sean Anderson, u-boot, git; +Cc: Tom Rini
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;
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode
2024-03-22 11:53 ` Michal Simek
@ 2024-03-22 16:38 ` Sean Anderson
0 siblings, 0 replies; 5+ messages in thread
From: Sean Anderson @ 2024-03-22 16:38 UTC (permalink / raw)
To: Michal Simek, u-boot, git; +Cc: Tom Rini
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode
2024-03-20 11:18 [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode Michal Simek
2024-03-21 16:20 ` Sean Anderson
@ 2024-03-25 14:15 ` Michal Simek
1 sibling, 0 replies; 5+ messages in thread
From: Michal Simek @ 2024-03-25 14:15 UTC (permalink / raw)
To: u-boot, git; +Cc: Sean Anderson, Tom Rini
On 3/20/24 12: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;
>
Applied.
M
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-03-25 14:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-20 11:18 [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode Michal Simek
2024-03-21 16:20 ` Sean Anderson
2024-03-22 11:53 ` Michal Simek
2024-03-22 16:38 ` Sean Anderson
2024-03-25 14:15 ` Michal Simek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox