Re: [PATCH] gpio: Initialize the irqchip valid_mask with a callback

["Andrew Jeffery" <andrew@xxxxxxxx>]

On Wed, 4 Sep 2019, at 23:31, Linus Walleij wrote:
> After changing the valid_mask for the struct gpio_chip
> to detect the need and presence of a valid mask with the
> presence of a .init_valid_mask() callback to fill it in,
> we augment the gpio_irq_chip to use the same logic.
> 
> Switch all driver using the gpio_irq_chio valid_mask
> over to this new method.
> 
> This makes sure the valid_mask for the gpio_irq_chip gets
> filled in when we add the gpio_chip, which makes it a
> little easier to switch over drivers using the old
> way of setting up gpio_irq_chip over to the new method
> of passing the gpio_irq_chip along with the gpio_chip.
> (See drivers/gpio/TODO for details.)
> 
> Cc: Patrice Chotard <patrice.chotard@xxxxxx>
> Cc: Andrew Lunn <andrew@xxxxxxx>
> Cc: Andrew Jeffery <andrew@xxxxxxxx>
> Cc: Joel Stanley <joel@xxxxxxxxx>
> Cc: Thierry Reding <treding@xxxxxxxxxx>
> Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> All is compile tested on top of the gpio "devel" branch.
> 
> Andy: I guess this would collide with my attempted
> rewrites of some Intel-related drivers, we can either
> merge this first and I will rebase and resend the
> other changes, or you can send me a pull request
> if you think some of my changes are working and I
> will instead rebase this on top of that, thanks.
> 
> Hans de Goede: I actually think that patch
> 48057ed1840fde9239b1e000bea1a0a1f07c5e99
> "gpio: Fix irqchip initialization order" fixes the
> issues you saw with the rewrite of int0002 earlier,
> and we suggested setting up the mask as part of the
> chip addition then, but this change is nice to have
> anyways.
> ---
>  drivers/gpio/gpio-aspeed.c                 | 13 ++++---
>  drivers/gpio/gpio-stmpe.c                  | 36 +++++++++++++------
>  drivers/gpio/gpio-tqmx86.c                 | 21 ++++++-----
>  drivers/gpio/gpiolib.c                     | 12 ++++---
>  drivers/pinctrl/intel/pinctrl-baytrail.c   | 16 ++++++++-
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 42 +++++++++++++---------
>  drivers/platform/x86/intel_int0002_vgpio.c | 11 ++++--
>  include/linux/gpio/driver.h                | 16 ++++++---
>  8 files changed, 112 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> index 9defe25d4721..7bcd83dbc3e3 100644
> --- a/drivers/gpio/gpio-aspeed.c
> +++ b/drivers/gpio/gpio-aspeed.c
> @@ -689,8 +689,11 @@ static struct irq_chip aspeed_gpio_irqchip = {
>  	.irq_set_type	= aspeed_gpio_set_type,
>  };
>  
> -static void set_irq_valid_mask(struct aspeed_gpio *gpio)
> +static void aspeed_init_irq_valid_mask(struct gpio_chip *gc,
> +				       unsigned long *valid_mask,
> +				       unsigned int ngpios)
>  {
> +	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
>  	const struct aspeed_bank_props *props = gpio->config->props;
>  
>  	while (!is_bank_props_sentinel(props)) {
> @@ -704,7 +707,7 @@ static void set_irq_valid_mask(struct aspeed_gpio *gpio)
>  			if (i >= gpio->config->nr_gpios)
>  				break;
>  
> -			clear_bit(i, gpio->chip.irq.valid_mask);
> +			clear_bit(i, valid_mask);
>  		}
>  
>  		props++;
> @@ -1203,7 +1206,7 @@ static int __init aspeed_gpio_probe(struct 
> platform_device *pdev)
>  		girq->parents[0] = gpio->irq;
>  		girq->default_type = IRQ_TYPE_NONE;
>  		girq->handler = handle_bad_irq;
> -		girq->need_valid_mask = true;
> +		girq->init_valid_mask = aspeed_init_irq_valid_mask;
>  	}
>  
>  	gpio->offset_timer =
> @@ -1215,10 +1218,6 @@ static int __init aspeed_gpio_probe(struct 
> platform_device *pdev)
>  	if (rc < 0)
>  		return rc;
>  
> -	/* Now the valid mask is allocated */
> -	if (gpio->irq)
> -		set_irq_valid_mask(gpio);
> -
>  	return 0;
>  }

For the Aspeed changes:

Reviewed-by: Andrew Jeffery <andrew@xxxxxxxx>