pinctrl: at91: Pass irqchip when adding gpiochip

Message ID 20191001130645.8350-1-linus.walleij@linaro.org
State Accepted
Commit 35dea5d746b2ba5f56fe3562d6cc1843a632d471
Headers show
Series
  • pinctrl: at91: Pass irqchip when adding gpiochip
Related show

Commit Message

Linus Walleij Oct. 1, 2019, 1:06 p.m.
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: at91 is a little bit special since it registers
up to 3 gpio_chips with the same parent handler, but just
passing girq->parent_handler and the parent on the first
of them should cut it.

Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/pinctrl/pinctrl-at91.c | 47 ++++++++++++++++------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

-- 
2.21.0

Comments

Ludovic Desroches Oct. 9, 2019, 1:07 p.m. | #1
On Tue, Oct 01, 2019 at 03:06:45PM +0200, 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: at91 is a little bit special since it registers

> up to 3 gpio_chips with the same parent handler, but just

> passing girq->parent_handler and the parent on the first

> of them should cut it.

> 

> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>

> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>

> Cc: Ludovic Desroches <ludovic.desroches@microchip.com>

> Cc: Thierry Reding <thierry.reding@gmail.com>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com> 


By the way, tested with a sama5d3_xplained board.

Thanks

Ludovic

> ---

>  drivers/pinctrl/pinctrl-at91.c | 47 ++++++++++++++++------------------

>  1 file changed, 22 insertions(+), 25 deletions(-)

> 

> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c

> index d6e7e9f0ddec..117075b5798f 100644

> --- a/drivers/pinctrl/pinctrl-at91.c

> +++ b/drivers/pinctrl/pinctrl-at91.c

