[PATCH net-next 2/4] net: ethernet: ti: am65-cpsw: streamline .probe() error handling

Roger Quadros
2025-01-15T16:43:16Z

Keep things simple by explicitly cleaning up on .probe() error
path or .remove(). Get rid of devm_add/remove_action() usage.

Rename am65_cpsw_disable_serdes_phy() to
am65_cpsw_nuss_cleanup_slave_ports() and move it right before
am65_cpsw_nuss_init_slave_ports().

Get rid of am65_cpsw_nuss_phylink_cleanup() and introduce
am65_cpsw_nuss_cleanup_ndevs() right before am65_cpsw_nuss_init_ndevs()

Move channel initiailzation code out of am65_cpsw_nuss_register_ndevs()
into new function am65_cpsw_nuss_init_chns().
Add am65_cpsw_nuss_remove_chns() to do reverse of
am65_cpsw_nuss_init_chns().

Add am65_cpsw_nuss_unregister_ndev() to do reverse of
am65_cpsw_nuss_register_ndevs().

Use the introduced helpers in probe/remove.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 276 ++++++++++++++-----------------
1 file changed, 126 insertions(+), 150 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 36c29d3db329..783ec461dbdc 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2056,20 +2056,6 @@ static int am65_cpsw_enable_phy(struct phy *phy)
return 0;
}

-static void am65_cpsw_disable_serdes_phy(struct am65_cpsw_common *common)
-{
- struct am65_cpsw_port *port;
- struct phy *phy;
- int i;
-
- for (i = 0; i < common->port_num; i++) {
- port = &common->ports[i];
- phy = port->slave.serdes_phy;
- if (phy)
- am65_cpsw_disable_phy(phy);
- }
-}
-
static int am65_cpsw_init_serdes_phy(struct device *dev, struct device_node *port_np,
struct am65_cpsw_port *port)
{
@@ -2222,14 +2208,20 @@ static void am65_cpsw_nuss_slave_disable_unused(struct am65_cpsw_port *port)
cpsw_sl_ctl_reset(port->slave.mac_sl);
}

-static void am65_cpsw_nuss_free_tx_chns(void *data)
+static void am65_cpsw_nuss_cleanup_tx_chns(struct am65_cpsw_common *common)
{
- struct am65_cpsw_common *common = data;
+ struct device *dev = common->dev;
int i;

+ common->tx_ch_rate_msk = 0;
for (i = 0; i < common->tx_ch_num; i++) {
struct am65_cpsw_tx_chn *tx_chn = &common->tx_chns[i];

+ if (tx_chn->irq)
+ devm_free_irq(dev, tx_chn->irq, tx_chn);
+
+ netif_napi_del(&tx_chn->napi_tx);
+
if (!IS_ERR_OR_NULL(tx_chn->desc_pool))
k3_cppi_desc_pool_destroy(tx_chn->desc_pool);

@@ -2240,26 +2232,6 @@ static void am65_cpsw_nuss_free_tx_chns(void *data)
}
}

-static void am65_cpsw_nuss_remove_tx_chns(struct am65_cpsw_common *common)
-{
- struct device *dev = common->dev;
- int i;
-
- devm_remove_action(dev, am65_cpsw_nuss_free_tx_chns, common);
-
- common->tx_ch_rate_msk = 0;
- for (i = 0; i < common->tx_ch_num; i++) {
- struct am65_cpsw_tx_chn *tx_chn = &common->tx_chns[i];
-
- if (tx_chn->irq)
- devm_free_irq(dev, tx_chn->irq, tx_chn);
-
- netif_napi_del(&tx_chn->napi_tx);
- }
-
- am65_cpsw_nuss_free_tx_chns(common);
-}
-
static int am65_cpsw_nuss_ndev_add_tx_napi(struct am65_cpsw_common *common)
{
struct device *dev = common->dev;
@@ -2360,36 +2332,14 @@ static int am65_cpsw_nuss_init_tx_chns(struct am65_cpsw_common *common)
}

ret = am65_cpsw_nuss_ndev_add_tx_napi(common);
- if (ret) {
+ if (ret)
dev_err(dev, "Failed to add tx NAPI %d\n", ret);
- goto err;
- }

err:
- i = devm_add_action(dev, am65_cpsw_nuss_free_tx_chns, common);
- if (i) {
- dev_err(dev, "Failed to add free_tx_chns action %d\n", i);
- return i;
- }
-
return ret;
}

