[U-Boot] single build of uboot supporting env stored in dynamically detected boot device

[U-Boot] single build of uboot supporting env stored in dynamically detected boot device

[Tim Harvey]

Greetings,

It seems the U-Boot's env storage is implemented by various types of
storage from common/env_*.c, one (and only one) which is included by
common/Makefile based off of the CONFIG_ENV_IS_IN_* config.

I want to build a single uboot image which can be installed on one of
several supported boot devices (NAND, SATA, SD for example) and have
the env storage hooked to the device that was detected as the boot
device. Am I correct in assuming this is not currently possible and
that instead I would have to build 3 versions of uboot each with one
type of storage defined?

If it turns out that this is not currently possible, I'm open to
suggestions on how to rectify it (or if you think I'm crazy for
wanting to have a single uboot image for the above please explain your
reasoning). I'm thinking I would start by making the env_init/saveenv
functions (I believe those are the only ones that need to be
implemented by an env storage subsystem) unique function names, then
creating a wrapper for those functions that call the right one.

Regards,

Tim

[U-Boot] single build of uboot supporting env stored in dynamically detected boot device

[Stefan Roese]

Hi Tim,

(added Igor to cc)

On 10.04.2015 00:58, Tim Harvey wrote:
> It seems the U-Boot's env storage is implemented by various types of
> storage from common/env_*.c, one (and only one) which is included by
> common/Makefile based off of the CONFIG_ENV_IS_IN_* config.
>
> I want to build a single uboot image which can be installed on one of
> several supported boot devices (NAND, SATA, SD for example) and have
> the env storage hooked to the device that was detected as the boot
> device. Am I correct in assuming this is not currently possible and
> that instead I would have to build 3 versions of uboot each with one
> type of storage defined?
>
> If it turns out that this is not currently possible, I'm open to
> suggestions on how to rectify it (or if you think I'm crazy for
> wanting to have a single uboot image for the above please explain your
> reasoning). I'm thinking I would start by making the env_init/saveenv
> functions (I believe those are the only ones that need to be
> implemented by an env storage subsystem) unique function names, then
> creating a wrapper for those functions that call the right one.

IIRC, Compulab has such a feature implemented in some of their U-Boot 
versions. Igor, please either correct me if I'm wrong. Or elaborate how 
you have implemented this dynamic env feature.

Thanks,
Stefan

[U-Boot] single build of uboot supporting env stored in dynamically detected boot device

[Tim Harvey]

On Thu, Apr 9, 2015 at 11:07 PM, Stefan Roese <sr@denx.de> wrote:
> Hi Tim,
>
> (added Igor to cc)
>
>
> On 10.04.2015 00:58, Tim Harvey wrote:
>>
>> It seems the U-Boot's env storage is implemented by various types of
>> storage from common/env_*.c, one (and only one) which is included by
>> common/Makefile based off of the CONFIG_ENV_IS_IN_* config.
>>
>> I want to build a single uboot image which can be installed on one of
>> several supported boot devices (NAND, SATA, SD for example) and have
>> the env storage hooked to the device that was detected as the boot
>> device. Am I correct in assuming this is not currently possible and
>> that instead I would have to build 3 versions of uboot each with one
>> type of storage defined?
>>
>> If it turns out that this is not currently possible, I'm open to
>> suggestions on how to rectify it (or if you think I'm crazy for
>> wanting to have a single uboot image for the above please explain your
>> reasoning). I'm thinking I would start by making the env_init/saveenv
>> functions (I believe those are the only ones that need to be
>> implemented by an env storage subsystem) unique function names, then
>> creating a wrapper for those functions that call the right one.
>
>
> IIRC, Compulab has such a feature implemented in some of their U-Boot
> versions. Igor, please either correct me if I'm wrong. Or elaborate how you
> have implemented this dynamic env feature.
>
> Thanks,
> Stefan
>

Igor,

I think what Stefan might be thinking of is ENV_IS_EMBEDED used in
cm-bf537. It looks like this support was originally for the renesas
sh775* boards which don't look to be maintained any longer. I haven't
gone through it in depth yet but it seems this has to do with
embedding the env within U-Boot binary itself yet I'm clear why this
isn't just using the default_environment array (maybe it predated that
notion?). If I'm understanding it correctly ENV_IS_EMBEDDED is not
really what I'm looking for and likely that support could use some
cleanup and updating.

