Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754792Ab1BNRJk (ORCPT ); Mon, 14 Feb 2011 12:09:40 -0500 Received: from www.tglx.de ([62.245.132.106]:57222 "EHLO www.tglx.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752009Ab1BNRJg (ORCPT ); Mon, 14 Feb 2011 12:09:36 -0500 Date: Mon, 14 Feb 2011 18:08:51 +0100 (CET) From: Thomas Gleixner To: LKML cc: Andrew Morton , Gregory Bean , Jean Delvare , Lennert Buytenhek Subject: Re: [patch] drivers: gpio: sx150x: Do not access I2C from mask/unmask functions In-Reply-To: <20110214164634.510293676@linutronix.de> Message-ID: References: <20110214164634.510293676@linutronix.de> User-Agent: Alpine 2.00 (LFD 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4921 Lines: 144 On Mon, 14 Feb 2011, Thomas Gleixner wrote: Ooops. quilt refresh should be mandatory for quilt mail. One day I'll fix that. -------> Subject: drivers: gpio: sx150x: Do not access I2C from mask/unmask functions From: Thomas Gleixner Date: Mon, 14 Feb 2011 16:58:11 +0100 irq_chip->irq_mask/unmask are called with interrupts disabled and irq_desc->lock held. So we cannot access i2c from this context. That's what irq_bus_sync_unlock() is for. Store the masked information in the chip data structure and update the i2c bus from the irq_bus_sync_unlock() callback. This does not need a while(pending) loop because the update to this is always serialized via the bus lock, so we never have more than one pin update pending. Signed-off-by: Thomas Gleixner Cc: Andrew Morton Cc: Gregory Bean Cc: Jean Delvare Cc: Lennert Buytenhek --- drivers/gpio/sx150x.c | 54 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 17 deletions(-) Index: linux-2.6-tip/drivers/gpio/sx150x.c =================================================================== --- linux-2.6-tip.orig/drivers/gpio/sx150x.c +++ linux-2.6-tip/drivers/gpio/sx150x.c @@ -25,6 +25,8 @@ #include #include +#define NO_UPDATE_PENDING -1 + struct sx150x_device_data { u8 reg_pullup; u8 reg_pulldn; @@ -47,8 +49,11 @@ struct sx150x_chip { const struct sx150x_device_data *dev_cfg; int irq_summary; int irq_base; + int irq_update; u32 irq_sense; - unsigned long irq_set_type_pending; + u32 irq_masked; + u32 dev_sense; + u32 dev_masked; struct irq_chip irq_chip; struct mutex lock; }; @@ -312,9 +317,8 @@ static void sx150x_irq_mask(struct irq_d chip = container_of(ic, struct sx150x_chip, irq_chip); n = d->irq - chip->irq_base; - - sx150x_write_cfg(chip, n, 1, chip->dev_cfg->reg_irq_mask, 1); - sx150x_write_cfg(chip, n, 2, chip->dev_cfg->reg_sense, 0); + chip->irq_masked |= (1 << n); + chip->irq_update = n; } static void sx150x_irq_unmask(struct irq_data *d) @@ -326,9 +330,8 @@ static void sx150x_irq_unmask(struct irq chip = container_of(ic, struct sx150x_chip, irq_chip); n = d->irq - chip->irq_base; - sx150x_write_cfg(chip, n, 1, chip->dev_cfg->reg_irq_mask, 0); - sx150x_write_cfg(chip, n, 2, chip->dev_cfg->reg_sense, - chip->irq_sense >> (n * 2)); + chip->irq_masked &= ~(1 << n); + chip->irq_update = n; } static int sx150x_irq_set_type(struct irq_data *d, unsigned int flow_type) @@ -350,7 +353,7 @@ static int sx150x_irq_set_type(struct ir chip->irq_sense &= ~(3UL << (n * 2)); chip->irq_sense |= val << (n * 2); - chip->irq_set_type_pending |= BIT(n); + chip->irq_update = n; return 0; } @@ -404,15 +407,29 @@ static void sx150x_irq_bus_sync_unlock(s chip = container_of(ic, struct sx150x_chip, irq_chip); - while (chip->irq_set_type_pending) { - n = __ffs(chip->irq_set_type_pending); - chip->irq_set_type_pending &= ~BIT(n); - if (!(irq_to_desc(n + chip->irq_base)->status & IRQ_MASKED)) - sx150x_write_cfg(chip, n, 2, - chip->dev_cfg->reg_sense, - chip->irq_sense >> (n * 2)); - } + if (chip->irq_update == NO_UPDATE_PENDING) + goto out; + n = chip->irq_update; + chip->irq_update = NO_UPDATE_PENDING; + + /* Avoid updates if nothing changed */ + if (chip->dev_sense == chip->irq_sense && + chip->dev_sense == chip->irq_masked) + goto out; + + chip->dev_sense = chip->irq_sense; + chip->dev_masked = chip->irq_masked; + + if (chip->irq_masked & (1 << n)) { + sx150x_write_cfg(chip, n, 1, chip->dev_cfg->reg_irq_mask, 1); + sx150x_write_cfg(chip, n, 2, chip->dev_cfg->reg_sense, 0); + } else { + sx150x_write_cfg(chip, n, 1, chip->dev_cfg->reg_irq_mask, 0); + sx150x_write_cfg(chip, n, 2, chip->dev_cfg->reg_sense, + chip->irq_sense >> (n * 2)); + } +out: mutex_unlock(&chip->lock); } @@ -445,8 +462,11 @@ static void sx150x_init_chip(struct sx15 chip->irq_chip.irq_bus_sync_unlock = sx150x_irq_bus_sync_unlock; chip->irq_summary = -1; chip->irq_base = -1; + chip->irq_masked = ~0; chip->irq_sense = 0; - chip->irq_set_type_pending = 0; + chip->dev_masked = ~0; + chip->dev_sense = 0; + chip->irq_update = NO_UPDATE_PENDING; } static int sx150x_init_io(struct sx150x_chip *chip, u8 base, u16 cfg) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/