Hi Alexander,
On 03/11/2015 03:08 PM, Alexander Sverdlin wrote:
There are several problems in the function:
- "to_cnt" variable does nothing
- schedule_timeout() call without setting current state does nothing
- "allow_sleep" parameter is not really used
Refactor the function so that it really tries to wait. In case of timeout try
to recover the bus and finally initialize the controller. We cannot really
do more than that, so it makes no sense to check BB bit after init.
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
---
drivers/i2c/busses/i2c-davinci.c | 53 +++++++++++++++++++++----------------
1 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 98759ae..ca22479 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -152,6 +152,26 @@ static void davinci_i2c_clock_pulse(unsigned int scl_pin)
}
}
+/*
+ * Wait until specific bit in status register has particular value
+ * Function returns 0 if condition was met,
+ * -ETIMEDOUT in case of timeout.
+ */
+static int i2c_davinci_wait_status_change(struct davinci_i2c_dev *dev, u16 mask,
+ u16 val)
do we really need it as separate function? (even if it looks good:)
+{
+ unsigned long timeout = jiffies + dev->adapter.timeout;
+
+ while ((davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG) & mask) != val) {
+ if (time_after(jiffies, timeout))
+ return -ETIMEDOUT;
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ schedule_timeout(1);
schedule_timeout_uninterruptible()
+ }
+
+ return 0;
+}
+
/* This routine does i2c bus recovery as specified in the
* i2c protocol Rev. 03 section 3.16 titled "Bus clear"
*/
@@ -269,29 +289,16 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
/*
* Waiting for bus not busy
*/
-static int i2c_davinci_wait_bus_not_busy(struct davinci_i2c_dev *dev,
- char allow_sleep)
+static int i2c_davinci_wait_bus_not_busy(struct davinci_i2c_dev *dev)
{
- unsigned long timeout;
- static u16 to_cnt;
-
- timeout = jiffies + dev->adapter.timeout;
- while (davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG)
- & DAVINCI_I2C_STR_BB) {
- if (to_cnt <= DAVINCI_I2C_MAX_TRIES) {
- if (time_after(jiffies, timeout)) {
- dev_warn(dev->dev,
- "timeout waiting for bus ready\n");
- to_cnt++;
- return -ETIMEDOUT;
- } else {
- to_cnt = 0;
- davinci_i2c_recover_bus(dev);
- i2c_davinci_init(dev);
- }
- }
- if (allow_sleep)
- schedule_timeout(1);
+ if (i2c_davinci_wait_status_change(dev, DAVINCI_I2C_STR_BB, 0)) {
+ dev_warn(dev->dev, "timeout waiting for bus ready\n");
+ davinci_i2c_recover_bus(dev);
+ i2c_davinci_init(dev);
+ /*
+ * the bus should not be busy after init, otherwise something
+ * is badly broken
+ */
I think you should recheck BB and return error if it's still detected.
}
return 0;
@@ -449,7 +456,7 @@ i2c_davinci_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
dev_dbg(dev->dev, "%s: msgs: %d\n", __func__, num);
- ret = i2c_davinci_wait_bus_not_busy(dev, 1);
+ ret = i2c_davinci_wait_bus_not_busy(dev);
if (ret < 0) {
dev_warn(dev->dev, "timeout waiting for bus ready\n");
return ret;
regards,
-grygorii
--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
