2013-10-15 12:26:37

by Yi Zhang

[permalink] [raw]
Subject: [PATCH v2] regmap: irq: clear status when disable irq

clear the status bit if the mask register doesn't prevent
the chip level irq from being asserted

OR in the following sequence, there will be irq storm happens:
1) interrupt is triggered;
2) another thread disables it(the mask bit is set);
3) _Then_ the interrupt thread is not ACKed(the status bit is not cleared),
and it's ignored;
4) if the irq is still asserted because of the uncleared status bit,
the irq storm happens;

Signed-off-by: Yi Zhang <[email protected]>
---
drivers/base/regmap/regmap-irq.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index d10456f..9b0148f 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -61,7 +61,7 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
{
struct regmap_irq_chip_data *d = irq_data_get_irq_chip_data(data);
struct regmap *map = d->map;
- int i, ret;
+ int i, j, ret, bits_length;
u32 reg;

if (d->chip->runtime_pm) {
@@ -105,6 +105,31 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
"Failed to sync wakes in %x: %d\n",
reg, ret);
}
+
+ if (!d->chip->init_ack_masked)
+ continue;
+ /*
+ * Ack all the masked interrupts uncondictionly,
+ * OR if there is masked interrupt which hasn't been Acked,
+ * it'll be ignored in irq handler, then may introduce irq storm
+ */
+ if (d->chip->ack_base) {
+ reg = d->chip->ack_base +
+ (i * map->reg_stride * d->irq_reg_stride);
+
+ bits_length = d->map->format.val_bytes * BITS_PER_BYTE;
+ for (j = 0; j < bits_length; j++) {
+ if (!(d->mask_buf[i] & (0x1 << j))) {
+ ret = regmap_update_bits(d->map, reg,
+ (0x1 << j), 0);
+ if (ret != 0)
+ dev_err(d->map->dev,
+ "Failed to ack 0x%x: %d\n",
+ reg, ret);
+ }
+ }
+ /* no need to sync status_buf, update in irq handler */
+ }
}

if (d->chip->runtime_pm)
--
1.7.9.5


2013-10-18 00:11:07

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2] regmap: irq: clear status when disable irq

On Tue, Oct 15, 2013 at 08:23:30PM +0800, Yi Zhang wrote:

I'm still not sure this is doing the right thing.

> + for (j = 0; j < bits_length; j++) {
> + if (!(d->mask_buf[i] & (0x1 << j))) {

This is checking to see if the bit is masked...

> + ret = regmap_update_bits(d->map, reg,
> + (0x1 << j), 0);

...then writing a zero to that bit. For most chips with a write 1 to
clear acknowledge this will result in all set bits in the register being
acked except the currently masked one (though if there are two masked
bits all will be acked).

It would be much quicker to just write mask_buf[i] back to the device to
acknoweldge all masked bits at once rather than try to do it
individually and the result should be the same unless the chip requires
us to set zero (which probably ought to be a quirk).


Attachments:
(No filename) (820.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments