Skip to contentSkip to conversation

gpio: dwapb: avoid error message for optional IRQ

PATCH In review Andy Shevchenko 3 participants 18 messages May 12, 2020 – May 18, 2020

Subsystems (2)
GPIO Synopsys DesignWare apb GPIO driver
lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

PATCH 1/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]>
Cc Serge Semin <[email protected]>
---
 drivers/gpio/gpio-dwapb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpio-dwapb.c +1 -1
index 8639c4a7f46974..5bc5057f071f37 100644
@@ -542,7 +542,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
author
#1 lore raw

PATCH 2/4 gpio: dwapb: Don't use 0 as valid Linux interrupt number

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

Signed-off-by Andy Shevchenko <[email protected]>
Cc Serge Semin <[email protected]>
---
 drivers/gpio/gpio-dwapb.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/gpio/gpio-dwapb.c +8 -8
index 5bc5057f071f37..78662d6d73634e 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);
 		}

@@ -531,23 +531,23 @@ 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 err, 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);
+			err = of_irq_get(np, j);
 		else if (has_acpi_companion(dev))
-			pp->irq[j] = platform_get_irq_optional(to_platform_device(dev), j);
+			err = platform_get_irq_optional(to_platform_device(dev), j);
+		if (err < 0)
+			continue;
 
-		if (pp->irq[j] >= 0)
-			pp->has_irq = true;
+		pp->irq[j] = err;
 	}
 
+	pp->has_irq = memchr_inv(pp->irq, 0, sizeof(pp->irq)) != NULL;
 	if (!pp->has_irq)
 		dev_warn(dev, "no irq for port%d\n", pp->idx);
 }
Signature · 1 line
2.26.2
#2 lore raw

Re: [PATCH v1 2/4] gpio: dwapb: Don't use 0 as valid Linux interrupt number

Quoted 23 lines · Andy Shevchenko

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

Signed-off-by Andy Shevchenko <[email protected]>
Cc Serge Semin <[email protected]>
---
 drivers/gpio/gpio-dwapb.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/gpio/gpio-dwapb.c +1 -1
index 5bc5057f071f37..78662d6d73634e 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);
 		}

drivers/gpio/gpio-dwapb.c static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode, · line 534

Quoted 5 lines
drivers/gpio/gpio-dwapb.c +1 -1
@@ -531,23 +531,23 @@ 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 err, j;

Err might be used uninitialized here. The compiler also says so:

drivers/gpio/gpio-dwapb.c:560:14: warning: ‘err’ may be used uninitialized in this function [-Wmaybe-uninitialized]
pp->irq[j] = err;

drivers/gpio/gpio-dwapb.c:547:6: note: ‘err’ was declared here
int err, j;

Could you please make it initialized with error number like -ENXIO by default?

Also if it was just a single issue in my mind, I wouldn't have probably payed much attention to this. But since you need to send v2 anyway, I'd prefer to have a positive naming here since normally both of_irq_get() and platform_get_irq_optional() return IRQ number, and error is returned only on failure. So checking an erroneous value of a positively named variable seems more natural, rather than copying an error-named variable to a normal variable. To cut it short could you please rename err to something like irq?

Quoted 13 lines
drivers/gpio/gpio-dwapb.c +2 -4
 	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);
+			err = of_irq_get(np, j);
 		else if (has_acpi_companion(dev))
-			pp->irq[j] = platform_get_irq_optional(to_platform_device(dev), j);
+			err = platform_get_irq_optional(to_platform_device(dev), j);

drivers/gpio/gpio-dwapb.c static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode, · line 550

Quoted 10 lines
drivers/gpio/gpio-dwapb.c +4 -2
+		if (err < 0)
+			continue;
 
-		if (pp->irq[j] >= 0)
-			pp->has_irq = true;
+		pp->irq[j] = err;
 	}
 
 
+	pp->has_irq = memchr_inv(pp->irq, 0, sizeof(pp->irq)) != NULL;

