Re: [PATCH v1 1/2] net: ftgmac100: move phy connect out from ftgmac100_setup_mdio
From: Andrew Lunn <andrew@lunn.ch>
Date: 2020-10-17 20:32:42
Also in:
lkml, openbmc
On Thu, Oct 15, 2020 at 03:49:16PM +0300, Ivan Mikhaylov wrote:
quoted hunk ↗ jump to hunk
Split MDIO registration and PHY connect into ftgmac100_setup_mdio and ftgmac100_mii_probe. Signed-off-by: Ivan Mikhaylov <redacted> --- drivers/net/ethernet/faraday/ftgmac100.c | 92 ++++++++++++------------ 1 file changed, 47 insertions(+), 45 deletions(-)diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 87236206366f..6997e121824b 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c@@ -1044,11 +1044,47 @@ static void ftgmac100_adjust_link(struct net_device *netdev) schedule_work(&priv->reset_task); } -static int ftgmac100_mii_probe(struct ftgmac100 *priv, phy_interface_t intf) +static int ftgmac100_mii_probe(struct net_device *netdev) { - struct net_device *netdev = priv->netdev; + struct ftgmac100 *priv = netdev_priv(netdev); + struct platform_device *pdev = to_platform_device(priv->dev); + struct device_node *np = pdev->dev.of_node; + phy_interface_t phy_intf = PHY_INTERFACE_MODE_RGMII; struct phy_device *phydev;
Reverse Christmas tree.
+ /* Get PHY mode from device-tree */
+ if (np) {
+ /* Default to RGMII. It's a gigabit part after all */
+ phy_intf = of_get_phy_mode(np, &phy_intf);
+ if (phy_intf < 0)
+ phy_intf = PHY_INTERFACE_MODE_RGMII;I know you are just moving code around, but it is better to do:
+ err = of_get_phy_mode(np, &phy_intf); + if (err) + phy_intf = PHY_INTERFACE_MODE_RGMII;
With the code you have, you are probably going to get an email about assigning an int to an unsigned int type from Colin..
quoted hunk ↗ jump to hunk
@@ -1860,6 +1854,14 @@ static int ftgmac100_probe(struct platform_device *pdev) err = ftgmac100_setup_mdio(netdev); if (err) goto err_setup_mdio; + + err = ftgmac100_mii_probe(netdev); + if (err) { + dev_err(priv->dev, "MII probe failed!\n"); + mdiobus_unregister(priv->mii_bus); + goto err_setup_mdio; + }
It is more uniform to add a new label and add the
mdiobus_unregister(priv->mii_bus) there. All the other error handling
works like that.
Andrew