[PATCH v2 0/4] regmap: mmio: Extending to support IO ports

[PATCH v2 0/4] regmap: mmio: Extending to support IO ports

[Andy Shevchenko]

Currently regmap MMIO doesn't support IO ports, while being inconsistent
in used IO accessors. Fix the latter and extend framework with the
former.

Changelog v2:
- dropped the first two patches (Mark)
- split the last patch to two (Mark)

Andy Shevchenko (4):
  regmap: mmio: Remove mmio_relaxed member from context
  regmap: mmio: Get rid of broken 64-bit IO
  regmap: mmio: Introduce IO accessors that can talk to IO port
  regmap: mmio: Fix MMIO accessors to avoid talking to IO port

 drivers/base/regmap/regmap-mmio.c | 140 +++++++++++++++++++-----------
 include/linux/regmap.h            |   3 +
 2 files changed, 91 insertions(+), 52 deletions(-)

-- 
2.35.1

[PATCH v2 1/4] regmap: mmio: Remove mmio_relaxed member from context

[Andy Shevchenko]

There is no need to keep mmio_relaxed member in the context, it's
onetime used during generation of the context. Remove it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/regmap/regmap-mmio.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index 71f16be7e717..3a5c81e4ce84 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -16,7 +16,6 @@
 struct regmap_mmio_context {
 	void __iomem *regs;
 	unsigned int val_bytes;
-	bool relaxed_mmio;
 
 	bool attached_clk;
 	struct clk *clk;
@@ -290,7 +289,6 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 
 	ctx->regs = regs;
 	ctx->val_bytes = config->val_bits / 8;
-	ctx->relaxed_mmio = config->use_relaxed_mmio;
 	ctx->clk = ERR_PTR(-ENODEV);
 
 	switch (regmap_get_val_endian(dev, &regmap_mmio, config)) {
@@ -301,7 +299,7 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 #endif
 		switch (config->val_bits) {
 		case 8:
-			if (ctx->relaxed_mmio) {
+			if (config->use_relaxed_mmio) {
 				ctx->reg_read = regmap_mmio_read8_relaxed;
 				ctx->reg_write = regmap_mmio_write8_relaxed;
 			} else {
@@ -310,7 +308,7 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 			}
 			break;
 		case 16:
-			if (ctx->relaxed_mmio) {
+			if (config->use_relaxed_mmio) {
 				ctx->reg_read = regmap_mmio_read16le_relaxed;
 				ctx->reg_write = regmap_mmio_write16le_relaxed;
 			} else {
@@ -319,7 +317,7 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 			}
 			break;
 		case 32:
-			if (ctx->relaxed_mmio) {
+			if (config->use_relaxed_mmio) {
 				ctx->reg_read = regmap_mmio_read32le_relaxed;
 				ctx->reg_write = regmap_mmio_write32le_relaxed;
 			} else {
@@ -329,7 +327,7 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 			break;
 #ifdef CONFIG_64BIT
 		case 64:
-			if (ctx->relaxed_mmio) {
+			if (config->use_relaxed_mmio) {
 				ctx->reg_read = regmap_mmio_read64le_relaxed;
 				ctx->reg_write = regmap_mmio_write64le_relaxed;
 			} else {
-- 
2.35.1

[PATCH v2 2/4] regmap: mmio: Get rid of broken 64-bit IO

[Andy Shevchenko]

The current implementation, besides having no active users, is broken
by design of regmap. For 64-bit IO we need to supply 64-bit value,
otherwise there is no way to handle upper 32 bits in 64-bit register.

Hence, remove the broken IO accessors for good and wait for real user
that can fix entire regmap API for that.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/regmap/regmap-mmio.c | 49 -------------------------------
 1 file changed, 49 deletions(-)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index 3a5c81e4ce84..b1bd93ea405e 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -32,9 +32,6 @@ static int regmap_mmio_regbits_check(size_t reg_bits)
 	case 8:
 	case 16:
 	case 32:
-#ifdef CONFIG_64BIT
-	case 64:
-#endif
 		return 0;
 	default:
 		return -EINVAL;
@@ -56,11 +53,6 @@ static int regmap_mmio_get_min_stride(size_t val_bits)
 	case 32:
 		min_stride = 4;
 		break;
-#ifdef CONFIG_64BIT
-	case 64:
-		min_stride = 8;
-		break;
-#endif
 	default:
 		return -EINVAL;
 	}
@@ -124,22 +116,6 @@ static void regmap_mmio_write32be(struct regmap_mmio_context *ctx,
 	iowrite32be(val, ctx->regs + reg);
 }
 
-#ifdef CONFIG_64BIT
-static void regmap_mmio_write64le(struct regmap_mmio_context *ctx,
-				  unsigned int reg,
-				  unsigned int val)
-{
-	writeq(val, ctx->regs + reg);
-}
-
-static void regmap_mmio_write64le_relaxed(struct regmap_mmio_context *ctx,
-				  unsigned int reg,
-				  unsigned int val)
-{
-	writeq_relaxed(val, ctx->regs + reg);
-}
-#endif
-
 static int regmap_mmio_write(void *context, unsigned int reg, unsigned int val)
 {
 	struct regmap_mmio_context *ctx = context;
@@ -207,20 +183,6 @@ static unsigned int regmap_mmio_read32be(struct regmap_mmio_context *ctx,
 	return ioread32be(ctx->regs + reg);
 }
 
-#ifdef CONFIG_64BIT
-static unsigned int regmap_mmio_read64le(struct regmap_mmio_context *ctx,
-				         unsigned int reg)
-{
-	return readq(ctx->regs + reg);
-}
-
-static unsigned int regmap_mmio_read64le_relaxed(struct regmap_mmio_context *ctx,
-						 unsigned int reg)
-{
-	return readq_relaxed(ctx->regs + reg);
-}
-#endif
-
 static int regmap_mmio_read(void *context, unsigned int reg, unsigned int *val)
 {
 	struct regmap_mmio_context *ctx = context;
@@ -325,17 +287,6 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 				ctx->reg_write = regmap_mmio_write32le;
 			}
 			break;
-#ifdef CONFIG_64BIT
-		case 64:
-			if (config->use_relaxed_mmio) {
-				ctx->reg_read = regmap_mmio_read64le_relaxed;
-				ctx->reg_write = regmap_mmio_write64le_relaxed;
-			} else {
-				ctx->reg_read = regmap_mmio_read64le;
-				ctx->reg_write = regmap_mmio_write64le;
-			}
-			break;
-#endif
 		default:
 			ret = -EINVAL;
 			goto err_free;
-- 
2.35.1

[PATCH v2 3/4] regmap: mmio: Introduce IO accessors that can talk to IO port

[Andy Shevchenko]

Some users may use regmap MMIO for IO ports, and this can be done
by assigning ioreadXX()/iowriteXX() and their Big Endian counterparts
to the regmap context.

Add IO port support with a corresponding flag added.

While doing that, make sure that user won't select relaxed MMIO access
along with IO port because the latter have no relaxed variants.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/regmap/regmap-mmio.c | 105 +++++++++++++++++++++++++++---
 include/linux/regmap.h            |   3 +
 2 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index b1bd93ea405e..37f79e912d01 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -74,6 +74,12 @@ static void regmap_mmio_write8_relaxed(struct regmap_mmio_context *ctx,
 	writeb_relaxed(val, ctx->regs + reg);
 }
 
+static void regmap_mmio_iowrite8(struct regmap_mmio_context *ctx,
+				 unsigned int reg, unsigned int val)
+{
+	iowrite8(val, ctx->regs + reg);
+}
+
 static void regmap_mmio_write16le(struct regmap_mmio_context *ctx,
 				  unsigned int reg,
 				  unsigned int val)
@@ -88,6 +94,12 @@ static void regmap_mmio_write16le_relaxed(struct regmap_mmio_context *ctx,
 	writew_relaxed(val, ctx->regs + reg);
 }
 
+static void regmap_mmio_iowrite16le(struct regmap_mmio_context *ctx,
+				    unsigned int reg, unsigned int val)
+{
+	iowrite16(val, ctx->regs + reg);
+}
+
 static void regmap_mmio_write16be(struct regmap_mmio_context *ctx,
 				  unsigned int reg,
 				  unsigned int val)
@@ -95,6 +107,12 @@ static void regmap_mmio_write16be(struct regmap_mmio_context *ctx,
 	iowrite16be(val, ctx->regs + reg);
 }
 
+static void regmap_mmio_iowrite16be(struct regmap_mmio_context *ctx,
+				    unsigned int reg, unsigned int val)
+{
+	iowrite16be(val, ctx->regs + reg);
+}
+
 static void regmap_mmio_write32le(struct regmap_mmio_context *ctx,
 				  unsigned int reg,
 				  unsigned int val)
@@ -109,6 +127,12 @@ static void regmap_mmio_write32le_relaxed(struct regmap_mmio_context *ctx,
 	writel_relaxed(val, ctx->regs + reg);
 }
 
+static void regmap_mmio_iowrite32le(struct regmap_mmio_context *ctx,
+				    unsigned int reg, unsigned int val)
+{
+	iowrite32(val, ctx->regs + reg);
+}
+
 static void regmap_mmio_write32be(struct regmap_mmio_context *ctx,
 				  unsigned int reg,
 				  unsigned int val)
@@ -116,6 +140,12 @@ static void regmap_mmio_write32be(struct regmap_mmio_context *ctx,
 	iowrite32be(val, ctx->regs + reg);
 }
 
+static void regmap_mmio_iowrite32be(struct regmap_mmio_context *ctx,
+				    unsigned int reg, unsigned int val)
+{
+	iowrite32be(val, ctx->regs + reg);
+}
+
 static int regmap_mmio_write(void *context, unsigned int reg, unsigned int val)
 {
 	struct regmap_mmio_context *ctx = context;
@@ -147,6 +177,12 @@ static unsigned int regmap_mmio_read8_relaxed(struct regmap_mmio_context *ctx,
 	return readb_relaxed(ctx->regs + reg);
 }
 
+static unsigned int regmap_mmio_ioread8(struct regmap_mmio_context *ctx,
+					unsigned int reg)
+{
+	return ioread8(ctx->regs + reg);
+}
+
 static unsigned int regmap_mmio_read16le(struct regmap_mmio_context *ctx,
 				         unsigned int reg)
 {
@@ -159,12 +195,24 @@ static unsigned int regmap_mmio_read16le_relaxed(struct regmap_mmio_context *ctx
 	return readw_relaxed(ctx->regs + reg);
 }
 
+static unsigned int regmap_mmio_ioread16le(struct regmap_mmio_context *ctx,
+					   unsigned int reg)
+{
+	return ioread16(ctx->regs + reg);
+}
+
 static unsigned int regmap_mmio_read16be(struct regmap_mmio_context *ctx,
 				         unsigned int reg)
 {
 	return ioread16be(ctx->regs + reg);
 }
 
+static unsigned int regmap_mmio_ioread16be(struct regmap_mmio_context *ctx,
+					   unsigned int reg)
+{
+	return ioread16be(ctx->regs + reg);
+}
+
 static unsigned int regmap_mmio_read32le(struct regmap_mmio_context *ctx,
 				         unsigned int reg)
 {
@@ -177,12 +225,24 @@ static unsigned int regmap_mmio_read32le_relaxed(struct regmap_mmio_context *ctx
 	return readl_relaxed(ctx->regs + reg);
 }
 
+static unsigned int regmap_mmio_ioread32le(struct regmap_mmio_context *ctx,
+					   unsigned int reg)
+{
+	return ioread32(ctx->regs + reg);
+}
+
 static unsigned int regmap_mmio_read32be(struct regmap_mmio_context *ctx,
 				         unsigned int reg)
 {
 	return ioread32be(ctx->regs + reg);
 }
 
+static unsigned int regmap_mmio_ioread32be(struct regmap_mmio_context *ctx,
+					   unsigned int reg)
+{
+	return ioread32be(ctx->regs + reg);
+}
+
 static int regmap_mmio_read(void *context, unsigned int reg, unsigned int *val)
 {
 	struct regmap_mmio_context *ctx = context;
@@ -245,6 +305,9 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 	if (config->reg_stride < min_stride)
 		return ERR_PTR(-EINVAL);
 
+	if (config->use_relaxed_mmio && config->io_port)
+		return ERR_PTR(-EINVAL);
+
 	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
 		return ERR_PTR(-ENOMEM);
@@ -261,7 +324,10 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 #endif
 		switch (config->val_bits) {
 		case 8:
-			if (config->use_relaxed_mmio) {
+			if (config->io_port) {
+				ctx->reg_read = regmap_mmio_ioread8;
+				ctx->reg_write = regmap_mmio_iowrite8;
+			} else if (config->use_relaxed_mmio) {
 				ctx->reg_read = regmap_mmio_read8_relaxed;
 				ctx->reg_write = regmap_mmio_write8_relaxed;
 			} else {
@@ -270,7 +336,10 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 			}
 			break;
 		case 16:
-			if (config->use_relaxed_mmio) {
+			if (config->io_port) {
+				ctx->reg_read = regmap_mmio_ioread16le;
+				ctx->reg_write = regmap_mmio_iowrite16le;
+			} else if (config->use_relaxed_mmio) {
 				ctx->reg_read = regmap_mmio_read16le_relaxed;
 				ctx->reg_write = regmap_mmio_write16le_relaxed;
 			} else {
@@ -279,7 +348,10 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 			}
 			break;
 		case 32:
-			if (config->use_relaxed_mmio) {
+			if (config->io_port) {
+				ctx->reg_read = regmap_mmio_ioread32le;
+				ctx->reg_write = regmap_mmio_iowrite32le;
+			} else if (config->use_relaxed_mmio) {
 				ctx->reg_read = regmap_mmio_read32le_relaxed;
 				ctx->reg_write = regmap_mmio_write32le_relaxed;
 			} else {
@@ -298,16 +370,31 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 #endif
 		switch (config->val_bits) {
 		case 8:
-			ctx->reg_read = regmap_mmio_read8;
-			ctx->reg_write = regmap_mmio_write8;
+			if (config->io_port) {
+				ctx->reg_read = regmap_mmio_ioread8;
+				ctx->reg_write = regmap_mmio_iowrite8;
+			} else {
+				ctx->reg_read = regmap_mmio_read8;
+				ctx->reg_write = regmap_mmio_write8;
+			}
 			break;
 		case 16:
-			ctx->reg_read = regmap_mmio_read16be;
-			ctx->reg_write = regmap_mmio_write16be;
+			if (config->io_port) {
+				ctx->reg_read = regmap_mmio_ioread16be;
+				ctx->reg_write = regmap_mmio_iowrite16be;
+			} else {
+				ctx->reg_read = regmap_mmio_read16be;
+				ctx->reg_write = regmap_mmio_write16be;
+			}
 			break;
 		case 32:
-			ctx->reg_read = regmap_mmio_read32be;
-			ctx->reg_write = regmap_mmio_write32be;
+			if (config->io_port) {
+				ctx->reg_read = regmap_mmio_ioread32be;
+				ctx->reg_write = regmap_mmio_iowrite32be;
+			} else {
+				ctx->reg_read = regmap_mmio_read32be;
+				ctx->reg_write = regmap_mmio_write32be;
+			}
 			break;
 		default:
 			ret = -EINVAL;
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 7cf2157134ac..8cccc247cd37 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -311,6 +311,8 @@ typedef void (*regmap_unlock)(void *);
  *		  This field is a duplicate of a similar file in
  *		  'struct regmap_bus' and serves exact same purpose.
  *		   Use it only for "no-bus" cases.
+ * @io_port:	  Support IO port accessors. Makes sense only when MMIO vs. IO port
+ *		  access can be distinguished.
  * @max_register: Optional, specifies the maximum valid register address.
  * @wr_table:     Optional, points to a struct regmap_access_table specifying
  *                valid ranges for write access.
@@ -399,6 +401,7 @@ struct regmap_config {
 	size_t max_raw_write;
 
 	bool fast_io;
+	bool io_port;
 
 	unsigned int max_register;
 	const struct regmap_access_table *wr_table;
-- 
2.35.1

[PATCH v2 4/4] regmap: mmio: Fix MMIO accessors to avoid talking to IO port

[Andy Shevchenko]

Currently regmap MMIO is inconsistent with IO accessors. I.e.
the Big Endian counterparts are using ioreadXXbe() / iowriteXXbe()
which are not clean implementations of readXXbe().

That said, reimplement current Big Endian MMIO accessors by replacing
ioread()/iowrite() with respective read()/write() and swab() calls.

Note, there are no current in-kernel users that may utilize the
functionality of the IO ports on Big Endian hardware. All drivers
that use regmap MMIO either Little Endian, or they don't map IO
ports in a way that ioreadXX()/iowriteXX() may be utilized.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/regmap/regmap-mmio.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index 37f79e912d01..eed488aad1b0 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -104,7 +104,7 @@ static void regmap_mmio_write16be(struct regmap_mmio_context *ctx,
 				  unsigned int reg,
 				  unsigned int val)
 {
-	iowrite16be(val, ctx->regs + reg);
+	writew(swab16(val), ctx->regs + reg);
 }
 
 static void regmap_mmio_iowrite16be(struct regmap_mmio_context *ctx,
@@ -137,7 +137,7 @@ static void regmap_mmio_write32be(struct regmap_mmio_context *ctx,
 				  unsigned int reg,
 				  unsigned int val)
 {
-	iowrite32be(val, ctx->regs + reg);
+	writel(swab32(val), ctx->regs + reg);
 }
 
 static void regmap_mmio_iowrite32be(struct regmap_mmio_context *ctx,
@@ -204,7 +204,7 @@ static unsigned int regmap_mmio_ioread16le(struct regmap_mmio_context *ctx,
 static unsigned int regmap_mmio_read16be(struct regmap_mmio_context *ctx,
 				         unsigned int reg)
 {
-	return ioread16be(ctx->regs + reg);
+	return swab16(readw(ctx->regs + reg));
 }
 
 static unsigned int regmap_mmio_ioread16be(struct regmap_mmio_context *ctx,
@@ -234,7 +234,7 @@ static unsigned int regmap_mmio_ioread32le(struct regmap_mmio_context *ctx,
 static unsigned int regmap_mmio_read32be(struct regmap_mmio_context *ctx,
 				         unsigned int reg)
 {
-	return ioread32be(ctx->regs + reg);
+	return swab32(readl(ctx->regs + reg));
 }
 
 static unsigned int regmap_mmio_ioread32be(struct regmap_mmio_context *ctx,
-- 
2.35.1

Re: [PATCH v2 0/4] regmap: mmio: Extending to support IO ports

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]

On Mon, Aug 08, 2022 at 11:33:56PM +0300, Andy Shevchenko wrote:
> Currently regmap MMIO doesn't support IO ports, while being inconsistent
> in used IO accessors. Fix the latter and extend framework with the
> former.
> 
> Changelog v2:
> - dropped the first two patches (Mark)
> - split the last patch to two (Mark)
> 
> Andy Shevchenko (4):
>   regmap: mmio: Remove mmio_relaxed member from context
>   regmap: mmio: Get rid of broken 64-bit IO
>   regmap: mmio: Introduce IO accessors that can talk to IO port
>   regmap: mmio: Fix MMIO accessors to avoid talking to IO port
> 
>  drivers/base/regmap/regmap-mmio.c | 140 +++++++++++++++++++-----------
>  include/linux/regmap.h            |   3 +
>  2 files changed, 91 insertions(+), 52 deletions(-)
> 
> -- 
> 2.35.1

Thank you for adding this support. These changes should allow me to
update the PC104 drivers to make use of regmap MMIO for device
communication. Here's my ack for this patch series:

Acked-by: William Breathitt Gray <william.gray@linaro.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/4] regmap: mmio: Extending to support IO ports

[Mark Brown]

On Mon, 8 Aug 2022 23:33:56 +0300, Andy Shevchenko wrote:
> Currently regmap MMIO doesn't support IO ports, while being inconsistent
> in used IO accessors. Fix the latter and extend framework with the
> former.
> 
> Changelog v2:
> - dropped the first two patches (Mark)
> - split the last patch to two (Mark)
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next

Thanks!

[1/4] regmap: mmio: Remove mmio_relaxed member from context
      commit: ada79bca380009a85d1e643e5a4da0c079f28225
[2/4] regmap: mmio: Get rid of broken 64-bit IO
      commit: 159dfabd207628c983e0c3c5ef607f496ff5e6a5
[3/4] regmap: mmio: Introduce IO accessors that can talk to IO port
      commit: 93ce557679e1cf7742ad327d40a1499e7d8535b7
[4/4] regmap: mmio: Fix MMIO accessors to avoid talking to IO port
      commit: 7e7ba58c94127efa97c249e38cc2d1c0ed78b58f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark