| Message ID | 20180924143212.81251-2-mika.westerberg@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] pinctrl: cannonlake: Fix HOSTSW_OWN register offset of H variant | expand |
On Mon, Sep 24, 2018 at 05:32:12PM +0300, Mika Westerberg wrote: > This reverts commit 55aedef50d4d810670916d9fce4a40d5da2079e7. > > Commit 55aedef50d4d ("pinctrl: intel: Do pin translation when lock IRQ") > added special translation from GPIO number to hardware pin number to > irq_reqres/relres hooks to avoid failure when IRQs are requested. The > actual failure happened inside gpiochip_lock_as_irq() because it calls > gpiod_get_direction() and pinctrl-intel.c::intel_gpio_get_direction() > implementation originally missed the translation so the two hooks made > it work by skipping the ->get_direction() call entirely (it overwrote > the default GPIOLIB provided functions). > > The proper fix that adds translation to GPIO callbacks was merged with > commit 96147db1e1df ("pinctrl: intel: Do pin translation in other GPIO > operations as well"). This allows us to use the default GPIOLIB provided > functions again. > > In addition as find out by Benjamin Tissoires the two functions > (intel_gpio_irq_reqres()/intel_gpio_irq_relres()) now cause problems of > their own because they operate on pin numbers and pass that pin number > to gpiochip_lock_as_irq() which actually expects a GPIO number. > Since I was participating in the investigation, Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=199911 > Fixes: 55aedef50d4d ("pinctrl: intel: Do pin translation when lock IRQ") > Reported-and-tested-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/pinctrl/intel/pinctrl-intel.c | 32 --------------------------- > 1 file changed, 32 deletions(-) > > diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c > index f848db65169b..692c2bd1faed 100644 > --- a/drivers/pinctrl/intel/pinctrl-intel.c > +++ b/drivers/pinctrl/intel/pinctrl-intel.c > @@ -890,36 +890,6 @@ static const struct gpio_chip intel_gpio_chip = { > .set_config = gpiochip_generic_config, > }; > > -static int intel_gpio_irq_reqres(struct irq_data *d) > -{ > - struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > - struct intel_pinctrl *pctrl = gpiochip_get_data(gc); > - int pin; > - int ret; > - > - pin = intel_gpio_to_pin(pctrl, irqd_to_hwirq(d), NULL, NULL); > - if (pin >= 0) { > - ret = gpiochip_lock_as_irq(gc, pin); > - if (ret) { > - dev_err(pctrl->dev, "unable to lock HW IRQ %d for IRQ\n", > - pin); > - return ret; > - } > - } > - return 0; > -} > - > -static void intel_gpio_irq_relres(struct irq_data *d) > -{ > - struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > - struct intel_pinctrl *pctrl = gpiochip_get_data(gc); > - int pin; > - > - pin = intel_gpio_to_pin(pctrl, irqd_to_hwirq(d), NULL, NULL); > - if (pin >= 0) > - gpiochip_unlock_as_irq(gc, pin); > -} > - > static void intel_gpio_irq_ack(struct irq_data *d) > { > struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > @@ -1135,8 +1105,6 @@ static irqreturn_t intel_gpio_irq(int irq, void *data) > > static struct irq_chip intel_gpio_irqchip = { > .name = "intel-gpio", > - .irq_request_resources = intel_gpio_irq_reqres, > - .irq_release_resources = intel_gpio_irq_relres, > .irq_enable = intel_gpio_irq_enable, > .irq_ack = intel_gpio_irq_ack, > .irq_mask = intel_gpio_irq_mask, > -- > 2.18.0 >
On Mon, Sep 24, 2018 at 4:32 PM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > This reverts commit 55aedef50d4d810670916d9fce4a40d5da2079e7. > > Commit 55aedef50d4d ("pinctrl: intel: Do pin translation when lock IRQ") > added special translation from GPIO number to hardware pin number to > irq_reqres/relres hooks to avoid failure when IRQs are requested. The > actual failure happened inside gpiochip_lock_as_irq() because it calls > gpiod_get_direction() and pinctrl-intel.c::intel_gpio_get_direction() > implementation originally missed the translation so the two hooks made > it work by skipping the ->get_direction() call entirely (it overwrote > the default GPIOLIB provided functions). > > The proper fix that adds translation to GPIO callbacks was merged with > commit 96147db1e1df ("pinctrl: intel: Do pin translation in other GPIO > operations as well"). This allows us to use the default GPIOLIB provided > functions again. > > In addition as find out by Benjamin Tissoires the two functions > (intel_gpio_irq_reqres()/intel_gpio_irq_relres()) now cause problems of > their own because they operate on pin numbers and pass that pin number > to gpiochip_lock_as_irq() which actually expects a GPIO number. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=199911 > Fixes: 55aedef50d4d ("pinctrl: intel: Do pin translation when lock IRQ") > Reported-and-tested-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Patch applied for fixes with Andy's ACK. Yours, Linus Walleij
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c index f848db65169b..692c2bd1faed 100644 --- a/drivers/pinctrl/intel/pinctrl-intel.c +++ b/drivers/pinctrl/intel/pinctrl-intel.c @@ -890,36 +890,6 @@ static const struct gpio_chip intel_gpio_chip = { .set_config = gpiochip_generic_config, }; -static int intel_gpio_irq_reqres(struct irq_data *d) -{ - struct gpio_chip *gc = irq_data_get_irq_chip_data(d); - struct intel_pinctrl *pctrl = gpiochip_get_data(gc); - int pin; - int ret; - - pin = intel_gpio_to_pin(pctrl, irqd_to_hwirq(d), NULL, NULL); - if (pin >= 0) { - ret = gpiochip_lock_as_irq(gc, pin); - if (ret) { - dev_err(pctrl->dev, "unable to lock HW IRQ %d for IRQ\n", - pin); - return ret; - } - } - return 0; -} - -static void intel_gpio_irq_relres(struct irq_data *d) -{ - struct gpio_chip *gc = irq_data_get_irq_chip_data(d); - struct intel_pinctrl *pctrl = gpiochip_get_data(gc); - int pin; - - pin = intel_gpio_to_pin(pctrl, irqd_to_hwirq(d), NULL, NULL); - if (pin >= 0) - gpiochip_unlock_as_irq(gc, pin); -} - static void intel_gpio_irq_ack(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); @@ -1135,8 +1105,6 @@ static irqreturn_t intel_gpio_irq(int irq, void *data) static struct irq_chip intel_gpio_irqchip = { .name = "intel-gpio", - .irq_request_resources = intel_gpio_irq_reqres, - .irq_release_resources = intel_gpio_irq_relres, .irq_enable = intel_gpio_irq_enable, .irq_ack = intel_gpio_irq_ack, .irq_mask = intel_gpio_irq_mask,
This reverts commit 55aedef50d4d810670916d9fce4a40d5da2079e7. Commit 55aedef50d4d ("pinctrl: intel: Do pin translation when lock IRQ") added special translation from GPIO number to hardware pin number to irq_reqres/relres hooks to avoid failure when IRQs are requested. The actual failure happened inside gpiochip_lock_as_irq() because it calls gpiod_get_direction() and pinctrl-intel.c::intel_gpio_get_direction() implementation originally missed the translation so the two hooks made it work by skipping the ->get_direction() call entirely (it overwrote the default GPIOLIB provided functions). The proper fix that adds translation to GPIO callbacks was merged with commit 96147db1e1df ("pinctrl: intel: Do pin translation in other GPIO operations as well"). This allows us to use the default GPIOLIB provided functions again. In addition as find out by Benjamin Tissoires the two functions (intel_gpio_irq_reqres()/intel_gpio_irq_relres()) now cause problems of their own because they operate on pin numbers and pass that pin number to gpiochip_lock_as_irq() which actually expects a GPIO number. Link: https://bugzilla.kernel.org/show_bug.cgi?id=199911 Fixes: 55aedef50d4d ("pinctrl: intel: Do pin translation when lock IRQ") Reported-and-tested-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/pinctrl/intel/pinctrl-intel.c | 32 --------------------------- 1 file changed, 32 deletions(-)