I have an RFC patch I'm working on that will allow
CONFIG_ENV_IS_DYNAMIC that currently does the following:
 - makes env_init(), saveenv(), env_relocate_spec(), env_ptr,
env_name_spec namespace unique in env_*.c
 - provide a env_init(), saveenv(), env_relocate_spec() in
env_common.c using spl_boot_device() to determine the boot device and
calling the namespace unique impl instead
 - currently supports only mmc/nand

I don't see any clear maintainers of anything env related so I'm not
sure who else may want to be involved in this conversation.

(added Wolfgang to cc)

Tim

[U-Boot] single build of uboot supporting env stored in dynamically detected boot device

[Simon Glass]

Hi Tim,

On 10 April 2015 at 09:42, Tim Harvey <tharvey@gateworks.com> wrote:
> On Thu, Apr 9, 2015 at 11:07 PM, Stefan Roese <sr@denx.de> wrote:
>> Hi Tim,
>>
>> (added Igor to cc)
>>
>>
>> On 10.04.2015 00:58, Tim Harvey wrote:
>>>
>>> It seems the U-Boot's env storage is implemented by various types of
>>> storage from common/env_*.c, one (and only one) which is included by
>>> common/Makefile based off of the CONFIG_ENV_IS_IN_* config.
>>>
>>> I want to build a single uboot image which can be installed on one of
>>> several supported boot devices (NAND, SATA, SD for example) and have
>>> the env storage hooked to the device that was detected as the boot
>>> device. Am I correct in assuming this is not currently possible and
>>> that instead I would have to build 3 versions of uboot each with one
>>> type of storage defined?
>>>
>>> If it turns out that this is not currently possible, I'm open to
>>> suggestions on how to rectify it (or if you think I'm crazy for
>>> wanting to have a single uboot image for the above please explain your
>>> reasoning). I'm thinking I would start by making the env_init/saveenv
>>> functions (I believe those are the only ones that need to be
>>> implemented by an env storage subsystem) unique function names, then
>>> creating a wrapper for those functions that call the right one.
>>
>>
>> IIRC, Compulab has such a feature implemented in some of their U-Boot
>> versions. Igor, please either correct me if I'm wrong. Or elaborate how you
>> have implemented this dynamic env feature.
>>
>> Thanks,
>> Stefan
>>
>
> Igor,
>
> I think what Stefan might be thinking of is ENV_IS_EMBEDED used in
> cm-bf537. It looks like this support was originally for the renesas
> sh775* boards which don't look to be maintained any longer. I haven't
> gone through it in depth yet but it seems this has to do with
> embedding the env within U-Boot binary itself yet I'm clear why this
> isn't just using the default_environment array (maybe it predated that
> notion?). If I'm understanding it correctly ENV_IS_EMBEDDED is not
> really what I'm looking for and likely that support could use some
> cleanup and updating.
>
> I have an RFC patch I'm working on that will allow
> CONFIG_ENV_IS_DYNAMIC that currently does the following:
>  - makes env_init(), saveenv(), env_relocate_spec(), env_ptr,
> env_name_spec namespace unique in env_*.c
>  - provide a env_init(), saveenv(), env_relocate_spec() in
> env_common.c using spl_boot_device() to determine the boot device and
> calling the namespace unique impl instead
>  - currently supports only mmc/nand

I've wanted this feature for some years, and also the ability to
define the environment offset/address/size (and other parameters)
dynamically.

This feels like a job for driver model. We could have an environment
uclass with operations like load and save. Then mmc and nand could
implement drivers for this uclass.

Probably better that than a linker_list.

>
> I don't see any clear maintainers of anything env related so I'm not
> sure who else may want to be involved in this conversation.
>
> (added Wolfgang to cc)
>
> Tim

Regards,
Simon

[U-Boot] single build of uboot supporting env stored in dynamically detected boot device

[Tom Rini]

On Fri, Apr 10, 2015 at 10:39:48AM -0600, Simon Glass wrote:

> This feels like a job for driver model. We could have an environment
> uclass with operations like load and save. Then mmc and nand could
> implement drivers for this uclass.

Yes, this.  There are many ways to one-off multiple env location support
but to do it cleanly we really need DM.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150410/38b26a74/attachment.sig>

[U-Boot] single build of uboot supporting env stored in dynamically detected boot device

[Simon Glass]

+U-Boot

Hi Tim,