-static void am65_cpsw_nuss_free_rx_chns(void *data)
-{
- struct am65_cpsw_common *common = data;
- struct am65_cpsw_rx_chn *rx_chn;
-
- rx_chn = &common->rx_chns;
-
- if (!IS_ERR_OR_NULL(rx_chn->desc_pool))
- k3_cppi_desc_pool_destroy(rx_chn->desc_pool);
-
- if (!IS_ERR_OR_NULL(rx_chn->rx_chn))
- k3_udma_glue_release_rx_chn(rx_chn->rx_chn);
-}
-
-static void am65_cpsw_nuss_remove_rx_chns(struct am65_cpsw_common *common)
+static void am65_cpsw_nuss_cleanup_rx_chns(struct am65_cpsw_common *common)
{
struct device *dev = common->dev;
struct am65_cpsw_rx_chn *rx_chn;
@@ -2398,7 +2348,6 @@ static void am65_cpsw_nuss_remove_rx_chns(struct am65_cpsw_common *common)

rx_chn = &common->rx_chns;
flows = rx_chn->flows;
- devm_remove_action(dev, am65_cpsw_nuss_free_rx_chns, common);

for (i = 0; i < common->rx_ch_num_flows; i++) {
if (!(flows[i].irq < 0))
@@ -2406,7 +2355,11 @@ static void am65_cpsw_nuss_remove_rx_chns(struct am65_cpsw_common *common)
netif_napi_del(&flows[i].napi_rx);
}

- am65_cpsw_nuss_free_rx_chns(common);
+ if (!IS_ERR_OR_NULL(rx_chn->desc_pool))
+ k3_cppi_desc_pool_destroy(rx_chn->desc_pool);
+
+ if (!IS_ERR_OR_NULL(rx_chn->rx_chn))
+ k3_udma_glue_release_rx_chn(rx_chn->rx_chn);

common->rx_flow_id_base = -1;
}
@@ -2535,14 +2488,7 @@ static int am65_cpsw_nuss_init_rx_chns(struct am65_cpsw_common *common)

/* setup classifier to route priorities to flows */
cpsw_ale_classifier_setup_default(common->ale, common->rx_ch_num_flows);
-
err:
- i = devm_add_action(dev, am65_cpsw_nuss_free_rx_chns, common);
- if (i) {
- dev_err(dev, "Failed to add free_rx_chns action %d\n", i);
- return i;
- }
-
return ret;
}

@@ -2626,6 +2572,22 @@ static int am65_cpsw_init_cpts(struct am65_cpsw_common *common)
return 0;
}

+static void am65_cpsw_nuss_cleanup_slave_ports(struct am65_cpsw_common *common)
+{
+ struct am65_cpsw_port *port;
+ struct phy *phy;
+ int i;
+
+ for (i = 0; i < common->port_num; i++) {
+ port = &common->ports[i];
+ phy = port->slave.serdes_phy;
+ if (phy) {
+ am65_cpsw_disable_phy(phy);
+ port->slave.serdes_phy = NULL;
+ }
+ }
+}
+
static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
{
struct device_node *node, *port_np;
@@ -2743,18 +2705,6 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
return ret;
}

-static void am65_cpsw_nuss_phylink_cleanup(struct am65_cpsw_common *common)
-{
- struct am65_cpsw_port *port;
- int i;
-
- for (i = 0; i < common->port_num; i++) {
- port = &common->ports[i];
- if (port->slave.phylink)
- phylink_destroy(port->slave.phylink);
- }
-}
-
static int
am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx)
{
@@ -2863,34 +2813,42 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx)
return 0;
}

-static int am65_cpsw_nuss_init_ndevs(struct am65_cpsw_common *common)
+static void am65_cpsw_nuss_cleanup_ndevs(struct am65_cpsw_common *common)
{
- int ret;
+ struct am65_cpsw_port *port;
int i;

for (i = 0; i < common->port_num; i++) {
- ret = am65_cpsw_nuss_init_port_ndev(common, i);
- if (ret)
- return ret;
+ port = &common->ports[i];
+ if (port->disabled)
+ continue;
+
+ if (port->slave.phylink) {
+ phylink_destroy(port->slave.phylink);
+ port->slave.phylink = NULL;
+ }
+
+ if (port->ndev) {
+ free_netdev(port->ndev);
+ port->ndev = NULL;
+ }
}

- return ret;
+ common->dma_ndev = NULL;
}

