On 01.12.25 11:32, Chee, Tien Fong wrote: > 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. >
Makes sense as well, then the other approach fits better. Jan -- Siemens AG, Foundational Technologies Linux Expert Center