> @@ -1723,9 +1723,11 @@ static int at91_gpio_of_irq_setup(struct platform_device *pdev,

>  	struct at91_gpio_chip   *prev = NULL;

>  	struct irq_data		*d = irq_get_irq_data(at91_gpio->pioc_virq);

>  	struct irq_chip		*gpio_irqchip;

> -	int ret, i;

> +	struct gpio_irq_chip	*girq;

> +	int i;

>  

> -	gpio_irqchip = devm_kzalloc(&pdev->dev, sizeof(*gpio_irqchip), GFP_KERNEL);

> +	gpio_irqchip = devm_kzalloc(&pdev->dev, sizeof(*gpio_irqchip),

> +				    GFP_KERNEL);

>  	if (!gpio_irqchip)

>  		return -ENOMEM;

>  

> @@ -1747,33 +1749,30 @@ static int at91_gpio_of_irq_setup(struct platform_device *pdev,

>  	 * handler will perform the actual work of handling the parent

>  	 * interrupt.

>  	 */

> -	ret = gpiochip_irqchip_add(&at91_gpio->chip,

> -				   gpio_irqchip,

> -				   0,

> -				   handle_edge_irq,

> -				   IRQ_TYPE_NONE);

> -	if (ret) {

> -		dev_err(&pdev->dev, "at91_gpio.%d: Couldn't add irqchip to gpiochip.\n",

> -			at91_gpio->pioc_idx);

> -		return ret;

> -	}

> +	girq = &at91_gpio->chip.irq;

> +	girq->chip = gpio_irqchip;

> +	girq->default_type = IRQ_TYPE_NONE;

> +	girq->handler = handle_edge_irq;

>  

> -	/* The top level handler handles one bank of GPIOs, except

> +	/*

> +	 * The top level handler handles one bank of GPIOs, except

>  	 * on some SoC it can handle up to three...

>  	 * We only set up the handler for the first of the list.

>  	 */

>  	gpiochip_prev = irq_get_handler_data(at91_gpio->pioc_virq);

>  	if (!gpiochip_prev) {

> -		/* Then register the chain on the parent IRQ */

> -		gpiochip_set_chained_irqchip(&at91_gpio->chip,

> -					     gpio_irqchip,

> -					     at91_gpio->pioc_virq,

> -					     gpio_irq_handler);

> +		girq->parent_handler = 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] = at91_gpio->pioc_virq;

>  		return 0;

>  	}

>  

>  	prev = gpiochip_get_data(gpiochip_prev);

> -

>  	/* we can only have 2 banks before */

>  	for (i = 0; i < 2; i++) {

>  		if (prev->next) {

> @@ -1903,6 +1902,10 @@ static int at91_gpio_probe(struct platform_device *pdev)

>  	range->npins = chip->ngpio;

>  	range->gc = chip;

>  

> +	ret = at91_gpio_of_irq_setup(pdev, at91_chip);

> +	if (ret)

> +		goto gpiochip_add_err;

> +

>  	ret = gpiochip_add_data(chip, at91_chip);

>  	if (ret)

>  		goto gpiochip_add_err;

> @@ -1910,16 +1913,10 @@ static int at91_gpio_probe(struct platform_device *pdev)

>  	gpio_chips[alias_idx] = at91_chip;

>  	gpio_banks = max(gpio_banks, alias_idx + 1);

>  

> -	ret = at91_gpio_of_irq_setup(pdev, at91_chip);

> -	if (ret)

> -		goto irq_setup_err;

> -

>  	dev_info(&pdev->dev, "at address %p\n", at91_chip->regbase);

>  

>  	return 0;

>  

> -irq_setup_err:

> -	gpiochip_remove(chip);

>  gpiochip_add_err:

>  clk_enable_err:

>  	clk_disable_unprepare(at91_chip->clock);

> -- 

> 2.21.0

> 

>

Patch

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index d6e7e9f0ddec..117075b5798f 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1723,9 +1723,11 @@  static int at91_gpio_of_irq_setup(struct platform_device *pdev,
 	struct at91_gpio_chip   *prev = NULL;
 	struct irq_data		*d = irq_get_irq_data(at91_gpio->pioc_virq);
 	struct irq_chip		*gpio_irqchip;
-	int ret, i;
+	struct gpio_irq_chip	*girq;
+	int i;
 
-	gpio_irqchip = devm_kzalloc(&pdev->dev, sizeof(*gpio_irqchip), GFP_KERNEL);
+	gpio_irqchip = devm_kzalloc(&pdev->dev, sizeof(*gpio_irqchip),
+				    GFP_KERNEL);
 	if (!gpio_irqchip)
 		return -ENOMEM;
 
@@ -1747,33 +1749,30 @@  static int at91_gpio_of_irq_setup(struct platform_device *pdev,
 	 * handler will perform the actual work of handling the parent
 	 * interrupt.
 	 */
-	ret = gpiochip_irqchip_add(&at91_gpio->chip,
-				   gpio_irqchip,
-				   0,
-				   handle_edge_irq,
-				   IRQ_TYPE_NONE);
-	if (ret) {
-		dev_err(&pdev->dev, "at91_gpio.%d: Couldn't add irqchip to gpiochip.\n",
-			at91_gpio->pioc_idx);
-		return ret;
-	}
+	girq = &at91_gpio->chip.irq;
+	girq->chip = gpio_irqchip;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_edge_irq;
 
-	/* The top level handler handles one bank of GPIOs, except
+	/*
+	 * The top level handler handles one bank of GPIOs, except
 	 * on some SoC it can handle up to three...
 	 * We only set up the handler for the first of the list.
 	 */
 	gpiochip_prev = irq_get_handler_data(at91_gpio->pioc_virq);
 	if (!gpiochip_prev) {
-		/* Then register the chain on the parent IRQ */
-		gpiochip_set_chained_irqchip(&at91_gpio->chip,
-					     gpio_irqchip,
-					     at91_gpio->pioc_virq,
-					     gpio_irq_handler);
+		girq->parent_handler = 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] = at91_gpio->pioc_virq;
 		return 0;
 	}
 
 	prev = gpiochip_get_data(gpiochip_prev);
-
 	/* we can only have 2 banks before */
 	for (i = 0; i < 2; i++) {
 		if (prev->next) {
@@ -1903,6 +1902,10 @@  static int at91_gpio_probe(struct platform_device *pdev)
 	range->npins = chip->ngpio;
 	range->gc = chip;
 
+	ret = at91_gpio_of_irq_setup(pdev, at91_chip);
+	if (ret)
+		goto gpiochip_add_err;
+
 	ret = gpiochip_add_data(chip, at91_chip);
 	if (ret)
 		goto gpiochip_add_err;
@@ -1910,16 +1913,10 @@  static int at91_gpio_probe(struct platform_device *pdev)
 	gpio_chips[alias_idx] = at91_chip;
 	gpio_banks = max(gpio_banks, alias_idx + 1);
 
-	ret = at91_gpio_of_irq_setup(pdev, at91_chip);
-	if (ret)
-		goto irq_setup_err;
-
 	dev_info(&pdev->dev, "at address %p\n", at91_chip->regbase);
 
 	return 0;
 
-irq_setup_err:
-	gpiochip_remove(chip);
 gpiochip_add_err:
 clk_enable_err:
 	clk_disable_unprepare(at91_chip->clock);