On 15/05/15 10:46, Ray Jui wrote: [snip] >> + ret = -ETIMEDOUT; >> + } else { >> + /* we are in polling mode */ >> + u32 bsc_intrp; >> + unsigned long time_left = jiffies + timeout; >> + >> + do { >> + bsc_intrp = bsc_readl(dev, iic_enable) & >> + BSC_IIC_EN_INTRP_MASK; >> + if (time_after(jiffies, time_left)) { >> + ret = -ETIMEDOUT; >> + break; >> + } >> + cpu_relax(); >> + } while (!bsc_intrp); >> + brcmstb_i2c_enable_disable_irq(dev, INT_DISABLE); > > I had a question on this during the previous review. You said you will > investigate but never really answer my question or add a comment in the > code to explain it. My question was, why do you need to disable > interrupt in the end of the polling loop? Do you use interrupt at all in > polling mode? If so, why? I suspect this may have to do with the special Level 2 interrupt controller (drivers/irqchip/irq-bcm7120-l2.c) used between the Level 1 interrupt signaled to the CPU (<irq-bcm7038-l1.c or ARM GIC) and the interrupt generation on the BSC controller side. The L2 interrupt controller only signals interrupts, but the actual acknowledgement of the interrupt needs to happen at the BSC controller level, if we do not do that, I suspect the controller keep ssignaling a pending interrupt, so we would need to clear it to move forward with next events. -- Florian -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html