diff mbox series

arm: socfpga: arria10: Add save_boot_params()

Message ID 20200226190144.65467-1-ley.foon.tan@intel.com
State Superseded
Headers show
Series arm: socfpga: arria10: Add save_boot_params() | expand

Commit Message

Tan, Ley Foon Feb. 26, 2020, 7:01 p.m. UTC
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(+)

Comments

Marek Vasut March 1, 2020, 3:12 p.m. UTC | #1
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);
>
Tan, Ley Foon March 2, 2020, 7:20 a.m. UTC | #2
> -----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
Marek Vasut March 2, 2020, 10:15 a.m. UTC | #3
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.
Ley Foon Tan March 3, 2020, 9:14 a.m. UTC | #4
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
Marek Vasut March 3, 2020, 12:12 p.m. UTC | #5
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 ?

[...]
Tan, Ley Foon March 4, 2020, 12:36 a.m. UTC | #6
> -----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
Marek Vasut March 4, 2020, 12:18 p.m. UTC | #7
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 ? :)
Ley Foon Tan March 5, 2020, 8:56 a.m. UTC | #8
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
Marek Vasut March 5, 2020, 12:22 p.m. UTC | #9
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 ?
Tan, Ley Foon March 6, 2020, 12:33 a.m. UTC | #10
> -----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
Marek Vasut March 6, 2020, 12:43 a.m. UTC | #11
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.
Tan, Ley Foon March 6, 2020, 3:15 a.m. UTC | #12
> -----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 mbox series

Patch

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);