* Re: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver [not found] <D99582E5322435468A77E74BB0039E7B1D22784CF0@SRV02.hamburg.garz-fricke.de> @ 2012-04-26 11:01 ` Nikolaus Voss 0 siblings, 0 replies; 9+ messages in thread From: Nikolaus Voss @ 2012-04-26 11:01 UTC (permalink / raw) To: Carsten Behling Cc: Marc-Oliver Westerburg, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, h.feurstein-Re5JQEeQqe8AvxtiuMwx3w, w.sang-bIcnvbaLZ9MEGnE8C9+IrQ Hi Carsten, On Wed, 25 Apr 2012, Carsten Behling wrote: > I cannot find the fix for the RXRDY overrun bug in this patch: The fix refers to the bug when RXRDY flag was masked out by TXCOMP, reported by Hubert. I hope this is fixed with v10. [...] > here you only report the status flags after a complete transfer, but > an overrun can happen after every byte. After reading the status > register after an overrun for the next byte this info will be lost ! Right, I didn't consider this. Would you mind to test if the attached patch on top of v10 fixes your problem? Thanks, Niko P.S. Are you using a RM9200? Seems that this SOC has some problems... diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c index 97f4365..df0f6ed 100644 --- a/drivers/i2c/busses/i2c-at91.c +++ b/drivers/i2c/busses/i2c-at91.c @@ -186,9 +186,11 @@ static irqreturn_t atmel_twi_interrupt(int irq, void *dev_id) else if (irqstatus & AT91_TWI_TXRDY) at91_twi_write_next_byte(dev); + /* catch error flags */ + dev->transfer_status |= status; + if (irqstatus & AT91_TWI_TXCOMP) { at91_disable_twi_interrupts(dev); - dev->transfer_status = status; complete(&dev->cmd_complete); } @@ -203,6 +205,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev) (dev->msg->flags & I2C_M_RD) ? "read" : "write", dev->buf_len); INIT_COMPLETION(dev->cmd_complete); + dev->transfer_status = 0; if (dev->msg->flags & I2C_M_RD) { unsigned start_flags = AT91_TWI_START; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver @ 2012-04-26 11:01 ` Nikolaus Voss 0 siblings, 0 replies; 9+ messages in thread From: Nikolaus Voss @ 2012-04-26 11:01 UTC (permalink / raw) To: Carsten Behling Cc: Marc-Oliver Westerburg, linux-arm-kernel, linux-kernel, linux-i2c, h.feurstein, w.sang Hi Carsten, On Wed, 25 Apr 2012, Carsten Behling wrote: > I cannot find the fix for the RXRDY overrun bug in this patch: The fix refers to the bug when RXRDY flag was masked out by TXCOMP, reported by Hubert. I hope this is fixed with v10. [...] > here you only report the status flags after a complete transfer, but > an overrun can happen after every byte. After reading the status > register after an overrun for the next byte this info will be lost ! Right, I didn't consider this. Would you mind to test if the attached patch on top of v10 fixes your problem? Thanks, Niko P.S. Are you using a RM9200? Seems that this SOC has some problems... diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c index 97f4365..df0f6ed 100644 --- a/drivers/i2c/busses/i2c-at91.c +++ b/drivers/i2c/busses/i2c-at91.c @@ -186,9 +186,11 @@ static irqreturn_t atmel_twi_interrupt(int irq, void *dev_id) else if (irqstatus & AT91_TWI_TXRDY) at91_twi_write_next_byte(dev); + /* catch error flags */ + dev->transfer_status |= status; + if (irqstatus & AT91_TWI_TXCOMP) { at91_disable_twi_interrupts(dev); - dev->transfer_status = status; complete(&dev->cmd_complete); } @@ -203,6 +205,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev) (dev->msg->flags & I2C_M_RD) ? "read" : "write", dev->buf_len); INIT_COMPLETION(dev->cmd_complete); + dev->transfer_status = 0; if (dev->msg->flags & I2C_M_RD) { unsigned start_flags = AT91_TWI_START; ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <D99582E5322435468A77E74BB0039E7B1D22784E8B@SRV02.hamburg.garz-fricke.de>]
* RE: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver [not found] ` <D99582E5322435468A77E74BB0039E7B1D22784E8B@SRV02.hamburg.garz-fricke.de> @ 2012-04-27 8:04 ` Voss, Nikolaus 0 siblings, 0 replies; 9+ messages in thread From: Voss, Nikolaus @ 2012-04-27 8:04 UTC (permalink / raw) To: 'Carsten Behling' Cc: 'devel@ayanes.com', 'Wolfram Sang', 'linux-kernel@vger.kernel.org', 'Hubert Feurstein', 'linux-i2c@vger.kernel.org', 'Marc-Oliver Westerburg', 'linux-arm-kernel@lists.infradead.org' Hi Carsten, Carsten Behling wrote on 2012-04-27: >> INIT_COMPLETION(dev->cmd_complete); >> + dev->transfer_status = 0; >> if (dev->msg->flags & I2C_M_RD) { >> unsigned start_flags = AT91_TWI_START; > > this patch will not work, because you reset 'dev->transfer_status' > before it is evaluated on errors (AT91_TWI_NACK, AT91_TWI_OVRE). it should, because transfer_status is updated by the ISR between wait_for_completion_interruptible_timeout() and the error evaluation. Please try it out. > > P.S. Are you using a RM9200? Seems that this SOC has some problems... > > Yes, our ECO920 uses the RM9200, but we support it with an very old kernel > (2.6.21). I remember there were many problems with I2C. Ok, but good to hear the driver basically works with the RM9200. Niko ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver @ 2012-04-27 8:04 ` Voss, Nikolaus 0 siblings, 0 replies; 9+ messages in thread From: Voss, Nikolaus @ 2012-04-27 8:04 UTC (permalink / raw) To: linux-arm-kernel Hi Carsten, Carsten Behling wrote on 2012-04-27: >> INIT_COMPLETION(dev->cmd_complete); >> + dev->transfer_status = 0; >> if (dev->msg->flags & I2C_M_RD) { >> unsigned start_flags = AT91_TWI_START; > > this patch will not work, because you reset 'dev->transfer_status' > before it is evaluated on errors (AT91_TWI_NACK, AT91_TWI_OVRE). it should, because transfer_status is updated by the ISR between wait_for_completion_interruptible_timeout() and the error evaluation. Please try it out. > > P.S. Are you using a RM9200? Seems that this SOC has some problems... > > Yes, our ECO920 uses the RM9200, but we support it with an very old kernel > (2.6.21). I remember there were many problems with I2C. Ok, but good to hear the driver basically works with the RM9200. Niko ^ permalink raw reply [flat|nested] 9+ messages in thread
* AW: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver 2012-04-27 8:04 ` Voss, Nikolaus (?) @ 2012-04-27 8:36 ` Carsten Behling -1 siblings, 0 replies; 9+ messages in thread From: Carsten Behling @ 2012-04-27 8:36 UTC (permalink / raw) To: Voss, Nikolaus Cc: 'devel@ayanes.com', 'Wolfram Sang', 'linux-kernel@vger.kernel.org', 'Hubert Feurstein', 'linux-i2c@vger.kernel.org', Marc-Oliver Westerburg, 'linux-arm-kernel@lists.infradead.org' Hi Niko, you a right. Driver works with that patch. Mit freundlichen Grüßen / Best regards Carsten Behling Development Engineer Garz & Fricke GmbH Tempowerkring 2, 21079 Hamburg - Germany Amtsgericht Hamburg HRB 60514 Geschäftsführer: Manfred Garz, Matthias Fricke Phone: +49 (0) 40 791 899 - 56 Fax: +49 40 / 791 899 - 39 www.garz-fricke.com -----Ursprüngliche Nachricht----- Von: Voss, Nikolaus [mailto:N.Voss@weinmann.de] Gesendet: Freitag, 27. April 2012 10:05 An: Carsten Behling Cc: Marc-Oliver Westerburg; 'devel@ayanes.com'; 'Wolfram Sang'; 'linux-arm-kernel@lists.infradead.org'; 'linux-kernel@vger.kernel.org'; 'linux-i2c@vger.kernel.org'; 'Hubert Feurstein' Betreff: RE: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver Hi Carsten, Carsten Behling wrote on 2012-04-27: >> INIT_COMPLETION(dev->cmd_complete); >> + dev->transfer_status = 0; >> if (dev->msg->flags & I2C_M_RD) { >> unsigned start_flags = AT91_TWI_START; > > this patch will not work, because you reset 'dev->transfer_status' > before it is evaluated on errors (AT91_TWI_NACK, AT91_TWI_OVRE). it should, because transfer_status is updated by the ISR between wait_for_completion_interruptible_timeout() and the error evaluation. Please try it out. > > P.S. Are you using a RM9200? Seems that this SOC has some problems... > > Yes, our ECO920 uses the RM9200, but we support it with an very old kernel > (2.6.21). I remember there were many problems with I2C. Ok, but good to hear the driver basically works with the RM9200. Niko ^ permalink raw reply [flat|nested] 9+ messages in thread
* AW: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver @ 2012-04-27 8:36 ` Carsten Behling 0 siblings, 0 replies; 9+ messages in thread From: Carsten Behling @ 2012-04-27 8:36 UTC (permalink / raw) To: Voss, Nikolaus Cc: Marc-Oliver Westerburg, 'devel@ayanes.com', 'Wolfram Sang', 'linux-arm-kernel@lists.infradead.org', 'linux-kernel@vger.kernel.org', 'linux-i2c@vger.kernel.org', 'Hubert Feurstein' Hi Niko, you a right. Driver works with that patch. Mit freundlichen Grüßen / Best regards Carsten Behling Development Engineer Garz & Fricke GmbH Tempowerkring 2, 21079 Hamburg - Germany Amtsgericht Hamburg HRB 60514 Geschäftsführer: Manfred Garz, Matthias Fricke Phone: +49 (0) 40 791 899 - 56 Fax: +49 40 / 791 899 - 39 www.garz-fricke.com -----Ursprüngliche Nachricht----- Von: Voss, Nikolaus [mailto:N.Voss@weinmann.de] Gesendet: Freitag, 27. April 2012 10:05 An: Carsten Behling Cc: Marc-Oliver Westerburg; 'devel@ayanes.com'; 'Wolfram Sang'; 'linux-arm-kernel@lists.infradead.org'; 'linux-kernel@vger.kernel.org'; 'linux-i2c@vger.kernel.org'; 'Hubert Feurstein' Betreff: RE: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver Hi Carsten, Carsten Behling wrote on 2012-04-27: >> INIT_COMPLETION(dev->cmd_complete); >> + dev->transfer_status = 0; >> if (dev->msg->flags & I2C_M_RD) { >> unsigned start_flags = AT91_TWI_START; > > this patch will not work, because you reset 'dev->transfer_status' > before it is evaluated on errors (AT91_TWI_NACK, AT91_TWI_OVRE). it should, because transfer_status is updated by the ISR between wait_for_completion_interruptible_timeout() and the error evaluation. Please try it out. > > P.S. Are you using a RM9200? Seems that this SOC has some problems... > > Yes, our ECO920 uses the RM9200, but we support it with an very old kernel > (2.6.21). I remember there were many problems with I2C. Ok, but good to hear the driver basically works with the RM9200. Niko ^ permalink raw reply [flat|nested] 9+ messages in thread
* AW: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver @ 2012-04-27 8:36 ` Carsten Behling 0 siblings, 0 replies; 9+ messages in thread From: Carsten Behling @ 2012-04-27 8:36 UTC (permalink / raw) To: linux-arm-kernel Hi Niko, you a right. Driver works with that patch. Mit freundlichen Gr??en / Best regards Carsten Behling Development Engineer Garz & Fricke GmbH Tempowerkring 2, 21079 Hamburg - Germany Amtsgericht Hamburg HRB 60514 Gesch?ftsf?hrer: Manfred Garz, Matthias Fricke Phone: +49 (0) 40 791 899 - 56 Fax: +49 40 / 791 899 - 39 www.garz-fricke.com -----Urspr?ngliche Nachricht----- Von: Voss, Nikolaus [mailto:N.Voss at weinmann.de] Gesendet: Freitag, 27. April 2012 10:05 An: Carsten Behling Cc: Marc-Oliver Westerburg; 'devel at ayanes.com'; 'Wolfram Sang'; 'linux-arm-kernel at lists.infradead.org'; 'linux-kernel at vger.kernel.org'; 'linux-i2c at vger.kernel.org'; 'Hubert Feurstein' Betreff: RE: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver Hi Carsten, Carsten Behling wrote on 2012-04-27: >> INIT_COMPLETION(dev->cmd_complete); >> + dev->transfer_status = 0; >> if (dev->msg->flags & I2C_M_RD) { >> unsigned start_flags = AT91_TWI_START; > > this patch will not work, because you reset 'dev->transfer_status' > before it is evaluated on errors (AT91_TWI_NACK, AT91_TWI_OVRE). it should, because transfer_status is updated by the ISR between wait_for_completion_interruptible_timeout() and the error evaluation. Please try it out. > > P.S. Are you using a RM9200? Seems that this SOC has some problems... > > Yes, our ECO920 uses the RM9200, but we support it with an very old kernel > (2.6.21). I remember there were many problems with I2C. Ok, but good to hear the driver basically works with the RM9200. Niko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: AW: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver 2012-04-27 8:36 ` Carsten Behling @ 2012-05-09 19:07 ` Adrian Yanes -1 siblings, 0 replies; 9+ messages in thread From: Adrian Yanes @ 2012-05-09 19:07 UTC (permalink / raw) To: linux-arm-kernel; +Cc: linux-kernel, linux-i2c >> Yes, our ECO920 uses the RM9200, but we support it with an very old kernel >> (2.6.21). I remember there were many problems with I2C. I really doubt this, as I indicated in a previous email[1], the RM9200 has a hardware "bug". So either you performed the testing with less than 2 bytes of data and in a really low frequency (<50kHz) or we are missing something. Can you provide more details about which kind of testing you performed? Thanks 1 - http://article.gmane.org/gmane.linux.kernel/1287445/match= ^ permalink raw reply [flat|nested] 9+ messages in thread
* AW: [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver @ 2012-05-09 19:07 ` Adrian Yanes 0 siblings, 0 replies; 9+ messages in thread From: Adrian Yanes @ 2012-05-09 19:07 UTC (permalink / raw) To: linux-arm-kernel >> Yes, our ECO920 uses the RM9200, but we support it with an very old kernel >> (2.6.21). I remember there were many problems with I2C. I really doubt this, as I indicated in a previous email[1], the RM9200 has a hardware "bug". So either you performed the testing with less than 2 bytes of data and in a really low frequency (<50kHz) or we are missing something. Can you provide more details about which kind of testing you performed? Thanks 1 - http://article.gmane.org/gmane.linux.kernel/1287445/match= ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-05-09 19:07 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <D99582E5322435468A77E74BB0039E7B1D22784CF0@SRV02.hamburg.garz-fricke.de>
[not found] ` <D99582E5322435468A77E74BB0039E7B1D22784CF0-6hi2BFpdtA3X4XV0xjfKZxOngtiEHj0sqGK+G5UkneY@public.gmane.org>
2012-04-26 11:01 ` [PATCH v10 3/4] drivers/i2c/busses/i2c-at91.c: add new driver Nikolaus Voss
2012-04-26 11:01 ` Nikolaus Voss
[not found] ` <D99582E5322435468A77E74BB0039E7B1D22784E8B@SRV02.hamburg.garz-fricke.de>
2012-04-27 8:04 ` Voss, Nikolaus
2012-04-27 8:04 ` Voss, Nikolaus
2012-04-27 8:36 ` AW: " Carsten Behling
2012-04-27 8:36 ` Carsten Behling
2012-04-27 8:36 ` Carsten Behling
2012-05-09 19:07 ` Adrian Yanes
2012-05-09 19:07 ` Adrian Yanes
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.