[PATCH 0/3] gpio: drop a few unneeded irq_{request,release}_resources implementations

[PATCH 0/3] gpio: drop a few unneeded irq_{request,release}_resources implementations

[Thomas Petazzoni]

Hello,

Three GPIO drivers in the tree implement the
irq_{request,release}_resources hooks with what is in fact the default
implementation provided by the GPIO core, making this driver-specific
code redundant.

It is worth mentioning that I do not own any of the affected
platforms, to the three patches have only been built-tested. Each is
Cc'ed to the appropriate maintainer, except gpio-em that has no
platform-specific maintainer.

Best regards,

Thomas Petazzoni

Thomas Petazzoni (3):
  gpio: bcm-kona: drop ->irq_{request,release}_resources hooks
  gpio: dwapb: drop ->irq_{request,release}_resources hooks
  gpio: em: drop ->irq_{request,release}_resources hooks

 drivers/gpio/gpio-bcm-kona.c | 16 ----------------
 drivers/gpio/gpio-dwapb.c    | 27 ---------------------------
 drivers/gpio/gpio-em.c       | 25 -------------------------
 3 files changed, 68 deletions(-)

-- 
2.20.1

[PATCH 1/3] gpio: bcm-kona: drop ->irq_{request,release}_resources hooks

[Thomas Petazzoni]

Those hooks implement the exact same behavior as the default hooks by
gpiolib, so there is no point in having a duplicated definition in
gpio-bcm-kona.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: bcm-kernel-feedback-list@broadcom.com
---
Note: this commit was only build tested.
---
 drivers/gpio/gpio-bcm-kona.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
index c5536a509b59..0ae806edabc2 100644
--- a/drivers/gpio/gpio-bcm-kona.c
+++ b/drivers/gpio/gpio-bcm-kona.c
@@ -484,28 +484,12 @@ static void bcm_kona_gpio_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
-static int bcm_kona_gpio_irq_reqres(struct irq_data *d)
-{
-	struct bcm_kona_gpio *kona_gpio = irq_data_get_irq_chip_data(d);
-
-	return gpiochip_reqres_irq(&kona_gpio->gpio_chip, d->hwirq);
-}
-
-static void bcm_kona_gpio_irq_relres(struct irq_data *d)
-{
-	struct bcm_kona_gpio *kona_gpio = irq_data_get_irq_chip_data(d);
-
-	gpiochip_relres_irq(&kona_gpio->gpio_chip, d->hwirq);
-}
-
 static struct irq_chip bcm_gpio_irq_chip = {
 	.name = "bcm-kona-gpio",
 	.irq_ack = bcm_kona_gpio_irq_ack,
 	.irq_mask = bcm_kona_gpio_irq_mask,
 	.irq_unmask = bcm_kona_gpio_irq_unmask,
 	.irq_set_type = bcm_kona_gpio_irq_set_type,
-	.irq_request_resources = bcm_kona_gpio_irq_reqres,
-	.irq_release_resources = bcm_kona_gpio_irq_relres,
 };
 
 static struct of_device_id const bcm_kona_gpio_of_match[] = {
-- 
2.20.1

[PATCH 2/3] gpio: dwapb: drop ->irq_{request,release}_resources hooks

[Thomas Petazzoni]

Those hooks implement the exact same behavior as the default hooks by
gpiolib, so there is no point in having a duplicated definition in
gpio-dwapb.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Hoan Tran <hotran@apm.com>
---
Note: this commit was only build tested.
---
 drivers/gpio/gpio-dwapb.c | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 044888fd96a1..93eba2c6f328 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -250,31 +250,6 @@ static void dwapb_irq_disable(struct irq_data *d)
 	spin_unlock_irqrestore(&gc->bgpio_lock, flags);
 }
 
-static int dwapb_irq_reqres(struct irq_data *d)
-{
-	struct irq_chip_generic *igc = irq_data_get_irq_chip_data(d);
-	struct dwapb_gpio *gpio = igc->private;
-	struct gpio_chip *gc = &gpio->ports[0].gc;
-	int ret;
-
-	ret = gpiochip_lock_as_irq(gc, irqd_to_hwirq(d));
-	if (ret) {
-		dev_err(gpio->dev, "unable to lock HW IRQ %lu for IRQ\n",
-			irqd_to_hwirq(d));
-		return ret;
-	}
-	return 0;
-}
-
-static void dwapb_irq_relres(struct irq_data *d)
-{
-	struct irq_chip_generic *igc = irq_data_get_irq_chip_data(d);
-	struct dwapb_gpio *gpio = igc->private;
-	struct gpio_chip *gc = &gpio->ports[0].gc;
-
-	gpiochip_unlock_as_irq(gc, irqd_to_hwirq(d));
-}
-
 static int dwapb_irq_set_type(struct irq_data *d, u32 type)
 {
 	struct irq_chip_generic *igc = irq_data_get_irq_chip_data(d);
@@ -428,8 +403,6 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
 		ct->chip.irq_set_type = dwapb_irq_set_type;
 		ct->chip.irq_enable = dwapb_irq_enable;
 		ct->chip.irq_disable = dwapb_irq_disable;
-		ct->chip.irq_request_resources = dwapb_irq_reqres;
-		ct->chip.irq_release_resources = dwapb_irq_relres;
 #ifdef CONFIG_PM_SLEEP
 		ct->chip.irq_set_wake = dwapb_irq_set_wake;
 #endif
-- 
2.20.1

[PATCH 3/3] gpio: em: drop ->irq_{request,release}_resources hooks

[Thomas Petazzoni]

Those hooks implement the exact same behavior as the default hooks by
gpiolib, so there is no point in having a duplicated definition in
gpio-dwapb.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Note: this commit was only build tested.
---
 drivers/gpio/gpio-em.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/drivers/gpio/gpio-em.c b/drivers/gpio/gpio-em.c
index 982e699a5b81..e27c56854ee7 100644
--- a/drivers/gpio/gpio-em.c
+++ b/drivers/gpio/gpio-em.c
@@ -98,29 +98,6 @@ static void em_gio_irq_enable(struct irq_data *d)
 	em_gio_write(p, GIO_IEN, BIT(irqd_to_hwirq(d)));
 }
 
