Hello Philip,
On 13.10.23 11:43, Philip Richard Oberfichtner wrote:
> This adds a generic I2C bootcounter adhering to driver model to replace
> the previously removed legacy implementation.
>
> There is no change in functionality, it can be used on any I2C device.
> The device tree configuration may look like this for example:
>
> bootcount {
> compatible = "u-boot,bootcount-i2c";
> i2c-bus = <&i2c1>;
> address = <0x52>;
Hmm.. do we really need this here with DTS. Why not using a phandle
to a real i2c device? Something like this for example:
i2cbcdev = &i2c_rtc;
with
&i2c1 {
i2c_rtc: rtc@68 {
[...]
and so there is no need for knowing the bus and address ...
> offset = <0x11>;
> };
>
> Signed-off-by: Philip Richard Oberfichtner <p...@denx.de>
> ---
> drivers/bootcount/Kconfig | 10 +++
> drivers/bootcount/Makefile | 1 +
> drivers/bootcount/bootcount_dm_i2c.c | 126 +++++++++++++++++++++++++++
> 3 files changed, 137 insertions(+)
> create mode 100644 drivers/bootcount/bootcount_dm_i2c.c
>
> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
> index 7a2548ace2..1cf1de2b6d 100644
> --- a/drivers/bootcount/Kconfig
> +++ b/drivers/bootcount/Kconfig
> @@ -109,6 +109,16 @@ config DM_BOOTCOUNT_RTC
> Accesses to the backing store are performed using the write16
> and read16 ops of DM RTC devices.
>
> +config DM_BOOTCOUNT_I2C
> + bool "Driver Model boot counter on I2C device"
> + depends on DM_I2C
> + help
> + Enable support for the bootcounter on a generic i2c device, like a
> + RTC or PMIC. This requires an 'i2c-bus', the i2c chip 'address' and
> + the 'offset' to read to and write from. All of the three settings are
> + defined as device tree properties using the "u-boot,bootcount-i2c"
> + compatible string.
> +
> config DM_BOOTCOUNT_I2C_EEPROM
> bool "Support i2c eeprom devices as a backing store for bootcount"
> depends on I2C_EEPROM
> diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile
> index d6d2389c16..e7771f5b36 100644
> --- a/drivers/bootcount/Makefile
> +++ b/drivers/bootcount/Makefile
> @@ -13,5 +13,6 @@ obj-$(CONFIG_DM_BOOTCOUNT) += bootcount-uclass.o
> obj-$(CONFIG_DM_BOOTCOUNT_PMIC_PFUZE100) += pmic_pfuze100.o
> obj-$(CONFIG_DM_BOOTCOUNT_RTC) += rtc.o
> obj-$(CONFIG_DM_BOOTCOUNT_I2C_EEPROM) += i2c-eeprom.o
> +obj-$(CONFIG_DM_BOOTCOUNT_I2C) += bootcount_dm_i2c.o
> obj-$(CONFIG_DM_BOOTCOUNT_SPI_FLASH) += spi-flash.o
> obj-$(CONFIG_DM_BOOTCOUNT_SYSCON) += bootcount_syscon.o
> diff --git a/drivers/bootcount/bootcount_dm_i2c.c
> b/drivers/bootcount/bootcount_dm_i2c.c
> new file mode 100644
> index 0000000000..227641f77e
> --- /dev/null
> +++ b/drivers/bootcount/bootcount_dm_i2c.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2023
> + * Philip Richard Oberfichtner <p...@denx.de>
> + *
> + * Based on previous work from Heiko Schocher (legacy bootcount_i2c.c driver)
> + */
> +
> +#include <bootcount.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <i2c.h>
> +
> +#define BC_MAGIC 0x55
> +
> +struct bootcount_i2c_priv {
> + struct udevice *chip;
may rename chip to i2cbcdev or bcdev ?
> + unsigned int offset;
> +};
> +
> +static int bootcount_i2c_set(struct udevice *dev, const u32 val)
> +{
> + int ret;
> + struct bootcount_i2c_priv *priv = dev_get_priv(dev);
> +
> + ret = dm_i2c_reg_write(priv->chip, priv->offset, BC_MAGIC);
> + if (ret < 0)
> + goto err_exit;
> +
> + ret = dm_i2c_reg_write(priv->chip, priv->offset + 1, val & 0xff);
> + if (ret < 0)
> + goto err_exit;
> +
> + return 0;
> +
> +err_exit:
> + log_debug("%s: Error writing to I2C device (%d)\n", __func__, ret);
> + return ret;
> +}
> +
> +static int bootcount_i2c_get(struct udevice *dev, u32 *val)
> +{
> + int ret;
> + struct bootcount_i2c_priv *priv = dev_get_priv(dev);
> +
> + ret = dm_i2c_reg_read(priv->chip, priv->offset);
> + if (ret < 0)
> + goto err_exit;
> +
> + if ((ret & 0xff) != BC_MAGIC) {
> + log_debug("%s: Invalid Magic, reset bootcounter.\n", __func__);
> + *val = 0;
> + return bootcount_i2c_set(dev, 0);
> + }
> +
> + ret = dm_i2c_reg_read(priv->chip, priv->offset + 1);
> + if (ret < 0)
> + goto err_exit;
> +
> + *val = ret;
> + return 0;
> +
> +err_exit:
> + log_debug("%s: Error reading from I2C device (%d)\n", __func__, ret);
> + return ret;
> +}
> +
> +static int bootcount_i2c_probe(struct udevice *dev)
> +{
> + struct bootcount_i2c_priv *priv = dev_get_priv(dev);
> + struct ofnode_phandle_args phandle_args;
> + struct udevice *bus;
> + unsigned int addr;
> + int ret;
> +
> + ret = dev_read_u32(dev, "offset", &priv->offset);
> + if (ret) {
> + log_debug("%s: Unable to get offset\n", __func__);
> + return ret;
> + }
> +
> + ret = dev_read_u32(dev, "address", &addr);
> + if (ret) {
> + log_debug("%s: Unable to get chip address\n", __func__);
> + return ret;
> + }
> +
> + ret = dev_read_phandle_with_args(dev, "i2c-bus", NULL, 0, 0,
> &phandle_args);
> + if (ret) {
> + log_debug("%s: Unable to get phandle\n", __func__);
> + return ret;
> + }
> +
> + ret = uclass_get_device_by_ofnode(UCLASS_I2C, phandle_args.node, &bus);
> + if (ret) {
> + log_debug("%s: Unable to get i2c bus\n", __func__);
> + return ret;
> + }
> +
> + ret = i2c_get_chip(bus, addr, 1, &priv->chip);
> + if (ret) {
> + log_debug("%s: Unable to get i2c chip\n", __func__);
> + return ret;
> + }
when you use a phandle, you can replace the part from reading "offset"
with this:
uclass_get_device_by_phandle(UCLASS_I2C, dev, "i2cbcdev", &priv->bcdev);
of course plus error checking...
> +
> + return 0;
> +}
> +
> +static const struct bootcount_ops bootcount_i2c_ops = {
> + .get = bootcount_i2c_get,
> + .set = bootcount_i2c_set,
> +};
> +
> +static const struct udevice_id bootcount_i2c_ids[] = {
> + { .compatible = "u-boot,bootcount-i2c" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(bootcount_i2c) = {
> + .name = "bootcount-i2c",
> + .id = UCLASS_BOOTCOUNT,
> + .priv_auto = sizeof(struct bootcount_i2c_priv),
> + .probe = bootcount_i2c_probe,
> + .of_match = bootcount_i2c_ids,
> + .ops = &bootcount_i2c_ops,
> +};
>
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
