gpio: dwapb: Call acpi_gpiochip_free_interrupts() on GPIO chip de-registration

PATCH In review Andy Shevchenko 2 participants 5 messages May 19, 2020 – May 25, 2020

Subsystems (3)

GPIO Multifunction devices (MFD) Synopsys DesignWare apb GPIO driver

lore

Share & tools

lore

Andy Shevchenko

author

2020-05-19 13:12 UTC #0 lore raw

PATCH v3 1/4 gpio: dwapb: Call acpi_gpiochip_free_interrupts() on GPIO chip de-registration

Add missed acpi_gpiochip_free_interrupts() call when unregistering ports.

While at it, drop extra check to call acpi_gpiochip_request_interrupts(). There is no need to have an additional check to call acpi_gpiochip_request_interrupts(). Even without any interrupts available the registered ACPI Event handlers can be useful for debugging purposes.

Fixes e6cb3486f5a1 ("gpio: dwapb: add gpio-signaled acpi event support")

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Serge Semin <[email protected]>

Tested-by Serge Semin <[email protected]>

---
v3: appended tags (Serge)

 drivers/gpio/gpio-dwapb.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

drivers/gpio/gpio-dwapb.c +16 -9

index 8639c4a7f469..e5d844304f8d 100644
@@ -505,26 +505,33 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
 		dwapb_configure_irqs(gpio, port, pp);
 
 	err = gpiochip_add_data(&port->gc, port);
-	if (err)
+	if (err) {
 		dev_err(gpio->dev, "failed to register gpiochip for port%d\n",
 			port->idx);
-	else
-		port->is_registered = true;
+		return err;
+	}
 
 	/* Add GPIO-signaled ACPI event support */
-	if (pp->has_irq)
-		acpi_gpiochip_request_interrupts(&port->gc);
+	acpi_gpiochip_request_interrupts(&port->gc);
 
-	return err;
+	port->is_registered = true;
+
+	return 0;
 }
 
 static void dwapb_gpio_unregister(struct dwapb_gpio *gpio)
 {
 	unsigned int m;
 
-	for (m = 0; m < gpio->nr_ports; ++m)
-		if (gpio->ports[m].is_registered)
-			gpiochip_remove(&gpio->ports[m].gc);
+	for (m = 0; m < gpio->nr_ports; ++m) {
+		struct dwapb_gpio_port *port = &gpio->ports[m];
+
+		if (!port->is_registered)
+			continue;
+
+		acpi_gpiochip_free_interrupts(&port->gc);
+		gpiochip_remove(&port->gc);
+	}
 }
 
 static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode,

Signature · 1 line

2.26.2

Andy Shevchenko

author

2020-05-19 13:12 UTC #1 lore raw

PATCH v3 2/4 gpio: dwapb: avoid error message for optional IRQ

platform_get_irq() will generate an error message if the requested IRQ is not present. Use platform_get_irq_optional() to avoid the error message being generated.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Serge Semin <[email protected]>

Tested-by Serge Semin <[email protected]>

Reviewed-by Serge Semin <[email protected]>

---
v3: appended tags (Serge)

 drivers/gpio/gpio-dwapb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpio/gpio-dwapb.c +1 -1

index e5d844304f8d..944dae80d687 100644
@@ -549,7 +549,7 @@ static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode,
 		if (np)
 			pp->irq[j] = of_irq_get(np, j);
 		else if (has_acpi_companion(dev))
-			pp->irq[j] = platform_get_irq(to_platform_device(dev), j);
+			pp->irq[j] = platform_get_irq_optional(to_platform_device(dev), j);
 
 		if (pp->irq[j] >= 0)
 			pp->has_irq = true;

Signature · 1 line

2.26.2

Andy Shevchenko

author

2020-05-19 13:12 UTC #2 lore raw

PATCH v3 3/4 gpio: dwapb: Don't use IRQ 0 as valid Linux interrupt

In reply to Andy Shevchenko

IRQ 0 is not valid in Linux interrupt number space. Refactor the code with this kept in mind.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Serge Semin <[email protected]>

Tested-by Serge Semin <[email protected]>

---
v3: appended tags (Serge)

 drivers/gpio/gpio-dwapb.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/gpio/gpio-dwapb.c +7 -7

index 944dae80d687..d3765672c42b 100644
@@ -417,7 +417,7 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
 		int i;
 
 		for (i = 0; i < pp->ngpio; i++) {
-			if (pp->irq[i] >= 0)
+			if (pp->irq[i])
 				irq_set_chained_handler_and_data(pp->irq[i],
 						dwapb_irq_handler, gpio);
 		}