Sorry, but I don't see why is setting the has_irq flag in the loop above worse than using memchr_inv()? As I see it since we need to perform the loop above anyway, it would be normal to update the flag synchronously there instead of traversing the irq's array byte-by-byte again in the memchr_inv() method. Moreover (memchr_inv() != NULL) seems harder to read. A kernel hacker needs to keep in mind the method semantics, that it returns non-null if unmatched character found in the array, to get the line logic. Setting "has_irq = true" is straightforward - if IRQ's found then set the flag to true. So if you don't have a strong opinion against my reasoning could you please get the setting the has_irq flag back in to the loop above?

-Sergey

Quoted 6 lines
drivers/gpio/gpio-dwapb.c +0 -0
 	if (!pp->has_irq)
 		dev_warn(dev, "no irq for port%d\n", pp->idx);
 }
Signature · 1 line
2.26.2
author
#3 lore raw

Re: [PATCH v1 2/4] gpio: dwapb: Don't use 0 as valid Linux interrupt number

Quoted 3 lines · Serge Semin
Quoted 2 lines · Andy Shevchenko

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

Serge, thanks for review, my answers below.

...

Quoted 9 lines

drivers/gpio/gpio-dwapb.c static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode, · line 534

Quoted 1 line
drivers/gpio/gpio-dwapb.c +1 -0
+	int err, j;

Err might be used uninitialized here. The compiler also says so:

drivers/gpio/gpio-dwapb.c:560:14: warning: ‘err’ may be used uninitialized in this function [-Wmaybe-uninitialized]
pp->irq[j] = err;

drivers/gpio/gpio-dwapb.c:547:6: note: ‘err’ was declared here
int err, j;

Good catch! I'm puzzled how I missed it :-(
I usually compile with W=1.

Hmm... gcc (Debian 9.3.0-12) 9.3.0 building for i386, doesn't give me a warning, but I see that there is potentially possible that scenario, so, I'm going to fix it.

Quoted 1 line

Could you please make it initialized with error number like -ENXIO by default?

Will do.

Quoted 7 lines

Also if it was just a single issue in my mind, I wouldn't have probably payed much attention to this. But since you need to send v2 anyway, I'd prefer to have a positive naming here since normally both of_irq_get() and platform_get_irq_optional() return IRQ number, and error is returned only on failure. So checking an erroneous value of a positively named variable seems more natural, rather than copying an error-named variable to a normal variable. To cut it short could you please rename err to something like irq?

Makes sense.

...

Quoted 4 lines

drivers/gpio/gpio-dwapb.c static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode, · line 550

Quoted 1 line
drivers/gpio/gpio-dwapb.c +1 -0
+	pp->has_irq = memchr_inv(pp->irq, 0, sizeof(pp->irq)) != NULL;

Sorry, but I don't see why is setting the has_irq flag in the loop above worse than using memchr_inv()? As I see it since we need to perform the loop above anyway, it

Quoted 8 lines

would be normal to update the flag synchronously there instead of traversing the irq's array byte-by-byte again in the memchr_inv() method. Moreover (memchr_inv() != NULL) seems harder to read. A kernel hacker needs to keep in mind the method semantics, that it returns non-null if unmatched character found in the array, to get the line logic. Setting "has_irq = true" is straightforward - if IRQ's found then set the flag to true. So if you don't have a strong opinion against my reasoning could you please get the setting the has_irq flag back in to the loop above?

It's done in that way for a reason of the next clean ups, i.e. moving this to the actual user of it. I hope you already read further patches to see the intention behind this change. So, I prefer to leave it as is, however I agree with you in general.

Btw, we may also leave the domain when even no IRQ is available as some other drivers do, but I consider it less plausible than using memchr_inv().

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#4 lore raw

PATCH 3/4 gpio: dwapb: Drop extra check to call acpi_gpiochip_request_interrupts()

In reply to Andy Shevchenko

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.

While at it, add missed acpi_gpiochip_free_interrupts() call when unregistering ports.

Signed-off-by Andy Shevchenko <[email protected]>
Cc Serge Semin <[email protected]>
---
 drivers/gpio/gpio-dwapb.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)
