From a3db3d3a52f0f159e0aa996792987acd8c14d2ad Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Sun, 20 Nov 2016 20:14:14 +0100 Subject: [PATCH 1/6] net: dsa: mv88e6xxx: Fix typos when removing g1 interrupts Simple typos, s/g2/g1/ Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/dsa/mv88e6xxx/chip.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index d6d9d66b81ce5..6aa81d2d8f63b 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -415,11 +415,11 @@ static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip) int irq, virq; for (irq = 0; irq < 16; irq++) { - virq = irq_find_mapping(chip->g2_irq.domain, irq); + virq = irq_find_mapping(chip->g1_irq.domain, irq); irq_dispose_mapping(virq); } - irq_domain_remove(chip->g2_irq.domain); + irq_domain_remove(chip->g1_irq.domain); } static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip) From 46712644d6e72d8a66f20f51e260e1c3d1eddfeb Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Sun, 20 Nov 2016 20:14:15 +0100 Subject: [PATCH 2/6] net: dsa: mv88e6xxx: Fix unconditional irq freeing Trying to remove an IRQ domain that was not created results in an Opps. Add the necessary checks that the irqs were created before freeing them. Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/dsa/mv88e6xxx/chip.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 6aa81d2d8f63b..b843052d32bd6 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -3897,10 +3897,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev) out_mdio: mv88e6xxx_mdio_unregister(chip); out_g2_irq: - if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT)) + if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT) && chip->irq > 0) mv88e6xxx_g2_irq_free(chip); out_g1_irq: - mv88e6xxx_g1_irq_free(chip); + if (chip->irq > 0) + mv88e6xxx_g1_irq_free(chip); out: return err; } @@ -3914,9 +3915,11 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev) mv88e6xxx_unregister_switch(chip); mv88e6xxx_mdio_unregister(chip); - if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT)) - mv88e6xxx_g2_irq_free(chip); - mv88e6xxx_g1_irq_free(chip); + if (chip->irq > 0) { + if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT)) + mv88e6xxx_g2_irq_free(chip); + mv88e6xxx_g1_irq_free(chip); + } } static const struct of_device_id mv88e6xxx_of_match[] = { From 3460a5770ce97d7caeb4b11766c7bb2d74d0d29c Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Sun, 20 Nov 2016 20:14:16 +0100 Subject: [PATCH 3/6] net: dsa: mv88e6xxx: Mask g1 interrupts and free interrupt Fix the g1 interrupt free code such that is masks any further interrupts, and then releases the interrupt. Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/dsa/mv88e6xxx/chip.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index b843052d32bd6..8fcef7e0d3ba1 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -413,6 +413,13 @@ static const struct irq_domain_ops mv88e6xxx_g1_irq_domain_ops = { static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip) { int irq, virq; + u16 mask; + + mv88e6xxx_g1_read(chip, GLOBAL_CONTROL, &mask); + mask |= GENMASK(chip->g1_irq.nirqs, 0); + mv88e6xxx_g1_write(chip, GLOBAL_CONTROL, mask); + + free_irq(chip->irq, chip); for (irq = 0; irq < 16; irq++) { virq = irq_find_mapping(chip->g1_irq.domain, irq); From 3dd0ef05f74693e55549ed790e350af5a392234f Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Sun, 20 Nov 2016 20:14:17 +0100 Subject: [PATCH 4/6] net: dsa: mv88e6xxx: Fix cleanup on error for g1 interrupt setup On error, remask the interrupts, release all maps, and remove the domain. This cannot be done using the mv88e6xxx_g1_irq_free() because some of these actions are not idempotent. Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/dsa/mv88e6xxx/chip.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 8fcef7e0d3ba1..614b2f68d401f 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -431,8 +431,8 @@ static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip) static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip) { - int err, irq; - u16 reg; + int err, irq, virq; + u16 reg, mask; chip->g1_irq.nirqs = chip->info->g1_irqs; chip->g1_irq.domain = irq_domain_add_simple( @@ -447,32 +447,41 @@ static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip) chip->g1_irq.chip = mv88e6xxx_g1_irq_chip; chip->g1_irq.masked = ~0; - err = mv88e6xxx_g1_read(chip, GLOBAL_CONTROL, ®); + err = mv88e6xxx_g1_read(chip, GLOBAL_CONTROL, &mask); if (err) - goto out; + goto out_mapping; - reg &= ~GENMASK(chip->g1_irq.nirqs, 0); + mask &= ~GENMASK(chip->g1_irq.nirqs, 0); - err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL, reg); + err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL, mask); if (err) - goto out; + goto out_disable; /* Reading the interrupt status clears (most of) them */ err = mv88e6xxx_g1_read(chip, GLOBAL_STATUS, ®); if (err) - goto out; + goto out_disable; err = request_threaded_irq(chip->irq, NULL, mv88e6xxx_g1_irq_thread_fn, IRQF_ONESHOT | IRQF_TRIGGER_FALLING, dev_name(chip->dev), chip); if (err) - goto out; + goto out_disable; return 0; -out: - mv88e6xxx_g1_irq_free(chip); +out_disable: + mask |= GENMASK(chip->g1_irq.nirqs, 0); + mv88e6xxx_g1_write(chip, GLOBAL_CONTROL, mask); + +out_mapping: + for (irq = 0; irq < 16; irq++) { + virq = irq_find_mapping(chip->g1_irq.domain, irq); + irq_dispose_mapping(virq); + } + + irq_domain_remove(chip->g1_irq.domain); return err; } From 8e757eba074fff8460dc91717695e009b5f93f76 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Sun, 20 Nov 2016 20:14:18 +0100 Subject: [PATCH 5/6] net: dsa: mv88e6xxx: Fix releasing for the global2 interrupts It is not possible to use devm_request_threaded_irq() because we have two stacked interrupt controllers in one device. The lower interrupt controller cannot be removed until the upper is fully removed. This happens too late with the devm API, resulting in error messages about removing a domain while there is still an active interrupt. Swap to using request_threaded_irq() and manage the release of the interrupt manually. Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/dsa/mv88e6xxx/global2.c | 28 +++++++++++++++++---------- drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 + 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c index 1a0b13521d137..536a27c9735fd 100644 --- a/drivers/net/dsa/mv88e6xxx/global2.c +++ b/drivers/net/dsa/mv88e6xxx/global2.c @@ -507,6 +507,9 @@ void mv88e6xxx_g2_irq_free(struct mv88e6xxx_chip *chip) { int irq, virq; + free_irq(chip->device_irq, chip); + irq_dispose_mapping(chip->device_irq); + for (irq = 0; irq < 16; irq++) { virq = irq_find_mapping(chip->g2_irq.domain, irq); irq_dispose_mapping(virq); @@ -517,8 +520,7 @@ void mv88e6xxx_g2_irq_free(struct mv88e6xxx_chip *chip) int mv88e6xxx_g2_irq_setup(struct mv88e6xxx_chip *chip) { - int device_irq; - int err, irq; + int err, irq, virq; if (!chip->dev->of_node) return -EINVAL; @@ -534,22 +536,28 @@ int mv88e6xxx_g2_irq_setup(struct mv88e6xxx_chip *chip) chip->g2_irq.chip = mv88e6xxx_g2_irq_chip; chip->g2_irq.masked = ~0; - device_irq = irq_find_mapping(chip->g1_irq.domain, - GLOBAL_STATUS_IRQ_DEVICE); - if (device_irq < 0) { - err = device_irq; + chip->device_irq = irq_find_mapping(chip->g1_irq.domain, + GLOBAL_STATUS_IRQ_DEVICE); + if (chip->device_irq < 0) { + err = chip->device_irq; goto out; } - err = devm_request_threaded_irq(chip->dev, device_irq, NULL, - mv88e6xxx_g2_irq_thread_fn, - IRQF_ONESHOT, "mv88e6xxx-g1", chip); + err = request_threaded_irq(chip->device_irq, NULL, + mv88e6xxx_g2_irq_thread_fn, + IRQF_ONESHOT, "mv88e6xxx-g1", chip); if (err) goto out; return 0; + out: - mv88e6xxx_g2_irq_free(chip); + for (irq = 0; irq < 16; irq++) { + virq = irq_find_mapping(chip->g2_irq.domain, irq); + irq_dispose_mapping(virq); + } + + irq_domain_remove(chip->g2_irq.domain); return err; } diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h index 929613021eff7..a3869504f8817 100644 --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h @@ -714,6 +714,7 @@ struct mv88e6xxx_chip { struct mv88e6xxx_irq g1_irq; struct mv88e6xxx_irq g2_irq; int irq; + int device_irq; }; struct mv88e6xxx_bus_ops { From 61f7c3f8032c840da4fb6b8a8714cb31e0b8ace8 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Sun, 20 Nov 2016 20:14:19 +0100 Subject: [PATCH 6/6] net: dsa: mv88e6xxx: Hold the mutex while freeing g1 interrupts Freeing interrupts requires switch register access to mask the interrupts. Hence we must hold the register mutex. Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/dsa/mv88e6xxx/chip.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 614b2f68d401f..e30d0eaf2b5f1 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -3916,8 +3916,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev) if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT) && chip->irq > 0) mv88e6xxx_g2_irq_free(chip); out_g1_irq: - if (chip->irq > 0) + if (chip->irq > 0) { + mutex_lock(&chip->reg_lock); mv88e6xxx_g1_irq_free(chip); + mutex_unlock(&chip->reg_lock); + } out: return err; }