mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
Merge branch 'net-phy-fix-cleanup-after-probe-failure'
Xuanqiang Luo says: ==================== net: phy: fix cleanup after probe failure phy_probe() initializes the PHY driver, ports, SFP upstream, and LEDs in stages. Its error paths do not always release only the resources acquired at each stage. It can also mark the PHY ready before all setup succeeds. Port setup also leaves SFP cleanup split between phy_sfp_probe(), phy_setup_ports(), and phy_probe(), and default port setup ignores errors from attaching the port to the PHY driver. This series makes each initialization layer own its cleanup and propagates setup failures to the caller. Patch 1 splits the phy_probe() cleanup by initialization stage. Patch 2 makes SFP and port setup unwind their resources in the required order. Patch 3 sets PHY_READY only after LED setup succeeds. Patch 4 calls the PHY driver remove callback after later probe failures. Patch 5 propagates errors from default port setup. Patch 6 prevents a double-free after partial LED trigger registration. ==================== Link: https://patch.msgid.link/20260823035600.188864-1-xuanqiang.luo@linux.dev Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski
commit
1bb784eb6e
|
|
@ -1723,12 +1723,41 @@ static int phy_sfp_probe(struct phy_device *phydev)
|
|||
phydev->sfp_bus = NULL;
|
||||
}
|
||||
|
||||
if (!ret && phydev->sfp_bus)
|
||||
if (!ret && phydev->sfp_bus) {
|
||||
ret = phy_setup_sfp_port(phydev);
|
||||
if (ret) {
|
||||
sfp_bus_del_upstream(phydev->sfp_bus);
|
||||
phydev->sfp_bus = NULL;
|
||||
}
|
||||
}
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
/**
|
||||
* phy_sfp_release - release resources set up by phy_sfp_probe()
|
||||
* @phydev: the PHY device
|
||||
*
|
||||
* Release the SFP resources set up by a successful phy_sfp_probe(). Unregister
|
||||
* the upstream before destroying its phy_port, so SFP upstream callbacks cannot
|
||||
* race with port destruction.
|
||||
*/
|
||||
static void phy_sfp_release(struct phy_device *phydev)
|
||||
{
|
||||
struct phy_port *port, *tmp;
|
||||
|
||||
sfp_bus_del_upstream(phydev->sfp_bus);
|
||||
phydev->sfp_bus = NULL;
|
||||
|
||||
list_for_each_entry_safe(port, tmp, &phydev->ports, head) {
|
||||
if (!port->is_sfp)
|
||||
continue;
|
||||
|
||||
phy_del_port(phydev, port);
|
||||
phy_port_destroy(port);
|
||||
}
|
||||
}
|
||||
|
||||
static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
|
||||
{
|
||||
return phydrv->config_intr && phydrv->handle_interrupt;
|
||||
|
|
@ -3454,6 +3483,7 @@ static int phy_default_setup_single_port(struct phy_device *phydev)
|
|||
{
|
||||
struct phy_port *port = phy_port_alloc();
|
||||
unsigned long mode;
|
||||
int ret;
|
||||
|
||||
if (!port)
|
||||
return -ENOMEM;
|
||||
|
|
@ -3480,9 +3510,11 @@ static int phy_default_setup_single_port(struct phy_device *phydev)
|
|||
port->pairs = max_t(int, port->pairs,
|
||||
ethtool_linkmode_n_pairs(mode));
|
||||
|
||||
phy_add_port(phydev, port);
|
||||
ret = phy_add_port(phydev, port);
|
||||
if (ret)
|
||||
phy_port_destroy(port);
|
||||
|
||||
return 0;
|
||||
return ret;
|
||||
}
|
||||
|
||||
static int of_phy_ports(struct phy_device *phydev)
|
||||
|
|
@ -3547,13 +3579,13 @@ static int phy_setup_ports(struct phy_device *phydev)
|
|||
if (!phydev->is_genphy_driven) {
|
||||
ret = phy_sfp_probe(phydev);
|
||||
if (ret)
|
||||
goto out;
|
||||
goto err_ports;
|
||||
}
|
||||
|
||||
if (phydev->n_ports < phydev->max_n_ports) {
|
||||
ret = phy_default_setup_single_port(phydev);
|
||||
if (ret)
|
||||
goto out;
|
||||
goto err_sfp;
|
||||
}
|
||||
|
||||
linkmode_zero(ports_supported);
|
||||
|
|
@ -3580,7 +3612,9 @@ static int phy_setup_ports(struct phy_device *phydev)
|
|||
|
||||
return 0;
|
||||
|
||||
out:
|
||||
err_sfp:
|
||||
phy_sfp_release(phydev);
|
||||
err_ports:
|
||||
phy_cleanup_ports(phydev);
|
||||
return ret;
|
||||
}
|
||||
|
|
@ -3706,7 +3740,7 @@ static int phy_probe(struct device *dev)
|
|||
if (phydev->drv->probe) {
|
||||
err = phydev->drv->probe(phydev);
|
||||
if (err)
|
||||
goto out;
|
||||
goto out_reset;
|
||||
}
|
||||
|
||||
phy_disable_interrupts(phydev);
|
||||
|
|
@ -3727,7 +3761,7 @@ static int phy_probe(struct device *dev)
|
|||
err = genphy_read_abilities(phydev);
|
||||
|
||||
if (err)
|
||||
goto out;
|
||||
goto out_remove;
|
||||
|
||||
if (!linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
|
||||
phydev->supported))
|
||||
|
|
@ -3744,7 +3778,7 @@ static int phy_probe(struct device *dev)
|
|||
|
||||
err = phy_setup_ports(phydev);
|
||||
if (err)
|
||||
goto out;
|
||||
goto out_remove;
|
||||
|
||||
phy_advertise_supported(phydev);
|
||||
|
||||
|
|
@ -3753,7 +3787,7 @@ static int phy_probe(struct device *dev)
|
|||
*/
|
||||
err = genphy_c45_read_eee_adv(phydev, phydev->advertising_eee);
|
||||
if (err)
|
||||
goto out;
|
||||
goto out_sfp_release;
|
||||
|
||||
/* Get the EEE modes we want to prohibit. */
|
||||
of_set_phy_eee_broken(phydev);
|
||||
|
|
@ -3793,9 +3827,6 @@ static int phy_probe(struct device *dev)
|
|||
phydev->supported);
|
||||
}
|
||||
|
||||
/* Set the state to READY by default */
|
||||
phydev->state = PHY_READY;
|
||||
|
||||
/* Register the PHY LED triggers */
|
||||
if (!phydev->is_on_sfp_module)
|
||||
phy_led_triggers_register(phydev);
|
||||
|
|
@ -3806,20 +3837,27 @@ static int phy_probe(struct device *dev)
|
|||
if (IS_ENABLED(CONFIG_PHYLIB_LEDS) && !phy_driver_is_genphy(phydev)) {
|
||||
err = of_phy_leds(phydev);
|
||||
if (err)
|
||||
goto out;
|
||||
goto out_unreg_led_triggers;
|
||||
}
|
||||
|
||||
/* Set the state to READY by default */
|
||||
phydev->state = PHY_READY;
|
||||
|
||||
return 0;
|
||||
|
||||
out:
|
||||
sfp_bus_del_upstream(phydev->sfp_bus);
|
||||
phydev->sfp_bus = NULL;
|
||||
|
||||
phy_cleanup_ports(phydev);
|
||||
|
||||
out_unreg_led_triggers:
|
||||
if (!phydev->is_on_sfp_module)
|
||||
phy_led_triggers_unregister(phydev);
|
||||
|
||||
out_sfp_release:
|
||||
phy_sfp_release(phydev);
|
||||
phy_cleanup_ports(phydev);
|
||||
|
||||
out_remove:
|
||||
if (phydev->drv->remove)
|
||||
phydev->drv->remove(phydev);
|
||||
|
||||
out_reset:
|
||||
/* Re-assert the reset signal on error */
|
||||
phy_device_reset(phydev, 1);
|
||||
|
||||
|
|
@ -3840,9 +3878,7 @@ static int phy_remove(struct device *dev)
|
|||
|
||||
phydev->state = PHY_DOWN;
|
||||
|
||||
sfp_bus_del_upstream(phydev->sfp_bus);
|
||||
phydev->sfp_bus = NULL;
|
||||
|
||||
phy_sfp_release(phydev);
|
||||
phy_cleanup_ports(phydev);
|
||||
|
||||
if (phydev->drv && phydev->drv->remove)
|
||||
|
|
|
|||
|
|
@ -126,6 +126,7 @@ int phy_led_triggers_register(struct phy_device *phy)
|
|||
while (i--)
|
||||
phy_led_trigger_unregister(&phy->phy_led_triggers[i]);
|
||||
kfree(phy->phy_led_triggers);
|
||||
phy->phy_led_triggers = NULL;
|
||||
out_unreg_link:
|
||||
phy_led_trigger_unregister(phy->led_link_trigger);
|
||||
out_free_link:
|
||||
|
|
@ -141,10 +142,12 @@ void phy_led_triggers_unregister(struct phy_device *phy)
|
|||
{
|
||||
int i;
|
||||
|
||||
for (i = 0; i < phy->phy_num_led_triggers; i++)
|
||||
phy_led_trigger_unregister(&phy->phy_led_triggers[i]);
|
||||
kfree(phy->phy_led_triggers);
|
||||
phy->phy_led_triggers = NULL;
|
||||
if (phy->phy_led_triggers) {
|
||||
for (i = 0; i < phy->phy_num_led_triggers; i++)
|
||||
phy_led_trigger_unregister(&phy->phy_led_triggers[i]);
|
||||
kfree(phy->phy_led_triggers);
|
||||
phy->phy_led_triggers = NULL;
|
||||
}
|
||||
|
||||
if (phy->led_link_trigger) {
|
||||
phy_led_trigger_unregister(phy->led_link_trigger);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user