On Mon 22 Jan 05:03 PST 2018, Srinivas Ramana wrote: > +static void msm_gpio_irq_enable(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct msm_pinctrl *pctrl = gpiochip_get_data(gc); > + const struct msm_pingroup *g; > + unsigned long flags; > + u32 val; > + > + g = &pctrl->soc->groups[d->hwirq]; > + > + raw_spin_lock_irqsave(&pctrl->lock, flags); > + > + /* > + * clear the interrupt status bit before unmask to avoid > + * any erroneous interrupts that would have got latched > + * when the intterupt is not in use. > + */ > + val = readl(pctrl->regs + g->intr_status_reg); > + val &= ~BIT(g->intr_status_bit); > + writel(val, pctrl->regs + g->intr_status_reg); > + > + val = readl(pctrl->regs + g->intr_cfg_reg); > + val |= BIT(g->intr_enable_bit); > + writel(val, pctrl->regs + g->intr_cfg_reg); > + > + set_bit(d->hwirq, pctrl->enabled_irqs); > + > + raw_spin_unlock_irqrestore(&pctrl->lock, flags); > +} Hi Srinivas, This makes sense, but I would prefer if you extract this code into a common: static void __msm_gpio_irq_unmask(struct msm_pinctrl *pctrl, bool status_clear) which you call from the two callbacks. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html