On 10 April 2015 at 10:46, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Fri, Apr 10, 2015 at 9:39 AM, Simon Glass <sjg@chromium.org> wrote:
> >
> > I've wanted this feature for some years, and also the ability to
> > define the environment offset/address/size (and other parameters)
> > dynamically.
> >
> > This feels like a job for driver model. We could have an environment
> > uclass with operations like load and save. Then mmc and nand could
> > implement drivers for this uclass.
> >
> > Probably better that than a linker_list.
> >
>
> Simon,
>
> If its not too much of a time-sink I would love to take a stab at it.
>
> I've been too swamped to even follow all your work regarding DM. Could
> you point me to any existing
> docs/presentation/simple-example/similar-patches that will give me an
> idea of what needs to be done? Even spit-balling some psuedo-code
> would get me started.

Sure. There's a doc/driver-model/README.

Here are my ideas:

1. Add a new value to the enum in uclass-id.h

2. Create a env-uclass.c file (see demo-uclass.c and dm-demo.h for an
example). Add the operations you want to support. Enable it if
CONFIG_DM_ENV is defined (you can add this new option to Kconfig).

3. Add a sandbox environment driver which implements your operations -
 add a U_BOOT_DRIVER() for your driver, setting .ops to contain
functions that implement the uclass operations for sandbox. Then and
some tests (test/dm/env.c). Then it should be possible to use
saveenv(), etc. from sandbox. The implementation could store it in a
file, perhaps (e.g. using os_open(), etc.). This should speed up
development too since you don't need to test on real hardware.

4. Implement env_relocate() etc. in the uclass file, so that if
CONFIG_DM_ENV is defined you can't define the other ones -
CONFIG_ENV_IS_IN_... or you will get duplicate functions.

5. In your env_relocate(), you can do something like
uclass_get_device(UCLASS_ENV, 0, &dev) to get the default environment
device. Then call the correct operation.

6. Add more driver support - e.g. in env_mmc.c, add a U_BOOT_DRIVER()
for your mmc driver, setting .ops to contain functions that implement
the uclass operations for mmc.

That should be most of it. I don't think it's too tricky. It would be
a big win. I'm happy to help with review and advice. Plus I'll sign up
to do SPI once you are done.

Regards,
Simon

[U-Boot] single build of uboot supporting env stored in dynamically detected boot device

[Igor Grinberg]

Hi Tim,

Sorry for that late reply. I guess better late then never...

On 04/10/15 19:39, Simon Glass wrote:
> Hi Tim,
> 
> On 10 April 2015 at 09:42, Tim Harvey <tharvey@gateworks.com> wrote:
>> On Thu, Apr 9, 2015 at 11:07 PM, Stefan Roese <sr@denx.de> wrote:
>>> Hi Tim,
>>>
>>> (added Igor to cc)
>>>
>>>
>>> On 10.04.2015 00:58, Tim Harvey wrote:
>>>>
>>>> It seems the U-Boot's env storage is implemented by various types of
>>>> storage from common/env_*.c, one (and only one) which is included by
>>>> common/Makefile based off of the CONFIG_ENV_IS_IN_* config.
>>>>
>>>> I want to build a single uboot image which can be installed on one of
>>>> several supported boot devices (NAND, SATA, SD for example) and have
>>>> the env storage hooked to the device that was detected as the boot
>>>> device. Am I correct in assuming this is not currently possible and
>>>> that instead I would have to build 3 versions of uboot each with one
>>>> type of storage defined?
>>>>
>>>> If it turns out that this is not currently possible, I'm open to
>>>> suggestions on how to rectify it (or if you think I'm crazy for
>>>> wanting to have a single uboot image for the above please explain your
>>>> reasoning). I'm thinking I would start by making the env_init/saveenv
>>>> functions (I believe those are the only ones that need to be
>>>> implemented by an env storage subsystem) unique function names, then
>>>> creating a wrapper for those functions that call the right one.
>>>
>>>
>>> IIRC, Compulab has such a feature implemented in some of their U-Boot
>>> versions. Igor, please either correct me if I'm wrong. Or elaborate how you
>>> have implemented this dynamic env feature.

Yes, I did this for several of our boards.
I really wanted to clean it up and push upstream, but I barely had time
to implement it in some way to make it work...

>>>
>>> Thanks,
>>> Stefan
>>>
>>
>> Igor,
>>
>> I think what Stefan might be thinking of is ENV_IS_EMBEDED used in
>> cm-bf537.

Nope, that is not one of our boards... That's Blackfin.
Our board is cm-t35, but the change is not upstream, you can check
our downstream U-Boot here [1] if you would like to,
but it requires a huge rework, so I would recommend
implementing it from scratch using DM concepts.

