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

[Simon Glass]

Hi Stephen & Dennis,

On 4 August 2014 12:04, Stephen Warren <swarren at wwwdotorg.org> wrote:
> 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>

Thanks for the doc pointers.

>>>>>
>>>>> 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.

OK, that makes me more comfortable.

>
>
>>> 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.

OK, so long as that is clear then all is well. I thought you meant the
board author could change the scripts in order to tweak the process.

>
> 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.

OK.

>
>
>>>> 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.

Well that's another discussion, and fair enough you don't want to tie
the two together.

>
>
>> 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.

OK good.

>
>
>> 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.

I was referring more to things like the partitions that are searched,
and other things specific to your implementation. Dennis pointed me to
the syslinux stuff and that is pretty clear. I did try to bring it up
at one point but got lost in a maze of U-Boot scripts. Would like to
keep the U-Boot side as simple as possible.

>
>
>> 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.

OK
>
>
>> 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.
>

So that is it? I wonder whether if this is the only configuration
option, we might eventually write this feature in C as a new U-Boot
command, with an environment variable listing the devices in order?

>
>> 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.

The macro magic makes it hard to see what is going on though.

>
>
>>>> 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.

Sure but to my mind the list of bootable devices might well be
something that people want to change in configuration. But really I
was talking about the series to move environment variables and scripts
to a text file from the config/*.h files. That's quite a tricky
transition if we attempt it, and is made trickier by tricky macro
stuff in specific board header files. It's just a comment, not a
blocker.

Regards,
Simon