Re: [PATCH 2/2] i2c: omap: fix "Too much work in one IRQ" irq handling

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]

Hi again,

On Fri, Nov 14, 2014 at 09:47:56PM -0600, Felipe Balbi wrote:
> > > how ? This is an interesting bug which deserves further explanation.
> > 
> > Look at the loops above, and at the omap_i2c_complete_cmd:
> > 
> > static inline void
> > omap_i2c_complete_cmd(struct omap_i2c_dev *dev, u16 err)
> > {
> > 	dev->cmd_err |= err;
> > 	complete(&dev->cmd_complete);
> > }
> > 
> > You can see, loop will be aborted if counter reached 100. Final state
> > of transfer depends on values stored in the 'err' and 'dev->cmd_err'.
> > If 'err' and 'dev->cmd_err' are zero, than transfer would be aborted
> > with status 0.

look at the IRQ handler again:

| omap_i2c_isr_thread(int this_irq, void *dev_id)
| {
| 	struct omap_i2c_dev *dev = dev_id;
| 	unsigned long flags;
| 	u16 bits;
| 	u16 stat;
| 	int err = 0, count = 0;
| 
| 	spin_lock_irqsave(&dev->lock, flags);
| 	do {

[...]

| 		if (stat & OMAP_I2C_STAT_NACK) {
| 			err |= OMAP_I2C_STAT_NACK;
| 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK);
| 			break;
| 		}
| 
| 		if (stat & OMAP_I2C_STAT_AL) {
| 			dev_err(dev->dev, "Arbitration lost\n");
| 			err |= OMAP_I2C_STAT_AL;
| 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_AL);
| 			break;
| 		}

[...]

| 
| 	} while (stat);
| 
| 	omap_i2c_complete_cmd(dev, err);
| 
| out:
| 	spin_unlock_irqrestore(&dev->lock, flags);
| 
| 	return IRQ_HANDLED;
| }

How could I ever call omap_i2c_complete_cmd() with 'err' set as 0 if I
had either a NACK or Arbitration Lost ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]