From: Wei Fang <[email protected]> The NETC switch reached a known hardware state via a one-time PCIe FLR in probe(), and used devm_ allocations in netc_setup() for its ports, EMDIO buses, reference clocks, command BD rings and buffer pool table. Both are tied to the PCI device lifetime, not to the DSA netc_setup()/netc_teardown() lifetime.
In a multi-switch DSA tree, binding or unbinding any switch tears down and re-sets up the whole tree, so netc_teardown() and netc_setup() can run again without a PCI re-probe. Two problems then appear: - No fresh FLR occurs, so the hardware tables keep their stale entries while netc_setup() builds empty software state. Hardware and software drift out of sync. - The devm_ resources are not released by netc_teardown(), so each re-setup leaks memory and re-registers the same EMDIO bus, which fails. Add netc_switch_reset() and call it at the start of netc_setup() so every setup begins from a clean hardware state. Use pci_reset_function_locked() during probe(), where the device lock is already held, and pci_reset_function() otherwise; both reset the function and restore the PCI config space, unlike the bare pcie_flr() they replace. Convert the setup-path devm_ allocations to explicit alloc and free, and release them in netc_teardown() and on the error paths, so their lifetime matches the DSA setup and teardown. The NETC switch does not support multi-switch yet, so there is no functional problem today. This only reworks the driver so that multi-switch can be added later without hitting these issues. Signed-off-by: Wei Fang <[email protected]> --- drivers/net/dsa/netc/netc_main.c | 163 ++++++++++++++++++++++------- drivers/net/dsa/netc/netc_switch.h | 1 + 2 files changed, 129 insertions(+), 35 deletions(-) diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c index 77077352c1a5..c639ae67c9b7 100644 --- a/drivers/net/dsa/netc/netc_main.c +++ b/drivers/net/dsa/netc/netc_main.c @@ -135,7 +135,7 @@ static int netc_port_get_info_from_dt(struct netc_port *np, struct device *dev) { if (of_find_property(node, "clock-names", NULL)) { - np->ref_clk = devm_get_clk_from_child(dev, node, "ref"); + np->ref_clk = of_clk_get_by_name(node, "ref"); if (IS_ERR(np->ref_clk)) { dev_err(dev, "Port %d cannot get reference clock\n", np->dp->index); @@ -146,6 +146,24 @@ static int netc_port_get_info_from_dt(struct netc_port *np, return 0; } +static void netc_port_free_emdio_bus(struct netc_port *np) +{ + struct device *dev = np->switch_priv->dev; + struct enetc_mdio_priv *mdio_priv; + struct enetc_hw *hw; + + if (!np->emdio) + return; + + mdio_priv = np->emdio->priv; + hw = mdio_priv->hw; + + mdiobus_unregister(np->emdio); + mdiobus_free(np->emdio); + devm_kfree(dev, hw); + np->emdio = NULL; +} + static int netc_port_create_emdio_bus(struct netc_port *np, struct device_node *node) { @@ -161,9 +179,11 @@ static int netc_port_create_emdio_bus(struct netc_port *np, return dev_err_probe(dev, PTR_ERR(hw), "Failed to allocate enetc_hw\n"); - bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv)); - if (!bus) - return -ENOMEM; + bus = mdiobus_alloc_size(sizeof(*mdio_priv)); + if (!bus) { + err = -ENOMEM; + goto free_enetc_hw; + } bus->name = "NXP NETC switch external MDIO Bus"; bus->read = enetc_mdio_read_c22; @@ -177,14 +197,22 @@ static int netc_port_create_emdio_bus(struct netc_port *np, snprintf(bus->id, MII_BUS_ID_SIZE, "%s-p%d-emdio", dev_name(dev), np->dp->index); - err = devm_of_mdiobus_register(dev, bus, node); - if (err) - return dev_err_probe(dev, err, - "Cannot register EMDIO bus\n"); + err = of_mdiobus_register(bus, node); + if (err) { + dev_err(dev, "Cannot register EMDIO bus\n"); + goto free_mdiobus; + } np->emdio = bus; return 0; + +free_mdiobus: + mdiobus_free(bus); +free_enetc_hw: + devm_kfree(dev, hw); + + return err; } static int netc_port_create_mdio_bus(struct netc_port *np, @@ -238,6 +266,33 @@ static void netc_get_switch_capabilities(struct netc_switch *priv) priv->num_bp = FIELD_GET(BPCAPR_NUM_BP, val); } +static void netc_free_all_ports(struct netc_switch *priv) +{ + if (!priv->ports) + return; + + for (int i = 0; i < priv->info->num_ports; i++) { + struct netc_port *np = priv->ports[i]; + + if (!np) + continue; + + netc_port_free_emdio_bus(np); + + if (!IS_ERR_OR_NULL(np->ref_clk)) { + clk_put(np->ref_clk); + np->ref_clk = NULL; + } + + kfree(np); + priv->ports[i] = NULL; + } + + kfree(priv->ports); + priv->ports = NULL; + priv->port_bitmap = 0; +} + static int netc_init_all_ports(struct netc_switch *priv) { struct device *dev = priv->dev; @@ -246,9 +301,7 @@ static int netc_init_all_ports(struct netc_switch *priv) int ett_offset = 0; int err; - priv->ports = devm_kcalloc(dev, priv->info->num_ports, - sizeof(struct netc_port *), - GFP_KERNEL); + priv->ports = kzalloc_objs(*priv->ports, priv->info->num_ports); if (!priv->ports) return -ENOMEM; @@ -260,9 +313,11 @@ static int netc_init_all_ports(struct netc_switch *priv) * difficult for us to cover. */ for (int i = 0; i < priv->info->num_ports; i++) { - np = devm_kzalloc(dev, sizeof(*np), GFP_KERNEL); - if (!np) - return -ENOMEM; + np = kzalloc_obj(*np); + if (!np) { + err = -ENOMEM; + goto free_all_ports; + } np->switch_priv = priv; np->iobase = priv->regs.port + PORT_IOBASE(i); @@ -278,18 +333,23 @@ static int netc_init_all_ports(struct netc_switch *priv) err = netc_port_get_info_from_dt(np, dp->dn, dev); if (err) - return err; + goto free_all_ports; if (dsa_port_is_user(dp)) { err = netc_port_create_mdio_bus(np, dp->dn); if (err) { dev_err(dev, "Failed to create MDIO bus\n"); - return err; + goto free_all_ports; } } } return 0; + +free_all_ports: + netc_free_all_ports(priv); + + return err; } static void netc_init_ntmp_tbl_versions(struct netc_switch *priv) @@ -308,9 +368,7 @@ static int netc_init_all_cbdrs(struct netc_switch *priv) ntmp->cbdr_num = NETC_CBDR_NUM; ntmp->dev = priv->dev; - ntmp->ring = devm_kcalloc(ntmp->dev, ntmp->cbdr_num, - sizeof(struct netc_cbdr), - GFP_KERNEL); + ntmp->ring = kzalloc_objs(*ntmp->ring, ntmp->cbdr_num); if (!ntmp->ring) return -ENOMEM; @@ -336,6 +394,9 @@ static int netc_init_all_cbdrs(struct netc_switch *priv) for (i--; i >= 0; i--) ntmp_free_cbdr(&ntmp->ring[i]); + kfree(ntmp->ring); + ntmp->ring = NULL; + return err; } @@ -345,6 +406,9 @@ static void netc_remove_all_cbdrs(struct netc_switch *priv) for (int i = 0; i < NETC_CBDR_NUM; i++) ntmp_free_cbdr(&ntmp->ring[i]); + + kfree(ntmp->ring); + ntmp->ring = NULL; } static u32 netc_num_available_ports(struct netc_switch *priv) @@ -842,31 +906,49 @@ static void netc_ipv_to_buffer_pool_mapping(struct netc_switch *priv) } } +static void netc_switch_free_bpt_list(struct netc_switch *priv) +{ + kfree(priv->bpt_list); + priv->bpt_list = NULL; +} + static int netc_switch_bpt_default_config(struct netc_switch *priv) { + int err; + if (priv->num_bp < priv->info->num_ports) return -EINVAL; - priv->bpt_list = devm_kcalloc(priv->dev, priv->num_bp, - sizeof(struct bpt_cfge_data), - GFP_KERNEL); + priv->bpt_list = kzalloc_objs(*priv->bpt_list, priv->num_bp); if (!priv->bpt_list) return -ENOMEM; /* Initialize the maximum threshold of each buffer pool entry */ for (int i = 0; i < priv->num_bp; i++) { struct bpt_cfge_data *cfge = &priv->bpt_list[i]; - int err; cfge->max_thresh = cpu_to_le16(NETC_BP_THRESH); err = ntmp_bpt_update_entry(&priv->ntmp, i, cfge); if (err) - return err; + goto free_bpt_list; } netc_ipv_to_buffer_pool_mapping(priv); return 0; + +free_bpt_list: + netc_switch_free_bpt_list(priv); + + return err; +} + +static int netc_switch_reset(struct netc_switch *priv) +{ + if (priv->in_probe) + return pci_reset_function_locked(priv->pdev); + + return pci_reset_function(priv->pdev); } static int netc_setup(struct dsa_switch *ds) @@ -875,6 +957,10 @@ static int netc_setup(struct dsa_switch *ds) struct dsa_port *dp; int err; + err = netc_switch_reset(priv); + if (err) + return err; + err = netc_init_switch_id(priv); if (err) return err; @@ -887,7 +973,7 @@ static int netc_setup(struct dsa_switch *ds) err = netc_init_ntmp_user(priv); if (err) - return err; + goto free_all_ports; INIT_HLIST_HEAD(&priv->fdb_list); mutex_init(&priv->fdbt_lock); @@ -910,22 +996,25 @@ static int netc_setup(struct dsa_switch *ds) err = netc_add_standalone_vlan_entry(priv); if (err) - goto free_lock_and_ntmp_user; + goto free_bpt_list; err = netc_add_standalone_fdb_bcast_entry(priv); if (err) - goto free_lock_and_ntmp_user; + goto free_bpt_list; return 0; +free_bpt_list: + netc_switch_free_bpt_list(priv); free_lock_and_ntmp_user: - /* No need to clear the hardware state, netc_setup() is only called - * when the driver is bound, and FLR will be performed to reset the - * hardware state. + /* No need to clear the hardware state. netc_setup() resets the + * hardware via FLR, so the next setup starts from a clean state. */ mutex_destroy(&priv->fdbt_lock); mutex_destroy(&priv->vft_lock); netc_free_ntmp_user(priv); +free_all_ports: + netc_free_all_ports(priv); return err; } @@ -945,10 +1034,10 @@ static void netc_free_host_flood_rules(struct netc_switch *priv) dsa_switch_for_each_user_port(dp, priv->ds) { struct netc_port *np = priv->ports[dp->index]; - /* No need to clear the hardware IPFT entry. Because PCIe - * FLR will be performed when the switch is re-registered, - * it will reset hardware state. So only need to free the - * memory to avoid memory leak. + /* No need to clear the hardware IPFT entry. netc_setup() + * resets the hardware via FLR, so the next setup starts + * from a clean state. Only free the memory here to avoid + * a leak. */ kfree(np->host_flood); np->host_flood = NULL; @@ -963,6 +1052,8 @@ static void netc_teardown(struct dsa_switch *ds) netc_destroy_all_lists(priv); netc_free_host_flood_rules(priv); netc_free_ntmp_user(priv); + netc_switch_free_bpt_list(priv); + netc_free_all_ports(priv); } static bool netc_port_is_emdio_consumer(struct device_node *node) @@ -1045,7 +1136,6 @@ static int netc_switch_pci_init(struct pci_dev *pdev) void __iomem *base; int err; - pcie_flr(pdev); err = pcim_enable_device(pdev); if (err) return dev_err_probe(dev, err, "Failed to enable device\n"); @@ -2475,12 +2565,15 @@ static int netc_switch_probe(struct pci_dev *pdev, ds->ageing_time_max = U32_MAX; ds->priv = priv; priv->ds = ds; + priv->in_probe = true; err = dsa_register_switch(ds); if (err) return dev_err_probe(dev, err, "Failed to register DSA switch\n"); + priv->in_probe = false; + return 0; } diff --git a/drivers/net/dsa/netc/netc_switch.h b/drivers/net/dsa/netc/netc_switch.h index 305f2a92e2f9..4a8c5f9cde38 100644 --- a/drivers/net/dsa/netc/netc_switch.h +++ b/drivers/net/dsa/netc/netc_switch.h @@ -118,6 +118,7 @@ struct netc_switch { struct device *dev; struct dsa_switch *ds; u16 revision; + bool in_probe; const struct netc_switch_info *info; struct netc_switch_regs regs; -- 2.34.1
