* [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-19 5:22 ` Esben Haabendal 0 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-19 5:22 UTC (permalink / raw) To: linux-i2c-u79uwXL29TY76Z2rM5mHXA, 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. With this patch, it is possible to do an i2c_transfer() with additional i2c_msg's following the I2C_M_RD messages. It still needs to be resolved if it is possible to fix this issue by removing the STOP condition after reads in a robust way. 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..0199f9a 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.3.1 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-19 5:22 ` Esben Haabendal 0 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-19 5:22 UTC (permalink / raw) To: linux-i2c, linuxppc-dev This fixes MAL (arbitration lost) bug caused by illegal use of RSTA (repeated START) after STOP condition generated after last byte of reads. With this patch, it is possible to do an i2c_transfer() with additional i2c_msg's following the I2C_M_RD messages. It still needs to be resolved if it is possible to fix this issue by removing the STOP condition after reads in a robust way. Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> --- 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..0199f9a 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.3.1 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-19 5:22 ` Esben Haabendal @ 2009-05-26 11:30 ` Esben Haabendal -1 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-26 11:30 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk> wrote: > This fixes MAL (arbitration lost) bug caused by illegal use of > RSTA (repeated START) after STOP condition generated after last byte > of reads. With this patch, it is possible to do an i2c_transfer() with > additional i2c_msg's following the I2C_M_RD messages. > > It still needs to be resolved if it is possible to fix this issue > by removing the STOP condition after reads in a robust way. > > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> > --- > drivers/i2c/busses/i2c-mpc.c | 9 +++++++-- > 1 files changed, 7 insertions(+), 2 deletions(-) Any blockers to get this accepted? /Esben ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-26 11:30 ` Esben Haabendal 0 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-26 11:30 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk> w= rote: > This fixes MAL (arbitration lost) bug caused by illegal use of > RSTA (repeated START) after STOP condition generated after last byte > of reads. With this patch, it is possible to do an i2c_transfer() with > additional i2c_msg's following the I2C_M_RD messages. > > It still needs to be resolved if it is possible to fix this issue > by removing the STOP condition after reads in a robust way. > > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> > --- > =A0drivers/i2c/busses/i2c-mpc.c | =A0 =A09 +++++++-- > =A01 files changed, 7 insertions(+), 2 deletions(-) Any blockers to get this accepted? /Esben ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <d2b9ea600905260430i72290901p9266de4cade049c1-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-26 11:30 ` Esben Haabendal @ 2009-05-26 21:33 ` Ben Dooks -1 siblings, 0 replies; 41+ messages in thread From: Ben Dooks @ 2009-05-26 21:33 UTC (permalink / raw) To: Esben Haabendal Cc: Esben Haabendal, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: > On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org> wrote: > > This fixes MAL (arbitration lost) bug caused by illegal use of > > RSTA (repeated START) after STOP condition generated after last byte > > of reads. With this patch, it is possible to do an i2c_transfer() with > > additional i2c_msg's following the I2C_M_RD messages. > > > > It still needs to be resolved if it is possible to fix this issue > > by removing the STOP condition after reads in a robust way. > > > > 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(-) > > Any blockers to get this accepted? It would be nice to get an ack from someone who can actually test the driver before getting this merged. > /Esben > -- > 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 -- Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/) 'a smiley only costs 4 bytes' ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-26 21:33 ` Ben Dooks 0 siblings, 0 replies; 41+ messages in thread From: Ben Dooks @ 2009-05-26 21:33 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c, Esben Haabendal On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: > On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk> wrote: > > This fixes MAL (arbitration lost) bug caused by illegal use of > > RSTA (repeated START) after STOP condition generated after last byte > > of reads. With this patch, it is possible to do an i2c_transfer() with > > additional i2c_msg's following the I2C_M_RD messages. > > > > It still needs to be resolved if it is possible to fix this issue > > by removing the STOP condition after reads in a robust way. > > > > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> > > --- > > ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++++++-- > > ?1 files changed, 7 insertions(+), 2 deletions(-) > > Any blockers to get this accepted? It would be nice to get an ack from someone who can actually test the driver before getting this merged. > /Esben > -- > 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 -- Ben (ben@fluff.org, http://www.fluff.org/) 'a smiley only costs 4 bytes' ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <20090526213351.GG23114-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-26 21:33 ` Ben Dooks @ 2009-05-28 17:17 ` Wolfram Sang -1 siblings, 0 replies; 41+ messages in thread From: Wolfram Sang @ 2009-05-28 17:17 UTC (permalink / raw) To: Ben Dooks Cc: Esben Haabendal, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Esben Haabendal, Joakim Tjernlund [-- Attachment #1: Type: text/plain, Size: 600 bytes --] > > Any blockers to get this accepted? > > It would be nice to get an ack from someone who can actually test > the driver before getting this merged. I wanted to test it, but it does not apply due to line breaks (check @@-line). Also, I don't really have the time to dig into the topic, so I would only test it and give a tested-by-tag if it doesn't break anything here. I think Joakim would be a good candidate for an acked-by . -- 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] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-28 17:17 ` Wolfram Sang 0 siblings, 0 replies; 41+ messages in thread From: Wolfram Sang @ 2009-05-28 17:17 UTC (permalink / raw) To: Ben Dooks; +Cc: linuxppc-dev, Esben Haabendal, linux-i2c, Esben Haabendal [-- Attachment #1: Type: text/plain, Size: 600 bytes --] > > Any blockers to get this accepted? > > It would be nice to get an ack from someone who can actually test > the driver before getting this merged. I wanted to test it, but it does not apply due to line breaks (check @@-line). Also, I don't really have the time to dig into the topic, so I would only test it and give a tested-by-tag if it doesn't break anything here. I think Joakim would be a good candidate for an acked-by . -- 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] 41+ messages in thread
[parent not found: <20090528171726.GE3112-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-28 17:17 ` Wolfram Sang @ 2009-05-28 18:43 ` Joakim Tjernlund -1 siblings, 0 replies; 41+ messages in thread From: Joakim Tjernlund @ 2009-05-28 18:43 UTC (permalink / raw) To: Wolfram Sang Cc: Ben Dooks, Esben Haabendal, Esben Haabendal, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote on 28/05/2009 19:17:26: > > > > Any blockers to get this accepted? > > > > It would be nice to get an ack from someone who can actually test > > the driver before getting this merged. > > I wanted to test it, but it does not apply due to line breaks (check > @@-line). Also, I don't really have the time to dig into the topic, so I > would only test it and give a tested-by-tag if it doesn't break anything > here. I think Joakim would be a good candidate for an acked-by . It sure looks OK, even at a closer look :) Acked-by: Joakim Tjernlund <Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-28 18:43 ` Joakim Tjernlund 0 siblings, 0 replies; 41+ messages in thread From: Joakim Tjernlund @ 2009-05-28 18:43 UTC (permalink / raw) To: Wolfram Sang Cc: linuxppc-dev, Esben Haabendal, Esben Haabendal, Ben Dooks, linux-i2c Wolfram Sang <w.sang@pengutronix.de> wrote on 28/05/2009 19:17:26: > > > > Any blockers to get this accepted? > > > > It would be nice to get an ack from someone who can actually test > > the driver before getting this merged. > > I wanted to test it, but it does not apply due to line breaks (check > @@-line). Also, I don't really have the time to dig into the topic, so I > would only test it and give a tested-by-tag if it doesn't break anything > here. I think Joakim would be a good candidate for an acked-by . It sure looks OK, even at a closer look :) Acked-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-28 17:17 ` Wolfram Sang (?) (?) @ 2009-05-28 20:10 ` Esben Haabendal [not found] ` <d2b9ea600905281310p16f47ed0i454e25bf256d3010-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> -1 siblings, 1 reply; 41+ messages in thread From: Esben Haabendal @ 2009-05-28 20:10 UTC (permalink / raw) To: Wolfram Sang; +Cc: linuxppc-dev, Esben Haabendal, linux-i2c, Ben Dooks On Thu, May 28, 2009 at 7:17 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: >> > Any blockers to get this accepted? >> >> It would be nice to get an ack from someone who can actually test >> the driver before getting this merged. > > I wanted to test it, but it does not apply due to line breaks (check > @@-line). Also, I don't really have the time to dig into the topic, so I > would only test it and give a tested-by-tag if it doesn't break anything > here. I think Joakim would be a good candidate for an acked-by . I've checked both my copy in my "Sent" folder and the copy received from the list, and I cannot see any "line break" breakage of the patch. It must be your mail client that is messing with it, so I cannot really do it much better in that way. If necessary, I can push a branch to a public repo you can pull from. /Esben ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <d2b9ea600905281310p16f47ed0i454e25bf256d3010-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-28 20:10 ` Esben Haabendal @ 2009-05-28 20:34 ` Peter Korsgaard 0 siblings, 0 replies; 41+ messages in thread From: Peter Korsgaard @ 2009-05-28 20:34 UTC (permalink / raw) To: Esben Haabendal Cc: Wolfram Sang, Ben Dooks, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Esben Haabendal, Joakim Tjernlund >>>>> "Esben" == Esben Haabendal <esbenhaabendal-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes: Hi, >> I wanted to test it, but it does not apply due to line breaks (check >> @@-line). Also, I don't really have the time to dig into the topic, so I >> would only test it and give a tested-by-tag if it doesn't break anything >> here. I think Joakim would be a good candidate for an acked-by . Esben> I've checked both my copy in my "Sent" folder and the copy Esben> received from the list, and I cannot see any "line break" Esben> breakage of the patch. I guess Wolfram referred to the context line which was clearly word wrapped: @@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) The other lines look fine. -- Bye, Peter Korsgaard ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-28 20:34 ` Peter Korsgaard 0 siblings, 0 replies; 41+ messages in thread From: Peter Korsgaard @ 2009-05-28 20:34 UTC (permalink / raw) To: Esben Haabendal; +Cc: Esben Haabendal, linuxppc-dev, linux-i2c, Ben Dooks >>>>> "Esben" == Esben Haabendal <esbenhaabendal@gmail.com> writes: Hi, >> I wanted to test it, but it does not apply due to line breaks (check >> @@-line). Also, I don't really have the time to dig into the topic, so I >> would only test it and give a tested-by-tag if it doesn't break anything >> here. I think Joakim would be a good candidate for an acked-by . Esben> I've checked both my copy in my "Sent" folder and the copy Esben> received from the list, and I cannot see any "line break" Esben> breakage of the patch. I guess Wolfram referred to the context line which was clearly word wrapped: @@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) The other lines look fine. -- Bye, Peter Korsgaard ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <87ws8155md.fsf-uXGAPMMVk8amE9MCos8gUmSdvHPH+/yF@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-28 20:34 ` Peter Korsgaard @ 2009-05-28 20:41 ` Esben Haabendal -1 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-28 20:41 UTC (permalink / raw) To: Peter Korsgaard Cc: Wolfram Sang, Ben Dooks, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Esben Haabendal, Joakim Tjernlund > Esben> I've checked both my copy in my "Sent" folder and the copy > Esben> received from the list, and I cannot see any "line break" > Esben> breakage of the patch. > > I guess Wolfram referred to the context line which was clearly word wrapped: > > @@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap, > struct i2c_msg *msgs, int num) > > The other lines look fine. It's strange, that line looks perfectly fine when I check the mail in my GMail inbox and the outbox from the account I sent it from. /Esben -- 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] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-28 20:41 ` Esben Haabendal 0 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-28 20:41 UTC (permalink / raw) To: Peter Korsgaard; +Cc: Esben Haabendal, linuxppc-dev, linux-i2c, Ben Dooks > =A0Esben> I've checked both my copy in my "Sent" folder and the copy > =A0Esben> received from the list, and I cannot see any "line break" > =A0Esben> breakage of the patch. > > I guess Wolfram referred to the context line which was clearly word wrapp= ed: > > @@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap, > struct i2c_msg *msgs, int num) > > The other lines look fine. It's strange, that line looks perfectly fine when I check the mail in my GM= ail inbox and the outbox from the account I sent it from. /Esben --=20 Esben Haabendal, Senior Software Consultant Dor=E9Development ApS, Ved Stranden 1, 9560 Hadsund, DK-Denmark Phone: +45 51 92 53 93, E-mail: eha@doredevelopment.dk WWW: http://www.doredevelopment.dk ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <d2b9ea600905281341s24efa4e3l6277a8aa5dec5bd-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-28 20:41 ` Esben Haabendal @ 2009-05-28 21:08 ` Peter Korsgaard -1 siblings, 0 replies; 41+ messages in thread From: Peter Korsgaard @ 2009-05-28 21:08 UTC (permalink / raw) To: Esben Haabendal Cc: Wolfram Sang, Ben Dooks, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Esben Haabendal, Joakim Tjernlund >>>>> "Esben" == Esben Haabendal <esbenhaabendal-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes: Hi, Esben> It's strange, that line looks perfectly fine when I check the Esben> mail in my GMail inbox and the outbox from the account I sent Esben> it from. Well, it is here and in the archive: http://ozlabs.org/pipermail/linuxppc-dev/2009-May/072274.html Please consider using git send-email for patches. -- Bye, Peter Korsgaard ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-28 21:08 ` Peter Korsgaard 0 siblings, 0 replies; 41+ messages in thread From: Peter Korsgaard @ 2009-05-28 21:08 UTC (permalink / raw) To: Esben Haabendal; +Cc: Esben Haabendal, linuxppc-dev, linux-i2c, Ben Dooks >>>>> "Esben" == Esben Haabendal <esbenhaabendal@gmail.com> writes: Hi, Esben> It's strange, that line looks perfectly fine when I check the Esben> mail in my GMail inbox and the outbox from the account I sent Esben> it from. Well, it is here and in the archive: http://ozlabs.org/pipermail/linuxppc-dev/2009-May/072274.html Please consider using git send-email for patches. -- Bye, Peter Korsgaard ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <87octd5415.fsf-uXGAPMMVk8amE9MCos8gUmSdvHPH+/yF@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-28 21:08 ` Peter Korsgaard @ 2009-05-28 21:22 ` Esben Haabendal -1 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-28 21:22 UTC (permalink / raw) To: Peter Korsgaard Cc: Wolfram Sang, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA Peter Korsgaard wrote: >>>>>> "Esben" == Esben Haabendal <esbenhaabendal-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes: >>>>>> > > Hi, > > Esben> It's strange, that line looks perfectly fine when I check the > Esben> mail in my GMail inbox and the outbox from the account I sent > Esben> it from. > > Well, it is here and in the archive: > http://ozlabs.org/pipermail/linuxppc-dev/2009-May/072274.html > If you look at http://www.mail-archive.com/linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg01050.html instead, the patch is not broken. I find it more likely that the ozlabs.org archive is breaking my lines, than mail-archive.com putting broken lines together again. > Please consider using git send-email for patches. > I used git imap-send and thunderbird to do it. I will consider git send-email in the future, although I don't think my original e-mail were broken ;-) But ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-28 21:22 ` Esben Haabendal 0 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-28 21:22 UTC (permalink / raw) To: Peter Korsgaard; +Cc: linuxppc-dev, linux-i2c Peter Korsgaard wrote: >>>>>> "Esben" == Esben Haabendal <esbenhaabendal@gmail.com> writes: >>>>>> > > Hi, > > Esben> It's strange, that line looks perfectly fine when I check the > Esben> mail in my GMail inbox and the outbox from the account I sent > Esben> it from. > > Well, it is here and in the archive: > http://ozlabs.org/pipermail/linuxppc-dev/2009-May/072274.html > If you look at http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg01050.html instead, the patch is not broken. I find it more likely that the ozlabs.org archive is breaking my lines, than mail-archive.com putting broken lines together again. > Please consider using git send-email for patches. > I used git imap-send and thunderbird to do it. I will consider git send-email in the future, although I don't think my original e-mail were broken ;-) But ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-26 21:33 ` Ben Dooks @ 2009-12-03 15:09 ` Michael Lawnick -1 siblings, 0 replies; 41+ messages in thread From: Michael Lawnick @ 2009-12-03 15:09 UTC (permalink / raw) To: Ben Dooks Cc: Esben Haabendal, Esben Haabendal, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A Ben Dooks said the following: > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org> wrote: >> > This fixes MAL (arbitration lost) bug caused by illegal use of >> > RSTA (repeated START) after STOP condition generated after last byte >> > of reads. With this patch, it is possible to do an i2c_transfer() with >> > additional i2c_msg's following the I2C_M_RD messages. >> > >> > It still needs to be resolved if it is possible to fix this issue >> > by removing the STOP condition after reads in a robust way. >> > >> > 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(-) >> >> Any blockers to get this accepted? > > It would be nice to get an ack from someone who can actually test > the driver before getting this merged. > What is the state of this patch? Shouldn't we attack the problem on a more general way by inventing a Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)? This way the client driver is able to decide what it needs. If we do the choice within adapter, chance is about 50% to be wrong. Just my 2 Cents. -- Michael ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-12-03 15:09 ` Michael Lawnick 0 siblings, 0 replies; 41+ messages in thread From: Michael Lawnick @ 2009-12-03 15:09 UTC (permalink / raw) To: Ben Dooks; +Cc: linuxppc-dev, Esben Haabendal, Esben Haabendal, linux-i2c Ben Dooks said the following: > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk> wrote: >> > This fixes MAL (arbitration lost) bug caused by illegal use of >> > RSTA (repeated START) after STOP condition generated after last byte >> > of reads. With this patch, it is possible to do an i2c_transfer() with >> > additional i2c_msg's following the I2C_M_RD messages. >> > >> > It still needs to be resolved if it is possible to fix this issue >> > by removing the STOP condition after reads in a robust way. >> > >> > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> >> > --- >> > ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++++++-- >> > ?1 files changed, 7 insertions(+), 2 deletions(-) >> >> Any blockers to get this accepted? > > It would be nice to get an ack from someone who can actually test > the driver before getting this merged. > What is the state of this patch? Shouldn't we attack the problem on a more general way by inventing a Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)? This way the client driver is able to decide what it needs. If we do the choice within adapter, chance is about 50% to be wrong. Just my 2 Cents. -- Michael ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <4B17D4C5.3070100-Mmb7MZpHnFY@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-12-03 15:09 ` Michael Lawnick @ 2009-12-03 15:29 ` Ben Dooks -1 siblings, 0 replies; 41+ messages in thread From: Ben Dooks @ 2009-12-03 15:29 UTC (permalink / raw) To: Michael Lawnick Cc: Ben Dooks, Esben Haabendal, Esben Haabendal, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A On Thu, Dec 03, 2009 at 04:09:57PM +0100, Michael Lawnick wrote: > Ben Dooks said the following: > > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: > >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org> wrote: > >> > This fixes MAL (arbitration lost) bug caused by illegal use of > >> > RSTA (repeated START) after STOP condition generated after last byte > >> > of reads. With this patch, it is possible to do an i2c_transfer() with > >> > additional i2c_msg's following the I2C_M_RD messages. > >> > > >> > It still needs to be resolved if it is possible to fix this issue > >> > by removing the STOP condition after reads in a robust way. > >> > > >> > 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(-) > >> > >> Any blockers to get this accepted? > > > > It would be nice to get an ack from someone who can actually test > > the driver before getting this merged. > > > What is the state of this patch? > Shouldn't we attack the problem on a more general way by inventing a > Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)? > This way the client driver is able to decide what it needs. If we do the > choice within adapter, chance is about 50% to be wrong. The documentation for 'struct i2c_msg' already says the STOP should only be generated for the last message of the transfer. If STOP is being generated for a message that isn't the last in the transfer than this is incorrect behaviour. Unless otherwise indicated, I'll put the patch in the series that I'll send to Linus early next week. -- Ben Q: What's a light-year? A: One-third less calories than a regular year. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-12-03 15:29 ` Ben Dooks 0 siblings, 0 replies; 41+ messages in thread From: Ben Dooks @ 2009-12-03 15:29 UTC (permalink / raw) To: Michael Lawnick Cc: linuxppc-dev, Esben Haabendal, Esben Haabendal, Ben Dooks, linux-i2c On Thu, Dec 03, 2009 at 04:09:57PM +0100, Michael Lawnick wrote: > Ben Dooks said the following: > > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: > >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk> wrote: > >> > This fixes MAL (arbitration lost) bug caused by illegal use of > >> > RSTA (repeated START) after STOP condition generated after last byte > >> > of reads. With this patch, it is possible to do an i2c_transfer() with > >> > additional i2c_msg's following the I2C_M_RD messages. > >> > > >> > It still needs to be resolved if it is possible to fix this issue > >> > by removing the STOP condition after reads in a robust way. > >> > > >> > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> > >> > --- > >> > ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++++++-- > >> > ?1 files changed, 7 insertions(+), 2 deletions(-) > >> > >> Any blockers to get this accepted? > > > > It would be nice to get an ack from someone who can actually test > > the driver before getting this merged. > > > What is the state of this patch? > Shouldn't we attack the problem on a more general way by inventing a > Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)? > This way the client driver is able to decide what it needs. If we do the > choice within adapter, chance is about 50% to be wrong. The documentation for 'struct i2c_msg' already says the STOP should only be generated for the last message of the transfer. If STOP is being generated for a message that isn't the last in the transfer than this is incorrect behaviour. Unless otherwise indicated, I'll put the patch in the series that I'll send to Linus early next week. -- Ben Q: What's a light-year? A: One-third less calories than a regular year. ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <20091203152916.GC23152-SMNkleLxa3Z6Wcw2j4pizdi2O/JbrIOy@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-12-03 15:29 ` Ben Dooks @ 2009-12-03 15:49 ` Joakim Tjernlund -1 siblings, 0 replies; 41+ messages in thread From: Joakim Tjernlund @ 2009-12-03 15:49 UTC (permalink / raw) Cc: Ben Dooks, Esben Haabendal, Esben Haabendal, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, Michael Lawnick > > On Thu, Dec 03, 2009 at 04:09:57PM +0100, Michael Lawnick wrote: > > Ben Dooks said the following: > > > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: > > >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org> wrote: > > >> > This fixes MAL (arbitration lost) bug caused by illegal use of > > >> > RSTA (repeated START) after STOP condition generated after last byte > > >> > of reads. With this patch, it is possible to do an i2c_transfer() with > > >> > additional i2c_msg's following the I2C_M_RD messages. > > >> > > > >> > It still needs to be resolved if it is possible to fix this issue > > >> > by removing the STOP condition after reads in a robust way. > > >> > > > >> > 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(-) > > >> > > >> Any blockers to get this accepted? > > > > > > It would be nice to get an ack from someone who can actually test > > > the driver before getting this merged. > > > > > What is the state of this patch? > > Shouldn't we attack the problem on a more general way by inventing a > > Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)? > > This way the client driver is able to decide what it needs. If we do the > > choice within adapter, chance is about 50% to be wrong. > > The documentation for 'struct i2c_msg' already says the STOP should only > be generated for the last message of the transfer. If STOP is being > generated for a message that isn't the last in the transfer than this > is incorrect behaviour. > > Unless otherwise indicated, I'll put the patch in the series that I'll > send to Linus early next week. Don't, already fixed properly and in Linus tree already. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-12-03 15:49 ` Joakim Tjernlund 0 siblings, 0 replies; 41+ messages in thread From: Joakim Tjernlund @ 2009-12-03 15:49 UTC (permalink / raw) To: Ben Dooks Cc: Esben Haabendal, Michael Lawnick, linuxppc-dev, Esben Haabendal, Ben Dooks, linux-i2c > > On Thu, Dec 03, 2009 at 04:09:57PM +0100, Michael Lawnick wrote: > > Ben Dooks said the following: > > > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: > > >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk> wrote: > > >> > This fixes MAL (arbitration lost) bug caused by illegal use of > > >> > RSTA (repeated START) after STOP condition generated after last byte > > >> > of reads. With this patch, it is possible to do an i2c_transfer() with > > >> > additional i2c_msg's following the I2C_M_RD messages. > > >> > > > >> > It still needs to be resolved if it is possible to fix this issue > > >> > by removing the STOP condition after reads in a robust way. > > >> > > > >> > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> > > >> > --- > > >> > ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++++++-- > > >> > ?1 files changed, 7 insertions(+), 2 deletions(-) > > >> > > >> Any blockers to get this accepted? > > > > > > It would be nice to get an ack from someone who can actually test > > > the driver before getting this merged. > > > > > What is the state of this patch? > > Shouldn't we attack the problem on a more general way by inventing a > > Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)? > > This way the client driver is able to decide what it needs. If we do the > > choice within adapter, chance is about 50% to be wrong. > > The documentation for 'struct i2c_msg' already says the STOP should only > be generated for the last message of the transfer. If STOP is being > generated for a message that isn't the last in the transfer than this > is incorrect behaviour. > > Unless otherwise indicated, I'll put the patch in the series that I'll > send to Linus early next week. Don't, already fixed properly and in Linus tree already. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-12-03 15:29 ` Ben Dooks @ 2009-12-04 6:58 ` Michael Lawnick -1 siblings, 0 replies; 41+ messages in thread From: Michael Lawnick @ 2009-12-04 6:58 UTC (permalink / raw) To: Ben Dooks Cc: Esben Haabendal, Esben Haabendal, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A Ben Dooks said the following: > On Thu, Dec 03, 2009 at 04:09:57PM +0100, Michael Lawnick wrote: >> Ben Dooks said the following: >> > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: >> >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org> wrote: >> >> > This fixes MAL (arbitration lost) bug caused by illegal use of >> >> > RSTA (repeated START) after STOP condition generated after last byte >> >> > of reads. With this patch, it is possible to do an i2c_transfer() with >> >> > additional i2c_msg's following the I2C_M_RD messages. >> >> > >> >> > It still needs to be resolved if it is possible to fix this issue >> >> > by removing the STOP condition after reads in a robust way. >> >> > >> >> > 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(-) >> >> >> >> Any blockers to get this accepted? >> > >> > It would be nice to get an ack from someone who can actually test >> > the driver before getting this merged. >> > >> What is the state of this patch? >> Shouldn't we attack the problem on a more general way by inventing a >> Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)? >> This way the client driver is able to decide what it needs. If we do the >> choice within adapter, chance is about 50% to be wrong. > > The documentation for 'struct i2c_msg' already says the STOP should only > be generated for the last message of the transfer. If STOP is being > generated for a message that isn't the last in the transfer than this > is incorrect behaviour. Ah, now I see, this is a mpc-only problem of implementation (automatically generating stop in read function). I couldn't find the above mentioned specification of STOP/RESTART (seems I need new glasses) and was worried about whether my own implementation for OCTEON is correct. Thank you. -- Michael ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-12-04 6:58 ` Michael Lawnick 0 siblings, 0 replies; 41+ messages in thread From: Michael Lawnick @ 2009-12-04 6:58 UTC (permalink / raw) To: Ben Dooks; +Cc: linuxppc-dev, Esben Haabendal, Esben Haabendal, linux-i2c Ben Dooks said the following: > On Thu, Dec 03, 2009 at 04:09:57PM +0100, Michael Lawnick wrote: >> Ben Dooks said the following: >> > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: >> >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk> wrote: >> >> > This fixes MAL (arbitration lost) bug caused by illegal use of >> >> > RSTA (repeated START) after STOP condition generated after last byte >> >> > of reads. With this patch, it is possible to do an i2c_transfer() with >> >> > additional i2c_msg's following the I2C_M_RD messages. >> >> > >> >> > It still needs to be resolved if it is possible to fix this issue >> >> > by removing the STOP condition after reads in a robust way. >> >> > >> >> > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> >> >> > --- >> >> > ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++++++-- >> >> > ?1 files changed, 7 insertions(+), 2 deletions(-) >> >> >> >> Any blockers to get this accepted? >> > >> > It would be nice to get an ack from someone who can actually test >> > the driver before getting this merged. >> > >> What is the state of this patch? >> Shouldn't we attack the problem on a more general way by inventing a >> Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)? >> This way the client driver is able to decide what it needs. If we do the >> choice within adapter, chance is about 50% to be wrong. > > The documentation for 'struct i2c_msg' already says the STOP should only > be generated for the last message of the transfer. If STOP is being > generated for a message that isn't the last in the transfer than this > is incorrect behaviour. Ah, now I see, this is a mpc-only problem of implementation (automatically generating stop in read function). I couldn't find the above mentioned specification of STOP/RESTART (seems I need new glasses) and was worried about whether my own implementation for OCTEON is correct. Thank you. -- Michael ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-12-03 15:09 ` Michael Lawnick (?) (?) @ 2009-12-03 15:42 ` Joakim Tjernlund -1 siblings, 0 replies; 41+ messages in thread From: Joakim Tjernlund @ 2009-12-03 15:42 UTC (permalink / raw) To: Michael Lawnick Cc: linuxppc-dev, Esben Haabendal, Esben Haabendal, Ben Dooks, linux-i2c > > Ben Dooks said the following: > > On Tue, May 26, 2009 at 01:30:21PM +0200, Esben Haabendal wrote: > >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk> wrote: > >> > This fixes MAL (arbitration lost) bug caused by illegal use of > >> > RSTA (repeated START) after STOP condition generated after last byte > >> > of reads. With this patch, it is possible to do an i2c_transfer() with > >> > additional i2c_msg's following the I2C_M_RD messages. > >> > > >> > It still needs to be resolved if it is possible to fix this issue > >> > by removing the STOP condition after reads in a robust way. > >> > > >> > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> > >> > --- > >> > ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++++++-- > >> > ?1 files changed, 7 insertions(+), 2 deletions(-) > >> > >> Any blockers to get this accepted? > > > > It would be nice to get an ack from someone who can actually test > > the driver before getting this merged. > > > What is the state of this patch? > Shouldn't we attack the problem on a more general way by inventing a > Flag I2C_M_RESTART (or better I2C_M_NO_RESTART for backward compatibility)? > This way the client driver is able to decide what it needs. If we do the > choice within adapter, chance is about 50% to be wrong. I have sent another patch (which I think is in Linus tree already) that fixes the problem properly. Can't remember if the driver handles M_RESTART or not. That is a separate issue though. Jocke ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-26 11:30 ` Esben Haabendal @ 2009-05-28 19:31 ` Grant Likely -1 siblings, 0 replies; 41+ messages in thread From: Grant Likely @ 2009-05-28 19:31 UTC (permalink / raw) To: Esben Haabendal Cc: Esben Haabendal, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA On Tue, May 26, 2009 at 5:30 AM, Esben Haabendal <esbenhaabendal-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk> wrote: >> This fixes MAL (arbitration lost) bug caused by illegal use of >> RSTA (repeated START) after STOP condition generated after last byte >> of reads. With this patch, it is possible to do an i2c_transfer() with >> additional i2c_msg's following the I2C_M_RD messages. >> >> It still needs to be resolved if it is possible to fix this issue >> by removing the STOP condition after reads in a robust way. >> >> 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(-) > > Any blockers to get this accepted? It helps if you cc: developers/maintainers of the device. ie. Kumar for mpc8xxx, me for 52xx. This is the first time I noticed your posting. It will take me a few days before I get a chance to review it. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-28 19:31 ` Grant Likely 0 siblings, 0 replies; 41+ messages in thread From: Grant Likely @ 2009-05-28 19:31 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c, Esben Haabendal On Tue, May 26, 2009 at 5:30 AM, Esben Haabendal <esbenhaabendal@gmail.com> wrote: > On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk>= wrote: >> This fixes MAL (arbitration lost) bug caused by illegal use of >> RSTA (repeated START) after STOP condition generated after last byte >> of reads. With this patch, it is possible to do an i2c_transfer() with >> additional i2c_msg's following the I2C_M_RD messages. >> >> It still needs to be resolved if it is possible to fix this issue >> by removing the STOP condition after reads in a robust way. >> >> Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> >> --- >> =A0drivers/i2c/busses/i2c-mpc.c | =A0 =A09 +++++++-- >> =A01 files changed, 7 insertions(+), 2 deletions(-) > > Any blockers to get this accepted? It helps if you cc: developers/maintainers of the device. ie. Kumar for mpc8xxx, me for 52xx. This is the first time I noticed your posting. It will take me a few days before I get a chance to review it. g. --=20 Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <fa686aa40905281231o26c74a13v250bcedbd066e77b-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-28 19:31 ` Grant Likely @ 2009-05-28 20:15 ` Esben Haabendal -1 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-28 20:15 UTC (permalink / raw) To: Kumar Gala Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Grant Likely On Thu, May 28, 2009 at 9:31 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Tue, May 26, 2009 at 5:30 AM, Esben Haabendal wrote: >> On Tue, May 19, 2009 at 7:22 AM, 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. With this patch, it is possible to do an i2c_transfer() with >>> additional i2c_msg's following the I2C_M_RD messages. >>> >>> It still needs to be resolved if it is possible to fix this issue >>> by removing the STOP condition after reads in a robust way. >>> >>> 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(-) >> >> Any blockers to get this accepted? > > It helps if you cc: developers/maintainers of the device. ie. Kumar > for mpc8xxx, me for 52xx. > > This is the first time I noticed your posting. It will take me a few > days before I get a chance to review it. Kumar, will you take a look at this patch? /Esben ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-05-28 20:15 ` Esben Haabendal 0 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-05-28 20:15 UTC (permalink / raw) To: Kumar Gala; +Cc: linuxppc-dev, linux-i2c On Thu, May 28, 2009 at 9:31 PM, Grant Likely <grant.likely@secretlab.ca> w= rote: > On Tue, May 26, 2009 at 5:30 AM, Esben Haabendal wrote: >> On Tue, May 19, 2009 at 7:22 AM, 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. With this patch, it is possible to do an i2c_transfer() with >>> additional i2c_msg's following the I2C_M_RD messages. >>> >>> It still needs to be resolved if it is possible to fix this issue >>> by removing the STOP condition after reads in a robust way. >>> >>> Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> >>> --- >>> =A0drivers/i2c/busses/i2c-mpc.c | =A0 =A09 +++++++-- >>> =A01 files changed, 7 insertions(+), 2 deletions(-) >> >> Any blockers to get this accepted? > > It helps if you cc: developers/maintainers of the device. =A0ie. Kumar > for mpc8xxx, me for 52xx. > > This is the first time I noticed your posting. =A0It will take me a few > days before I get a chance to review it. Kumar, will you take a look at this patch? /Esben ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-28 20:15 ` Esben Haabendal (?) @ 2009-06-02 22:25 ` Ben Dooks -1 siblings, 0 replies; 41+ messages in thread From: Ben Dooks @ 2009-06-02 22:25 UTC (permalink / raw) To: Esben Haabendal; +Cc: Kumar Gala, linux-i2c, linuxppc-dev On Thu, May 28, 2009 at 10:15:22PM +0200, Esben Haabendal wrote: > On Thu, May 28, 2009 at 9:31 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > > On Tue, May 26, 2009 at 5:30 AM, Esben Haabendal wrote: > >> On Tue, May 19, 2009 at 7:22 AM, 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. With this patch, it is possible to do an i2c_transfer() with > >>> additional i2c_msg's following the I2C_M_RD messages. > >>> > >>> It still needs to be resolved if it is possible to fix this issue > >>> by removing the STOP condition after reads in a robust way. > >>> > >>> Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> > >>> --- > >>> ?drivers/i2c/busses/i2c-mpc.c | ? ?9 +++++++-- > >>> ?1 files changed, 7 insertions(+), 2 deletions(-) > >> > >> Any blockers to get this accepted? > > > > It helps if you cc: developers/maintainers of the device. ?ie. Kumar > > for mpc8xxx, me for 52xx. > > > > This is the first time I noticed your posting. ?It will take me a few > > days before I get a chance to review it. > > Kumar, will you take a look at this patch? is anyone else likely to review it, or should I merge? -- Ben (ben@fluff.org, http://www.fluff.org/) 'a smiley only costs 4 bytes' ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-19 5:22 ` Esben Haabendal @ 2009-06-02 23:12 ` Grant Likely -1 siblings, 0 replies; 41+ messages in thread From: Grant Likely @ 2009-06-02 23:12 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c, Ben Dooks Hi Esben and Ben, Sorry for the lateness in reviewing this. FWIW, here are my comments. g. On Mon, May 18, 2009 at 11:22 PM, Esben Haabendal <eha@doredevelopment.dk> wrote: > This fixes MAL (arbitration lost) bug caused by illegal use of > RSTA (repeated START) after STOP condition generated after last byte > of reads. With this patch, it is possible to do an i2c_transfer() with > additional i2c_msg's following the I2C_M_RD messages. > > It still needs to be resolved if it is possible to fix this issue > by removing the STOP condition after reads in a robust way. > > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> > --- > 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..0199f9a 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); > } Hmmm, seems to be a rather convoluted code path. Surely this could be handled in a clearer way. The whole (pmsg - 1) bit raises my eyebrows (I'd rather see msgs[i-1]). At the very least this deserves an comment describing what it is doing. The following might be better for the next person who has to read this code: + int restart = 0; for (i = 0; ret >= 0 && i < num; 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 (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); + /* Only set the restart flag if this was not an I2C_M_RD transaction + * because it causes an illegal use of + * RSTA (repeated START) after STOP condition generated after last byte + * of reads */ + restart = (pmsg->flags & I2C_M_RD == 0); } g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-06-02 23:12 ` Grant Likely 0 siblings, 0 replies; 41+ messages in thread From: Grant Likely @ 2009-06-02 23:12 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c, Ben Dooks Hi Esben and Ben, Sorry for the lateness in reviewing this. FWIW, here are my comments. g. On Mon, May 18, 2009 at 11:22 PM, Esben Haabendal <eha@doredevelopment.dk> wrote: > This fixes MAL (arbitration lost) bug caused by illegal use of > RSTA (repeated START) after STOP condition generated after last byte > of reads. With this patch, it is possible to do an i2c_transfer() with > additional i2c_msg's following the I2C_M_RD messages. > > It still needs to be resolved if it is possible to fix this issue > by removing the STOP condition after reads in a robust way. > > Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> > --- > =A0drivers/i2c/busses/i2c-mpc.c | =A0 =A09 +++++++-- > =A01 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c > index 4af5c09..0199f9a 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, struc= t > i2c_msg *msgs, int num) > =A0 =A0 =A0 =A0} > > =A0 =A0 =A0 =A0for (i =3D 0; ret >=3D 0 && i < num; i++) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 int restart =3D i; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pmsg =3D &msgs[i]; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev_dbg(i2c->dev, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"Doing %s %d bytes to 0x%0= 2x - %d of %d messages\n", > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pmsg->flags & I2C_M_RD ? "= read" : "write", > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pmsg->len, pmsg->addr, i += 1, num); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (i > 0 && ((pmsg - 1)->flags & I2C_M_RD)= ) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 restart =3D 0; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pmsg->flags & I2C_M_RD) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ret =3D > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_read(i2c, pmsg-= >addr, pmsg->buf, pmsg->len, > i); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_read(i2c, pmsg-= >addr, pmsg->buf, pmsg->len, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= restart); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ret =3D > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_write(i2c, pmsg= ->addr, pmsg->buf, pmsg->len, > i); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_write(i2c, pmsg= ->addr, pmsg->buf, pmsg->len, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= restart); > =A0 =A0 =A0 =A0} Hmmm, seems to be a rather convoluted code path. Surely this could be handled in a clearer way. The whole (pmsg - 1) bit raises my eyebrows (I'd rather see msgs[i-1]). At the very least this deserves an comment describing what it is doing. The following might be better for the next person who has to read this code: + int restart =3D 0; for (i =3D 0; ret >=3D 0 && i < num; i++) { pmsg =3D &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 (pmsg->flags & I2C_M_RD) ret =3D - mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i); + mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, + restart); else ret =3D - mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len= , i); + mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len= , + restart); + /* Only set the restart flag if this was not an I2C_M_RD transaction + * because it causes an illegal use of + * RSTA (repeated START) after STOP condition generated after last byte + * of reads */ + restart =3D (pmsg->flags & I2C_M_RD =3D=3D 0); } g. --=20 Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-06-02 23:12 ` Grant Likely @ 2009-06-03 6:01 ` Grant Likely -1 siblings, 0 replies; 41+ messages in thread From: Grant Likely @ 2009-06-03 6:01 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c, Ben Dooks On Tue, Jun 2, 2009 at 5:12 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > Hi Esben and Ben, > > Sorry for the lateness in reviewing this. FWIW, here are my comments. > > g. > > On Mon, May 18, 2009 at 11:22 PM, Esben Haabendal > <eha@doredevelopment.dk> wrote: >> This fixes MAL (arbitration lost) bug caused by illegal use of >> RSTA (repeated START) after STOP condition generated after last byte >> of reads. With this patch, it is possible to do an i2c_transfer() with >> additional i2c_msg's following the I2C_M_RD messages. >> >> It still needs to be resolved if it is possible to fix this issue >> by removing the STOP condition after reads in a robust way. >> >> Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> >> --- >> 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..0199f9a 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); >> } > > Hmmm, seems to be a rather convoluted code path. Surely this could be > handled in a clearer way. The whole (pmsg - 1) bit raises my eyebrows > (I'd rather see msgs[i-1]). At the very least this deserves an > comment describing what it is doing. The following might be better > for the next person who has to read this code: > > + int restart = 0; > for (i = 0; ret >= 0 && i < num; 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 (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); > + /* Only set the restart flag if this was not an > I2C_M_RD transaction > + * because it causes an illegal use of > + * RSTA (repeated START) after STOP condition > generated after last byte > + * of reads */ > + restart = (pmsg->flags & I2C_M_RD == 0); > } BTW, since you're touching these lines anyway, please clean up the whitespace usage. Since you have to break the line anyway, it no longer makes sense for the "ret = " to be on a separate line anymore. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-06-03 6:01 ` Grant Likely 0 siblings, 0 replies; 41+ messages in thread From: Grant Likely @ 2009-06-03 6:01 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c, Ben Dooks On Tue, Jun 2, 2009 at 5:12 PM, Grant Likely <grant.likely@secretlab.ca> wr= ote: > Hi Esben and Ben, > > Sorry for the lateness in reviewing this. =A0FWIW, here are my comments. > > g. > > On Mon, May 18, 2009 at 11:22 PM, Esben Haabendal > <eha@doredevelopment.dk> wrote: >> This fixes MAL (arbitration lost) bug caused by illegal use of >> RSTA (repeated START) after STOP condition generated after last byte >> of reads. With this patch, it is possible to do an i2c_transfer() with >> additional i2c_msg's following the I2C_M_RD messages. >> >> It still needs to be resolved if it is possible to fix this issue >> by removing the STOP condition after reads in a robust way. >> >> Signed-off-by: Esben Haabendal <eha@doredevelopment.dk> >> --- >> =A0drivers/i2c/busses/i2c-mpc.c | =A0 =A09 +++++++-- >> =A01 files changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c >> index 4af5c09..0199f9a 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, stru= ct >> i2c_msg *msgs, int num) >> =A0 =A0 =A0 =A0} >> >> =A0 =A0 =A0 =A0for (i =3D 0; ret >=3D 0 && i < num; i++) { >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 int restart =3D i; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pmsg =3D &msgs[i]; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev_dbg(i2c->dev, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"Doing %s %d bytes to 0x%= 02x - %d of %d messages\n", >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pmsg->flags & I2C_M_RD ? = "read" : "write", >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pmsg->len, pmsg->addr, i = + 1, num); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (i > 0 && ((pmsg - 1)->flags & I2C_M_RD= )) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 restart =3D 0; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pmsg->flags & I2C_M_RD) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ret =3D >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_read(i2c, pmsg= ->addr, pmsg->buf, pmsg->len, >> i); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_read(i2c, pmsg= ->addr, pmsg->buf, pmsg->len, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0restart); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ret =3D >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_write(i2c, pms= g->addr, pmsg->buf, pmsg->len, >> i); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_write(i2c, pms= g->addr, pmsg->buf, pmsg->len, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 restart); >> =A0 =A0 =A0 =A0} > > Hmmm, seems to be a rather convoluted code path. =A0Surely this could be > handled in a clearer way. =A0The whole (pmsg - 1) bit raises my eyebrows > (I'd rather see msgs[i-1]). =A0At the very least this deserves an > comment describing what it is doing. =A0The following might be better > for the next person who has to read this code: > > + =A0 =A0 =A0int restart =3D 0; > =A0 =A0 =A0 for (i =3D 0; ret >=3D 0 && i < num; i++) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 pmsg =3D &msgs[i]; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_dbg(i2c->dev, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "Doing %s %d bytes to 0x%02x = - %d of %d messages\n", > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pmsg->flags & I2C_M_RD ? "rea= d" : "write", > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pmsg->len, pmsg->addr, i + 1,= num); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (pmsg->flags & I2C_M_RD) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_read(i2c, pmsg-= >addr, pmsg->buf, pmsg->len, > i); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_read(i2c, pmsg-= >addr, pmsg->buf, pmsg->len, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= restart); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 else > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_write(i2c, pmsg= ->addr, pmsg->buf, pmsg->len, > i); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc_write(i2c, pmsg= ->addr, pmsg->buf, pmsg->len, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= restart); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Only set the restart flag if this was no= t an > I2C_M_RD transaction > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* because it causes an illegal use of > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* RSTA (repeated START) after STOP condi= tion > generated after last byte > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* of reads =A0*/ > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 restart =3D (pmsg->flags & I2C_M_RD =3D=3D = 0); > =A0 =A0 =A0 } BTW, since you're touching these lines anyway, please clean up the whitespace usage. Since you have to break the line anyway, it no longer makes sense for the "ret =3D " to be on a separate line anymore. g. --=20 Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <4A124202.4010201-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-05-19 5:22 ` Esben Haabendal @ 2009-06-14 13:16 ` Ben Dooks -1 siblings, 0 replies; 41+ messages in thread From: Ben Dooks @ 2009-06-14 13:16 UTC (permalink / raw) To: Esben Haabendal Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A is there a new version of this patch available? -- Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/) 'a smiley only costs 4 bytes' ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-06-14 13:16 ` Ben Dooks 0 siblings, 0 replies; 41+ messages in thread From: Ben Dooks @ 2009-06-14 13:16 UTC (permalink / raw) To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c is there a new version of this patch available? -- Ben (ben@fluff.org, http://www.fluff.org/) 'a smiley only costs 4 bytes' ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <20090614131605.GK20446-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>]
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg 2009-06-14 13:16 ` Ben Dooks @ 2009-06-14 14:04 ` Esben Haabendal -1 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-06-14 14:04 UTC (permalink / raw) To: Ben Dooks Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A Ben Dooks wrote: > is there a new version of this patch available? > > I will catch up on it ASAP. /Esben ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg @ 2009-06-14 14:04 ` Esben Haabendal 0 siblings, 0 replies; 41+ messages in thread From: Esben Haabendal @ 2009-06-14 14:04 UTC (permalink / raw) To: Ben Dooks; +Cc: linuxppc-dev, linux-i2c Ben Dooks wrote: > is there a new version of this patch available? > > I will catch up on it ASAP. /Esben ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2009-12-04 6:58 UTC | newest] Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-05-19 5:22 [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg Esben Haabendal 2009-05-19 5:22 ` Esben Haabendal 2009-05-26 11:30 ` Esben Haabendal 2009-05-26 11:30 ` Esben Haabendal [not found] ` <d2b9ea600905260430i72290901p9266de4cade049c1-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2009-05-26 21:33 ` Ben Dooks 2009-05-26 21:33 ` Ben Dooks [not found] ` <20090526213351.GG23114-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org> 2009-05-28 17:17 ` Wolfram Sang 2009-05-28 17:17 ` Wolfram Sang [not found] ` <20090528171726.GE3112-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 2009-05-28 18:43 ` Joakim Tjernlund 2009-05-28 18:43 ` Joakim Tjernlund 2009-05-28 20:10 ` Esben Haabendal [not found] ` <d2b9ea600905281310p16f47ed0i454e25bf256d3010-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2009-05-28 20:34 ` Peter Korsgaard 2009-05-28 20:34 ` Peter Korsgaard [not found] ` <87ws8155md.fsf-uXGAPMMVk8amE9MCos8gUmSdvHPH+/yF@public.gmane.org> 2009-05-28 20:41 ` Esben Haabendal 2009-05-28 20:41 ` Esben Haabendal [not found] ` <d2b9ea600905281341s24efa4e3l6277a8aa5dec5bd-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2009-05-28 21:08 ` Peter Korsgaard 2009-05-28 21:08 ` Peter Korsgaard [not found] ` <87octd5415.fsf-uXGAPMMVk8amE9MCos8gUmSdvHPH+/yF@public.gmane.org> 2009-05-28 21:22 ` Esben Haabendal 2009-05-28 21:22 ` Esben Haabendal 2009-12-03 15:09 ` Michael Lawnick 2009-12-03 15:09 ` Michael Lawnick [not found] ` <4B17D4C5.3070100-Mmb7MZpHnFY@public.gmane.org> 2009-12-03 15:29 ` Ben Dooks 2009-12-03 15:29 ` Ben Dooks [not found] ` <20091203152916.GC23152-SMNkleLxa3Z6Wcw2j4pizdi2O/JbrIOy@public.gmane.org> 2009-12-03 15:49 ` Joakim Tjernlund 2009-12-03 15:49 ` Joakim Tjernlund 2009-12-04 6:58 ` Michael Lawnick 2009-12-04 6:58 ` Michael Lawnick 2009-12-03 15:42 ` Joakim Tjernlund 2009-05-28 19:31 ` Grant Likely 2009-05-28 19:31 ` Grant Likely [not found] ` <fa686aa40905281231o26c74a13v250bcedbd066e77b-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2009-05-28 20:15 ` Esben Haabendal 2009-05-28 20:15 ` Esben Haabendal 2009-06-02 22:25 ` Ben Dooks 2009-06-02 23:12 ` Grant Likely 2009-06-02 23:12 ` Grant Likely 2009-06-03 6:01 ` Grant Likely 2009-06-03 6:01 ` Grant Likely [not found] ` <4A124202.4010201-SIcX2qgNSybd/EuGEV170n9LOBIZ5rWg@public.gmane.org> 2009-06-14 13:16 ` Ben Dooks 2009-06-14 13:16 ` Ben Dooks [not found] ` <20090614131605.GK20446-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org> 2009-06-14 14:04 ` Esben Haabendal 2009-06-14 14:04 ` Esben Haabendal
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.