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 27ebd90de43bb781208d66c0e43c29dce1ccd450..48914dfc8ce88ff54246a2f450ef2bc3ae9afe4d 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 8707c76b2fee10b08e7957a137ff89b67ea095fe..d690e648bc015efcd96d63b81c11969e9a6ceaae 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 0cdba29ae0a9ad25212f21a56acf76f46c1b5213..27436a937492d1afdefe77264bcd1afaf7d91856 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 d42558d1b002ff6c23466e5cc60c49a7833bb438..8ce2cd36847717f72a6e2057d8a23922591b7050 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;