[PATCH net-next 0/2] ATU and VTU irq fixes

STALE3140d

Revision v1 of 2 in this series.

7 messages, 3 authors, 2018-01-16 · open the first message on its own page

[PATCH net-next 0/2] ATU and VTU irq fixes

From: Andrew Lunn <andrew@lunn.ch>
Date: 2018-01-15 22:47:17

Further testing and core review found two sets of bugs.

Core review found a cut/paste error in the irq setup code.

A board which does not have an interrupt line from the switch to the
SoC, and experiancing an EPROBE_DEFER throw a splat when the ATU irq
was freed but never registered.

Andrew Lunn (2):
  net: dsa: mv88e6xxx: Return error from irq_find_mapping()
  net: dsa: mv88e6xxx: Free ATU/VTU irq only when there is chip irq

 drivers/net/dsa/mv88e6xxx/chip.c        | 6 ++++--
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 2 +-
 drivers/net/dsa/mv88e6xxx/global1_vtu.c | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.15.1

[PATCH net-next 2/2] net: dsa: mv88e6xxx: Free ATU/VTU irq only when there is chip irq

From: Andrew Lunn <andrew@lunn.ch>
Date: 2018-01-15 22:46:47

We only register the ATU and VTU irq when we have a chip level IRQ.
In the error path, we should only attempt to remove the ATU and VTU
irq if we also have a chip level IRQ.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 54cb00a27408..eb328bade225 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3999,9 +3999,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 out_mdio:
 	mv88e6xxx_mdios_unregister(chip);
 out_g1_vtu_prob_irq:
-	mv88e6xxx_g1_vtu_prob_irq_free(chip);
+	if (chip->irq > 0)
+		mv88e6xxx_g1_vtu_prob_irq_free(chip);
 out_g1_atu_prob_irq:
-	mv88e6xxx_g1_atu_prob_irq_free(chip);
+	if (chip->irq > 0)
+		mv88e6xxx_g1_atu_prob_irq_free(chip);
 out_g2_irq:
 	if (chip->info->g2_irqs > 0 && chip->irq > 0)
 		mv88e6xxx_g2_irq_free(chip);
-- 
2.15.1

[PATCH net-next 1/2] net: dsa: mv88e6xxx: Return error from irq_find_mapping()

From: Andrew Lunn <andrew@lunn.ch>
Date: 2018-01-15 22:47:02

Fix a cut/paste error. When irq_find_mapping() returns an error for
the ATU or VTU interrupt, return that error, not the value of
chip->device_irq.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 2 +-
 drivers/net/dsa/mv88e6xxx/global1_vtu.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index b97de9d36337..20d941f4273b 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -377,7 +377,7 @@ int mv88e6xxx_g1_atu_prob_irq_setup(struct mv88e6xxx_chip *chip)
 	chip->atu_prob_irq = irq_find_mapping(chip->g1_irq.domain,
 					      MV88E6XXX_G1_STS_IRQ_ATU_PROB);
 	if (chip->atu_prob_irq < 0)
