On Thu, Nov 7, 2019 at 10:49 AM Marek Vasut <ma...@denx.de> 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. > > [...] > > > diff --git a/arch/arm/mach-socfpga/include/mach/reset_manager.h > > b/arch/arm/mach-socfpga/include/mach/reset_manager.h > > index 6ad037e325..a5b6931350 100644 > > --- a/arch/arm/mach-socfpga/include/mach/reset_manager.h > > +++ b/arch/arm/mach-socfpga/include/mach/reset_manager.h > > @@ -6,6 +6,8 @@ > > #ifndef _RESET_MANAGER_H_ > > #define _RESET_MANAGER_H_ > > > > +extern phys_addr_t socfpga_rstmgr_base; > > Would it make sense to implement a getter function which would return > the reset manager base address , instead of using the extern ? Okay, I can add a function for it. > > [...] > > > diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c > > index 49dadd4c3d..901c432f82 100644 > > --- a/arch/arm/mach-socfpga/misc.c > > +++ b/arch/arm/mach-socfpga/misc.c > > @@ -22,6 +22,8 @@ > > > > DECLARE_GLOBAL_DATA_PTR; > > > > +phys_addr_t socfpga_rstmgr_base __section(".data"); > > + > > #ifdef CONFIG_SYS_L2_PL310 > > static const struct pl310_regs *const pl310 = > > (struct pl310_regs *)CONFIG_SYS_PL310_BASE; > > @@ -145,6 +147,8 @@ void socfpga_fpga_add(void *fpga_desc) > > > > int arch_cpu_init(void) > > { > > + socfpga_get_manager_addr(); > > + > > #ifdef CONFIG_HW_WATCHDOG > > /* > > * In case the watchdog is enabled, make sure to (re-)configure it > > @@ -202,3 +206,31 @@ U_BOOT_CMD(bridge, 3, 1, do_bridge, > > ); > > > > #endif > > + > > +static phys_addr_t socfpga_get_base_addr(const char *compat) > > +{ > > + const void *blob = gd->fdt_blob; > > + struct fdt_resource r; > > + int node; > > + int ret; > > + > > + node = fdt_node_offset_by_compatible(blob, -1, compat); > > + if (node < 0) > > + return 0; > > 0 is a valid address, so you want to discern 0 from errors here. I think > if you change the prototype of the function to e.g. > static int socfpga_get_rstmgr_base_addr(const char *compat, phys_addr_t > *base) {} , it should work. Then you can return error values as well as > the base address. Okay, will change that. > > > + if (!fdtdec_get_is_enabled(blob, node)) > > + return 0; > > + > > + ret = fdt_get_resource(blob, node, "reg", 0, &r); > > + if (ret) > > + return 0; > > + > > + return (phys_addr_t)r.start; > > +} > > + > > +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(). > > > +{ > > + socfpga_rstmgr_base = socfpga_get_base_addr("altr,rst-mgr"); > > + if (!socfpga_rstmgr_base) > > + hang(); > > +} > [...] > > > diff --git a/arch/arm/mach-socfpga/spl_a10.c > > b/arch/arm/mach-socfpga/spl_a10.c > > index b820cb0673..a0d80fd47e 100644 > > --- a/arch/arm/mach-socfpga/spl_a10.c > > +++ b/arch/arm/mach-socfpga/spl_a10.c > > @@ -106,6 +106,11 @@ void spl_board_init(void) > > > > void board_init_f(ulong dummy) > > { > > + if (spl_early_init()) > > + hang(); > > + > > + socfpga_get_manager_addr(); > > + > > dcache_disable(); > > > > socfpga_init_security_policies(); > > @@ -116,8 +121,6 @@ void board_init_f(ulong dummy) > > socfpga_per_reset_all(); > > socfpga_watchdog_disable(); > > > > - spl_early_init(); > > - > > /* Configure the clock based on handoff */ > > cm_basic_init(gd->fdt_blob); > > > > diff --git a/arch/arm/mach-socfpga/spl_gen5.c > > b/arch/arm/mach-socfpga/spl_gen5.c > > index 47e63709ad..c646331081 100644 > > --- a/arch/arm/mach-socfpga/spl_gen5.c > > +++ b/arch/arm/mach-socfpga/spl_gen5.c > > @@ -67,8 +67,14 @@ void board_init_f(ulong dummy) > > int ret; > > struct udevice *dev; > > > > + ret = spl_early_init(); > > + if (ret) > > + hang(); > > + > > + socfpga_get_manager_addr(); > > + > > /* > > - * First C code to run. Clear fake OCRAM ECC first as SBE > > + * Clear fake OCRAM ECC first as SBE > > * and DBE might triggered during power on > > */ > > reg = readl(&sysmgr_regs->eccgrp_ocram); > > @@ -128,12 +134,6 @@ void board_init_f(ulong dummy) > > debug_uart_init(); > > #endif > > > > - ret = spl_early_init(); > > - if (ret) { > > - debug("spl_early_init() failed: %d\n", ret); > > - hang(); > > - } > > - > > ret = uclass_get_device(UCLASS_RESET, 0, &dev); > > if (ret) > > debug("Reset init failed: %d\n", ret); > > 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.
Regards Ley Foon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot