On Wed, 12 Mar 2008 12:56:10 -0500, Seth Forshee <seth.forshee@xxxxxxxxx>
wrote:
> The IRQ handler in omap-i2c.c can sometimes clear status bits without
> actually processing them. In particular, error status bits will be
> ignored if any of the ARDY, RRDY, RDR, XRDY, or XDR bits are
> concurrently set.
>
> Signed-off-by: Seth Forshee <seth.forshee@xxxxxxxxx>
> ---
>
> More information:
>
> I originally noticed this problem on a custom 2430 board I am working
> with. Whenever an i2c chip driver calls i2c_probe() the device would
> be found on both i2c busses at each of the possible addresses for the
> device. I discovered that NACK was being set in the status register
> but omap_i2c_xfer() still returned success because ARDY was also set.
>
> This patch fixes this issue on my board and hasn't caused any problems
> (in my admittedly light i2c usage). I don't have immediate access to
> an SDP board however, so it has not been tested on that platform.
>
> drivers/i2c/busses/i2c-omap.c | 28 +++++++++++++---------------
> 1 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c
b/drivers/i2c/busses/i2c-omap.c
> index 4777466..a16d513 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -590,7 +590,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
> struct omap_i2c_dev *dev = dev_id;
> u16 bits;
> u16 stat, w;
> - int count = 0;
> + int err, count = 0;
int err = 0, count = 0; will be better and you avoid that err=0 before
using err right below.
>
> if (dev->idle)
> return IRQ_NONE;
> @@ -605,10 +605,19 @@ omap_i2c_isr(int this_irq, void *dev_id)
>
> omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, stat);
>
> - if (stat & OMAP_I2C_STAT_ARDY) {
> - omap_i2c_complete_cmd(dev, 0);
> - continue;
> + err = 0;
> + if (stat & OMAP_I2C_STAT_NACK) {
> + err |= OMAP_I2C_STAT_NACK;
> + omap_i2c_write_reg(dev, OMAP_I2C_CON_REG,
> + OMAP_I2C_CON_STP);
> }
> + if (stat & OMAP_I2C_STAT_AL) {
> + dev_err(dev->dev, "Arbitration lost\n");
> + err |= OMAP_I2C_STAT_AL;
> + }
> + if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_NACK |
> + OMAP_I2C_STAT_AL))
err should already hold them, how about
if (stat & err)
> + omap_i2c_complete_cmd(dev, err);
> if (stat & (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR)) {
> u8 num_bytes = 1;
> if (dev->fifo_size) {
> @@ -640,7 +649,6 @@ omap_i2c_isr(int this_irq, void *dev_id)
> }
> }
> omap_i2c_ack_stat(dev, stat & (OMAP_I2C_STAT_RRDY |
> OMAP_I2C_STAT_RDR));
> - continue;
> }
> if (stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)) {
> u8 num_bytes = 1;
> @@ -674,7 +682,6 @@ omap_i2c_isr(int this_irq, void *dev_id)
> omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
> }
> omap_i2c_ack_stat(dev, stat & (OMAP_I2C_STAT_XRDY |
> OMAP_I2C_STAT_XDR));
> - continue;
> }
> if (stat & OMAP_I2C_STAT_ROVR) {
> dev_err(dev->dev, "Receive overrun\n");
> @@ -684,15 +691,6 @@ omap_i2c_isr(int this_irq, void *dev_id)
> dev_err(dev->dev, "Transmit overflow\n");
> dev->cmd_err |= OMAP_I2C_STAT_XUDF;
> }
> - if (stat & OMAP_I2C_STAT_NACK) {
> - omap_i2c_complete_cmd(dev, OMAP_I2C_STAT_NACK);
> - omap_i2c_write_reg(dev, OMAP_I2C_CON_REG,
> - OMAP_I2C_CON_STP);
> - }
> - if (stat & OMAP_I2C_STAT_AL) {
> - dev_err(dev->dev, "Arbitration lost\n");
> - omap_i2c_complete_cmd(dev, OMAP_I2C_STAT_AL);
> - }
> }
>
> return count ? IRQ_HANDLED : IRQ_NONE;
> --
> 1.5.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards,
Felipe Balbi
http://felipebalbi.com
me@xxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
