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

[Igor Grinberg]

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.

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

> 
> In fact I wish we could support this for all environment types.
> Overall the environment drivers need some work to make them more
> similar...

Indeed, but not just that... It would be great to add a DM to env. drivers.
This will make it possible to use more than one environment driver
in runtime and make many people who struggle with it happy...


-- 
Regards,
Igor.