-		return chip->device_irq;
+		return chip->atu_prob_irq;
 
 	err = request_threaded_irq(chip->atu_prob_irq, NULL,
 				   mv88e6xxx_g1_atu_prob_irq_thread_fn,
diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
index 53d58a01484a..40fe81af64d5 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
@@ -570,7 +570,7 @@ int mv88e6xxx_g1_vtu_prob_irq_setup(struct mv88e6xxx_chip *chip)
 	chip->vtu_prob_irq = irq_find_mapping(chip->g1_irq.domain,
 					      MV88E6XXX_G1_STS_IRQ_VTU_PROB);
 	if (chip->vtu_prob_irq < 0)
-		return chip->device_irq;
+		return chip->chip->vtu_prob_irq;
 
 	err = request_threaded_irq(chip->vtu_prob_irq, NULL,
 				   mv88e6xxx_g1_vtu_prob_irq_thread_fn,
-- 
2.15.1

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Free ATU/VTU irq only when there is chip irq

From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2018-01-15 22:54:26

On 01/15/2018 02:45 PM, Andrew Lunn wrote:
quoted hunk
We only register the ATU and VTU irq when we have a chip level IRQ.
In the error path, we should only attempt to remove the ATU and VTU
irq if we also have a chip level IRQ.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 54cb00a27408..eb328bade225 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3999,9 +3999,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 out_mdio:
 	mv88e6xxx_mdios_unregister(chip);
 out_g1_vtu_prob_irq:
-	mv88e6xxx_g1_vtu_prob_irq_free(chip);
+	if (chip->irq > 0)
+		mv88e6xxx_g1_vtu_prob_irq_free(chip);
Why not move this check to mv88e6xxx_g1_vtu_prob_irq_free() and make it
a no-op if chip->irq <= 0?
-- 
Florian

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Free ATU/VTU irq only when there is chip irq

From: Andrew Lunn <andrew@lunn.ch>
Date: 2018-01-15 23:02:26

On Mon, Jan 15, 2018 at 02:54:22PM -0800, Florian Fainelli wrote:
On 01/15/2018 02:45 PM, Andrew Lunn wrote:
quoted
We only register the ATU and VTU irq when we have a chip level IRQ.
In the error path, we should only attempt to remove the ATU and VTU
irq if we also have a chip level IRQ.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 54cb00a27408..eb328bade225 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3999,9 +3999,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 out_mdio:
 	mv88e6xxx_mdios_unregister(chip);
 out_g1_vtu_prob_irq:
-	mv88e6xxx_g1_vtu_prob_irq_free(chip);
+	if (chip->irq > 0)
+		mv88e6xxx_g1_vtu_prob_irq_free(chip);
Why not move this check to mv88e6xxx_g1_vtu_prob_irq_free() and make it
a no-op if chip->irq <= 0?
Hi Florian

It keeps it symmetrical with the setup code:

        if (chip->irq > 0) {
                /* Has to be performed before the MDIO bus is created,
                 * because the PHYs will link there interrupts to these
                 * interrupt controllers
                 */
                mutex_lock(&chip->reg_lock);
                err = mv88e6xxx_g1_irq_setup(chip);
                mutex_unlock(&chip->reg_lock);

                if (err)
                        goto out;

                if (chip->info->g2_irqs > 0) {
                        err = mv88e6xxx_g2_irq_setup(chip);
                        if (err)
                                goto out_g1_irq;
                }

                err = mv88e6xxx_g1_atu_prob_irq_setup(chip);
                if (err)
                        goto out_g2_irq;

                err = mv88e6xxx_g1_vtu_prob_irq_setup(chip);
                if (err)
                        goto out_g1_atu_prob_irq;
        } 

The general flow in probe() is to only call functions if they are
needed. So i would prefer to keep to that.

	Andrew

Re: [PATCH net-next 0/2] ATU and VTU irq fixes

From: David Miller <davem@davemloft.net>
Date: 2018-01-16 20:18:35

From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, 15 Jan 2018 23:45:56 +0100
Further testing and core review found two sets of bugs.

Core review found a cut/paste error in the irq setup code.

A board which does not have an interrupt line from the switch to the
SoC, and experiancing an EPROBE_DEFER throw a splat when the ATU irq
was freed but never registered.
Series applied, thanks Andrew.

Re: [PATCH net-next 0/2] ATU and VTU irq fixes

From: David Miller <davem@davemloft.net>
Date: 2018-01-16 20:23:07

From: David Miller <davem@davemloft.net>
Date: Tue, 16 Jan 2018 15:18:33 -0500 (EST)
From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, 15 Jan 2018 23:45:56 +0100
quoted
Further testing and core review found two sets of bugs.

Core review found a cut/paste error in the irq setup code.

A board which does not have an interrupt line from the switch to the
SoC, and experiancing an EPROBE_DEFER throw a splat when the ATU irq
was freed but never registered.
Series applied, thanks Andrew.
Uhhh, this breaks the build.  Reverting...

drivers/net/dsa/mv88e6xxx/global1_vtu.c: In function ‘mv88e6xxx_g1_vtu_prob_irq_setup’:
drivers/net/dsa/mv88e6xxx/global1_vtu.c:573:14: error: ‘struct mv88e6xxx_chip’ has no member named ‘chip’
   return chip->chip->vtu_prob_irq;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help