* [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg
@ 2009-05-14 8:10 Esben Haabendal
[not found] ` <4A0BD1DB.4090305-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Esben Haabendal @ 2009-05-14 8:10 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A
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 <eha-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org>
---
drivers/i2c/busses/i2c-mpc.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 4af5c09..9fe05d9 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -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))
+ 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
^ permalink raw reply related [flat|nested] 9+ messages in thread[parent not found: <4A0BD1DB.4090305-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org>]
* Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg [not found] ` <4A0BD1DB.4090305-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org> @ 2009-05-14 8:34 ` Wolfram Sang [not found] ` <20090514083453.GB3043-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Wolfram Sang @ 2009-05-14 8:34 UTC (permalink / raw) To: Esben Haabendal Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A [-- Attachment #1: Type: text/plain, Size: 2105 bytes --] 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. > Signed-off-by: Esben Haabendal <eha-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org> > --- > drivers/i2c/busses/i2c-mpc.c | 9 +++++++-- > 1 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c > index 4af5c09..9fe05d9 100644 > --- a/drivers/i2c/busses/i2c-mpc.c > +++ b/drivers/i2c/busses/i2c-mpc.c > @@ -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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20090514083453.GB3043-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg [not found] ` <20090514083453.GB3043-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> @ 2009-05-14 8:48 ` Esben Haabendal 2009-05-14 9:58 ` Joakim Tjernlund 0 siblings, 1 reply; 9+ messages in thread From: Esben Haabendal @ 2009-05-14 8:48 UTC (permalink / raw) To: Wolfram Sang Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A Wolfram Sang wrote: > 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. > 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. > > >> Signed-off-by: Esben Haabendal <eha-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org> >> --- >> drivers/i2c/busses/i2c-mpc.c | 9 +++++++-- >> 1 files changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c >> index 4af5c09..9fe05d9 100644 >> --- a/drivers/i2c/busses/i2c-mpc.c >> +++ b/drivers/i2c/busses/i2c-mpc.c >> @@ -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. > >> + 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-14 8:48 ` Esben Haabendal @ 2009-05-14 9:58 ` Joakim Tjernlund [not found] ` <d2b9ea600905140317u7a4b9a70s574568d79a1ce7cc@mail.gmail.com> 0 siblings, 1 reply; 9+ messages in thread From: Joakim Tjernlund @ 2009-05-14 9:58 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c > > Wolfram Sang wrote: > > 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. > > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <d2b9ea600905140317u7a4b9a70s574568d79a1ce7cc@mail.gmail.com>]
[parent not found: <OF3CD158BA.04CCFBB5-ONC12575B6.005911D5-C12575B6.005CAFC2@transmode.se>]
[parent not found: <OF3CD158BA.04CCFBB5-ONC12575B6.005911D5-C12575B6.005CAFC2-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org>]
* Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg [not found] ` <OF3CD158BA.04CCFBB5-ONC12575B6.005911D5-C12575B6.005CAFC2-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> @ 2009-05-15 10:18 ` Esben Haabendal [not found] ` <d2b9ea600905150318s69b86b5ct3816b274bf343a79-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Esben Haabendal @ 2009-05-15 10:18 UTC (permalink / raw) To: Joakim Tjernlund Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA Hi Your patch (and the small addition to mpc_i2c_start) does not work for me. [ 35.765803] mpc_xfer() [ 35.785480] writeccr 0 [ 35.785505] writeccr 80 [ 35.785523] mpc_xfer: 1 bytes to 2c:W - 1 of 2 messages [ 35.798817] mpc_write addr=2c len=1 restart=0 [ 35.815327] writeccr f0 [ 35.815503] I2C: SR=a2 [ 35.818675] I2C: SR=a6 [ 35.821450] mpc_xfer: 1 bytes to 2c:R - 2 of 2 messages [ 35.827119] mpc_read addr=2c len=1 restart=1 [ 35.837463] writeccr f4 [ 35.837641] I2C: SR=a6 [ 35.840011] writeccr e8 [ 35.840133] I2C: SR=a3 [ 35.843596] writeccr 80 [ 35.843632] mpc_xfer() [ 35.855068] writeccr 0 [ 35.855093] writeccr 80 [ 35.855111] mpc_xfer: 1 bytes to 2c:W - 1 of 2 messages [ 35.865346] mpc_write addr=2c len=1 restart=0 [ 35.870109] writeccr f0 [ 35.870272] I2C: SR=a2 [ 35.873372] I2C: SR=a6 [ 35.875757] mpc_xfer: 1 bytes to 2c:R - 2 of 2 messages [ 35.881606] mpc_read addr=2c len=1 restart=1 [ 35.886290] writeccr f4 [ 35.886463] I2C: SR=a6 [ 35.889425] writeccr e8 [ 35.889575] I2C: SR=a7 [ 35.891944] writeccr 80 [ 35.961177] mpc_xfer() [ 35.972517] writeccr 0 [ 35.972541] writeccr 80 [ 35.972559] mpc_xfer: 1 bytes to 4e:W - 1 of 20 messages [ 35.982628] mpc_write addr=4e len=1 restart=0 [ 35.987389] writeccr f0 [ 35.987424] I2C: SR=b3 [ 35.990386] I2C: MAL [ 35.992971] i2c_wait(address) error: -5 [ 35.997215] writeccr 80 [ 35.997241] Error: i2c_transfer failed: -5 I have now spent a few hours trying a lot of different paths to fix this approach, but I simply cannot find a way to get i2c read to work without a trailing STOP condition with this controller. Is there a problem in applying my patch, as it should be "clean" improvement over the current implementation, which uses STOP condition, but does not work. Until Isomeone finds a way to get it to work without STOP, we will have a working driver, which I assume is what we want. Best regards, Esben Haabendal -- Esben Haabendal, Senior Software Consultant DoréDevelopment ApS, Ved Stranden 1, 9560 Hadsund, DK-Denmark Phone: +45 51 92 53 93, E-mail: eha-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org WWW: http://www.doredevelopment.dk ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <d2b9ea600905150318s69b86b5ct3816b274bf343a79-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg [not found] ` <d2b9ea600905150318s69b86b5ct3816b274bf343a79-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2009-05-15 11:05 ` Joakim Tjernlund [not found] ` <OFCADC684E.A2599121-ONC12575B7.003B8AB5-C12575B7.003CF795-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Joakim Tjernlund @ 2009-05-15 11:05 UTC (permalink / raw) To: Esben Haabendal Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A Esben Haabendal <esbenhaabendal-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote on 15/05/2009 12:18:39: > > I have now spent a few hours trying a lot of different paths to fix > this approach, but I simply cannot find a way to get i2c read to work > without a trailing STOP condition with this controller. I found a bug which lets me remove the "fix" and seems related to your problem. Updated patch last in mail > > Is there a problem in applying my patch, as it should be "clean" > improvement over the current implementation, which uses STOP > condition, but does not work. > > Until Isomeone finds a way to get it to work without STOP, we will > have a working driver, which I assume is what we want. Your patch appears OK too. I just wanted to see if a better fix was possible. Your patch is less risky and it is the safe bet so soon before release. Jocke diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index 6c1cddd..04eff40 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -189,9 +189,6 @@ static int mpc_write(struct mpc_i2c *i2c, int target, unsigned timeout = i2c->adap.timeout; u32 flags = restart ? CCR_RSTA : 0; - /* Start with MEN */ - if (!restart) - writeccr(i2c, CCR_MEN); /* Start as master */ writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags); /* Write target byte */ @@ -214,15 +211,12 @@ static int mpc_write(struct mpc_i2c *i2c, int target, } static int mpc_read(struct mpc_i2c *i2c, int target, - u8 * data, int length, int restart) + u8 * data, int length, int restart, int last) { unsigned timeout = i2c->adap.timeout; int i, result; u32 flags = restart ? CCR_RSTA : 0; - /* Start with MEN */ - if (!restart) - writeccr(i2c, CCR_MEN); /* Switch to read - restart */ writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags); /* Write target address byte - this time with the read flag set */ @@ -241,18 +235,18 @@ static int mpc_read(struct mpc_i2c *i2c, int target, readb(i2c->base + MPC_I2C_DR); } - for (i = 0; i < length; i++) { + for (i = length; i ; --i) { result = i2c_wait(i2c, timeout, 0); if (result < 0) return result; /* Generate txack on next to last byte */ - if (i == length - 2) + if (i == 2) writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_TXAK); - /* Generate stop on last byte */ - if (i == length - 1) - writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_TXAK); - data[i] = readb(i2c->base + MPC_I2C_DR); + /* Generate stop on last byte, iff last transaction */ + if (i == 1 && last) + writeccr(i2c, CCR_MIEN | CCR_MEN); + data[length-i] = readb(i2c->base + MPC_I2C_DR); } return length; @@ -294,7 +288,7 @@ static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) tm_i2c_select_mux(pmsg->addr); 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, i, i == num-1); else ret = mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i); ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <OFCADC684E.A2599121-ONC12575B7.003B8AB5-C12575B7.003CF795-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org>]
* Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg [not found] ` <OFCADC684E.A2599121-ONC12575B7.003B8AB5-C12575B7.003CF795-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> @ 2009-05-15 12:52 ` Esben Haabendal [not found] ` <d2b9ea600905150552u6951a92es5dd119dcc37e641d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Esben Haabendal @ 2009-05-15 12:52 UTC (permalink / raw) To: Joakim Tjernlund Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A Your new patch also does not work. Have you tested it? I already tried something very much what you sent here, I believe the only difference was that I named the "last" variable "stop". I also tried several other aproaches, and none of them worked. I would appreciate not to have to test all of them seperately again through this mailing list ;-) Anyway, your patch also is in conflict with the MPC8360ERM. The spec specifies that a repeated start must follow an ACK, and not a "NO ACK". When doing a repeated start after a NO ACK, the slave does not ACK the address (I get an RXAK). When doing as specified, ACK'ing the last byte read and then doing a repeated START, i2c_wait() fails due to CSR_MCF bit missing. I thought it would be possible to find somewhere to do a dummy read to get around this, but failed to do so without breaking something else. Could we go forward with my initial patch, and then continue the work on this repeated START approach for future releases? /Esben ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <d2b9ea600905150552u6951a92es5dd119dcc37e641d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg [not found] ` <d2b9ea600905150552u6951a92es5dd119dcc37e641d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2009-05-15 14:16 ` Joakim Tjernlund 2009-05-18 11:04 ` Wolfram Sang 1 sibling, 0 replies; 9+ messages in thread From: Joakim Tjernlund @ 2009-05-15 14:16 UTC (permalink / raw) To: Esben Haabendal Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A Esben Haabendal <esbenhaabendal-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote on 15/05/2009 14:52:28: > > Your new patch also does not work. > > Have you tested it? sure, works fine. I haven't stressed it too much though. > > I already tried something very much what you sent here, I believe the > only difference was that I named the "last" variable "stop". I also > tried several other aproaches, and none of them worked. I would > appreciate not to have to test all of them seperately again through > this mailing list ;-) :), point taken. > > Anyway, your patch also is in conflict with the MPC8360ERM. The spec > specifies that a repeated start must follow an ACK, and not a "NO > ACK". Ouch, will have to check too, but later. > > When doing a repeated start after a NO ACK, the slave does not ACK the > address (I get an RXAK). When doing as specified, ACK'ing the last > byte read and then doing a repeated START, i2c_wait() fails due to > CSR_MCF bit missing. I thought it would be possible to find somewhere > to do a dummy read to get around this, but failed to do so without > breaking something else. > > Could we go forward with my initial patch, and then continue the work > on this repeated START approach for future releases? Yes, go ahead. Jocke ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg [not found] ` <d2b9ea600905150552u6951a92es5dd119dcc37e641d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2009-05-15 14:16 ` Joakim Tjernlund @ 2009-05-18 11:04 ` Wolfram Sang 1 sibling, 0 replies; 9+ messages in thread From: Wolfram Sang @ 2009-05-18 11:04 UTC (permalink / raw) To: Esben Haabendal Cc: Joakim Tjernlund, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 497 bytes --] > Could we go forward with my initial patch, and then continue the work > on this repeated START approach for future releases? Then could you please send another version of your patch with the CodingStyle issue removed and an updated description what this patch fixes and what still needs to be resolved? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-05-18 11:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-14 8:10 [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg Esben Haabendal
[not found] ` <4A0BD1DB.4090305-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org>
2009-05-14 8:34 ` Wolfram Sang
[not found] ` <20090514083453.GB3043-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2009-05-14 8:48 ` Esben Haabendal
2009-05-14 9:58 ` Joakim Tjernlund
[not found] ` <d2b9ea600905140317u7a4b9a70s574568d79a1ce7cc@mail.gmail.com>
[not found] ` <OF3CD158BA.04CCFBB5-ONC12575B6.005911D5-C12575B6.005CAFC2@transmode.se>
[not found] ` <OF3CD158BA.04CCFBB5-ONC12575B6.005911D5-C12575B6.005CAFC2-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org>
2009-05-15 10:18 ` Esben Haabendal
[not found] ` <d2b9ea600905150318s69b86b5ct3816b274bf343a79-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-05-15 11:05 ` Joakim Tjernlund
[not found] ` <OFCADC684E.A2599121-ONC12575B7.003B8AB5-C12575B7.003CF795-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org>
2009-05-15 12:52 ` Esben Haabendal
[not found] ` <d2b9ea600905150552u6951a92es5dd119dcc37e641d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-05-15 14:16 ` Joakim Tjernlund
2009-05-18 11:04 ` Wolfram Sang
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).