Re: [PATCH 2/3] pinctrl:Intel: clear interrupt status for every IRQ setup

["Westerberg, Mika" <mika.westerberg@intel.com>]

On Mon, Mar 14, 2016 at 03:00:41PM +0200, Westerberg, Mika wrote:
> 
> > > Your set_type() is supporting edges but have all IRQs
> > > handled by handle_simple_irq() rather than handle_edge_irq()
> > > for the edges, which gives a more robust control flow
> > > from IRQ to ACK to calling the handler.
> > > 
> > > Zheng/Mika: please look at how the level/edge
> > > IRQs are handled in drivers/gpio/gpio-pl061.c
> > > where I *tried* to do things right, switching handler
> > > in .set_type() using irq_set_handler_locked(). I think
> > > you may need to use handle_edge_irq() for the edge IRQs
> > > and handle_level_irq() for the level IRQs just like I do
> > > in the PL061 driver.
> > 
> > The driver is already doing that as far as I can tell (see
> > intel_gpio_irq_type()).
> 
> I will check again if we are still missing something there.

Maybe we can implement ->enable() that clears the status right before
interrupt is unmasked? Something like below.

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index c0f5586218c4..b4873a4e25d5 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -648,6 +648,33 @@ static const struct gpio_chip intel_gpio_chip = {
 	.set = intel_gpio_set,
 };
 
+static void intel_gpio_irq_enable(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
+	const struct intel_community *community;
+	unsigned pin = irqd_to_hwirq(d);
+	unsigned long flags;
+
+	spin_lock_irqsave(&pctrl->lock, flags);
+
+	community = intel_get_community(pctrl, pin);
+	if (community) {
+		unsigned padno = pin_to_padno(community, pin);
+		unsigned gpp_offset = padno % community->gpp_size;
+		unsigned gpp = padno / community->gpp_size;
+		unsigned value;
+
+		writel(BIT(gpp_offset), community->regs + GPI_IS + gpp * 4);
+
+		value = readl(community->regs + community->ie_offset + gpp * 4);
+		value |= BIT(gpp_offset);
+		writel(value, community->regs + community->ie_offset + gpp * 4);
+	}
+
+	spin_unlock_irqrestore(&pctrl->lock, flags);
+}
+
 static void intel_gpio_irq_ack(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
@@ -856,6 +883,7 @@ static irqreturn_t intel_gpio_irq(int irq, void *data)
 
 static struct irq_chip intel_gpio_irqchip = {
 	.name = "intel-gpio",
+	.irq_enable = intel_gpio_irq_enable,
 	.irq_ack = intel_gpio_irq_ack,
 	.irq_mask = intel_gpio_irq_mask,
 	.irq_unmask = intel_gpio_irq_unmask,