2009-01-05 13:21:18

by Clifford Wolf

[permalink] [raw]
Subject: [PATCH] Incremental i2c-mpc driver fix for multi-master i2c busses.

Incremental i2c-mpc driver fix for multi-master i2c busses.

This is an incremental bugfix for the i2c-mpc driver. It is based
on the bugfix I've sent on 2008-12-22:

http://lkml.org/lkml/2008/12/22/99

There still was a remaining problem with multi-master i2c busses
when an i2c bus access is interrupted by a unix signal while
waiting for bus arbitration.

This is an extreamly rare case but I managed to stumble over it in
multi master i2c performance tests.

Tested with a freescale MPC8349E host cpu.

Signed-off-by: Clifford Wolf <[email protected]>

--- drivers/i2c/busses/i2c-mpc.c (revision 2216)
+++ drivers/i2c/busses/i2c-mpc.c (working copy)
@@ -105,7 +105,7 @@
schedule();
if (time_after(jiffies, orig_jiffies + timeout)) {
pr_debug("I2C: timeout\n");
- writeccr(i2c, 0);
+ writeccr(i2c, CCR_MEN);
result = -EIO;
break;
}
@@ -119,10 +119,10 @@

if (unlikely(result < 0)) {
pr_debug("I2C: wait interrupted\n");
- writeccr(i2c, 0);
+ writeccr(i2c, CCR_MEN);
} else if (unlikely(!(i2c->interrupt & CSR_MIF))) {
pr_debug("I2C: wait timeout\n");
- writeccr(i2c, 0);
+ writeccr(i2c, CCR_MEN);
result = -ETIMEDOUT;
}

@@ -267,7 +267,7 @@
while (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
if (signal_pending(current)) {
pr_debug("I2C: Interrupted\n");
- writeccr(i2c, 0);
+ writeccr(i2c, CCR_MEN);
return -EINTR;
}
if (time_after(jiffies, orig_jiffies + HZ)) {
@@ -369,6 +369,15 @@
goto fail_add;
}

+ /* Make sure the adapter is already in master mode some time before
+ * the first xfer to make sure arbitration control had enough time
+ * to monitor the bus. The CCR_MEN flag should never be cleared without
+ * makeing sure there is some delay between setting it again and
+ * starting the first transfer..
+ */
+ writeccr(i2c, CCR_MEN);
+ udelay(100);
+
return result;

fail_add:

--
The great thing about Object Oriented code is that it can make small,
simple problems look like large, complex ones.


2009-01-06 10:24:20

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH] Incremental i2c-mpc driver fix for multi-master i2c busses.

On Mon, Jan 05, 2009 at 02:21:02PM +0100, Clifford Wolf wrote:
> Incremental i2c-mpc driver fix for multi-master i2c busses.

Hi, you missed [email protected] which is the new home
for the i2c list. You should have received an auto-reply about
how this list has moved.

I'll probably try and get another merge out sometime this week.

> This is an incremental bugfix for the i2c-mpc driver. It is based
> on the bugfix I've sent on 2008-12-22:
>
> http://lkml.org/lkml/2008/12/22/99
>
> There still was a remaining problem with multi-master i2c busses
> when an i2c bus access is interrupted by a unix signal while
> waiting for bus arbitration.
>
> This is an extreamly rare case but I managed to stumble over it in
> multi master i2c performance tests.
>
> Tested with a freescale MPC8349E host cpu.
>
> Signed-off-by: Clifford Wolf <[email protected]>
>
> --- drivers/i2c/busses/i2c-mpc.c (revision 2216)
> +++ drivers/i2c/busses/i2c-mpc.c (working copy)
> @@ -105,7 +105,7 @@
> schedule();
> if (time_after(jiffies, orig_jiffies + timeout)) {
> pr_debug("I2C: timeout\n");
> - writeccr(i2c, 0);
> + writeccr(i2c, CCR_MEN);
> result = -EIO;
> break;
> }
> @@ -119,10 +119,10 @@
>
> if (unlikely(result < 0)) {
> pr_debug("I2C: wait interrupted\n");
> - writeccr(i2c, 0);
> + writeccr(i2c, CCR_MEN);
> } else if (unlikely(!(i2c->interrupt & CSR_MIF))) {
> pr_debug("I2C: wait timeout\n");
> - writeccr(i2c, 0);
> + writeccr(i2c, CCR_MEN);
> result = -ETIMEDOUT;
> }
>
> @@ -267,7 +267,7 @@
> while (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
> if (signal_pending(current)) {
> pr_debug("I2C: Interrupted\n");
> - writeccr(i2c, 0);
> + writeccr(i2c, CCR_MEN);
> return -EINTR;
> }
> if (time_after(jiffies, orig_jiffies + HZ)) {
> @@ -369,6 +369,15 @@
> goto fail_add;
> }
>
> + /* Make sure the adapter is already in master mode some time before
> + * the first xfer to make sure arbitration control had enough time
> + * to monitor the bus. The CCR_MEN flag should never be cleared without
> + * makeing sure there is some delay between setting it again and
> + * starting the first transfer..
> + */
> + writeccr(i2c, CCR_MEN);
> + udelay(100);
> +
> return result;
>
> fail_add:

--
Ben ([email protected], http://www.fluff.org/)

'a smiley only costs 4 bytes'

2009-01-12 22:14:16

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Incremental i2c-mpc driver fix for multi-master i2c busses.

On Mon, 5 Jan 2009 14:21:02 +0100
Clifford Wolf <[email protected]> wrote:

> Incremental i2c-mpc driver fix for multi-master i2c busses.
>
> This is an incremental bugfix for the i2c-mpc driver. It is based
> on the bugfix I've sent on 2008-12-22:
>
> http://lkml.org/lkml/2008/12/22/99
>
> There still was a remaining problem with multi-master i2c busses
> when an i2c bus access is interrupted by a unix signal while
> waiting for bus arbitration.
>
> This is an extreamly rare case but I managed to stumble over it in
> multi master i2c performance tests.
>
> Tested with a freescale MPC8349E host cpu.

That's not a very good changelog - it has basically no information,
apart from the linked-to original changelog.

And the linked-to changelog has no description of the bug which is
being fixed.

> --- drivers/i2c/busses/i2c-mpc.c (revision 2216)
> +++ drivers/i2c/busses/i2c-mpc.c (working copy)

Please prepare patches in `patch -p1' form:

--- a/drivers/i2c/busses/i2c-mpc.c
+++ a/drivers/i2c/busses/i2c-mpc.c