| Message ID | 20200226190144.65467-1-ley.foon.tan@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm: socfpga: arria10: Add save_boot_params() | expand |
On 2/26/20 8:01 PM, Ley Foon Tan wrote: [...] > +#define BOOTROM_SHARED_MEM_ADDR (CONFIG_SYS_INIT_RAM_ADDR + 0x40000 \ > + - 0x800) > +#define RST_STATUS_SHARED_ADDR (BOOTROM_SHARED_MEM_ADDR + 0x438) Are all these magic values needed or is there some more descriptive macro name available for them ? > +u32 rst_mgr_status __section(.data); static u32 ... ? > + > +/* > + * Bootrom will clear the status register in reset manager and stores the > + * reset status value in shared memory. Bootrom stores shared data at last > + * 2KB of onchip RAM. > + * This function save reset status provided by BootROM to rst_mgr_status. > + * More information about reset status register value can be found in reset > + * manager register description. > + * When running in debugger without Bootrom, r0 to r3 are random values. > + * So, skip save the value when r0 is not BootROM shared data address. > + * > + * r0 - Contains the pointer to the shared memory block. The shared > + * memory block is located in the top 2 KB of on-chip RAM. > + * r1 - contains the length of the shared memory. > + * r2 - unused and set to 0x0. > + * r3 - points to the version block. > + */ > +void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2, > + unsigned long r3) > +{ > + if (r0 == BOOTROM_SHARED_MEM_ADDR) > + rst_mgr_status = readl(RST_STATUS_SHARED_ADDR); > + > + save_boot_params_ret(); > +} > + > u32 spl_boot_device(void) > { > const u32 bsel = readl(socfpga_get_sysmgr_addr() + SYSMGR_A10_BOOTINFO); >
> -----Original Message----- > From: Marek Vasut <marex at denx.de> > Sent: Sunday, March 1, 2020 11:12 PM > To: Tan, Ley Foon <ley.foon.tan at intel.com>; u-boot at lists.denx.de > Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>; Ley Foon Tan > <lftan.linux at gmail.com>; See, Chin Liang <chin.liang.see at intel.com>; Chee, > Tien Fong <tien.fong.chee at intel.com> > Subject: Re: [PATCH] arm: socfpga: arria10: Add save_boot_params() > > On 2/26/20 8:01 PM, Ley Foon Tan wrote: > [...] > > +#define BOOTROM_SHARED_MEM_ADDR > (CONFIG_SYS_INIT_RAM_ADDR + 0x40000 \ > > + - 0x800) > > +#define RST_STATUS_SHARED_ADDR > (BOOTROM_SHARED_MEM_ADDR + 0x438) > > Are all these magic values needed or is there some more descriptive macro > name available for them ? 0x40000 is onchip ram size and 0x800 is 2KB size. I can convert these to 2 macos. > > > +u32 rst_mgr_status __section(.data); > > static u32 ... ? Other files might want to use this. Thanks. Regards Ley Foon
On 3/2/20 8:20 AM, Tan, Ley Foon wrote: Hi, [...] >> On 2/26/20 8:01 PM, Ley Foon Tan wrote: >> [...] >>> +#define BOOTROM_SHARED_MEM_ADDR >> (CONFIG_SYS_INIT_RAM_ADDR + 0x40000 \ >>> + - 0x800) >>> +#define RST_STATUS_SHARED_ADDR >> (BOOTROM_SHARED_MEM_ADDR + 0x438) >> >> Are all these magic values needed or is there some more descriptive macro >> name available for them ? > 0x40000 is onchip ram size and 0x800 is 2KB size. > I can convert these to 2 macos. Aren't those already defined in include/configs/socfpga_common.h ? >>> +u32 rst_mgr_status __section(.data); >> >> static u32 ... ? > Other files might want to use this. But they currently don't ? If others want to use it, they can remove the static then.
On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut <marex at denx.de> wrote: > > On 3/2/20 8:20 AM, Tan, Ley Foon wrote: > Hi, > > [...] > > >> On 2/26/20 8:01 PM, Ley Foon Tan wrote: > >> [...] > >>> +#define BOOTROM_SHARED_MEM_ADDR > >> (CONFIG_SYS_INIT_RAM_ADDR + 0x40000 \ > >>> + - 0x800) > >>> +#define RST_STATUS_SHARED_ADDR > >> (BOOTROM_SHARED_MEM_ADDR + 0x438) > >> > >> Are all these magic values needed or is there some more descriptive macro > >> name available for them ? > > 0x40000 is onchip ram size and 0x800 is 2KB size. > > I can convert these to 2 macos. > > Aren't those already defined in include/configs/socfpga_common.h ? socfpga_common.h have this: #define CONFIG_SYS_INIT_RAM_SIZE (0x40000 - CONFIG_SYS_SPL_MALLOC_SIZE) But, we can't use it here. > > >>> +u32 rst_mgr_status __section(.data); > >> > >> static u32 ... ? > > Other files might want to use this. > > But they currently don't ? If others want to use it, they can remove the > static then. Okay. Regards Ley Foon
On 3/3/20 10:14 AM, Ley Foon Tan wrote: > On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut <marex at denx.de> wrote: >> >> On 3/2/20 8:20 AM, Tan, Ley Foon wrote: >> Hi, >> >> [...] >> >>>> On 2/26/20 8:01 PM, Ley Foon Tan wrote: >>>> [...] >>>>> +#define BOOTROM_SHARED_MEM_ADDR >>>> (CONFIG_SYS_INIT_RAM_ADDR + 0x40000 \ >>>>> + - 0x800) >>>>> +#define RST_STATUS_SHARED_ADDR >>>> (BOOTROM_SHARED_MEM_ADDR + 0x438) >>>> >>>> Are all these magic values needed or is there some more descriptive macro >>>> name available for them ? >>> 0x40000 is onchip ram size and 0x800 is 2KB size. >>> I can convert these to 2 macos. >> >> Aren't those already defined in include/configs/socfpga_common.h ? > socfpga_common.h have this: > #define CONFIG_SYS_INIT_RAM_SIZE (0x40000 - CONFIG_SYS_SPL_MALLOC_SIZE) > > But, we can't use it here. Why ? [...]
> -----Original Message----- > From: Marek Vasut <marex at denx.de> > Sent: Tuesday, March 3, 2020 8:13 PM > To: Ley Foon Tan <lftan.linux at gmail.com> > Cc: Tan, Ley Foon <ley.foon.tan at intel.com>; u-boot at lists.denx.de; Simon > Goldschmidt <simon.k.r.goldschmidt at gmail.com>; See, Chin Liang > <chin.liang.see at intel.com>; Chee, Tien Fong <tien.fong.chee at intel.com> > Subject: Re: [PATCH] arm: socfpga: arria10: Add save_boot_params() > > On 3/3/20 10:14 AM, Ley Foon Tan wrote: > > On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut <marex at denx.de> wrote: > >> > >> On 3/2/20 8:20 AM, Tan, Ley Foon wrote: > >> Hi, > >> > >> [...] > >> > >>>> On 2/26/20 8:01 PM, Ley Foon Tan wrote: > >>>> [...] > >>>>> +#define BOOTROM_SHARED_MEM_ADDR > >>>> (CONFIG_SYS_INIT_RAM_ADDR + 0x40000 \ > >>>>> + - 0x800) #define > >>>>> +RST_STATUS_SHARED_ADDR > >>>> (BOOTROM_SHARED_MEM_ADDR + 0x438) > >>>> > >>>> Are all these magic values needed or is there some more descriptive > >>>> macro name available for them ? > >>> 0x40000 is onchip ram size and 0x800 is 2KB size. > >>> I can convert these to 2 macos. > >> > >> Aren't those already defined in include/configs/socfpga_common.h ? > > socfpga_common.h have this: > > #define CONFIG_SYS_INIT_RAM_SIZE (0x40000 - > CONFIG_SYS_SPL_MALLOC_SIZE) > > > > But, we can't use it here. > > Why ? CONFIG_SYS_INIT_RAM_SIZE is minus with CONFIG_SYS_SPL_MALLOC_SIZE, not 256KB (0x40000). Regards Ley Foon
On 3/4/20 1:36 AM, Tan, Ley Foon wrote: > > >> -----Original Message----- >> From: Marek Vasut <marex at denx.de> >> Sent: Tuesday, March 3, 2020 8:13 PM >> To: Ley Foon Tan <lftan.linux at gmail.com> >> Cc: Tan, Ley Foon <ley.foon.tan at intel.com>; u-boot at lists.denx.de; Simon >> Goldschmidt <simon.k.r.goldschmidt at gmail.com>; See, Chin Liang >> <chin.liang.see at intel.com>; Chee, Tien Fong <tien.fong.chee at intel.com> >> Subject: Re: [PATCH] arm: socfpga: arria10: Add save_boot_params() >> >> On 3/3/20 10:14 AM, Ley Foon Tan wrote: >>> On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut <marex at denx.de> wrote: >>>> >>>> On 3/2/20 8:20 AM, Tan, Ley Foon wrote: >>>> Hi, >>>> >>>> [...] >>>> >>>>>> On 2/26/20 8:01 PM, Ley Foon Tan wrote: >>>>>> [...] >>>>>>> +#define BOOTROM_SHARED_MEM_ADDR >>>>>> (CONFIG_SYS_INIT_RAM_ADDR + 0x40000 \ >>>>>>> + - 0x800) #define >>>>>>> +RST_STATUS_SHARED_ADDR >>>>>> (BOOTROM_SHARED_MEM_ADDR + 0x438) >>>>>> >>>>>> Are all these magic values needed or is there some more descriptive >>>>>> macro name available for them ? >>>>> 0x40000 is onchip ram size and 0x800 is 2KB size. >>>>> I can convert these to 2 macos. >>>> >>>> Aren't those already defined in include/configs/socfpga_common.h ? >>> socfpga_common.h have this: >>> #define CONFIG_SYS_INIT_RAM_SIZE (0x40000 - >> CONFIG_SYS_SPL_MALLOC_SIZE) >>> >>> But, we can't use it here. >> >> Why ? > CONFIG_SYS_INIT_RAM_SIZE is minus with CONFIG_SYS_SPL_MALLOC_SIZE, not 256KB (0x40000). Then define the init ram size macro somewhere in arch/arm/mach-socfpga/include and use it in both the configs/ and your code ? :)
On Wed, Mar 4, 2020 at 8:32 PM Marek Vasut <marex at denx.de> wrote: > > On 3/4/20 1:36 AM, Tan, Ley Foon wrote: > > > > > >> -----Original Message----- > >> From: Marek Vasut <marex at denx.de> > >> Sent: Tuesday, March 3, 2020 8:13 PM > >> To: Ley Foon Tan <lftan.linux at gmail.com> > >> Cc: Tan, Ley Foon <ley.foon.tan at intel.com>; u-boot at lists.denx.de; Simon > >> Goldschmidt <simon.k.r.goldschmidt at gmail.com>; See, Chin Liang > >> <chin.liang.see at intel.com>; Chee, Tien Fong <tien.fong.chee at intel.com> > >> Subject: Re: [PATCH] arm: socfpga: arria10: Add save_boot_params() > >> > >> On 3/3/20 10:14 AM, Ley Foon Tan wrote: > >>> On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut <marex at denx.de> wrote: > >>>> > >>>> On 3/2/20 8:20 AM, Tan, Ley Foon wrote: > >>>> Hi, > >>>> > >>>> [...] > >>>> > >>>>>> On 2/26/20 8:01 PM, Ley Foon Tan wrote: > >>>>>> [...] > >>>>>>> +#define BOOTROM_SHARED_MEM_ADDR > >>>>>> (CONFIG_SYS_INIT_RAM_ADDR + 0x40000 \ > >>>>>>> + - 0x800) #define > >>>>>>> +RST_STATUS_SHARED_ADDR > >>>>>> (BOOTROM_SHARED_MEM_ADDR + 0x438) > >>>>>> > >>>>>> Are all these magic values needed or is there some more descriptive > >>>>>> macro name available for them ? > >>>>> 0x40000 is onchip ram size and 0x800 is 2KB size. > >>>>> I can convert these to 2 macos. > >>>> > >>>> Aren't those already defined in include/configs/socfpga_common.h ? > >>> socfpga_common.h have this: > >>> #define CONFIG_SYS_INIT_RAM_SIZE (0x40000 - > >> CONFIG_SYS_SPL_MALLOC_SIZE) > >>> > >>> But, we can't use it here. > >> > >> Why ? > > CONFIG_SYS_INIT_RAM_SIZE is minus with CONFIG_SYS_SPL_MALLOC_SIZE, not 256KB (0x40000). > > Then define the init ram size macro somewhere in > arch/arm/mach-socfpga/include and use it in both the configs/ and your > code ? :) Look at existing header files under arch/arm/mach-socfpga/include/, can add to base_addr_a10.h. Regards Ley Foon
On 3/5/20 9:56 AM, Ley Foon Tan wrote: Hi, [...] >>>>>> Aren't those already defined in include/configs/socfpga_common.h ? >>>>> socfpga_common.h have this: >>>>> #define CONFIG_SYS_INIT_RAM_SIZE (0x40000 - >>>> CONFIG_SYS_SPL_MALLOC_SIZE) >>>>> >>>>> But, we can't use it here. >>>> >>>> Why ? >>> CONFIG_SYS_INIT_RAM_SIZE is minus with CONFIG_SYS_SPL_MALLOC_SIZE, not 256KB (0x40000). >> >> Then define the init ram size macro somewhere in >> arch/arm/mach-socfpga/include and use it in both the configs/ and your >> code ? :) > Look at existing header files under arch/arm/mach-socfpga/include/, > can add to base_addr_a10.h. Isn't that base address common to A10 and CV/AV ?
> -----Original Message----- > From: Marek Vasut <marex at denx.de> > Sent: Thursday, March 5, 2020 8:23 PM > To: Ley Foon Tan <lftan.linux at gmail.com> > Cc: Tan, Ley Foon <ley.foon.tan at intel.com>; u-boot at lists.denx.de; Simon > Goldschmidt <simon.k.r.goldschmidt at gmail.com>; See, Chin Liang > <chin.liang.see at intel.com>; Chee, Tien Fong <tien.fong.chee at intel.com> > Subject: Re: [PATCH] arm: socfpga: arria10: Add save_boot_params() > > On 3/5/20 9:56 AM, Ley Foon Tan wrote: > Hi, > > [...] > > >>>>>> Aren't those already defined in include/configs/socfpga_common.h ? > >>>>> socfpga_common.h have this: > >>>>> #define CONFIG_SYS_INIT_RAM_SIZE (0x40000 - > >>>> CONFIG_SYS_SPL_MALLOC_SIZE) > >>>>> > >>>>> But, we can't use it here. > >>>> > >>>> Why ? > >>> CONFIG_SYS_INIT_RAM_SIZE is minus with > CONFIG_SYS_SPL_MALLOC_SIZE, not 256KB (0x40000). > >> > >> Then define the init ram size macro somewhere in > >> arch/arm/mach-socfpga/include and use it in both the configs/ and > >> your code ? :) > > Look at existing header files under arch/arm/mach-socfpga/include/, > > can add to base_addr_a10.h. > > Isn't that base address common to A10 and CV/AV ? No, CV/AV uses base_addr_ac5.h. Regards Ley Foon
On 3/6/20 1:33 AM, Tan, Ley Foon wrote: [...] >>>>>>>> Aren't those already defined in include/configs/socfpga_common.h ? >>>>>>> socfpga_common.h have this: >>>>>>> #define CONFIG_SYS_INIT_RAM_SIZE (0x40000 - >>>>>> CONFIG_SYS_SPL_MALLOC_SIZE) >>>>>>> >>>>>>> But, we can't use it here. >>>>>> >>>>>> Why ? >>>>> CONFIG_SYS_INIT_RAM_SIZE is minus with >> CONFIG_SYS_SPL_MALLOC_SIZE, not 256KB (0x40000). >>>> >>>> Then define the init ram size macro somewhere in >>>> arch/arm/mach-socfpga/include and use it in both the configs/ and >>>> your code ? :) >>> Look at existing header files under arch/arm/mach-socfpga/include/, >>> can add to base_addr_a10.h. >> >> Isn't that base address common to A10 and CV/AV ? > No, CV/AV uses base_addr_ac5.h. Then it might make sense to define the macro in both.
> -----Original Message----- > From: Marek Vasut <marex at denx.de> > Sent: Friday, March 6, 2020 8:43 AM > To: Tan, Ley Foon <ley.foon.tan at intel.com>; Ley Foon Tan > <lftan.linux at gmail.com> > Cc: u-boot at lists.denx.de; Simon Goldschmidt > <simon.k.r.goldschmidt at gmail.com>; See, Chin Liang > <chin.liang.see at intel.com>; Chee, Tien Fong <tien.fong.chee at intel.com> > Subject: Re: [PATCH] arm: socfpga: arria10: Add save_boot_params() > > On 3/6/20 1:33 AM, Tan, Ley Foon wrote: > [...] > >>>>>>>> Aren't those already defined in > include/configs/socfpga_common.h ? > >>>>>>> socfpga_common.h have this: > >>>>>>> #define CONFIG_SYS_INIT_RAM_SIZE (0x40000 - > >>>>>> CONFIG_SYS_SPL_MALLOC_SIZE) > >>>>>>> > >>>>>>> But, we can't use it here. > >>>>>> > >>>>>> Why ? > >>>>> CONFIG_SYS_INIT_RAM_SIZE is minus with > >> CONFIG_SYS_SPL_MALLOC_SIZE, not 256KB (0x40000). > >>>> > >>>> Then define the init ram size macro somewhere in > >>>> arch/arm/mach-socfpga/include and use it in both the configs/ and > >>>> your code ? :) > >>> Look at existing header files under arch/arm/mach-socfpga/include/, > >>> can add to base_addr_a10.h. > >> > >> Isn't that base address common to A10 and CV/AV ? > > No, CV/AV uses base_addr_ac5.h. > > Then it might make sense to define the macro in both. Okay. Regards Ley Foon
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c index d9ef851054..867b1c906e 100644 --- a/arch/arm/mach-socfpga/spl_a10.c +++ b/arch/arm/mach-socfpga/spl_a10.c @@ -33,6 +33,36 @@ DECLARE_GLOBAL_DATA_PTR; +#define BOOTROM_SHARED_MEM_ADDR (CONFIG_SYS_INIT_RAM_ADDR + 0x40000 \ + - 0x800) +#define RST_STATUS_SHARED_ADDR (BOOTROM_SHARED_MEM_ADDR + 0x438) +u32 rst_mgr_status __section(.data); + +/* + * Bootrom will clear the status register in reset manager and stores the + * reset status value in shared memory. Bootrom stores shared data at last + * 2KB of onchip RAM. + * This function save reset status provided by BootROM to rst_mgr_status. + * More information about reset status register value can be found in reset + * manager register description. + * When running in debugger without Bootrom, r0 to r3 are random values. + * So, skip save the value when r0 is not BootROM shared data address. + * + * r0 - Contains the pointer to the shared memory block. The shared + * memory block is located in the top 2 KB of on-chip RAM. + * r1 - contains the length of the shared memory. + * r2 - unused and set to 0x0. + * r3 - points to the version block. + */ +void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2, + unsigned long r3) +{ + if (r0 == BOOTROM_SHARED_MEM_ADDR) + rst_mgr_status = readl(RST_STATUS_SHARED_ADDR); + + save_boot_params_ret(); +} + u32 spl_boot_device(void) { const u32 bsel = readl(socfpga_get_sysmgr_addr() + SYSMGR_A10_BOOTINFO);
Add save_boot_params() to save reset status value from bootrom. Bootrom will clear the status register in reset manager and stores the reset status value in shared memory. Bootrom stores shared data at last 2KB of onchip RAM. This function save reset status provided by BootROM to rst_mgr_status. More information about reset status register value can be found in reset manager register description. When running in debugger without Bootrom, r0 to r3 are random values. So, skip save the value when r0 is not BootROM shared data address. Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com> --- arch/arm/mach-socfpga/spl_a10.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)