| Message ID | 20160314142607.GI1793@lahna.fi.intel.com |
|---|---|
| State | New |
| Headers | show |
> 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, > }; > The gpio_irq_enable callback way looks better. I will do the test with this solution and submit it if the unit test passed (no unexpected gpio interrupt). Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,