Merge patch series "bootcount: Small clean-up and fix, and dm_i2c single-word support"

Niko Mauno <niko.mauno@vaisala.com> says:

In this series, we first introduce a clean-up where we switch to use
predefined bit masks instead of hard-coded values for count and magic
halves in the single-word (32-bit) boot count scheme.

Then we fix a case of missing boot count value masking in single-word
scenario in bootcount.c, which allowed clobbering of the magic half
when storing the value. With this change the clobbering preventing
behavior becomes consistent with existing single word bootcount storing
implementations in bootcount_at91.c and bootcount_davinci.c.

Finally, we enable the DM I2C bootcount driver to work also in single
word (4 byte) mode, in addition to the pre-existing half-word (2 byte)
mode. By default the driver still operates in half word mode as so far,
but can now be used alternatively in single word mode by adding
'size = <0x4>;' in the associated device tree node.

Link: https://lore.kernel.org/r/cover.1770197302.git.niko.mauno@vaisala.com

drivers/bootcount/bootcount.c

@@ -19,7 +19,8 @@ __weak void bootcount_store(ulong a)
 	uintptr_t flush_end;
 
 #if defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD)
-	raw_bootcount_store(reg, (CONFIG_SYS_BOOTCOUNT_MAGIC & 0xffff0000) | a);
+	raw_bootcount_store(reg, (CONFIG_SYS_BOOTCOUNT_MAGIC & BOOTCOUNT_MAGIC_MASK)
+		| (a & BOOTCOUNT_COUNT_MASK));
 
 	flush_end = roundup(CONFIG_SYS_BOOTCOUNT_ADDR + 4,
 			    CONFIG_SYS_CACHELINE_SIZE);
@@ -40,10 +41,10 @@ __weak ulong bootcount_load(void)
 #if defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD)
 	u32 tmp = raw_bootcount_load(reg);
 
-	if ((tmp & 0xffff0000) != (CONFIG_SYS_BOOTCOUNT_MAGIC & 0xffff0000))
+	if ((tmp & BOOTCOUNT_MAGIC_MASK) != (CONFIG_SYS_BOOTCOUNT_MAGIC & BOOTCOUNT_MAGIC_MASK))
 		return 0;
 	else
-		return (tmp & 0x0000ffff);
+		return (tmp & BOOTCOUNT_COUNT_MASK);
 #else
 	if (raw_bootcount_load(reg + 4) != CONFIG_SYS_BOOTCOUNT_MAGIC)
 		return 0;