drivers/gpio/gpio-dwapb.c +16 -9
index 78662d6d73634e..2975f2d369874a 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
#5 lore raw

Re: [PATCH v1 3/4] gpio: dwapb: Drop extra check to call acpi_gpiochip_request_interrupts()

Quoted 25 lines · Andy Shevchenko

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.

While at it, add missed acpi_gpiochip_free_interrupts() call when unregistering ports.

Signed-off-by Andy Shevchenko <[email protected]>
Cc Serge Semin <[email protected]>
---
 drivers/gpio/gpio-dwapb.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)
drivers/gpio/gpio-dwapb.c +1 -1
index 78662d6d73634e..2975f2d369874a 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);

drivers/gpio/gpio-dwapb.c static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, · line 516

Quoted 10 lines
drivers/gpio/gpio-dwapb.c +3 -4
-	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);

Hm, perhaps replacing it with:
+ if (pp->idx == 0)
+ acpi_gpiochip_request_interrupts(&port->gc);
could be more appropriate seeing Port A only supports IRQs, which we'd point out by the (idx == 0) conditional statement. So we don't have to call the method at most four times for each available port. Though judging by the acpi_gpiochip_request_interrupts() function internals it will just ignore GPIO chips with no IRQ support. Andy, It's up to you to decide. I'm not against the change the way it is, but if you agree that signifying the IRQs affiliation would be better, then please fill free to add the conditional statement I suggested.

Quoted 15 lines
drivers/gpio/gpio-dwapb.c +3 -4
-	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);

drivers/gpio/gpio-dwapb.c static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, · line 535

Quoted 10 lines
drivers/gpio/gpio-dwapb.c +9 -0
+	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);
+	}
 }

Could you please move this change to a dedicated patch? It seems to me this alteration might be appropriate to be ported to the stable kernels seeing it fixes e6cb3486f5a1 ("gpio: dwapb: add gpio-signaled acpi event support"). Linus, what do you think?

-Sergey

Quoted 5 lines
drivers/gpio/gpio-dwapb.c +0 -0
 static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode,
Signature · 1 line
2.26.2
#6 lore raw

Re: [PATCH v1 3/4] gpio: dwapb: Drop extra check to call acpi_gpiochip_request_interrupts()

Quoted 11 lines · Serge Semin
Quoted 10 lines · Andy Shevchenko

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.

While at it, add missed acpi_gpiochip_free_interrupts() call when unregistering ports.

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

---

[nip]

Quoted 7 lines

drivers/gpio/gpio-dwapb.c static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, · line 515

Quoted 3 lines
drivers/gpio/gpio-dwapb.c +1 -2
-	if (pp->has_irq)
-		acpi_gpiochip_request_interrupts(&port->gc);
+	acpi_gpiochip_request_interrupts(&port->gc);

Hm, perhaps replacing it with:

(quoted diff) +2 -0
+	if (pp->idx == 0)
+		acpi_gpiochip_request_interrupts(&port->gc);
Quoted 9 lines

could be more appropriate seeing Port A only supports IRQs, which we'd point out by the (idx == 0) conditional statement. So we don't have to call the method at most four times for each available port. Though judging by the acpi_gpiochip_request_interrupts() function internals it will just ignore GPIO chips with no IRQ support. Andy, It's up to you to decide. I'm not against the change the way it is, but if you agree that signifying the IRQs affiliation would be better, then please fill free to add the conditional statement I suggested.

Please see my comment below, before you decide what to do with this part of the patch.

[nip]

Quoted 24 lines
Quoted 8 lines

static void dwapb_gpio_unregister(struct dwapb_gpio *gpio) {

drivers/gpio/gpio-dwapb.c +0 -3
 	unsigned int m;
 
-	for (m = 0; m < gpio->nr_ports; ++m)
-		if (gpio->ports[m].is_registered)
-			gpiochip_remove(&gpio->ports[m].gc);

drivers/gpio/gpio-dwapb.c static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, · line 535

Quoted 10 lines
drivers/gpio/gpio-dwapb.c +9 -0
+	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);
+	}
 }

