[U-Boot] [PATCH v1] env_mmc: configure environment offsets via device tree

Sat Apr 1 04:20:48 UTC 2017

Hi Igor,

On 14 March 2017 at 07:11, Igor Grinberg <grinberg at compulab.co.il> wrote:
> Hi Simon,
>
> On 03/12/17 22:21, Simon Glass wrote:
>> Hi Igor,
>>
>> On 5 March 2017 at 01:39, Igor Grinberg <grinberg at compulab.co.il> wrote:
>>> Hi Simon,
>>>
>>> On 03/03/17 06:53, Simon Glass wrote:
>>>> Hi Igor,
>>>>
>>>> On 22 February 2017 at 00:35, Igor Grinberg <grinberg at compulab.co.il> wrote:
>>>>> Hi Philipp, Simon,
>>>>>
>>>>> On 02/22/17 05:59, Simon Glass wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 20 February 2017 at 02:18, Dr. Philipp Tomsich
>>>>>> <philipp.tomsich at theobroma-systems.com> wrote:
>>>>>>>
>>>>>>> On 20 Feb 2017, at 08:22, Igor Grinberg <grinberg at compulab.co.il> wrote:
>>>>>>>
>>>>>>> That sounds too odd...
>>>>>>> DT's purpose is to describe the h/w... and that does not look so...
>>>>>>> We also, have a dt file name in the environment, so this creates will create
>>>>>>> a chicken and an egg problem…
>>>>>>>
>>>>>>>
>>>>>>> I don’t really follow… as far as I knew the DT name would have to come
>>>>>>> from some other source anyway, as the device containing the env might
>>>>>>> only be described through the device tree (e.g. mmc0).
>>>>>>>
>>>>>>>
>>>>>>> Why? U-Boot can live pretty well w/o DT.
>>>>>>>
>>>>>>>
>>>>>>> If U-Boot runs without DT, then nothing will/should change about how the
>>>>>>> setting
>>>>>>> is retrieved from CONFIG_ENV_OFFSET.
>>>>>
>>>>> ok.
>>>>>
>>>>>>>
>>>>>>> The platform that motivated this change is ARCH_SUNXI, which does not use
>>>>>>> per-board defines but aims to have one generic bootloader per-SoC.
>>>>>
>>>>> Well, after a ten year experience with boars and different SoC vendors,
>>>>> I don't think it is possible...
>>>>> Unless all boards are copies of their respective reference design...
>>>>>
>>>>>>>
>>>>>>> I really don't think we should go that direction. DT is not meant to provide
>>>>>>> a solution to all your problems...
>>>>>>>
>>>>>>>
>>>>>>> I don’t see how this is different from other entries in chosen and config as
>>>>>>> of today:
>>>>>>> common/autoboot.c allows an override through /config/bootdelay
>>>>>>> common/board_r.c uses /config/load-environment
>>>>>>> common/cli.c can pull in /config/bootcmd
>>>>>>> drivers/serial/serial-uclass.c uses /chosen/stdout-path
>>>>>>>
>>>>>>> In fact, it is the absence of this mechanism that is causing problems today:
>>>>>>> CONFIG_ENV_OFFSET is not configurable through Kconfig, so we would
>>>>>>> need board-specific defines (e.g. CONFIG_SUNXI_BOARD_LYNX) and
>>>>>>> matching #ifdef primitives in a shared header (sunxi-common.h in our case).
>>>>>>>
>>>>>>>
>>>>>>> Right. Exactly, I think we should move the CONFIG_ENV_OFFSET to Kconfig.
>>>>>>> And that will solve the problem.
>>>>>>>
>>>>>>>
>>>>>>> Doing this would still get into the way of architectures that want to build
>>>>>>> a single
>>>>>>> ‘universal’ bootloader for their SoC: the ENV_OFFSET may not be the same
>>>>>>> across all board and vendor configurations. This can easily be handled with
>>>>>>> the
>>>>>>> (optional) prop in the DT, but not with the compile-time ENV_OFFSET.
>>>>>
>>>>> I think Kconfig would be enough for this, but please try your approach.
>>>>> The 'universal' thing will probably work if you don't have too many boards and
>>>>> they don't differ too much from each other...
>>>>>
>>>>>>>
>>>>>>> If we decide to this, I’d at least like to introduce the function call to
>>>>>>> (the weak
>>>>>>> function) mmc_get_env_addr(…), so we can override this in the board code.
>>>>>
>>>>> That is how it works today:
>>>>> include/environment.h:185:extern int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr);
>>>>>
>>>>>>>
>>>>>>> So putting this in the DT is the best (and least intrusive) option
>>>>>>> available.
>>>>>>>
>>>>>>>
>>>>>>> Ok. I see your point now. Yet I think we should keep the DT to its purpose -
>>>>>>> which
>>>>>>> is to describe the h/w (and not the s/w placement layout).
>>>>>>
>>>>>> Well actually we put things like flash map in there and the
>>>>>> environment position seems like a very good thing to have there.
>>>>>
>>>>> Well, I thought so too... But I had a discussion with kernel people
>>>>> some time ago and they discourage this approach and would not like to
>>>>> have the flash mapping in the DT.
>>>>
>>>> I'm sorry to hear that, but it doesn't change the fact that it is very
>>>> useful for dealing with hardware-specific differences between models.
>>>
>>> According to kernel guys, these are not h/w specific, but rather device
>>> specific... and it actually makes sense - on the same h/w design different
>>> applications can use different flash mapping - just like the block devices.
>>
>> Really? That sounds like a pretty esoteric case to me.
>
> Well, yes.. It might be esoteric, but strong enough to push back on
> some patches we have sent to the DT in kernel...
>
>>
>> In my experience the flash mapping is normally fixed at production
>> time and there may be parts that are write-protected, e.g. with a
>> hardware switch.
>
> Well, I think this is completely true in x86 world... where things
> are much more strict.

