[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Robert P. J. Day]

  never embarrassed to make a fool of myself, i have to admit that,
while crawling through examples of u-boot boards that define dual
partitions, i am confused by the following in
include/configs/taurus.h (only some lines reproduced):

  #define CONFIG_BOOTARGS_AXM                                           \
        "\0"    \
        "flash_nfs=run nand_kernel;run nfsargs;run addip;upgrade_available;"\
        "flash_self=run nand_kernel;run setbootargs;upgrade_available;" \
        "flash_self_test=run nand_kernel;run setbootargs addtest; "     \
        "upgrade_available;bootm ${kernel_ram};reset\0"
        "net_nfs=run boot_file;tftp ${kernel_ram} ${bootfile};"         \
        "run nfsargs;run addip;upgrade_available;bootm "                \
                "${kernel_ram};reset\0"                                 \

        ... snip ...

        "upgrade_available=0\0"
  #endif

what does it mean for "upgrade_available;" to be in the middle of some
of those command definitions? it's just a variable, what does it
represent? am i just clueless for never having noticed this sort of
thing before?

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Wolfgang Denk]

Dear "Robert P. J. Day",

In message <alpine.LFD.2.20.1607221529190.7337@localhost.localdomain> you wrote:
> 
>   never embarrassed to make a fool of myself, i have to admit that,
> while crawling through examples of u-boot boards that define dual
> partitions, i am confused by the following in
> include/configs/taurus.h (only some lines reproduced):

This is your fault.  You omit the important parts, i. e. the lines
which end with a "\0" marker which will actually terminate the
definition of one variable and start the next one.

>   #define CONFIG_BOOTARGS_AXM                                           \
>         "\0"    \
>         "flash_nfs=run nand_kernel;run nfsargs;run addip;upgrade_available;"\
>         "flash_self=run nand_kernel;run setbootargs;upgrade_available;" \
>         "flash_self_test=run nand_kernel;run setbootargs addtest; "     \
>         "upgrade_available;bootm ${kernel_ram};reset\0"
>         "net_nfs=run boot_file;tftp ${kernel_ram} ${bootfile};"         \
>         "run nfsargs;run addip;upgrade_available;bootm "                \
>                 "${kernel_ram};reset\0"                                 \
> 
>         ... snip ...
> 
>         "upgrade_available=0\0"
>   #endif
> 
> what does it mean for "upgrade_available;" to be in the middle of some
> of those command definitions? it's just a variable, what does it
> represent? am i just clueless for never having noticed this sort of
> thing before?

The code really looks like that:

189         "flash_nfs=run nand_kernel;run nfsargs;run addip;upgrade_available;"\
190         "bootm ${kernel_ram};reset\0"                                   \

191         "flash_self=run nand_kernel;run setbootargs;upgrade_available;" \
192         "bootm ${kernel_ram};reset\0"                                   \

193         "flash_self_test=run nand_kernel;run setbootargs addtest; "     \
194         "upgrade_available;bootm ${kernel_ram};reset\0"                 \

So the code would run the "upgrade_available" _command_ as part of
the command sequences stored in the flash_nfs, flash_self resp.
flash_self_test environment variables.

Note that it has to be a (builtin) command, as a variable reference
would require a "$upgrade_available".

The "upgrade_available" is a board-specific command, implemented in
board/siemens/taurus/taurus.c, function do_upgrade_available().

You may also want to read the README:

 911 - Bootcount:
 ...
 917                 CONFIG_BOOTCOUNT_ENV
 918                 If no softreset save registers are found on the hardware
 919                 "bootcount" is stored in the environment. To prevent a
 920                 saveenv on all reboots, the environment variable
 921                 "upgrade_available" is used. If "upgrade_available" is
 922                 0, "bootcount" is always 0, if "upgrade_available" is
 923                 1 "bootcount" is incremented in the environment.
 924                 So the Userspace Applikation must set the "upgrade_available"
 925                 and "bootcount" variable to 0, if a boot was successfully.

This is a special measure for boards where there was no good place to
store the boot counter variable in some hardware register, so it was
instead placed in the environment.


Adding Heiko on Cc: so he might add to this explanation if I have
missed something, or interpreted the code incorrectly.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You Don't Have To Be 'Damned' To Work Here, But It Helps!!!
                                             - Terry Pratchett, _Eric_

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Robert P. J. Day]

On Sat, 23 Jul 2016, Wolfgang Denk wrote:

> Dear "Robert P. J. Day",
>
> In message <alpine.LFD.2.20.1607221529190.7337@localhost.localdomain> you wrote:
> >
> >   never embarrassed to make a fool of myself, i have to admit that,
> > while crawling through examples of u-boot boards that define dual
> > partitions, i am confused by the following in
> > include/configs/taurus.h (only some lines reproduced):
>
> This is your fault.  You omit the important parts, i. e. the lines
> which end with a "\0" marker which will actually terminate the
> definition of one variable and start the next one.

  ... snip ...

  ack, you're right, totally my fault. i was tired.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Robert P. J. Day]

On Sat, 23 Jul 2016, Wolfgang Denk wrote:

> Dear "Robert P. J. Day",
>
> In message <alpine.LFD.2.20.1607221529190.7337@localhost.localdomain> you wrote:
> >
> >   never embarrassed to make a fool of myself, i have to admit that,
> > while crawling through examples of u-boot boards that define dual
> > partitions, i am confused by the following in
> > include/configs/taurus.h (only some lines reproduced):

... snip ..

> So the code would run the "upgrade_available" _command_ as part of
> the command sequences stored in the flash_nfs, flash_self resp.
> flash_self_test environment variables.
>
> Note that it has to be a (builtin) command, as a variable reference
> would require a "$upgrade_available".
>
> The "upgrade_available" is a board-specific command, implemented in
> board/siemens/taurus/taurus.c, function do_upgrade_available().