-static int em_gio_irq_reqres(struct irq_data *d)
-{
-	struct em_gio_priv *p = irq_data_get_irq_chip_data(d);
-	int ret;
-
-	ret = gpiochip_lock_as_irq(&p->gpio_chip, irqd_to_hwirq(d));
-	if (ret) {
-		dev_err(p->gpio_chip.parent,
-			"unable to lock HW IRQ %lu for IRQ\n",
-			irqd_to_hwirq(d));
-		return ret;
-	}
-	return 0;
-}
-
-static void em_gio_irq_relres(struct irq_data *d)
-{
-	struct em_gio_priv *p = irq_data_get_irq_chip_data(d);
-
-	gpiochip_unlock_as_irq(&p->gpio_chip, irqd_to_hwirq(d));
-}
-
-
 #define GIO_ASYNC(x) (x + 8)
 
 static unsigned char em_gio_sense_table[IRQ_TYPE_SENSE_MASK + 1] = {
@@ -344,8 +321,6 @@ static int em_gio_probe(struct platform_device *pdev)
 	irq_chip->irq_mask = em_gio_irq_disable;
 	irq_chip->irq_unmask = em_gio_irq_enable;
 	irq_chip->irq_set_type = em_gio_irq_set_type;
-	irq_chip->irq_request_resources = em_gio_irq_reqres;
-	irq_chip->irq_release_resources = em_gio_irq_relres;
 	irq_chip->flags	= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND;
 
 	p->irq_domain = irq_domain_add_simple(pdev->dev.of_node, ngpios, 0,
-- 
2.20.1

Re: [PATCH 0/3] gpio: drop a few unneeded irq_{request,release}_resources implementations

[Linus Walleij]

On Fri, Feb 8, 2019 at 12:52 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:

> Three GPIO drivers in the tree implement the
> irq_{request,release}_resources hooks with what is in fact the default
> implementation provided by the GPIO core, making this driver-specific
> code redundant.

So you could think!

But the GPIOLIB_IRQCHIP code only kicks in if the GPIO driver
is using the gpiolib irqchip helpers, i.e. calls gpiolib_irqchip_add_*.
And those three are the ones that still have to implement their
own irqchips.

The helpers did come about because all drivers started to
duplicate code like this.

The designware chip should ideally be converted to use
hierarchical irqdomain since that seems to be what it is doing,
but I think the hierarchical irqdomain didn't even exist when that
driver was submitted.

Yours,
Linus Walleij

Re: [PATCH 1/3] gpio: bcm-kona: drop ->irq_{request,release}_resources hooks

[Scott Branden]

On 2019-02-08 3:51 a.m., Thomas Petazzoni wrote:
> Those hooks implement the exact same behavior as the default hooks by
> gpiolib, so there is no point in having a duplicated definition in
> gpio-bcm-kona.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Acked-by: Scott Branden <scott.branden@broadcom.com>
> Cc: Ray Jui <rjui@broadcom.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Scott Branden <sbranden@broadcom.com>
> Cc: bcm-kernel-feedback-list@broadcom.com
> ---
> Note: this commit was only build tested.
> ---
>   drivers/gpio/gpio-bcm-kona.c | 16 ----------------
>   1 file changed, 16 deletions(-)
>
> diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
> index c5536a509b59..0ae806edabc2 100644
> --- a/drivers/gpio/gpio-bcm-kona.c
> +++ b/drivers/gpio/gpio-bcm-kona.c
> @@ -484,28 +484,12 @@ static void bcm_kona_gpio_irq_handler(struct irq_desc *desc)
>   	chained_irq_exit(chip, desc);
>   }
>   
> -static int bcm_kona_gpio_irq_reqres(struct irq_data *d)
> -{
> -	struct bcm_kona_gpio *kona_gpio = irq_data_get_irq_chip_data(d);
> -
> -	return gpiochip_reqres_irq(&kona_gpio->gpio_chip, d->hwirq);
> -}
> -
> -static void bcm_kona_gpio_irq_relres(struct irq_data *d)
> -{
> -	struct bcm_kona_gpio *kona_gpio = irq_data_get_irq_chip_data(d);
> -
> -	gpiochip_relres_irq(&kona_gpio->gpio_chip, d->hwirq);
> -}
> -
>   static struct irq_chip bcm_gpio_irq_chip = {
>   	.name = "bcm-kona-gpio",
>   	.irq_ack = bcm_kona_gpio_irq_ack,
>   	.irq_mask = bcm_kona_gpio_irq_mask,
>   	.irq_unmask = bcm_kona_gpio_irq_unmask,
>   	.irq_set_type = bcm_kona_gpio_irq_set_type,
> -	.irq_request_resources = bcm_kona_gpio_irq_reqres,
> -	.irq_release_resources = bcm_kona_gpio_irq_relres,
>   };
>   
>   static struct of_device_id const bcm_kona_gpio_of_match[] = {