i2c: designware: Convert driver to using regmap API

Seeing the DW I2C driver is using flags-based accessors with two
conditional clauses it would be better to replace them with the regmap
API IO methods and to initialize the regmap object with read/write
callbacks specific to the controller registers map implementation. This
will be also handy for the drivers with non-standard registers mapping
(like an embedded into the Baikal-T1 System Controller DW I2C block, which
glue-driver is a part of this series).

As before the driver tries to detect the mapping setup at probe stage and
creates a regmap object accordingly, which will be used by the rest of the
code to correctly access the controller registers. In two places it was
appropriate to convert the hand-written read-modify-write and
read-poll-loop design patterns to the corresponding regmap API
ready-to-use methods.

Note the regmap IO methods return value is checked only at the probe
stage. The rest of the code won't do this because basically we have
MMIO-based regmap so non of the read/write methods can fail (this also
won't be needed for the Baikal-T1-specific I2C controller).

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
[wsa: fix type of 'rx_valid' and remove outdated kdoc var description]
Signed-off-by: Wolfram Sang <wsa@kernel.org>

Author: fancer

drivers/i2c/busses/Kconfig

@@ -526,6 +526,7 @@ config I2C_DAVINCI
 
 config I2C_DESIGNWARE_CORE
 	tristate
+	select REGMAP
 
 config I2C_DESIGNWARE_SLAVE
 	bool "Synopsys DesignWare Slave"

drivers/i2c/busses/i2c-designware-common.c

@@ -21,6 +21,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/swab.h>
 #include <linux/types.h>
 
@@ -57,66 +58,122 @@ static char *abort_sources[] = {
 		"incorrect slave-transmitter mode configuration",
 };
 
-u32 dw_readl(struct dw_i2c_dev *dev, int offset)
+static int dw_reg_read(void *context, unsigned int reg, unsigned int *val)
 {
-	u32 value;
+	struct dw_i2c_dev *dev = context;
 
-	if (dev->flags & ACCESS_16BIT)
-		value = readw_relaxed(dev->base + offset) |
-			(readw_relaxed(dev->base + offset + 2) << 16);
-	else
-		value = readl_relaxed(dev->base + offset);
+	*val = readl_relaxed(dev->base + reg);
 
-	if (dev->flags & ACCESS_SWAP)
-		return swab32(value);
-	else
-		return value;
+	return 0;
 }
 
-void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
+static int dw_reg_write(void *context, unsigned int reg, unsigned int val)
 {
-	if (dev->flags & ACCESS_SWAP)
-		b = swab32(b);
-
-	if (dev->flags & ACCESS_16BIT) {
-		writew_relaxed((u16)b, dev->base + offset);
-		writew_relaxed((u16)(b >> 16), dev->base + offset + 2);
-	} else {
-		writel_relaxed(b, dev->base + offset);
-	}
+	struct dw_i2c_dev *dev = context;
+
+	writel_relaxed(val, dev->base + reg);
+
+	return 0;
+}
+
+static int dw_reg_read_swab(void *context, unsigned int reg, unsigned int *val)
+{
+	struct dw_i2c_dev *dev = context;
+
+	*val = swab32(readl_relaxed(dev->base + reg));
+
+	return 0;
+}
+
+static int dw_reg_write_swab(void *context, unsigned int reg, unsigned int val)
+{
+	struct dw_i2c_dev *dev = context;
+
+	writel_relaxed(swab32(val), dev->base + reg);
+
+	return 0;
+}
+
+static int dw_reg_read_word(void *context, unsigned int reg, unsigned int *val)
+{
+	struct dw_i2c_dev *dev = context;
+
+	*val = readw_relaxed(dev->base + reg) |
+		(readw_relaxed(dev->base + reg + 2) << 16);
+
+	return 0;
+}
+
+static int dw_reg_write_word(void *context, unsigned int reg, unsigned int val)
+{
+	struct dw_i2c_dev *dev = context;
+
+	writew_relaxed(val, dev->base + reg);
+	writew_relaxed(val >> 16, dev->base + reg + 2);
+
+	return 0;
 }
 
 /**
- * i2c_dw_set_reg_access() - Set register access flags
+ * i2c_dw_init_regmap() - Initialize registers map
  * @dev: device private data
  *
- * Autodetects needed register access mode and sets access flags accordingly.
- * This must be called before doing any other register access.
+ * Autodetects needed register access mode and creates the regmap with
+ * corresponding read/write callbacks. This must be called before doing any
+ * other register access.
  */
-int i2c_dw_set_reg_access(struct dw_i2c_dev *dev)
+int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
 {
+	struct regmap_config map_cfg = {
+		.reg_bits = 32,
+		.val_bits = 32,
+		.reg_stride = 4,
+		.disable_locking = true,
+		.reg_read = dw_reg_read,
+		.reg_write = dw_reg_write,
+		.max_register = DW_IC_COMP_TYPE,
+	};
 	u32 reg;
 	int ret;
 
+	/*
+	 * Skip detecting the registers map configuration if the regmap has
+	 * already been provided by a higher code.
+	 */
+	if (dev->map)
+		return 0;
+
 	ret = i2c_dw_acquire_lock(dev);
 	if (ret)
 		return ret;
 
-	reg = dw_readl(dev, DW_IC_COMP_TYPE);
+	reg = readl(dev->base + DW_IC_COMP_TYPE);
 	i2c_dw_release_lock(dev);
 
 	if (reg == swab32(DW_IC_COMP_TYPE_VALUE)) {
-		/* Configure register endianness access */
-		dev->flags |= ACCESS_SWAP;
+		map_cfg.reg_read = dw_reg_read_swab;
+		map_cfg.reg_write = dw_reg_write_swab;
 	} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
-		/* Configure register access mode 16bit */
-		dev->flags |= ACCESS_16BIT;
+		map_cfg.reg_read = dw_reg_read_word;
+		map_cfg.reg_write = dw_reg_write_word;
 	} else if (reg != DW_IC_COMP_TYPE_VALUE) {
 		dev_err(dev->dev,
 			"Unknown Synopsys component type: 0x%08x\n", reg);
 		return -ENODEV;
 	}
 
+	/*
+	 * Note we'll check the return value of the regmap IO accessors only
+	 * at the probe stage. The rest of the code won't do this because
+	 * basically we have MMIO-based regmap so non of the read/write methods
+	 * can fail.
+	 */
+	dev->map = devm_regmap_init(dev->dev, NULL, dev, &map_cfg);
+	if (IS_ERR(dev->map)) {
+		dev_err(dev->dev, "Failed to init the registers map\n");
+		return PTR_ERR(dev->map);
+	}
+
 	return 0;
 }
 
