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

[Sean Anderson]

On 9/12/22 2:56 PM, Pali Rohár wrote:
> 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.
> 

You can use TEST_STATIC from test/export.h this case.

--Sean