Hi Doug,
On Wednesday 12 of June 2013 10:33:19 Doug Anderson wrote:
> A level-triggered interrupt should be acked after the interrupt line
> becomes inactive and before it is unmasked, or else another interrupt
> will be immediately triggered. Acking before or after calling the
> handler is not enough.
Nice catch.
I guess that pinctrl-s3c64xx will need similar fix as well, won't it?
Also one comment inline.
> Signed-off-by: Luigi Semenzato <semenz...@chromium.org>
> Signed-off-by: Doug Anderson <diand...@chromium.org>
> ---
> drivers/pinctrl/pinctrl-exynos.c | 42
> ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42
> insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-exynos.c
> b/drivers/pinctrl/pinctrl-exynos.c index c0729a3..67b7a27 100644
> --- a/drivers/pinctrl/pinctrl-exynos.c
> +++ b/drivers/pinctrl/pinctrl-exynos.c
> @@ -81,11 +81,32 @@ static void exynos_gpio_irq_unmask(struct irq_data
> *irqd) struct samsung_pin_bank *bank =
> irq_data_get_irq_chip_data(irqd); struct samsung_pinctrl_drv_data *d =
> bank->drvdata;
> unsigned long reg_mask = d->ctrl->geint_mask + bank->eint_offset;
> + unsigned long reg_con = d->ctrl->geint_con + bank->eint_offset;
> + unsigned int pin = irqd->hwirq;
> + unsigned int shift = EXYNOS_EINT_CON_LEN * pin;
> + unsigned int con, trig_type;
> unsigned long mask;
> unsigned long flags;
>
> spin_lock_irqsave(&bank->slock, flags);
>
> + /*
> + * Ack level interrupts right before unmask
> + *
> + * If we don't do this we'll get a double-interrupt. Level
triggered
> + * interrupts must not fire an interrupt if the level is not
> + * _currently_ active, even if it was active while the interrupt
was
> + * masked.
> + */
> + con = readl(d->virt_base + reg_con);
> + trig_type = (con >> shift) & EXYNOS_EINT_CON_MASK;
> + switch (trig_type) {
> + case EXYNOS_EINT_LEVEL_HIGH:
> + case EXYNOS_EINT_LEVEL_LOW:
> + exynos_gpio_irq_ack(irqd);
> + break;
> + }
I think you can eliminate most of the code by doing this following way:
if (irqd_get_trigger_type(irqd) & IRQ_TYPE_LEVEL_MASK)
exynos_gpio_irq_ack(irqd);
Best regards,
Tomasz
> +
> mask = readl(d->virt_base + reg_mask);
> mask &= ~(1 << irqd->hwirq);
> writel(mask, d->virt_base + reg_mask);
> @@ -299,11 +320,32 @@ static void exynos_wkup_irq_unmask(struct irq_data
> *irqd) struct samsung_pin_bank *b = irq_data_get_irq_chip_data(irqd);
> struct samsung_pinctrl_drv_data *d = b->drvdata;
> unsigned long reg_mask = d->ctrl->weint_mask + b->eint_offset;
> + unsigned long reg_con = d->ctrl->weint_con + b->eint_offset;
> + unsigned int pin = irqd->hwirq;
> + unsigned long shift = EXYNOS_EINT_CON_LEN * pin;
> + unsigned long con, trig_type;
> unsigned long mask;
> unsigned long flags;
>
> spin_lock_irqsave(&b->slock, flags);
>
> + /*
> + * Ack level interrupts right before unmask
> + *
> + * If we don't do this we'll get a double-interrupt. Level
triggered
> + * interrupts must not fire an interrupt if the level is not
> + * _currently_ active, even if it was active while the interrupt
was
> + * masked.
> + */
> + con = readl(d->virt_base + reg_con);
> + trig_type = (con >> shift) & EXYNOS_EINT_CON_MASK;
> + switch (trig_type) {
> + case EXYNOS_EINT_LEVEL_HIGH:
> + case EXYNOS_EINT_LEVEL_LOW:
> + exynos_wkup_irq_ack(irqd);
> + break;
> + }
> +
> mask = readl(d->virt_base + reg_mask);
> mask &= ~(1 << irqd->hwirq);
> writel(mask, d->virt_base + reg_mask);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
