Kever, > On 13 Apr 2018, at 09:51, Kever Yang <kever.y...@rock-chips.com> wrote: > On 04/08/2018 09:45 AM, Kever Yang wrote: > >>>> +__weak int arch_cpu_init(void) >>>> +{ >>>> + return 0; >>>> +} >>>> + >>>> +__weak int rk_board_init_f(void) >>>> +{ >>>> + return 0; >>>> +} >>> This doesn't really help in modularising our board-support and I am >>> not a fan of adding something like 'rk_board_init_f' in the first place. >>> >>> Instead this should be implemented in a way that actually makes the >>> code structure easier and more resilient for the future (having __weak >>> functions at the architecture-level doesn't really help)... in fact >>> the only other uses of __weak in the U-Boot source-base are within >>> SPL, as there's no other way to provide hooks there. >> I know your proposal is to use DM for board init, then could you make it >> more >> clear about how to handle this in your solution? >> We need to do: >> - same board init flow for all rockchip platform; >> - something different but common in soc level; >> - something different in board level; > > I didn't see your response for this, could you send out your patches?
This isn’t at the stage of a patch-set yet… I had asked for comments to this, so we could design this in a way that benefits all platforms. > I admit that I'm not very clear about the limitation of '__weak' function, > but I do see there are many '__weak' function in common/board_f/r.c, > and my common board file is connect to the board_r.c. I like __weak as a way to provide a hook for something that is part of the common API (so it’s ok, if spl.c uses this). However, I don’t want individual platforms to suddenly expose new extension points. And with the two examples above (arch_cpu_init and rk_board_init_f), you basically highlight what’s wrong about using __weak at this level: 1 arch_cpu_init is an extension point to do low-level initialisation for a CPU (not a board). Implementation for it usually live below arch/arm/cpu and takes care of things like MMU maintenance. Now we suddenly provide this below arch/arm/mach-rockchip … and using a __weak function. This goes against everything that users will expect. So just move it to arch/arm/cpu (you’ll probably need to have 2 separate ones for armv7 and armv8) and nothing unexpected will ever happen. 2 If we rk_board_init_f here, we are again changing the extension API of U-Boot: board_init_f belongs to each board (i.e. any board can expect to override it w/o ill effect), but now you’d suddenly create a link error. Instead users need to override rk_board_init_f. This is a documentation nightmare (and the current solution would be to provide a common function that all board_init_f implementations could call as their head or tail…). My question—as to whether the DM could/should be extended to CPUs, SoCs, architectures and boards—was meant to discuss exactly these observed issues in how boards and SoCs today interact. I usually don’t mind to touch APIs and extend them (or the driver model), but a solution for how to handle board/SoC/CPU init sequences is nothing I want to start before getting an actual design discussion going and reaching something resembling a consensus of how this aspect of U-Boot should be structured in a year’s (or two years’) time. > > @Simon, @Tom, > Could you kindly give some comment here? > > Thanks, > - Kever _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot