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.
Cc: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>
Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
Signed-off-by: Alexander Monakov <amonakov@xxxxxxxxx>
---
drivers/i2c/busses/i2c-designware-common.c | 20 +++++++++-----------
drivers/i2c/busses/i2c-designware-core.h | 13 +++++++++++--
drivers/i2c/busses/i2c-designware-master.c | 8 ++++----
drivers/i2c/busses/i2c-designware-slave.c | 6 +++---
4 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 27ebd90de43b..48914dfc8ce8 100644
--- 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 8707c76b2fee..6c61ac85c936 100644
--- 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,17 @@ 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 a80926d86a29..6d92f9777581 100644
--- 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);
disable_irq(dev->irq);
/* If the slave address is ten bit address, enable 10BITADDR */
@@ -210,7 +210,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);
/* Clear and enable interrupts */
dw_readl(dev, DW_IC_CLR_INTR);
@@ -461,7 +461,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 d42558d1b002..8ce2cd368477 100644
--- 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;
--
2.11.0
