| Message ID | 20191227230447.32458-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | pinctrl: baytrail: Do not clear IRQ flags on direct-irq enabled pins | expand |
On Sat, Dec 28, 2019 at 12:04:47AM +0100, Hans de Goede wrote: > Suspending Goodix touchscreens requires changing the interrupt pin to > output before sending them a power-down command. Followed by wiggling > the interrupt pin to wake the device up, after which it is put back > in input mode. > > On Bay Trail devices with a Goodix touchscreen direct-irq mode is used > in combination with listing the pin as a normal GpioIo resource. > > This works fine, until the goodix driver gets rmmod-ed and then insmod-ed > again. In this case byt_gpio_disable_free() calls > byt_gpio_clear_triggering() which clears the IRQ flags and after that the > (direct) IRQ no longer triggers. > > This commit fixes this by adding a check for the BYT_DIRECT_IRQ_EN flag > to byt_gpio_clear_triggering(). > > Note that byt_gpio_clear_triggering() only gets called from > byt_gpio_disable_free() for direct-irq enabled pins, as these are excluded > from the irq_valid mask by byt_init_irq_valid_mask(). > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
On Sat, Dec 28, 2019 at 12:04 AM Hans de Goede <hdegoede@redhat.com> wrote: > Suspending Goodix touchscreens requires changing the interrupt pin to > output before sending them a power-down command. Followed by wiggling > the interrupt pin to wake the device up, after which it is put back > in input mode. > > On Bay Trail devices with a Goodix touchscreen direct-irq mode is used > in combination with listing the pin as a normal GpioIo resource. > > This works fine, until the goodix driver gets rmmod-ed and then insmod-ed > again. In this case byt_gpio_disable_free() calls > byt_gpio_clear_triggering() which clears the IRQ flags and after that the > (direct) IRQ no longer triggers. > > This commit fixes this by adding a check for the BYT_DIRECT_IRQ_EN flag > to byt_gpio_clear_triggering(). > > Note that byt_gpio_clear_triggering() only gets called from > byt_gpio_disable_free() for direct-irq enabled pins, as these are excluded > from the irq_valid mask by byt_init_irq_valid_mask(). > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Patch applied as non-critical fix for v5.6 with Mika's ACK. (Tell me if it's critical.) Yours, Linus Walleij
On Tue, Jan 07, 2020 at 11:35:17AM +0100, Linus Walleij wrote: > On Sat, Dec 28, 2019 at 12:04 AM Hans de Goede <hdegoede@redhat.com> wrote: > > > Suspending Goodix touchscreens requires changing the interrupt pin to > > output before sending them a power-down command. Followed by wiggling > > the interrupt pin to wake the device up, after which it is put back > > in input mode. > > > > On Bay Trail devices with a Goodix touchscreen direct-irq mode is used > > in combination with listing the pin as a normal GpioIo resource. > > > > This works fine, until the goodix driver gets rmmod-ed and then insmod-ed > > again. In this case byt_gpio_disable_free() calls > > byt_gpio_clear_triggering() which clears the IRQ flags and after that the > > (direct) IRQ no longer triggers. > > > > This commit fixes this by adding a check for the BYT_DIRECT_IRQ_EN flag > > to byt_gpio_clear_triggering(). > > > > Note that byt_gpio_clear_triggering() only gets called from > > byt_gpio_disable_free() for direct-irq enabled pins, as these are excluded > > from the irq_valid mask by byt_init_irq_valid_mask(). > > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Patch applied as non-critical fix for v5.6 with Mika's ACK. > (Tell me if it's critical.) Can we collect it in our tree (what we are consider a proper for this) and submit a PR?
On Tue, Jan 7, 2020 at 1:38 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Tue, Jan 07, 2020 at 11:35:17AM +0100, Linus Walleij wrote: > > Patch applied as non-critical fix for v5.6 with Mika's ACK. > > (Tell me if it's critical.) > > Can we collect it in our tree (what we are consider a proper for this) and > submit a PR? Sure that's true, I dropped the patch from my tree. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Jan 7, 2020 at 1:55 PM Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Jan 7, 2020 at 1:38 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Jan 07, 2020 at 11:35:17AM +0100, Linus Walleij wrote: > > > > Patch applied as non-critical fix for v5.6 with Mika's ACK. > > > (Tell me if it's critical.) > > > > Can we collect it in our tree (what we are consider a proper for this) and > > submit a PR? > > Sure that's true, I dropped the patch from my tree. > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> I also dropped the other Baytrail patch I shouldn't have picked up. (Replace WARN...) I blame coming back from vacation for misremembering how things work... Yours, Linus Walleij
On Tue, Jan 07, 2020 at 01:57:02PM +0100, Linus Walleij wrote: > On Tue, Jan 7, 2020 at 1:55 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Tue, Jan 7, 2020 at 1:38 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Tue, Jan 07, 2020 at 11:35:17AM +0100, Linus Walleij wrote: > > > > > > Patch applied as non-critical fix for v5.6 with Mika's ACK. > > > > (Tell me if it's critical.) > > > > > > Can we collect it in our tree (what we are consider a proper for this) and > > > submit a PR? > > > > Sure that's true, I dropped the patch from my tree. > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > I also dropped the other Baytrail patch I shouldn't have picked up. > (Replace WARN...) > > I blame coming back from vacation for misremembering how things > work... It means you truly relaxed now! Thanks!
On Sat, Dec 28, 2019 at 12:04:47AM +0100, Hans de Goede wrote: > Suspending Goodix touchscreens requires changing the interrupt pin to > output before sending them a power-down command. Followed by wiggling > the interrupt pin to wake the device up, after which it is put back > in input mode. > > On Bay Trail devices with a Goodix touchscreen direct-irq mode is used > in combination with listing the pin as a normal GpioIo resource. > > This works fine, until the goodix driver gets rmmod-ed and then insmod-ed > again. In this case byt_gpio_disable_free() calls > byt_gpio_clear_triggering() which clears the IRQ flags and after that the > (direct) IRQ no longer triggers. > > This commit fixes this by adding a check for the BYT_DIRECT_IRQ_EN flag > to byt_gpio_clear_triggering(). > > Note that byt_gpio_clear_triggering() only gets called from > byt_gpio_disable_free() for direct-irq enabled pins, as these are excluded > from the irq_valid mask by byt_init_irq_valid_mask(). > Pushed to my review and testing queue, thanks! > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/pinctrl/intel/pinctrl-baytrail.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c > index db55761c90cc..844b89f230d7 100644 > --- a/drivers/pinctrl/intel/pinctrl-baytrail.c > +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c > @@ -742,8 +742,13 @@ static void byt_gpio_clear_triggering(struct intel_pinctrl *vg, unsigned int off > > raw_spin_lock_irqsave(&byt_lock, flags); > value = readl(reg); > + /* Do not clear direct-irq enabled irqs (from gpio_disable_free) */ > + if (value & BYT_DIRECT_IRQ_EN) > + goto out; > + > value &= ~(BYT_TRIG_POS | BYT_TRIG_NEG | BYT_TRIG_LVL); > writel(value, reg); > +out: > raw_spin_unlock_irqrestore(&byt_lock, flags); > } > > -- > 2.24.1 >
diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c index db55761c90cc..844b89f230d7 100644 --- a/drivers/pinctrl/intel/pinctrl-baytrail.c +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c @@ -742,8 +742,13 @@ static void byt_gpio_clear_triggering(struct intel_pinctrl *vg, unsigned int off raw_spin_lock_irqsave(&byt_lock, flags); value = readl(reg); + /* Do not clear direct-irq enabled irqs (from gpio_disable_free) */ + if (value & BYT_DIRECT_IRQ_EN) + goto out; + value &= ~(BYT_TRIG_POS | BYT_TRIG_NEG | BYT_TRIG_LVL); writel(value, reg); +out: raw_spin_unlock_irqrestore(&byt_lock, flags); }
Suspending Goodix touchscreens requires changing the interrupt pin to output before sending them a power-down command. Followed by wiggling the interrupt pin to wake the device up, after which it is put back in input mode. On Bay Trail devices with a Goodix touchscreen direct-irq mode is used in combination with listing the pin as a normal GpioIo resource. This works fine, until the goodix driver gets rmmod-ed and then insmod-ed again. In this case byt_gpio_disable_free() calls byt_gpio_clear_triggering() which clears the IRQ flags and after that the (direct) IRQ no longer triggers. This commit fixes this by adding a check for the BYT_DIRECT_IRQ_EN flag to byt_gpio_clear_triggering(). Note that byt_gpio_clear_triggering() only gets called from byt_gpio_disable_free() for direct-irq enabled pins, as these are excluded from the irq_valid mask by byt_init_irq_valid_mask(). Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/pinctrl/intel/pinctrl-baytrail.c | 5 +++++ 1 file changed, 5 insertions(+)