Could you please move this change to a dedicated patch? It seems to me this alteration might be appropriate to be ported to the stable kernels seeing it fixes e6cb3486f5a1 ("gpio: dwapb: add gpio-signaled acpi event support"). Linus, what do you think?

Quoted 2 lines

-Sergey

BTW after moving the change with acpi_gpiochip_free_interrupts() into a dedicated patch, you can freely merge the rest of this patch into the last one of this series. So the has_irq flag cleanup would be performed in a single commit. Especially if you implement the comment I provided above regarding conditional (idx == 0) calling of the acpi_gpiochip_request_interrupts() method.

So your series will look like this:
gpio: dwapb: avoid error message for optional IRQ
gpio: dwapb: Don't use 0 as valid Linux interrupt number
gpio: dwapb: Call acpi_gpiochip_free_interrupts() on GPIO chip de-registration (<= This commit can be moved to the head of the series as being marked by the Fixes tag)
gpio: dwapb: Remove redundant has_irq flag support

-Sergey

author
#7 lore raw

Re: [PATCH v1 3/4] gpio: dwapb: Drop extra check to call acpi_gpiochip_request_interrupts()

Quoted 2 lines · Serge Semin
Quoted 1 line · Serge Semin

On Tue, May 12, 2020 at 09:45:12PM +0300, Andy Shevchenko wrote:

...

Quoted 23 lines
Quoted 23 lines
Quoted 7 lines

static void dwapb_gpio_unregister(struct dwapb_gpio *gpio) {

drivers/gpio/gpio-dwapb.c +0 -3
 	unsigned int m;
 
-	for (m = 0; m < gpio->nr_ports; ++m)
-		if (gpio->ports[m].is_registered)
-			gpiochip_remove(&gpio->ports[m].gc);

drivers/gpio/gpio-dwapb.c static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, · line 535

Quoted 10 lines
drivers/gpio/gpio-dwapb.c +9 -0
+	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);
+	}
 }

Could you please move this change to a dedicated patch? It seems to me this alteration might be appropriate to be ported to the stable kernels seeing it fixes e6cb3486f5a1 ("gpio: dwapb: add gpio-signaled acpi event support"). Linus, what do you think?

Quoted 8 lines
Quoted 2 lines

-Sergey

BTW after moving the change with acpi_gpiochip_free_interrupts() into a dedicated patch, you can freely merge the rest of this patch into the last one of this series. So the has_irq flag cleanup would be performed in a single commit. Especially if you implement the comment I provided above regarding conditional (idx == 0) calling of the acpi_gpiochip_request_interrupts() method.

I was thinking about this split and came to the conclusion that it will be a bit awkward to introduce additional check in the ->dwapb_gpio_unregister() for freeing ACPI Event handling which will be removed by one of following patch (even taking into consideration backporting).

What I propose here is an alternative, i.e. I make this patch as first in the series and will focus / dedicate it as a fix rather than clean up.

Quoted 6 lines

So your series will look like this:
gpio: dwapb: avoid error message for optional IRQ
gpio: dwapb: Don't use 0 as valid Linux interrupt number

gpio: dwapb: Call acpi_gpiochip_free_interrupts() on GPIO chip de-registration (<= This commit can be moved to the head of the series as being marked by the Fixes tag)

Yes, something like this, but keeping this form of the patch.

Thanks for the suggestion!

Quoted 1 line

gpio: dwapb: Remove redundant has_irq flag support

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#8 lore raw

Re: [PATCH v1 3/4] gpio: dwapb: Drop extra check to call acpi_gpiochip_request_interrupts()

In reply to Serge Semin
Quoted 4 lines · Serge Semin
Quoted 3 lines · Andy Shevchenko

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