... more snip ...

  yes, my confusion was based on that i had no idea there was an
"upgrade_available" builtin command defined in taurus.c (i was looking
only at the header file at the time), in addition to the same-named
variable. as soon as you pointed that out, it all fell into place.

  so i'm good, and it all makes sense. thanks muchly.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Robert P. J. Day]

On Sat, 23 Jul 2016, Wolfgang Denk wrote:

> You may also want to read the README:
>
>  911 - Bootcount:
>  ...
>  917                 CONFIG_BOOTCOUNT_ENV
>  918                 If no softreset save registers are found on the hardware
>  919                 "bootcount" is stored in the environment. To prevent a
>  920                 saveenv on all reboots, the environment variable
>  921                 "upgrade_available" is used. If "upgrade_available" is
>  922                 0, "bootcount" is always 0, if "upgrade_available" is
>  923                 1 "bootcount" is incremented in the environment.
>  924                 So the Userspace Applikation must set the "upgrade_available"
>  925                 and "bootcount" variable to 0, if a boot was successfully.

  i note there seems to be some redundancy in the README. early on:

    911 - Bootcount:
    912                 CONFIG_BOOTCOUNT_LIMIT
    913                 Implements a mechanism for detecting a repeating reboot
    914                 cycle, see:
    915                 http://www.denx.de/wiki/view/DULG/UBootBootCountLimit
    916
    917                 CONFIG_BOOTCOUNT_ENV
    918                 If no softreset save registers are found on the hardware
    919                 "bootcount" is stored in the environment. To prevent a
    920                 saveenv on all reboots, the environment variable
    921                 "upgrade_available" is used. If "upgrade_available" is
    922                 0, "bootcount" is always 0, if "upgrade_available" is
    923                 1 "bootcount" is incremented in the environment.
    924                 So the Userspace Applikation must set the "upgrade_available"
    925                 and "bootcount" variable to 0, if a boot was successfully.

and much further down:

   3088 - bootcount support:
   3089                 CONFIG_BOOTCOUNT_LIMIT
   3090
   3091                 This enables the bootcounter support, see:
   3092                 http://www.denx.de/wiki/DULG/UBootBootCountLimit
   3093
   3094                 CONFIG_AT91SAM9XE
   3095                 enable special bootcounter support on at91sam9xe based boards.
   3096                 CONFIG_BLACKFIN
   3097                 enable special bootcounter support on blackfin based boards.
   3098                 CONFIG_SOC_DA8XX
   3099                 enable special bootcounter support on da850 based boards.
   3100                 CONFIG_BOOTCOUNT_RAM
   3101                 enable support for the bootcounter in RAM
   3102                 CONFIG_BOOTCOUNT_I2C
   3103                 enable support for the bootcounter on an i2c (like RTC) device.
   3104                         CONFIG_SYS_I2C_RTC_ADDR = i2c chip address
   3105                         CONFIG_SYS_BOOTCOUNT_ADDR = i2c addr which is used for
   3106                                                     the bootcounter.
   3107                         CONFIG_BOOTCOUNT_ALEN = address len

can these two sections not be combined? i can add that to my cleanup
list as well.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Wolfgang Denk]

Dear Robert,

In message <alpine.LFD.2.20.1607230737010.12216@localhost.localdomain> you wrote:
> 
>   i note there seems to be some redundancy in the README. early on:
> 
>     911 - Bootcount:
>     912                 CONFIG_BOOTCOUNT_LIMIT
>     913                 Implements a mechanism for detecting a repeating reboot
>     914                 cycle, see:
>     915                 http://www.denx.de/wiki/view/DULG/UBootBootCountLimit
...
> and much further down:
> 
>    3088 - bootcount support:
>    3089                 CONFIG_BOOTCOUNT_LIMIT
>    3090
>    3091                 This enables the bootcounter support, see:
>    3092                 http://www.denx.de/wiki/DULG/UBootBootCountLimit

Indeed.

>    3094                 CONFIG_AT91SAM9XE
>    3095                 enable special bootcounter support on at91sam9xe based boards.
>    3096                 CONFIG_BLACKFIN
>    3097                 enable special bootcounter support on blackfin based boards.
>    3098                 CONFIG_SOC_DA8XX
>    3099                 enable special bootcounter support on da850 based boards.

This is name space pollution t best, and has potential to cause
unwanted side effects.  This needs thorough checking and cleanup, if
it should turn out thatthese macros are used only to select specific
bootcount implementations - in that case, they should be renamed into
something like CONFIG_BOOTCOUNT_* or such.

Heiko, maybe you could have a look tat that, please?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The important thing about being a leader is not being right or wrong,
but being *certain*.                    - Terry Pratchett, _Truckers_

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Robert P. J. Day]

On Mon, 25 Jul 2016, Wolfgang Denk wrote:

> Dear Robert,
>
> In message <alpine.LFD.2.20.1607230737010.12216@localhost.localdomain> you wrote:
>
> >    3094                 CONFIG_AT91SAM9XE
> >    3095                 enable special bootcounter support on at91sam9xe based boards.
> >    3096                 CONFIG_BLACKFIN
> >    3097                 enable special bootcounter support on blackfin based boards.
> >    3098                 CONFIG_SOC_DA8XX
> >    3099                 enable special bootcounter support on da850 based boards.
>
> This is name space pollution t best, and has potential to cause
> unwanted side effects.  This needs thorough checking and cleanup, if
> it should turn out thatthese macros are used only to select specific
> bootcount implementations - in that case, they should be renamed
> into something like CONFIG_BOOTCOUNT_* or such.
>
> Heiko, maybe you could have a look at that, please?

  i'm not sure it's as bad as it looks, since those macros are used
specifically in drivers/bootcount/Makefile:

  obj-y                           += bootcount.o
  obj-$(CONFIG_AT91SAM9XE)        += bootcount_at91.o
  obj-$(CONFIG_BLACKFIN)          += bootcount_blackfin.o
  obj-$(CONFIG_SOC_DA8XX)         += bootcount_davinci.o
  obj-$(CONFIG_BOOTCOUNT_AM33XX)  += bootcount_davinci.o
  obj-$(CONFIG_BOOTCOUNT_RAM)     += bootcount_ram.o
  obj-$(CONFIG_BOOTCOUNT_ENV)     += bootcount_env.o
  obj-$(CONFIG_BOOTCOUNT_I2C)     += bootcount_i2c.o

and drivers/bootcount/ is processed only if:

  obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/

but i do see the single, more precise example of
CONFIG_BOOTCOUNT_AM33XX, so someone else can decide if anything
should be renamed here.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Wolfgang Denk]

Dear Robert,

In message <alpine.LFD.2.20.1607250558110.24069@localhost.localdomain> you wrote:
>
>   i'm not sure it's as bad as it looks, since those macros are used
> specifically in drivers/bootcount/Makefile:

You are right, and apparently it is working fine so far.  But it is
confusing, and...

>   obj-$(CONFIG_SOC_DA8XX)         += bootcount_davinci.o
>   obj-$(CONFIG_BOOTCOUNT_AM33XX)  += bootcount_davinci.o

..this would make more sense to me if it was called

	CONFIG_BOOTCOUNT_DAVINCI

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Do not simplify the design of a program if a way can be found to make
it complex and wonderful.

[U-Boot] a few questions about saving bootcount in the environment

[Heiko Schocher]

Hello Wolfgang,

Am 25.07.2016 um 15:26 schrieb Wolfgang Denk:
> Dear Robert,
>
> In message <alpine.LFD.2.20.1607250558110.24069@localhost.localdomain> you wrote:
>>
>>    i'm not sure it's as bad as it looks, since those macros are used
>> specifically in drivers/bootcount/Makefile:
>
> You are right, and apparently it is working fine so far.  But it is
> confusing, and...
>
>>    obj-$(CONFIG_SOC_DA8XX)         += bootcount_davinci.o
>>    obj-$(CONFIG_BOOTCOUNT_AM33XX)  += bootcount_davinci.o
>
> ..this would make more sense to me if it was called
>
> 	CONFIG_BOOTCOUNT_DAVINCI

Yep, correct!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Robert P. J. Day]

On Mon, 25 Jul 2016, Wolfgang Denk wrote:

> Dear Robert,
>
> In message <alpine.LFD.2.20.1607250558110.24069@localhost.localdomain> you wrote:
> >
> >   i'm not sure it's as bad as it looks, since those macros are used
> > specifically in drivers/bootcount/Makefile:
>
> You are right, and apparently it is working fine so far.  But it is
> confusing, and...
>
> >   obj-$(CONFIG_SOC_DA8XX)         += bootcount_davinci.o
> >   obj-$(CONFIG_BOOTCOUNT_AM33XX)  += bootcount_davinci.o
>
> ..this would make more sense to me if it was called
>
> 	CONFIG_BOOTCOUNT_DAVINCI

  after pondering on this, i'm going to disagree, and here's why.

  first, i think the *only* way you should be able to select the
bootcount feature is by defining CONFIG_BOOTCOUNT_LIMIT -- that should
be the only thing you need to select, and it should apply equally well
to *everyone*, regardless of whether they need weird or special
processing. this matches what you see in drivers/Makefile:

  obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/

which establishes that that config setting is the *only* way you get
to further process what's in the bootcount directory.

  now, once you're *in* that directory, then you can start checking if
you need any special processing based on processor or target board or
whatever so, at that point, i'm no longer as offended as i once was by
Makefile lines like:

  obj-$(CONFIG_SOC_DA8XX)         += bootcount_davinci.o

that simply says, "if it's this target, then i need that
target-specific bootcount code." and that actually seems quite
reasonable, now that i think about it.

  what now looks silly is a line like:

  obj-$(CONFIG_BOOTCOUNT_AM33XX)  += bootcount_davinci.o

because that config variable is somewhat redundant. we already know
that the only way you get to process bootcount code is by setting
CONFIG_BOOTCOUNT_LIMIT, so it's kind of goofy to further define
*another* BOOTCOUNT variable that's target specific, since you still
need the first one, which leads to the following silliness in
include/configs/am335x_evm.h:

  #define CONFIG_BOOTCOUNT_LIMIT
  #define CONFIG_BOOTCOUNT_AM33XX    <--- oh, yuck

the first define already selects bootcount functionality; that second
define just looks silly and somewhat redundant.

  i think it's fine to define further BOOTCOUNT-related variables that
select general *classes* of bootcount functionality, like:

  CONFIG_BOOTCOUNT_ENV
  CONFIG_BOOTCOUNT_RAM
  CONFIG_BOOTCOUNT_I2C

but for really target-specific bootcount implementations, i have no
problem with tests like:

  obj-$(CONFIG_AT91SAM9XE)        += bootcount_at91.o
  obj-$(CONFIG_BLACKFIN)          += bootcount_blackfin.o
  obj-$(CONFIG_SOC_DA8XX)         += bootcount_davinci.o

the one that now offends me is this one:

  obj-$(CONFIG_BOOTCOUNT_AM33XX)  += bootcount_davinci.o

which i think should be rewritten as simply:

  obj-$(CONFIG_AM33XX)            += bootcount_davinci.o

(or using whatever CONFIG setting selects AM33XX).

  in short (and, yes, i'm waiting for a massive build to complete so i
have time to kill), i would avoid creating any new Kbuild variables of
the form CONFIG_BOOTCOUNT_target_board. in fact, in a perfect world, i
would have the directory drivers/bootcount/ contain *only* general
bootcount functionality (generic, RAM, I2C, ...), and perhaps move the
target-specific stuff to those target-specific source directories. but
that's just me.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Robert P. J. Day]

  a short followup to my earlier post after i did a bit more reading
... i can see a bit more clearly the purpose of the Kbuild variable
CONFIG_BOOTCOUNT_AM33XX being used in drivers/bootcount/Makefile:

  obj-$(CONFIG_BOOTCOUNT_AM33XX)  += bootcount_davinci.o

since, apparently, only some(?) AM33xx targets use the corresponding
davinci RTC that can be used for this (if i'm reading that correctly):

  $ grep -r "#define.*CONFIG_BOOTCOUNT_AM33XX" *
  include/configs/am335x_sl50.h:#define CONFIG_BOOTCOUNT_AM33XX
  include/configs/bav335x.h:#define CONFIG_BOOTCOUNT_AM33XX
  include/configs/baltos.h:#define CONFIG_BOOTCOUNT_AM33XX
  include/configs/am335x_evm.h:#define CONFIG_BOOTCOUNT_AM33XX
  include/configs/brppt1.h:#define CONFIG_BOOTCOUNT_AM33XX
  $

so i can appreciate that you can't just check CONFIG_AM33XX as that
wouldn't apply to all of the other AM33xx targets.

  however, rather than invent a whole new variable for this, i think
it would be clearer if that Makefile tested specifically for the
property in question -- the presence of the davinci RTC or something
similar, perhaps checking CONFIG_RTC_DAVINCI in conjunction with
CONFIG_AM33XX?

  in short, i think it's cleaner to avoid creating whole new Kbuild
variables if one can use existing ones that make Makefiles easier to
understand. but now i admit i'm just being picky.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h

[Heiko Schocher]

Hello Robert,

sorry for replying late, but I was on vacation ... and as I got
while I was on vacation!, a new Internetconnection my current
Internetaccess state is unstable, incl. loosing EMails, because
my provider disabled my EMail address (seems currently already
re-enabled ... so sorry, if I not respond to EMails here on this
list)

Am 25.07.2016 um 12:03 schrieb Robert P. J. Day:
> On Mon, 25 Jul 2016, Wolfgang Denk wrote:
>
>> Dear Robert,
>>
>> In message <alpine.LFD.2.20.1607230737010.12216@localhost.localdomain> you wrote:
>>
>>>     3094                 CONFIG_AT91SAM9XE
>>>     3095                 enable special bootcounter support on at91sam9xe based boards.
>>>     3096                 CONFIG_BLACKFIN
>>>     3097                 enable special bootcounter support on blackfin based boards.
>>>     3098                 CONFIG_SOC_DA8XX
>>>     3099                 enable special bootcounter support on da850 based boards.
>>
>> This is name space pollution t best, and has potential to cause
>> unwanted side effects.  This needs thorough checking and cleanup, if
>> it should turn out thatthese macros are used only to select specific
>> bootcount implementations - in that case, they should be renamed
>> into something like CONFIG_BOOTCOUNT_* or such.
>>
>> Heiko, maybe you could have a look at that, please?
>
>    i'm not sure it's as bad as it looks, since those macros are used
> specifically in drivers/bootcount/Makefile:
>
>    obj-y                           += bootcount.o
>    obj-$(CONFIG_AT91SAM9XE)        += bootcount_at91.o
>    obj-$(CONFIG_BLACKFIN)          += bootcount_blackfin.o
>    obj-$(CONFIG_SOC_DA8XX)         += bootcount_davinci.o
>    obj-$(CONFIG_BOOTCOUNT_AM33XX)  += bootcount_davinci.o
>    obj-$(CONFIG_BOOTCOUNT_RAM)     += bootcount_ram.o
>    obj-$(CONFIG_BOOTCOUNT_ENV)     += bootcount_env.o
>    obj-$(CONFIG_BOOTCOUNT_I2C)     += bootcount_i2c.o
>
> and drivers/bootcount/ is processed only if:
>
>    obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/

Yes, excatly.

>
> but i do see the single, more precise example of
> CONFIG_BOOTCOUNT_AM33XX, so someone else can decide if anything
> should be renamed here.

I think the

 >    obj-$(CONFIG_SOC_DA8XX)         += bootcount_davinci.o
 >    obj-$(CONFIG_BOOTCOUNT_AM33XX)  += bootcount_davinci.o

part, should be renamed into "CONFIG_BOOTCOUNT_DAVINCI" ... and
may all symbols should start with "CONFIG_BOOTCOUNT_*" ...

Can you proide a patch for this?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] a few questions about saving bootcount in the environment

[Robert P. J. Day]

  a few quick questions about this feature before i move on to the
more widely-used stuff involving bootcount.

  first, it seems that there's not a lot of saving bootcount in the
environment ... as i see it, there's the taurus board, and there's the
boards that include "siemens-am33x-common.h", of which i see six:

include/configs/rut.h:#include "siemens-am33x-common.h"
include/configs/draco.h:#include "siemens-am33x-common.h"
include/configs/etamin.h:#include "siemens-am33x-common.h"
include/configs/rastaban.h:#include "siemens-am33x-common.h"
include/configs/pxm2.h:#include "siemens-am33x-common.h"
include/configs/thuban.h:#include "siemens-am33x-common.h"

so this tells me that there's not a whole lot of that feature being
used, so i won't spend much time on it.

  also, just to confirm, the "upgrade_available" variable is used
