diff mbox series

[v2] imx8m: Use a sane SYS_MALLOC_F_LEN default

Message ID 20220323161020.812583-1-festevam@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show
Series [v2] imx8m: Use a sane SYS_MALLOC_F_LEN default | expand

Commit Message

Fabio Estevam March 23, 2022, 4:10 p.m. UTC
From: Fabio Estevam <festevam@denx.de>

When adding new features to imx8m boards, such as DM clock support,
the malloc area can be exhausted.

To avoid such issue, provide a reasonable default for the
SYS_MALLOC_F_LEN size.

Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v1:
- Add the imx8m default inside the main Kconfig - Heinrich

 Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Tom Rini March 23, 2022, 4:16 p.m. UTC | #1
On Wed, Mar 23, 2022 at 01:10:20PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> When adding new features to imx8m boards, such as DM clock support,
> the malloc area can be exhausted.
> 
> To avoid such issue, provide a reasonable default for the
> SYS_MALLOC_F_LEN size.
> 
> Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
> Changes since v1:
> - Add the imx8m default inside the main Kconfig - Heinrich
> 
>  Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Kconfig b/Kconfig
> index 9dd9ec7f6dfb..060e327e30c6 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -232,10 +232,11 @@ config SYS_MALLOC_F_LEN
>  	depends on SYS_MALLOC_F
>  	default 0x1000 if AM33XX
>  	default 0x4000 if SANDBOX
> -	default 0x2000 if (ARCH_IMX8 || ARCH_IMX8M || ARCH_MX7 || \
> +	default 0x2000 if (ARCH_IMX8 || ARCH_MX7 || \
>  			   ARCH_MX7ULP || ARCH_MX6 || ARCH_MX5 || \
>  			   ARCH_LS1012A || ARCH_LS1021A || ARCH_LS1043A || \
>  			   ARCH_LS1046A || ARCH_QEMU || ARCH_SUNXI || ARCH_OWL)
> +	default 0x10000 if ARCH_IMX8M

It should probably also cover the rest of imx8, and how close to you
think imx6 is?  And, git grep'ing I see a bunch of layerscape also
overriding this with something larger.
Fabio Estevam March 23, 2022, 4:23 p.m. UTC | #2
On Wed, Mar 23, 2022 at 1:16 PM Tom Rini <trini@konsulko.com> wrote:

> It should probably also cover the rest of imx8, and how close to you
> think imx6 is?  And, git grep'ing I see a bunch of layerscape also
> overriding this with something larger.

I am not comfortable in changing imx8qxp/imx8qm/layerscape as I cannot
test them.
Tom Rini March 23, 2022, 4:27 p.m. UTC | #3
On Wed, Mar 23, 2022 at 01:23:29PM -0300, Fabio Estevam wrote:
> On Wed, Mar 23, 2022 at 1:16 PM Tom Rini <trini@konsulko.com> wrote:
> 
> > It should probably also cover the rest of imx8, and how close to you
> > think imx6 is?  And, git grep'ing I see a bunch of layerscape also
> > overriding this with something larger.
> 
> I am not comfortable in changing imx8qxp/imx8qm/layerscape as I cannot
> test them.

Well, this will be for -next, and we can get some more people on the
patch list, and this seems to be an area where everyone is either:
- Kicking the value up a bit for themselves
- Having hard to figure out problems booting the platform because it's
  too small a value until they see someone else picked a larger value.

So lets raise these a bit and get some acks, please.
Adam Ford March 23, 2022, 4:38 p.m. UTC | #4
On Wed, Mar 23, 2022 at 11:27 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Mar 23, 2022 at 01:23:29PM -0300, Fabio Estevam wrote:
> > On Wed, Mar 23, 2022 at 1:16 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > > It should probably also cover the rest of imx8, and how close to you
> > > think imx6 is?  And, git grep'ing I see a bunch of layerscape also
> > > overriding this with something larger.
> >
> > I am not comfortable in changing imx8qxp/imx8qm/layerscape as I cannot
> > test them.
>
> Well, this will be for -next, and we can get some more people on the
> patch list, and this seems to be an area where everyone is either:
> - Kicking the value up a bit for themselves
> - Having hard to figure out problems booting the platform because it's
>   too small a value until they see someone else picked a larger value.
>
> So lets raise these a bit and get some acks, please.

Tested-by: Adam Ford <aford173@gmail.com> #imx8mm-beacon, imx8mn-beacon

In both cases, after 'make savedefconfig' the updated defconfig file
removed the entry to SYS_MALLOC_F_LEN since it was already the same
value.

Thanks for doing that!

adam

>
> --
> Tom
Heiko Thiery March 24, 2022, 7:51 a.m. UTC | #5
Hi Fabio,

Am Mi., 23. März 2022 um 17:10 Uhr schrieb Fabio Estevam <festevam@gmail.com>:
>
> From: Fabio Estevam <festevam@denx.de>
>
> When adding new features to imx8m boards, such as DM clock support,
> the malloc area can be exhausted.
>
> To avoid such issue, provide a reasonable default for the
> SYS_MALLOC_F_LEN size.
>
> Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Fabio Estevam <festevam@denx.de>

I tested this on an imx8mq board (kontron_pitx_imx8m_defconfig).

Tested-by: Heiko Thiery <heiko.thiery@gmail.com>

> ---
> Changes since v1:
> - Add the imx8m default inside the main Kconfig - Heinrich
>
>  Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Kconfig b/Kconfig
> index 9dd9ec7f6dfb..060e327e30c6 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -232,10 +232,11 @@ config SYS_MALLOC_F_LEN
>         depends on SYS_MALLOC_F
>         default 0x1000 if AM33XX
>         default 0x4000 if SANDBOX
> -       default 0x2000 if (ARCH_IMX8 || ARCH_IMX8M || ARCH_MX7 || \
> +       default 0x2000 if (ARCH_IMX8 || ARCH_MX7 || \
>                            ARCH_MX7ULP || ARCH_MX6 || ARCH_MX5 || \
>                            ARCH_LS1012A || ARCH_LS1021A || ARCH_LS1043A || \
>                            ARCH_LS1046A || ARCH_QEMU || ARCH_SUNXI || ARCH_OWL)
> +       default 0x10000 if ARCH_IMX8M
>         default 0x400
>         help
>           Before relocation, memory is very limited on many platforms. Still,
> --
> 2.25.1
>
Fabio Estevam March 28, 2022, 1:56 p.m. UTC | #6
Hi Tom,

On Wed, Mar 23, 2022 at 1:27 PM Tom Rini <trini@konsulko.com> wrote:

> Well, this will be for -next, and we can get some more people on the
> patch list, and this seems to be an area where everyone is either:
> - Kicking the value up a bit for themselves
> - Having hard to figure out problems booting the platform because it's
>   too small a value until they see someone else picked a larger value.
>
> So lets raise these a bit and get some acks, please.

Sure, I sent a v3 as requested.
diff mbox series

Patch

diff --git a/Kconfig b/Kconfig
index 9dd9ec7f6dfb..060e327e30c6 100644
--- a/Kconfig
+++ b/Kconfig
@@ -232,10 +232,11 @@  config SYS_MALLOC_F_LEN
 	depends on SYS_MALLOC_F
 	default 0x1000 if AM33XX
 	default 0x4000 if SANDBOX
-	default 0x2000 if (ARCH_IMX8 || ARCH_IMX8M || ARCH_MX7 || \
+	default 0x2000 if (ARCH_IMX8 || ARCH_MX7 || \
 			   ARCH_MX7ULP || ARCH_MX6 || ARCH_MX5 || \
 			   ARCH_LS1012A || ARCH_LS1021A || ARCH_LS1043A || \
 			   ARCH_LS1046A || ARCH_QEMU || ARCH_SUNXI || ARCH_OWL)
+	default 0x10000 if ARCH_IMX8M
 	default 0x400
 	help
 	  Before relocation, memory is very limited on many platforms. Still,