The ftgmac100 supports NC-SI mode, dedicated PHY and fixed-link
PHY. The dedicated PHY is using the phy_handle property to get
phy device handle and the fixed-link phy is using the fixed-link
property to register a fixed-link phy device.

In of_phy_get_and_connect function, it help driver to get and register
these PHYs handle.
Therefore, here refactors this part by using of_phy_get_and_connect.

Fixes: 38561ded50d0 ("net: ftgmac100: support fixed link")
Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property")
Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 31 ++++++------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 0b61f548fd18..ae0235a7a74e 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1918,35 +1918,17 @@ static int ftgmac100_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "Connecting PHY failed\n");
 			goto err_phy_connect;
 		}
-	} else if (np && of_phy_is_fixed_link(np)) {
-		struct phy_device *phy;
-
-		err = of_phy_register_fixed_link(np);
-		if (err) {
-			dev_err(&pdev->dev, "Failed to register fixed PHY\n");
-			goto err_phy_connect;
-		}
-
-		phy = of_phy_get_and_connect(priv->netdev, np,
-					     &ftgmac100_adjust_link);
-		if (!phy) {
-			dev_err(&pdev->dev, "Failed to connect to fixed PHY\n");
-			of_phy_deregister_fixed_link(np);
-			err = -EINVAL;
-			goto err_phy_connect;
-		}
-
-		/* Display what we found */
-		phy_attached_info(phy);
-	} else if (np && of_get_property(np, "phy-handle", NULL)) {
+	} else if (np && (of_phy_is_fixed_link(np) ||
+			  of_get_property(np, "phy-handle", NULL))) {
 		struct phy_device *phy;
 
 		/* Support "mdio"/"phy" child nodes for ast2400/2500 with
 		 * an embedded MDIO controller. Automatically scan the DTS for
 		 * available PHYs and register them.
 		 */
-		if (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
-		    of_device_is_compatible(np, "aspeed,ast2500-mac")) {
+		if (of_get_property(np, "phy-handle", NULL) &&
+		    (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
+		     of_device_is_compatible(np, "aspeed,ast2500-mac"))) {
 			err = ftgmac100_setup_mdio(netdev);
 			if (err)
 				goto err_setup_mdio;
@@ -1963,7 +1945,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
 		/* Indicate that we support PAUSE frames (see comment in
 		 * Documentation/networking/phy.rst)
 		 */
-		phy_support_asym_pause(phy);
+		if (of_get_property(np, "phy-handle", NULL))
+			phy_support_asym_pause(phy);
 
 		/* Display what we found */
 		phy_attached_info(phy);
-- 
2.25.1

On Fri, Oct 11, 2024 at 04:16:33PM +0800, Jacky Chou wrote:
> The ftgmac100 supports NC-SI mode, dedicated PHY and fixed-link
> PHY. The dedicated PHY is using the phy_handle property to get
> phy device handle and the fixed-link phy is using the fixed-link
> property to register a fixed-link phy device.
> 
> In of_phy_get_and_connect function, it help driver to get and register
> these PHYs handle.
> Therefore, here refactors this part by using of_phy_get_and_connect.
> 
> Fixes: 38561ded50d0 ("net: ftgmac100: support fixed link")
> Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property")

Fixes: implies something is broken. What is actually wrong with this
code? What sort of problem does a user see?

> -		phy_support_asym_pause(phy);
> +		if (of_get_property(np, "phy-handle", NULL))
> +			phy_support_asym_pause(phy);

This is probably wrong. This is the MAC layer telling phylib that the
MAC supports asym pause. It should makes no difference to the MAC what
sort of PHY is being used, all the MAC is looking at/sending is pause
frames.

    Andrew

---
pw-bot: cr