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

[Simon Glass]

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.

> +
> +       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? :

> +
> +       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