-static void am65_cpsw_nuss_cleanup_ndev(struct am65_cpsw_common *common)
+static int am65_cpsw_nuss_init_ndevs(struct am65_cpsw_common *common)
{
- struct am65_cpsw_port *port;
+ int ret;
int i;

for (i = 0; i < common->port_num; i++) {
- port = &common->ports[i];
- if (!port->ndev)
- continue;
- if (port->ndev->reg_state == NETREG_REGISTERED)
- unregister_netdev(port->ndev);
- free_netdev(port->ndev);
- port->ndev = NULL;
+ ret = am65_cpsw_nuss_init_port_ndev(common, i);
+ if (ret)
+ return ret;
}
+
+ return ret;
}

static void am65_cpsw_port_offload_fwd_mark_update(struct am65_cpsw_common *common)
@@ -3338,21 +3296,29 @@ static void am65_cpsw_unregister_devlink(struct am65_cpsw_common *common)
devlink_free(common->devlink);
}

-static int am65_cpsw_nuss_register_ndevs(struct am65_cpsw_common *common)
+static void am65_cpsw_nuss_cleanup_chns(struct am65_cpsw_common *common)
+{
+ am65_cpsw_nuss_cleanup_rx_chns(common);
+ am65_cpsw_nuss_cleanup_tx_chns(common);
+}
+
+static int am65_cpsw_nuss_init_chns(struct am65_cpsw_common *common)
{
struct am65_cpsw_rx_chn *rx_chan = &common->rx_chns;
struct am65_cpsw_tx_chn *tx_chan = common->tx_chns;
- struct device *dev = common->dev;
- struct am65_cpsw_port *port;
- int ret = 0, i;
+ int ret, i;

/* init tx channels */
ret = am65_cpsw_nuss_init_tx_chns(common);
if (ret)
return ret;
+
+ /* init rx channels */
ret = am65_cpsw_nuss_init_rx_chns(common);
- if (ret)
+ if (ret) {
+ am65_cpsw_nuss_cleanup_tx_chns(common);
return ret;
+ }

/* The DMA Channels are not guaranteed to be in a clean state.
* Reset and disable them to ensure that they are back to the
@@ -3371,13 +3337,32 @@ static int am65_cpsw_nuss_register_ndevs(struct am65_cpsw_common *common)

k3_udma_glue_disable_rx_chn(rx_chan->rx_chn);

- ret = am65_cpsw_nuss_register_devlink(common);
- if (ret)
- return ret;
+ return 0;
+}
+
+static void am65_cpsw_nuss_unregister_ndev(struct am65_cpsw_common *common)
+{
+ struct am65_cpsw_port *port;
+ int i;

for (i = 0; i < common->port_num; i++) {
port = &common->ports[i];
+ if (!port->ndev)
+ continue;
+
+ if (port->ndev->reg_state == NETREG_REGISTERED)
+ unregister_netdev(port->ndev);
+ }
+}
+
+static int am65_cpsw_nuss_register_ndevs(struct am65_cpsw_common *common)
+{
+ struct device *dev = common->dev;
+ struct am65_cpsw_port *port;
+ int ret = 0, i;

+ for (i = 0; i < common->port_num; i++) {
+ port = &common->ports[i];
if (!port->ndev)
continue;

@@ -3387,25 +3372,11 @@ static int am65_cpsw_nuss_register_ndevs(struct am65_cpsw_common *common)
if (ret) {
dev_err(dev, "error registering slave net device%i %d\n",
i, ret);
- goto err_cleanup_ndev;
+ return ret;
}
}

- ret = am65_cpsw_register_notifiers(common);
- if (ret)
- goto err_cleanup_ndev;
-
- /* can't auto unregister ndev using devm_add_action() due to
- * devres release sequence in DD core for DMA
- */
-
return 0;
-
-err_cleanup_ndev:
- am65_cpsw_nuss_cleanup_ndev(common);
- am65_cpsw_unregister_devlink(common);
-
- return ret;
}