drivers/gpio/gpio-dwapb.c static int dwapb_gpio_add_port(struct dwapb_gpio *gpio, · line 515

Quoted 3 lines
drivers/gpio/gpio-dwapb.c +1 -2
-	if (pp->has_irq)
-		acpi_gpiochip_request_interrupts(&port->gc);
+	acpi_gpiochip_request_interrupts(&port->gc);

Hm, perhaps replacing it with:

(quoted diff) +2 -0
+	if (pp->idx == 0)
+		acpi_gpiochip_request_interrupts(&port->gc);
Quoted 8 lines

could be more appropriate seeing Port A only supports IRQs, which we'd point out by the (idx == 0) conditional statement. So we don't have to call the method at most four times for each available port. Though judging by the acpi_gpiochip_request_interrupts() function internals it will just ignore GPIO chips with no IRQ support. Andy, It's up to you to decide. I'm not against the change the way it is, but if you agree that signifying the IRQs affiliation would be better, then please fill free to add the conditional statement I suggested.

It's really harmless to call it for each port. It allows as a side effect see issues with ACPI tables which may refer to a wrong port / device and thus getting no certain event handled. I prefer to unconditionally call it.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#9 lore raw

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

Signed-off-by Andy Shevchenko <[email protected]>
Cc Serge Semin <[email protected]>
---
 drivers/gpio/gpio-dwapb.c                | 12 +++++++-----
 include/linux/platform_data/gpio-dwapb.h |  1 -
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/gpio/gpio-dwapb.c +7 -5
index 2975f2d369874a..87834adccc4534 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);

@@ -553,10 +559,6 @@ static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode,
 
 		pp->irq[j] = err;
 	}
-
-	pp->has_irq = memchr_inv(pp->irq, 0, sizeof(pp->irq)) != NULL;
-	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)
include/linux/platform_data/gpio-dwapb.h +0 -1
index 3c606c450d0596..ff1be737bad6aa 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
build bot
#10 lore raw

Re: [PATCH v1 4/4] gpio: dwapb: Remove unneeded has_irq member in struct dwapb_port_property

