[U-Boot] [PATCH 8/8] Migrate generic bootcount to Kconfig

[Lukasz Majewski]

Hi Alex,

> On Tue, Feb 13, 2018 at 2:41 PM, Lukasz Majewski <lukma at denx.de>
> wrote:
> > Hi Alex,
> >  
> >> On Mon, Feb 12, 2018 at 8:48 AM, Lukasz Majewski <lukma at denx.de>
> >> wrote:  
> >> > Hi Alex,
> >> >  
> >> >> On Sun, Feb 11, 2018 at 9:44 PM, Lukasz Majewski <lukma at denx.de>
> >> >> wrote:  
> >> >> > On Sun, 11 Feb 2018 21:04:46 +0000
> >> >> > Alex Kiernan <alex.kiernan at gmail.com> wrote:
> >> >> >  
> >> >> >>
> >> >> >> That said, squashing in that change doesn't obviously break
> >> >> >> anything for me, and is probably a step in the right
> >> >> >> direction.
> >> >> >>
> >> >> >> I'll see what Travis thinks.
> >> >> >>  
> >> >> >
> >> >> > We will probably receive build breaks...  
> >> >>
> >> >> Yup... https://travis-ci.org/akiernan/u-boot/jobs/340344489
> >> >>
> >> >> Just tried again on one of those failures (x600) with the the
> >> >> default removed and just set the on board that uses CONFIG_EXT,
> >> >> but that then fails at config time.  
> >> >
> >> > Ok. I see
> >> >  
> >> >>
> >> >> TBH I'd actually like to take it out of Kconfig (which I
> >> >> realise is the wrong direction) as it just feels fundamentally
> >> >> wrong the way it is.  
> >> >
> >> > To finish moving SYS_BOOTCOUNT_ADDR to Kconfig we would need to
> >> > add its definition to each eligible ./configs/<board>_defconfig
> >> > file.
> >> >
> >> > Then we would have the previous behaviour preserved.
> >> >  
> >> >> I don't know what the U-Boot approach configuration like this
> >> >> is... struggling to find prior art.  
> >> >
> >> > Let's ask Tom for his opinion as he did much such work before.
> >> >  
> >>
> >> Thoughts:
> >>
> >> - Do nothing, leave CONFIG_SYS_BOOTCOUNT_ADDR as a purely
> >> CONFIG_EXT piece of Kconfig
> >> - Rename CONFIG_SYS_BOOTCOUNT_ADDR to something like
> >> CONFIG_SYS_BOOTCOUNT_ADDR_EXT for just CONFIG_EXT
> >> - Remove CONFIG_SYS_BOOTCOUNT_ADDR from Kconfig altogether (and
> >> rename to say SYS_BOOTCOUNT_ADDR)
> >> - Pick through every config building defaults - okay for some
> >> boards, but lots have it defined based on other memory offsets
> >>
> >> I think the only real options are the last two.  
> >
> > I would go for third option - Remove the SYS_BOOTCOUNT_ADDR from
> > Kconfig (also for EXT).
> >  
> 
> I agree.
> 
> >>
> >> Whatever we do, I think CONFIG_SYS_BOOTCOUNT_ADDR wants splitting
> >> into at least two:
> >>
> >> - I2C - an offset from an I2C base for the bootcounter  
> >   - RAM/SoC memory - location of special register to store
> > bootcounter  
> >> - Others - an actual address used for storing the bootcounter  
> >
> >
> >  
> >>
> >> I'm struggling to see why EXT is the way it is - AFAICS the
> >> location it uses to access/return the bootcounter is basically
> >> local to the two functions in bootcount_ext and could just be a
> >> local variable. 
> >
> > Maybe we can replace it with local, static variable then?
> >
> >
> > As said above - I would remove the (wrongly?) used BOOTCOUNT_ADDR in
> > Kconfig.
> >  
> 
> Just removing SYS_BOOTCOUNT_ADDR from Kconfig and putting the default
> back into bootcount_ext seems like the simplest correct change. I'll
> add that onto the series and throw it at Travis.

Great. I'm looking forward for next verison of the code - as I do have
some code to be put on top of it.

Thanks for help.

> 
> > Then, for next merge window we can play around with moving
> > CONFIG_SYS_BOOTCOUNT_ADDR per boards if needed.
> >
> > I just do have a feeling that -rc2 is too late for it....
> >
> > Also, wrong SYS_BOOTCOUNT_ADDR will not be caught in build testing
> > and may cause boards to silency break.
> >  
> 
> Yeah...
> 



Best regards,

Lukasz Majewski

--

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180214/f5614789/attachment.sig>