[PATCH] ls10xxx: Use a sane SPL_SYS_MALLOC_F_LEN default

[PATCH] ls10xxx: Use a sane SPL_SYS_MALLOC_F_LEN default

[Sean Anderson]

SPL_SYS_MALLOC_F_LEN defaults to SYS_MALLOC_F_LEN. 0x10000 (the new
default) is 64 KiB, or around half of the total OCRAM size. Revert to
the previous default of 0x2000. This fixes SPL boot.

Fixes: 545eceb520 ("imx8/ls10xx: Use a sane SYS_MALLOC_F_LEN default")
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Kconfig b/Kconfig
index bdae59e06f..2a8ccd79ec 100644
--- a/Kconfig
+++ b/Kconfig
@@ -285,6 +285,7 @@ config SPL_SYS_MALLOC_F_LEN
 	depends on SYS_MALLOC_F && SPL
 	default 0 if !SPL_FRAMEWORK
 	default 0x2800 if RCAR_GEN3
+	default 0x2000 if ARCH_LS1012A || ARCH_LS1021A || ARCH_LS1043A || ARCH_LS1046A
 	default SYS_MALLOC_F_LEN
 	help
 	  In SPL memory is very limited on many platforms. Still,
-- 
2.35.1.1320.gc452695387.dirty

Re: [PATCH] ls10xxx: Use a sane SPL_SYS_MALLOC_F_LEN default

[Fabio Estevam]

Hi Sean,

On Thu, Apr 28, 2022 at 1:02 PM Sean Anderson <sean.anderson@seco.com> wrote:
>
> SPL_SYS_MALLOC_F_LEN defaults to SYS_MALLOC_F_LEN. 0x10000 (the new
> default) is 64 KiB, or around half of the total OCRAM size. Revert to
> the previous default of 0x2000. This fixes SPL boot.
>
> Fixes: 545eceb520 ("imx8/ls10xx: Use a sane SYS_MALLOC_F_LEN default")
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Thanks for the fix.

Would it be better to fix it like this instead?

--- a/Kconfig
+++ b/Kconfig
@@ -257,9 +257,7 @@ config SYS_MALLOC_F_LEN
        default 0x4000 if SANDBOX || RISCV || ARCH_APPLE || ROCKCHIP_RK3368 || \
                          ROCKCHIP_RK3399
        default 0x8000 if RCAR_GEN3
-       default 0x10000 if ARCH_IMX8 || (ARCH_IMX8M && !IMX8MQ) || \
-                          ARCH_LS1012A || ARCH_LS1021A || ARCH_LS1043A || \
-                          ARCH_LS1046A
+       default 0x10000 if ARCH_IMX8 || (ARCH_IMX8M && !IMX8MQ)
        default 0x2000
        help
          Before relocation, memory is very limited on many platforms. Still,

Re: [PATCH] ls10xxx: Use a sane SPL_SYS_MALLOC_F_LEN default

[Sean Anderson]

Hi Fabio,

On 4/28/22 12:23 PM, Fabio Estevam wrote:
> Hi Sean,
> 
> On Thu, Apr 28, 2022 at 1:02 PM Sean Anderson <sean.anderson@seco.com> wrote:
>>
>> SPL_SYS_MALLOC_F_LEN defaults to SYS_MALLOC_F_LEN. 0x10000 (the new
>> default) is 64 KiB, or around half of the total OCRAM size. Revert to
>> the previous default of 0x2000. This fixes SPL boot.
>>
>> Fixes: 545eceb520 ("imx8/ls10xx: Use a sane SYS_MALLOC_F_LEN default")
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> 
> Thanks for the fix.
> 
> Would it be better to fix it like this instead?
> 
> --- a/Kconfig
> +++ b/Kconfig
> @@ -257,9 +257,7 @@ config SYS_MALLOC_F_LEN
>         default 0x4000 if SANDBOX || RISCV || ARCH_APPLE || ROCKCHIP_RK3368 || \
>                           ROCKCHIP_RK3399
>         default 0x8000 if RCAR_GEN3
> -       default 0x10000 if ARCH_IMX8 || (ARCH_IMX8M && !IMX8MQ) || \
> -                          ARCH_LS1012A || ARCH_LS1021A || ARCH_LS1043A || \
> -                          ARCH_LS1046A
> +       default 0x10000 if ARCH_IMX8 || (ARCH_IMX8M && !IMX8MQ)
>         default 0x2000
>         help
>           Before relocation, memory is very limited on many platforms. Still,
> 

