Skip to content

Commit a58d882

Browse files
AgentDkuba-moo
authored andcommitted
net: dsa: mv88e6xxx: propperly shutdown PPU re-enable timer on destroy
The mv88e6xxx has an internal PPU that polls PHY state. If we want to access the internal PHYs, we need to disable the PPU first. Because that is a slow operation, a 10ms timer is used to re-enable it, canceled with every access, so bulk operations effectively only disable it once and re-enable it some 10ms after the last access. If a PHY is accessed and then the mv88e6xxx module is removed before the 10ms are up, the PPU re-enable ends up accessing a dangling pointer. This especially affects probing during bootup. The MDIO bus and PHY registration may succeed, but registration with the DSA framework may fail later on (e.g. because the CPU port depends on another, very slow device that isn't done probing yet, returning -EPROBE_DEFER). In this case, probe() fails, but the MDIO subsystem may already have accessed the MIDO bus or PHYs, arming the timer. This is fixed as follows: - If probe fails after mv88e6xxx_phy_init(), make sure we also call mv88e6xxx_phy_destroy() before returning - In mv88e6xxx_remove(), make sure we do the teardown in the correct order, calling mv88e6xxx_phy_destroy() after unregistering the switch device. - In mv88e6xxx_phy_destroy(), destroy both the timer and the work item that the timer might schedule, synchronously waiting in case one of the callbacks already fired and destroying the timer first, before waiting for the work item. - Access to the PPU is guarded by a mutex, the worker acquires it with a mutex_trylock(), not proceeding with the expensive shutdown if that fails. We grab the mutex in mv88e6xxx_phy_destroy() to make sure the slow PPU shutdown is already done or won't even enter, when we wait for the work item. Fixes: 2e5f032 ("dsa: add support for the Marvell 88E6131 switch chip") Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at> Reviewed-by: Vladimir Oltean <olteanv@gmail.com> Link: https://patch.msgid.link/20250401135705.92760-1-david.oberhollenzer@sigma-star.at Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 40eb4a0 commit a58d882

File tree

2 files changed

+10
-4
lines changed

2 files changed

+10
-4
lines changed

drivers/net/dsa/mv88e6xxx/chip.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7350,13 +7350,13 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
73507350
err = mv88e6xxx_switch_reset(chip);
73517351
mv88e6xxx_reg_unlock(chip);
73527352
if (err)
7353-
goto out;
7353+
goto out_phy;
73547354

73557355
if (np) {
73567356
chip->irq = of_irq_get(np, 0);
73577357
if (chip->irq == -EPROBE_DEFER) {
73587358
err = chip->irq;
7359-
goto out;
7359+
goto out_phy;
73607360
}
73617361
}
73627362

@@ -7375,7 +7375,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
73757375
mv88e6xxx_reg_unlock(chip);
73767376

73777377
if (err)
7378-
goto out;
7378+
goto out_phy;
73797379

73807380
if (chip->info->g2_irqs > 0) {
73817381
err = mv88e6xxx_g2_irq_setup(chip);
@@ -7409,6 +7409,8 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
74097409
mv88e6xxx_g1_irq_free(chip);
74107410
else
74117411
mv88e6xxx_irq_poll_free(chip);
7412+
out_phy:
7413+
mv88e6xxx_phy_destroy(chip);
74127414
out:
74137415
if (pdata)
74147416
dev_put(pdata->netdev);
@@ -7431,7 +7433,6 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
74317433
mv88e6xxx_ptp_free(chip);
74327434
}
74337435

7434-
mv88e6xxx_phy_destroy(chip);
74357436
mv88e6xxx_unregister_switch(chip);
74367437

74377438
mv88e6xxx_g1_vtu_prob_irq_free(chip);
@@ -7444,6 +7445,8 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
74447445
mv88e6xxx_g1_irq_free(chip);
74457446
else
74467447
mv88e6xxx_irq_poll_free(chip);
7448+
7449+
mv88e6xxx_phy_destroy(chip);
74477450
}
74487451

74497452
static void mv88e6xxx_shutdown(struct mdio_device *mdiodev)

drivers/net/dsa/mv88e6xxx/phy.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,10 @@ static void mv88e6xxx_phy_ppu_state_init(struct mv88e6xxx_chip *chip)
229229

230230
static void mv88e6xxx_phy_ppu_state_destroy(struct mv88e6xxx_chip *chip)
231231
{
232+
mutex_lock(&chip->ppu_mutex);
232233
del_timer_sync(&chip->ppu_timer);
234+
cancel_work_sync(&chip->ppu_work);
235+
mutex_unlock(&chip->ppu_mutex);
233236
}
234237

235238
int mv88e6185_phy_ppu_read(struct mv88e6xxx_chip *chip, struct mii_bus *bus,

0 commit comments

Comments
 (0)