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

[Alex Kiernan]

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.

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

-- 
Alex Kiernan