* [PATCH v2] i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()
@ 2015-05-13 13:03 Alexander Sverdlin
[not found] ` <55534BA0.3000506-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Sverdlin @ 2015-05-13 13:03 UTC (permalink / raw
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Kevin Hilman,
Sekhar Nori, Grygorii Strashko, Santosh Shilimkar,
Vishwanathrao Badarkhe, Manish, Murali Karicheri
Cc: Lawnick Michael 61283229, Mike Looijmans, Mastalski Bartosz
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.
Signed-off-by: Alexander Sverdlin <alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
---
Changes in v2:
- rebased on 110bc76729d4 of Linus's tree;
drivers/i2c/busses/i2c-davinci.c | 42 +++++++++++++++++--------------------
1 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index df96ab6..03afdee 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -350,29 +350,25 @@ static struct i2c_bus_recovery_info davinci_i2c_scl_recovery_info = {
/*
* 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;
- i2c_recover_bus(&dev->adapter);
- }
- }
- if (allow_sleep)
- schedule_timeout(1);
- }
+ unsigned long timeout = jiffies + dev->adapter.timeout;
+
+ do {
+ if (!(davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG) & DAVINCI_I2C_STR_BB))
+ return 0;
+ schedule_timeout_uninterruptible(1);
+ } while (time_before_eq(jiffies, timeout));
+
+ dev_warn(dev->dev, "timeout waiting for bus ready\n");
+ i2c_recover_bus(dev);
+
+ /*
+ * if bus is still "busy" here, it's most probably a HW problem like
+ * short-circuit
+ */
+ if (davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG) & DAVINCI_I2C_STR_BB)
+ return -EIO;
return 0;
}
@@ -508,7 +504,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;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()
[not found] ` <55534BA0.3000506-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
@ 2015-05-14 11:19 ` Grygorii.Strashko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
2015-06-02 15:53 ` Wolfram Sang
2015-06-09 10:58 ` [PATCH v3] " Alexander Sverdlin
2 siblings, 0 replies; 6+ messages in thread
From: Grygorii.Strashko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org @ 2015-05-14 11:19 UTC (permalink / raw
To: Alexander Sverdlin, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Wolfram Sang, Kevin Hilman, Sekhar Nori, Santosh Shilimkar,
Vishwanathrao Badarkhe, Manish, Murali Karicheri
Cc: Lawnick Michael 61283229, Mike Looijmans, Mastalski Bartosz
On 05/13/2015 04:03 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.
Reviewed-by: Grygorii Strashko <grygorii.strashko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> ---
> Changes in v2:
> - rebased on 110bc76729d4 of Linus's tree;
>
> drivers/i2c/busses/i2c-davinci.c | 42 +++++++++++++++++--------------------
> 1 files changed, 19 insertions(+), 23 deletions(-)
--
regards,
-grygorii
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()
[not found] ` <55534BA0.3000506-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
2015-05-14 11:19 ` Grygorii.Strashko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
@ 2015-06-02 15:53 ` Wolfram Sang
2015-06-09 10:54 ` Alexander Sverdlin
2015-06-09 10:58 ` [PATCH v3] " Alexander Sverdlin
2 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2015-06-02 15:53 UTC (permalink / raw
To: Alexander Sverdlin
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Kevin Hilman, Sekhar Nori,
Grygorii Strashko, Santosh Shilimkar,
Vishwanathrao Badarkhe, Manish, Murali Karicheri,
Lawnick Michael 61283229, Mike Looijmans, Mastalski Bartosz
[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]
On Wed, May 13, 2015 at 03:03:28PM +0200, 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.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
Don't you get a build warning?
drivers/i2c/busses/i2c-davinci.c: In function 'i2c_davinci_wait_bus_not_busy':
drivers/i2c/busses/i2c-davinci.c:364:18: warning: passing argument 1 of 'i2c_recover_bus' from incompatible pointer type
i2c_recover_bus(dev);
^
In file included from drivers/i2c/busses/i2c-davinci.c:26:0:
include/linux/i2c.h:446:5: note: expected 'struct i2c_adapter *' but argument is of type 'struct davinci_i2c_dev *'
int i2c_recover_bus(struct i2c_adapter *adap);
Or sparse complains, too:
drivers/i2c/busses/i2c-davinci.c:364:25: warning: incorrect type in argument 1 (different base types)
drivers/i2c/busses/i2c-davinci.c:364:25: expected struct i2c_adapter *adap
drivers/i2c/busses/i2c-davinci.c:364:25: got struct davinci_i2c_dev *dev
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()
2015-06-02 15:53 ` Wolfram Sang
@ 2015-06-09 10:54 ` Alexander Sverdlin
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Sverdlin @ 2015-06-09 10:54 UTC (permalink / raw
To: ext Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA
Hello Wolfram,
On 02/06/15 17:53, ext Wolfram Sang 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.
>> >
>> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> Don't you get a build warning?
>
> drivers/i2c/busses/i2c-davinci.c: In function 'i2c_davinci_wait_bus_not_busy':
> drivers/i2c/busses/i2c-davinci.c:364:18: warning: passing argument 1 of 'i2c_recover_bus' from incompatible pointer type
> i2c_recover_bus(dev);
> ^
> In file included from drivers/i2c/busses/i2c-davinci.c:26:0:
> include/linux/i2c.h:446:5: note: expected 'struct i2c_adapter *' but argument is of type 'struct davinci_i2c_dev *'
> int i2c_recover_bus(struct i2c_adapter *adap);
oh, this is awful copy-paster error... I'll re-send!
--
Best regards,
Alexander Sverdlin.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()
[not found] ` <55534BA0.3000506-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
2015-05-14 11:19 ` Grygorii.Strashko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
2015-06-02 15:53 ` Wolfram Sang
@ 2015-06-09 10:58 ` Alexander Sverdlin
[not found] ` <5576C6D5.9070800-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
2 siblings, 1 reply; 6+ messages in thread
From: Alexander Sverdlin @ 2015-06-09 10:58 UTC (permalink / raw
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Kevin Hilman,
Sekhar Nori, Grygorii Strashko, Santosh Shilimkar,
Vishwanathrao Badarkhe, Manish, Murali Karicheri
Cc: Lawnick Michael 61283229, Mike Looijmans, Mastalski Bartosz
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.
Signed-off-by: Alexander Sverdlin <alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
---
Changes in v3:
- corrected argument of i2c_recover_bus()
Changes in v2:
- rebased on 110bc76729d4 of Linus's tree;
drivers/i2c/busses/i2c-davinci.c | 42 +++++++++++++++++--------------------
1 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index df96ab6..03afdee 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -350,29 +350,25 @@ static struct i2c_bus_recovery_info davinci_i2c_scl_recovery_info = {
/*
* 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;
- i2c_recover_bus(&dev->adapter);
- }
- }
- if (allow_sleep)
- schedule_timeout(1);
- }
+ unsigned long timeout = jiffies + dev->adapter.timeout;
+
+ do {
+ if (!(davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG) & DAVINCI_I2C_STR_BB))
+ return 0;
+ schedule_timeout_uninterruptible(1);
+ } while (time_before_eq(jiffies, timeout));
+
+ dev_warn(dev->dev, "timeout waiting for bus ready\n");
+ i2c_recover_bus(&dev->adapter);
+
+ /*
+ * if bus is still "busy" here, it's most probably a HW problem like
+ * short-circuit
+ */
+ if (davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG) & DAVINCI_I2C_STR_BB)
+ return -EIO;
return 0;
}
@@ -508,7 +504,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;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy()
[not found] ` <5576C6D5.9070800-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
@ 2015-06-10 13:14 ` Wolfram Sang
0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2015-06-10 13:14 UTC (permalink / raw
To: Alexander Sverdlin
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Kevin Hilman, Sekhar Nori,
Grygorii Strashko, Santosh Shilimkar,
Vishwanathrao Badarkhe, Manish, Murali Karicheri,
Lawnick Michael 61283229, Mike Looijmans, Mastalski Bartosz
[-- Attachment #1: Type: text/plain, Size: 516 bytes --]
On Tue, Jun 09, 2015 at 12:58:29PM +0200, 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.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
Applied to for-next, thanks!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-06-10 13:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-13 13:03 [PATCH v2] i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy() Alexander Sverdlin
[not found] ` <55534BA0.3000506-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
2015-05-14 11:19 ` Grygorii.Strashko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
2015-06-02 15:53 ` Wolfram Sang
2015-06-09 10:54 ` Alexander Sverdlin
2015-06-09 10:58 ` [PATCH v3] " Alexander Sverdlin
[not found] ` <5576C6D5.9070800-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
2015-06-10 13:14 ` Wolfram Sang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.