Linux-GPIO Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] gpio: aspeed: Pass irqchip when adding gpiochip
@ 2019-08-09 12:55 Linus Walleij
  2019-08-13  4:36 ` Andrew Jeffery
  2019-08-13  4:44 ` Joel Stanley
  0 siblings, 2 replies; 4+ messages in thread
From: Linus Walleij @ 2019-08-09 12:55 UTC (permalink / raw)
  To: linux-gpio
  Cc: Bartosz Golaszewski, Linus Walleij, Joel Stanley, Andrew Jeffery,
	Benjamin Herrenschmidt, Thierry Reding

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@jms.id.au>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Hi Aspeed folks, it'd be great if you could test/review this
patch.
---
 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


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

* Re: [PATCH] gpio: aspeed: Pass irqchip when adding gpiochip
  2019-08-09 12:55 [PATCH] gpio: aspeed: Pass irqchip when adding gpiochip Linus Walleij
@ 2019-08-13  4:36 ` Andrew Jeffery
  2019-08-13  4:44 ` Joel Stanley
  1 sibling, 0 replies; 4+ messages in thread
From: Andrew Jeffery @ 2019-08-13  4:36 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio
  Cc: Bartosz Golaszewski, Joel Stanley, Benjamin Herrenschmidt,
	Thierry Reding



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@jms.id.au>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> 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@aj.id.au>
Tested-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  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
> 
>

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

* Re: [PATCH] gpio: aspeed: Pass irqchip when adding gpiochip
  2019-08-09 12:55 [PATCH] gpio: aspeed: Pass irqchip when adding gpiochip Linus Walleij
  2019-08-13  4:36 ` Andrew Jeffery
@ 2019-08-13  4:44 ` Joel Stanley
  2019-08-15  8:01   ` Linus Walleij
  1 sibling, 1 reply; 4+ messages in thread
From: Joel Stanley @ 2019-08-13  4:44 UTC (permalink / raw)
  To: Linus Walleij
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Andrew Jeffery,
	Benjamin Herrenschmidt, Thierry Reding

On Fri, 9 Aug 2019 at 12:55, Linus Walleij <linus.walleij@linaro.org> 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@jms.id.au>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Hi Aspeed folks, it'd be great if you could test/review this
> patch.
> ---
>  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);

Is there a reason this is kcalloc and not kzalloc?

Other than that,

Acked-by: Joel Stanley <joel@jms.id.au>

Cheers,

Joel

> +               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
>

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

* Re: [PATCH] gpio: aspeed: Pass irqchip when adding gpiochip
  2019-08-13  4:44 ` Joel Stanley
@ 2019-08-15  8:01   ` Linus Walleij
  0 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2019-08-15  8:01 UTC (permalink / raw)
  To: Joel Stanley
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Andrew Jeffery,
	Benjamin Herrenschmidt, Thierry Reding

On Tue, Aug 13, 2019 at 6:44 AM Joel Stanley <joel@jms.id.au> wrote:
> On Fri, 9 Aug 2019 at 12:55, Linus Walleij <linus.walleij@linaro.org> wrote:

> > +               girq->parents = devm_kcalloc(&pdev->dev, 1,
> > +                                            sizeof(*girq->parents),
> > +                                            GFP_KERNEL);
>
> Is there a reason this is kcalloc and not kzalloc?

Following the pattern that we have one item of sizeof().
The parents can be several, so we try to follow this
pattern so people get it right.

> Acked-by: Joel Stanley <joel@jms.id.au>

Thanks!

Yours,
Linus Walleij

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-09 12:55 [PATCH] gpio: aspeed: Pass irqchip when adding gpiochip Linus Walleij
2019-08-13  4:36 ` Andrew Jeffery
2019-08-13  4:44 ` Joel Stanley
2019-08-15  8:01   ` Linus Walleij

Linux-GPIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-gpio/0 linux-gpio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-gpio linux-gpio/ https://lore.kernel.org/linux-gpio \
		linux-gpio@vger.kernel.org linux-gpio@archiver.kernel.org
	public-inbox-index linux-gpio


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-gpio


AGPL code for this site: git clone https://public-inbox.org/ public-inbox