All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpio: Fix irqchip initialization order
@ 2019-08-20  8:05 Linus Walleij
  2019-08-23  6:48 ` Linus Walleij
  2019-08-23  8:31 ` Wei Xu
  0 siblings, 2 replies; 5+ messages in thread
From: Linus Walleij @ 2019-08-20  8:05 UTC (permalink / raw)
  To: linux-gpio
  Cc: Bartosz Golaszewski, Linus Walleij, Thierry Reding,
	Grygorii Strashko, Wei Xu, Andy Shevchenko

The new API for registering a gpio_irq_chip along with a
gpio_chip has a different semantic ordering than the old
API which added the irqchip explicitly after registering
the gpio_chip.

Move the calls to add the gpio_irq_chip *last* in the
function, so that the different hooks setting up OF and
ACPI and machine gpio_chips are called *before* we try
to register the interrupts, preserving the elder semantic
order.

This cropped up in the PL061 driver which used to work
fine with no special ACPI quirks, but started to misbehave
using the new API.

Fixes: e0d897289813 ("gpio: Implement tighter IRQ chip integration")
Cc: Thierry Reding <treding@nvidia.com>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Reported-by: Wei Xu <xuwei5@hisilicon.com>
Reported-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Wei: it would be great if you could test this and
confirm if it solves your problem, so I can apply this
for fixes.
---
 drivers/gpio/gpiolib.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 80a2a2cb673b..cca749010cd0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1373,21 +1373,13 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
 	if (status)
 		goto err_remove_from_list;
 
-	status = gpiochip_irqchip_init_valid_mask(chip);
-	if (status)
-		goto err_remove_from_list;
-
 	status = gpiochip_alloc_valid_mask(chip);
 	if (status)
-		goto err_remove_irqchip_mask;
-
-	status = gpiochip_add_irqchip(chip, lock_key, request_key);
-	if (status)
-		goto err_free_gpiochip_mask;
+		goto err_remove_from_list;
 
 	status = of_gpiochip_add(chip);
 	if (status)
-		goto err_remove_chip;
+		goto err_free_gpiochip_mask;
 
 	status = gpiochip_init_valid_mask(chip);
 	if (status)
@@ -1413,6 +1405,14 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
 
 	machine_gpiochip_add(chip);
 
