Hi Stefano, > Hi Lukasz, > > On 22/02/2018 12:52, Lukasz Majewski wrote: > > Hi Stefano, > > > >> If the bootcounter address is in a cached memory, > >> a flush of dcache must occur after updateing the bootcounter. > >> > >> Issue found on i.MX6 where bootcounter is put into the internal > >> (cached) IRAM. > >> > >> Signed-off-by: Stefano Babic <sba...@denx.de> > >> --- > >> drivers/bootcount/bootcount.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/bootcount/bootcount.c > >> b/drivers/bootcount/bootcount.c index d5ce450..48594a6 100644 > >> --- a/drivers/bootcount/bootcount.c > >> +++ b/drivers/bootcount/bootcount.c > >> @@ -59,6 +59,9 @@ __weak void bootcount_store(ulong a) > >> raw_bootcount_store(reg, a); > >> raw_bootcount_store(reg + 4, BOOTCOUNT_MAGIC); > >> #endif /* defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD */ > >> + flush_dcache_range(CONFIG_SYS_BOOTCOUNT_ADDR, > >> + CONFIG_SYS_BOOTCOUNT_ADDR + > >> + CONFIG_SYS_CACHELINE_SIZE); > > > > Is it safe to flush the whole cache line? > > Is there is some drawbacks ?
I think not - as we are now in u-boot. (I do have patches, which add also support for bootcount in SPL - I will send them when Kconfig prerequisities go into mainline). > > flush_dcache_range() requires that addresses are aligned with > CONFIG_SYS_CACHELINE_SIZE. I cannot flush a single word. Yes. Correct. > > > > > For iMX6Q I do use SNVS_LPGR register (0x020CC068). > > It is a long story... :-) > > > It will preserve > > its content after reset caused by WDT (also reset command in > > u-boot). I also do use the SINGLEWORD to store "magic" and boot > > count in a single 32 bit number. > > > > I used it in the past - Heiko found issues with recent kernel versions > because kernel is using it. That means, after a rebootwe get what the > kernel has written in it, not the bootcounter anymore. You mean the SRC_GPRx registers? > > It looks like, too, that SNVS behavior changes between i.MX6 variants. > Even in manual, there are discordances (on i.MX6Q seems can be used, > on DL seems to be reserved,...). What I can say.... :/ > > Due to recent issues, I searched for a suitable place in IRAM. Ok. > > Anyway, this has nothing to do with the issue. If the storage with the > bootcounter is cached, it must be flushed. Yes. Of course. > > > > You may also want to consider using SRC_GPRx registers: > > https://community.nxp.com/message/985790?commentID=985790&et=watches.email.thread#comment-985790 > > > > See above, waiting for Heiko's comment,too. Ok. Maybe the reply from the NXP community is not complete... > > > > > As it shall be safe to use them for bootcount scenario. > > Rather, it looks like it is not safe. Or it was safe, it is not. Or it > depends on i.MX6 revisions.... It would be great if Heiko could share the problem with SRC_GPRx registers. I would like to know the root cause for the future usage. > > > However, I do > > prefer SNVS_LPGR. > > You're lucky you do not yet go into trouble :-) Devlopment by luck :-) > > > > >> } > >> > >> __weak ulong bootcount_load(void) > > > > > > > > Best regards, > Stefano > > 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: w...@denx.de
pgpxTViUnnDDb.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot