Re: [PATCH 1/3] gpio: dln2: fix issue when an IRQ is unmasked then enabled
On Mon, Jan 5, 2015 at 6:55 PM, Linus Walleij linus.wall...@linaro.org wrote: On Wed, Dec 31, 2014 at 9:55 PM, Linus Walleij linus.wall...@linaro.org wrote: On Thu, Dec 11, 2014 at 2:02 PM, Octavian Purdila octavian.purd...@intel.com wrote: As noticed during suspend/resume operations, the IRQ can be unmasked then disabled in suspend and eventually enabled in resume, but without being unmasked. The current implementation does not take into account interactions between mask/unmask and enable/disable interrupts, and thus in the above scenarios the IRQs remain unactive. To fix this we removed the enable/disable operations as they fallback to mask/unmask anyway. We also remove the pending bitmaks as it is already done in irq_data (i.e. IRQS_PENDING). Signed-off-by: Octavian Purdila octavian.purd...@intel.com Patch applied for fixes. Bah now that I see there are several versions of the patch set floating around and also MFD patches I don't quite understand how acute this is or how it's to be applied. Hi Linus, Oops I did not noticed you applied the first version. It should not matter anyway since I did not make any modifications to the GPIO patches in the second version - I just doubled checked it now. - Are these regression fixes or nice to have for next kernel release? The first patch is a fix. The second is more of a cleanup patch. - Are the GPIO patches independent of the MFD patch? Yes, the GPIO patches are independent of the MFD patches. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] gpio: dln2: fix issue when an IRQ is unmasked then enabled
On Wed, Dec 31, 2014 at 9:55 PM, Linus Walleij linus.wall...@linaro.org wrote: On Thu, Dec 11, 2014 at 2:02 PM, Octavian Purdila octavian.purd...@intel.com wrote: As noticed during suspend/resume operations, the IRQ can be unmasked then disabled in suspend and eventually enabled in resume, but without being unmasked. The current implementation does not take into account interactions between mask/unmask and enable/disable interrupts, and thus in the above scenarios the IRQs remain unactive. To fix this we removed the enable/disable operations as they fallback to mask/unmask anyway. We also remove the pending bitmaks as it is already done in irq_data (i.e. IRQS_PENDING). Signed-off-by: Octavian Purdila octavian.purd...@intel.com Patch applied for fixes. Bah now that I see there are several versions of the patch set floating around and also MFD patches I don't quite understand how acute this is or how it's to be applied. - Are these regression fixes or nice to have for next kernel release? - Are the GPIO patches independent of the MFD patch? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] gpio: dln2: fix issue when an IRQ is unmasked then enabled
On Thu, Dec 11, 2014 at 2:02 PM, Octavian Purdila octavian.purd...@intel.com wrote: As noticed during suspend/resume operations, the IRQ can be unmasked then disabled in suspend and eventually enabled in resume, but without being unmasked. The current implementation does not take into account interactions between mask/unmask and enable/disable interrupts, and thus in the above scenarios the IRQs remain unactive. To fix this we removed the enable/disable operations as they fallback to mask/unmask anyway. We also remove the pending bitmaks as it is already done in irq_data (i.e. IRQS_PENDING). Signed-off-by: Octavian Purdila octavian.purd...@intel.com Patch applied for fixes. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] gpio: dln2: fix issue when an IRQ is unmasked then enabled
As noticed during suspend/resume operations, the IRQ can be unmasked
then disabled in suspend and eventually enabled in resume, but without
being unmasked.
The current implementation does not take into account interactions
between mask/unmask and enable/disable interrupts, and thus in the
above scenarios the IRQs remain unactive.
To fix this we removed the enable/disable operations as they fallback
to mask/unmask anyway.
We also remove the pending bitmaks as it is already done in irq_data
(i.e. IRQS_PENDING).
Signed-off-by: Octavian Purdila octavian.purd...@intel.com
---
drivers/gpio/gpio-dln2.c | 53 +++-
1 file changed, 7 insertions(+), 46 deletions(-)
diff --git a/drivers/gpio/gpio-dln2.c b/drivers/gpio/gpio-dln2.c
index 978b51e..28a62c4 100644
--- a/drivers/gpio/gpio-dln2.c
+++ b/drivers/gpio/gpio-dln2.c
@@ -64,9 +64,8 @@ struct dln2_gpio {
*/
DECLARE_BITMAP(output_enabled, DLN2_GPIO_MAX_PINS);
- DECLARE_BITMAP(irqs_masked, DLN2_GPIO_MAX_PINS);
- DECLARE_BITMAP(irqs_enabled, DLN2_GPIO_MAX_PINS);
- DECLARE_BITMAP(irqs_pending, DLN2_GPIO_MAX_PINS);
+ /* active IRQs - not synced to hardware */
+ DECLARE_BITMAP(unmasked_irqs, DLN2_GPIO_MAX_PINS);
struct dln2_irq_work *irq_work;
};
@@ -303,29 +302,19 @@ static void dln2_irq_work(struct work_struct *w)
struct dln2_gpio *dln2 = iw-dln2;
u8 type = iw-type DLN2_GPIO_EVENT_MASK;
- if (test_bit(iw-pin, dln2-irqs_enabled))
+ if (test_bit(iw-pin, dln2-unmasked_irqs))
dln2_gpio_set_event_cfg(dln2, iw-pin, type, 0);
else
dln2_gpio_set_event_cfg(dln2, iw-pin, DLN2_GPIO_EVENT_NONE, 0);
}
-static void dln2_irq_enable(struct irq_data *irqd)
-{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
- struct dln2_gpio *dln2 = container_of(gc, struct dln2_gpio, gpio);
- int pin = irqd_to_hwirq(irqd);
-
- set_bit(pin, dln2-irqs_enabled);
- schedule_work(dln2-irq_work[pin].work);
-}
-
-static void dln2_irq_disable(struct irq_data *irqd)
+static void dln2_irq_unmask(struct irq_data *irqd)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
struct dln2_gpio *dln2 = container_of(gc, struct dln2_gpio, gpio);
int pin = irqd_to_hwirq(irqd);
- clear_bit(pin, dln2-irqs_enabled);
+ set_bit(pin, dln2-unmasked_irqs);
schedule_work(dln2-irq_work[pin].work);
}
@@ -335,27 +324,8 @@ static void dln2_irq_mask(struct irq_data *irqd)
struct dln2_gpio *dln2 = container_of(gc, struct dln2_gpio, gpio);
int pin = irqd_to_hwirq(irqd);
- set_bit(pin, dln2-irqs_masked);
-}
-
-static void dln2_irq_unmask(struct irq_data *irqd)
-{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
- struct dln2_gpio *dln2 = container_of(gc, struct dln2_gpio, gpio);
- struct device *dev = dln2-gpio.dev;
- int pin = irqd_to_hwirq(irqd);
-
- if (test_and_clear_bit(pin, dln2-irqs_pending)) {
- int irq;
-
- irq = irq_find_mapping(dln2-gpio.irqdomain, pin);
- if (!irq) {
- dev_err(dev, pin %d not mapped to IRQ\n, pin);
- return;
- }
-
- generic_handle_irq(irq);
- }
+ clear_bit(pin, dln2-unmasked_irqs);
+ schedule_work(dln2-irq_work[pin].work);
}
static int dln2_irq_set_type(struct irq_data *irqd, unsigned type)
@@ -389,8 +359,6 @@ static int dln2_irq_set_type(struct irq_data *irqd,
unsigned type)
static struct irq_chip dln2_gpio_irqchip = {
.name = dln2-irq,
- .irq_enable = dln2_irq_enable,
- .irq_disable = dln2_irq_disable,
.irq_mask = dln2_irq_mask,
.irq_unmask = dln2_irq_unmask,
.irq_set_type = dln2_irq_set_type,
@@ -425,13 +393,6 @@ static void dln2_gpio_event(struct platform_device *pdev,
u16 echo,
return;
}
- if (!test_bit(pin, dln2-irqs_enabled))
- return;
- if (test_bit(pin, dln2-irqs_masked)) {
- set_bit(pin, dln2-irqs_pending);
- return;
- }
-
switch (dln2-irq_work[pin].type) {
case DLN2_GPIO_EVENT_CHANGE_RISING:
if (event-value)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
