[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 at 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
========================================================================