Re: [PATCH 08/12] regmap: indirect: Add indirect regmap support
From: matthew . gerlach
Date: Wed Nov 16 2022 - 12:00:36 EST
On Mon, 14 Nov 2022, Marco Pagani wrote:
On 2022-11-08 15:43, Ilpo Järvinen wrote:
Add support for indirect register access via a regmap interface.
Indirect register access is a generic way to access registers directly.
One use case is accessing registers on Intel FPGA IPs with e.g. PMCI or
HSSI.
Co-developed-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
---
drivers/base/regmap/Kconfig | 3 +
drivers/base/regmap/Makefile | 1 +
drivers/base/regmap/regmap-indirect.c | 128 ++++++++++++++++++++++++++
include/linux/regmap.h | 55 +++++++++++
4 files changed, 187 insertions(+)
create mode 100644 drivers/base/regmap/regmap-indirect.c
diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 159bac6c5046..94e5ca5434cf 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -65,3 +65,6 @@ config REGMAP_I3C
config REGMAP_SPI_AVMM
tristate
depends on SPI
+
+config REGMAP_INDIRECT
+ tristate
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 11facb32a027..6221a4740806 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o
obj-$(CONFIG_REGMAP_SPI_AVMM) += regmap-spi-avmm.o
obj-$(CONFIG_REGMAP_MDIO) += regmap-mdio.o
+obj-$(CONFIG_REGMAP_INDIRECT) += regmap-indirect.o
diff --git a/drivers/base/regmap/regmap-indirect.c b/drivers/base/regmap/regmap-indirect.c
new file mode 100644
index 000000000000..3ceb0c044c7c
--- /dev/null
+++ b/drivers/base/regmap/regmap-indirect.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Indirect Register Access.
+ *
+ * Copyright (C) 2020-2022 Intel Corporation, Inc.
+ */
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+
+struct indirect_ctx {
+ void __iomem *base;
+ struct device *dev;
+ const struct regmap_indirect_cfg *indirect_cfg;
+};
+
+static int indirect_bus_idle_cmd(struct indirect_ctx *ctx)
+{
+ unsigned int cmd;
+ int ret;
+
+ writel(ctx->indirect_cfg->idle_cmd, ctx->base + ctx->indirect_cfg->cmd_offset);
+
+ ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->cmd_offset, cmd,
+ cmd == ctx->indirect_cfg->idle_cmd,
+ ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->timeout_us);
+ if (ret)
+ dev_err(ctx->dev, "timed out waiting idle cmd (residual cmd=0x%x)\n", cmd);
+
+ return ret;
+}
+
+static int indirect_bus_reg_read(void *context, unsigned int reg,
+ unsigned int *val)
+{
+ struct indirect_ctx *ctx = context;
+ unsigned int cmd, ack, tmpval;
+ int ret;
+
+ cmd = readl(ctx->base + ctx->indirect_cfg->cmd_offset);
+ if (cmd != ctx->indirect_cfg->idle_cmd)
+ dev_warn(ctx->dev, "residual cmd 0x%x on read entry\n", cmd);
+
+ writel(reg, ctx->base + ctx->indirect_cfg->addr_offset);
+ writel(ctx->indirect_cfg->read_cmd, ctx->base + ctx->indirect_cfg->cmd_offset);
+
+ ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->ack_offset, ack,
+ (ack & ctx->indirect_cfg->ack_mask) == ctx->indirect_cfg->ack_mask,
+ ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->timeout_us);
+ if (ret) {
+ dev_err(ctx->dev, "read timed out on reg 0x%x ack 0x%x\n", reg, ack);
+ goto out;
This goto needs to be removed. The code needs to fall throuch and call
indirect_bus_idle_cmd().
+ }
+
+ tmpval = readl(ctx->base + ctx->indirect_cfg->read_offset);
+
+ if (indirect_bus_idle_cmd(ctx)) {
+ if (!ret)
Could you please explain why it is necessary to insert an idle_cmd after each
bus read or write? Do the bus read and write methods assume there will be an
idle_cmd in the cmd register at initialization? Is this assumption safe if the
device detaches and reattaches? Isn't the "if (!ret)" check redundant since
ret is always == 0?
The idle_cmd ensures the RD or WR bits and the ACK bits are cleared before
the next transaction. The if (!ret) doesn't override the error returned by
readl_poll_timeout.
+ ret = -ETIMEDOUT;
+ goto out;
+ }
+
+ *val = tmpval;
+out:
+ return ret;
+}
+
+static int indirect_bus_reg_write(void *context, unsigned int reg,
+ unsigned int val)
+{
+ struct indirect_ctx *ctx = context;
+ unsigned int cmd, ack;
+ int ret;
+
+ cmd = readl(ctx->base + ctx->indirect_cfg->cmd_offset);
+ if (cmd != ctx->indirect_cfg->idle_cmd)
+ dev_warn(ctx->dev, "residual cmd 0x%x on write entry\n", cmd);
+
+ writel(val, ctx->base + ctx->indirect_cfg->write_offset);
+ writel(reg, ctx->base + ctx->indirect_cfg->addr_offset);
+ writel(ctx->indirect_cfg->write_cmd, ctx->base + ctx->indirect_cfg->cmd_offset);
+
+ ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->ack_offset, ack,
+ (ack & ctx->indirect_cfg->ack_mask) == ctx->indirect_cfg->ack_mask,
+ ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->timeout_us);
+ if (ret)
+ dev_err(ctx->dev, "write timed out on reg 0x%x ack 0x%x\n", reg, ack);
+
+ if (indirect_bus_idle_cmd(ctx)) {
+ if (!ret)
+ ret = -ETIMEDOUT;
+ }
+
+ return ret;
+}
+
+static const struct regmap_bus indirect_bus = {
+ .reg_write = indirect_bus_reg_write,
+ .reg_read = indirect_bus_reg_read,
+};
+
+struct regmap *__devm_regmap_init_indirect(struct device *dev,
+ void __iomem *base,
+ struct regmap_config *cfg,
+ struct lock_class_key *lock_key,
+ const char *lock_name)
+{
+ struct indirect_ctx *ctx;
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return NULL;
+
+ ctx->base = base;
+ ctx->dev = dev;
+ ctx->indirect_cfg = cfg->indirect_cfg;
+
+ return __devm_regmap_init(dev, &indirect_bus, ctx, cfg, lock_key, lock_name);
+}
+EXPORT_SYMBOL_GPL(__devm_regmap_init_indirect);
+
+MODULE_DESCRIPTION("Indirect Register Access");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index ca3434dca3a0..adaa7bca4f60 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -190,6 +190,41 @@ enum regmap_endian {
REGMAP_ENDIAN_NATIVE,
};
+/**
+ * struct regmap_indirect_cfg - A configuration for indirect register access
+ *
+ * @cmd_offset: Command register offset
+ * @idle_cmd: Idle command
+ * @read_cmd: Read command
+ * @write_cmd: Write command
+ *
+ * @ack_offset: Command acknowledgment register offset
+ * @ack_mask: Command acknowledgment bit mask
+ *
+ * @addr_offset: Address register offset
+ * @read_offset: Read register offset
+ * @write_offset: Write register offset
+ *
+ * @sleep_us: Command wait sleep (usecs)
+ * @timeout_us: Command timeout (usecs)
+ */
+struct regmap_indirect_cfg {
+ unsigned int cmd_offset;
+ u32 idle_cmd;
+ u32 read_cmd;
+ u32 write_cmd;
+
+ unsigned int ack_offset;
+ u32 ack_mask;
+
+ unsigned int addr_offset;
+ unsigned int read_offset;
+ unsigned int write_offset;
+
+ unsigned long sleep_us;
+ unsigned long timeout_us;
+};
+
/**
* struct regmap_range - A register range, used for access related checks
* (readable/writeable/volatile/precious checks)
@@ -431,6 +466,8 @@ struct regmap_config {
const struct regmap_range_cfg *ranges;
unsigned int num_ranges;
+ const struct regmap_indirect_cfg *indirect_cfg;
+
bool use_hwlock;
bool use_raw_spinlock;
unsigned int hwlock_id;
@@ -693,6 +730,12 @@ struct regmap *__devm_regmap_init_spi_avmm(struct spi_device *spi,
const struct regmap_config *config,
struct lock_class_key *lock_key,
const char *lock_name);
+struct regmap *__devm_regmap_init_indirect(struct device *dev,
+ void __iomem *base,
+ struct regmap_config *cfg,
+ struct lock_class_key *lock_key,
+ const char *lock_name);
+
/*
* Wrapper for regmap_init macros to include a unique lockdep key and name
* for each call. No-op if CONFIG_LOCKDEP is not set.
@@ -1148,6 +1191,18 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
__regmap_lockdep_wrapper(__devm_regmap_init_spi_avmm, #config, \
spi, config)
+/**
+ * devm_regmap_init_indirect - create a regmap for indirect register access
+ * @dev: device creating the regmap
+ * @base: __iomem point to base of memory with mailbox
+ * @cfg: regmap_config describing interface
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+#define devm_regmap_init_indirect(dev, base, config) \
+ __regmap_lockdep_wrapper(__devm_regmap_init_indirect, #config, \
+ dev, base, config)
+
int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk);
void regmap_mmio_detach_clk(struct regmap *map);
void regmap_exit(struct regmap *map);
Thanks,
Marco