* [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN @ 2022-03-21 14:26 Heiko Thiery 2022-03-21 14:30 ` Fabio Estevam 2022-03-22 9:25 ` Heinrich Schuchardt 0 siblings, 2 replies; 18+ messages in thread From: Heiko Thiery @ 2022-03-21 14:26 UTC (permalink / raw) To: u-boot; +Cc: Stefano Babic, Fabio Estevam, Peng Fan, Heiko Thiery 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 CONFIG_SPL_GPIO=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y -- 2.30.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-21 14:26 [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN Heiko Thiery @ 2022-03-21 14:30 ` Fabio Estevam 2022-03-22 9:25 ` Heinrich Schuchardt 1 sibling, 0 replies; 18+ messages in thread From: Fabio Estevam @ 2022-03-21 14:30 UTC (permalink / raw) To: Heiko Thiery; +Cc: U-Boot-Denx, Stefano Babic, Peng Fan Hi Heiko, On Mon, Mar 21, 2022 at 11:26 AM Heiko Thiery <heiko.thiery@gmail.com> 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 Thanks for the rework: Reviewed-by: Fabio Estevam <festevam@denx.de> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-21 14:26 [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN Heiko Thiery 2022-03-21 14:30 ` Fabio Estevam @ 2022-03-22 9:25 ` Heinrich Schuchardt 2022-03-22 12:25 ` Tom Rini 2022-03-23 18:04 ` Simon Glass 1 sibling, 2 replies; 18+ messages in thread From: Heinrich Schuchardt @ 2022-03-22 9:25 UTC (permalink / raw) To: Heiko Thiery, Tom Rini, Simon Glass Cc: Stefano Babic, Fabio Estevam, Peng Fan, u-boot 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? Best regards Heinrich > CONFIG_SPL_GPIO=y > CONFIG_SPL_LIBCOMMON_SUPPORT=y > CONFIG_SPL_LIBGENERIC_SUPPORT=y ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-22 9:25 ` Heinrich Schuchardt @ 2022-03-22 12:25 ` Tom Rini 2022-03-22 12:34 ` Heiko Thiery 2022-03-23 18:04 ` Simon Glass 1 sibling, 1 reply; 18+ messages in thread From: Tom Rini @ 2022-03-22 12:25 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Heiko Thiery, Simon Glass, Stefano Babic, Fabio Estevam, Peng Fan, u-boot [-- Attachment #1: Type: text/plain, Size: 1393 bytes --] On Tue, Mar 22, 2022 at 10:25:12AM +0100, Heinrich Schuchardt 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? That's well more than the default for imx8 of 0x2000 and even for the default for sandbox of 0x4000, does it need to be that huge? Or was that the "I'm sick of this problem" value? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-22 12:25 ` Tom Rini @ 2022-03-22 12:34 ` Heiko Thiery 2022-03-23 13:14 ` Tom Rini 0 siblings, 1 reply; 18+ messages in thread From: Heiko Thiery @ 2022-03-22 12:34 UTC (permalink / raw) To: Tom Rini Cc: Heinrich Schuchardt, Simon Glass, Stefano Babic, Fabio Estevam, Peng Fan, u-boot Hi Tom, Am Di., 22. März 2022 um 13:25 Uhr schrieb Tom Rini <trini@konsulko.com>: > > On Tue, Mar 22, 2022 at 10:25:12AM +0100, Heinrich Schuchardt 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? > > That's well more than the default for imx8 of 0x2000 and even for the > default for sandbox of 0x4000, does it need to be that huge? Or was > that the "I'm sick of this problem" value? > I did test a pending patch that enables a DM clock driver for the imx8mq. With that patch a huge amount of memory seems to be needed for all that clock infrastructure. -- Heiko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-22 12:34 ` Heiko Thiery @ 2022-03-23 13:14 ` Tom Rini 2022-03-23 14:09 ` Adam Ford 0 siblings, 1 reply; 18+ messages in thread From: Tom Rini @ 2022-03-23 13:14 UTC (permalink / raw) To: Heiko Thiery Cc: Heinrich Schuchardt, Simon Glass, Stefano Babic, Fabio Estevam, Peng Fan, u-boot [-- Attachment #1: Type: text/plain, Size: 1960 bytes --] On Tue, Mar 22, 2022 at 01:34:34PM +0100, Heiko Thiery wrote: > Hi Tom, > > > Am Di., 22. März 2022 um 13:25 Uhr schrieb Tom Rini <trini@konsulko.com>: > > > > On Tue, Mar 22, 2022 at 10:25:12AM +0100, Heinrich Schuchardt 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? > > > > That's well more than the default for imx8 of 0x2000 and even for the > > default for sandbox of 0x4000, does it need to be that huge? Or was > > that the "I'm sick of this problem" value? > > > > I did test a pending patch that enables a DM clock driver for the > imx8mq. With that patch a huge amount of memory seems to be needed for > all that clock infrastructure. OK, then we should adjust upwards the default for imx8 in the toplevel Kconfig. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 13:14 ` Tom Rini @ 2022-03-23 14:09 ` Adam Ford 0 siblings, 0 replies; 18+ messages in thread From: Adam Ford @ 2022-03-23 14:09 UTC (permalink / raw) To: Tom Rini Cc: Heiko Thiery, Heinrich Schuchardt, Simon Glass, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List On Wed, Mar 23, 2022 at 8:14 AM Tom Rini <trini@konsulko.com> wrote: > > On Tue, Mar 22, 2022 at 01:34:34PM +0100, Heiko Thiery wrote: > > Hi Tom, > > > > > > Am Di., 22. März 2022 um 13:25 Uhr schrieb Tom Rini <trini@konsulko.com>: > > > > > > On Tue, Mar 22, 2022 at 10:25:12AM +0100, Heinrich Schuchardt 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? > > > > > > That's well more than the default for imx8 of 0x2000 and even for the > > > default for sandbox of 0x4000, does it need to be that huge? Or was > > > that the "I'm sick of this problem" value? > > > > > > > I did test a pending patch that enables a DM clock driver for the > > imx8mq. With that patch a huge amount of memory seems to be needed for > > all that clock infrastructure. > > OK, then we should adjust upwards the default for imx8 in the toplevel > Kconfig. I have been using 0x10000 for a while without issue on both an imx8mm and imx8mn. When I used that value, it was more of a desire to overshoot the minimal requirements so as features are added, we don't have to keep adjusting the memory. I support making it the platform default. adam > > -- > Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-22 9:25 ` Heinrich Schuchardt 2022-03-22 12:25 ` Tom Rini @ 2022-03-23 18:04 ` Simon Glass 2022-03-23 18:26 ` Heiko Thiery 1 sibling, 1 reply; 18+ messages in thread From: Simon Glass @ 2022-03-23 18:04 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Heiko Thiery, Tom Rini, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List 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? Anyway, changing it for imx8 seem right to me, as Tom says. Regards, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 18:04 ` Simon Glass @ 2022-03-23 18:26 ` Heiko Thiery 2022-03-23 18:33 ` Sean Anderson 0 siblings, 1 reply; 18+ messages in thread From: Heiko Thiery @ 2022-03-23 18:26 UTC (permalink / raw) To: Simon Glass Cc: Heinrich Schuchardt, Tom Rini, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List 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. > > Anyway, changing it for imx8 seem right to me, as Tom says. > > Regards, > Simon -- Heiko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 18:26 ` Heiko Thiery @ 2022-03-23 18:33 ` Sean Anderson 2022-03-23 18:49 ` Tom Rini 2022-03-23 18:53 ` Simon Glass 0 siblings, 2 replies; 18+ messages in thread From: Sean Anderson @ 2022-03-23 18:33 UTC (permalink / raw) To: Heiko Thiery, Simon Glass Cc: Heinrich Schuchardt, Tom Rini, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List 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. --Sean ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 18:33 ` Sean Anderson @ 2022-03-23 18:49 ` Tom Rini 2022-03-23 18:53 ` Simon Glass 2022-03-23 18:53 ` Simon Glass 1 sibling, 1 reply; 18+ messages in thread From: Tom Rini @ 2022-03-23 18:49 UTC (permalink / raw) To: Sean Anderson Cc: Heiko Thiery, Simon Glass, Heinrich Schuchardt, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List [-- Attachment #1: Type: text/plain, Size: 2854 bytes --] 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. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 18:49 ` Tom Rini @ 2022-03-23 18:53 ` Simon Glass 2022-03-23 18:59 ` Tom Rini 0 siblings, 1 reply; 18+ messages in thread From: Simon Glass @ 2022-03-23 18:53 UTC (permalink / raw) To: Tom Rini Cc: Sean Anderson, Heiko Thiery, Heinrich Schuchardt, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List 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. Regards, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 18:53 ` Simon Glass @ 2022-03-23 18:59 ` Tom Rini 2022-03-23 19:38 ` Simon Glass 0 siblings, 1 reply; 18+ messages in thread From: Tom Rini @ 2022-03-23 18:59 UTC (permalink / raw) To: Simon Glass Cc: Sean Anderson, Heiko Thiery, Heinrich Schuchardt, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List [-- Attachment #1: Type: text/plain, Size: 3789 bytes --] 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. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 18:59 ` Tom Rini @ 2022-03-23 19:38 ` Simon Glass 0 siblings, 0 replies; 18+ messages in thread From: Simon Glass @ 2022-03-23 19:38 UTC (permalink / raw) To: Tom Rini Cc: Sean Anderson, Heiko Thiery, Heinrich Schuchardt, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 18:33 ` Sean Anderson 2022-03-23 18:49 ` Tom Rini @ 2022-03-23 18:53 ` Simon Glass 2022-03-23 19:19 ` Sean Anderson 1 sibling, 1 reply; 18+ messages in thread From: Simon Glass @ 2022-03-23 18:53 UTC (permalink / raw) To: Sean Anderson Cc: Heiko Thiery, Heinrich Schuchardt, Tom Rini, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List Hi Sean, On Wed, 23 Mar 2022 at 12:33, Sean Anderson <seanga2@gmail.com> 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. With the new DM tag support we could move uncommon fields in struct udevice to tags, such as parent_plat_, uclass_plat_, driver_data, uclass_priv_, parent_priv_, dma_offset. It would add a small amount of code but save data. We could call this DM_TINY_DATA and It might save 64 bytes per device. Also the #ifdef CONFIG_DEVRES should use CONFIG_IS_ENABLED(DEVRES) so that devres_head is not included in SPL. Regards, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 18:53 ` Simon Glass @ 2022-03-23 19:19 ` Sean Anderson 2022-03-23 19:38 ` Simon Glass 2022-03-23 20:03 ` Adam Ford 0 siblings, 2 replies; 18+ messages in thread From: Sean Anderson @ 2022-03-23 19:19 UTC (permalink / raw) To: Simon Glass Cc: Heiko Thiery, Heinrich Schuchardt, Tom Rini, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List On 3/23/22 2:53 PM, Simon Glass wrote: > Hi Sean, > > On Wed, 23 Mar 2022 at 12:33, Sean Anderson <seanga2@gmail.com> 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. > > With the new DM tag support we could move uncommon fields in struct > udevice to tags, such as parent_plat_, uclass_plat_, driver_data, > uclass_priv_, parent_priv_, dma_offset. It would add a small amount of > code but save data. We could call this DM_TINY_DATA and It might save > 64 bytes per device. > > Also the #ifdef CONFIG_DEVRES should use CONFIG_IS_ENABLED(DEVRES) so > that devres_head is not included in SPL. Personally, I think we could get away with a much smaller amount of data per-clock, and not make every clock a separate device. The primary reason each CCF clock gets its own device is that we cache things like clock rates. Because of this, we need to keep track of children so we can invalidate their rates correctly. But caching rates is probably not that much faster than just recalculating (especially since most of the time it's just a division). We also don't try and find the best way to achieve a clock speed like Linux does. If we don't cache clock rates, we don't need to keep track of a clock's children. Further, we could likely also eliminate tracking enable/disable counts. We don't have fine-grained low power states like Linux does, so generally when someone disables a clock we're getting in the way by trying to keep track of the count. This is all a rather big change, so we're stuck with the status quo for now, but it's the direction I would like to move in going ahead. I know Marek would rather just not create as many clocks in SPL/pre-reloc. --Sean ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 19:19 ` Sean Anderson @ 2022-03-23 19:38 ` Simon Glass 2022-03-23 20:03 ` Adam Ford 1 sibling, 0 replies; 18+ messages in thread From: Simon Glass @ 2022-03-23 19:38 UTC (permalink / raw) To: Sean Anderson Cc: Heiko Thiery, Heinrich Schuchardt, Tom Rini, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List Hi Sean, On Wed, 23 Mar 2022 at 13:19, Sean Anderson <seanga2@gmail.com> wrote: > > On 3/23/22 2:53 PM, Simon Glass wrote: > > Hi Sean, > > > > On Wed, 23 Mar 2022 at 12:33, Sean Anderson <seanga2@gmail.com> 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. > > > > With the new DM tag support we could move uncommon fields in struct > > udevice to tags, such as parent_plat_, uclass_plat_, driver_data, > > uclass_priv_, parent_priv_, dma_offset. It would add a small amount of > > code but save data. We could call this DM_TINY_DATA and It might save > > 64 bytes per device. > > > > Also the #ifdef CONFIG_DEVRES should use CONFIG_IS_ENABLED(DEVRES) so > > that devres_head is not included in SPL. > > Personally, I think we could get away with a much smaller amount of data > per-clock, and not make every clock a separate device. The primary reason > each CCF clock gets its own device is that we cache things like clock > rates. Because of this, we need to keep track of children so we can > invalidate their rates correctly. But caching rates is probably not that > much faster than just recalculating (especially since most of the time > it's just a division). We also don't try and find the best way to achieve > a clock speed like Linux does. If we don't cache clock rates, we don't > need to keep track of a clock's children. > > Further, we could likely also eliminate tracking enable/disable counts. > We don't have fine-grained low power states like Linux does, so generally > when someone disables a clock we're getting in the way by trying to keep > track of the count. > > This is all a rather big change, so we're stuck with the status quo for > now, but it's the direction I would like to move in going ahead. I know > Marek would rather just not create as many clocks in SPL/pre-reloc. This all makes sense. But updating 'struct udevice' would benefit every subsystem, particular arm64. The struct is just too damn big. Regards, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN 2022-03-23 19:19 ` Sean Anderson 2022-03-23 19:38 ` Simon Glass @ 2022-03-23 20:03 ` Adam Ford 1 sibling, 0 replies; 18+ messages in thread From: Adam Ford @ 2022-03-23 20:03 UTC (permalink / raw) To: Sean Anderson Cc: Simon Glass, Heiko Thiery, Heinrich Schuchardt, Tom Rini, Stefano Babic, Fabio Estevam, Peng Fan, U-Boot Mailing List On Wed, Mar 23, 2022 at 2:19 PM Sean Anderson <seanga2@gmail.com> wrote: > > On 3/23/22 2:53 PM, Simon Glass wrote: > > Hi Sean, > > > > On Wed, 23 Mar 2022 at 12:33, Sean Anderson <seanga2@gmail.com> 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. > > > > With the new DM tag support we could move uncommon fields in struct > > udevice to tags, such as parent_plat_, uclass_plat_, driver_data, > > uclass_priv_, parent_priv_, dma_offset. It would add a small amount of > > code but save data. We could call this DM_TINY_DATA and It might save > > 64 bytes per device. > > > > Also the #ifdef CONFIG_DEVRES should use CONFIG_IS_ENABLED(DEVRES) so > > that devres_head is not included in SPL. > > Personally, I think we could get away with a much smaller amount of data > per-clock, and not make every clock a separate device. The primary reason > each CCF clock gets its own device is that we cache things like clock > rates. Because of this, we need to keep track of children so we can > invalidate their rates correctly. But caching rates is probably not that > much faster than just recalculating (especially since most of the time > it's just a division). We also don't try and find the best way to achieve > a clock speed like Linux does. If we don't cache clock rates, we don't > need to keep track of a clock's children. > > Further, we could likely also eliminate tracking enable/disable counts. > We don't have fine-grained low power states like Linux does, so generally > when someone disables a clock we're getting in the way by trying to keep > track of the count. > > This is all a rather big change, so we're stuck with the status quo for > now, but it's the direction I would like to move in going ahead. I know > Marek would rather just not create as many clocks in SPL/pre-reloc. We have config options to enable/disable peripherals. For the imx8m CCF, what if we inserted a series of ifdefs for the various peripherals. If peripheral-X is configured, enable the corresponding clocks. It doesn't necessarily make sense to me to load a bunch of clocks in SPL if the corresponding peripherals are not used, but in U-Boot they might be. It would make the CCF a bit more ugly, but it could help conserve some RAM in SPL/TPL. adam > > --Sean > ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-03-23 20:03 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-21 14:26 [PATCH v2] board: kontron: increase the CONFIG_SYS_MALLOC_F_LEN Heiko Thiery 2022-03-21 14:30 ` Fabio Estevam 2022-03-22 9:25 ` Heinrich Schuchardt 2022-03-22 12:25 ` Tom Rini 2022-03-22 12:34 ` Heiko Thiery 2022-03-23 13:14 ` Tom Rini 2022-03-23 14:09 ` Adam Ford 2022-03-23 18:04 ` Simon Glass 2022-03-23 18:26 ` Heiko Thiery 2022-03-23 18:33 ` Sean Anderson 2022-03-23 18:49 ` Tom Rini 2022-03-23 18:53 ` Simon Glass 2022-03-23 18:59 ` Tom Rini 2022-03-23 19:38 ` Simon Glass 2022-03-23 18:53 ` Simon Glass 2022-03-23 19:19 ` Sean Anderson 2022-03-23 19:38 ` Simon Glass 2022-03-23 20:03 ` Adam Ford
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.