| Message ID | 20191030122914.967-1-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gpiolib: Switch order of valid mask and hw init | expand |
Hi, On 30-10-2019 13:29, Linus Walleij wrote: > The GPIO irqchip needs to initialize the valid mask > before initializing the IRQ hardware, because sometimes > the latter require the former to be executed first. > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Reported-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Ack, I was thinking along these lines myself too, but I was not sure if this would be an acceptable solution: Acked-by: Hans de Goede <hdegoede@redhat.com> > --- > Thinking of applying this for fixes to fix some part > of the problems that Hans is seeing. So you want to get this into 5.4, so that when "pinctrl: intel: baytrail: Pass irqchip when adding gpiochip" lands in 5.5 this is already in place. Ok, I've just checked all the existing users if the init_hw callback and none of them use init_valid_mask so for them to order should not matter. So yes getting this into 5.4 would be good. This fixes 2 of the 3 issues I mentioned in my other mail, the NULL pointer deref and the false_positive error messages from byt_gpio_irq_init_hw(). But as I guess you are aware, that still leaves us with the third problem: "acpi_gpiochip_request_interrupts() gets called before gpiochip_add_pin_range() is called from pinctrl-baytrail.c, causing the GPIO lookup of any ACPI _AEI handlers to fail, resulting in errors like this one: byt_gpio INT33FC:02: Failed to request GPIO for pin 0x13: -517 And none of the _AEI handlers working" Regards, Hans > --- > drivers/gpio/gpiolib.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 9afbc0612126..e865c889ba8d 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -1411,11 +1411,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, > > machine_gpiochip_add(chip); > > - ret = gpiochip_irqchip_init_hw(chip); > + ret = gpiochip_irqchip_init_valid_mask(chip); > if (ret) > goto err_remove_acpi_chip; > > - ret = gpiochip_irqchip_init_valid_mask(chip); > + ret = gpiochip_irqchip_init_hw(chip); > if (ret) > goto err_remove_acpi_chip; > >
On Wed, Oct 30, 2019 at 01:29:14PM +0100, Linus Walleij wrote: > The GPIO irqchip needs to initialize the valid mask > before initializing the IRQ hardware, because sometimes > the latter require the former to be executed first. > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
On Wed, Oct 30, 2019 at 01:29:14PM +0100, Linus Walleij wrote: > The GPIO irqchip needs to initialize the valid mask > before initializing the IRQ hardware, because sometimes > the latter require the former to be executed first. > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Reported-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > Thinking of applying this for fixes to fix some part > of the problems that Hans is seeing. > --- > drivers/gpio/gpiolib.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 9afbc0612126..e865c889ba8d 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -1411,11 +1411,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, > > machine_gpiochip_add(chip); > > - ret = gpiochip_irqchip_init_hw(chip); > + ret = gpiochip_irqchip_init_valid_mask(chip); > if (ret) > goto err_remove_acpi_chip; > > - ret = gpiochip_irqchip_init_valid_mask(chip); > + ret = gpiochip_irqchip_init_hw(chip); > if (ret) > goto err_remove_acpi_chip; > > -- > 2.21.0 >
On Wed, Oct 30, 2019 at 01:43:38PM +0100, Hans de Goede wrote: > On 30-10-2019 13:29, Linus Walleij wrote: > But as I guess you are aware, that still leaves us with the third > problem: "acpi_gpiochip_request_interrupts() gets called before > gpiochip_add_pin_range() is called from pinctrl-baytrail.c, causing > the GPIO lookup of any ACPI _AEI handlers to fail, resulting in > errors like this one: > > byt_gpio INT33FC:02: Failed to request GPIO for pin 0x13: -517 > > And none of the _AEI handlers working" I think we may postpone the Baytrail patches, if we don't find a solution for the above soon. Nevertheless, this change on its own good to have. Hans, thanks for testing!
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 9afbc0612126..e865c889ba8d 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1411,11 +1411,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, machine_gpiochip_add(chip); - ret = gpiochip_irqchip_init_hw(chip); + ret = gpiochip_irqchip_init_valid_mask(chip); if (ret) goto err_remove_acpi_chip; - ret = gpiochip_irqchip_init_valid_mask(chip); + ret = gpiochip_irqchip_init_hw(chip); if (ret) goto err_remove_acpi_chip;
The GPIO irqchip needs to initialize the valid mask before initializing the IRQ hardware, because sometimes the latter require the former to be executed first. Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Reported-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Thinking of applying this for fixes to fix some part of the problems that Hans is seeing. --- drivers/gpio/gpiolib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)