Re: [PATCH v2 1/4] gpio: dwapb: Call acpi_gpiochip_free_interrupts() on GPIO chip de-registration

[Serge Semin <fancer.lancer@xxxxxxxxx>]

On Mon, May 18, 2020 at 08:41:35PM +0300, Andy Shevchenko wrote:
> Add missed acpi_gpiochip_free_interrupts() call when unregistering ports.
> 
> While at it, drop extra check to call acpi_gpiochip_request_interrupts().
> There is no need to have an additional check to call
> acpi_gpiochip_request_interrupts(). Even without any interrupts available
> the registered ACPI Event handlers can be useful for debugging purposes.
> 
> Fixes: e6cb3486f5a1 ("gpio: dwapb: add gpio-signaled acpi event support")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: Serge Semin <fancer.lancer@xxxxxxxxx>

Andy, thanks for the series. After fixing patch 3, supposedly you were going to
to do it like this:
-               if (irq > 0)
-                       pp->irq[j] = irq;
-
-               if (pp->irq[j] >= 0)
-                       pp->has_irq = true;
+               if (irq > 0) {
+                       pp->irq[j] = irq;
+                       pp->has_irq = true;
+               }
or similar as you prefer, feel free to add my tag to the whole series:
Acked-by: Serge Semin <fancer.lancer@xxxxxxxxx>
Tested-by: Serge Semin <fancer.lancer@xxxxxxxxx>

-Sergey

> ---
> v2: made it 1st in the series, add Fixes (Serge)
>  drivers/gpio/gpio-dwapb.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 8639c4a7f469..e5d844304f8d 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -505,26 +505,33 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
>  		dwapb_configure_irqs(gpio, port, pp);
>  
>  	err = gpiochip_add_data(&port->gc, port);
> -	if (err)
> +	if (err) {
>  		dev_err(gpio->dev, "failed to register gpiochip for port%d\n",
>  			port->idx);
> -	else
> -		port->is_registered = true;
> +		return err;
> +	}
>  
>  	/* Add GPIO-signaled ACPI event support */
> -	if (pp->has_irq)
> -		acpi_gpiochip_request_interrupts(&port->gc);
> +	acpi_gpiochip_request_interrupts(&port->gc);
>  
> -	return err;
> +	port->is_registered = true;
> +
> +	return 0;
>  }
>  
>  static void dwapb_gpio_unregister(struct dwapb_gpio *gpio)
>  {
>  	unsigned int m;
>  
> -	for (m = 0; m < gpio->nr_ports; ++m)
> -		if (gpio->ports[m].is_registered)
> -			gpiochip_remove(&gpio->ports[m].gc);
> +	for (m = 0; m < gpio->nr_ports; ++m) {
> +		struct dwapb_gpio_port *port = &gpio->ports[m];
> +
> +		if (!port->is_registered)
> +			continue;
> +
> +		acpi_gpiochip_free_interrupts(&port->gc);
> +		gpiochip_remove(&port->gc);
> +	}
>  }
>  
>  static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode,
> -- 
> 2.26.2
>