[U-Boot] [RFC PATCH 08/11] sunxi: SPL: add FIT config selector for Pine64 boards

[Siarhei Siamashka <siarhei.siamashka@gmail.com>]

On Fri, 20 Jan 2017 21:55:53 +0000
Andr? Przywara <andre.przywara@arm.com> wrote:

> On 20/01/17 21:35, Maxime Ripard wrote:
> 
> Hi Maxime,
> 
> thanks for having a look!
> 
> > On Fri, Jan 20, 2017 at 01:53:28AM +0000, Andre Przywara wrote:  
> >> For a board or platform to support FIT loading in the SPL, it has to
> >> provide a board_fit_config_name_match() routine, which helps to select
> >> one of possibly multiple DTBs contained in a FIT image.
> >> Provide a simple function to cover the two different Pine64 models,
> >> which can be easily told apart by looking at the amount of installed
> >> RAM.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >> ---
> >>  board/sunxi/board.c | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> >> index 5365638..bbbb826 100644
> >> --- a/board/sunxi/board.c
> >> +++ b/board/sunxi/board.c
> >> @@ -726,3 +726,16 @@ int ft_board_setup(void *blob, bd_t *bd)
> >>  #endif
> >>  	return 0;
> >>  }
> >> +
> >> +#ifdef CONFIG_SPL_LOAD_FIT
> >> +int board_fit_config_name_match(const char *name)
> >> +{
> >> +#ifdef CONFIG_MACH_SUN50I
> >> +	if ((gd->ram_size > 512 * 1024 * 1024))
> >> +		return !strcmp(name, "sun50i-a64-pine64-plus");
> >> +	else
> >> +		return !strcmp(name, "sun50i-a64-pine64");
> >> +#endif
> >> +	return -1;
> >> +}  
> > 
> > That looks fishy. It means that any A64 board with CONFIG_SPL_LOAD_FIT
> > enabled will be considered a pine64 board?  
> 
> Yes, at least for now, because that's the only A64 board we officially
> support so far.
> And yes again, it is fishy. It is more a demo or a placeholder for now,
> because you _need_ an implementation of board_fit_config_name_match()
> once you enable CONFIG_SPL_LOAD_FIT.
> 
> One solution would be to have only one DTB supported per build, so
> board_fit_config_name_match() always returns 0.
> 
> Another (better) solution would be to store the board name in the SPL
> header, which could be done later when flashing the image. Then this
> function would just return strcmp(name, spl_header_name) or 0 if no name
> is found for whatever reason.

Yes, this is a good solution in the case if we have some non-removable
storage on the board (SPI NOR flash, NAND, EEPROM or something else).
We can discuss it separately.

But the current generation of Pine64 boards don't have any
non-removable storage yet. My understanding is that you tried to
provide the DTB selection, which is based on circumstantial evidences,
such as the RAM size. IMHO this is also a valid selection method,
because it can reduce the number of required OS image variants.

Yes, this is not urgent and we can indeed temporarily use separate
defconfigs for different Pine64 board variants.

> Ideas welcome. To be honest, this .dtb selection is nice, but I am not
> married to it. It usefulness maybe limited at the moment.

The board specific information is normally stored either in the
defconfig file or in the device tree, rather than gets hardcoded in
the sources. We can probably add some extra constraints information
to the device tree. And these extra constraints can be passed to the
board_fit_config_name_match() function in some way.

Then the sun50i-a64-pine64 device tree file can specify that this board
is expected to have exactly 512 MiB of RAM. Having this information,
the board_fit_config_name_match() function will fail to match it if
the actual RAM size is wrong.

This approach is similar to what I used in
    https://github.com/ssvb/sunxi-bootsetup/releases/tag/20141215-sunxi-bootsetup-prototype
    http://lists.denx.de/pipermail/u-boot/2015-January/202306.html
    
The sunxi-bootsetup stub used the SoC type id and RAM size to filter
the list of potentially matching boards. It worked very nicely in
practice.

-- 
Best regards,
Siarhei Siamashka