I love your patch! Yet something to improve:

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on next-20200512]
[cannot apply to linus/master v5.7-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Andy-Shevchenko/gpio-dwapb-avoid-error-message-for-optional-IRQ/20200513-025227
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-a003-20200512 (attached as .config)

reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate

Reported-by kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_gpio_setup':

>> drivers/mfd/intel_quark_i2c_gpio.c:219:19: error: 'struct dwapb_port_property' has no member named 'has_irq'

pdata->properties->has_irq = true;
^~

vim +219 drivers/mfd/intel_quark_i2c_gpio.c

60ae5b9f5cdd80 Raymond Tan   2015-02-02  189  
60ae5b9f5cdd80 Raymond Tan   2015-02-02  190  static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
60ae5b9f5cdd80 Raymond Tan   2015-02-02  191  {
60ae5b9f5cdd80 Raymond Tan   2015-02-02  192  	struct dwapb_platform_data *pdata;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  193  	struct resource *res = (struct resource *)cell->resources;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  194  	struct device *dev = &pdev->dev;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  195  
60ae5b9f5cdd80 Raymond Tan   2015-02-02  196  	res[INTEL_QUARK_IORES_MEM].start =
60ae5b9f5cdd80 Raymond Tan   2015-02-02  197  		pci_resource_start(pdev, MFD_GPIO_BAR);
60ae5b9f5cdd80 Raymond Tan   2015-02-02  198  	res[INTEL_QUARK_IORES_MEM].end =
60ae5b9f5cdd80 Raymond Tan   2015-02-02  199  		pci_resource_end(pdev, MFD_GPIO_BAR);
60ae5b9f5cdd80 Raymond Tan   2015-02-02  200  
60ae5b9f5cdd80 Raymond Tan   2015-02-02  201  	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
60ae5b9f5cdd80 Raymond Tan   2015-02-02  202  	if (!pdata)
60ae5b9f5cdd80 Raymond Tan   2015-02-02  203  		return -ENOMEM;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  204  
60ae5b9f5cdd80 Raymond Tan   2015-02-02  205  	/* For intel quark x1000, it has only one port: portA */
60ae5b9f5cdd80 Raymond Tan   2015-02-02  206  	pdata->nports = INTEL_QUARK_GPIO_NPORTS;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  207  	pdata->properties = devm_kcalloc(dev, pdata->nports,
60ae5b9f5cdd80 Raymond Tan   2015-02-02  208  					 sizeof(*pdata->properties),
60ae5b9f5cdd80 Raymond Tan   2015-02-02  209  					 GFP_KERNEL);
60ae5b9f5cdd80 Raymond Tan   2015-02-02  210  	if (!pdata->properties)
60ae5b9f5cdd80 Raymond Tan   2015-02-02  211  		return -ENOMEM;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  212  
60ae5b9f5cdd80 Raymond Tan   2015-02-02  213  	/* Set the properties for portA */
4ba8cfa79f44a9 Jiang Qiu     2016-04-28  214  	pdata->properties->fwnode	= NULL;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  215  	pdata->properties->idx		= 0;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  216  	pdata->properties->ngpio	= INTEL_QUARK_MFD_NGPIO;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  217  	pdata->properties->gpio_base	= INTEL_QUARK_MFD_GPIO_BASE;
e6ca26abd37606 Phil Edworthy 2018-04-26  218  	pdata->properties->irq[0]	= pdev->irq;
e6ca26abd37606 Phil Edworthy 2018-04-26 @219  	pdata->properties->has_irq	= true;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  220  	pdata->properties->irq_shared	= true;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  221  
60ae5b9f5cdd80 Raymond Tan   2015-02-02  222  	cell->platform_data = pdata;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  223  	cell->pdata_size = sizeof(*pdata);
60ae5b9f5cdd80 Raymond Tan   2015-02-02  224  
60ae5b9f5cdd80 Raymond Tan   2015-02-02  225  	return 0;
60ae5b9f5cdd80 Raymond Tan   2015-02-02  226  }
60ae5b9f5cdd80 Raymond Tan   2015-02-02  227

:::::: The code at line 219 was first introduced by commit
:::::: e6ca26abd37606ba4864f20c85d3fe4a2173b93f gpio: dwapb: Add support for 1 interrupt per port A GPIO

:::::: TO: Phil Edworthy <[email protected]> :::::: CC: Linus Walleij <[email protected]>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

author
#11 lore raw

Re: [PATCH v1 4/4] gpio: dwapb: Remove unneeded has_irq member in struct dwapb_port_property

Quoted 23 lines · kbuild test robot

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on next-20200512]
[cannot apply to linus/master v5.7-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Andy-Shevchenko/gpio-dwapb-avoid-error-message-for-optional-IRQ/20200513-025227
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-a003-20200512 (attached as .config)

reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate

Quoted 8 lines
Reported-by kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_gpio_setup':

Quoted 1 line
Quoted 1 line

drivers/mfd/intel_quark_i2c_gpio.c:219:19: error: 'struct dwapb_port_property' has no member named 'has_irq'

pdata->properties->has_irq = true;
^~

Indeed. I have that removed by other WIP patch in my tree. Thanks!

Signature · 2 lines
With Best Regards,
Andy Shevchenko
build bot
#12 lore raw

Re: [PATCH v1 4/4] gpio: dwapb: Remove unneeded has_irq member in struct dwapb_port_property

In reply to Andy Shevchenko
I love your patch! Yet something to improve:

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on next-20200512]
[cannot apply to linus/master v5.7-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Andy-Shevchenko/gpio-dwapb-avoid-error-message-for-optional-IRQ/20200513-025227
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386

If you fix the issue, kindly add following tag as appropriate

Reported-by kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/mfd/intel_quark_i2c_gpio.c: In function 'intel_quark_gpio_setup':

>> drivers/mfd/intel_quark_i2c_gpio.c:219:19: error: 'struct dwapb_port_property' has no member named 'has_irq'

pdata->properties->has_irq = true;
^~

# https://github.com/0day-ci/linux/commit/f8686f0bb4ff7fdded832484a559f1454a2a51d2
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout f8686f0bb4ff7fdded832484a559f1454a2a51d2
vim +219 drivers/mfd/intel_quark_i2c_gpio.c

60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  189  
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  190  static int intel_quark_gpio_setup(struct pci_dev *pdev, struct mfd_cell *cell)
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  191  {
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  192  	struct dwapb_platform_data *pdata;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  193  	struct resource *res = (struct resource *)cell->resources;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  194  	struct device *dev = &pdev->dev;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  195  
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  196  	res[INTEL_QUARK_IORES_MEM].start =
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  197  		pci_resource_start(pdev, MFD_GPIO_BAR);
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  198  	res[INTEL_QUARK_IORES_MEM].end =
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  199  		pci_resource_end(pdev, MFD_GPIO_BAR);
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  200  
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  201  	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  202  	if (!pdata)
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  203  		return -ENOMEM;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  204  
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  205  	/* For intel quark x1000, it has only one port: portA */
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  206  	pdata->nports = INTEL_QUARK_GPIO_NPORTS;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  207  	pdata->properties = devm_kcalloc(dev, pdata->nports,
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  208  					 sizeof(*pdata->properties),
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  209  					 GFP_KERNEL);
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  210  	if (!pdata->properties)
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  211  		return -ENOMEM;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  212  
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  213  	/* Set the properties for portA */
4ba8cfa79f44a948 Jiang Qiu     2016-04-28  214  	pdata->properties->fwnode	= NULL;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  215  	pdata->properties->idx		= 0;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  216  	pdata->properties->ngpio	= INTEL_QUARK_MFD_NGPIO;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  217  	pdata->properties->gpio_base	= INTEL_QUARK_MFD_GPIO_BASE;
e6ca26abd37606ba Phil Edworthy 2018-04-26  218  	pdata->properties->irq[0]	= pdev->irq;
e6ca26abd37606ba Phil Edworthy 2018-04-26 @219  	pdata->properties->has_irq	= true;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  220  	pdata->properties->irq_shared	= true;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  221  
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  222  	cell->platform_data = pdata;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  223  	cell->pdata_size = sizeof(*pdata);
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  224  
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  225  	return 0;
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  226  }
60ae5b9f5cdd80c5 Raymond Tan   2015-02-02  227

:::::: The code at line 219 was first introduced by commit
:::::: e6ca26abd37606ba4864f20c85d3fe4a2173b93f gpio: dwapb: Add support for 1 interrupt per port A GPIO

:::::: TO: Phil Edworthy <[email protected]> :::::: CC: Linus Walleij <[email protected]>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave(a)lists.01.org

Reviewed-by on v1 4/4
#13 lore raw

Re: [PATCH v1 4/4] gpio: dwapb: Remove unneeded has_irq member in struct dwapb_port_property

In reply to Andy Shevchenko
Quoted 15 lines · 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.

Signed-off-by Andy Shevchenko <[email protected]>
Cc Serge Semin <[email protected]>
---
 drivers/gpio/gpio-dwapb.c                | 12 +++++++-----
 include/linux/platform_data/gpio-dwapb.h |  1 -
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/gpio/gpio-dwapb.c +0 -0
index 2975f2d369874a..87834adccc4534 100644

drivers/gpio/gpio-dwapb.c static void dwapb_configure_irqs(struct dwapb_gpio *gpio, · line 374

Quoted 9 lines
drivers/gpio/gpio-dwapb.c +5 -0
@@ -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;
+	}
+

Ah, that's why you added the memchr_inv() method in patch 2. So to move it to dwapb_configure_irqs() at this point. Anyway I still think, that it would be better to leave the has_irq initialization in the loop there, but here you could just remove that assignment. For this patch:

Reviewed-by Serge Semin <[email protected]>

I'll test the whole series up when you send v2. Our hardware is equipped with two DW APB GPIO IPs with Port A enabled for each. One of them is connected to an interrupt controller by a single line.

-Sergey

Quoted 39 lines
drivers/gpio/gpio-dwapb.c +0 -0
 	gpio->domain = irq_domain_create_linear(fwnode, ngpio,
 						 &irq_generic_chip_ops, gpio);
drivers/gpio/gpio-dwapb.c +2 -5
 	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);

