Patchwork [09/19] usbnet: smsc95xx: Connect to phydev

login
register
mail settings
Submitter Marek Vasut
Date Jan. 3, 2019, 1:10 a.m.
Message ID <20190103011040.25974-10-marex@denx.de>
Download mbox | patch
Permalink /patch/691891/
State New
Headers show

Comments

Marek Vasut - Jan. 3, 2019, 1:10 a.m.
Add code to detect and connect to PHY. The internal PHY of the SMSC95xx
is a regular SMSC LAN8700 and the driver only supports the internal PHY,
so just use the SMSC PHY driver to configure the PHY. Note that the
driver does a lot of extra configuration of the PHY, which is left in
to avoid breakage. Some of the extra configuration is sorted out by
later patches in this series.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Nisar Sayed <Nisar.Sayed@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: linux-usb@vger.kernel.org
To: netdev@vger.kernel.org
---
 drivers/net/usb/smsc95xx.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)
Andrew Lunn - Jan. 3, 2019, 1:22 p.m.
On Thu, Jan 03, 2019 at 02:10:30AM +0100, Marek Vasut wrote:
> Add code to detect and connect to PHY. The internal PHY of the SMSC95xx
> is a regular SMSC LAN8700 and the driver only supports the internal PHY,
> so just use the SMSC PHY driver to configure the PHY. Note that the
> driver does a lot of extra configuration of the PHY, which is left in
> to avoid breakage. Some of the extra configuration is sorted out by
> later patches in this series.

Hi Marek

A MAC driver is not expected to touch the PHY at all. Please try to
remove as much of the extra configuration as possible, adding it to
the LAN8700 PHY driver as needed.

You also have to be careful of locking. phylib takes the phydev lock
when calling into the PHY driver. Anything the MAC does to the PHY is
not going to be done with this lock held. So bad things can happen.

    Andrew
Marek Vasut - Jan. 4, 2019, 2:18 a.m.
On 1/3/19 2:22 PM, Andrew Lunn wrote:
> On Thu, Jan 03, 2019 at 02:10:30AM +0100, Marek Vasut wrote:
>> Add code to detect and connect to PHY. The internal PHY of the SMSC95xx
>> is a regular SMSC LAN8700 and the driver only supports the internal PHY,
>> so just use the SMSC PHY driver to configure the PHY. Note that the
>> driver does a lot of extra configuration of the PHY, which is left in
>> to avoid breakage. Some of the extra configuration is sorted out by
>> later patches in this series.
> 
> Hi Marek
> 
> A MAC driver is not expected to touch the PHY at all.

Well, sure, that's not how the SMSC95xx driver is implemented. This
series tries to make that a bit better.

> Please try to
> remove as much of the extra configuration as possible, adding it to
> the LAN8700 PHY driver as needed.

Sure, that's the plan.

> You also have to be careful of locking. phylib takes the phydev lock
> when calling into the PHY driver. Anything the MAC does to the PHY is
> not going to be done with this lock held. So bad things can happen.

I think I can add a patch which grabs the lock in the MDIO accessors
used by the SMSC95xx driver, at least temporarily, until it is fully
migrated to phydev ? Or is there a better option ?

Patch

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 2cd5e1d34559..ce61be8ee32b 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -78,6 +78,7 @@  struct smsc95xx_priv {
 	struct delayed_work carrier_check;
 	struct usbnet *dev;
 	struct mii_bus *mii_bus;
+	struct phy_device *phydev;
 };
 
 static bool turbo_mode = true;
@@ -960,6 +961,11 @@  static int smsc95xx_start_rx_path(struct usbnet *dev)
 	return smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
 }
 
+
+static void smsc95xx_adjust_link(struct net_device *netdev)
+{
+}
+
 static int smsc95xx_phy_initialize(struct usbnet *dev)
 {
 	int bmcr, ret, timeout = 0;
@@ -1308,6 +1314,10 @@  static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	if (ret)
 		goto err_mdiobus_alloc;
 
+	ret = mdiobus_register(bus);
+	if (ret)
+		goto err_mdiobus_register;
+
 	/* Initialize MII structure */
 	dev->mii.dev = dev->net;
 	dev->mii.mdio_read = smsc95xx_mdio_read;
@@ -1316,9 +1326,24 @@  static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->mii.reg_num_mask = 0x1f;
 	dev->mii.phy_id = SMSC95XX_INTERNAL_PHY_ID;
 
-	ret = mdiobus_register(bus);
-	if (ret)
+	/* Connect to PHY */
+	pdata->phydev = phy_find_first(pdata->mii_bus);
+	if (!pdata->phydev) {
+		netdev_err(dev->net, "no PHY found\n");
+		ret = -EIO;
 		goto err_mdiobus_register;
+	}
+
+	ret = phy_connect_direct(dev->net, pdata->phydev,
+				 &smsc95xx_adjust_link,
+				 PHY_INTERFACE_MODE_MII);
+	if (ret) {
+		netdev_err(dev->net, "Could not connect to PHY device\n");
+		goto err_mdiobus_register;
+	}
+
+	genphy_resume(pdata->phydev);
+	phy_start(pdata->phydev);
 
 	ret = smsc95xx_reset_post(dev);
 	if (ret)