* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
@ 2016-07-22 19:36 Robert P. J. Day
2016-07-22 22:01 ` Wolfgang Denk
0 siblings, 1 reply; 12+ messages in thread
From: Robert P. J. Day @ 2016-07-22 19:36 UTC (permalink / raw)
To: u-boot
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
========================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-22 19:36 [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h Robert P. J. Day
@ 2016-07-22 22:01 ` Wolfgang Denk
2016-07-23 4:32 ` Robert P. J. Day
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Wolfgang Denk @ 2016-07-22 22:01 UTC (permalink / raw)
To: u-boot
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_
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-22 22:01 ` Wolfgang Denk
@ 2016-07-23 4:32 ` Robert P. J. Day
2016-07-23 10:18 ` Robert P. J. Day
2016-07-23 11:42 ` Robert P. J. Day
2 siblings, 0 replies; 12+ messages in thread
From: Robert P. J. Day @ 2016-07-23 4:32 UTC (permalink / raw)
To: u-boot
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
========================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-22 22:01 ` Wolfgang Denk
2016-07-23 4:32 ` Robert P. J. Day
@ 2016-07-23 10:18 ` Robert P. J. Day
2016-07-23 11:42 ` Robert P. J. Day
2 siblings, 0 replies; 12+ messages in thread
From: Robert P. J. Day @ 2016-07-23 10:18 UTC (permalink / raw)
To: u-boot
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
========================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-22 22:01 ` Wolfgang Denk
2016-07-23 4:32 ` Robert P. J. Day
2016-07-23 10:18 ` Robert P. J. Day
@ 2016-07-23 11:42 ` Robert P. J. Day
2016-07-25 4:54 ` Wolfgang Denk
2 siblings, 1 reply; 12+ messages in thread
From: Robert P. J. Day @ 2016-07-23 11:42 UTC (permalink / raw)
To: u-boot
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
========================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-23 11:42 ` Robert P. J. Day
@ 2016-07-25 4:54 ` Wolfgang Denk
2016-07-25 10:03 ` Robert P. J. Day
0 siblings, 1 reply; 12+ messages in thread
From: Wolfgang Denk @ 2016-07-25 4:54 UTC (permalink / raw)
To: u-boot
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_
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-25 4:54 ` Wolfgang Denk
@ 2016-07-25 10:03 ` Robert P. J. Day
2016-07-25 13:26 ` Wolfgang Denk
2016-07-25 17:07 ` Heiko Schocher
0 siblings, 2 replies; 12+ messages in thread
From: Robert P. J. Day @ 2016-07-25 10:03 UTC (permalink / raw)
To: u-boot
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
========================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-25 10:03 ` Robert P. J. Day
@ 2016-07-25 13:26 ` Wolfgang Denk
2016-07-25 17:24 ` [U-Boot] a few questions about saving bootcount in the environment Heiko Schocher
2016-07-26 11:02 ` [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h Robert P. J. Day
2016-07-25 17:07 ` Heiko Schocher
1 sibling, 2 replies; 12+ messages in thread
From: Wolfgang Denk @ 2016-07-25 13:26 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-25 10:03 ` Robert P. J. Day
2016-07-25 13:26 ` Wolfgang Denk
@ 2016-07-25 17:07 ` Heiko Schocher
1 sibling, 0 replies; 12+ messages in thread
From: Heiko Schocher @ 2016-07-25 17:07 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] a few questions about saving bootcount in the environment
2016-07-25 13:26 ` Wolfgang Denk
@ 2016-07-25 17:24 ` Heiko Schocher
2016-07-26 11:02 ` [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h Robert P. J. Day
1 sibling, 0 replies; 12+ messages in thread
From: Heiko Schocher @ 2016-07-25 17:24 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-25 13:26 ` Wolfgang Denk
2016-07-25 17:24 ` [U-Boot] a few questions about saving bootcount in the environment Heiko Schocher
@ 2016-07-26 11:02 ` Robert P. J. Day
2016-07-26 11:22 ` Robert P. J. Day
1 sibling, 1 reply; 12+ messages in thread
From: Robert P. J. Day @ 2016-07-26 11:02 UTC (permalink / raw)
To: u-boot
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
========================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h
2016-07-26 11:02 ` [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h Robert P. J. Day
@ 2016-07-26 11:22 ` Robert P. J. Day
0 siblings, 0 replies; 12+ messages in thread
From: Robert P. J. Day @ 2016-07-26 11:22 UTC (permalink / raw)
To: u-boot
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
========================================================================
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-07-26 11:22 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-22 19:36 [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h Robert P. J. Day
2016-07-22 22:01 ` Wolfgang Denk
2016-07-23 4:32 ` Robert P. J. Day
2016-07-23 10:18 ` Robert P. J. Day
2016-07-23 11:42 ` Robert P. J. Day
2016-07-25 4:54 ` Wolfgang Denk
2016-07-25 10:03 ` Robert P. J. Day
2016-07-25 13:26 ` Wolfgang Denk
2016-07-25 17:24 ` [U-Boot] a few questions about saving bootcount in the environment Heiko Schocher
2016-07-26 11:02 ` [U-Boot] confused by "upgrade_available=0\0" in include/configs/taurus.h Robert P. J. Day
2016-07-26 11:22 ` Robert P. J. Day
2016-07-25 17:07 ` Heiko Schocher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.