@@ -553,10 +559,6 @@ static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode,
 
 		pp->irq[j] = err;
 	}
-
-	pp->has_irq = memchr_inv(pp->irq, 0, sizeof(pp->irq)) != NULL;
-	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)
include/linux/platform_data/gpio-dwapb.h +0 -1
index 3c606c450d0596..ff1be737bad6aa 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
author
#14 lore raw

Re: [PATCH v1 4/4] gpio: dwapb: Remove unneeded has_irq member in struct dwapb_port_property

Quoted 3 lines · Serge Semin
Quoted 2 lines · 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.

...

Quoted 8 lines

drivers/gpio/gpio-dwapb.c static void dwapb_configure_irqs(struct dwapb_gpio *gpio, · line 373

Quoted 5 lines
drivers/gpio/gpio-dwapb.c +5 -0
+	if (memchr_inv(pp->irq, 0, sizeof(pp->irq)) == NULL) {
+		dev_warn(gpio->dev, "no IRQ for port%d\n", pp->idx);
+		return;
+	}
+

Ah, that's why you added the memchr_inv() method in patch 2. So to move it to dwapb_configure_irqs() at this point.

Yes.

Quoted 3 lines

Anyway I still think, that it would be
better to leave the has_irq initialization in the loop there, but here you could just remove that assignment.

I think you noticed that I don't like to ping-pong in the series, for what you propose it would be something like

original --->>>

	if (pp->irq[j] >= 0)
		pp->has_irq = true;

after patch 2 (if your suggestion applied) --->>>

	if (irq > 0) {
		pp->irq[j] = irq;
		pp->has->irq = true;
	}

after this patch --->>>

	if (irq > 0)
		pp->irq[j] = irq;

I prefer not to do this.

OTOH, I guess it might work if we leave original conditional separate to assignment of IRQ itself (with '>= 0' -> '> 0' being replaced).

I'll look what can I do here.

Quoted 3 lines

For this patch:

Reviewed-by Serge Semin <[email protected]>

Thank you!

Quoted 3 lines

I'll test the whole series up when you send v2. Our hardware is equipped with two DW APB GPIO IPs with Port A enabled for each. One of them is connected to an interrupt controller by a single line.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#15 lore raw
In reply to Andy Shevchenko
Quoted 3 lines · Andy Shevchenko

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.

I will wait for Serge and others to comment before I'll send v2 with a fix to the last patch.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#16 lore raw

Hello Andy

Quoted 7 lines · Andy Shevchenko
Quoted 3 lines · Andy Shevchenko

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.

I will wait for Serge and others to comment before I'll send v2 with a fix to the last patch.

Thanks for the patchset. I'll have it reviewed it in a few days.

-Sergey

Quoted 6 lines
Signature · 3 lines
-- 
With Best Regards,
Andy Shevchenko
Reviewed-by on v1 1/4
#17 lore raw
In reply to Andy Shevchenko

Hello Andy,

Thanks for sending this cleanup series. By comments are below and inlined in the individual patches further in the patchset.

Quoted 15 lines · Andy Shevchenko

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]>
Cc Serge Semin <[email protected]>
---
 drivers/gpio/gpio-dwapb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpio-dwapb.c +0 -0
index 8639c4a7f46974..5bc5057f071f37 100644

drivers/gpio/gpio-dwapb.c static void dwapb_get_irq(struct device *dev, struct fwnode_handle *fwnode, · line 545

Quoted 5 lines
drivers/gpio/gpio-dwapb.c +1 -1
@@ -542,7 +542,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 only we didn't have sub-nodes describing the GPIO ports, we could have used the platform_get_irq_optional() here for both DT and ACPI based devices... Since we do, there is no way but to have the "if (np) else if(has_acpi_companion())" conditional statement here. Thanks for sending this patch.

Reviewed-by Serge Semin <[email protected]>

-Sergey

Quoted 6 lines
drivers/gpio/gpio-dwapb.c +0 -0
 		if (pp->irq[j] >= 0)
 			pp->has_irq = true;
Signature · 1 line
2.26.2