int am65_cpsw_nuss_update_tx_rx_chns(struct am65_cpsw_common *common,
@@ -3413,17 +3384,10 @@ int am65_cpsw_nuss_update_tx_rx_chns(struct am65_cpsw_common *common,
{
int ret;

- am65_cpsw_nuss_remove_tx_chns(common);
- am65_cpsw_nuss_remove_rx_chns(common);
-
+ am65_cpsw_nuss_cleanup_chns(common);
common->tx_ch_num = num_tx;
common->rx_ch_num_flows = num_rx;
- ret = am65_cpsw_nuss_init_tx_chns(common);
- if (ret)
- return ret;
-
- ret = am65_cpsw_nuss_init_rx_chns(common);
-
+ ret = am65_cpsw_nuss_init_chns(common);
return ret;
}

@@ -3599,7 +3563,7 @@ static int am65_cpsw_nuss_probe(struct platform_device *pdev)

ret = am65_cpsw_nuss_init_slave_ports(common);
if (ret)
- goto err_of_clear;
+ goto err_ports_clear;

/* init common data */
ale_params.dev = dev;
@@ -3613,7 +3577,7 @@ static int am65_cpsw_nuss_probe(struct platform_device *pdev)
if (IS_ERR(common->ale)) {
dev_err(dev, "error initializing ale engine\n");
ret = PTR_ERR(common->ale);
- goto err_of_clear;
+ goto err_ports_clear;
}

ale_entries = common->ale->params.ale_entries;
@@ -3622,7 +3586,7 @@ static int am65_cpsw_nuss_probe(struct platform_device *pdev)
GFP_KERNEL);
ret = am65_cpsw_init_cpts(common);
if (ret)
- goto err_of_clear;
+ goto err_ports_clear;

/* init ports */
for (i = 0; i < common->port_num; i++)
@@ -3634,19 +3598,39 @@ static int am65_cpsw_nuss_probe(struct platform_device *pdev)

ret = am65_cpsw_nuss_init_ndevs(common);
if (ret)
- goto err_ndevs_clear;
+ goto err_cpts_release;
+
+ ret = am65_cpsw_nuss_init_chns(common);
+ if (ret)
+ goto err_cleanup_ndevs;
+
+ ret = am65_cpsw_nuss_register_devlink(common);
+ if (ret)
+ goto err_cleanup_chns;

ret = am65_cpsw_nuss_register_ndevs(common);
if (ret)
- goto err_ndevs_clear;
+ goto err_unregister_devlink;
+
+ ret = am65_cpsw_register_notifiers(common);
+ if (ret)
+ goto err_unregister_ndev;

pm_runtime_put(dev);
return 0;

-err_ndevs_clear:
- am65_cpsw_nuss_cleanup_ndev(common);
- am65_cpsw_nuss_phylink_cleanup(common);
+err_unregister_ndev:
+ am65_cpsw_nuss_unregister_ndev(common);
+err_unregister_devlink:
+ am65_cpsw_unregister_devlink(common);
+err_cleanup_chns:
+ am65_cpsw_nuss_cleanup_chns(common);
+err_cleanup_ndevs:
+ am65_cpsw_nuss_cleanup_ndevs(common);
+err_cpts_release:
am65_cpts_release(common->cpts);
+err_ports_clear:
+ am65_cpsw_nuss_cleanup_slave_ports(common);
err_of_clear:
if (common->mdio_dev)
of_platform_device_destroy(common->mdio_dev, NULL);
@@ -3675,15 +3659,12 @@ static void am65_cpsw_nuss_remove(struct platform_device *pdev)
}

am65_cpsw_unregister_notifiers(common);
-
- /* must unregister ndevs here because DD release_driver routine calls
- * dma_deconfigure(dev) before devres_release_all(dev)
- */
- am65_cpsw_nuss_cleanup_ndev(common);
+ am65_cpsw_nuss_unregister_ndev(common);
am65_cpsw_unregister_devlink(common);
- am65_cpsw_nuss_phylink_cleanup(common);
+ am65_cpsw_nuss_cleanup_chns(common);
+ am65_cpsw_nuss_cleanup_ndevs(common);
am65_cpts_release(common->cpts);
- am65_cpsw_disable_serdes_phy(common);
+ am65_cpsw_nuss_cleanup_slave_ports(common);

if (common->mdio_dev)
of_platform_device_destroy(common->mdio_dev, NULL);
@@ -3723,9 +3704,7 @@ static int am65_cpsw_nuss_suspend(struct device *dev)
}

am65_cpts_suspend(common->cpts);
-
- am65_cpsw_nuss_remove_rx_chns(common);
- am65_cpsw_nuss_remove_tx_chns(common);
+ am65_cpsw_nuss_cleanup_chns(common);

return 0;
}
@@ -3738,10 +3717,7 @@ static int am65_cpsw_nuss_resume(struct device *dev)
struct net_device *ndev;
int i, ret;

- ret = am65_cpsw_nuss_init_tx_chns(common);
- if (ret)
- return ret;
- ret = am65_cpsw_nuss_init_rx_chns(common);
+ ret = am65_cpsw_nuss_init_chns(common);
if (ret)
return ret;

--
2.34.1