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@denx.de