On Mon, Jan 23, 2017 at 10:55:55PM +0000, André Przywara wrote: > On 23/01/17 17:29, Maxime Ripard wrote: > > On Sat, Jan 21, 2017 at 03:15:27PM +0000, André Przywara wrote: > >>>>> 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.przyw...@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 > > > > I guess an easy way around this would be to add a Kconfig option for > > the pine64, like I tried to do for the CHIP (but never ended up > > merging). That way, at least we won't impact the other board, and we > > can have that default. > > Yes, that sounds indeed like a possibility. > > I came up with this snippet yesterday (rough copy): > > /* Check if there is a DT name stored in the SPL header and use that. */ > if (spl->offset_default_dt_name) { > cmp_str = SPL_ADDR + spl->offset_default_dt_name; > } else { > #ifdef CONFIG_DEFAULT_DEVICE_TREE > cmp_str = CONFIG_DEFAULT_DEVICE_TREE; > #else > return 0; > #endif > }; > > /* Differentiate the two Pine64 board DTs by their DRAM size. */ > if (strstr(name, "-pine64") && strstr(cmp_str, "-pine64")) { > if ((gd->ram_size > 512 * 1024 * 1024)) > return !strstr(name, "plus"); > else > return !!strstr(name, "plus"); > } else { > return strcmp(name, cmp_str); > } > > That admittedly doesn't look very pretty, but should cover all the > cases: DT name from SPL header, default compile time DT, Pine64 check. > We could also guard the last part with a CONFIG_SUNXI_PINE64_DETECT > symbol to save some space in case this is not needed. > > Does that make sense?
Yep, that works for me (with the config option) Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
signature.asc
Description: PGP signature