Hi Pali, On Mon, 12 Sept 2022 at 15:58, Sean Anderson <sean.ander...@seco.com> wrote: > > > > 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 <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. > > > > 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.
Good point, Sean. Also LTO take away most//all of the cost of making a static function global. Regards, Simon