Kever,
On 13 Apr 2018, at 09:51, Kever Yang kever.yang@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