*exclusively* for the case of bootcount in the environment and nowhere
else, correct? so, again, if i'm not using the environment, i don't
care about it.

  finally, i read this in the README:

  CONFIG_BOOTCOUNT_ENV
  If no softreset save registers are found on the hardware
  "bootcount" is stored in the environment. To prevent a
  saveenv on all reboots, the environment variable
  "upgrade_available" is used. If "upgrade_available" is
  0, "bootcount" is always 0, if "upgrade_available" is
  1 "bootcount" is incremented in the environment.
  So the Userspace Applikation must set the "upgrade_available"
  and "bootcount" variable to 0, if a boot was successfully.

now, i can see where one wants to reset "bootcount" to zero once you
boot successfully, but why would you also set "upgrade_available" to
zero? don't you want to keep using that feature when you boot in the
future?

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] a few questions about saving bootcount in the environment

[Wolfgang Denk]

Dear Robert,

In message <alpine.LFD.2.20.1607231318540.16761@localhost.localdomain> you wrote:
> 
> so this tells me that there's not a whole lot of that feature being
> used, so i won't spend much time on it.

Right, it is only a last resort when you cannot find any better place
to storeit (in a hardware register that survives resets).

>   also, just to confirm, the "upgrade_available" variable is used
> *exclusively* for the case of bootcount in the environment and nowhere
> else, correct? so, again, if i'm not using the environment, i don't
> care about it.

Right.  It is only needed to reduce the number of writes to the
environment.

> now, i can see where one wants to reset "bootcount" to zero once you
> boot successfully, but why would you also set "upgrade_available" to
> zero? don't you want to keep using that feature when you boot in the
> future?

--> Heiko ?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Include the success of others in your dreams for your own success.

[U-Boot] a few questions about saving bootcount in the environment

[Tom Rini]

On Mon, Jul 25, 2016 at 06:57:21AM +0200, Wolfgang Denk wrote:
> Dear Robert,
> 
> In message <alpine.LFD.2.20.1607231318540.16761@localhost.localdomain> you wrote:
> > 
> > so this tells me that there's not a whole lot of that feature being
> > used, so i won't spend much time on it.
> 
> Right, it is only a last resort when you cannot find any better place
> to storeit (in a hardware register that survives resets).

That's not strictly true.  One of the things I noticed recently is that
Mender uses bootcount, in environment, as a least common denominator.
And thrown in a file in a filesystem, in so far as you trust the
underlying black box to be good about reads/writes and wear levelling,
it's robust enough (for certain values of robust and enough).  We're
dipping into one of those areas where experts have varying opinions on
what's good enough, hence all the qualifiers.  But it is a useful
option.  And neatly circumvents the need for a "driver" to clear the
count too.

[snip]
> > now, i can see where one wants to reset "bootcount" to zero once you
> > boot successfully, but why would you also set "upgrade_available" to
> > zero? don't you want to keep using that feature when you boot in the
> > future?
> 
> --> Heiko ?

Heiko wrote a kernel driver, for the non-env side of things at least at
one point, and the DT bindings didn't get approved.  I picked it up
years ago, tried, and had different DT binding approval problems and
never cycled back to it while I was at TI playing with that kind of
stuff.  Which is why the bootcount in env side of things is so
attractive, it's just fw_setenv/printenv once you're happy with your
system boot.

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

[U-Boot] a few questions about saving bootcount in the environment

[Wolfgang Denk]

Dear Tom,

In message <20160725135754.GN14698@bill-the-cat> you wrote:
> 
> > Right, it is only a last resort when you cannot find any better place
> > to storeit (in a hardware register that survives resets).
> 
> That's not strictly true.  One of the things I noticed recently is that
> Mender uses bootcount, in environment, as a least common denominator.
> And thrown in a file in a filesystem, in so far as you trust the
> underlying black box to be good about reads/writes and wear levelling,
> it's robust enough (for certain values of robust and enough).  We're
> dipping into one of those areas where experts have varying opinions on
> what's good enough, hence all the qualifiers.  But it is a useful
> option.  And neatly circumvents the need for a "driver" to clear the
> count too.

Agreed.  Let me rephrase the warning, then: it is always a good idea
to minimize the number of writes to the environment, especially when
done automagically.  One should at least avoid to re-write it on every
boot, especially when storage is some (flash based) storage device
with a limited number of erase/write cycles.  And even on other stoage
each write access includes the risk of errors.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
People don't like to be lectured to, but if you can make them  laugh,
their  defenses  come  down,  and for the time being they've accepted
whatever truth is embedded in your humor.             - Paul Krassner

[U-Boot] a few questions about saving bootcount in the environment

[Robert P. J. Day]

On Mon, 25 Jul 2016, Wolfgang Denk wrote:

> Dear Tom,
>
> In message <20160725135754.GN14698@bill-the-cat> you wrote:
> >
> > > Right, it is only a last resort when you cannot find any better place
> > > to storeit (in a hardware register that survives resets).
> >
> > That's not strictly true.  One of the things I noticed recently is that
> > Mender uses bootcount, in environment, as a least common denominator.
> > And thrown in a file in a filesystem, in so far as you trust the
> > underlying black box to be good about reads/writes and wear levelling,
> > it's robust enough (for certain values of robust and enough).  We're
> > dipping into one of those areas where experts have varying opinions on
> > what's good enough, hence all the qualifiers.  But it is a useful
> > option.  And neatly circumvents the need for a "driver" to clear the
> > count too.
>
> Agreed.  Let me rephrase the warning, then: it is always a good idea
> to minimize the number of writes to the environment, especially when
> done automagically.  One should at least avoid to re-write it on every
> boot, especially when storage is some (flash based) storage device
> with a limited number of erase/write cycles.  And even on other stoage
> each write access includes the risk of errors.

  i'm sure i'm asking the obvious, but if every boot to user space is
