Hi Pali,
On Sun, 11 Sept 2022 at 03:39, Pali Rohár <p...@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 <p...@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
