[U-Boot] [PATCH 1/3] config: introduce a generic $bootcmd

[Stephen Warren]

On 08/04/2014 04:13 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 31 July 2014 17:00, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 07/31/2014 04:03 PM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 30 July 2014 23:37, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>
>>>> From: Dennis Gilmore <dennis at ausil.us>
>>>>
>>>> This generic $bootcmd, and associated support macros, automatically
>>>> searches a defined set of storage devices (or network protocols) for an
>>>> extlinux configuration file or U-Boot boot script in various standardized
>>>> locations. Distros that install such a boot config file/script in those
>>>> standard locations will get easy-to-set-up booting on HW that enables
>>>> this generic $bootcmd.
>>>>
>>>> Boards can define the set of devices from which boot is attempted, and
>>>> the order in which they are attempted. Users may later customize this
>>>> set/order by edting $boot_targets.
>>>>
>>>> Users may interrupt the boot process and boot from a specific device
>>>> simply by executing e.g.:
>>>>
>>>> $ run bootcmd_mmc1
>>>> or:
>>>> $ run bootcmd_pxe
>>>>
>>>> This patch was originally written by Dennis Gilmore based on Tegra and
>>>> rpi_b boot scripts. I have made the following modifications since then:
>>>>
>>>> * Boards must define the BOOT_TARGET_DEVICES macro in order to specify
>>>>     the set of devices (and order) from which to attempt boot. If needed,
>>>>     we can define a default directly in config_distro_bootcmd.h.
>>>>
>>>> * Removed $env_import and related variables; nothing used them, and I
>>>>     think it's better for boards to pre-load an environment customization
>>>>     file using CONFIG_PREBOOT if they need.
>>>>
>>>> * Renamed a bunch of variables to suit my whims:-)
>>>>
>>>> Signed-off-by: Dennis Gilmore <dennis at ausil.us>
>>>> Signed-off-by: Stephen Warren <swarren at nvidia.com>
>>>
>>>
>>> I do understand the desirability of getting something sorted in this
>>> area. But I am not thrilled with all the macro magic. How does this
>>> fit with the new Kconfig setup? It encourages a single setting for
>>> each variable, and I feel that the #ifdefs are not compatible with
>>> that.
>>
>>
>> I think Kconfig would be completely unsuitable for this kind of detailed
>> configuration. Kconfig is great for enabling/disabling features, but
>> applying it here feels too much to me. Equally, Kconfig is rather new in
>> U-Boot. It'd be better to enable the feature so that distros can rely on it,
>> and then refactor it later if required.
>
> Are you saying that we can reimplement this in a nicer way and distros
> will still see the same behaviour?

I expect we could.

The only thing distros should rely upon is that if they put 
extlinux.conf in the right directory on their media, it will get loaded 
and executed.

There are obviously various ways that could be implemented in U-Boot, or 
indeed other bootloaders.

>> I do feel that actually implementing the boot script as U-Boot environment
>> variables is much preferred over hard-coding any algorithm into U-Boot. That
>> way, the user can easily modify the scripts as they desire. If we just put
>> e.g. $boot_targets into the environment or DT, but none of the other scripts
>> in this patch, the user would be much more severely restricted in how they
>> could reconfigure the system to act how they want.
>
> But that worries me. It means that it is easy for one board to deviate
> from what is essentially an undocumented boot standard, and then we
> will end up having to support that use case in the future.
>
> Or if it is documented, where is that?

I was talking about an end-user changing the boot process.

An individual board could only change the boot scripts by either not 
using config_distro_bootcmd.h, or by explicitly overriding something 
that it does. Either of those would simply mean that the board doesn't 
provide the standard boot environment to distros, and as such wouldn't 
be expected to boot distros in the standard way.

Note that all we're talking about here is that U-Boot can search all (or 
perhaps most) attached storage devices for extlinux.conf and interpret 
it correctly. This patch adds the "search for" part; the definition of 
"interpret it correctly" is already part of the implementation of the 
"pxe" and "sysboot" commands in U-Boot.

