Skip to contentSkip to conversation

irqchip: bcm7120-l2: fix error handling of irq_of_parse_and_map

PATCH In review Dmitry Torokhov 4 participants 4 messages Nov 14, 2014 – Nov 26, 2014

Subsystems (3)
irqchip drivers Broadcom BCM7XXX Arm architecture Broadcom bmips MIPS architecture
lore
Share & tools
lore

Dmitry Torokhov

author
#0 lore raw

PATCH 1/1 irqchip: bcm7120-l2: fix error handling of irq_of_parse_and_map

Return value of irq_of_parse_and_map() is unsigned int, with 0 indicating failure, so testing for negative result never works.

Signed-off-by Dmitry Torokhov <[email protected]>

---

Not tested, found by casual code inspection.

 drivers/irqchip/irq-bcm7120-l2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/irqchip/irq-bcm7120-l2.c +2 -2
index e7c6155..8eec8e1 100644
@@ -102,9 +102,9 @@ static int bcm7120_l2_intc_init_one(struct device_node *dn,
 	unsigned int idx;
 
 	parent_irq = irq_of_parse_and_map(dn, irq);
-	if (parent_irq < 0) {
+	if (!parent_irq) {
 		pr_err("failed to map interrupt %d\n", irq);
-		return parent_irq;
+		return -EINVAL;
 	}
 
 	/* For multiple parent IRQs with multiple words, this looks like:
Signature · 5 lines
2.1.0.rc2.206.gedb03e5


-- 
Dmitry
Acked-by on v1 1/1
#1 lore raw

2014-11-14 14:16 GMT-08:00 Dmitry Torokhov <[email protected]>:

Quoted 4 lines

Return value of irq_of_parse_and_map() is unsigned int, with 0 indicating failure, so testing for negative result never works.

Signed-off-by Dmitry Torokhov <[email protected]>
Acked-by Florian Fainelli <[email protected]>
Quoted 29 lines

---

Not tested, found by casual code inspection.

 drivers/irqchip/irq-bcm7120-l2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/irqchip/irq-bcm7120-l2.c +2 -3
index e7c6155..8eec8e1 100644
@@ -102,9 +102,9 @@ static int bcm7120_l2_intc_init_one(struct device_node *dn,
        unsigned int idx;
 
        parent_irq = irq_of_parse_and_map(dn, irq);
-       if (parent_irq < 0) {
+       if (!parent_irq) {
                pr_err("failed to map interrupt %d\n", irq);
-               return parent_irq;
+               return -EINVAL;
        }
 
        /* For multiple parent IRQs with multiple words, this looks like:
--

2.1.0.rc2.206.gedb03e5

--
Dmitry

Signature · 1 line
Florian
Tested-by on v1 1/1
#2 lore raw
In reply to Dmitry Torokhov
Quoted 17 lines · Dmitry Torokhov

Return value of irq_of_parse_and_map() is unsigned int, with 0 indicating failure, so testing for negative result never works.

Signed-off-by Dmitry Torokhov <[email protected]>

---

Not tested, found by casual code inspection.

 drivers/irqchip/irq-bcm7120-l2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/irqchip/irq-bcm7120-l2.c +0 -0
index e7c6155..8eec8e1 100644
@@ -102,9 +102,9 @@ static int bcm7120_l2_intc_init_one(struct device_node *dn,
        unsigned int idx;

drivers/irqchip/irq-bcm7120-l2.c static int bcm7120_l2_intc_init_one(struct device_node *dn, · line 110

Quoted 10 lines
drivers/irqchip/irq-bcm7120-l2.c +2 -2
 
        parent_irq = irq_of_parse_and_map(dn, irq);
-       if (parent_irq < 0) {
+       if (!parent_irq) {
                pr_err("failed to map interrupt %d\n", irq);
-               return parent_irq;
+               return -EINVAL;
        }
 
        /* For multiple parent IRQs with multiple words, this looks like:

Hi Dmitry,

Thanks for the review. For this patch and for your irq-brcmstb-l2.c patch, I'll add:

Tested-by Kevin Cernekee <[email protected]>

The same bug also showed up in my new irq-bcm7038-l1.c driver; it will be fixed in the initial submission.

#3 lore raw
In reply to Dmitry Torokhov
Quoted 10 lines · Dmitry Torokhov

Return value of irq_of_parse_and_map() is unsigned int, with 0 indicating failure, so testing for negative result never works.

Signed-off-by Dmitry Torokhov <[email protected]>

---

Not tested, found by casual code inspection.

 drivers/irqchip/irq-bcm7120-l2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Both applied to irqchip/urgent with Florian's Ack and Kevin's Tested-by.

thx,

Jason.