[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Simon Glass]

Hi Bin,

On 11 April 2017 at 20:39, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Sat, Apr 1, 2017 at 12:21 PM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Tom,
>>
>> On 22 March 2017 at 09:13, Tom Rini <trini at konsulko.com> wrote:
>>> On Wed, Mar 22, 2017 at 08:43:54AM -0600, Simon Glass wrote:
>>>> Hi Tom,
>>>>
>>>> On 22 March 2017 at 08:37, Tom Rini <trini at konsulko.com> wrote:
>>>> > On Wed, Mar 22, 2017 at 07:05:38AM -0600, Simon Glass wrote:
>>>> >> Hi Tom,
>>>> >>
>>>> >> On 19 March 2017 at 18:47, Tom Rini <trini at konsulko.com> wrote:
>>>> >> > On Sun, Mar 19, 2017 at 12:59:19PM -0600, Simon Glass wrote:
>>>> >> >> At present we have a lot of ad-hoc init functions related to boards, for
>>>> >> >> example board_early_init_f(), board_misc_init_f() and dram_init().
>>>> >> >>
>>>> >> >> There are used in different ways by different boards as useful hooks to
>>>> >> >> do the required init and sequence it correctly. Some functions are always
>>>> >> >> enabled but have a __weak default. Some are controlled by the existence
>>>> >> >> of a CONFIG.
>>>> >> >>
>>>> >> >> There are two main init sequences: board_init_f() (f for running from
>>>> >> >> read-only flash) which runs before relocation and board_init_r() (r for
>>>> >> >> relocated) which runs afterwards.
>>>> >> >>
>>>> >> >> One problem with the current sequence is that it has a lot of
>>>> >> >> arch-specific #ifdefs around various functions. There are also #ifdefs
>>>> >> >> for various features. There has been quite a bit of discussion about how
>>>> >> >> to tidy this up and at least one RFC series[1].
>>>> >> >>
>>>> >> >> Now that we have driver model we can use this to deal with the init
>>>> >> >> sequences. This approach has several advantages:
>>>> >> >>
>>>> >> >> - We have a path to remove the #ifdefs
>>>> >> >> - It is easy for multiple parts of the code to implement the same hook
>>>> >> >> - We can track what is called and what is not
>>>> >> >> - We don't need weak functions
>>>> >> >> - We can eventually adjust the sequence to improve naming or to add new
>>>> >> >> init phases
>>>> >> >> - It provides a model for how we might deal with ft_board_setup() and
>>>> >> >> friends
>>>> >> >>
>>>> >> >> This series starts the process of replacing the pre-relocation init
>>>> >> >> sequence with a driver-model solution. It defines a uclass, adds tests
>>>> >> >> and converts sandbox and a few x86 boards over to use this new setup.
>>>> >> >>
>>>> >> >> This series is not ready for use yet as the rest of the init sequence
>>>> >> >> hooks need to be converted. But there is enough here to show the idea.
>>>> >> >>
>>>> >> >> Comments welcome.
>>>> >> >>
>>>> >> >> [1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html
>>>> >> >
>>>> >> > How does this look, size wise?  With all of these conversions and
>>>> >> > clean-ups, we really need to be size concious as well as it all keeps
>>>> >> > adding up.  Thanks!
>>>> >>
>>>> >> It include size a bit - e.g. x86 808 bytes of text, although that does
>>>> >> include a few extra features.
>>>> >
>>>> > How about if we don't include some of the extra debug/demo type features
>>>> > (which are useful at times, certainly) ?  We keep adding things that add
>>>> > a few bytes here and a few bytes there and it all adds up.
>>>>
>>>> Yes it's very important that the base version doesn't increase size,
>>>> or at least only minimally. I should have examined that more closely
>>>> in the RFC - my intent was really to get comments on the approach,
>>>>
>>>> >
>>>> > [snip]
>>>> >> I think I can use a linker-list approach to reduce the overhead. But I
>>>> >> still think the driver has value as it provides a means of adding
>>>> >> hooks to do board-specific things from drivers, something that we keep
>>>> >> running into. Also the idea of a board 'driver' seems conceptually
>>>> >> useful.
>>>> >>
>>>> >> So one approach would be to have:
>>>> >>
>>>> >> 1. A linker-list-based board hook setup, where you can declare, for example:
>>>> >>
>>>> >> static int ivybridge_dram_init(void)
>>>> >> {
>>>> >>  ...
>>>> >> }
>>>> >> U_BOOT_BOARD_HOOK(ivybridge_dram_init, BOARD_F_DRAM_INIT);
>>>> >>
>>>> >> This should be pretty cheap, perhaps <200 bytes with some care.
>>>> >>
>>>> >>
>>>> >> 2. An optional BOARD uclass which can be used for more involved
>>>> >> situations, but with a higher code size penalty.
>>>> >
>>>> > OK.  But I also recall that we had talked about trying to condense and
>>>> > re-factor things.  My worry about an approach like this is it allows for
>>>> > us to more easily get back into the bad habits of having each
>>>> > architecture solve similar problems with different solutions.
>>>>
>>>> Yes that's true and I've been pushing back on this for a while. For
>>>> example there is quite a bit of pressure to add board-specific init
>>>> code to drivers with driver model. So far I think we have been able to
>>>> avoid this using device tree and other drivers. For example if MMC
>>>> needs a clock we can find the required clock by phandle and call the
>>>> clock driver.
>>>>
>>>> So are you thinking we should limit this to just a simple hook
>>>> approach for now, and then consider the board uclass down the track?
>>>
>>> Looking over init_sequence_[rf], I can certainly see the case for "ug,
>>> this is ugly and we need to make it better" (and I now wonder if we
>>> don't have a lot more places where we need INIT_FUNC_WATCHDOG_RESET,
>>> anyhow...).  But for the moment we seem to be able to resist adding more
>>> calls here.  And I would like to see if we can rework / reduce the
>>> current table before we try and simplify and clean-up the mechanism that
>>> we use to handle them.
>>
>> I agree, and I have some concern that making it easier to extend the
>> init sequence might end up with less consistency between archs as to
>> the sequence we go through during init.
>>
>> For now I've done two series to tidy up board_f. There is more to do
>> though. We can park this series until we get a bit closer (it might be
>> quite a while).
>>
>
> I did not track this series. What's our next step regarding to this
> series? I see some of them are x86-specific which I can apply, no?

I think it is on hold for now. But yes you can apply the x86 patches.

I may come back to this later after the init sequence is tidied more,
hopefully later in the year.

Regards,
Simon