>>> Would it be possible to put the settings in the device tree somehow
>>> instead of CONFIGs?
>>
>>
>> At least part of the information isn't a HW description, but rather
>> user-/vendor configuration. So, I don't think it's appropriate to put this
>> into DT. Equally, very few U-Boot platforms currently use DT, and I
>> certainly hope it doesn't become mandatory in any way. So, using DT for this
>> purpose would severely limit the platforms where this feature was available.
>
> The only platforms I see support for in your series are Tegra (which
> has DT) and Raspberry PI.

Those are the only platforms I put into my patch set. In Dennis 
Gilmore's previous patch set, he converted 3 other platforms, and since 
I posted my series, someone posted a conversion for sunxi too.

> Adding basic DT support is not really
> onerous so doesn't impose any real limits on adoption.

It implies that the board/SoC maintainers buy into the idea that using 
DT is a useful thing to do for U-Boot. I believe there's plenty of 
disagreement with that on Tegra already, just not vocal.

> In any case I'm
> mostly just responding to what I see might become a large and
> mandatory script setup in U-Boot. Would it not be better now to
> document this clearly and specify that changes are not supported?

The fact that changes aren't allowed is rather implied by opting in to 
using the header in the first place.

Dennis has a patch that provides documentation for the concepts that he 
included in his series, upon which I based mine. I assume he'll respin 
that patch, since he received some comments on it when posted.

> Then
> we might be able to refactor it later and still retain compatibility.
> If we have a clear API separate from the implementation then it is
> easier to live with the scripts knowing we can do things a different
> way later.

That said, I'm not sure what aspect of documentation is needed. This 
patch doesn't introduce any new API. The patch is simply about searching 
for an extlinux.conf file and executing it. The important "ABI" things 
are implied by the definition of extlinux.conf (which already has 
documentation) not by this patch.

> Looking at this from a driver model perspective we would probably want
> to have a list of devices to try, in a certain order. Certainly driver
> model would support the approach in this series, but it would be a
> very ugly way of achieving the goal IMO.
>
> BTW in your series bootpart is always 1 - is that always the case for
> all boards?

For now yes. At some point, I did intend to enhance the scripts to use 
the "default" partition on the media, as defined by the partition 
table's bootable flag. I haven't implemented that yet. I expect that 
it'd work something like: unset $bootpart in order to use that "default" 
partition, or set it to some specific value in order to use that 
specific partition. IIRC, that's already how disk-oriented commands 
parse their command-line arguments.

> As to the question of HW description, I'm not sure I follow. The
> devices that are attached to the hardware presumably appear in the DT,
> the partition is fixed anyway, all you need to know is whether it is a
> bootable device or not. What part of the description is not related to
> the hardware?

The concept of bootable, and the order in which bootable devices should 
be searched, are SW policy, not HW definition.

> In trying to figure out what was going on, I was hampered by the fact
> that autoconf.mk does not get the full environment (e.g. since BOOTDEV
> doesn't have a CONFIG_ prefix). I can see what it doesn't, I think.

I don't quite understand how the contents of autoconf.mk is relevant. 
The scripts that config_distro_bootcmd.h defines are self-contained, so 
I think you can just read that file.

>>> I did send a series some time ago that aimed to improve the default
>>> environment specification in config files - it was parked while
>>> Kconfig was going on, but we could revisit it.
>>
>>
>> I think we'd still need to use a C pre-processor (or some other code
>> generation/templating tool) even with that scheme in place. So, I think the
>> two are orthogonal.
>
> Yes, but the more of this sort of stuff we add to U-Boot the harder it
> becomes to revisit. As you may recall the tegra config files were
> already pretty hard to move over.

The conversion to Kconfig didn't seem to change any of the Tegra config 
files.

In my opinion at least, Kconfig shouldn't seek to replace 
include/configs/tegra_*.h, but rather should control user-visible 
options rather than internal details.