Re: [net-next,v3,08/15] net: ftgmac100: Move NCSI probe code into a helper
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-02-04 03:34:23
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: ftgmac100: Move NCSI probe code into a helper This commit refactors ftgmac100_probe() by extracting NCSI-specific probe code into a new helper function ftgmac100_probe_ncsi(). The commit message states "No functional change intended."
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index e511d6d5f7c2..9d8256d4d368 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c@@ -1839,6 +1839,39 @@ static bool ftgmac100_has_child_node(struct device_node *np, const char *name)return ret; } +static int ftgmac100_probe_ncsi(struct net_device *netdev, + struct ftgmac100 *priv, + struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct phy_device *phydev; + int err; + + if (!IS_ENABLED(CONFIG_NET_NCSI)) { + dev_err(&pdev->dev, "NCSI stack not enabled\n"); + return -EINVAL; + } + + dev_info(&pdev->dev, "Using NCSI interface\n"); + priv->use_ncsi = true; + priv->ndev = ncsi_register_dev(netdev, ftgmac100_ncsi_handler); + if (!priv->ndev) + return -EINVAL; + + phydev = fixed_phy_register(&ncsi_phy_status, np); + if (IS_ERR(phydev)) { + dev_err(&pdev->dev, "failed to register fixed PHY device\n"); + return PTR_ERR(phydev);
If fixed_phy_register() fails here, doesn't priv->ndev leak? The NCSI device was registered via ncsi_register_dev() above, but this error path returns without calling ncsi_unregister_dev().
+ }
+ err = phy_connect_direct(netdev, phydev, ftgmac100_adjust_link,
+ PHY_INTERFACE_MODE_RMII);
+ if (err) {
+ dev_err(&pdev->dev, "Connecting PHY failed\n");
+ fixed_phy_unregister(phydev);
+ }
+ return err;Similarly, if phy_connect_direct() fails, fixed_phy_unregister() is called but ncsi_unregister_dev(priv->ndev) is not. [ ... ]
quoted hunk ↗ jump to hunk
@@ -1928,32 +1960,9 @@ static int ftgmac100_probe(struct platform_device *pdev) } if (np && of_get_property(np, "use-ncsi", NULL)) { - if (!IS_ENABLED(CONFIG_NET_NCSI)) { - dev_err(&pdev->dev, "NCSI stack not enabled\n"); - err = -EINVAL; - goto err_phy_connect;
[ ... ]
- phydev = fixed_phy_register(&ncsi_phy_status, np);
- if (IS_ERR(phydev)) {
- dev_err(&pdev->dev, "failed to register fixed PHY device\n");
- err = PTR_ERR(phydev);
- goto err_phy_connect;The old code jumped to err_phy_connect on failure, which falls through to err_ncsi_dev where ncsi_unregister_dev(priv->ndev) is called.
+ err = ftgmac100_probe_ncsi(netdev, priv, pdev); + if (err) + goto err_setup_mdio;
The new code jumps to err_setup_mdio, which skips the err_ncsi_dev label entirely. Combined with the helper not cleaning up priv->ndev on its error paths, this appears to be a behavioral change from the original code. -- pw-bot: cr