successful, bootcount will have a value of zero each time, yes? so if
user space code checks it and it's zero, then, you're done, no need to
write.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] a few questions about saving bootcount in the environment

[Wolfgang Denk]

Dear Robert,

In message <alpine.LFD.2.20.1607251355090.3253@localhost.localdomain> you wrote:
> 
>   i'm sure i'm asking the obvious, but if every boot to user space is
> successful, bootcount will have a value of zero each time, yes? so if
> user space code checks it and it's zero, then, you're done, no need to
> write.

As Heiko already explained, the update of the boot counter (= read
from env, increment, rewrite environment to persistent storage) is
done in U-Boot, and without "upgrade_available" it would happen on
each and every boot of the system.

The boot counter is implemented in U-Boot - only the resetting is done
in user space (and requires another write).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Generally speaking, there are other ways to accomplish whatever it is
that you think you need ...                               - Doug Gwyn

[U-Boot] a few questions about saving bootcount in the environment

[Robert P. J. Day]

On Mon, 25 Jul 2016, Wolfgang Denk wrote:

> Dear Robert,
>
> In message <alpine.LFD.2.20.1607251355090.3253@localhost.localdomain> you wrote:
> >
> >   i'm sure i'm asking the obvious, but if every boot to user space is
> > successful, bootcount will have a value of zero each time, yes? so if
> > user space code checks it and it's zero, then, you're done, no need to
> > write.
>
> As Heiko already explained, the update of the boot counter (= read
> from env, increment, rewrite environment to persistent storage) is
> done in U-Boot, and without "upgrade_available" it would happen on
> each and every boot of the system.
>
> The boot counter is implemented in U-Boot - only the resetting is done
> in user space (and requires another write).

  whoops, i missed that bit, just trying to catch up now.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] a few questions about saving bootcount in the environment

[Heiko Schocher]

Hello Tom,

Am 25.07.2016 um 15:57 schrieb Tom Rini:
> On Mon, Jul 25, 2016 at 06:57:21AM +0200, Wolfgang Denk wrote:
>> Dear Robert,
>>
>> In message <alpine.LFD.2.20.1607231318540.16761@localhost.localdomain> you wrote:
>>>
>>> so this tells me that there's not a whole lot of that feature being
>>> used, so i won't spend much time on it.
>>
>> Right, it is only a last resort when you cannot find any better place
>> to storeit (in a hardware register that survives resets).
>
> That's not strictly true.  One of the things I noticed recently is that
> Mender uses bootcount, in environment, as a least common denominator.
> And thrown in a file in a filesystem, in so far as you trust the
> underlying black box to be good about reads/writes and wear levelling,
> it's robust enough (for certain values of robust and enough).  We're
> dipping into one of those areas where experts have varying opinions on
> what's good enough, hence all the qualifiers.  But it is a useful
> option.  And neatly circumvents the need for a "driver" to clear the
> count too.
>
> [snip]
>>> now, i can see where one wants to reset "bootcount" to zero once you
>>> boot successfully, but why would you also set "upgrade_available" to
>>> zero? don't you want to keep using that feature when you boot in the
>>> future?
>>
>> --> Heiko ?
>
> Heiko wrote a kernel driver, for the non-env side of things at least at
> one point, and the DT bindings didn't get approved.  I picked it up
> years ago, tried, and had different DT binding approval problems and
> never cycled back to it while I was at TI playing with that kind of
> stuff.  Which is why the bootcount in env side of things is so
> attractive, it's just fw_setenv/printenv once you're happy with your
> system boot.

Yep ... why do we need "upgrade_available"?

in ./common/autoboot.c on *every* boot bootcount_store() gets
called, so on every boot (which can happen very often) the Environment
would be written ... not good, for long life of your product (if
you have the Env in raw nand flash for example, as for the siemens
boards)

If the bootcounter feature is only needed if you really did an
update, to most writtes are unnecessary ... so the Userspace App can
set also "upgrade_available" to enable the bootcount feature in case
we save the bootcounter in the Environment ... and save a lot of
writtes to the Env

But if you say "I don;t care how often I write the Environment" you
simply can set "upgrade_available" to 1 and have the "original"
behaviour of the bootcount feature (incrementing bootcount on each
reboot ...)

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] a few questions about saving bootcount in the environment

[Robert P. J. Day]

On Mon, 25 Jul 2016, Heiko Schocher wrote:

... snip ...

> Yep ... why do we need "upgrade_available"?
>
> in ./common/autoboot.c on *every* boot bootcount_store() gets
> called, so on every boot (which can happen very often) the Environment
> would be written ... not good, for long life of your product (if
> you have the Env in raw nand flash for example, as for the siemens
> boards)
>
> If the bootcounter feature is only needed if you really did an
> update, to most writtes are unnecessary ... so the Userspace App can
> set also "upgrade_available" to enable the bootcount feature in case
> we save the bootcounter in the Environment ... and save a lot of
> writtes to the Env
>
> But if you say "I don;t care how often I write the Environment" you
> simply can set "upgrade_available" to 1 and have the "original"
> behaviour of the bootcount feature (incrementing bootcount on each
> reboot ...)

  ok, i am now *totally* confused, so let's start at the beginning.
