| Message ID | 1534240262-8904-4-git-send-email-philipp.tomsich@theobroma-systems.com |
|---|---|
| State | Superseded |
| Delegated to: | Philipp Tomsich |
| Headers | show |
| Series | Update RV3029 driver to DM and add DM-backed bootcount support | expand |
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@theobroma-systems.com> > Tested-by: Klaus Goger <klaus.goger@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@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? 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)? 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@denx.de
Lukasz, > On 14 Aug 2018, at 13:10, Lukasz Majewski <lukma@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@theobroma-systems.com> >> Tested-by: Klaus Goger <klaus.goger@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@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@denx.de
Hi Philipp, On 14 August 2018 at 05:39, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > Lukasz, > >> On 14 Aug 2018, at 13:10, Lukasz Majewski <lukma@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@theobroma-systems.com> >>> Tested-by: Klaus Goger <klaus.goger@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@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. I have to agree with Lukasz that this should really be a uclass with a driver for RTC and perhaps one for EEPROM. But we also have patches roaming around for a BOARD uclass, which provides for reading settings from the board, which could of course use any kind of storage. Would that be another option? Regards, Simon
Simon, > On 17 Aug 2018, at 14:49, Simon Glass <sjg@chromium.org> wrote: > > Hi Philipp, > > On 14 August 2018 at 05:39, Dr. Philipp Tomsich > <philipp.tomsich@theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote: >> Lukasz, >> >>> On 14 Aug 2018, at 13:10, Lukasz Majewski <lukma@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@theobroma-systems.com> >>>> Tested-by: Klaus Goger <klaus.goger@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@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. > > I have to agree with Lukasz that this should really be a uclass with a > driver for RTC and perhaps one for EEPROM. > > But we also have patches roaming around for a BOARD uclass, which > provides for reading settings from the board, which could of course > use any kind of storage. Would that be another option? I’ll change this over to be a new BOOTCOUNT uclass, although my reservation regarding how this partitions a RTC’s or EEPROM’s address space remains. I guess, we’ll have to live with that. I don’t think this should be merged with the BOARD uclass, as the bootcount is a standalone feature that could be configured differently even for the same board (i.e. this is not similar enough to reading a board id to merge it into the BOARD uclass). Thanks, Philipp.
Hi Philipp, On 17 August 2018 at 06:56, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > Simon, > > On 17 Aug 2018, at 14:49, Simon Glass <sjg@chromium.org> wrote: > > Hi Philipp, > > On 14 August 2018 at 05:39, Dr. Philipp Tomsich > <philipp.tomsich@theobroma-systems.com> wrote: > > Lukasz, > > On 14 Aug 2018, at 13:10, Lukasz Majewski <lukma@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@theobroma-systems.com> > Tested-by: Klaus Goger <klaus.goger@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@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. > > > I have to agree with Lukasz that this should really be a uclass with a > driver for RTC and perhaps one for EEPROM. > > But we also have patches roaming around for a BOARD uclass, which > provides for reading settings from the board, which could of course > use any kind of storage. Would that be another option? > > > I’ll change this over to be a new BOOTCOUNT uclass, although my reservation > regarding how this partitions a RTC’s or EEPROM’s address space remains. > I guess, we’ll have to live with that. I don't fully understand this but will await the patch for this. I'm assuming a DT property would be needed. > > I don’t think this should be merged with the BOARD uclass, as the bootcount > is a standalone feature that could be configured differently even for the > same > board (i.e. this is not similar enough to reading a board id to merge it > into the > BOARD uclass). > OK. I suppose the BOARD uclass could make use of a BOOTCOUNT device if needed. Regards, Simon
Simon, > On 23 Aug 2018, at 12:45, Simon Glass <sjg@chromium.org> wrote: > > Hi Philipp, > > On 17 August 2018 at 06:56, Dr. Philipp Tomsich > <philipp.tomsich@theobroma-systems.com> wrote: >> Simon, >> >> On 17 Aug 2018, at 14:49, Simon Glass <sjg@chromium.org> wrote: >> >> Hi Philipp, >> >> On 14 August 2018 at 05:39, Dr. Philipp Tomsich >> <philipp.tomsich@theobroma-systems.com> wrote: >> >> Lukasz, >> >> On 14 Aug 2018, at 13:10, Lukasz Majewski <lukma@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@theobroma-systems.com> >> Tested-by: Klaus Goger <klaus.goger@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@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. >> >> >> I have to agree with Lukasz that this should really be a uclass with a >> driver for RTC and perhaps one for EEPROM. >> >> But we also have patches roaming around for a BOARD uclass, which >> provides for reading settings from the board, which could of course >> use any kind of storage. Would that be another option? >> >> >> I’ll change this over to be a new BOOTCOUNT uclass, although my reservation >> regarding how this partitions a RTC’s or EEPROM’s address space remains. >> I guess, we’ll have to live with that. > > I don't fully understand this but will await the patch for this. I'm > assuming a DT property would be needed. A RTC device may have more than the 2 bytes of SRAM, which might be subdivided into multiple storage regions (e.g. first 2 bytes for bootcount, next 6 bytes for something else). My concerns are about modelling this as part of the device-tree. When we have a bootcount-uclass, this will only be visible in the -u-boot.dtsi and it makes sense to require all info regarding bootcount-placement within the delegate device to be in the u-boot only node. Let’s assume the following (pseudo-DTS) example: bootcount { compatible = “u-boot,bootcount-rtc”; rtc = <&my-rtc>; offset = <0x38>; } my-rtc { compatible = “…”; vendor,some-other-info-offset-in-sram-offset = <0x38>; } My concern/reservation is that such a situation might be detected very late, due to the offset not being in the rtc’s node… then again, this might not even ever become an issue and my fear of it might only be informed by my own perception. Thanks, Philipp. >> I don’t think this should be merged with the BOARD uclass, as the bootcount >> is a standalone feature that could be configured differently even for the >> same >> board (i.e. this is not similar enough to reading a board id to merge it >> into the >> BOARD uclass). >> > > OK. I suppose the BOARD uclass could make use of a BOOTCOUNT device if needed. > > Regards, > Simon
Hi Philipp, On 23 August 2018 at 07:48, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > Simon, > >> On 23 Aug 2018, at 12:45, Simon Glass <sjg@chromium.org> wrote: >> >> Hi Philipp, >> >> On 17 August 2018 at 06:56, Dr. Philipp Tomsich >> <philipp.tomsich@theobroma-systems.com> wrote: >>> Simon, >>> >>> On 17 Aug 2018, at 14:49, Simon Glass <sjg@chromium.org> wrote: >>> >>> Hi Philipp, >>> >>> On 14 August 2018 at 05:39, Dr. Philipp Tomsich >>> <philipp.tomsich@theobroma-systems.com> wrote: >>> >>> Lukasz, >>> >>> On 14 Aug 2018, at 13:10, Lukasz Majewski <lukma@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@theobroma-systems.com> >>> Tested-by: Klaus Goger <klaus.goger@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@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. >>> >>> >>> I have to agree with Lukasz that this should really be a uclass with a >>> driver for RTC and perhaps one for EEPROM. >>> >>> But we also have patches roaming around for a BOARD uclass, which >>> provides for reading settings from the board, which could of course >>> use any kind of storage. Would that be another option? >>> >>> >>> I’ll change this over to be a new BOOTCOUNT uclass, although my reservation >>> regarding how this partitions a RTC’s or EEPROM’s address space remains. >>> I guess, we’ll have to live with that. >> >> I don't fully understand this but will await the patch for this. I'm >> assuming a DT property would be needed. > > A RTC device may have more than the 2 bytes of SRAM, which might be subdivided > into multiple storage regions (e.g. first 2 bytes for bootcount, next 6 bytes for something > else). My concerns are about modelling this as part of the device-tree. > > When we have a bootcount-uclass, this will only be visible in the -u-boot.dtsi and it > makes sense to require all info regarding bootcount-placement within the delegate > device to be in the u-boot only node. > Let’s assume the following (pseudo-DTS) example: > bootcount { > compatible = “u-boot,bootcount-rtc”; > rtc = <&my-rtc>; > offset = <0x38>; > } > > my-rtc { > compatible = “…”; > vendor,some-other-info-offset-in-sram-offset = <0x38>; > } > > My concern/reservation is that such a situation might be detected very late, due to > the offset not being in the rtc’s node… then again, this might not even ever become > an issue and my fear of it might only be informed by my own perception. Yes that's possible. But I suppose this problem comes up in various places and we can worry about it later? Regards, Simon
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@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; +}