@@ -538,20 +538,20 @@ static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode,
 			  struct dwapb_port_property *pp)
 {
 	struct device_node *np = NULL;
-	int j;
+	int irq = -ENXIO, j;
 
 	if (fwnode_property_read_bool(fwnode, "interrupt-controller"))
 		np = to_of_node(fwnode);
 
 	for (j = 0; j < pp->ngpio; j++) {
-		pp->irq[j] = -ENXIO;
-
 		if (np)
-			pp->irq[j] = of_irq_get(np, j);
+			irq = of_irq_get(np, j);
 		else if (has_acpi_companion(dev))
-			pp->irq[j] = platform_get_irq_optional(to_platform_device(dev), j);
+			irq = platform_get_irq_optional(to_platform_device(dev), j);
+		if (irq > 0)
+			pp->irq[j] = irq;
 
-		if (pp->irq[j] >= 0)
+		if (pp->irq[j])
 			pp->has_irq = true;
 	}

Signature · 1 line

2.26.2

Andy Shevchenko

author

2020-05-19 13:12 UTC #3 lore raw

PATCH v3 4/4 gpio: dwapb: Remove unneeded has_irq member in struct dwapb_port_property

In reply to Andy Shevchenko

has_irq member of struct dwapb_port_property is used only in one place, so, make it local test instead and remove from the structure. This local test is using memchr_inv() which is quite efficient in comparison to the original loop and possible little overhead can be neglected.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Lee Jones <[email protected]>

Acked-by Serge Semin <[email protected]>

Tested-by Serge Semin <[email protected]>

---
v3: appended tags (Serge, Lee)

 drivers/gpio/gpio-dwapb.c                | 14 +++++++-------
 drivers/mfd/intel_quark_i2c_gpio.c       |  1 -
 include/linux/platform_data/gpio-dwapb.h |  1 -
 3 files changed, 7 insertions(+), 9 deletions(-)

drivers/gpio/gpio-dwapb.c +7 -7

index d3765672c42b..1d8d55bd63aa 100644
@@ -366,6 +366,11 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
 	irq_hw_number_t hwirq;
 	int err, i;
 
+	if (memchr_inv(pp->irq, 0, sizeof(pp->irq)) == NULL) {
+		dev_warn(gpio->dev, "no IRQ for port%d\n", pp->idx);
+		return;
+	}
+
 	gpio->domain = irq_domain_create_linear(fwnode, ngpio,
 						 &irq_generic_chip_ops, gpio);
 	if (!gpio->domain)

@@ -501,7 +506,8 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
 	if (pp->idx == 0)
 		port->gc.set_config = dwapb_gpio_set_config;
 
-	if (pp->has_irq)
+	/* Only port A can provide interrupts in all configurations of the IP */
+	if (pp->idx == 0)
 		dwapb_configure_irqs(gpio, port, pp);
 
 	err = gpiochip_add_data(&port->gc, port);

@@ -550,13 +556,7 @@ static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode,
 			irq = platform_get_irq_optional(to_platform_device(dev), j);
 		if (irq > 0)
 			pp->irq[j] = irq;
-
-		if (pp->irq[j])
-			pp->has_irq = true;
 	}
-
-	if (!pp->has_irq)
-		dev_warn(dev, "no irq for port%d\n", pp->idx);
 }
 
 static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev)

drivers/mfd/intel_quark_i2c_gpio.c +0 -1

index 41326b48da55..84ca7902e1df 100644
@@ -216,7 +216,6 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 	pdata->properties->ngpio	= INTEL_QUARK_MFD_NGPIO;
 	pdata->properties->gpio_base	= INTEL_QUARK_MFD_GPIO_BASE;
 	pdata->properties->irq[0]	= pdev->irq;
-	pdata->properties->has_irq	= true;
 	pdata->properties->irq_shared	= true;
 
 	cell->platform_data = pdata;

include/linux/platform_data/gpio-dwapb.h +0 -1

index 3c606c450d05..ff1be737bad6 100644
@@ -12,7 +12,6 @@ struct dwapb_port_property {
 	unsigned int	ngpio;
 	unsigned int	gpio_base;
 	int		irq[32];
-	bool		has_irq;
 	bool		irq_shared;
 };

Signature · 1 line

2.26.2

Linus Walleij

2020-05-25 09:05 UTC #4 lore raw

In reply to Andy Shevchenko

Quoted 7 lines · Andy Shevchenko

Add missed acpi_gpiochip_free_interrupts() call when unregistering ports.
While at it, drop extra check to call acpi_gpiochip_request_interrupts(). There is no need to have an additional check to call acpi_gpiochip_request_interrupts(). Even without any interrupts available the registered ACPI Event handlers can be useful for debugging purposes.

Quoted 6 lines

Fixes e6cb3486f5a1 ("gpio: dwapb: add gpio-signaled acpi event support")
Signed-off-by Andy Shevchenko <[email protected]>
Acked-by Serge Semin <[email protected]>
Tested-by Serge Semin <[email protected]>
---
v3: appended tags (Serge)

All four patches applied, thanks!

Yours,
Linus Walleij