i2c: designware: refactor low-level enable/disable

Low-level controller enable function __i2c_dw_enable is overloaded to
also handle disabling. What's worse, even though the documentation
requires polling the IC_ENABLE_STATUS register when disabling, this
is not done: polling needs to be requested specifically by calling
__i2c_dw_enable_and_wait, which can also poll on enabling, but that
doesn't work if the IC_ENABLE_STATUS register is not implemented.
This is quite confusing if not in fact backwards.

Especially since the documentation says that disabling should be
followed by polling, the driver should be using a separate function
where it does one-shot disables to make the optimization stand out.

This refactors the two functions so that requested status is given
in the name rather than in a boolean argument. Specifically:

 - __i2c_dw_enable: enable without polling (in accordance with docs)
 - __i2c_dw_disable: disable and do poll (also as suggested by docs)
 - __i2c_dw_disable_nowait: disable without polling (Linux-specific)

No functional change.

Signed-off-by: Alexander Monakov <amonakov@ispras.ru>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
[wsa: fixed blank lines in header file]
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 27ebd90..48914df 100644 (file)
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -149,18 +149,17 @@ u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
        return ((ic_clk * (tLOW + tf) + 500000) / 1000000) - 1 + offset;
 }
 
-void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
-{
-       dw_writel(dev, enable, DW_IC_ENABLE);
-}
-
-void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable)
+void __i2c_dw_disable(struct dw_i2c_dev *dev)
 {
        int timeout = 100;
 
        do {
-               __i2c_dw_enable(dev, enable);
-               if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
+               __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)
                        return;
 
                /*
@@ -171,8 +170,7 @@ void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable)
                usleep_range(25, 250);
        } while (timeout--);
 
-       dev_warn(dev->dev, "timeout in %sabling adapter\n",
-                enable ? "en" : "dis");
+       dev_warn(dev->dev, "timeout in disabling adapter\n");
 }
 
 unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
@@ -277,7 +275,7 @@ u32 i2c_dw_func(struct i2c_adapter *adap)
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
        /* Disable controller */
-       __i2c_dw_enable_and_wait(dev, false);
+       __i2c_dw_disable(dev);
 
        /* Disable all interupts */
        dw_writel(dev, 0, DW_IC_INTR_MASK);

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 8707c76..d690e64 100644 (file)
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -297,8 +297,6 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset);
 void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
 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);
-void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable);
-void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable);
 unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev);
 int i2c_dw_prepare_clk(struct dw_i2c_dev *dev, bool prepare);
 int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
@@ -309,6 +307,18 @@ 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);
+}
+
+static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
+{
+       dw_writel(dev, 0, DW_IC_ENABLE);
+}
+
+void __i2c_dw_disable(struct dw_i2c_dev *dev);
+
 extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
 extern int i2c_dw_probe(struct dw_i2c_dev *dev);
 #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_SLAVE)

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 0cdba29..27436a9 100644 (file)
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -81,7 +81,7 @@ static int i2c_dw_init_master(struct dw_i2c_dev *dev)
        comp_param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
 
        /* Disable the adapter */
-       __i2c_dw_enable_and_wait(dev, false);
+       __i2c_dw_disable(dev);
 
        /* Set standard and fast speed deviders for high/low periods */
 
@@ -180,7 +180,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
        u32 ic_con, ic_tar = 0;
 
        /* Disable the adapter */
-       __i2c_dw_enable_and_wait(dev, false);
+       __i2c_dw_disable(dev);
 
        /* If the slave address is ten bit address, enable 10BITADDR */
        ic_con = dw_readl(dev, DW_IC_CON);
@@ -209,7 +209,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
        i2c_dw_disable_int(dev);
 
        /* Enable the adapter */
-       __i2c_dw_enable(dev, true);
+       __i2c_dw_enable(dev);
 
        /* Dummy read to avoid the register getting stuck on Bay Trail */
        dw_readl(dev, DW_IC_ENABLE_STATUS);
@@ -462,7 +462,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
         * additional interrupts are a hardware bug or this driver doesn't
         * handle them correctly yet.
         */
-       __i2c_dw_enable(dev, false);
+       __i2c_dw_disable_nowait(dev);
 
        if (dev->msg_err) {
                ret = dev->msg_err;

diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index d42558d..8ce2cd3 100644 (file)
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -75,7 +75,7 @@ static int i2c_dw_init_slave(struct dw_i2c_dev *dev)
        comp_param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
 
        /* Disable the adapter. */
-       __i2c_dw_enable_and_wait(dev, false);
+       __i2c_dw_disable(dev);
 
        /* Configure SDA Hold Time if required. */
        reg = dw_readl(dev, DW_IC_COMP_VERSION);
@@ -119,11 +119,11 @@ static int i2c_dw_reg_slave(struct i2c_client *slave)
         * Set slave address in the IC_SAR register,
         * the address to which the DW_apb_i2c responds.
         */
-       __i2c_dw_enable(dev, false);
+       __i2c_dw_disable_nowait(dev);
        dw_writel(dev, slave->addr, DW_IC_SAR);
        dev->slave = slave;
 
-       __i2c_dw_enable(dev, true);
+       __i2c_dw_enable(dev);
 
        dev->cmd_err = 0;
        dev->msg_write_idx = 0;