it's my understanding that selecting CONFIG_BOOTCOUNT_LIMIT means you
want to use the boot counter, it does *not* select what method you
want to use to store the boot counter, correct? but here's the
confusing part.

  if i look in common/autoboot.c, i see:

    const char *bootdelay_process(void)
    {
          char *s;
          int bootdelay;
    #ifdef CONFIG_BOOTCOUNT_LIMIT
          unsigned long bootcount = 0;
          unsigned long bootlimit = 0;
    #endif /* CONFIG_BOOTCOUNT_LIMIT */

    #ifdef CONFIG_BOOTCOUNT_LIMIT
          bootcount = bootcount_load();
          bootcount++;
          bootcount_store(bootcount);
          setenv_ulong("bootcount", bootcount);      <----- ?????
          bootlimit = getenv_ulong("bootlimit", 10, 0);
    #endif /* CONFIG_BOOTCOUNT_LIMIT */

          ... snip ...

and that's what confuses the heck out of me. i thought that, depending
on the technique i wanted to use to store the boot counter (RAM, I2C,
whatever), it would use *exclusively* that technique, and while i can
see the use of the routines bootcount_load() and bootcount_store(),
which are redefined based on the technique you select:

  $ grep -rw bootcount_store *
  bootcount_at91.c:void bootcount_store(ulong a)
  bootcount_blackfin.c:void bootcount_store(ulong cnt)
  bootcount.c:__weak void bootcount_store(ulong a)
  bootcount_davinci.c:void bootcount_store(ulong a)
  bootcount_env.c:void bootcount_store(ulong a)
  bootcount_i2c.c:void bootcount_store(ulong a)
  bootcount_i2c.c:	bootcount_store(0);
  bootcount_ram.c:void bootcount_store(ulong a)
  $

it appears that, no matter what, the environment *is* updated every
single time because of this line in the bootdelay_process() routine:

  setenv_ulong("bootcount", bootcount);

why? it seems, from the above, that no matter what boot counter
mechanism you use, the environment will be updated because of that
line.

  i can certainly see in bootcount_env.c where using
"upgrade_available" can deselect writing to the environment based on
invoking bootcount_store(), but that in no way stops that explicit
call to setenv_ulong() in that routine above.

  so how am i misunderstanding this? it seems that no matter what boot
counter storage mechanism i select, i'll be writing to the environment
every time *anyway*.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] a few questions about saving bootcount in the environment

[Wolfgang Denk]

Dear Robert,

In message <alpine.LFD.2.20.1607260734020.10018@localhost.localdomain> you wrote:
> 
>   ok, i am now *totally* confused, so let's start at the beginning.
> it's my understanding that selecting CONFIG_BOOTCOUNT_LIMIT means you

right.

> want to use the boot counter, it does *not* select what method you
> want to use to store the boot counter, correct? but here's the
> confusing part.

Which exact part is confusing?

>     #ifdef CONFIG_BOOTCOUNT_LIMIT
>           bootcount = bootcount_load();
>           bootcount++;
>           bootcount_store(bootcount);
>           setenv_ulong("bootcount", bootcount);      <----- ?????
>           bootlimit = getenv_ulong("bootlimit", 10, 0);
>     #endif /* CONFIG_BOOTCOUNT_LIMIT */

This is only the equivalent of a "setenv", so you have a variable in
the environment which you can use in scripts or compare against
"bootlimit".

There is NO automatic "saveenv" here...

> and that's what confuses the heck out of me. i thought that, depending
> on the technique i wanted to use to store the boot counter (RAM, I2C,

The key word here is "store", i. e. store in such a way that the value
survives a warm boot / reset.  The copy in RAM which is created above
does NOT survive.

> whatever), it would use *exclusively* that technique, and while i can
> see the use of the routines bootcount_load() and bootcount_store(),
> which are redefined based on the technique you select:

These methods access the persistent copy of the variable.  The
setenv_ulong() just creates a transient work copy.

> it appears that, no matter what, the environment *is* updated every
> single time because of this line in the bootdelay_process() routine:
> 
>   setenv_ulong("bootcount", bootcount);

Yes, it is.  But it is NOT saved to persistent storage.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In an organization, each person rises to the level of his own  incom-
petency                                         - The Peter Principle

[U-Boot] a few questions about saving bootcount in the environment

[Robert P. J. Day]

On Tue, 26 Jul 2016, Wolfgang Denk wrote:

> Dear Robert,
>
> In message <alpine.LFD.2.20.1607260734020.10018@localhost.localdomain> you wrote:
> >
> >   ok, i am now *totally* confused, so let's start at the beginning.
> > it's my understanding that selecting CONFIG_BOOTCOUNT_LIMIT means you
>
> right.
>
> > want to use the boot counter, it does *not* select what method you
> > want to use to store the boot counter, correct? but here's the
> > confusing part.
>
> Which exact part is confusing?
>
> >     #ifdef CONFIG_BOOTCOUNT_LIMIT
> >           bootcount = bootcount_load();
> >           bootcount++;
> >           bootcount_store(bootcount);
> >           setenv_ulong("bootcount", bootcount);      <----- ?????
> >           bootlimit = getenv_ulong("bootlimit", 10, 0);
> >     #endif /* CONFIG_BOOTCOUNT_LIMIT */
>
> This is only the equivalent of a "setenv", so you have a variable in
> the environment which you can use in scripts or compare against
> "bootlimit".
>
> There is NO automatic "saveenv" here...

  ah, you're right, you're right, sorry. little by little ...

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] a few questions about saving bootcount in the environment

[Heiko Schocher]

Hello Robert,