+	status = gpiochip_irqchip_init_valid_mask(chip);
+	if (status)
+		goto err_remove_acpi_chip;
+
+	status = gpiochip_add_irqchip(chip, lock_key, request_key);
+	if (status)
+		goto err_remove_irqchip_mask;
+
 	/*
 	 * By first adding the chardev, and then adding the device,
 	 * we get a device node entry in sysfs under
@@ -1424,21 +1424,21 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
 	if (gpiolib_initialized) {
 		status = gpiochip_setup_dev(gdev);
 		if (status)
-			goto err_remove_acpi_chip;
+			goto err_remove_irqchip;
 	}
 	return 0;
 
+err_remove_irqchip:
+	gpiochip_irqchip_remove(chip);
+err_remove_irqchip_mask:
+	gpiochip_irqchip_free_valid_mask(chip);
 err_remove_acpi_chip:
 	acpi_gpiochip_remove(chip);
 err_remove_of_chip:
 	gpiochip_free_hogs(chip);
 	of_gpiochip_remove(chip);
-err_remove_chip:
-	gpiochip_irqchip_remove(chip);
 err_free_gpiochip_mask:
 	gpiochip_free_valid_mask(chip);
-err_remove_irqchip_mask:
-	gpiochip_irqchip_free_valid_mask(chip);
 err_remove_from_list:
 	spin_lock_irqsave(&gpio_lock, flags);
 	list_del(&gdev->list);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] gpio: Fix irqchip initialization order
  2019-08-20  8:05 [PATCH] gpio: Fix irqchip initialization order Linus Walleij
@ 2019-08-23  6:48 ` Linus Walleij
  2019-08-23  8:31 ` Wei Xu
  1 sibling, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2019-08-23  6:48 UTC (permalink / raw)
  To: open list:GPIO SUBSYSTEM, Wei Xu
  Cc: Bartosz Golaszewski, Thierry Reding, Grygorii Strashko, Andy Shevchenko

On Tue, Aug 20, 2019 at 10:05 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> Wei: it would be great if you could test this and
> confirm if it solves your problem, so I can apply this
> for fixes.

Wei, could you test this patch? I have a strong urge
to push it to Torvalds as a fix for this problem but it'd
be super if you could test it.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] gpio: Fix irqchip initialization order
  2019-08-20  8:05 [PATCH] gpio: Fix irqchip initialization order Linus Walleij
  2019-08-23  6:48 ` Linus Walleij
@ 2019-08-23  8:31 ` Wei Xu
  2019-08-23  9:01   ` Linus Walleij
  1 sibling, 1 reply; 5+ messages in thread
From: Wei Xu @ 2019-08-23  8:31 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio
  Cc: Bartosz Golaszewski, Thierry Reding, Grygorii Strashko, Andy Shevchenko

Hi Linus,

On 2019/8/20 16:05, Linus Walleij wrote:
> The new API for registering a gpio_irq_chip along with a
> gpio_chip has a different semantic ordering than the old
> API which added the irqchip explicitly after registering
> the gpio_chip.
>
> Move the calls to add the gpio_irq_chip *last* in the
> function, so that the different hooks setting up OF and
> ACPI and machine gpio_chips are called *before* we try
> to register the interrupts, preserving the elder semantic
> order.
>
> This cropped up in the PL061 driver which used to work
> fine with no special ACPI quirks, but started to misbehave
> using the new API.
>
> Fixes: e0d897289813 ("gpio: Implement tighter IRQ chip integration")
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reported-by: Wei Xu <xuwei5@hisilicon.com>
> Reported-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Wei: it would be great if you could test this and
> confirm if it solves your problem, so I can apply this
> for fixes.

Sorry for the late reply!
There are some issues about my mail filter.

Yes, this fixes the problem and tested based on gpio/devel branch.
So,

Tested-by: Wei Xu <xuwei5@hisilicon.com>

Thanks!

The log is as below:

     estuary:/$ cat /proc/interrupts
                CPU0
       2:       2610     GICv3  27 Level     arch_timer
       4:         33     GICv3  33 Level     uart-pl011
      42:          0     GICv3  23 Level     arm-pmu
      43:          0  ARMH0061:00   3 Edge      ACPI:Event
     IPI0:         0       Rescheduling interrupts
     IPI1:         0       Function call interrupts
     IPI2:         0       CPU stop interrupts
     IPI3:         0       CPU stop (for crash dump) interrupts
     IPI4:         0       Timer broadcast interrupts
     IPI5:         0       IRQ work interrupts
     IPI6:         0       CPU wake-up interrupts
     Err:          0

     estuary:/$ dmesg | more
     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x411fd070]
     [    0.000000] Linux version 5.3.0-rc1-00036-gbe23e9a 
(joyx@Turing-Arch-b) (gcc version 4.9.1 20140505 (prerelease) 
(crosstool-NG linaro-1.13.1-4.9-2014.05 - Linaro GCC 4.9-2014.05)) #64 
SMP Fri Aug 23 16:05:08 CST 2019

     commit be23e9a097e55d6733eec4336c078fda93339265
     Author: Linus Walleij <linus.walleij@linaro.org>
     Date:   Tue Aug 20 10:05:27 2019 +0200

         gpio: Fix irqchip initialization order

Best Regards,
Wei

> ---
>   drivers/gpio/gpiolib.c | 30 +++++++++++++++---------------
>   1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 80a2a2cb673b..cca749010cd0 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1373,21 +1373,13 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
>   	if (status)
>   		goto err_remove_from_list;
>   
> -	status = gpiochip_irqchip_init_valid_mask(chip);
> -	if (status)
> -		goto err_remove_from_list;
> -
>   	status = gpiochip_alloc_valid_mask(chip);
>   	if (status)
> -		goto err_remove_irqchip_mask;
> -
> -	status = gpiochip_add_irqchip(chip, lock_key, request_key);
> -	if (status)
> -		goto err_free_gpiochip_mask;
> +		goto err_remove_from_list;
>   
>   	status = of_gpiochip_add(chip);
>   	if (status)
> -		goto err_remove_chip;
> +		goto err_free_gpiochip_mask;
>   
>   	status = gpiochip_init_valid_mask(chip);
>   	if (status)
> @@ -1413,6 +1405,14 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
>   
>   	machine_gpiochip_add(chip);
>   
> +	status = gpiochip_irqchip_init_valid_mask(chip);
> +	if (status)
> +		goto err_remove_acpi_chip;
> +
> +	status = gpiochip_add_irqchip(chip, lock_key, request_key);
> +	if (status)
> +		goto err_remove_irqchip_mask;
> +
>   	/*
>   	 * By first adding the chardev, and then adding the device,
>   	 * we get a device node entry in sysfs under
> @@ -1424,21 +1424,21 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
>   	if (gpiolib_initialized) {
>   		status = gpiochip_setup_dev(gdev);
>   		if (status)
> -			goto err_remove_acpi_chip;
> +			goto err_remove_irqchip;
>   	}
>   	return 0;
>   
> +err_remove_irqchip:
> +	gpiochip_irqchip_remove(chip);
> +err_remove_irqchip_mask:
> +	gpiochip_irqchip_free_valid_mask(chip);
>   err_remove_acpi_chip:
>   	acpi_gpiochip_remove(chip);
>   err_remove_of_chip:
>   	gpiochip_free_hogs(chip);
>   	of_gpiochip_remove(chip);
> -err_remove_chip:
> -	gpiochip_irqchip_remove(chip);
>   err_free_gpiochip_mask:
>   	gpiochip_free_valid_mask(chip);
> -err_remove_irqchip_mask:
> -	gpiochip_irqchip_free_valid_mask(chip);
>   err_remove_from_list:
>   	spin_lock_irqsave(&gpio_lock, flags);
>   	list_del(&gdev->list);



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] gpio: Fix irqchip initialization order
  2019-08-23  8:31 ` Wei Xu
@ 2019-08-23  9:01   ` Linus Walleij
  2019-08-23 10:25     ` Andy Shevchenko
  0 siblings, 1 reply; 5+ messages in thread
From: Linus Walleij @ 2019-08-23  9:01 UTC (permalink / raw)
  To: Wei Xu
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Thierry Reding,
	Grygorii Strashko, Andy Shevchenko

On Fri, Aug 23, 2019 at 10:31 AM Wei Xu <xuwei5@hisilicon.com> wrote:

> Yes, this fixes the problem and tested based on gpio/devel branch.
> So,
>
> Tested-by: Wei Xu <xuwei5@hisilicon.com>

Awesome! Thanks a lot Wei!
I'll get this to Torvalds tree ASAP.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] gpio: Fix irqchip initialization order
  2019-08-23  9:01   ` Linus Walleij
@ 2019-08-23 10:25     ` Andy Shevchenko
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2019-08-23 10:25 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Wei Xu, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	Thierry Reding, Grygorii Strashko

On Fri, Aug 23, 2019 at 12:02 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Aug 23, 2019 at 10:31 AM Wei Xu <xuwei5@hisilicon.com> wrote:
>
> > Yes, this fixes the problem and tested based on gpio/devel branch.
> > So,
> >
> > Tested-by: Wei Xu <xuwei5@hisilicon.com>
>
> Awesome! Thanks a lot Wei!
> I'll get this to Torvalds tree ASAP.

Thanks, Linus, to fix this!

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-08-23 10:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-20  8:05 [PATCH] gpio: Fix irqchip initialization order Linus Walleij
2019-08-23  6:48 ` Linus Walleij
2019-08-23  8:31 ` Wei Xu
2019-08-23  9:01   ` Linus Walleij
2019-08-23 10:25     ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.