Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN

[Simon Glass <sjg@chromium.org>]

Hi Tom,

On Wed, 23 Mar 2022 at 12:59, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Mar 23, 2022 at 12:53:59PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 23 Mar 2022 at 12:49, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Wed, Mar 23, 2022 at 02:33:12PM -0400, Sean Anderson wrote:
> > > > On 3/23/22 2:26 PM, Heiko Thiery wrote:
> > > > > Hi Simon,
> > > > >
> > > > > Am Mi., 23. März 2022 um 19:04 Uhr schrieb Simon Glass <sjg@chromium.org>:
> > > > > >
> > > > > > Hi Heinrich,
> > > > > >
> > > > > > On Tue, 22 Mar 2022 at 03:25, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > > > >
> > > > > > > On 3/21/22 15:26, Heiko Thiery wrote:
> > > > > > > > It was observed that enabling additional DM modules the configured
> > > > > > > > malloc value is not sufficient. So lets increase the value.
> > > > > > > >
> > > > > > > > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> > > > > > > > ---
> > > > > > > > v2:
> > > > > > > >    - add a more proper commit message to explan why the value was increased
> > > > > > > >
> > > > > > > >    configs/kontron_pitx_imx8m_defconfig | 1 +
> > > > > > > >    1 file changed, 1 insertion(+)
> > > > > > > >
> > > > > > > > diff --git a/configs/kontron_pitx_imx8m_defconfig b/configs/kontron_pitx_imx8m_defconfig
> > > > > > > > index 76430213e3..30c3586937 100644
> > > > > > > > --- a/configs/kontron_pitx_imx8m_defconfig
> > > > > > > > +++ b/configs/kontron_pitx_imx8m_defconfig
> > > > > > > > @@ -2,6 +2,7 @@ CONFIG_ARM=y
> > > > > > > >    CONFIG_ARCH_IMX8M=y
> > > > > > > >    CONFIG_SYS_TEXT_BASE=0x40200000
> > > > > > > >    CONFIG_SYS_MALLOC_LEN=0x600000
> > > > > > > > +CONFIG_SYS_MALLOC_F_LEN=0x10000
> > > > > > >
> > > > > > > @Heiko
> > > > > > > Should we really adjust this on board level? Won't we have the same
> > > > > > > problem on all imx8m boards?
> > > > > > >
> > > > > > > Why don't you change the default for all i.mx8 boards in /Kconfig?
> > > > > > >
> > > > > > > @Tom, @Simon
> > > > > > > Shouldn't we replace the default of 0x400 by 0x2000 generally?
> > > > > >
> > > > > > I don't think that is a good idea. That is a lot of memory! Many
> > > > > > platforms don't need that much.
> > > > > >
> > > > > > I wonder what is driving this large amount. Is it pinctrl?
> > > > >
> > > > > The increase comes from the introduction of a clock driver for the
> > > > > imx8mq platform.
> > > >
> > > > Yes, the problem is that CCF creates a udevice+clk+private data for
> > > > every clock. This runs about 150-200 bytes per clock on a 64-bit
> > > > platform. In addition, many physical clocks are modeled as several
> > > > logical clocks plus a composite. This means a platform with maybe
> > > > 20-30 physical clocks can easily allocate 10k-20k to create
> > > > the clock tree.
> > >
> > > And to cycle back to what I mentioned on IRC at some point, I disagree
> > > with the notion many platforms don't need "that" might.  Yes, 0x10000
> > > tends to be on the higher end, but there's larger still boards, but we
> > > should likely set a new default X if DM (and similar, default Y if
> > > SPL_DM, and Z for TPL) and bump the boards up that are below that but
> > > more than current default, to that default.  A value of 0x400 is
> > > unreasonably small for DM and likely anything less than 0x4000 (what
> > > sandbox picks) is going to be problematic.
> >
> > Perhaps we can set the default for something that doesn't use CCF, to
> > start? It seems that CCF can really blow things up, so we could have a
> > larger default when that is used.
>
> I think this is just the example of the day of the more general issue.
> There's 369 boards setting a non-default value for
> CONFIG_SYS_MALLOC_F_LEN.  A bunch of them are overriding their already
> non-0x400 default provided in ./Kconfig.  More attention and thought
> about what to do here is needed.  And probably bumping the default for
> DM cases up.

Yes, makes sense. Increasing the default from 0x400 should help,
although if something is tight then it won't boot.

We sort-of have a way to discover the effective config values with the
moveconfig tool now, and the -i option allows looking up values that
are implied (although I have not tried it with a 'hex' config.

Also, if there is plenty of RAM on imx8 (or some other SoC), then we
may as well use it, at least by default.

I'm avoiding any new endeavours while so much is outstanding, so I
hope someone else can step up to look at this.

Regards,
Simon