mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: barebox@lists.infradead.org
Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Subject: [PATCH 04/11] regmap: align regmap_bulk_read/write API with Linux
Date: Fri, 20 Oct 2023 09:18:46 +0200	[thread overview]
Message-ID: <20231020071853.2826528-5-a.fatoum@pengutronix.de> (raw)
In-Reply-To: <20231020071853.2826528-1-a.fatoum@pengutronix.de>

Since its inception in 2016, barebox regmap_bulk_read and
regmap_bulk_write expected the last argument to be the total
length of data to access in bytes.

Its namesake Linux version has the same prototype, but interprets the
last argument as number of elements to write, i.e.
bytes / regmap_get_val_bytes(map).

This went unnoticed so far, because barebox users are either using
1-byte regmaps, the code was written specifically for barebox
or the code is yet unused such as the KSZ switch 64-bit accessors.

Avoid nasty future surprises by switching implementation and users
to the Linux interpretation of the last argument. As courtesy for
out-of-tree board code, we poison the symbol when regmap.h is included,
so out of tree code doesn't silently run into the inverse issue.

Files with regmap.h replaced by linux/regmap.h added and no other changes
are already compatible with the new definitions. All other files are
adapted to the new definition through division by the value size in
bytes.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/mach-imx/iim.c            |  2 +-
 drivers/base/regmap/regmap-multi.c |  6 +++---
 drivers/base/regmap/regmap.c       | 25 +++++++++++--------------
 drivers/mfd/rn5t568.c              |  2 +-
 drivers/net/ksz_common.h           |  2 +-
 drivers/nvmem/bsec.c               |  4 ++--
 drivers/nvmem/ocotp.c              |  8 ++++----
 drivers/nvmem/regmap.c             | 14 ++++++++++++--
 drivers/nvmem/snvs_lpgpr.c         |  6 +++---
 drivers/rtc/rtc-pcf85363.c         |  2 +-
 include/linux/regmap.h             | 10 ++++++++--
 include/regmap.h                   | 29 +++++++++++++++++++++++++++++
 12 files changed, 76 insertions(+), 34 deletions(-)

diff --git a/arch/arm/mach-imx/iim.c b/arch/arm/mach-imx/iim.c
index 8dc70d3caa26..90ca644c2e62 100644
--- a/arch/arm/mach-imx/iim.c
+++ b/arch/arm/mach-imx/iim.c
@@ -19,7 +19,7 @@
 #include <malloc.h>
 #include <of.h>
 #include <io.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <regulator.h>
 #include <linux/err.h>
 
diff --git a/drivers/base/regmap/regmap-multi.c b/drivers/base/regmap/regmap-multi.c
index e3f5b9aec1e4..74f3648eb439 100644
--- a/drivers/base/regmap/regmap-multi.c
+++ b/drivers/base/regmap/regmap-multi.c
@@ -5,7 +5,7 @@
 
 #include <common.h>
 #include <fcntl.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <linux/bitfield.h>
 #include <linux/export.h>
 