That would also work. The primary problem is that between v2 and v3 the default
got raised from 0x2000 to 0x10000. Do you remember why you added layerscape
stuff in the first place? The only non-default value for this property for this
family was to set it to 0x6000 for some ls2 (ls3?) SoCs (e.g. ls1088).

--Sean

Re: [PATCH] ls10xxx: Use a sane SPL_SYS_MALLOC_F_LEN default

[Fabio Estevam]

Hi Sean,

On 28/04/2022 13:31, Sean Anderson wrote:

> That would also work. The primary problem is that between v2 and v3 the 
> default
> got raised from 0x2000 to 0x10000. Do you remember why you added 
> layerscape
> stuff in the first place? The only non-default value for this property 
> for this
> family was to set it to 0x6000 for some ls2 (ls3?) SoCs (e.g. ls1088).

My original patch only touched imx8m, but Tom suggested I also bump 
Layerscape too.

Please send a v2 with my suggestion and feel free to add a:

Reviewed-by: Fabio Estevam <festevam@denx.de>989-80 Email: 
festevam@denx.de

Re: [PATCH] ls10xxx: Use a sane SPL_SYS_MALLOC_F_LEN default

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

On Thu, Apr 28, 2022 at 12:31:08PM -0400, Sean Anderson wrote:
> Hi Fabio,
> 
> On 4/28/22 12:23 PM, Fabio Estevam wrote:
> > Hi Sean,
> > 
> > On Thu, Apr 28, 2022 at 1:02 PM Sean Anderson <sean.anderson@seco.com> wrote:
> >>
> >> SPL_SYS_MALLOC_F_LEN defaults to SYS_MALLOC_F_LEN. 0x10000 (the new
> >> default) is 64 KiB, or around half of the total OCRAM size. Revert to
> >> the previous default of 0x2000. This fixes SPL boot.
> >>
> >> Fixes: 545eceb520 ("imx8/ls10xx: Use a sane SYS_MALLOC_F_LEN default")
> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> > 
> > Thanks for the fix.
> > 
> > Would it be better to fix it like this instead?
> > 
> > --- a/Kconfig
> > +++ b/Kconfig
> > @@ -257,9 +257,7 @@ config SYS_MALLOC_F_LEN
> >         default 0x4000 if SANDBOX || RISCV || ARCH_APPLE || ROCKCHIP_RK3368 || \
> >                           ROCKCHIP_RK3399
> >         default 0x8000 if RCAR_GEN3
> > -       default 0x10000 if ARCH_IMX8 || (ARCH_IMX8M && !IMX8MQ) || \
> > -                          ARCH_LS1012A || ARCH_LS1021A || ARCH_LS1043A || \
> > -                          ARCH_LS1046A
> > +       default 0x10000 if ARCH_IMX8 || (ARCH_IMX8M && !IMX8MQ)
> >         default 0x2000
> >         help
> >           Before relocation, memory is very limited on many platforms. Still,
> > 
> 
> That would also work. The primary problem is that between v2 and v3 the default
> got raised from 0x2000 to 0x10000. Do you remember why you added layerscape
> stuff in the first place? The only non-default value for this property for this
> family was to set it to 0x6000 for some ls2 (ls3?) SoCs (e.g. ls1088).

So yes, I asked since I _think_ before layerscape was using a
smaller-than-0x2000 value and we have had a number of problems with
that.  Sorry for the noise!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]