The software free queue interrupt is routed through port 1 even on boards which only connect a PHY to port 0. Since PHY setup was moved into probe, the PHY-less port fails to bind and releases the threaded handler needed by the shared queue. Publishing the port before PHY setup also leaves a dangling parent pointer when probe fails and devres frees the netdev private data. Opening the sibling can then dereference freed memory. Keep children bound as IRQ-only ports when no PHY is described or when PHY setup or netdev registration fails permanently, so an unusable interface does not disable its sibling. Disconnect any attached PHY after registration failure. Continue to propagate deferred-probe and mandatory resource errors. Resolve referenced PHYs explicitly and defer probe until they register. Preserve fixed-link registration and PHY connection errors, and release fixed links on probe failure or removal. This avoids permanently binding a port IRQ-only just because its MDIO bus has not probed yet. Publish a port to the parent only after its IRQ and optional netdev are ready, and protect port lifetime with RTNL. Refuse shared queue setup until both ports have completed initialization. Keep shared hardware initialization parent-owned across individual child removal so recreating one child cannot reset a live sibling. Mask firmware interrupt state before child IRQ handlers are registered, and use the allocated ring rather than an order sentinel to decide whether the queue still needs initial setup. Fixes: 3e813d61401a ("net: gemini: Clean up phy registration") Signed-off-by: Linus Walleij --- drivers/net/ethernet/cortina/gemini.c | 165 ++++++++++++++++++++++++++-------- 1 file changed, 129 insertions(+), 36 deletions(-) diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c index 0b05a1552983..1b959ca853c6 100644 --- a/drivers/net/ethernet/cortina/gemini.c +++ b/drivers/net/ethernet/cortina/gemini.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -155,6 +156,7 @@ struct gemini_ethernet_port { struct gemini_ethernet { struct device *dev; void __iomem *base; + /* Protected by RTNL. */ struct gemini_ethernet_port *port0; struct gemini_ethernet_port *port1; bool initialized; @@ -359,18 +361,51 @@ static void gmac_adjust_link(struct net_device *netdev) gmac_enable_tx_rx(netdev); } +static void gmac_deregister_fixed_link(void *data) +{ + of_phy_deregister_fixed_link(data); +} + static int gmac_setup_phy(struct net_device *netdev) { struct gemini_ethernet_port *port = netdev_priv(netdev); union gmac_status status = { .bits32 = 0 }; struct device *dev = port->dev; + struct device_node *phy_np; + phy_interface_t interface; struct phy_device *phy; + int ret; + + ret = of_get_phy_mode(dev->of_node, &interface); + if (ret) + return ret; + + if (of_phy_is_fixed_link(dev->of_node)) { + ret = of_phy_register_fixed_link(dev->of_node); + if (ret) + return ret; - phy = of_phy_get_and_connect(netdev, - dev->of_node, - gmac_adjust_link); + ret = devm_add_action_or_reset(dev, gmac_deregister_fixed_link, + dev->of_node); + if (ret) + return ret; + + phy_np = of_node_get(dev->of_node); + } else { + phy_np = of_parse_phandle(dev->of_node, "phy-handle", 0); + if (!phy_np) + return -ENODEV; + } + + phy = of_phy_find_device(phy_np); + of_node_put(phy_np); if (!phy) - return -ENODEV; + return -EPROBE_DEFER; + + ret = phy_connect_direct(netdev, phy, gmac_adjust_link, interface); + put_device(&phy->mdio.dev); + if (ret) + return ret; netdev->phydev = phy; phy_set_max_speed(phy, SPEED_1000); @@ -1051,10 +1086,17 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port) u32 en; int ret; + ASSERT_RTNL(); + + /* The software free queue interrupt is routed through port 1. */ + if (!geth->initialized || !geth->port0 || !geth->port1) + return -ENODEV; + if (netdev->dev_id == 0) - other_netdev = geth->port1->netdev; + other_port = geth->port1; else - other_netdev = geth->port0->netdev; + other_port = geth->port0; + other_netdev = other_port ? other_port->netdev : NULL; if (other_netdev && netif_running(other_netdev)) return -EBUSY; @@ -1065,7 +1107,6 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port) new_size, port->rxq_order); if (other_netdev) { - other_port = netdev_priv(other_netdev); new_size += 1 << (other_port->rxq_order + 1); netdev_dbg(other_netdev, "port %d size: %d order %d\n", other_netdev->dev_id, @@ -1076,7 +1117,7 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port) new_order = min(15, ilog2(new_size - 1) + 1); dev_dbg(geth->dev, "set shared queue to size %d order %d\n", new_size, new_order); - if (geth->freeq_order == new_order) + if (geth->freeq_ring && geth->freeq_order == new_order) return 0; spin_lock_irqsave(&geth->irq_lock, flags); @@ -2387,8 +2428,35 @@ static irqreturn_t gemini_port_irq(int irq, void *data) return ret; } +static void gemini_port_clear(struct gemini_ethernet_port *port) +{ + struct gemini_ethernet *geth = port->geth; + + ASSERT_RTNL(); + + if (!port->id && geth->port0 == port) + geth->port0 = NULL; + else if (port->id && geth->port1 == port) + geth->port1 = NULL; + else + return; +} + +static void gemini_ethernet_disable_irqs(struct gemini_ethernet *geth) +{ + writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_0_REG); + writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_1_REG); + writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_2_REG); + writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_3_REG); + writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG); +} + static void gemini_port_remove(struct gemini_ethernet_port *port) { + rtnl_lock(); + gemini_port_clear(port); + rtnl_unlock(); + if (port->netdev) { unregister_netdev(port->netdev); phy_disconnect(port->netdev->phydev); @@ -2398,19 +2466,15 @@ static void gemini_port_remove(struct gemini_ethernet_port *port) static void gemini_ethernet_init(struct gemini_ethernet *geth) { + ASSERT_RTNL(); + /* Only do this once both ports are online */ if (geth->initialized) return; - if (geth->port0 && geth->port1) - geth->initialized = true; - else + if (!geth->port0 || !geth->port1) return; - writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_0_REG); - writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_1_REG); - writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_2_REG); - writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_3_REG); - writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG); + gemini_ethernet_disable_irqs(geth); /* Interrupt config: * @@ -2442,10 +2506,20 @@ static void gemini_ethernet_init(struct gemini_ethernet *geth) writel(0, geth->base + GLOBAL_HWFQ_RWPTR_REG); geth->freeq_frag_order = DEFAULT_RX_BUF_ORDER; - /* This makes the queue resize on probe() so that we - * set up and enable the queue IRQ. FIXME: fragile. - */ - geth->freeq_order = 1; + geth->initialized = true; +} + +static void gemini_port_publish(struct gemini_ethernet_port *port) +{ + struct gemini_ethernet *geth = port->geth; + + ASSERT_RTNL(); + + if (!port->id) + geth->port0 = port; + else + geth->port1 = port; + gemini_ethernet_init(geth); } static void gemini_port_save_mac_addr(struct gemini_ethernet_port *port) @@ -2541,15 +2615,6 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev) reset_control_reset(port->reset); usleep_range(100, 500); - /* Assign pointer in the main state container */ - if (!id) - geth->port0 = port; - else - geth->port1 = port; - - /* This will just be done once both ports are up and reset */ - gemini_ethernet_init(geth); - platform_set_drvdata(pdev, port); /* Set up and register the netdev */ @@ -2600,17 +2665,39 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev) if (ret) goto unprepare; + if (!of_property_present(np, "phy-handle") && + !of_phy_is_fixed_link(np)) { + dev_info(dev, "no PHY\n"); + goto irq_only; + } + ret = gmac_setup_phy(netdev); if (ret) { - netdev_err(netdev, - "PHY init failed\n"); - goto unprepare; + dev_err_probe(dev, ret, "PHY init failed\n"); + if (ret == -EPROBE_DEFER) + goto unprepare; + goto irq_only; } - ret = register_netdev(netdev); - if (ret) - goto unprepare; + rtnl_lock(); + ret = register_netdevice(netdev); + if (!ret) + gemini_port_publish(port); + rtnl_unlock(); + if (ret) { + phy_disconnect(netdev->phydev); + dev_err(dev, "netdev registration failed: %d\n", ret); + goto irq_only; + } + + return 0; +irq_only: + dev_info(dev, "keeping port for shared IRQ\n"); + port->netdev = NULL; + rtnl_lock(); + gemini_port_publish(port); + rtnl_unlock(); return 0; unprepare: @@ -2706,6 +2793,9 @@ static int gemini_ethernet_probe(struct platform_device *pdev) spin_lock_init(&geth->irq_lock); spin_lock_init(&geth->freeq_lock); + /* Quiesce firmware state before child IRQ handlers are registered. */ + gemini_ethernet_disable_irqs(geth); + /* The children will use this */ platform_set_drvdata(pdev, geth); @@ -2717,10 +2807,13 @@ static void gemini_ethernet_remove(struct platform_device *pdev) { struct gemini_ethernet *geth = platform_get_drvdata(pdev); + rtnl_lock(); + geth->initialized = false; + rtnl_unlock(); + devm_of_platform_depopulate(&pdev->dev); writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG); geth_cleanup_freeq(geth); - geth->initialized = false; } static const struct of_device_id gemini_ethernet_of_match[] = { -- 2.56.0