This fixes MAL (arbitration lost) bug caused by illegal use of
RSTA (repeated START) after STOP condition generated after last byte
of reads.
Signed-off-by: Esben Haabendal <redacted>
---
drivers/i2c/busses/i2c-mpc.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
From: Wolfram Sang <hidden> Date: 2009-05-14 08:35:02
On Thu, May 14, 2009 at 10:10:03AM +0200, Esben Haabendal wrote:
This fixes MAL (arbitration lost) bug caused by illegal use of
RSTA (repeated START) after STOP condition generated after last byte
of reads.
Could you elaborate a bit, please? Like an example when the original bug
occured (so I could reproduce easily) and how this patch actually solves
the problem.
@@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs, int num)
}
for (i = 0; ret >= 0 && i < num; i++) {
+ int restart = i;
pmsg = &msgs[i];
dev_dbg(i2c->dev,
"Doing %s %d bytes to 0x%02x - %d of %d messages\n",
pmsg->flags & I2C_M_RD ? "read" : "write",
pmsg->len, pmsg->addr, i + 1, num);
+ if (i > 0 && ((pmsg-1)->flags & I2C_M_RD))
CodingStyle: Spaces around '-'
+ restart = 0;
if (pmsg->flags & I2C_M_RD)
ret =
- mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+ restart);
else
ret =
- mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+ restart);
}
mpc_i2c_stop(i2c);
return (ret < 0) ? ret : num;
--
1.6.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 14, 2009 at 10:10:03AM +0200, Esben Haabendal wrote:
quoted
This fixes MAL (arbitration lost) bug caused by illegal use of
RSTA (repeated START) after STOP condition generated after last byte
of reads.
Could you elaborate a bit, please? Like an example when the original bug
occured (so I could reproduce easily) and how this patch actually solves
the problem.
Sure. I used the following code to initialize a CS4265 chip:
(simplified here for the example)
static int
i2c_init_chip(void)
{
int err=0;
char write_dac_control_1[] = { 0x03, chip->regs.dac_control_1 };
char seek_chip_id[] = { 0x01 };
char write_power_control[] = { 0x02, chip->regs.power_control };
struct i2c_msg cs4265_init_cmds[] = {
{ chip->i2c_addr, 0, 2, write_dac_control_1 },
{ chip->i2c_addr, 0, 1, seek_chip_id },
{ chip->i2c_addr, I2C_M_RD, 1, ®s.chip_id },
{ chip->i2c_addr, 0, 2, write_power_control },
};
err = i2c_transfer(i2c, cs4265_init_cmds,
sizeof(cs4265_init_cmds)/sizeof(*cs4265_init_cmds));
if (err < 0) {
printk(KERN_ERR "i2c_transfer failed: %d\n", err);
return err;
}
return 0;
}
I have added some additional debug output and traces something like this:
[ 36.114297] writeccr 80 [MEN]
[ 36.114324] mpc_xfer: 2 bytes to 4f:W - 1 of 5 messages
[ 36.120176] next 1 bytes to 4f:W
[ 36.124159] mpc_write addr=4f len=2
[ 36.128044] writeccr 80 [MEN]
[ 36.128062] writeccr f0 [MEN MIEN MSTA MTX] -> START
[ 36.128219] I2C: SR=a2
[ 36.131528] I2C: SR=a6
[ 36.134406] I2C: SR=a2
[ 36.137165] mpc_xfer: 1 bytes to 4f:W - 2 of 5 messages
[ 36.142802] next 1 bytes to 4f:R
[ 36.146699] mpc_write addr=4f len=1
[ 36.150583] writeccr f4 [MEN MIEN MSTA MTX RSTA] -> repeated START
[ 36.150758] I2C: SR=a2
[ 36.153851] I2C: SR=a6
[ 36.156236] mpc_xfer: 1 bytes to 4f:R - 3 of 5 messages
[ 36.162081] next 2 bytes to 4f:W
[ 36.166062] mpc_read addr=4f len=1
[ 36.169858] writeccr f4 [MEN MIEN MSTA MTX RSTA] -> repeated START
[ 36.170031] I2C: SR=a6
[ 36.172993] writeccr e8 [MEN MIEN MSTA TXAK] -> receive & no ACK
[ 36.173143] I2C: SR=a7
[ 36.175511] writeccr c8 [MEN MIEN TXAK] -> STOP
[ 36.175529] mpc_xfer: 2 bytes to 4f:W - 4 of 5 messages
[ 36.181804] next 2 bytes to 4f:W
[ 36.185788] mpc_write addr=4f len=2
[ 36.189672] writeccr f4 [MEN MIEN MSTA MTX RSTA] -> repeated START
[ 36.189704] I2C: SR=b7
[ 36.192072] I2C: MAL
[ 36.195075] i2c_wait(address) error: -5
[ 36.199311] writeccr 80
The problem is that after the STOP condition, the following i2c_msg will be
attempted with a repeated START, which according to specification will
cause a MAL, which also happens. My MPC8360ERM reads:
"Attempting a repeated START at the wrong time (or if the bus is owned
by another master), results in loss of arbitration."
Which I know read as it that we must own the I2C bus when using RSTA.
@@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs, int num)
}
for (i = 0; ret >= 0 && i < num; i++) {
+ int restart = i;
pmsg = &msgs[i];
dev_dbg(i2c->dev,
"Doing %s %d bytes to 0x%02x - %d of %d messages\n",
pmsg->flags & I2C_M_RD ? "read" : "write",
pmsg->len, pmsg->addr, i + 1, num);
+ if (i > 0 && ((pmsg-1)->flags & I2C_M_RD))
CodingStyle: Spaces around '-'
Ok.
quoted
+ restart = 0;
if (pmsg->flags & I2C_M_RD)
ret =
- mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+ restart);
else
ret =
- mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+ restart);
}
mpc_i2c_stop(i2c);
return (ret < 0) ? ret : num;
--
1.6.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 14, 2009 at 10:10:03AM +0200, Esben Haabendal wrote:
quoted
This fixes MAL (arbitration lost) bug caused by illegal use of
RSTA (repeated START) after STOP condition generated after last byte
of reads.
Could you elaborate a bit, please? Like an example when the original bug
occured (so I could reproduce easily) and how this patch actually solves
the problem.
Sure. I used the following code to initialize a CS4265 chip:
(simplified here for the example)
static int
i2c_init_chip(void)
{
int err=0;
char write_dac_control_1[] = { 0x03, chip->regs.dac_control_1 };
char seek_chip_id[] = { 0x01 };
char write_power_control[] = { 0x02, chip->regs.power_control };
struct i2c_msg cs4265_init_cmds[] = {
{ chip->i2c_addr, 0, 2, write_dac_control_1 },
{ chip->i2c_addr, 0, 1, seek_chip_id },
{ chip->i2c_addr, I2C_M_RD, 1, ®s.chip_id },
{ chip->i2c_addr, 0, 2, write_power_control },
};
err = i2c_transfer(i2c, cs4265_init_cmds,
sizeof(cs4265_init_cmds)/sizeof(*cs4265_init_cmds));
if (err < 0) {
printk(KERN_ERR "i2c_transfer failed: %d\n", err);
return err;
}
return 0;
}
I have added some additional debug output and traces something like this:
[ 36.114297] writeccr 80 [MEN]
[ 36.114324] mpc_xfer: 2 bytes to 4f:W - 1 of 5 messages
[ 36.120176] next 1 bytes to 4f:W
[ 36.124159] mpc_write addr=4f len=2
[ 36.128044] writeccr 80 [MEN]
[ 36.128062] writeccr f0 [MEN MIEN MSTA MTX] -> START
[ 36.128219] I2C: SR=a2
[ 36.131528] I2C: SR=a6
[ 36.134406] I2C: SR=a2
[ 36.137165] mpc_xfer: 1 bytes to 4f:W - 2 of 5 messages
[ 36.142802] next 1 bytes to 4f:R
[ 36.146699] mpc_write addr=4f len=1
[ 36.150583] writeccr f4 [MEN MIEN MSTA MTX RSTA] -> repeated START
[ 36.150758] I2C: SR=a2
[ 36.153851] I2C: SR=a6
[ 36.156236] mpc_xfer: 1 bytes to 4f:R - 3 of 5 messages
[ 36.162081] next 2 bytes to 4f:W
[ 36.166062] mpc_read addr=4f len=1
[ 36.169858] writeccr f4 [MEN MIEN MSTA MTX RSTA] -> repeated START
[ 36.170031] I2C: SR=a6
[ 36.172993] writeccr e8 [MEN MIEN MSTA TXAK] -> receive & no ACK
[ 36.173143] I2C: SR=a7
[ 36.175511] writeccr c8 [MEN MIEN TXAK] -> STOP
[ 36.175529] mpc_xfer: 2 bytes to 4f:W - 4 of 5 messages
[ 36.181804] next 2 bytes to 4f:W
[ 36.185788] mpc_write addr=4f len=2
[ 36.189672] writeccr f4 [MEN MIEN MSTA MTX RSTA] -> repeated START
[ 36.189704] I2C: SR=b7
[ 36.192072] I2C: MAL
[ 36.195075] i2c_wait(address) error: -5
[ 36.199311] writeccr 80
The problem is that after the STOP condition, the following i2c_msg will be
attempted with a repeated START, which according to specification will
cause a MAL, which also happens. My MPC8360ERM reads:
"Attempting a repeated START at the wrong time (or if the bus is owned
by another master), results in loss of arbitration."
Which I know read as it that we must own the I2C bus when using RSTA.
I agree with the theory, but isn't the problem here that STOP is performed in
the middle of this transaction?
Remove the STOP and RSTA will work I think.
Jocke