>> It looks like this support was originally for the renesas
>> sh775* boards which don't look to be maintained any longer. I haven't
>> gone through it in depth yet but it seems this has to do with
>> embedding the env within U-Boot binary itself yet I'm clear why this
>> isn't just using the default_environment array (maybe it predated that
>> notion?). If I'm understanding it correctly ENV_IS_EMBEDDED is not
>> really what I'm looking for and likely that support could use some
>> cleanup and updating.

Yep. We do not use ENV_IS_EMBEDDED on our boards.

>>
>> I have an RFC patch I'm working on that will allow
>> CONFIG_ENV_IS_DYNAMIC that currently does the following:
>>  - makes env_init(), saveenv(), env_relocate_spec(), env_ptr,
>> env_name_spec namespace unique in env_*.c

Yep. I had this too.

>>  - provide a env_init(), saveenv(), env_relocate_spec() in
>> env_common.c using spl_boot_device() to determine the boot device and
>> calling the namespace unique impl instead

Yes.

>>  - currently supports only mmc/nand

Same stands for cm-t35 downstream U-Boot...

In addition, my downstream implementation has the "env dev" command
to switch between the available env storage devices on the fly.
And that enabled also copying the env from device to device, if needed.

One of the huge drawbacks of my downstream implementation is that
since I had no time to figure out how to properly patch the functions
location after relocation, I had a huge array of ifdefs for all the cases...

> 
> I've wanted this feature for some years, and also the ability to
> define the environment offset/address/size (and other parameters)
> dynamically.
> 
> This feels like a job for driver model. We could have an environment
> uclass with operations like load and save. Then mmc and nand could
> implement drivers for this uclass.

Yes, I think we have discussed and agreed on this in D?sseldorf, right?

> 
> Probably better that than a linker_list.

Agreed, much better.

[1] - https://github.com/compulab/cm-t3x-u-boot

-- 
Regards,
Igor.

[U-Boot] single build of uboot supporting env stored in dynamically detected boot device

[Tim Harvey]

On Tue, Apr 21, 2015 at 11:17 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Tim,
>
> Sorry for that late reply. I guess better late then never...
>
<snip>
>
> Yes, I did this for several of our boards.
> I really wanted to clean it up and push upstream, but I barely had time
> to implement it in some way to make it work...

Hi Igor,

I know the feeling - I was in the same boat.

>
<snip>
>>>
>>> I have an RFC patch I'm working on that will allow
>>> CONFIG_ENV_IS_DYNAMIC that currently does the following:
>>>  - makes env_init(), saveenv(), env_relocate_spec(), env_ptr,
>>> env_name_spec namespace unique in env_*.c
>
> Yep. I had this too.
>
>>>  - provide a env_init(), saveenv(), env_relocate_spec() in
>>> env_common.c using spl_boot_device() to determine the boot device and
>>> calling the namespace unique impl instead
>
> Yes.
>
>>>  - currently supports only mmc/nand
>
> Same stands for cm-t35 downstream U-Boot...

sounds like you and I did about the same thing. If anyone reading this
needs it for reference:
 - https://github.com/Gateworks/u-boot-imx6/commit/ed166eb5c8a83f7489f900d2217763f0fd458f17
 - https://github.com/Gateworks/u-boot-imx6/commit/c398f755a0483e55b661c96c119dd5f3b953eda9

the above are based on U-Boot 2014.04 although I have the same patch
for U-Boot 2015.04

>
> In addition, my downstream implementation has the "env dev" command
> to switch between the available env storage devices on the fly.
> And that enabled also copying the env from device to device, if needed.
>
> One of the huge drawbacks of my downstream implementation is that
> since I had no time to figure out how to properly patch the functions
> location after relocation, I had a huge array of ifdefs for all the cases...
>
>>
>> I've wanted this feature for some years, and also the ability to
>> define the environment offset/address/size (and other parameters)
>> dynamically.
>>
>> This feels like a job for driver model. We could have an environment
>> uclass with operations like load and save. Then mmc and nand could
>> implement drivers for this uclass.
>
> Yes, I think we have discussed and agreed on this in D?sseldorf, right?

I'm hoping to have some time in the next month or so to come back and
propose a proper upstreamable patch using DM. The DM part seems easy
enough, but I will first need get around all the various uses of
CONFIG_ENV_SIZE.

Regards,

Tim