Hi Jan,
On 1/12/2025 5:48 pm, Jan Kiszka wrote:
[CAUTION: This email is from outside your organization. Unless you trust the
sender, do not click on links or open attachments as it may be a fraudulent
email attempting to steal your information and/or compromise your computer.]
On 25.11.25 04:31, Chee, Tien Fong wrote:
Hi Jan,
On 14/11/2025 8:07 pm, Jan Kiszka wrote:
[CAUTION: This email is from outside your organization. Unless you
trust the sender, do not click on links or open attachments as it may
be a fraudulent email attempting to steal your information and/or
compromise your computer.]
From: Jan Kiszka <[email protected]>
The Altera system manager device is only provided for the Agilex
targets. All others were left with an uninitialized socfpga_sysmgr_base
after that refactoring, breaking any boot attempts already in SPL.
Tested only on Cyclone5 / DE0-Nano-SoC.
Fixes: 35638172f99a ("arm: socfpga: agilex5: Add new driver model for
system manager in Agilex5")
Signed-off-by: Jan Kiszka <[email protected]>
---
arch/arm/mach-socfpga/misc.c | 7 +++++++
arch/arm/mach-socfpga/spl_a10.c | 1 +
arch/arm/mach-socfpga/spl_gen5.c | 1 +
arch/arm/mach-socfpga/spl_n5x.c | 1 +
arch/arm/mach-socfpga/spl_s10.c | 1 +
5 files changed, 11 insertions(+)
diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
index 76747c2196a..f859d54dce4 100644
--- a/arch/arm/mach-socfpga/misc.c
+++ b/arch/arm/mach-socfpga/misc.c
@@ -277,6 +277,8 @@ void socfpga_get_managers_addr(void)
void socfpga_get_sys_mgr_addr(void)
{
int ret;
+#if defined(CONFIG_TARGET_SOCFPGA_AGILEX) || \
+ defined(TARGET_SOCFPGA_AGILEX7M) || defined(TARGET_SOCFPGA_AGILEX5)
struct udevice *dev;
ofnode node = ofnode_get_aliases_node("sysmgr");
@@ -293,6 +295,11 @@ void socfpga_get_sys_mgr_addr(void)
} else {
socfpga_sysmgr_base = (phys_addr_t)dev_read_addr(dev);
}
+#else
+ ret = socfpga_get_base_addr("altr,sys-mgr",
&socfpga_sysmgr_base);
+ if (ret)
+ hang();
+#endif
}
phys_addr_t socfpga_get_rstmgr_addr(void)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/
spl_a10.c
index c20376f7f8e..26828077c51 100644
--- a/arch/arm/mach-socfpga/spl_a10.c
+++ b/arch/arm/mach-socfpga/spl_a10.c
@@ -250,6 +250,7 @@ void board_init_f(ulong dummy)
if (spl_early_init())
hang();
+ socfpga_get_sys_mgr_addr();
socfpga_get_managers_addr();
dcache_disable();
diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/
spl_gen5.c
index df79cfe0f7f..f1c654e1566 100644
--- a/arch/arm/mach-socfpga/spl_gen5.c
+++ b/arch/arm/mach-socfpga/spl_gen5.c
@@ -72,6 +72,7 @@ void board_init_f(ulong dummy)
if (ret)
hang();
+ socfpga_get_sys_mgr_addr();
socfpga_get_managers_addr();
/*
diff --git a/arch/arm/mach-socfpga/spl_n5x.c b/arch/arm/mach-socfpga/
spl_n5x.c
index 81283ef7162..ed4e6f38b31 100644
--- a/arch/arm/mach-socfpga/spl_n5x.c
+++ b/arch/arm/mach-socfpga/spl_n5x.c
@@ -31,6 +31,7 @@ void board_init_f(ulong dummy)
if (ret)
hang();
+ socfpga_get_sys_mgr_addr();
socfpga_get_managers_addr();
/* Ensure watchdog is paused when debugging is happening */
diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/
spl_s10.c
index fa83ff96adc..460a6f88416 100644
--- a/arch/arm/mach-socfpga/spl_s10.c
+++ b/arch/arm/mach-socfpga/spl_s10.c
@@ -33,6 +33,7 @@ void board_init_f(ulong dummy)
if (ret)
hang();
+ socfpga_get_sys_mgr_addr();
socfpga_get_managers_addr();
/* Ensure watchdog is paused when debugging is happening */
Thanks for the quick fix. I’ve reviewed your patch along with Brian’s fix
(https://patchwork.ozlabs.org/project/uboot/patch/20251114160423.1518-1-
[email protected]/).
Both address the same issue, but Brian’s approach keeps the change local
to misc.c and avoids spreading SoC specific conditionals into SPL paths,
which fits better with the ongoing DM migration.
Functionally they both work, but Brian’s version looks cleaner for long-
term maintenance.
Could you take a look at Brian’s patch as well and let us know if you
see any issues or corner cases we might have missed?
Sorry, think I forgot to reply back than: The difference is that my
patch already prepares for having a driver model for the other boards as
well, establishing that socfpga_get_sys_mgr_addr sets the address, only
catching the differences in that function. That looks more invasive but
should be design-wise in line with what you want to achieve, at least
mid to long-term.
Thanks for the feedback. My intent is to keep socfpga_get_sys_mgr_addr()
as the entry point for the sysmgr driver model.
That way, when we see it called on a platform, it clearly indicates the
sysmgr driver is running under DM.
No cleanup will be needed for socfpga_get_sys_mgr_addr() later, since
there’s no legacy method for accessing the sysmgr inside.
Best regards,
Tien Fong
