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

Lukasz Majewski
2018-02-13T14:41:26Z

Hi Alex,

> On Mon, Feb 12, 2018 at 8:48 AM, Lukasz Majewski <lukma@denx.de>
> wrote:
> > Hi Alex,
> >
> >> On Sun, Feb 11, 2018 at 9:44 PM, Lukasz Majewski <lukma@denx.de>
> >> wrote:
> >> > On Sun, 11 Feb 2018 21:04:46 +0000
> >> > Alex Kiernan <alex.kiernan@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).

>
> 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.

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.

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/20180213/0894ba15/attachment.sig>