Am 26.07.2016 um 14:21 schrieb Robert P. J. Day:
> On Mon, 25 Jul 2016, Heiko Schocher wrote:
>
> ... snip ...
>
>> Yep ... why do we need "upgrade_available"?
>>
>> in ./common/autoboot.c on *every* boot bootcount_store() gets
>> called, so on every boot (which can happen very often) the Environment
>> would be written ... not good, for long life of your product (if
>> you have the Env in raw nand flash for example, as for the siemens
>> boards)
>>
>> If the bootcounter feature is only needed if you really did an
>> update, to most writtes are unnecessary ... so the Userspace App can
>> set also "upgrade_available" to enable the bootcount feature in case
>> we save the bootcounter in the Environment ... and save a lot of
>> writtes to the Env
>>
>> But if you say "I don;t care how often I write the Environment" you
>> simply can set "upgrade_available" to 1 and have the "original"
>> behaviour of the bootcount feature (incrementing bootcount on each
>> reboot ...)
>
>    ok, i am now *totally* confused, so let's start at the beginning.
> it's my understanding that selecting CONFIG_BOOTCOUNT_LIMIT means you
> want to use the boot counter, it does *not* select what method you
> want to use to store the boot counter, correct? but here's the

Yes, thats correct.

> confusing part.
>
>    if i look in common/autoboot.c, i see:
>
>      const char *bootdelay_process(void)
>      {
>            char *s;
>            int bootdelay;
>      #ifdef CONFIG_BOOTCOUNT_LIMIT
>            unsigned long bootcount = 0;
>            unsigned long bootlimit = 0;
>      #endif /* CONFIG_BOOTCOUNT_LIMIT */
>
>      #ifdef CONFIG_BOOTCOUNT_LIMIT
>            bootcount = bootcount_load();
>            bootcount++;
>            bootcount_store(bootcount);
>            setenv_ulong("bootcount", bootcount);      <----- ?????
>            bootlimit = getenv_ulong("bootlimit", 10, 0);
>      #endif /* CONFIG_BOOTCOUNT_LIMIT */
>
>            ... snip ...
>
> and that's what confuses the heck out of me. i thought that, depending
> on the technique i wanted to use to store the boot counter (RAM, I2C,
> whatever), it would use *exclusively* that technique, and while i can
> see the use of the routines bootcount_load() and bootcount_store(),
> which are redefined based on the technique you select:
>
>    $ grep -rw bootcount_store *
>    bootcount_at91.c:void bootcount_store(ulong a)
>    bootcount_blackfin.c:void bootcount_store(ulong cnt)
>    bootcount.c:__weak void bootcount_store(ulong a)
>    bootcount_davinci.c:void bootcount_store(ulong a)
>    bootcount_env.c:void bootcount_store(ulong a)
>    bootcount_i2c.c:void bootcount_store(ulong a)
>    bootcount_i2c.c:	bootcount_store(0);
>    bootcount_ram.c:void bootcount_store(ulong a)
>    $
>
> it appears that, no matter what, the environment *is* updated every
> single time because of this line in the bootdelay_process() routine:
>
>    setenv_ulong("bootcount", bootcount);
>
> why? it seems, from the above, that no matter what boot counter

Yes, the bootcount variable gets set here ... but the environment
gets not written...

> mechanism you use, the environment will be updated because of that
> line.

The Environment gets written when saveenv() gets called ... or?

>    i can certainly see in bootcount_env.c where using
> "upgrade_available" can deselect writing to the environment based on
> invoking bootcount_store(), but that in no way stops that explicit
> call to setenv_ulong() in that routine above.
>
>    so how am i misunderstanding this? it seems that no matter what boot
> counter storage mechanism i select, i'll be writing to the environment
> every time *anyway*.

The Environmentvariable gets updated on boot, but not written into
the storage device.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] a few questions about saving bootcount in the environment

[Robert P. J. Day]

On Tue, 26 Jul 2016, Heiko Schocher wrote:

... snip ...

> > it appears that, no matter what, the environment *is* updated
> > every single time because of this line in the bootdelay_process()
> > routine:
> >
> >    setenv_ulong("bootcount", bootcount);
> >
> > why? it seems, from the above, that no matter what boot counter
>
> Yes, the bootcount variable gets set here ... but the environment
> gets not written...
>
> > mechanism you use, the environment will be updated because of that
> > line.
>
> The Environment gets written when saveenv() gets called ... or?

... snip ...

  yes, i already apologized to wolfgang for being so clueless, i
didn't see the difference between "setenv" and "saveenv". anyway, i'm
still writing a wiki page on this that i can use for future embedded
linux classes, so ... onward.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

[U-Boot] a few questions about saving bootcount in the environment

[Heiko Schocher]

Hello Robert,

Am 27.07.2016 um 14:03 schrieb Robert P. J. Day:
> On Tue, 26 Jul 2016, Heiko Schocher wrote:
>
> ... snip ...
>
>>> it appears that, no matter what, the environment *is* updated
>>> every single time because of this line in the bootdelay_process()
>>> routine:
>>>
>>>     setenv_ulong("bootcount", bootcount);
>>>
>>> why? it seems, from the above, that no matter what boot counter
>>
>> Yes, the bootcount variable gets set here ... but the environment
>> gets not written...
>>
>>> mechanism you use, the environment will be updated because of that
>>> line.
>>
>> The Environment gets written when saveenv() gets called ... or?
>
> ... snip ...
>
>    yes, i already apologized to wolfgang for being so clueless, i
> didn't see the difference between "setenv" and "saveenv". anyway, i'm
> still writing a wiki page on this that i can use for future embedded
> linux classes, so ... onward.

no problem, I saw your response to wolfgang to late ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] a few questions about saving bootcount in the environment

[Wolfgang Denk]

Dear Tom,

I wrote:

> Agreed.  Let me rephrase the warning, then: it is always a good idea
> to minimize the number of writes to the environment, especially when
> done automagically.  One should at least avoid to re-write it on every
> boot, especially when storage is some (flash based) storage device
> with a limited number of erase/write cycles.  And even on other stoage
> each write access includes the risk of errors.

And of course access to the environment in inherently much slower than
a register write. This may not matter in many cases, but when
optimizing boot time it may be important, too.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Some people march to the beat of a different drummer. And some people
tango!