[PATCH] of/irq: improve error message on irq discovery process failure

Subsystems: open firmware and flattened device tree, the rest

STALE3564d

3 messages, 3 authors, 2016-11-10 · open the first message on its own page

[PATCH] of/irq: improve error message on irq discovery process failure

From: Guilherme G. Piccoli <hidden>
Date: 2016-11-09 14:05:08

On PowerPC machines some PCI slots might not have Level-triggered
interrupts capability (also know as Level Signaled Interrupts - LSI),
leading of_irq_parse_pci() to complain by presenting error messages
on the kernel log - in this case, the properties "interrupt-map" and
"interrupt-map-mask" are not present on the device's node on device
tree.

This patch introduces a different message for this specific case,
and it also reduces the level of the message from error to warning.
Before this patch, when an adapter was plugged in a slot without Level
interrupts capabilities, we saw generic error messages like this:

    [54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-22

Now, with this applied, we see the following specific message:

    [19.947] pci 0014:60:00.0: of_irq_parse_pci() gave up. The slot of this
    device has no Level-triggered Interrupts capability.

No functional changes were introduced.

Signed-off-by: Guilherme G. Piccoli <redacted>
---
 drivers/of/irq.c        | 5 ++++-
 drivers/of/of_pci_irq.c | 8 +++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 393fea8..1ad6882 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -275,7 +275,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 	of_node_put(ipar);
 	of_node_put(newpar);
 
-	return -EINVAL;
+	/* Positive non-zero return means no Level-triggered Interrupts
+	 * capability was found.
+	 */
+	return ENOENT;
 }
 EXPORT_SYMBOL_GPL(of_irq_parse_raw);
 
diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
index 2306313..9b6f387 100644
--- a/drivers/of/of_pci_irq.c
+++ b/drivers/of/of_pci_irq.c
@@ -89,8 +89,14 @@ int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq
 	laddr[0] = cpu_to_be32((pdev->bus->number << 16) | (pdev->devfn << 8));
 	laddr[1] = laddr[2] = cpu_to_be32(0);
 	rc = of_irq_parse_raw(laddr, out_irq);
-	if (rc)
+
+	if (rc < 0) {
 		goto err;
+	} else if (rc > 0) {
+		dev_warn(&pdev->dev,
+			"of_irq_parse_pci() gave up. The slot of this device has no Level-triggered Interrupts capability.\n");
+		return -rc;
+	}
 	return 0;
 err:
 	dev_err(&pdev->dev, "of_irq_parse_pci() failed with rc=%d\n", rc);
-- 
2.1.0

Re: [PATCH] of/irq: improve error message on irq discovery process failure

From: Rob Herring <robh+dt@kernel.org>
Date: 2016-11-09 18:05:23

On Wed, Nov 9, 2016 at 8:05 AM, Guilherme G. Piccoli
[off-list ref] wrote:
quoted hunk
On PowerPC machines some PCI slots might not have Level-triggered
interrupts capability (also know as Level Signaled Interrupts - LSI),
leading of_irq_parse_pci() to complain by presenting error messages
on the kernel log - in this case, the properties "interrupt-map" and
"interrupt-map-mask" are not present on the device's node on device
tree.

This patch introduces a different message for this specific case,
and it also reduces the level of the message from error to warning.
Before this patch, when an adapter was plugged in a slot without Level
interrupts capabilities, we saw generic error messages like this:

    [54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-22

Now, with this applied, we see the following specific message:

    [19.947] pci 0014:60:00.0: of_irq_parse_pci() gave up. The slot of this
    device has no Level-triggered Interrupts capability.

No functional changes were introduced.

Signed-off-by: Guilherme G. Piccoli <redacted>
---
 drivers/of/irq.c        | 5 ++++-
 drivers/of/of_pci_irq.c | 8 +++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 393fea8..1ad6882 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -275,7 +275,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
        of_node_put(ipar);
        of_node_put(newpar);

-       return -EINVAL;
+       /* Positive non-zero return means no Level-triggered Interrupts
+        * capability was found.
+        */
+       return ENOENT;
It's not really a normal pattern to return positive errno values. You
should return a negative value and check for that specific error value
or perhaps move the print statement into this function.

Rob

Re: [PATCH] of/irq: improve error message on irq discovery process failure

From: Benjamin Herrenschmidt <hidden>
Date: 2016-11-10 21:28:32

On Wed, 2016-11-09 at 12:05 -0200, Guilherme G. Piccoli wrote:
quoted hunk
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 393fea8..1ad6882 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -275,7 +275,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
        of_node_put(ipar);
        of_node_put(newpar);
 
-       return -EINVAL;
+       /* Positive non-zero return means no Level-triggered Interrupts
+        * capability was found.
+        */
+       return ENOENT;
 }
 EXPORT_SYMBOL_GPL(of_irq_parse_raw);
I'm not fan. I'd rather it's -ENOENT and the callers can check for that
specific code rather than playing with the sign.

Cheers,
Ben.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help