[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BANLkTi=zKvJx5QMHCsnhtEkiJ2yy649QEQ@mail.gmail.com>
Date: Fri, 13 May 2011 13:23:43 -0500
From: Margarita Olaya <magi@...mlogic.co.uk>
To: Jack Stone <jwjstone@...tmail.fm>
Cc: linux-kernel@...r.kernel.org, Liam Girdwood <lrg@...mlogic.co.uk>,
Mark Brown <broonie@...nsource.wolfsonmicro.com>,
sameo@...ux.intel.com
Subject: Re: [PATCHv2 2/4] tps65912: irq: add interrupt controller
Hi Jack,
On Thu, May 12, 2011 at 2:56 PM, Jack Stone <jwjstone@...tmail.fm> wrote:
> Hi Margarita,
>
> I'm no irq expert so the following might be wrong.
>
> This comment seems to contradict the code - the comment states that all
> interrupts are clear on read but the irq handler explicitly writes the
> value back to the interrupt register.
>
> Did you mean the comment to say that the irq handler explicitly clears
> the interrupts it handles?
Yeap, the current comment is not correct, I will update it, for
tps65912 we need to write 1 to clear the irq.
Thanks for review.
Regards,
Margarita
>
> On 12/05/2011 19:43, Margarita Olaya wrote:
>> +++ b/drivers/mfd/tps65912-irq.c
>> +/*
>> + * This is a threaded IRQ handler so can access I2C/SPI. Since all
>> + * interrupts are clear on read the IRQ line will be reasserted and
>> + * the physical IRQ will be handled again if another interrupt is
>> + * asserted while we run - in the normal course of events this is a
>> + * rare occurrence so we save I2C/SPI reads. We're also assuming that
>> + * it's rare to get lots of interrupts firing simultaneously so try to
>> + * minimise I/O.
>> + */
>> +static irqreturn_t tps65912_irq(int irq, void *irq_data)
>> +{
>> + struct tps65912 *tps65912 = irq_data;
>> + u32 irq_sts;
>> + u32 irq_mask;
>> + u8 reg;
>> + int i;
>> +
>> +
>> + tps65912->read(tps65912, TPS65912_INT_STS, 1, ®);
>> + irq_sts = reg;
>> + tps65912->read(tps65912, TPS65912_INT_STS2, 1, ®);
>> + irq_sts |= reg << 8;
>> + tps65912->read(tps65912, TPS65912_INT_STS3, 1, ®);
>> + irq_sts |= reg << 16;
>> + tps65912->read(tps65912, TPS65912_INT_STS4, 1, ®);
>> + irq_sts |= reg << 24;
>> +
>> + tps65912->read(tps65912, TPS65912_INT_MSK, 1, ®);
>> + irq_mask = reg;
>> + tps65912->read(tps65912, TPS65912_INT_MSK2, 1, ®);
>> + irq_mask |= reg << 8;
>> + tps65912->read(tps65912, TPS65912_INT_MSK3, 1, ®);
>> + irq_mask |= reg << 16;
>> + tps65912->read(tps65912, TPS65912_INT_MSK4, 1, ®);
>> + irq_mask |= reg << 24;
>> +
>> + irq_sts &= ~irq_mask;
>> + if (!irq_sts)
>> + return IRQ_NONE;
>> +
>> + for (i = 0; i < tps65912->irq_num; i++) {
>> + if (!(irq_sts & (1 << i)))
>> + continue;
>> +
>> + handle_nested_irq(tps65912->irq_base + i);
>> + }
>> +
>> + /* Write the STS register back to clear IRQs we handled */
>> + reg = irq_sts & 0xFF;
>> + irq_sts >>= 8;
>> + if (reg)
>> + tps65912->write(tps65912, TPS65912_INT_STS, 1, ®);
>> + reg = irq_sts & 0xFF;
>> + irq_sts >>= 8;
>> + if (reg)
>> + tps65912->write(tps65912, TPS65912_INT_STS2, 1, ®);
>> + reg = irq_sts & 0xFF;
>> + irq_sts >>= 8;
>> + if (reg)
>> + tps65912->write(tps65912, TPS65912_INT_STS3, 1, ®);
>> + reg = irq_sts & 0xFF;
>> + if (reg)
>> + tps65912->write(tps65912, TPS65912_INT_STS4, 1, ®);
>> +
>> + return IRQ_HANDLED;
>> +}
>
> Thanks,
>
> Jack
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists