[U-Boot] [PATCH 3/3] bootcount: add DM-based backing store for bootcount
Dr. Philipp Tomsich
philipp.tomsich at theobroma-systems.com
Tue Aug 14 11:39:59 UTC 2018
Lukasz,
> On 14 Aug 2018, at 13:10, Lukasz Majewski <lukma at denx.de> wrote:
>
> Hi Philipp,
>
>> The original bootcount methods do not provide an interface to DM and
>> rely on a static configuration for I2C devices (e.g. bus, chip-addr,
>> etc. are configured through defines statically). On a modern system
>> that exposes multiple devices in a DTS-configurable way, this is less
>> than optimal and a interface to DM-based devices will be desirable.
>>
>> This adds a simple driver that is DM-aware and configurable via DTS:
>> the /chosen/u-boot,bootcount-device property is used to detect the DM
>> device storing the bootcount and deviceclass-specific commands are
>> used to read/write the bootcount.
>>
>> Initially, this provides support for the following DM devices:
>> * RTC devices implementing the read8/write8 ops
>>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
>> Tested-by: Klaus Goger <klaus.goger at theobroma-systems.com>
>> ---
>>
>> doc/device-tree-bindings/chosen.txt | 27 +++++++++++
>> drivers/bootcount/Kconfig | 12 +++++
>> drivers/bootcount/Makefile | 1 +
>> drivers/bootcount/bootcount_dm.c | 93
>> +++++++++++++++++++++++++++++++++++++ 4 files changed, 133
>> insertions(+) create mode 100644 drivers/bootcount/bootcount_dm.c
>>
>> diff --git a/doc/device-tree-bindings/chosen.txt
>> b/doc/device-tree-bindings/chosen.txt index da7b4e6..734fd15 100644
>> --- a/doc/device-tree-bindings/chosen.txt
>> +++ b/doc/device-tree-bindings/chosen.txt
>> @@ -42,6 +42,33 @@ Example
>> };
>> };
>>
>> +u-boot,bootcount-device property
>> +--------------------------------
>> +In a DM-based system, the bootcount may be stored in a device known
>> to +the DM framework (e.g. in a battery-backed SRAM area within a RTC
>> +device). To identify the device to be used for the bootcount, the
>> +u-boot,bootcount-device property needs to point to the target device.
>> +
>> +Further configuration in the target device's node may be required
>> +(e.g. an offset into an I2C RTC's address space), depending on the
>> +UCLASS of the target device.
>> +
>> +Example
>> +-------
>> +/ {
>> + chosen {
>> + u-boot,bootcount-device = &rv3029;
>> + };
>> +
>> + i2c2 {
>> + rv3029: rtc at 56 {
>> + compatible = "mc,rv3029";
>> + reg = <0x56>;
>> + u-boot,bootcount-offset = <0x38>;
>> + };
>> + };
>> +};
>> +
>> u-boot,spl-boot-order property
>> ------------------------------
>>
>> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
>> index d335ed1..cdde7b5 100644
>> --- a/drivers/bootcount/Kconfig
>> +++ b/drivers/bootcount/Kconfig
>> @@ -70,6 +70,18 @@ config BOOTCOUNT_AT91
>> bool "Boot counter for Atmel AT91SAM9XE"
>> depends on AT91SAM9XE
>>
>> +config BOOTCOUNT_DM
>> + bool "Boot counter in a device-model device"
>> + help
>> + Enables reading/writing the bootcount in a device-model
>> + device referenced from the /chosen node. The type of the
>> + device is detected from DM and any additional configuration
>> + information (e.g. the offset into a RTC device that
>> supports
>> + read32/write32) is read from the device's node.
>> +
>> + At this time the following DM device classes are supported:
>> + * RTC (using read32/write32)
>> +
>> endchoice
>>
>> config BOOTCOUNT_ALEN
>> diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile
>> index 68bc006..e8ed729 100644
>> --- a/drivers/bootcount/Makefile
>> +++ b/drivers/bootcount/Makefile
>> @@ -7,3 +7,4 @@ obj-$(CONFIG_BOOTCOUNT_RAM) += bootcount_ram.o
>> obj-$(CONFIG_BOOTCOUNT_ENV) += bootcount_env.o
>> obj-$(CONFIG_BOOTCOUNT_I2C) += bootcount_i2c.o
>> obj-$(CONFIG_BOOTCOUNT_EXT) += bootcount_ext.o
>> +obj-$(CONFIG_BOOTCOUNT_DM) += bootcount_dm.o
>> diff --git a/drivers/bootcount/bootcount_dm.c
>> b/drivers/bootcount/bootcount_dm.c new file mode 100644
>> index 0000000..99bdb88
>> --- /dev/null
>> +++ b/drivers/bootcount/bootcount_dm.c
>> @@ -0,0 +1,93 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2018 Theobroma Systems Design und Consulting GmbH
>> + */
>> +
>> +#include <common.h>
>> +#include <bootcount.h>
>> +#include <dm.h>
>> +#include <rtc.h>
>> +
>> +const u8 bootcount_magic = 0xbc;
>> +
>> +static void bootcount_store_rtc(struct udevice *dev, ulong a)
>> +{
>> +#if CONFIG_IS_ENABLED(DM_RTC)
>> + u32 offset;
>> + const char *offset_propname = "u-boot,bootcount-offset";
>> + const u16 val = bootcount_magic << 8 | (a & 0xff);
>> +
>> + if (dev_read_u32(dev, offset_propname, &offset) < 0) {
>> + debug("%s: requires %s\n", __func__,
>> offset_propname);
>> + return;
>> + }
>> +
>> + if (rtc_write16(dev, offset, val) < 0) {
>> + debug("%s: rtc_write16 failed\n", __func__);
>> + return;
>> + }
>> +#endif
>> +}
>> +
>> +static u32 bootcount_load_rtc(struct udevice *dev)
>> +{
>> +#if CONFIG_IS_ENABLED(DM_RTC)
>> + u32 offset;
>> + const char *offset_propname = "u-boot,bootcount-offset";
>> + u16 val;
>> +
>> + if (dev_read_u32(dev, offset_propname, &offset) < 0) {
>> + debug("%s: requires %s\n", __func__,
>> offset_propname);
>> + goto fail;
>> + }
>> +
>> + if (rtc_read16(dev, offset, &val) < 0) {
>> + debug("%s: rtc_write16 failed\n", __func__);
>> + goto fail;
>> + }
>> +
>> + if (val >> 8 == bootcount_magic)
>> + return val & 0xff;
>> +
>> + debug("%s: bootcount magic does not match on %04x\n",
>> __func__, val);
>> + fail:
>> +#endif
>> + return 0;
>> +}
>> +
>> +/* Now implement the generic default functions */
>> +void bootcount_store(ulong a)
>> +{
>> + struct udevice *dev;
>> + ofnode node;
>> + const char *propname = "u-boot,bootcount-device";
>> +
>> + node = ofnode_get_chosen_node(propname);
>> + if (!ofnode_valid(node)) {
>> + debug("%s: no '%s'\n", __func__, propname);
>> + return;
>> + }
>> +
>> + /* RTC devices */
>> + if (!uclass_get_device_by_ofnode(UCLASS_RTC, node, &dev))
>> + return bootcount_store_rtc(dev, a);
>> +}
>> +
>> +ulong bootcount_load(void)
>> +{
>> + struct udevice *dev;
>> + ofnode node;
>> + const char *propname = "u-boot,bootcount-device";
>> +
>> + node = ofnode_get_chosen_node(propname);
>> + if (!ofnode_valid(node)) {
>> + debug("%s: no '%s'\n", __func__, propname);
>> + return 0;
>> + }
>> +
>> + /* RTC devices */
>> + if (!uclass_get_device_by_ofnode(UCLASS_RTC, node, &dev))
>> + return bootcount_load_rtc(dev);
>> +
>> + return 0;
>> +}
>
> Thanks for your patch.
>
> However, if I may ask - would it be possible to add support for EEPROM
> based bootcount in an easy way?
This was always intended and is the reason why there’s a "RTC devices”
comment in bootcount_load.
If someone wants to store their bootcount in an I2C EEPROM, they just
need to add a “uclass_get_device_by_ofnode(UCLASS_I2C_EEPROM, …)”
with the appropriate logic in bootcount_load and bootcount_store.
> I mean - do you think that it would be feasible to have
> bootcount-uclass, which would support generic load/store functions with
> DM drivers for RTC, EEPROM or even simple write to SFR registers (as it
> is just an offset in the end)?
I thought about this, but didn’t go down that route as a bootcount will usually
be stored in an existing DM device (RTC, I2C-EEPROM, NOR flash). If we
add individual bootcount-devices wrapping these, then we’d be left with the
following in the -u-boot.dtsi:
bootcount {
compatible = “u-boot,bootcount-rtc”
rtc = &rv3029;
offset = <0x38>
}
While this nicely collects all the info together in one node, there’s the drawback
of users that might define multiple devices and set their status to “okay”… which
might cause inconsistent bootcount values across multiple devices.
For this reason, I went with the other design choice of viewing the various bootcount
implementations as “bootcount methods” (i.e. logic storing and retrieving a bootcount
somewhere). In the case of DM backends this means that the appropriate method
is to (a) identify the device by its uclass and (b) call the appropriate read/write method.
I briefly toyed with the idea of adding infrastructure to the DM core to get the device
for an ofnode (independent of its uclass) and adding a generic dm_read/dm_write
that would dispatch to the appropriate uclass’ read/write after looking at the uclass
of a udevice passed in. I didn’t go down that route as it seemed to contradict the
architecture of DM (i.e. devices are stored in per-uclass lists) in U-Boot.
One more thing that influenced the current modelling in the DTS: it prefer to have
all info regarding how the SRAM in a RTC—same for the memory in an EEPROM—
is subdivided in the RTC node. If this was in a separate node (as the bootcount
node above), someone might reuse the same SRAM addresses for different
purposes from different nodes causing inadvertent overwriting of live data.
>
>
> 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 at denx.de
More information about the U-Boot
mailing list