[PATCH net-next v1] lan743x: replace devicetree phy parse code with library function

[PATCH net-next v1] lan743x: replace devicetree phy parse code with library function

[Sven Van Asbroeck]

From: Sven Van Asbroeck <thesven73@gmail.com>

The code in this driver which parses the devicetree to determine
the phy/fixed link setup, can be replaced by a single library
function: of_phy_get_and_connect().

Behaviour is identical, except that the library function will
complain when 'phy-connection-type' is omitted, instead of
blindly using PHY_INTERFACE_MODE_NA, which would result in an
invalid phy configuration.

The library function no longer brings out the exact phy_mode,
but the driver doesn't need this, because phy_interface_is_rgmii()
queries the phydev directly. Remove 'phy_mode' from the private
adapter struct.

While we're here, log info about the attached phy on connect,
this is useful because the phy type and connection method is now
fully configurable via the devicetree.

Tested on a lan7430 chip with built-in phy. Verified that adding
fixed-link/phy-connection-type in the devicetree results in a
fixed-link setup. Used ethtool to verify that the devicetree
settings are used.

Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # lan7430
Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>
---

Tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git # 0064c5c1b3bf

To: Bryan Whitehead <bryan.whitehead@microchip.com>
Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
To: "David S. Miller" <davem@davemloft.net>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Roelof Berg <rberg@berg-solutions.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

 drivers/net/ethernet/microchip/lan743x_main.c | 35 +++++--------------
 drivers/net/ethernet/microchip/lan743x_main.h |  1 -
 2 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 0c9b938ee0ea..e88789bf2d0f 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -957,7 +957,7 @@ static void lan743x_phy_link_status_change(struct net_device *netdev)
 		data = lan743x_csr_read(adapter, MAC_CR);
 
 		/* set interface mode */
-		if (phy_interface_mode_is_rgmii(adapter->phy_mode))
+		if (phy_interface_is_rgmii(phydev))
 			/* RGMII */
 			data &= ~MAC_CR_MII_EN_;
 		else
@@ -1013,33 +1013,14 @@ static void lan743x_phy_close(struct lan743x_adapter *adapter)
 
 static int lan743x_phy_open(struct lan743x_adapter *adapter)
 {
+	struct net_device *netdev = adapter->netdev;
 	struct lan743x_phy *phy = &adapter->phy;
-	struct phy_device *phydev = NULL;
-	struct device_node *phynode;
-	struct net_device *netdev;
+	struct phy_device *phydev;
 	int ret = -EIO;
 
-	netdev = adapter->netdev;
-	phynode = of_node_get(adapter->pdev->dev.of_node);
-
-	if (phynode) {
-		/* try devicetree phy, or fixed link */
-		of_get_phy_mode(phynode, &adapter->phy_mode);
-
-		if (of_phy_is_fixed_link(phynode)) {
-			ret = of_phy_register_fixed_link(phynode);
-			if (ret) {
-				netdev_err(netdev,
-					   "cannot register fixed PHY\n");
-				of_node_put(phynode);
-				goto return_error;
-			}
-		}
-		phydev = of_phy_connect(netdev, phynode,
-					lan743x_phy_link_status_change, 0,
-					adapter->phy_mode);
-		of_node_put(phynode);
-	}
+	/* try devicetree phy, or fixed link */
+	phydev = of_phy_get_and_connect(netdev, adapter->pdev->dev.of_node,
+					lan743x_phy_link_status_change);
 
 	if (!phydev) {
 		/* try internal phy */
@@ -1047,10 +1028,9 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter)
 		if (!phydev)
 			goto return_error;
 
-		adapter->phy_mode = PHY_INTERFACE_MODE_GMII;
 		ret = phy_connect_direct(netdev, phydev,
 					 lan743x_phy_link_status_change,
-					 adapter->phy_mode);
+					 PHY_INTERFACE_MODE_GMII);
 		if (ret)
 			goto return_error;
 	}
@@ -1065,6 +1045,7 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter)
 
 	phy_start(phydev);
 	phy_start_aneg(phydev);
+	phy_attached_info(phydev);
 	return 0;
 
 return_error:
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index a536f4a4994d..3a0e70daa88f 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -703,7 +703,6 @@ struct lan743x_rx {
 struct lan743x_adapter {
 	struct net_device       *netdev;
 	struct mii_bus		*mdiobus;
-	phy_interface_t		phy_mode;
 	int                     msg_enable;
 #ifdef CONFIG_PM
 	u32			wolopts;
-- 
2.17.1

Re: [PATCH net-next v1] lan743x: replace devicetree phy parse code with library function

[Andrew Lunn]

On Mon, Nov 16, 2020 at 12:01:55PM -0500, Sven Van Asbroeck wrote:
> From: Sven Van Asbroeck <thesven73@gmail.com>
> 
> The code in this driver which parses the devicetree to determine
> the phy/fixed link setup, can be replaced by a single library
> function: of_phy_get_and_connect().
> 
> Behaviour is identical, except that the library function will
> complain when 'phy-connection-type' is omitted, instead of
> blindly using PHY_INTERFACE_MODE_NA, which would result in an
> invalid phy configuration.
> 
> The library function no longer brings out the exact phy_mode,
> but the driver doesn't need this, because phy_interface_is_rgmii()
> queries the phydev directly. Remove 'phy_mode' from the private
> adapter struct.
> 
> While we're here, log info about the attached phy on connect,
> this is useful because the phy type and connection method is now
> fully configurable via the devicetree.
> 
> Tested on a lan7430 chip with built-in phy. Verified that adding
> fixed-link/phy-connection-type in the devicetree results in a
> fixed-link setup. Used ethtool to verify that the devicetree
> settings are used.
> 
> Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # lan7430
> Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v1] lan743x: replace devicetree phy parse code with library function

[Jakub Kicinski]

On Tue, 17 Nov 2020 03:09:56 +0100 Andrew Lunn wrote:
> On Mon, Nov 16, 2020 at 12:01:55PM -0500, Sven Van Asbroeck wrote:
> > From: Sven Van Asbroeck <thesven73@gmail.com>
> > 
> > The code in this driver which parses the devicetree to determine
> > the phy/fixed link setup, can be replaced by a single library
> > function: of_phy_get_and_connect().
> > 
> > Behaviour is identical, except that the library function will
> > complain when 'phy-connection-type' is omitted, instead of
> > blindly using PHY_INTERFACE_MODE_NA, which would result in an
> > invalid phy configuration.
> > 
> > The library function no longer brings out the exact phy_mode,
> > but the driver doesn't need this, because phy_interface_is_rgmii()
> > queries the phydev directly. Remove 'phy_mode' from the private
> > adapter struct.
> > 
> > While we're here, log info about the attached phy on connect,
> > this is useful because the phy type and connection method is now
> > fully configurable via the devicetree.
> > 
> > Tested on a lan7430 chip with built-in phy. Verified that adding
> > fixed-link/phy-connection-type in the devicetree results in a
> > fixed-link setup. Used ethtool to verify that the devicetree
> > settings are used.
> > 
> > Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # lan7430
> > Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>  
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Applied, thanks!

Re: [PATCH net-next v1] lan743x: replace devicetree phy parse code with library function

[Sven Van Asbroeck]

On Tue, Nov 17, 2020 at 1:47 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 17 Nov 2020 03:09:56 +0100 Andrew Lunn wrote:
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Applied, thanks!

Thank you Andrew and Jakub !