On 21/01/17 04:05, Siarhei Siamashka wrote:
Hi Siarhei,
thanks for your comments!
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.
I totally agree. I rebased your patch to latest mainline already and will send out something later.
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, the point is that for U-Boot's purposes the only difference between the Pine64 and Pine64+ (apart from DRAM size, which is autodetected) is using MII vs. RGMII for the Ethernet PHY. The sun8i_emac driver gets this information from the DT only, so there is no point at all for different defconfigs. And the DRAM size is a safe indicator for that difference, at least if we confine our view to Pine64 boards.
Now how other boards fit in here is a separate discussion IMO, so let's solve one problem after the other. I just thought that we could use:
#ifdef CONFIG_SPL_LOAD_FIT int board_fit_config_name_match(const char *name) { return strcmp(name, CONFIG_DEFAULT_DEVICE_TREE); } #endif
for any general case.
Yes, this is not urgent and we can indeed temporarily use separate defconfigs for different Pine64 board variants.
I agree it's not a showstopper and we can work out a solution later. But in the long run I'd really like to have one firmware build suitable for the two Pine64 models, so we just need to figure out how to fit the DRAM size comparison code in here, maybe via some board specific #define to use the routine from this patch vs. the generic routine above?
Btw: I think I tested a non-plus Pine64 some time ago and (Fast) Ethernet didn't work out of the box back then, so I guess at the moment we support the Pine64+ only anyway.
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.
The problem is that at the moment this function gets only passed the configuration _name_ from the FIT property, which happens to be the DT name. At this point the DT is not loaded yet, so the function cannot take the actual DT into account (if this what you are hinting at).
And I am not sure if it's worth to actually load any DTs you want to check and do the actual comparison with the DT and not just the name. Sounds like a bit of a stretch to me.
This approach is similar to what I used in https://github.com/ssvb/sunxi-bootsetup/releases/tag/20141215-sunxi-bootsetu... http://lists.denx.de/pipermail/u-boot/2015-January/202306.html
Personally I am a big fan of using one image to rule them all (or at least many of them), but I don't think we are there yet. Also it dos not seems to be very popular with others. So let's get the basics upstream first and then take a look at how we can achieve this.
Are you around at FOSDEM? Then we could discuss this there more efficiently.
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.
I need to check your code and the email threads from back then, but in general I agree that this is the ultimate goal.
Cheers, Andre.