@@ -46,7 +46,7 @@ static ssize_t regmap_multi_cdev_read(struct cdev *cdev, void *buf, size_t count
 		return -EINVAL;
 
 	count = ALIGN_DOWN(count, rwsize);
-	return regmap_bulk_read(map, offset, buf, count) ?: count;
+	return regmap_bulk_read(map, offset, buf, count / rwsize) ?: count;
 }
 
 static ssize_t regmap_multi_cdev_write(struct cdev *cdev, const void *buf, size_t count,
@@ -60,7 +60,7 @@ static ssize_t regmap_multi_cdev_write(struct cdev *cdev, const void *buf, size_
 		return -EINVAL;
 
 	count = ALIGN_DOWN(count, rwsize);
-	return regmap_bulk_write(map, offset, buf, count) ?: count;
+	return regmap_bulk_write(map, offset, buf, count / rwsize) ?: count;
 }
 
 static struct cdev_operations regmap_multi_fops = {
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 191217d20144..4d896c677b28 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -12,7 +12,7 @@
  */
 
 #include <common.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <malloc.h>
 #include <linux/log2.h>
 
@@ -248,21 +248,17 @@ int regmap_write_bits(struct regmap *map, unsigned int reg,
  * @map: Register map to read from
  * @reg: First register to be read from
  * @val: Pointer to store read value
- * @val_len: Size of data to read
+ * @val_count: Number of registers to read
  *
  * A value of zero will be returned on success, a negative errno will
  * be returned in error cases.
  */
 int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
-		     size_t val_len)
+		     size_t val_count)
 {
-	size_t val_bytes = map->format.val_bytes;
-	size_t val_count = val_len / val_bytes;
 	unsigned int v;
 	int ret, i;
 
-	if (val_len % val_bytes)
-		return -EINVAL;
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
 	if (val_count == 0)
@@ -312,20 +308,17 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
  * @reg: Initial register to write to
  * @val: Block of data to be written, laid out for direct transmission to the
  *       device
- * @val_len: Length of data pointed to by val.
+ * @val_len: Number of registers to write
  *
  * A value of zero will be returned on success, a negative errno will
  * be returned in error cases.
  */
 int regmap_bulk_write(struct regmap *map, unsigned int reg,
-		     const void *val, size_t val_len)
+		     const void *val, size_t val_count)
 {
 	size_t val_bytes = map->format.val_bytes;
-	size_t val_count = val_len / val_bytes;
 	int ret, i;
 
-	if (val_len % val_bytes)
-		return -EINVAL;
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
 	if (val_count == 0)
@@ -388,9 +381,11 @@ static ssize_t regmap_cdev_read(struct cdev *cdev, void *buf, size_t count, loff
 		       unsigned long flags)
 {
 	struct regmap *map = container_of(cdev, struct regmap, cdev);
+	size_t val_bytes = map->format.val_bytes;
 	int ret;
 
-	ret = regmap_bulk_read(map, offset, buf, count);
+	count = ALIGN_DOWN(count, val_bytes);
+	ret = regmap_bulk_read(map, offset, buf, count / val_bytes);
 	if (ret)
 		return ret;
 
@@ -401,9 +396,11 @@ static ssize_t regmap_cdev_write(struct cdev *cdev, const void *buf, size_t coun
 			unsigned long flags)
 {
 	struct regmap *map = container_of(cdev, struct regmap, cdev);
+	size_t val_bytes = map->format.val_bytes;
 	int ret;
 
-	ret = regmap_bulk_write(map, offset, buf, count);
+	count = ALIGN_DOWN(count, val_bytes);
+	ret = regmap_bulk_write(map, offset, buf, count / val_bytes);
 	if (ret)
 		return ret;
 
diff --git a/drivers/mfd/rn5t568.c b/drivers/mfd/rn5t568.c
index f649df944bf2..12de689734db 100644
--- a/drivers/mfd/rn5t568.c
+++ b/drivers/mfd/rn5t568.c
@@ -13,7 +13,7 @@
 #include <i2c/i2c.h>
 #include <init.h>
 #include <of.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <reset_source.h>
 #include <restart.h>
 
diff --git a/drivers/net/ksz_common.h b/drivers/net/ksz_common.h
index 995054d6e8c6..291488fe3485 100644
--- a/drivers/net/ksz_common.h
+++ b/drivers/net/ksz_common.h
@@ -3,7 +3,7 @@
 #define NET_KSZ_COMMON_H_
 
 #include <linux/swab.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <linux/bitops.h>
 #include <platform_data/ksz9477_reg.h>
 
diff --git a/drivers/nvmem/bsec.c b/drivers/nvmem/bsec.c
index c381ee0836e8..7f24063b9ed6 100644
--- a/drivers/nvmem/bsec.c
+++ b/drivers/nvmem/bsec.c
@@ -13,7 +13,7 @@
 #include <net.h>
 #include <io.h>
 #include <of.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <mach/stm32mp/bsec.h>
 #include <machine_id.h>
 #include <linux/nvmem-provider.h>
@@ -82,7 +82,7 @@ static void stm32_bsec_set_unique_machine_id(struct regmap *map)
 	int ret;
 
 	ret = regmap_bulk_read(map, BSEC_OTP_SERIAL * 4,
-			       unique_id, sizeof(unique_id));
+			       unique_id, sizeof(unique_id) / 4);
 	if (ret)
 		return;
 
diff --git a/drivers/nvmem/ocotp.c b/drivers/nvmem/ocotp.c
index c22e5d9585fa..641c825986cb 100644
--- a/drivers/nvmem/ocotp.c
+++ b/drivers/nvmem/ocotp.c
@@ -24,7 +24,7 @@
 #include <io.h>
 #include <of.h>
 #include <clock.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <linux/clk.h>
 #include <machine_id.h>
 #ifdef CONFIG_ARCH_IMX
@@ -622,7 +622,7 @@ static int imx_ocotp_read_mac(const struct imx_ocotp_data *data,
 	u8 buf[MAC_BYTES];
 	int ret;
 
-	ret = regmap_bulk_read(map, offset, buf, MAC_BYTES);
+	ret = regmap_bulk_read(map, offset, buf, MAC_BYTES / 4);
 
 	if (ret < 0)
 		return ret;
@@ -649,7 +649,7 @@ static int imx_ocotp_set_mac(struct param_d *param, void *priv)
 	struct ocotp_priv_ethaddr *ethaddr = priv;
 	int ret;
 
-	ret = regmap_bulk_read(ethaddr->map, ethaddr->offset, buf, MAC_BYTES);
+	ret = regmap_bulk_read(ethaddr->map, ethaddr->offset, buf, MAC_BYTES / 4);
 	if (ret < 0)
 		return ret;
 
@@ -662,7 +662,7 @@ static int imx_ocotp_set_mac(struct param_d *param, void *priv)
 					  OCOTP_MAC_TO_HW);
 
 	return regmap_bulk_write(ethaddr->map, ethaddr->offset,
-				 buf, MAC_BYTES);
+				 buf, MAC_BYTES / 4);
 }
 
 static struct regmap_bus imx_ocotp_regmap_bus = {
diff --git a/drivers/nvmem/regmap.c b/drivers/nvmem/regmap.c
index 313e92fb7da8..fa5405d7a86a 100644
--- a/drivers/nvmem/regmap.c
+++ b/drivers/nvmem/regmap.c
@@ -6,12 +6,22 @@
 #include <errno.h>
 #include <init.h>
 #include <io.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <linux/nvmem-provider.h>
 
 static int nvmem_regmap_write(void *ctx, unsigned offset, const void *val, size_t bytes)
 {
-	return regmap_bulk_write(ctx, offset, val, bytes);
+	struct regmap *map = ctx;
+
+	/*
+	 * eFuse writes going through this function may be irreversible,
+	 * so expect users to observe alignment.
+	 */
+	if (bytes % regmap_get_val_bytes(map))
+		return -EINVAL;
+
+	return regmap_bulk_write(map, offset, val,
+				 bytes / regmap_get_val_bytes(map));
 }
 
 static int nvmem_regmap_read(void *ctx, unsigned offset, void *buf, size_t bytes)
diff --git a/drivers/nvmem/snvs_lpgpr.c b/drivers/nvmem/snvs_lpgpr.c
index dfbd5397d2f2..9bbee6d587a4 100644
--- a/drivers/nvmem/snvs_lpgpr.c
+++ b/drivers/nvmem/snvs_lpgpr.c
@@ -10,7 +10,7 @@
 #include <of.h>
 #include <of_device.h>
 #include <malloc.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <mfd/syscon.h>
 #include <linux/nvmem-provider.h>
 
@@ -61,7 +61,7 @@ static int snvs_lpgpr_write(void *ctx, unsigned offset, const void *val, size_t
 		return -EPERM;
 
 	return regmap_bulk_write(priv->regmap, dcfg->offset + offset, val,
-				 bytes);
+				 bytes / 4);
 }
 
 static int snvs_lpgpr_read(void *ctx, unsigned offset, void *val, size_t bytes)
@@ -70,7 +70,7 @@ static int snvs_lpgpr_read(void *ctx, unsigned offset, void *val, size_t bytes)
 	const struct snvs_lpgpr_cfg *dcfg = priv->dcfg;
 
 	return regmap_bulk_read(priv->regmap, dcfg->offset + offset,
-				val, bytes);
+				val, bytes / 4);
 }
 
 static int snvs_lpgpr_probe(struct device *dev)
diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 5b2c4e62b035..bcc251e1380e 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -16,7 +16,7 @@
 #include <malloc.h>
 #include <errno.h>
 #include <i2c/i2c.h>
-#include <regmap.h>
+#include <linux/regmap.h>
 #include <rtc.h>
 #include <linux/rtc.h>
 #include <linux/bcd.h>
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index aacb80cfa235..690dc3b1dccb 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -199,10 +199,16 @@ int regmap_multi_register_cdev(struct regmap *map8,
 int regmap_write(struct regmap *map, unsigned int reg, unsigned int val);
 int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val);
 
+#ifndef regmap_bulk_read
+#define regmap_bulk_read regmap_bulk_read
 int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
-		    size_t val_len);
+		    size_t val_count);
+#endif
+#ifndef regmap_bulk_write
+#define regmap_bulk_write regmap_bulk_write
 int regmap_bulk_write(struct regmap *map, unsigned int reg,
-		     const void *val, size_t val_len);
+		     const void *val, size_t val_count);
+#endif
 
 int regmap_get_val_bytes(struct regmap *map);
 int regmap_get_max_register(struct regmap *map);
diff --git a/include/regmap.h b/include/regmap.h
index f4a194f10433..f2c395a89654 100644
--- a/include/regmap.h
+++ b/include/regmap.h
@@ -2,6 +2,35 @@
 #ifndef __REGMAP_H
 #define __REGMAP_H
 
