* [RFC] I2C-MPC: Fix up error handling
@ 2006-06-09 7:54 Jean Delvare
2006-07-30 11:14 ` Jean Delvare
0 siblings, 1 reply; 4+ messages in thread
From: Jean Delvare @ 2006-06-09 7:54 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sylvain Munaut
Hi all,
Could anyone please comment on this i2c-mpc patch, and give it some
testing on different systems? I'm fine with the error propagation
fixes, but less fine with the random resets being added, and rather
unhappy with the "retry once more just in case" thing, which I think,
if really needed, should instead be implemented using the standard
i2c_adapter.retries mechanism.
Thanks.
From: Kumar Gala <galak@kernel.crashing.org>
* If we have an Unfinished (MCF) or Arbitration Lost (MAL) error and
the bus is still busy reset the controller. This prevents the
controller from getting in a hung state for transactions for other
devices.
* Fixed up propogating the errors from i2c_wait.
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Cc: Jean Delvare <khali@linux-fr.org>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
drivers/i2c/busses/i2c-mpc.c | 43 +++++++++++++++++++++++----------
1 file changed, 31 insertions(+), 12 deletions(-)
diff -puN drivers/i2c/busses/i2c-mpc.c~i2c-mpc-fix-up-error-handling drivers/i2c/busses/i2c-mpc.c
--- devel/drivers/i2c/busses/i2c-mpc.c~i2c-mpc-fix-up-error-handling 2006-06-01 20:22:55.000000000 -0700
+++ devel-akpm/drivers/i2c/busses/i2c-mpc.c 2006-06-01 20:22:55.000000000 -0700
@@ -115,11 +115,20 @@ static int i2c_wait(struct mpc_i2c *i2c,
if (!(x & CSR_MCF)) {
pr_debug("I2C: unfinished\n");
+
+ /* reset the controller if the bus is still busy */
+ if (x & CSR_MBB)
+ writeccr(i2c, 0);
+
return -EIO;
}
if (x & CSR_MAL) {
pr_debug("I2C: MAL\n");
+
+ /* reset the controller if the bus is still busy */
+ if (x & CSR_MBB)
+ writeccr(i2c, 0);
return -EIO;
}
@@ -160,7 +169,7 @@ static void mpc_i2c_stop(struct mpc_i2c
static int mpc_write(struct mpc_i2c *i2c, int target,
const u8 * data, int length, int restart)
{
- int i;
+ int i, ret;
unsigned timeout = i2c->adap.timeout;
u32 flags = restart ? CCR_RSTA : 0;
@@ -172,15 +181,17 @@ static int mpc_write(struct mpc_i2c *i2c
/* Write target byte */
writeb((target << 1), i2c->base + MPC_I2C_DR);
- if (i2c_wait(i2c, timeout, 1) < 0)
- return -1;
+ ret = i2c_wait(i2c, timeout, 1);
+ if (ret < 0)
+ return ret;
for (i = 0; i < length; i++) {
/* Write data byte */
writeb(data[i], i2c->base + MPC_I2C_DR);
- if (i2c_wait(i2c, timeout, 1) < 0)
- return -1;
+ ret = i2c_wait(i2c, timeout, 1);
+ if (ret < 0)
+ return ret;
}
return 0;
@@ -190,7 +201,7 @@ static int mpc_read(struct mpc_i2c *i2c,
u8 * data, int length, int restart)
{
unsigned timeout = i2c->adap.timeout;
- int i;
+ int i, ret;
u32 flags = restart ? CCR_RSTA : 0;
/* Start with MEN */
@@ -201,8 +212,9 @@ static int mpc_read(struct mpc_i2c *i2c,
/* Write target address byte - this time with the read flag set */
writeb((target << 1) | 1, i2c->base + MPC_I2C_DR);
- if (i2c_wait(i2c, timeout, 1) < 0)
- return -1;
+ ret = i2c_wait(i2c, timeout, 1);
+ if (ret < 0)
+ return ret;
if (length) {
if (length == 1)
@@ -214,8 +226,9 @@ static int mpc_read(struct mpc_i2c *i2c,
}
for (i = 0; i < length; i++) {
- if (i2c_wait(i2c, timeout, 0) < 0)
- return -1;
+ ret = i2c_wait(i2c, timeout, 0);
+ if (ret < 0)
+ return ret;
/* Generate txack on next to last byte */
if (i == length - 2)
@@ -246,8 +259,13 @@ static int mpc_xfer(struct i2c_adapter *
return -EINTR;
}
if (time_after(jiffies, orig_jiffies + HZ)) {
- pr_debug("I2C: timeout\n");
- return -EIO;
+ writeccr(i2c, 0);
+
+ /* try one more time before we error */
+ if (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
+ pr_debug("I2C: timeout\n");
+ return -EIO;
+ }
}
schedule();
}
@@ -325,6 +343,7 @@ static int fsl_i2c_probe(struct platform
goto fail_irq;
}
+ writeccr(i2c, 0);
mpc_i2c_setclock(i2c);
platform_set_drvdata(pdev, i2c);
_
--
Jean Delvare
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] I2C-MPC: Fix up error handling
2006-06-09 7:54 [RFC] I2C-MPC: Fix up error handling Jean Delvare
@ 2006-07-30 11:14 ` Jean Delvare
2006-07-30 16:23 ` Kumar Gala
0 siblings, 1 reply; 4+ messages in thread
From: Jean Delvare @ 2006-07-30 11:14 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sylvain Munaut
Quoting myself:
> Could anyone please comment on this i2c-mpc patch, and give it some
> testing on different systems? I'm fine with the error propagation
> fixes, but less fine with the random resets being added, and rather
> unhappy with the "retry once more just in case" thing, which I think,
> if really needed, should instead be implemented using the standard
> i2c_adapter.retries mechanism.
Can anyone please comment on this patch? Andrew Morton keeps forwarding
it to me, but I don't think I can accept it as is (see my complaints
above.)
> From: Kumar Gala <galak@kernel.crashing.org>
>
> * If we have an Unfinished (MCF) or Arbitration Lost (MAL) error and
> the bus is still busy reset the controller. This prevents the
> controller from getting in a hung state for transactions for other
> devices.
>
> * Fixed up propogating the errors from i2c_wait.
>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> Cc: Jean Delvare <khali@linux-fr.org>
> Cc: Greg KH <greg@kroah.com>
> Signed-off-by: Andrew Morton <akpm@osdl.org>
> ---
>
> drivers/i2c/busses/i2c-mpc.c | 43 +++++++++++++++++++++++----------
> 1 file changed, 31 insertions(+), 12 deletions(-)
>
> diff -puN drivers/i2c/busses/i2c-mpc.c~i2c-mpc-fix-up-error-handling drivers/i2c/busses/i2c-mpc.c
> --- devel/drivers/i2c/busses/i2c-mpc.c~i2c-mpc-fix-up-error-handling 2006-06-01 20:22:55.000000000 -0700
> +++ devel-akpm/drivers/i2c/busses/i2c-mpc.c 2006-06-01 20:22:55.000000000 -0700
> @@ -115,11 +115,20 @@ static int i2c_wait(struct mpc_i2c *i2c,
>
> if (!(x & CSR_MCF)) {
> pr_debug("I2C: unfinished\n");
> +
> + /* reset the controller if the bus is still busy */
> + if (x & CSR_MBB)
> + writeccr(i2c, 0);
> +
> return -EIO;
> }
>
> if (x & CSR_MAL) {
> pr_debug("I2C: MAL\n");
> +
> + /* reset the controller if the bus is still busy */
> + if (x & CSR_MBB)
> + writeccr(i2c, 0);
> return -EIO;
> }
>
> @@ -160,7 +169,7 @@ static void mpc_i2c_stop(struct mpc_i2c
> static int mpc_write(struct mpc_i2c *i2c, int target,
> const u8 * data, int length, int restart)
> {
> - int i;
> + int i, ret;
> unsigned timeout = i2c->adap.timeout;
> u32 flags = restart ? CCR_RSTA : 0;
>
> @@ -172,15 +181,17 @@ static int mpc_write(struct mpc_i2c *i2c
> /* Write target byte */
> writeb((target << 1), i2c->base + MPC_I2C_DR);
>
> - if (i2c_wait(i2c, timeout, 1) < 0)
> - return -1;
> + ret = i2c_wait(i2c, timeout, 1);
> + if (ret < 0)
> + return ret;
>
> for (i = 0; i < length; i++) {
> /* Write data byte */
> writeb(data[i], i2c->base + MPC_I2C_DR);
>
> - if (i2c_wait(i2c, timeout, 1) < 0)
> - return -1;
> + ret = i2c_wait(i2c, timeout, 1);
> + if (ret < 0)
> + return ret;
> }
>
> return 0;
> @@ -190,7 +201,7 @@ static int mpc_read(struct mpc_i2c *i2c,
> u8 * data, int length, int restart)
> {
> unsigned timeout = i2c->adap.timeout;
> - int i;
> + int i, ret;
> u32 flags = restart ? CCR_RSTA : 0;
>
> /* Start with MEN */
> @@ -201,8 +212,9 @@ static int mpc_read(struct mpc_i2c *i2c,
> /* Write target address byte - this time with the read flag set */
> writeb((target << 1) | 1, i2c->base + MPC_I2C_DR);
>
> - if (i2c_wait(i2c, timeout, 1) < 0)
> - return -1;
> + ret = i2c_wait(i2c, timeout, 1);
> + if (ret < 0)
> + return ret;
>
> if (length) {
> if (length == 1)
> @@ -214,8 +226,9 @@ static int mpc_read(struct mpc_i2c *i2c,
> }
>
> for (i = 0; i < length; i++) {
> - if (i2c_wait(i2c, timeout, 0) < 0)
> - return -1;
> + ret = i2c_wait(i2c, timeout, 0);
> + if (ret < 0)
> + return ret;
>
> /* Generate txack on next to last byte */
> if (i == length - 2)
> @@ -246,8 +259,13 @@ static int mpc_xfer(struct i2c_adapter *
> return -EINTR;
> }
> if (time_after(jiffies, orig_jiffies + HZ)) {
> - pr_debug("I2C: timeout\n");
> - return -EIO;
> + writeccr(i2c, 0);
> +
> + /* try one more time before we error */
> + if (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
> + pr_debug("I2C: timeout\n");
> + return -EIO;
> + }
> }
> schedule();
> }
> @@ -325,6 +343,7 @@ static int fsl_i2c_probe(struct platform
> goto fail_irq;
> }
>
> + writeccr(i2c, 0);
> mpc_i2c_setclock(i2c);
> platform_set_drvdata(pdev, i2c);
--
Jean Delvare
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] I2C-MPC: Fix up error handling
2006-07-30 11:14 ` Jean Delvare
@ 2006-07-30 16:23 ` Kumar Gala
2006-07-31 6:57 ` Jean Delvare
0 siblings, 1 reply; 4+ messages in thread
From: Kumar Gala @ 2006-07-30 16:23 UTC (permalink / raw)
To: Jean Delvare; +Cc: linuxppc-dev, Sylvain Munaut
On Jul 30, 2006, at 6:14 AM, Jean Delvare wrote:
> Quoting myself:
>> Could anyone please comment on this i2c-mpc patch, and give it some
>> testing on different systems? I'm fine with the error propagation
>> fixes, but less fine with the random resets being added, and rather
>> unhappy with the "retry once more just in case" thing, which I think,
>> if really needed, should instead be implemented using the standard
>> i2c_adapter.retries mechanism.
>
> Can anyone please comment on this patch? Andrew Morton keeps
> forwarding
> it to me, but I don't think I can accept it as is (see my complaints
> above.)
If I find some time, let me split out the error propagation so we can
get that fix up stream. The other code was to handle a buggy device
that was connected in one system I was working on.
- kumar
>> From: Kumar Gala <galak@kernel.crashing.org>
>>
>> * If we have an Unfinished (MCF) or Arbitration Lost (MAL) error and
>> the bus is still busy reset the controller. This prevents the
>> controller from getting in a hung state for transactions for other
>> devices.
>>
>> * Fixed up propogating the errors from i2c_wait.
>>
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>> Cc: Jean Delvare <khali@linux-fr.org>
>> Cc: Greg KH <greg@kroah.com>
>> Signed-off-by: Andrew Morton <akpm@osdl.org>
>> ---
>>
>> drivers/i2c/busses/i2c-mpc.c | 43 ++++++++++++++++++++++
>> +----------
>> 1 file changed, 31 insertions(+), 12 deletions(-)
>>
>> diff -puN drivers/i2c/busses/i2c-mpc.c~i2c-mpc-fix-up-error-
>> handling drivers/i2c/busses/i2c-mpc.c
>> --- devel/drivers/i2c/busses/i2c-mpc.c~i2c-mpc-fix-up-error-
>> handling 2006-06-01 20:22:55.000000000 -0700
>> +++ devel-akpm/drivers/i2c/busses/i2c-mpc.c 2006-06-01
>> 20:22:55.000000000 -0700
>> @@ -115,11 +115,20 @@ static int i2c_wait(struct mpc_i2c *i2c,
>>
>> if (!(x & CSR_MCF)) {
>> pr_debug("I2C: unfinished\n");
>> +
>> + /* reset the controller if the bus is still busy */
>> + if (x & CSR_MBB)
>> + writeccr(i2c, 0);
>> +
>> return -EIO;
>> }
>>
>> if (x & CSR_MAL) {
>> pr_debug("I2C: MAL\n");
>> +
>> + /* reset the controller if the bus is still busy */
>> + if (x & CSR_MBB)
>> + writeccr(i2c, 0);
>> return -EIO;
>> }
>>
>> @@ -160,7 +169,7 @@ static void mpc_i2c_stop(struct mpc_i2c
>> static int mpc_write(struct mpc_i2c *i2c, int target,
>> const u8 * data, int length, int restart)
>> {
>> - int i;
>> + int i, ret;
>> unsigned timeout = i2c->adap.timeout;
>> u32 flags = restart ? CCR_RSTA : 0;
>>
>> @@ -172,15 +181,17 @@ static int mpc_write(struct mpc_i2c *i2c
>> /* Write target byte */
>> writeb((target << 1), i2c->base + MPC_I2C_DR);
>>
>> - if (i2c_wait(i2c, timeout, 1) < 0)
>> - return -1;
>> + ret = i2c_wait(i2c, timeout, 1);
>> + if (ret < 0)
>> + return ret;
>>
>> for (i = 0; i < length; i++) {
>> /* Write data byte */
>> writeb(data[i], i2c->base + MPC_I2C_DR);
>>
>> - if (i2c_wait(i2c, timeout, 1) < 0)
>> - return -1;
>> + ret = i2c_wait(i2c, timeout, 1);
>> + if (ret < 0)
>> + return ret;
>> }
>>
>> return 0;
>> @@ -190,7 +201,7 @@ static int mpc_read(struct mpc_i2c *i2c,
>> u8 * data, int length, int restart)
>> {
>> unsigned timeout = i2c->adap.timeout;
>> - int i;
>> + int i, ret;
>> u32 flags = restart ? CCR_RSTA : 0;
>>
>> /* Start with MEN */
>> @@ -201,8 +212,9 @@ static int mpc_read(struct mpc_i2c *i2c,
>> /* Write target address byte - this time with the read flag set */
>> writeb((target << 1) | 1, i2c->base + MPC_I2C_DR);
>>
>> - if (i2c_wait(i2c, timeout, 1) < 0)
>> - return -1;
>> + ret = i2c_wait(i2c, timeout, 1);
>> + if (ret < 0)
>> + return ret;
>>
>> if (length) {
>> if (length == 1)
>> @@ -214,8 +226,9 @@ static int mpc_read(struct mpc_i2c *i2c,
>> }
>>
>> for (i = 0; i < length; i++) {
>> - if (i2c_wait(i2c, timeout, 0) < 0)
>> - return -1;
>> + ret = i2c_wait(i2c, timeout, 0);
>> + if (ret < 0)
>> + return ret;
>>
>> /* Generate txack on next to last byte */
>> if (i == length - 2)
>> @@ -246,8 +259,13 @@ static int mpc_xfer(struct i2c_adapter *
>> return -EINTR;
>> }
>> if (time_after(jiffies, orig_jiffies + HZ)) {
>> - pr_debug("I2C: timeout\n");
>> - return -EIO;
>> + writeccr(i2c, 0);
>> +
>> + /* try one more time before we error */
>> + if (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
>> + pr_debug("I2C: timeout\n");
>> + return -EIO;
>> + }
>> }
>> schedule();
>> }
>> @@ -325,6 +343,7 @@ static int fsl_i2c_probe(struct platform
>> goto fail_irq;
>> }
>>
>> + writeccr(i2c, 0);
>> mpc_i2c_setclock(i2c);
>> platform_set_drvdata(pdev, i2c);
>
> --
> Jean Delvare
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] I2C-MPC: Fix up error handling
2006-07-30 16:23 ` Kumar Gala
@ 2006-07-31 6:57 ` Jean Delvare
0 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2006-07-31 6:57 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Munaut, Sylvain
> > Quoting myself:
> > > Could anyone please comment on this i2c-mpc patch, and give it some
> > > testing on different systems? I'm fine with the error propagation
> > > fixes, but less fine with the random resets being added, and rather
> > > unhappy with the "retry once more just in case" thing, which I think,
> > > if really needed, should instead be implemented using the standard
> > > i2c_adapter.retries mechanism.
> >
> > Can anyone please comment on this patch? Andrew Morton keeps
> > forwarding it to me, but I don't think I can accept it as is (see
> > my complaints above.)
>
> If I find some time, let me split out the error propagation so we can
> get that fix up stream. The other code was to handle a buggy device
> that was connected in one system I was working on.
Sounds like a good plan, thanks.
--
Jean Delvare
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-07-31 6:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-09 7:54 [RFC] I2C-MPC: Fix up error handling Jean Delvare
2006-07-30 11:14 ` Jean Delvare
2006-07-30 16:23 ` Kumar Gala
2006-07-31 6:57 ` Jean Delvare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).