| Message ID | 20200220092830.1345351-1-hs@denx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bootcounter: add DM support for memory based bootcounter | expand |
Hi Heiko, On Thu, 20 Feb 2020 at 02:28, Heiko Schocher <hs at denx.de> wrote: > > add DM/DTS support for the memory based bootcounter > in drivers/bootcount/bootcount.c. > > Let the old implementation in, so boards which have > not yet convert to DM/DTS do not break. > > Signed-off-by: Heiko Schocher <hs at denx.de> > --- > Travis build: > > https://travis-ci.org/hsdenx/u-boot-test/builds/652839618 > > doc/device-tree-bindings/misc/bootcounter.txt | 21 +++++ > drivers/bootcount/Kconfig | 5 ++ > drivers/bootcount/Makefile | 1 + > drivers/bootcount/bootcount.c | 86 +++++++++++++++++++ > 4 files changed, 113 insertions(+) > create mode 100644 doc/device-tree-bindings/misc/bootcounter.txt > > diff --git a/doc/device-tree-bindings/misc/bootcounter.txt b/doc/device-tree-bindings/misc/bootcounter.txt > new file mode 100644 > index 0000000000..f4a4a731b9 > --- /dev/null > +++ b/doc/device-tree-bindings/misc/bootcounter.txt > @@ -0,0 +1,21 @@ > +U-Boot bootcounter Devicetree Binding > +===================================== > + > +The device tree node describes the U-Boot bootcounter > +memory based device binding. > + > +Required properties : > + > +- compatible : "uboot,bootcount"; I think we use u-boot in other bindings > +- singleword : set this, if you have only one word space > + for storing the bootcounter. single-word I think this is a boolean property, right? What is a word? Is it 32 bits? Also, what does it actually mean/do? > + > +Example > +------- > + > +MPC83xx based board: > + > +bootcount at 0x13ff8 { > + compatible = "uboot,bootcount"; > + reg = <0x13ff8 0x08>; > +}; > diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig > index 0e506c9ea2..88203607a8 100644 > --- a/drivers/bootcount/Kconfig > +++ b/drivers/bootcount/Kconfig > @@ -106,6 +106,11 @@ config DM_BOOTCOUNT_I2C_EEPROM > pointing to the underlying i2c eeprom device) and an optional 'offset' > property are supported. > > +config BOOTCOUNT_MEM > + bool "memory based bootcounter" > + help > + Memory based bootcount, compatible = "uboot,bootcount"; > + > endmenu > > endif > diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile > index 73ccfb5a08..059d40d16b 100644 > --- a/drivers/bootcount/Makefile > +++ b/drivers/bootcount/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0+ > > obj-$(CONFIG_BOOTCOUNT_GENERIC) += bootcount.o > +obj-$(CONFIG_BOOTCOUNT_MEM) += bootcount.o > obj-$(CONFIG_BOOTCOUNT_AT91) += bootcount_at91.o > obj-$(CONFIG_BOOTCOUNT_AM33XX) += bootcount_davinci.o > obj-$(CONFIG_BOOTCOUNT_RAM) += bootcount_ram.o > diff --git a/drivers/bootcount/bootcount.c b/drivers/bootcount/bootcount.c > index 7a6d03dcca..53bd416cf6 100644 > --- a/drivers/bootcount/bootcount.c > +++ b/drivers/bootcount/bootcount.c > @@ -8,6 +8,7 @@ > #include <cpu_func.h> > #include <linux/compiler.h> > > +#if !defined(CONFIG_DM_BOOTCOUNT) > /* Now implement the generic default functions */ > __weak void bootcount_store(ulong a) > { > @@ -49,3 +50,88 @@ __weak ulong bootcount_load(void) > return raw_bootcount_load(reg); > #endif /* defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD) */ > } > +#else > +#include <dm.h> > + Comment for struct > +struct bootcount_mem_priv { > + phys_addr_t base; > + u8 singleword; bool? > +}; > + > +static int bootcount_mem_get(struct udevice *dev, u32 *a) > +{ > + struct bootcount_mem_priv *priv = dev_get_priv(dev); > + void *reg = (void *)priv->base; > + u32 magic = CONFIG_SYS_BOOTCOUNT_MAGIC; > + > + if (priv->singleword) { > + u32 tmp = raw_bootcount_load(reg); > + > + if ((tmp & 0xffff0000) != (magic & 0xffff0000)) > + return -ENODEV; > + > + *a = (tmp & 0x0000ffff); > + } else { > + if (raw_bootcount_load(reg + 4) != magic) > + return -ENODEV; > + > + *a = raw_bootcount_load(reg); > + } > + > + return 0; > +}; > + > +static int bootcount_mem_set(struct udevice *dev, const u32 a) > +{ > + struct bootcount_mem_priv *priv = dev_get_priv(dev); > + void *reg = (void *)priv->base; > + u32 magic = CONFIG_SYS_BOOTCOUNT_MAGIC; > + uintptr_t flush_start = rounddown(priv->base, > + CONFIG_SYS_CACHELINE_SIZE); > + uintptr_t flush_end; > + > + if (priv->singleword) { > + raw_bootcount_store(reg, (magic & 0xffff0000) | a); > + flush_end = roundup(priv->base + 4, > + CONFIG_SYS_CACHELINE_SIZE); > + } else { > + raw_bootcount_store(reg, a); > + raw_bootcount_store(reg + 4, magic); > + flush_end = roundup(priv->base + 8, > + CONFIG_SYS_CACHELINE_SIZE); > + } > + flush_dcache_range(flush_start, flush_end); > + > + return 0; > +}; > + > +static const struct bootcount_ops bootcount_mem_ops = { > + .get = bootcount_mem_get, > + .set = bootcount_mem_set, > +}; > + > +static int bootcount_mem_probe(struct udevice *dev) > +{ > + struct bootcount_mem_priv *priv = dev_get_priv(dev); > + > + priv->base = (phys_addr_t)devfdt_get_addr(dev); dev_read_addr() > + priv->singleword = dev_read_u32_default(dev, "singleword", 0); dev_read_bool() ? > + > + return 0; > +} > + > +static const struct udevice_id bootcount_mem_ids[] = { > + { .compatible = "uboot,bootcount" }, > + { .compatible = "u-boot,bootcount" }, Can we just have the second one? > + { } > +}; > + > +U_BOOT_DRIVER(bootcount_mem) = { > + .name = "bootcount-mem", > + .id = UCLASS_BOOTCOUNT, > + .priv_auto_alloc_size = sizeof(struct bootcount_mem_priv), > + .probe = bootcount_mem_probe, > + .of_match = bootcount_mem_ids, > + .ops = &bootcount_mem_ops, > +}; > +#endif > -- > 2.24.1 > Regards, Simon
Hello Simon, Am 21.02.2020 um 01:50 schrieb Simon Glass: > Hi Heiko, > > On Thu, 20 Feb 2020 at 02:28, Heiko Schocher <hs at denx.de> wrote: >> >> add DM/DTS support for the memory based bootcounter >> in drivers/bootcount/bootcount.c. >> >> Let the old implementation in, so boards which have >> not yet convert to DM/DTS do not break. >> >> Signed-off-by: Heiko Schocher <hs at denx.de> >> --- >> Travis build: >> >> https://travis-ci.org/hsdenx/u-boot-test/builds/652839618 >> >> doc/device-tree-bindings/misc/bootcounter.txt | 21 +++++ >> drivers/bootcount/Kconfig | 5 ++ >> drivers/bootcount/Makefile | 1 + >> drivers/bootcount/bootcount.c | 86 +++++++++++++++++++ >> 4 files changed, 113 insertions(+) >> create mode 100644 doc/device-tree-bindings/misc/bootcounter.txt >> >> diff --git a/doc/device-tree-bindings/misc/bootcounter.txt b/doc/device-tree-bindings/misc/bootcounter.txt >> new file mode 100644 >> index 0000000000..f4a4a731b9 >> --- /dev/null >> +++ b/doc/device-tree-bindings/misc/bootcounter.txt >> @@ -0,0 +1,21 @@ >> +U-Boot bootcounter Devicetree Binding >> +===================================== >> + >> +The device tree node describes the U-Boot bootcounter >> +memory based device binding. >> + >> +Required properties : >> + >> +- compatible : "uboot,bootcount"; > > I think we use u-boot in other bindings Yes. I just porting some mpc83xx boards to DM/DTS and they use "uboot,bootcounter" ... but I think, it should be possible to change this in the respectiv DTS files. > >> +- singleword : set this, if you have only one word space >> + for storing the bootcounter. > > single-word Yep > > I think this is a boolean property, right? Yes. > What is a word? Is it 32 bits? Also, what does it actually mean/do? If enabled bootcounter driver does use one word (32 bit) only. See CONFIG_SYS_BOOTCOUNT_SINGLEWORD in current version of mainline driver. >> + >> +Example >> +------- >> + >> +MPC83xx based board: >> + >> +bootcount at 0x13ff8 { >> + compatible = "uboot,bootcount"; >> + reg = <0x13ff8 0x08>; >> +}; >> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig >> index 0e506c9ea2..88203607a8 100644 >> --- a/drivers/bootcount/Kconfig >> +++ b/drivers/bootcount/Kconfig >> @@ -106,6 +106,11 @@ config DM_BOOTCOUNT_I2C_EEPROM >> pointing to the underlying i2c eeprom device) and an optional 'offset' >> property are supported. >> >> +config BOOTCOUNT_MEM >> + bool "memory based bootcounter" >> + help >> + Memory based bootcount, compatible = "uboot,bootcount"; >> + >> endmenu >> >> endif >> diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile >> index 73ccfb5a08..059d40d16b 100644 >> --- a/drivers/bootcount/Makefile >> +++ b/drivers/bootcount/Makefile >> @@ -1,6 +1,7 @@ >> # SPDX-License-Identifier: GPL-2.0+ >> >> obj-$(CONFIG_BOOTCOUNT_GENERIC) += bootcount.o >> +obj-$(CONFIG_BOOTCOUNT_MEM) += bootcount.o >> obj-$(CONFIG_BOOTCOUNT_AT91) += bootcount_at91.o >> obj-$(CONFIG_BOOTCOUNT_AM33XX) += bootcount_davinci.o >> obj-$(CONFIG_BOOTCOUNT_RAM) += bootcount_ram.o >> diff --git a/drivers/bootcount/bootcount.c b/drivers/bootcount/bootcount.c >> index 7a6d03dcca..53bd416cf6 100644 >> --- a/drivers/bootcount/bootcount.c >> +++ b/drivers/bootcount/bootcount.c >> @@ -8,6 +8,7 @@ >> #include <cpu_func.h> >> #include <linux/compiler.h> >> >> +#if !defined(CONFIG_DM_BOOTCOUNT) >> /* Now implement the generic default functions */ >> __weak void bootcount_store(ulong a) >> { >> @@ -49,3 +50,88 @@ __weak ulong bootcount_load(void) >> return raw_bootcount_load(reg); >> #endif /* defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD) */ >> } >> +#else >> +#include <dm.h> >> + > > Comment for struct Of course, I add one, sorry for this. >> +struct bootcount_mem_priv { >> + phys_addr_t base; >> + u8 singleword; > > bool? Yes, I change this to bool. >> +}; >> + >> +static int bootcount_mem_get(struct udevice *dev, u32 *a) >> +{ >> + struct bootcount_mem_priv *priv = dev_get_priv(dev); >> + void *reg = (void *)priv->base; >> + u32 magic = CONFIG_SYS_BOOTCOUNT_MAGIC; >> + >> + if (priv->singleword) { >> + u32 tmp = raw_bootcount_load(reg); >> + >> + if ((tmp & 0xffff0000) != (magic & 0xffff0000)) >> + return -ENODEV; >> + >> + *a = (tmp & 0x0000ffff); >> + } else { >> + if (raw_bootcount_load(reg + 4) != magic) >> + return -ENODEV; >> + >> + *a = raw_bootcount_load(reg); >> + } >> + >> + return 0; >> +}; >> + >> +static int bootcount_mem_set(struct udevice *dev, const u32 a) >> +{ >> + struct bootcount_mem_priv *priv = dev_get_priv(dev); >> + void *reg = (void *)priv->base; >> + u32 magic = CONFIG_SYS_BOOTCOUNT_MAGIC; >> + uintptr_t flush_start = rounddown(priv->base, >> + CONFIG_SYS_CACHELINE_SIZE); >> + uintptr_t flush_end; >> + >> + if (priv->singleword) { >> + raw_bootcount_store(reg, (magic & 0xffff0000) | a); >> + flush_end = roundup(priv->base + 4, >> + CONFIG_SYS_CACHELINE_SIZE); >> + } else { >> + raw_bootcount_store(reg, a); >> + raw_bootcount_store(reg + 4, magic); >> + flush_end = roundup(priv->base + 8, >> + CONFIG_SYS_CACHELINE_SIZE); >> + } >> + flush_dcache_range(flush_start, flush_end); >> + >> + return 0; >> +}; >> + >> +static const struct bootcount_ops bootcount_mem_ops = { >> + .get = bootcount_mem_get, >> + .set = bootcount_mem_set, >> +}; >> + >> +static int bootcount_mem_probe(struct udevice *dev) >> +{ >> + struct bootcount_mem_priv *priv = dev_get_priv(dev); >> + >> + priv->base = (phys_addr_t)devfdt_get_addr(dev); > > dev_read_addr() Ok, I check it with dev_read_addr() > >> + priv->singleword = dev_read_u32_default(dev, "singleword", 0); > > dev_read_bool() ? Yep. > >> + >> + return 0; >> +} >> + >> +static const struct udevice_id bootcount_mem_ids[] = { >> + { .compatible = "uboot,bootcount" }, >> + { .compatible = "u-boot,bootcount" }, > > Can we just have the second one? I ask, if we can change the DTS files they use. > >> + { } >> +}; >> + >> +U_BOOT_DRIVER(bootcount_mem) = { >> + .name = "bootcount-mem", >> + .id = UCLASS_BOOTCOUNT, >> + .priv_auto_alloc_size = sizeof(struct bootcount_mem_priv), >> + .probe = bootcount_mem_probe, >> + .of_match = bootcount_mem_ids, >> + .ops = &bootcount_mem_ops, >> +}; >> +#endif >> -- >> 2.24.1 >> > > Regards, > Simon Thanks for the review. bye, Heiko >
diff --git a/doc/device-tree-bindings/misc/bootcounter.txt b/doc/device-tree-bindings/misc/bootcounter.txt new file mode 100644 index 0000000000..f4a4a731b9 --- /dev/null +++ b/doc/device-tree-bindings/misc/bootcounter.txt @@ -0,0 +1,21 @@ +U-Boot bootcounter Devicetree Binding +===================================== + +The device tree node describes the U-Boot bootcounter +memory based device binding. + +Required properties : + +- compatible : "uboot,bootcount"; +- singleword : set this, if you have only one word space + for storing the bootcounter. + +Example +------- + +MPC83xx based board: + +bootcount at 0x13ff8 { + compatible = "uboot,bootcount"; + reg = <0x13ff8 0x08>; +}; diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig index 0e506c9ea2..88203607a8 100644 --- a/drivers/bootcount/Kconfig +++ b/drivers/bootcount/Kconfig @@ -106,6 +106,11 @@ config DM_BOOTCOUNT_I2C_EEPROM pointing to the underlying i2c eeprom device) and an optional 'offset' property are supported. +config BOOTCOUNT_MEM + bool "memory based bootcounter" + help + Memory based bootcount, compatible = "uboot,bootcount"; + endmenu endif diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile index 73ccfb5a08..059d40d16b 100644 --- a/drivers/bootcount/Makefile +++ b/drivers/bootcount/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ obj-$(CONFIG_BOOTCOUNT_GENERIC) += bootcount.o +obj-$(CONFIG_BOOTCOUNT_MEM) += bootcount.o obj-$(CONFIG_BOOTCOUNT_AT91) += bootcount_at91.o obj-$(CONFIG_BOOTCOUNT_AM33XX) += bootcount_davinci.o obj-$(CONFIG_BOOTCOUNT_RAM) += bootcount_ram.o diff --git a/drivers/bootcount/bootcount.c b/drivers/bootcount/bootcount.c index 7a6d03dcca..53bd416cf6 100644 --- a/drivers/bootcount/bootcount.c +++ b/drivers/bootcount/bootcount.c @@ -8,6 +8,7 @@ #include <cpu_func.h> #include <linux/compiler.h> +#if !defined(CONFIG_DM_BOOTCOUNT) /* Now implement the generic default functions */ __weak void bootcount_store(ulong a) { @@ -49,3 +50,88 @@ __weak ulong bootcount_load(void) return raw_bootcount_load(reg); #endif /* defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD) */ } +#else +#include <dm.h> + +struct bootcount_mem_priv { + phys_addr_t base; + u8 singleword; +}; + +static int bootcount_mem_get(struct udevice *dev, u32 *a) +{ + struct bootcount_mem_priv *priv = dev_get_priv(dev); + void *reg = (void *)priv->base; + u32 magic = CONFIG_SYS_BOOTCOUNT_MAGIC; + + if (priv->singleword) { + u32 tmp = raw_bootcount_load(reg); + + if ((tmp & 0xffff0000) != (magic & 0xffff0000)) + return -ENODEV; + + *a = (tmp & 0x0000ffff); + } else { + if (raw_bootcount_load(reg + 4) != magic) + return -ENODEV; + + *a = raw_bootcount_load(reg); + } + + return 0; +}; + +static int bootcount_mem_set(struct udevice *dev, const u32 a) +{ + struct bootcount_mem_priv *priv = dev_get_priv(dev); + void *reg = (void *)priv->base; + u32 magic = CONFIG_SYS_BOOTCOUNT_MAGIC; + uintptr_t flush_start = rounddown(priv->base, + CONFIG_SYS_CACHELINE_SIZE); + uintptr_t flush_end; + + if (priv->singleword) { + raw_bootcount_store(reg, (magic & 0xffff0000) | a); + flush_end = roundup(priv->base + 4, + CONFIG_SYS_CACHELINE_SIZE); + } else { + raw_bootcount_store(reg, a); + raw_bootcount_store(reg + 4, magic); + flush_end = roundup(priv->base + 8, + CONFIG_SYS_CACHELINE_SIZE); + } + flush_dcache_range(flush_start, flush_end); + + return 0; +}; + +static const struct bootcount_ops bootcount_mem_ops = { + .get = bootcount_mem_get, + .set = bootcount_mem_set, +}; + +static int bootcount_mem_probe(struct udevice *dev) +{ + struct bootcount_mem_priv *priv = dev_get_priv(dev); + + priv->base = (phys_addr_t)devfdt_get_addr(dev); + priv->singleword = dev_read_u32_default(dev, "singleword", 0); + + return 0; +} + +static const struct udevice_id bootcount_mem_ids[] = { + { .compatible = "uboot,bootcount" }, + { .compatible = "u-boot,bootcount" }, + { } +}; + +U_BOOT_DRIVER(bootcount_mem) = { + .name = "bootcount-mem", + .id = UCLASS_BOOTCOUNT, + .priv_auto_alloc_size = sizeof(struct bootcount_mem_priv), + .probe = bootcount_mem_probe, + .of_match = bootcount_mem_ids, + .ops = &bootcount_mem_ops, +}; +#endif
add DM/DTS support for the memory based bootcounter in drivers/bootcount/bootcount.c. Let the old implementation in, so boards which have not yet convert to DM/DTS do not break. Signed-off-by: Heiko Schocher <hs at denx.de> --- Travis build: https://travis-ci.org/hsdenx/u-boot-test/builds/652839618 doc/device-tree-bindings/misc/bootcounter.txt | 21 +++++ drivers/bootcount/Kconfig | 5 ++ drivers/bootcount/Makefile | 1 + drivers/bootcount/bootcount.c | 86 +++++++++++++++++++ 4 files changed, 113 insertions(+) create mode 100644 doc/device-tree-bindings/misc/bootcounter.txt