+#include <linux/compiler.h>
+#include <linux/types.h>
+
+extern void __compiletime_error("Last argument is now number of registers, not bytes. Fix "
+				"it and include <linux/regmap.h> instead")
+__regmap_bulk_api_changed(void);
+
+struct regmap;
+
+#ifndef regmap_bulk_read
+#define regmap_bulk_read regmap_bulk_read
+static inline int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
+				   size_t val_bytes)
+{
+	__regmap_bulk_api_changed();
+	return -1;
+}
+#endif
+
+#ifndef regmap_bulk_write
+#define regmap_bulk_write regmap_bulk_write
+static inline int regmap_bulk_write(struct regmap *map, unsigned int reg,
+				    const void *val, size_t val_bytes)
+{
+	__regmap_bulk_api_changed();
+	return -1;
+}
+#endif
+
 #include <linux/regmap.h>
 
 #endif /* __REGMAP_H */
-- 
2.39.2




  parent reply	other threads:[~2023-10-20  7:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-20  7:18 [PATCH 00/11] " Ahmad Fatoum
2023-10-20  7:18 ` [PATCH 01/11] regmap: add support for bulk 64-bit reads and writes Ahmad Fatoum
2023-10-20  7:18 ` [PATCH 02/11] nvmem: regmap: do not use regmap_bulk_read Ahmad Fatoum
2023-10-20  7:18 ` [PATCH 03/11] regmap: move regmap.h content to linux/regmap.h Ahmad Fatoum
2023-10-20  7:18 ` Ahmad Fatoum [this message]
2023-10-20  7:18 ` [PATCH 05/11] mfd: syscon: do not include regmap.h from mfd/syscon.h Ahmad Fatoum
2023-10-20  7:18 ` [PATCH 06/11] mfd: axp20x: remove dependency on regmap.h Ahmad Fatoum
2023-10-20  7:18 ` [PATCH 07/11] mfd: pfuze: disable mfd/pfuze.h " Ahmad Fatoum
2023-10-20  7:18 ` [PATCH 08/11] mfd: atmel-smc: remove dependency of mfd/syscon/atmel-smc.h " Ahmad Fatoum
2023-10-20  7:18 ` [PATCH 09/11] mfd: atmel-smc: include needed headers directly Ahmad Fatoum
2023-10-20  7:18 ` [PATCH 10/11] mfd: stm32-timers: remove dependency of mfd/stm32-timers.h on regmap.h Ahmad Fatoum
2023-10-20  7:18 ` [PATCH 11/11] treewide: switch regmap.h include to linux/regmap.h Ahmad Fatoum
2023-10-23  9:45 ` [PATCH 00/11] regmap: align regmap_bulk_read/write API with Linux Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231020071853.2826528-5-a.fatoum@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox