[PATCH 1/1] arm: imx8m: imx8mm-cl-iot-gate: Add support for detect memory size

[PaulLiu]

Hi Marcel,

Thanks for the comment.
But isn't it more straightforward to use TCM data?
I mean it is already tested when DRAM initializes.
Also there's already a commit that avoids U-boot using address more than
4GB on imx8m platform for some reasons I don't know in detail but the
commit is e27bddff .
Because the base is 0x40000000. So 4GB address means it is about 3G left.
But the largest memory we can purchase is 4GB. Which is already beyond the
limit.
And I don't have Compluab 4GB boards.
So I think we should avoid directly testing the address more than 4GB addr?
We just report the correct memory size here based on the DRAM init test.
And restrict the memory usage by board_get_usable_ram_top() by previous
commit.

The Compulab board is using various memory chips. They only let you to
choose the size, not the company/brand. So the dram init code actually test
every brand of the chips at beginning (SPL) so we have the correct brand
and size stored in TCM area. I have 5 Compulab boards on my hand. All of
them are 2GB ram but different brand. But it is still not covered all the
combinations I think.

So, can we keep using the TCM data rather than do the actual testing?

Yours,
Paul






On Mon, Aug 23, 2021 at 11:14 PM Marcel Ziswiler <
marcel.ziswiler at toradex.com> wrote:

> Hi Paul
>
> On Mon, 2021-08-23 at 10:49 +0800, Ying-Chun Liu wrote:
> > From: "Ying-Chun Liu (PaulLiu)" <paulliu at debian.org>
> >
> > When purchasing imx8mm-cl-iot-gate it is able to customize the
> > memory size. It could be 1GB, 2GB and 4GB. We implement
> > board_phys_sdram_size() to detect the memory size for usage.
> >
> > Signed-off-by: Ying-Chun Liu (PaulLiu) <paulliu at debian.org>
> > Cc: Fabio Estevam <festevam at denx.de>
> > Cc: Frieder Schrempf <frieder.schrempf at kontron.de>
> > Cc: uboot-imx <uboot-imx at nxp.com>
> > ---
> >  .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   | 25 +++++++++++++++++++
> >  include/configs/imx8mm-cl-iot-gate.h          |  2 +-
> >  2 files changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> b/board/compulab/imx8mm-cl-iot-gate/imx8mm-
> > cl-iot-gate.c
> > index eabcc842a4..01c6011b75 100644
> > --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> > +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> > @@ -14,8 +14,33 @@
> >  #include <asm/arch/sys_proto.h>
> >  #include <asm/io.h>
> >
> > +#include "ddr/ddr.h"
> > +
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > +int board_phys_sdram_size(phys_size_t *size)
> > +{
> > +       struct lpddr4_tcm_desc *lpddr4_tcm_desc =
> > +               (struct lpddr4_tcm_desc *)TCM_DATA_CFG;
> > +
> > +       switch (lpddr4_tcm_desc->size) {
> > +       case 4096:
> > +       case 2048:
> > +       case 1024:
> > +               *size = (1L << 20) * lpddr4_tcm_desc->size;
> > +               break;
> > +       default:
> > +               printf("%s: DRAM size %uM is not supported\n",
> > +                      __func__,
> > +                      lpddr4_tcm_desc->size);
> > +               while (1)
> > +                       ;
> > +               break;
> > +       };
>
> Why not simply using generic get_ram_size() as we e.g. did here [1]?
>
> > +
> > +       return 0;
> > +}
> > +
> >  static int setup_fec(void)
> >  {
> >         if (IS_ENABLED(CONFIG_FEC_MXC)) {
> > diff --git a/include/configs/imx8mm-cl-iot-gate.h
> b/include/configs/imx8mm-cl-iot-gate.h
> > index faeee2178c..1e835563d6 100644
> > --- a/include/configs/imx8mm-cl-iot-gate.h
> > +++ b/include/configs/imx8mm-cl-iot-gate.h
> > @@ -158,7 +158,7 @@
> >
> >  #define CONFIG_SYS_SDRAM_BASE          0x40000000
> >  #define PHYS_SDRAM                     0x40000000
> > -#define PHYS_SDRAM_SIZE                        0x80000000 /* 2GB DDR */
> > +#define PHYS_SDRAM_SIZE                        0x40000000 /* 1GB DDR */
>
> In our implementation we use PHYS_SDRAM_SIZE as the upper limit which
> get_ram_size() then actually tests for.
>
> >  #define CONFIG_MXC_UART_BASE           UART3_BASE_ADDR
>
> [1] https://marc.info/?l=u-boot&m=160387919927218
>
> Cheers
>
> Marcel
>