Re: [PATCH v6 08/11] i2c: designware: Convert driver to using regmap API

From: Andy Shevchenko
Date: Thu May 28 2020 - 06:04:36 EST

On Thu, May 28, 2020 at 12:33:18PM +0300, Serge Semin wrote:
> 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).

Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

Thanks!

>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>
> Tested-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>
> Acked-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>
> Cc: Alexey Malahov <Alexey.Malahov@xxxxxxxxxxxxxxxxxxxx>
> Cc: Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx
> Cc: linux-mips@xxxxxxxxxxxxxxx
>
> ---
>
> Changelog v5:
> - Keep alphabetical order of the include statements.
> - Discard explicit u16-type cast in the dw_reg_write_word() method.
>
> Changelog v6:
> - Add comma in the last explicitly initialized member of the map_cfg
> struct regmap_config instance.
> ---
> drivers/i2c/busses/Kconfig | 1 +
> drivers/i2c/busses/i2c-designware-common.c | 178 +++++++++++++++------
> drivers/i2c/busses/i2c-designware-core.h | 22 ++-
> drivers/i2c/busses/i2c-designware-master.c | 125 ++++++++-------
> drivers/i2c/busses/i2c-designware-slave.c | 77 ++++-----
> 5 files changed, 248 insertions(+), 155 deletions(-)
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 7cd279c36898..259e2325712a 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/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"
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index ed302342f8db..0b190a3c837c 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/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,123 @@ 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
> + * @base: Registers map base address
> *
> - * 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 +385,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,14 +419,16 @@ 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);
> @@ -370,7 +436,8 @@ void __i2c_dw_disable(struct dw_i2c_dev *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 +516,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 +558,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 +582,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 +595,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");
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index b9ef9b0deef0..f5bbe3d6bcf8 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/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);
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index 95eeec53c744..2cba21b945d8 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -18,6 +18,7 @@
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> #include <linux/reset.h>
>
> #include "i2c-designware-core.h"
> @@ -25,11 +26,11 @@
> static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
> {
> /* Configure Tx/Rx FIFO threshold levels */
> - dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
> - dw_writel(dev, 0, DW_IC_RX_TL);
> + regmap_write(dev->map, DW_IC_TX_TL, dev->tx_fifo_depth / 2);
> + regmap_write(dev->map, DW_IC_RX_TL, 0);
>
> /* Configure the I2C master */
> - dw_writel(dev, dev->master_cfg, DW_IC_CON);
> + regmap_write(dev->map, DW_IC_CON, dev->master_cfg);
> }
>
> static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
> @@ -44,8 +45,11 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
> ret = i2c_dw_acquire_lock(dev);
> if (ret)
> return ret;
> - comp_param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
> +
> + ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, &comp_param1);
> i2c_dw_release_lock(dev);
> + if (ret)
> + return ret;
>
> /* Set standard and fast speed dividers for high/low periods */
> sda_falling_time = t->sda_fall_ns ?: 300; /* ns */
> @@ -187,22 +191,22 @@ static int i2c_dw_init_master(struct dw_i2c_dev *dev)
> __i2c_dw_disable(dev);
>
> /* Write standard speed timing parameters */
> - dw_writel(dev, dev->ss_hcnt, DW_IC_SS_SCL_HCNT);
> - dw_writel(dev, dev->ss_lcnt, DW_IC_SS_SCL_LCNT);
> + regmap_write(dev->map, DW_IC_SS_SCL_HCNT, dev->ss_hcnt);
> + regmap_write(dev->map, DW_IC_SS_SCL_LCNT, dev->ss_lcnt);
>
> /* Write fast mode/fast mode plus timing parameters */
> - dw_writel(dev, dev->fs_hcnt, DW_IC_FS_SCL_HCNT);
> - dw_writel(dev, dev->fs_lcnt, DW_IC_FS_SCL_LCNT);
> + regmap_write(dev->map, DW_IC_FS_SCL_HCNT, dev->fs_hcnt);
> + regmap_write(dev->map, DW_IC_FS_SCL_LCNT, dev->fs_lcnt);
>
> /* Write high speed timing parameters if supported */
> if (dev->hs_hcnt && dev->hs_lcnt) {
> - dw_writel(dev, dev->hs_hcnt, DW_IC_HS_SCL_HCNT);
> - dw_writel(dev, dev->hs_lcnt, DW_IC_HS_SCL_LCNT);
> + regmap_write(dev->map, DW_IC_HS_SCL_HCNT, dev->hs_hcnt);
> + regmap_write(dev->map, DW_IC_HS_SCL_LCNT, dev->hs_lcnt);
> }
>
> /* Write SDA hold time if supported */
> if (dev->sda_hold_time)
> - dw_writel(dev, dev->sda_hold_time, DW_IC_SDA_HOLD);
> + regmap_write(dev->map, DW_IC_SDA_HOLD, dev->sda_hold_time);
>
> i2c_dw_configure_fifo_master(dev);
> i2c_dw_release_lock(dev);
> @@ -213,15 +217,15 @@ static int i2c_dw_init_master(struct dw_i2c_dev *dev)
> static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
> {
> struct i2c_msg *msgs = dev->msgs;
> - u32 ic_con, ic_tar = 0;
> + u32 ic_con = 0, ic_tar = 0;
> + u32 dummy;
>
> /* Disable the adapter */
> __i2c_dw_disable(dev);
>
> /* If the slave address is ten bit address, enable 10BITADDR */
> - ic_con = dw_readl(dev, DW_IC_CON);
> if (msgs[dev->msg_write_idx].flags & I2C_M_TEN) {
> - ic_con |= DW_IC_CON_10BITADDR_MASTER;
> + ic_con = DW_IC_CON_10BITADDR_MASTER;
> /*
> * If I2C_DYNAMIC_TAR_UPDATE is set, the 10-bit addressing
> * mode has to be enabled via bit 12 of IC_TAR register.
> @@ -229,17 +233,17 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
> * detected from registers.
> */
> ic_tar = DW_IC_TAR_10BITADDR_MASTER;
> - } else {
> - ic_con &= ~DW_IC_CON_10BITADDR_MASTER;
> }
>
> - dw_writel(dev, ic_con, DW_IC_CON);
> + regmap_update_bits(dev->map, DW_IC_CON, DW_IC_CON_10BITADDR_MASTER,
> + ic_con);
>
> /*
> * Set the slave (target) address and enable 10-bit addressing mode
> * if applicable.
> */
> - dw_writel(dev, msgs[dev->msg_write_idx].addr | ic_tar, DW_IC_TAR);
> + regmap_write(dev->map, DW_IC_TAR,
> + msgs[dev->msg_write_idx].addr | ic_tar);
>
> /* Enforce disabled interrupts (due to HW issues) */
> i2c_dw_disable_int(dev);
> @@ -248,11 +252,11 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
> __i2c_dw_enable(dev);
>
> /* Dummy read to avoid the register getting stuck on Bay Trail */
> - dw_readl(dev, DW_IC_ENABLE_STATUS);
> + regmap_read(dev->map, DW_IC_ENABLE_STATUS, &dummy);
>
> /* Clear and enable interrupts */
> - dw_readl(dev, DW_IC_CLR_INTR);
> - dw_writel(dev, DW_IC_INTR_MASTER_MASK, DW_IC_INTR_MASK);
> + regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
> + regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_MASTER_MASK);
> }
>
> /*
> @@ -271,6 +275,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
> u32 buf_len = dev->tx_buf_len;
> u8 *buf = dev->tx_buf;
> bool need_restart = false;
> + unsigned int flr;
>
> intr_mask = DW_IC_INTR_MASTER_MASK;
>
> @@ -303,8 +308,11 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
> need_restart = true;
> }
>
> - tx_limit = dev->tx_fifo_depth - dw_readl(dev, DW_IC_TXFLR);
> - rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
> + regmap_read(dev->map, DW_IC_TXFLR, &flr);
> + tx_limit = dev->tx_fifo_depth - flr;
> +
> + regmap_read(dev->map, DW_IC_RXFLR, &flr);
> + rx_limit = dev->rx_fifo_depth - flr;
>
> while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
> u32 cmd = 0;
> @@ -337,11 +345,14 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
> if (dev->rx_outstanding >= dev->rx_fifo_depth)
> break;
>
> - dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
> + regmap_write(dev->map, DW_IC_DATA_CMD,
> + cmd | 0x100);
> rx_limit--;
> dev->rx_outstanding++;
> - } else
> - dw_writel(dev, cmd | *buf++, DW_IC_DATA_CMD);
> + } else {
> + regmap_write(dev->map, DW_IC_DATA_CMD,
> + cmd | *buf++);
> + }
> tx_limit--; buf_len--;
> }
>
> @@ -371,7 +382,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
> if (dev->msg_err)
> intr_mask = 0;
>
> - dw_writel(dev, intr_mask, DW_IC_INTR_MASK);
> + regmap_write(dev->map, DW_IC_INTR_MASK, intr_mask);
> }
>
> static u8
> @@ -399,7 +410,7 @@ i2c_dw_read(struct dw_i2c_dev *dev)
> int rx_valid;
>
> for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
> - u32 len;
> + u32 len, tmp;
> u8 *buf;
>
> if (!(msgs[dev->msg_read_idx].flags & I2C_M_RD))
> @@ -413,18 +424,18 @@ i2c_dw_read(struct dw_i2c_dev *dev)
> buf = dev->rx_buf;
> }
>
> - rx_valid = dw_readl(dev, DW_IC_RXFLR);
> + regmap_read(dev->map, DW_IC_RXFLR, &rx_valid);
>
> for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
> u32 flags = msgs[dev->msg_read_idx].flags;
>
> - *buf = dw_readl(dev, DW_IC_DATA_CMD);
> + regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
> /* Ensure length byte is a valid value */
> if (flags & I2C_M_RECV_LEN &&
> - *buf <= I2C_SMBUS_BLOCK_MAX && *buf > 0) {
> - len = i2c_dw_recv_len(dev, *buf);
> + tmp <= I2C_SMBUS_BLOCK_MAX && tmp > 0) {
> + len = i2c_dw_recv_len(dev, tmp);
> }
> - buf++;
> + *buf++ = tmp;
> dev->rx_outstanding--;
> }
>
> @@ -542,7 +553,7 @@ static const struct i2c_adapter_quirks i2c_dw_quirks = {
>
> static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
> {
> - u32 stat;
> + u32 stat, dummy;
>
> /*
> * The IC_INTR_STAT register just indicates "enabled" interrupts.
> @@ -550,47 +561,47 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
> * in the IC_RAW_INTR_STAT register.
> *
> * That is,
> - * stat = dw_readl(IC_INTR_STAT);
> + * stat = readl(IC_INTR_STAT);
> * equals to,
> - * stat = dw_readl(IC_RAW_INTR_STAT) & dw_readl(IC_INTR_MASK);
> + * stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
> *
> * The raw version might be useful for debugging purposes.
> */
> - stat = dw_readl(dev, DW_IC_INTR_STAT);
> + regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
>
> /*
> * Do not use the IC_CLR_INTR register to clear interrupts, or
> * you'll miss some interrupts, triggered during the period from
> - * dw_readl(IC_INTR_STAT) to dw_readl(IC_CLR_INTR).
> + * readl(IC_INTR_STAT) to readl(IC_CLR_INTR).
> *
> * Instead, use the separately-prepared IC_CLR_* registers.
> */
> if (stat & DW_IC_INTR_RX_UNDER)
> - dw_readl(dev, DW_IC_CLR_RX_UNDER);
> + regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &dummy);
> if (stat & DW_IC_INTR_RX_OVER)
> - dw_readl(dev, DW_IC_CLR_RX_OVER);
> + regmap_read(dev->map, DW_IC_CLR_RX_OVER, &dummy);
> if (stat & DW_IC_INTR_TX_OVER)
> - dw_readl(dev, DW_IC_CLR_TX_OVER);
> + regmap_read(dev->map, DW_IC_CLR_TX_OVER, &dummy);
> if (stat & DW_IC_INTR_RD_REQ)
> - dw_readl(dev, DW_IC_CLR_RD_REQ);
> + regmap_read(dev->map, DW_IC_CLR_RD_REQ, &dummy);
> if (stat & DW_IC_INTR_TX_ABRT) {
> /*
> * The IC_TX_ABRT_SOURCE register is cleared whenever
> * the IC_CLR_TX_ABRT is read. Preserve it beforehand.
> */
> - dev->abort_source = dw_readl(dev, DW_IC_TX_ABRT_SOURCE);
> - dw_readl(dev, DW_IC_CLR_TX_ABRT);
> + regmap_read(dev->map, DW_IC_TX_ABRT_SOURCE, &dev->abort_source);
> + regmap_read(dev->map, DW_IC_CLR_TX_ABRT, &dummy);
> }
> if (stat & DW_IC_INTR_RX_DONE)
> - dw_readl(dev, DW_IC_CLR_RX_DONE);
> + regmap_read(dev->map, DW_IC_CLR_RX_DONE, &dummy);
> if (stat & DW_IC_INTR_ACTIVITY)
> - dw_readl(dev, DW_IC_CLR_ACTIVITY);
> + regmap_read(dev->map, DW_IC_CLR_ACTIVITY, &dummy);
> if (stat & DW_IC_INTR_STOP_DET)
> - dw_readl(dev, DW_IC_CLR_STOP_DET);
> + regmap_read(dev->map, DW_IC_CLR_STOP_DET, &dummy);
> if (stat & DW_IC_INTR_START_DET)
> - dw_readl(dev, DW_IC_CLR_START_DET);
> + regmap_read(dev->map, DW_IC_CLR_START_DET, &dummy);
> if (stat & DW_IC_INTR_GEN_CALL)
> - dw_readl(dev, DW_IC_CLR_GEN_CALL);
> + regmap_read(dev->map, DW_IC_CLR_GEN_CALL, &dummy);
>
> return stat;
> }
> @@ -612,7 +623,7 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
> * Anytime TX_ABRT is set, the contents of the tx/rx
> * buffers are flushed. Make sure to skip them.
> */
> - dw_writel(dev, 0, DW_IC_INTR_MASK);
> + regmap_write(dev->map, DW_IC_INTR_MASK, 0);
> goto tx_aborted;
> }
>
> @@ -633,9 +644,9 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
> complete(&dev->cmd_complete);
> else if (unlikely(dev->flags & ACCESS_INTR_MASK)) {
> /* Workaround to trigger pending interrupt */
> - stat = dw_readl(dev, DW_IC_INTR_MASK);
> + regmap_read(dev->map, DW_IC_INTR_MASK, &stat);
> i2c_dw_disable_int(dev);
> - dw_writel(dev, stat, DW_IC_INTR_MASK);
> + regmap_write(dev->map, DW_IC_INTR_MASK, stat);
> }
>
> return 0;
> @@ -646,8 +657,8 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
> struct dw_i2c_dev *dev = dev_id;
> u32 stat, enabled;
>
> - enabled = dw_readl(dev, DW_IC_ENABLE);
> - stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
> + regmap_read(dev->map, DW_IC_ENABLE, &enabled);
> + regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
> dev_dbg(dev->dev, "enabled=%#x stat=%#x\n", enabled, stat);
> if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
> return IRQ_NONE;
> @@ -739,7 +750,7 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
> dev->disable = i2c_dw_disable;
> dev->disable_int = i2c_dw_disable_int;
>
> - ret = i2c_dw_set_reg_access(dev);
> + ret = i2c_dw_init_regmap(dev);
> if (ret)
> return ret;
>
> @@ -747,7 +758,9 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
> if (ret)
> return ret;
>
> - i2c_dw_set_fifo_size(dev);
> + ret = i2c_dw_set_fifo_size(dev);
> + if (ret)
> + return ret;
>
> ret = dev->init(dev);
> if (ret)
> diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
> index 576e7af4e94b..44974b53a626 100644
> --- a/drivers/i2c/busses/i2c-designware-slave.c
> +++ b/drivers/i2c/busses/i2c-designware-slave.c
> @@ -14,18 +14,19 @@
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
>
> #include "i2c-designware-core.h"
>
> static void i2c_dw_configure_fifo_slave(struct dw_i2c_dev *dev)
> {
> /* Configure Tx/Rx FIFO threshold levels. */
> - dw_writel(dev, 0, DW_IC_TX_TL);
> - dw_writel(dev, 0, DW_IC_RX_TL);
> + regmap_write(dev->map, DW_IC_TX_TL, 0);
> + regmap_write(dev->map, DW_IC_RX_TL, 0);
>
> /* Configure the I2C slave. */
> - dw_writel(dev, dev->slave_cfg, DW_IC_CON);
> - dw_writel(dev, DW_IC_INTR_SLAVE_MASK, DW_IC_INTR_MASK);
> + regmap_write(dev->map, DW_IC_CON, dev->slave_cfg);
> + regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_SLAVE_MASK);
> }
>
> /**
> @@ -49,7 +50,7 @@ static int i2c_dw_init_slave(struct dw_i2c_dev *dev)
>
> /* Write SDA hold time if supported */
> if (dev->sda_hold_time)
> - dw_writel(dev, dev->sda_hold_time, DW_IC_SDA_HOLD);
> + regmap_write(dev->map, DW_IC_SDA_HOLD, dev->sda_hold_time);
>
> i2c_dw_configure_fifo_slave(dev);
> i2c_dw_release_lock(dev);
> @@ -72,7 +73,7 @@ static int i2c_dw_reg_slave(struct i2c_client *slave)
> * the address to which the DW_apb_i2c responds.
> */
> __i2c_dw_disable_nowait(dev);
> - dw_writel(dev, slave->addr, DW_IC_SAR);
> + regmap_write(dev->map, DW_IC_SAR, slave->addr);
> dev->slave = slave;
>
> __i2c_dw_enable(dev);
> @@ -103,7 +104,7 @@ static int i2c_dw_unreg_slave(struct i2c_client *slave)
>
> static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
> {
> - u32 stat;
> + u32 stat, dummy;
>
> /*
> * The IC_INTR_STAT register just indicates "enabled" interrupts.
> @@ -111,39 +112,39 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
> * in the IC_RAW_INTR_STAT register.
> *
> * That is,
> - * stat = dw_readl(IC_INTR_STAT);
> + * stat = readl(IC_INTR_STAT);
> * equals to,
> - * stat = dw_readl(IC_RAW_INTR_STAT) & dw_readl(IC_INTR_MASK);
> + * stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
> *
> * The raw version might be useful for debugging purposes.
> */
> - stat = dw_readl(dev, DW_IC_INTR_STAT);
> + regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
>
> /*
> * Do not use the IC_CLR_INTR register to clear interrupts, or
> * you'll miss some interrupts, triggered during the period from
> - * dw_readl(IC_INTR_STAT) to dw_readl(IC_CLR_INTR).
> + * readl(IC_INTR_STAT) to readl(IC_CLR_INTR).
> *
> * Instead, use the separately-prepared IC_CLR_* registers.
> */
> if (stat & DW_IC_INTR_TX_ABRT)
> - dw_readl(dev, DW_IC_CLR_TX_ABRT);
> + regmap_read(dev->map, DW_IC_CLR_TX_ABRT, &dummy);
> if (stat & DW_IC_INTR_RX_UNDER)
> - dw_readl(dev, DW_IC_CLR_RX_UNDER);
> + regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &dummy);
> if (stat & DW_IC_INTR_RX_OVER)
> - dw_readl(dev, DW_IC_CLR_RX_OVER);
> + regmap_read(dev->map, DW_IC_CLR_RX_OVER, &dummy);
> if (stat & DW_IC_INTR_TX_OVER)
> - dw_readl(dev, DW_IC_CLR_TX_OVER);
> + regmap_read(dev->map, DW_IC_CLR_TX_OVER, &dummy);
> if (stat & DW_IC_INTR_RX_DONE)
> - dw_readl(dev, DW_IC_CLR_RX_DONE);
> + regmap_read(dev->map, DW_IC_CLR_RX_DONE, &dummy);
> if (stat & DW_IC_INTR_ACTIVITY)
> - dw_readl(dev, DW_IC_CLR_ACTIVITY);
> + regmap_read(dev->map, DW_IC_CLR_ACTIVITY, &dummy);
> if (stat & DW_IC_INTR_STOP_DET)
> - dw_readl(dev, DW_IC_CLR_STOP_DET);
> + regmap_read(dev->map, DW_IC_CLR_STOP_DET, &dummy);
> if (stat & DW_IC_INTR_START_DET)
> - dw_readl(dev, DW_IC_CLR_START_DET);
> + regmap_read(dev->map, DW_IC_CLR_START_DET, &dummy);
> if (stat & DW_IC_INTR_GEN_CALL)
> - dw_readl(dev, DW_IC_CLR_GEN_CALL);
> + regmap_read(dev->map, DW_IC_CLR_GEN_CALL, &dummy);
>
> return stat;
> }
> @@ -155,14 +156,14 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
>
> static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
> {
> - u32 raw_stat, stat, enabled;
> - u8 val, slave_activity;
> + u32 raw_stat, stat, enabled, tmp;
> + u8 val = 0, slave_activity;
>
> - stat = dw_readl(dev, DW_IC_INTR_STAT);
> - enabled = dw_readl(dev, DW_IC_ENABLE);
> - raw_stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
> - slave_activity = ((dw_readl(dev, DW_IC_STATUS) &
> - DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
> + regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
> + regmap_read(dev->map, DW_IC_ENABLE, &enabled);
> + regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
> + regmap_read(dev->map, DW_IC_STATUS, &tmp);
> + slave_activity = ((tmp & DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
>
> if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
> return 0;
> @@ -177,7 +178,8 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
> if (stat & DW_IC_INTR_RD_REQ) {
> if (slave_activity) {
> if (stat & DW_IC_INTR_RX_FULL) {
> - val = dw_readl(dev, DW_IC_DATA_CMD);
> + regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
> + val = tmp;
>
> if (!i2c_slave_event(dev->slave,
> I2C_SLAVE_WRITE_RECEIVED,
> @@ -185,24 +187,24 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
> dev_vdbg(dev->dev, "Byte %X acked!",
> val);
> }
> - dw_readl(dev, DW_IC_CLR_RD_REQ);
> + regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
> stat = i2c_dw_read_clear_intrbits_slave(dev);
> } else {
> - dw_readl(dev, DW_IC_CLR_RD_REQ);
> - dw_readl(dev, DW_IC_CLR_RX_UNDER);
> + regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
> + regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
> stat = i2c_dw_read_clear_intrbits_slave(dev);
> }
> if (!i2c_slave_event(dev->slave,
> I2C_SLAVE_READ_REQUESTED,
> &val))
> - dw_writel(dev, val, DW_IC_DATA_CMD);
> + regmap_write(dev->map, DW_IC_DATA_CMD, val);
> }
> }
>
> if (stat & DW_IC_INTR_RX_DONE) {
> if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
> &val))
> - dw_readl(dev, DW_IC_CLR_RX_DONE);
> + regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
>
> i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
> stat = i2c_dw_read_clear_intrbits_slave(dev);
> @@ -210,7 +212,8 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
> }
>
> if (stat & DW_IC_INTR_RX_FULL) {
> - val = dw_readl(dev, DW_IC_DATA_CMD);
> + regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
> + val = tmp;
> if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
> &val))
> dev_vdbg(dev->dev, "Byte %X acked!", val);
> @@ -263,7 +266,7 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
> dev->disable = i2c_dw_disable;
> dev->disable_int = i2c_dw_disable_int;
>
> - ret = i2c_dw_set_reg_access(dev);
> + ret = i2c_dw_init_regmap(dev);
> if (ret)
> return ret;
>
> @@ -271,7 +274,9 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
> if (ret)
> return ret;
>
> - i2c_dw_set_fifo_size(dev);
> + ret = i2c_dw_set_fifo_size(dev);
> + if (ret)
> + return ret;
>
> ret = dev->init(dev);
> if (ret)
> --
> 2.26.2
>

--
With Best Regards,
Andy Shevchenko