[PATCH] bootcounter: add DM support for memory based bootcounter
Heiko Schocher
hs at denx.de
Sat Feb 22 13:17:13 CET 2020
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
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
More information about the U-Boot
mailing list