[PATCH] board_f: show_dram_config: Print also real DRAM size

[Pali Rohár]

On Monday 12 September 2022 07:34:47 Simon Glass wrote:
> Hi Pali,
> 
> On Sun, 11 Sept 2022 at 03:39, Pali Rohár <pali at kernel.org> wrote:
> >
> > 32-bit U-Boot builds cannot use more than around 2 GB of DDR memory. But on
> > some platforms/boards it is possible to connect also 4 GB SODIMM DDR memory.
> > U-Boot currently prints only effective size of RAM which can use, which may
> > be misleading as somebody would expect that this line prints total size of
> > connected DDR modules. So change show_dram_config code to prints both real
> > and effective DRAM size if they are different. If they are same then print
> > just one number like before. It is possible that effective size is just few
> > bytes smaller than the real size, so print both numbers only in case
> > function print_size() prints formats them differently.
> >
> > Signed-off-by: Pali Rohár <pali at kernel.org>
> > ---
> >  common/board_f.c | 31 ++++++++++++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/board_f.c b/common/board_f.c
> > index 9e34fbee147e..3131a06db940 100644
> > --- a/common/board_f.c
> > +++ b/common/board_f.c
> > @@ -54,6 +54,7 @@
> >  #include <asm/sections.h>
> >  #include <dm/root.h>
> >  #include <linux/errno.h>
> > +#include <linux/log2.h>
> >
> >  /*
> >   * Pointer to initial global data area
> > @@ -213,6 +214,30 @@ static int announce_dram_init(void)
> >         return 0;
> >  }
> >
> > +/*
> > + * Check if the sizes in their natural units written in decimal format with
> > + * one fraction number are same.
> > + */
> > +static int sizes_near(unsigned long long size1, unsigned long long size2)
> > +{
> > +       unsigned int size1_scale = ilog2(size1) / 10 * 10;
> > +       unsigned int size1_val = (10 * size1 + ((1ULL << size1_scale) >> 1)) >> size1_scale;
> > +       unsigned int size2_scale = ilog2(size2) / 10 * 10;
> > +       unsigned int size2_val = (10 * size2 + ((1ULL << size2_scale) >> 1)) >> size2_scale;
> 
> Can you put that expression into a function with a comment, etc.? It
> is a bit hard to understand.

Ok.

> > +
> > +       if (size1_val == 10240) {
> > +               size1_val = 10;
> > +               size1_scale += 10;
> > +       }
> > +
> > +       if (size2_val == 10240) {
> > +               size2_val = 10;
> > +               size2_scale += 10;
> > +       }
> 
> If you are doing the same thing to each, why bother? It should not
> affect the expression below, should it? :

This is interesting question, and the answer it that it is required and
affects comparison expression below. For example for the case when size1
is below 1GB limit, size2 is above 1GB limit and both values are near.
Imagine that size1 is approaching value 1GB from the left and size2 from
the right side.

> > +
> > +       return size1_scale == size2_scale && size1_val == size2_val;
> > +}
> > +
> >  static int show_dram_config(void)
> >  {
> >         unsigned long long size;
> > @@ -229,7 +254,11 @@ static int show_dram_config(void)
> >         }
> >         debug("\nDRAM:  ");
> >
> > -       print_size(size, "");
> > +       print_size(gd->ram_size, "");
> > +       if (!sizes_near(gd->ram_size, size)) {
> > +               printf(" (effective ");
> > +               print_size(size, ")");
> > +       }
> >         board_add_ram_info(0);
> >         putc('\n');
> >
> > --
> > 2.20.1
> >
> 
> Can we make this testable somehow? You could put the new code into a
> lib/ function, perhaps, and call it from a C unit test in test/lib ?
> 
> Regards,
> Simon

Meh... I do not know how to test such code. Due to size / optimization
requirements it is not a good idea to make function outside of board_f.c
file.