@@ -327,11 +384,17 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
 		return ret;
 
 	/* Configure SDA Hold Time if required */
-	reg = dw_readl(dev, DW_IC_COMP_VERSION);
+	ret = regmap_read(dev->map, DW_IC_COMP_VERSION, &reg);
+	if (ret)
+		goto err_release_lock;
+
 	if (reg >= DW_IC_SDA_HOLD_MIN_VERS) {
 		if (!dev->sda_hold_time) {
 			/* Keep previous hold time setting if no one set it */
-			dev->sda_hold_time = dw_readl(dev, DW_IC_SDA_HOLD);
+			ret = regmap_read(dev->map, DW_IC_SDA_HOLD,
+					  &dev->sda_hold_time);
+			if (ret)
+				goto err_release_lock;
 		}
 
 		/*
@@ -355,22 +418,25 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
 		dev->sda_hold_time = 0;
 	}
 
+err_release_lock:
 	i2c_dw_release_lock(dev);
 
-	return 0;
+	return ret;
 }
 
 void __i2c_dw_disable(struct dw_i2c_dev *dev)
 {
 	int timeout = 100;
+	u32 status;
 
 	do {
 		__i2c_dw_disable_nowait(dev);
 		/*
 		 * The enable status register may be unimplemented, but
 		 * in that case this test reads zero and exits the loop.
 		 */
-		if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == 0)
+		regmap_read(dev->map, DW_IC_ENABLE_STATUS, &status);
+		if ((status & 1) == 0)
 			return;
 
 		/*
@@ -449,22 +515,23 @@ void i2c_dw_release_lock(struct dw_i2c_dev *dev)
  */
 int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
 {
-	int timeout = TIMEOUT;
+	u32 status;
+	int ret;
 
-	while (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
-		if (timeout <= 0) {
-			dev_warn(dev->dev, "timeout waiting for bus ready\n");
-			i2c_recover_bus(&dev->adapter);
+	ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
+				       !(status & DW_IC_STATUS_ACTIVITY),
+				       1100, 20000);
+	if (ret) {
+		dev_warn(dev->dev, "timeout waiting for bus ready\n");
 
-			if (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY)
-				return -ETIMEDOUT;
-			return 0;
-		}
-		timeout--;
-		usleep_range(1000, 1100);
+		i2c_recover_bus(&dev->adapter);
+
+		regmap_read(dev->map, DW_IC_STATUS, &status);
+		if (!(status & DW_IC_STATUS_ACTIVITY))
+			ret = 0;
 	}
 
-	return 0;
+	return ret;
 }
 
 int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
@@ -490,15 +557,19 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 		return -EIO;
 }
 
-void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
+int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
 {
 	u32 param, tx_fifo_depth, rx_fifo_depth;
+	int ret;
 
 	/*
 	 * Try to detect the FIFO depth if not set by interface driver,
 	 * the depth could be from 2 to 256 from HW spec.
 	 */
-	param = dw_readl(dev, DW_IC_COMP_PARAM_1);
+	ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, &param);
+	if (ret)
+		return ret;
+
 	tx_fifo_depth = ((param >> 16) & 0xff) + 1;
 	rx_fifo_depth = ((param >> 8)  & 0xff) + 1;
 	if (!dev->tx_fifo_depth) {
@@ -510,6 +581,8 @@ void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
 		dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
 				rx_fifo_depth);
 	}
+
+	return 0;
 }
 
 u32 i2c_dw_func(struct i2c_adapter *adap)
@@ -521,17 +594,19 @@ u32 i2c_dw_func(struct i2c_adapter *adap)
 
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
+	u32 dummy;
+
 	/* Disable controller */
 	__i2c_dw_disable(dev);
 
 	/* Disable all interrupts */
-	dw_writel(dev, 0, DW_IC_INTR_MASK);
-	dw_readl(dev, DW_IC_CLR_INTR);
+	regmap_write(dev->map, DW_IC_INTR_MASK, 0);
+	regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
 }
 
 void i2c_dw_disable_int(struct dw_i2c_dev *dev)
 {
-	dw_writel(dev, 0, DW_IC_INTR_MASK);
+	regmap_write(dev->map, DW_IC_INTR_MASK, 0);
 }
 
 MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");

drivers/i2c/busses/i2c-designware-core.h

@@ -15,6 +15,7 @@
 #include <linux/dev_printk.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/regmap.h>
 #include <linux/types.h>
 
 #define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C |			\
@@ -126,8 +127,6 @@
 #define STATUS_WRITE_IN_PROGRESS	0x1
 #define STATUS_READ_IN_PROGRESS		0x2
 
-#define TIMEOUT			20 /* ms */
-
 /*
  * operation modes
  */
@@ -183,7 +182,9 @@ struct reset_control;
 /**
  * struct dw_i2c_dev - private i2c-designware data
  * @dev: driver model device node
+ * @map: IO registers map
  * @base: IO registers pointer
+ * @ext: Extended IO registers pointer
  * @cmd_complete: tx completion indicator
  * @clk: input reference clock
  * @pclk: clock required to access the registers
@@ -233,6 +234,7 @@ struct reset_control;
  */
 struct dw_i2c_dev {
 	struct device		*dev;
+	struct regmap		*map;
 	void __iomem		*base;
 	void __iomem		*ext;
 	struct completion	cmd_complete;
@@ -284,17 +286,13 @@ struct dw_i2c_dev {
 	bool			suspended;
 };
 
-#define ACCESS_SWAP		0x00000001
-#define ACCESS_16BIT		0x00000002
-#define ACCESS_INTR_MASK	0x00000004
-#define ACCESS_NO_IRQ_SUSPEND	0x00000008
+#define ACCESS_INTR_MASK	0x00000001
+#define ACCESS_NO_IRQ_SUSPEND	0x00000002
 
 #define MODEL_MSCC_OCELOT	0x00000100
 #define MODEL_MASK		0x00000f00
 
-u32 dw_readl(struct dw_i2c_dev *dev, int offset);
-void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
-int i2c_dw_set_reg_access(struct dw_i2c_dev *dev);
+int i2c_dw_init_regmap(struct dw_i2c_dev *dev);
 u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset);
 u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset);
 int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev);
@@ -304,19 +302,19 @@ int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
 void i2c_dw_release_lock(struct dw_i2c_dev *dev);
 int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
 int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
-void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
+int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
 u32 i2c_dw_func(struct i2c_adapter *adap);
 void i2c_dw_disable(struct dw_i2c_dev *dev);
 void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 
 static inline void __i2c_dw_enable(struct dw_i2c_dev *dev)
 {
-	dw_writel(dev, 1, DW_IC_ENABLE);
+	regmap_write(dev->map, DW_IC_ENABLE, 1);
 }
 
 static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
 {
-	dw_writel(dev, 0, DW_IC_ENABLE);
+	regmap_write(dev->map, DW_IC_ENABLE, 0);
 }
 
 void __i2c_dw_disable(struct dw_i2c_dev *dev);