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, ®); > + 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, ¶m); > + 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