@@ -74,10 +75,10 @@ static int bootcount_mem_get(struct udevice *dev, u32 *a)
 	if (priv->singleword) {
 		u32 tmp = raw_bootcount_load(reg);
 
-		if ((tmp & 0xffff0000) != (magic & 0xffff0000))
+		if ((tmp & BOOTCOUNT_MAGIC_MASK) != (magic & BOOTCOUNT_MAGIC_MASK))
 			return -ENODEV;
 
-		*a = (tmp & 0x0000ffff);
+		*a = (tmp & BOOTCOUNT_COUNT_MASK);
 	} else {
 		if (raw_bootcount_load(reg + 4) != magic)
 			return -ENODEV;
@@ -98,7 +99,8 @@ static int bootcount_mem_set(struct udevice *dev, const u32 a)
 	uintptr_t flush_end;
 
 	if (priv->singleword) {
-		raw_bootcount_store(reg, (magic & 0xffff0000) | a);
+		raw_bootcount_store(reg, (magic & BOOTCOUNT_MAGIC_MASK)
+			| (a & BOOTCOUNT_COUNT_MASK));
 		flush_end = roundup(priv->base + 4,
 				    CONFIG_SYS_CACHELINE_SIZE);
 	} else {

drivers/bootcount/bootcount_at91.c

@@ -3,6 +3,7 @@
 #include <asm/io.h>
 #include <asm/arch/hardware.h>
 #include <asm/arch/at91_gpbr.h>
+#include <bootcount.h>
 
 /*
  * We combine the CONFIG_SYS_BOOTCOUNT_MAGIC and bootcount in one 32-bit
@@ -13,7 +14,7 @@ void bootcount_store(ulong a)
 {
 	at91_gpbr_t *gpbr = (at91_gpbr_t *) ATMEL_BASE_GPBR;
 
-	writel((CONFIG_SYS_BOOTCOUNT_MAGIC & 0xffff0000) | (a & 0x0000ffff),
+	writel((CONFIG_SYS_BOOTCOUNT_MAGIC & BOOTCOUNT_MAGIC_MASK) | (a & BOOTCOUNT_COUNT_MASK),
 	       &gpbr->reg[AT91_GPBR_INDEX_BOOTCOUNT]);
 }
 
@@ -22,8 +23,8 @@ ulong bootcount_load(void)
 	at91_gpbr_t *gpbr = (at91_gpbr_t *) ATMEL_BASE_GPBR;
 
 	ulong val = readl(&gpbr->reg[AT91_GPBR_INDEX_BOOTCOUNT]);
-	if ((val & 0xffff0000) != (CONFIG_SYS_BOOTCOUNT_MAGIC & 0xffff0000))
+	if ((val & BOOTCOUNT_MAGIC_MASK) != (CONFIG_SYS_BOOTCOUNT_MAGIC & BOOTCOUNT_MAGIC_MASK))
 		return 0;
 	else
-		return val & 0x0000ffff;
+		return val & BOOTCOUNT_COUNT_MASK;
 }

drivers/bootcount/bootcount_davinci.c

@@ -24,7 +24,7 @@ void bootcount_store(ulong a)
 	writel(RTC_KICK0R_WE, &reg->kick0r);
 	writel(RTC_KICK1R_WE, &reg->kick1r);
 	raw_bootcount_store(&reg->scratch2,
-		(CONFIG_SYS_BOOTCOUNT_MAGIC & 0xffff0000) | (a & 0x0000ffff));
+		(CONFIG_SYS_BOOTCOUNT_MAGIC & BOOTCOUNT_MAGIC_MASK) | (a & BOOTCOUNT_COUNT_MASK));
 }
 
 ulong bootcount_load(void)
@@ -34,8 +34,8 @@ ulong bootcount_load(void)
 		(struct davinci_rtc *)CONFIG_SYS_BOOTCOUNT_ADDR;
 
 	val = raw_bootcount_load(&reg->scratch2);
-	if ((val & 0xffff0000) != (CONFIG_SYS_BOOTCOUNT_MAGIC & 0xffff0000))
+	if ((val & BOOTCOUNT_MAGIC_MASK) != (CONFIG_SYS_BOOTCOUNT_MAGIC & BOOTCOUNT_MAGIC_MASK))
 		return 0;
 	else
-		return val & 0x0000ffff;
+		return val & BOOTCOUNT_COUNT_MASK;
 }

drivers/bootcount/bootcount_dm_i2c.c

@@ -15,6 +15,7 @@
 struct bootcount_i2c_priv {
 	struct udevice *bcdev;
 	unsigned int offset;
+	unsigned int size;
 };
 
 static int bootcount_i2c_set(struct udevice *dev, const u32 val)
@@ -22,13 +23,22 @@ 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->bcdev, priv->offset, BC_MAGIC);
-	if (ret < 0)
-		goto err_exit;
+	if (priv->size == 4) {
+		u32 bc = (CONFIG_SYS_BOOTCOUNT_MAGIC & BOOTCOUNT_MAGIC_MASK)
+			| (val & BOOTCOUNT_COUNT_MASK);
 
-	ret = dm_i2c_reg_write(priv->bcdev, priv->offset + 1, val & 0xff);
-	if (ret < 0)
-		goto err_exit;
+		ret = dm_i2c_write(priv->bcdev, priv->offset, (uint8_t *)&bc, sizeof(bc));
+		if (ret < 0)
+			goto err_exit;
+	} else {
+		ret = dm_i2c_reg_write(priv->bcdev, priv->offset, BC_MAGIC);
+		if (ret < 0)
+			goto err_exit;
+
+		ret = dm_i2c_reg_write(priv->bcdev, priv->offset + 1, val & 0xff);
+		if (ret < 0)
+			goto err_exit;
+	}
 
 	return 0;
 
@@ -42,21 +52,39 @@ 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->bcdev, priv->offset);
-	if (ret < 0)
-		goto err_exit;
+	if (priv->size == 4) {
+		u32 bc;
 
-	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_read(priv->bcdev, priv->offset, (uint8_t *)&bc, sizeof(bc));
+		if (ret < 0)
+			goto err_exit;
+
+		if ((bc & BOOTCOUNT_MAGIC_MASK) !=
+		    (CONFIG_SYS_BOOTCOUNT_MAGIC & BOOTCOUNT_MAGIC_MASK)) {
+			log_debug("%s: Invalid Magic, reset bootcounter.\n", __func__);
+			*val = 0;
+			return bootcount_i2c_set(dev, 0);
+		}
+
+		*val = (bc & BOOTCOUNT_COUNT_MASK);
+	} else {
+		ret = dm_i2c_reg_read(priv->bcdev, 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->bcdev, priv->offset + 1);
+		if (ret < 0)
+			goto err_exit;
+
+		*val = ret;
 	}
 
-	ret = dm_i2c_reg_read(priv->bcdev, priv->offset + 1);
-	if (ret < 0)
-		goto err_exit;
-
-	*val = ret;
 	return 0;
 
 err_exit:
@@ -73,6 +101,12 @@ static int bootcount_i2c_probe(struct udevice *dev)
 	if (ret)
 		goto exit;
 
+	priv->size = dev_read_u32_default(dev, "size", 2);
+	if (priv->size != 2 && priv->size != 4) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
 	ret = i2c_get_chip_by_phandle(dev, "i2cbcdev", &priv->bcdev);
 
 exit:

include/bootcount.h

@@ -10,6 +10,7 @@
 #include <asm/io.h>
 #include <asm/byteorder.h>
 #include <env.h>
+#include <linux/bitops.h>
 
 #ifdef CONFIG_DM_BOOTCOUNT
 
@@ -59,6 +60,10 @@ int dm_bootcount_set(struct udevice *dev, u32 bootcount);
 
 #endif
 
+/* Bit masks for magic and count parts in single word scheme */
+#define BOOTCOUNT_MAGIC_MASK	GENMASK(31, 16)
+#define BOOTCOUNT_COUNT_MASK	GENMASK(15, 0)
+
 /** bootcount_store() - store the current bootcount */
 void bootcount_store(ulong);