[U-Boot] [PATCH v5 2/4] arm: socfpga: Convert reset manager from struct to defines

[Ley Foon Tan]

On Thu, Nov 7, 2019 at 4:33 PM Marek Vasut <marex at denx.de> wrote:
>
> On 11/7/19 9:06 AM, Ley Foon Tan wrote:
> > On Thu, Nov 7, 2019 at 10:49 AM Marek Vasut wrote:
> >>
> >> On 11/7/19 3:10 AM, Ley Foon Tan wrote:
> >>> Convert reset manager for Gen5, Arria 10 and Stratix 10 from struct
> >>> to defines.
> >>>
> >>> Change to get reset manager base address from DT node instead of using
> >>> #define.
> >>
> >> It seems the patch also moves spl_early_init() around ?
> > Yes, because spl_early_init() initialize DT stuff, so it needs to be
> > called before we get base address from DT.
>
> That really should be documented in the commit message though.
Okay.
>
> [...]
>
> >>> +void socfpga_get_manager_addr(void)
> >>
> >> You should rename this function, a lot of blocks on the Gen5 are called
> >> <something>-manager .
> > Okay, will change it something like socfpga_get_base_addr().
>
> If it's only used to retrieve the reset manager base, then it should say
> so in the function name.
socfpga_get_manager_addr() function will get base address for clkmgr,
rstmgr and sysmgr, not only reset manger.

>
> >>> +{
> >>> +     socfpga_rstmgr_base = socfpga_get_base_addr("altr,rst-mgr");
> >>> +     if (!socfpga_rstmgr_base)
> >>> +             hang();
> >>> +}
>
> [...]
>
> >>> diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
> >>> index ec65e1ce64..9a97a84e1e 100644
> >>> --- a/arch/arm/mach-socfpga/spl_s10.c
> >>> +++ b/arch/arm/mach-socfpga/spl_s10.c
> >>> @@ -14,6 +14,7 @@
> >>>  #include <asm/arch/clock_manager.h>
> >>>  #include <asm/arch/firewall_s10.h>
> >>>  #include <asm/arch/mailbox_s10.h>
> >>> +#include <asm/arch/misc.h>
> >>>  #include <asm/arch/reset_manager.h>
> >>>  #include <asm/arch/system_manager.h>
> >>>  #include <watchdog.h>
> >>> @@ -120,6 +121,12 @@ void board_init_f(ulong dummy)
> >>>       const struct cm_config *cm_default_cfg = cm_get_default_config();
> >>>       int ret;
> >>>
> >>> +     ret = spl_early_init();
> >>> +     if (ret)
> >>> +             hang();
> >>> +
> >>> +     socfpga_get_manager_addr();
> >>> +
> >>>  #ifdef CONFIG_HW_WATCHDOG
> >>>       /* Ensure watchdog is paused when debugging is happening */
> >>>       writel(SYSMGR_WDDBG_PAUSE_ALL_CPU, &sysmgr_regs->wddbg);
> >>> @@ -145,11 +152,6 @@ void board_init_f(ulong dummy)
> >>>       socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
> >>>       debug_uart_init();
> >>>  #endif
> >>> -     ret = spl_early_init();
> >>> -     if (ret) {
> >>> -             debug("spl_early_init() failed: %d\n", ret);
> >>> -             hang();
> >>> -     }
> >>>
> >>>       preloader_console_init();
> >>>       cm_print_clock_quick_summary();
> >>
> >> Are these three hunks above really supposed to be in this patch ?
> > Yes, as explained earlier,  spl_early_init() need to be called before
> > get base address from DT.
>
> OK, please just add it into the commit message.
Okay.

Thanks.
Regards
Ley Foon