It depends on the board, but all ARM Chromebooks have the same
restriction, for example.

>
>> The flash map generally needs to be understood by
>> both firmware and kernel.
>
> True, but that does not have to come from DT...
> They prefer the kernel command line over DT.
>
>> It is as much a feature of the hardware as
>> which serial console to use.
>
> This one I cannot agree to...
> We have had several cases where customers changed the flash partitioning for
> their application needs etc.
> So, that's why I've said it makes some sense.

If you don't need a fixed partition then you have some options:

- U-Boot determines the partition layout and passes it to Linux (e.g.
via command line)
- U-Boot doesn't touch the flash and Linux can work it out

But if you don't want a fixed layout, why are you talking about
putting it in DT in U-Boot? That would not make sense. The case for
putting it in U-Boot's DT is when it is fixed.

>
>>
>>>
>>>>
>>>> Building the same software multiple times with different #ifdefs is
>>>> often painful. Using a DT to handle these minor differences reduces
>>>> the number of build combinations and simplifies testing.
>>>
>>> Well, partially...
>>> I agree that building different binaries with #ifdefs is very painful
>>> and I highly discourage this approach.
>>> Using a DT to handle these differences, IMO, is better but also not that
>>> good, as it requires building multiple DT binaries (which I consider the
>>> same as multiple U-Boot binaries) and therefore does not reduce the number
>>> of build combinations... you just abstract the problem to a separate binary.
>>
>> It has a big positive impact on complexity though. You build U-Boot
>> once and attach the appropriate device tree at the end, perhaps during
>> production. This is similar to how Linux boots on different hardware
>> times, with just a different DT.
>
> Right! The thing is U-Boot is not Linux - it is a SoC/board specific
> bootloader.

The complexity of SoCs is growing all the time, and if you have to
support a lot of variants of one SoC it often makes sense to describe
the variations in a device tree, just as with Linux.

>
>>
>>>
>>> I'm more in favor of run time detection and adjustment of static data
>>> along with dynamic code run.
>>
>> Well that is certainly possible. I think the trade-off here depends on
>> your production syste,.
>
> Yep. We have lots of board configuration combinations and we certainly
> cannot build a DT for each combination...
> What we can do (and we are currently really good at this) is detect things
> at runtime and update the DT with the correct data for Linux to boot.

Sounds fine to me. There are many ways to do this.

>
>>
>>>
>>> Regarding the environment, I think it would be great to have U-Boot
>>> detect where environment location is and proceed with it.
>>> Just like the boot sequence stuff...
>>
>> Well that is a separate feature from what is being discussed here. But
>> IMO a solid flash map available to everything is better. You can
>> define where the environment is, and anything else you find.
>>
>>>
>>>>
>>>>>
>>>>>> So I
>>>>>> like this patch. There is too compile-time configuration in U-Boot
>>>>>> still...
>>>>>
>>>>> Yeah, I know you like it ;-) but DT is not the place for it,
>>>>> unless DT specs. guys decide to change its purpose and place
>>>>> s/w configuration straps inside...
>>>>>
>>>>> Although, U-Boot build process is not a pain at all, you might want
>>>>> to design another abstraction layer for s/w configuration straps.
>>>>> That way you will be able to keep the U-Boot core binary generic...
>>>>> Is it worse the effort? I don't know. Currently, having the board
>>>>> infrastructure, provides that layer and works fine.
>>>>
>>>> At present in U-Boot DT is that layer. We don't need to ban useful
>>>> additions like this and I really am not keen on adding another config
>>>> mechanism.
>>>
>>> So, how would you sync between the DTs in U-Boot and kernel?
>>
>> Do you mean how to sync the flashmap? It can be passed down to Linux
>> in the DT, if that is what you are asking?
>
> No, I mean the DT source code.

If Linux wants the flashmap in its command line (which is of course
inside the DT passed to Linux :-) then that's fine. Whatever it wants.
My point is that with U-Boot we should support having the flash map in
U-Boot's DT for its own use, for situations where the flashmap is
fixed. While I understand that Linux makes its own decisions on how it
wants to receive the flashmap from the boot loader, it is better for
U-Boot to make its own decisions on that matter for its internal use
too.

Regards.
Simon