Re: [PATCH] gpio: aspeed: Pass irqchip when adding gpiochip

["Andrew Jeffery" <andrew@xxxxxxxx>]

On Fri, 9 Aug 2019, at 22:25, Linus Walleij wrote:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
> 
> For chained irqchips this is a pretty straight-forward
> conversion.
> 
> Cc: Joel Stanley <joel@xxxxxxxxx>
> Cc: Andrew Jeffery <andrew@xxxxxxxx>
> Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> Cc: Thierry Reding <treding@xxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> Hi Aspeed folks, it'd be great if you could test/review this
> patch.

Used it to exercise our WIP GPIO model for qemu. After fixing the qemu bugs it
worked fine there. Booted it on a Witherspoon and things worked as expected
there too. From a brief inspection it looks fine to me.

Reviewed-by: Andrew Jeffery <andrew@xxxxxxxx>
Tested-by: Andrew Jeffery <andrew@xxxxxxxx>

> ---
>  drivers/gpio/gpio-aspeed.c | 60 ++++++++++++++++++--------------------
>  1 file changed, 29 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> index 13d80bfbc3b6..9defe25d4721 100644
> --- a/drivers/gpio/gpio-aspeed.c
> +++ b/drivers/gpio/gpio-aspeed.c
> @@ -711,32 +711,6 @@ static void set_irq_valid_mask(struct aspeed_gpio *gpio)
>  	}
>  }
>  
> -static int aspeed_gpio_setup_irqs(struct aspeed_gpio *gpio,
> -		struct platform_device *pdev)
> -{
> -	int rc;
> -
> -	rc = platform_get_irq(pdev, 0);
> -	if (rc < 0)
> -		return rc;
> -
> -	gpio->irq = rc;
> -
> -	set_irq_valid_mask(gpio);
> -
> -	rc = gpiochip_irqchip_add(&gpio->chip, &aspeed_gpio_irqchip,
> -			0, handle_bad_irq, IRQ_TYPE_NONE);
> -	if (rc) {
> -		dev_info(&pdev->dev, "Could not add irqchip\n");
> -		return rc;
> -	}
> -
> -	gpiochip_set_chained_irqchip(&gpio->chip, &aspeed_gpio_irqchip,
> -				     gpio->irq, aspeed_gpio_irq_handler);
> -
> -	return 0;
> -}
> -
>  static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,
>  					unsigned int offset, bool enable)
>  {
> @@ -1189,7 +1163,6 @@ static int __init aspeed_gpio_probe(struct 
> platform_device *pdev)
>  	gpio->chip.set_config = aspeed_gpio_set_config;
>  	gpio->chip.label = dev_name(&pdev->dev);
>  	gpio->chip.base = -1;
> -	gpio->chip.irq.need_valid_mask = true;
>  
>  	/* Allocate a cache of the output registers */
>  	banks = gpio->config->nr_gpios >> 5;
> @@ -1212,16 +1185,41 @@ static int __init aspeed_gpio_probe(struct 
> platform_device *pdev)
>  		aspeed_gpio_change_cmd_source(gpio, bank, 3, GPIO_CMDSRC_ARM);
>  	}
>  
> -	rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> -	if (rc < 0)
> -		return rc;
> +	/* Optionally set up an irqchip if there is an IRQ */
> +	rc = platform_get_irq(pdev, 0);
> +	if (rc > 0) {
> +		struct gpio_irq_chip *girq;
> +
> +		gpio->irq = rc;
> +		girq = &gpio->chip.irq;
> +		girq->chip = &aspeed_gpio_irqchip;
> +		girq->parent_handler = aspeed_gpio_irq_handler;
> +		girq->num_parents = 1;
> +		girq->parents = devm_kcalloc(&pdev->dev, 1,
> +					     sizeof(*girq->parents),
> +					     GFP_KERNEL);
> +		if (!girq->parents)
> +			return -ENOMEM;
> +		girq->parents[0] = gpio->irq;
> +		girq->default_type = IRQ_TYPE_NONE;
> +		girq->handler = handle_bad_irq;
> +		girq->need_valid_mask = true;
> +	}
>  
>  	gpio->offset_timer =
>  		devm_kzalloc(&pdev->dev, gpio->chip.ngpio, GFP_KERNEL);
>  	if (!gpio->offset_timer)
>  		return -ENOMEM;
>  
> -	return aspeed_gpio_setup_irqs(gpio, pdev);
> +	rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* Now the valid mask is allocated */
> +	if (gpio->irq)
> +		set_irq_valid_mask(gpio);
> +
> +	return 0;
>  }
>  
>  static struct platform_driver aspeed_gpio_driver = {
> -- 
> 2.21.0
> 
>