[PATCH] arch: arm: mach-socfpga: Fix non-Agilex targets

[Chee, Tien Fong]

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 <jan.kiszka at siemens.com>
>>>
>>> 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 <jan.kiszka at siemens.com>
>>> ---
>>>    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-
>> briansune at gmail.com/).
>>
>> 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