Re: [V3, 1/2] i2c: brcmstb: Add Broadcom settop SoC i2c controller driver

[Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

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