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@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@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@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"helpMemory 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