[PATCH net-next v2 0/6] Add PAL support to smsc95xx

[PATCH net-next v2 0/6] Add PAL support to smsc95xx

[Andre Edich]

To allow to probe external PHY drivers, this patch series adds use of
Phy Abstraction Layer to the smsc95xx driver.

Changes in v2:
- Moved 'net' patches from here to the separate patch series;
- Removed redundant call of the phy_start_aneg after phy_start;
- Removed netif_dbg tracing "speed, duplex, lcladv, and rmtadv";
- mdiobus: added dependency from the usbnet device;
- Moved making of the MII address from 'phy_id' and 'idx' into the
  function mii_address;
- Moved direct MDIO accesses under condition 'if (pdata->internal_phy)',
  as they only need for the internal PHY;
- To be sure, that this set of patches is git-bisectable, tested each
  sub-set of patches to be functional for both, internal and external
  PHYs, including suspend/resume test for the 'devices'
  (5.7.8-1-ARCH, Raspberry Pi 3 Model B).

Andre Edich (6):
  smsc95xx: remove redundant function arguments
  smsc95xx: use usbnet->driver_priv
  smsc95xx: add PAL support to use external PHY drivers
  smsc95xx: remove redundant link status checking
  smsc95xx: use PAL framework read/write functions
  smsc95xx: use PHY framework instead of MII library

 drivers/net/usb/smsc95xx.c | 413 +++++++++++++++++--------------------
 1 file changed, 194 insertions(+), 219 deletions(-)

-- 
2.27.0

[PATCH net-next v2 1/6] smsc95xx: remove redundant function arguments

[Andre Edich]

This patch removes arguments netdev and phy_id from the functions
smsc95xx_mdio_read_nopm and smsc95xx_mdio_write_nopm.  Both removed
arguments are recovered from a new argument `struct usbnet *dev`.

Signed-off-by: Andre Edich <andre.edich@microchip.com>
---
 drivers/net/usb/smsc95xx.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index bb4ccbda031a..3fdf7c2b2d25 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -261,16 +261,18 @@ static void __smsc95xx_mdio_write(struct net_device *netdev, int phy_id,
 	mutex_unlock(&dev->phy_mutex);
 }
 
-static int smsc95xx_mdio_read_nopm(struct net_device *netdev, int phy_id,
-				   int idx)
+static int smsc95xx_mdio_read_nopm(struct usbnet *dev, int idx)
 {
-	return __smsc95xx_mdio_read(netdev, phy_id, idx, 1);
+	struct mii_if_info *mii = &dev->mii;
+
+	return __smsc95xx_mdio_read(dev->net, mii->phy_id, idx, 1);
 }
 
-static void smsc95xx_mdio_write_nopm(struct net_device *netdev, int phy_id,
-				     int idx, int regval)
+static void smsc95xx_mdio_write_nopm(struct usbnet *dev, int idx, int regval)
 {
-	__smsc95xx_mdio_write(netdev, phy_id, idx, regval, 1);
+	struct mii_if_info *mii = &dev->mii;
+
+	__smsc95xx_mdio_write(dev->net, mii->phy_id, idx, regval, 1);
 }
 
 static int smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx)
@@ -1347,39 +1349,37 @@ static u32 smsc_crc(const u8 *buffer, size_t len, int filter)
 
 static int smsc95xx_enable_phy_wakeup_interrupts(struct usbnet *dev, u16 mask)
 {
-	struct mii_if_info *mii = &dev->mii;
 	int ret;
 
 	netdev_dbg(dev->net, "enabling PHY wakeup interrupts\n");
 
 	/* read to clear */
-	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_SRC);
+	ret = smsc95xx_mdio_read_nopm(dev, PHY_INT_SRC);
 	if (ret < 0)
 		return ret;
 
 	/* enable interrupt source */
-	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_MASK);
+	ret = smsc95xx_mdio_read_nopm(dev, PHY_INT_MASK);
 	if (ret < 0)
 		return ret;
 
 	ret |= mask;
 
-	smsc95xx_mdio_write_nopm(dev->net, mii->phy_id, PHY_INT_MASK, ret);
+	smsc95xx_mdio_write_nopm(dev, PHY_INT_MASK, ret);
 
 	return 0;
 }
 
 static int smsc95xx_link_ok_nopm(struct usbnet *dev)
 {
-	struct mii_if_info *mii = &dev->mii;
 	int ret;
 
 	/* first, a dummy read, needed to latch some MII phys */
-	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
+	ret = smsc95xx_mdio_read_nopm(dev, MII_BMSR);
 	if (ret < 0)
 		return ret;
 
-	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
+	ret = smsc95xx_mdio_read_nopm(dev, MII_BMSR);
 	if (ret < 0)
 		return ret;
 
@@ -1428,7 +1428,6 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
 static int smsc95xx_enter_suspend1(struct usbnet *dev)
 {
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-	struct mii_if_info *mii = &dev->mii;
 	u32 val;
 	int ret;
 
@@ -1436,17 +1435,17 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
 	 * compatibility with non-standard link partners
 	 */
 	if (pdata->features & FEATURE_PHY_NLP_CROSSOVER)
-		smsc95xx_mdio_write_nopm(dev->net, mii->phy_id,	PHY_EDPD_CONFIG,
-			PHY_EDPD_CONFIG_DEFAULT);
+		smsc95xx_mdio_write_nopm(dev, PHY_EDPD_CONFIG,
+					 PHY_EDPD_CONFIG_DEFAULT);
 
 	/* enable energy detect power-down mode */
-	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_MODE_CTRL_STS);
+	ret = smsc95xx_mdio_read_nopm(dev, PHY_MODE_CTRL_STS);
 	if (ret < 0)
 		return ret;
 
 	ret |= MODE_CTRL_STS_EDPWRDOWN_;
 
-	smsc95xx_mdio_write_nopm(dev->net, mii->phy_id, PHY_MODE_CTRL_STS, ret);
+	smsc95xx_mdio_write_nopm(dev, PHY_MODE_CTRL_STS, ret);
 
 	/* enter SUSPEND1 mode */
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-- 
2.27.0

[PATCH net-next v2 2/6] smsc95xx: use usbnet->driver_priv

[Andre Edich]

Using `void *driver_priv` instead of `unsigned long data[]` is more
straightforward way to recover the `struct smsc95xx_priv *` from the
`struct net_device *`.

Signed-off-by: Andre Edich <andre.edich@microchip.com>
---
 drivers/net/usb/smsc95xx.c | 61 +++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 3fdf7c2b2d25..f200684875fb 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -457,7 +457,7 @@ static unsigned int smsc95xx_hash(char addr[ETH_ALEN])
 static void smsc95xx_set_multicast(struct net_device *netdev)
 {
 	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	unsigned long flags;
 	int ret;
 
@@ -552,7 +552,7 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
 
 static int smsc95xx_link_reset(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	struct mii_if_info *mii = &dev->mii;
 	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
 	unsigned long flags;
@@ -620,7 +620,7 @@ static void smsc95xx_status(struct usbnet *dev, struct urb *urb)
 
 static void set_carrier(struct usbnet *dev, bool link)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 
 	if (pdata->link_ok == link)
 		return;
@@ -749,7 +749,7 @@ static void smsc95xx_ethtool_get_wol(struct net_device *net,
 				     struct ethtool_wolinfo *wolinfo)
 {
 	struct usbnet *dev = netdev_priv(net);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 
 	wolinfo->supported = SUPPORTED_WAKE;
 	wolinfo->wolopts = pdata->wolopts;
@@ -759,7 +759,7 @@ static int smsc95xx_ethtool_set_wol(struct net_device *net,
 				    struct ethtool_wolinfo *wolinfo)
 {
 	struct usbnet *dev = netdev_priv(net);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	int ret;
 
 	if (wolinfo->wolopts & ~SUPPORTED_WAKE)
@@ -798,7 +798,7 @@ static int get_mdix_status(struct net_device *net)
 static void set_mdix_status(struct net_device *net, __u8 mdix_ctrl)
 {
 	struct usbnet *dev = netdev_priv(net);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	int buf;
 
 	if ((pdata->chip_id == ID_REV_CHIP_ID_9500A_) ||
@@ -847,7 +847,7 @@ static int smsc95xx_get_link_ksettings(struct net_device *net,
 				       struct ethtool_link_ksettings *cmd)
 {
 	struct usbnet *dev = netdev_priv(net);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	int retval;
 
 	retval = usbnet_get_link_ksettings(net, cmd);
@@ -862,7 +862,7 @@ static int smsc95xx_set_link_ksettings(struct net_device *net,
 				       const struct ethtool_link_ksettings *cmd)
 {
 	struct usbnet *dev = netdev_priv(net);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	int retval;
 
 	if (pdata->mdix_ctrl != cmd->base.eth_tp_mdix_ctrl)
@@ -944,7 +944,7 @@ static int smsc95xx_set_mac_address(struct usbnet *dev)
 /* starts the TX path */
 static int smsc95xx_start_tx_path(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	unsigned long flags;
 	int ret;
 
@@ -964,7 +964,7 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
 /* Starts the Receive path */
 static int smsc95xx_start_rx_path(struct usbnet *dev, int in_pm)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	unsigned long flags;
 
 	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
@@ -1021,7 +1021,7 @@ static int smsc95xx_phy_initialize(struct usbnet *dev)
 
 static int smsc95xx_reset(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	u32 read_buf, write_buf, burst_cap;
 	int ret = 0, timeout;
 
@@ -1249,7 +1249,7 @@ static const struct net_device_ops smsc95xx_netdev_ops = {
 
 static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct smsc95xx_priv *pdata = NULL;
+	struct smsc95xx_priv *pdata;
 	u32 val;
 	int ret;
 
@@ -1261,13 +1261,12 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 		return ret;
 	}
 
-	dev->data[0] = (unsigned long)kzalloc(sizeof(struct smsc95xx_priv),
-					      GFP_KERNEL);
-
-	pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return -ENOMEM;
 
+	dev->driver_priv = pdata;
+
 	spin_lock_init(&pdata->mac_cr_lock);
 
 	/* LAN95xx devices do not alter the computed checksum of 0 to 0xffff.
@@ -1330,15 +1329,11 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 
 static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
-	if (pdata) {
-		cancel_delayed_work_sync(&pdata->carrier_check);
-		netif_dbg(dev, ifdown, dev->net, "free pdata\n");
-		kfree(pdata);
-		pdata = NULL;
-		dev->data[0] = 0;
-	}
+	struct smsc95xx_priv *pdata = dev->driver_priv;
+
+	cancel_delayed_work_sync(&pdata->carrier_check);
+	netif_dbg(dev, ifdown, dev->net, "free pdata\n");
+	kfree(pdata);
 }
 
 static u32 smsc_crc(const u8 *buffer, size_t len, int filter)
@@ -1388,7 +1383,7 @@ static int smsc95xx_link_ok_nopm(struct usbnet *dev)
 
 static int smsc95xx_enter_suspend0(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	u32 val;
 	int ret;
 
@@ -1427,7 +1422,7 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
 
 static int smsc95xx_enter_suspend1(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	u32 val;
 	int ret;
 
@@ -1474,7 +1469,7 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
 
 static int smsc95xx_enter_suspend2(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	u32 val;
 	int ret;
 
@@ -1496,7 +1491,7 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
 
 static int smsc95xx_enter_suspend3(struct usbnet *dev)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	u32 val;
 	int ret;
 
@@ -1535,7 +1530,7 @@ static int smsc95xx_enter_suspend3(struct usbnet *dev)
 
 static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	int ret;
 
 	if (!netif_running(dev->net)) {
@@ -1583,7 +1578,7 @@ static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
 static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	u32 val, link_up;
 	int ret;
 
@@ -1854,7 +1849,7 @@ static int smsc95xx_resume(struct usb_interface *intf)
 	u32 val;
 
 	BUG_ON(!dev);
-	pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	pdata = dev->driver_priv;
 	suspend_flags = pdata->suspend_flags;
 
 	netdev_dbg(dev->net, "resume suspend_flags=0x%02x\n", suspend_flags);
@@ -2074,7 +2069,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 
 static int smsc95xx_manage_power(struct usbnet *dev, int on)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 
 	dev->intf->needs_remote_wakeup = on;
 
-- 
2.27.0

Re: [PATCH net-next v2 2/6] smsc95xx: use usbnet->driver_priv

[Andrew Lunn]

On Thu, Jul 23, 2020 at 01:55:03PM +0200, Andre Edich wrote:
> Using `void *driver_priv` instead of `unsigned long data[]` is more
> straightforward way to recover the `struct smsc95xx_priv *` from the
> `struct net_device *`.
> 
> Signed-off-by: Andre Edich <andre.edich@microchip.com>

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

    Andrew

[PATCH net-next v2 3/6] smsc95xx: add PAL support to use external PHY drivers

[Andre Edich]

Generally, each PHY has their own configuration and it can be done
through an external PHY driver.  The smsc95xx driver uses only the
hard-coded internal PHY configuration.

This patch adds PAL (PHY Abstraction Layer) support to probe external
PHY drivers for configuring external PHYs.

Signed-off-by: Andre Edich <andre.edich@microchip.com>
---
 drivers/net/usb/smsc95xx.c | 201 ++++++++++++++++++++++++-------------
 1 file changed, 132 insertions(+), 69 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index f200684875fb..9d2710f6d396 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -18,6 +18,8 @@
 #include <linux/usb/usbnet.h>
 #include <linux/slab.h>
 #include <linux/of_net.h>
+#include <linux/mdio.h>
+#include <linux/phy.h>
 #include "smsc95xx.h"
 
 #define SMSC_CHIPNAME			"smsc95xx"
@@ -64,6 +66,9 @@ struct smsc95xx_priv {
 	bool link_ok;
 	struct delayed_work carrier_check;
 	struct usbnet *dev;
+	struct mii_bus *mdiobus;
+	struct phy_device *phydev;
+	bool internal_phy;
 };
 
 static bool turbo_mode = true;
@@ -286,6 +291,22 @@ static void smsc95xx_mdio_write(struct net_device *netdev, int phy_id, int idx,
 	__smsc95xx_mdio_write(netdev, phy_id, idx, regval, 0);
 }
 
+static int smsc95xx_mdiobus_read(struct mii_bus *bus, int phy_id, int idx)
+{
+	struct usbnet *dev = bus->priv;
+
+	return __smsc95xx_mdio_read(dev->net, phy_id, idx, 0);
+}
+
+static int smsc95xx_mdiobus_write(struct mii_bus *bus, int phy_id, int idx,
+				  u16 regval)
+{
+	struct usbnet *dev = bus->priv;
+
+	__smsc95xx_mdio_write(dev->net, phy_id, idx, regval, 0);
+	return 0;
+}
+
 static int __must_check smsc95xx_wait_eeprom(struct usbnet *dev)
 {
 	unsigned long start_time = jiffies;
@@ -559,15 +580,20 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 	u16 lcladv, rmtadv;
 	int ret;
 
-	/* clear interrupt status */
-	ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
-	if (ret < 0)
-		return ret;
-
 	ret = smsc95xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL_);
 	if (ret < 0)
 		return ret;
 
+	if (pdata->internal_phy) {
+		/* clear interrupt status */
+		ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
+		if (ret < 0)
+			return ret;
+
+		smsc95xx_mdio_write(dev->net, mii->phy_id, PHY_INT_MASK,
+				    PHY_INT_MASK_DEFAULT_);
+	}
+
 	mii_check_media(mii, 1, 1);
 	mii_ethtool_gset(&dev->mii, &ecmd);
 	lcladv = smsc95xx_mdio_read(dev->net, mii->phy_id, MII_ADVERTISE);
@@ -851,10 +877,10 @@ static int smsc95xx_get_link_ksettings(struct net_device *net,
 	int retval;
 
 	retval = usbnet_get_link_ksettings(net, cmd);
-
-	cmd->base.eth_tp_mdix = pdata->mdix_ctrl;
-	cmd->base.eth_tp_mdix_ctrl = pdata->mdix_ctrl;
-
+	if (pdata->internal_phy) {
+		cmd->base.eth_tp_mdix = pdata->mdix_ctrl;
+		cmd->base.eth_tp_mdix_ctrl = pdata->mdix_ctrl;
+	}
 	return retval;
 }
 
@@ -863,14 +889,12 @@ static int smsc95xx_set_link_ksettings(struct net_device *net,
 {
 	struct usbnet *dev = netdev_priv(net);
 	struct smsc95xx_priv *pdata = dev->driver_priv;
-	int retval;
-
-	if (pdata->mdix_ctrl != cmd->base.eth_tp_mdix_ctrl)
-		set_mdix_status(net, cmd->base.eth_tp_mdix_ctrl);
+	u8 mdix_ctrl = cmd->base.eth_tp_mdix_ctrl;
 
-	retval = usbnet_set_link_ksettings(net, cmd);
+	if (pdata->mdix_ctrl != mdix_ctrl && pdata->internal_phy)
+		set_mdix_status(net, mdix_ctrl);
 
-	return retval;
+	return usbnet_set_link_ksettings(net, cmd);
 }
 
 static const struct ethtool_ops smsc95xx_ethtool_ops = {
@@ -974,51 +998,6 @@ static int smsc95xx_start_rx_path(struct usbnet *dev, int in_pm)
 	return __smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr, in_pm);
 }
 
-static int smsc95xx_phy_initialize(struct usbnet *dev)
-{
-	int bmcr, ret, timeout = 0;
-
-	/* Initialize MII structure */
-	dev->mii.dev = dev->net;
-	dev->mii.mdio_read = smsc95xx_mdio_read;
-	dev->mii.mdio_write = smsc95xx_mdio_write;
-	dev->mii.phy_id_mask = 0x1f;
-	dev->mii.reg_num_mask = 0x1f;
-	dev->mii.phy_id = SMSC95XX_INTERNAL_PHY_ID;
-
-	/* reset phy and wait for reset to complete */
-	smsc95xx_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
-
-	do {
-		msleep(10);
-		bmcr = smsc95xx_mdio_read(dev->net, dev->mii.phy_id, MII_BMCR);
-		timeout++;
-	} while ((bmcr & BMCR_RESET) && (timeout < 100));
-
-	if (timeout >= 100) {
-		netdev_warn(dev->net, "timeout on PHY Reset");
-		return -EIO;
-	}
-
-	smsc95xx_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
-		ADVERTISE_ALL | ADVERTISE_CSMA | ADVERTISE_PAUSE_CAP |
-		ADVERTISE_PAUSE_ASYM);
-
-	/* read to clear */
-	ret = smsc95xx_mdio_read(dev->net, dev->mii.phy_id, PHY_INT_SRC);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to read PHY_INT_SRC during init\n");
-		return ret;
-	}
-
-	smsc95xx_mdio_write(dev->net, dev->mii.phy_id, PHY_INT_MASK,
-		PHY_INT_MASK_DEFAULT_);
-	mii_nway_restart(&dev->mii);
-
-	netif_dbg(dev, ifup, dev->net, "phy initialised successfully\n");
-	return 0;
-}
-
 static int smsc95xx_reset(struct usbnet *dev)
 {
 	struct smsc95xx_priv *pdata = dev->driver_priv;
@@ -1200,12 +1179,6 @@ static int smsc95xx_reset(struct usbnet *dev)
 
 	smsc95xx_set_multicast(dev->net);
 
-	ret = smsc95xx_phy_initialize(dev);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to init PHY\n");
-		return ret;
-	}
-
 	ret = smsc95xx_read_reg(dev, INT_EP_CTL, &read_buf);
 	if (ret < 0)
 		return ret;
@@ -1291,14 +1264,59 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	if (ret)
 		goto free_pdata;
 
+	pdata->mdiobus = mdiobus_alloc();
+	if (!pdata->mdiobus) {
+		ret = -ENOMEM;
+		goto free_pdata;
+	}
+
+	ret = smsc95xx_read_reg(dev, HW_CFG, &val);
+	if (ret < 0)
+		goto free_mdio;
+
+	pdata->internal_phy = !(val & HW_CFG_PSEL_);
+	if (pdata->internal_phy)
+		pdata->mdiobus->phy_mask = ~(1u << SMSC95XX_INTERNAL_PHY_ID);
+
+	pdata->mdiobus->priv = dev;
+	pdata->mdiobus->read = smsc95xx_mdiobus_read;
+	pdata->mdiobus->write = smsc95xx_mdiobus_write;
+	pdata->mdiobus->name = "smsc95xx-mdiobus";
+	pdata->mdiobus->parent = &dev->udev->dev;
+
+	dev->mii.phy_id_mask = 0x1f;
+	dev->mii.reg_num_mask = 0x1f;
+
+	snprintf(pdata->mdiobus->id, ARRAY_SIZE(pdata->mdiobus->id),
+		 "usb-%03d:%03d", dev->udev->bus->busnum, dev->udev->devnum);
+
+	ret = mdiobus_register(pdata->mdiobus);
+	if (ret) {
+		netdev_err(dev->net, "Could not register MDIO bus\n");
+		goto free_mdio;
+	}
+
+	pdata->phydev = phy_find_first(pdata->mdiobus);
+	if (!pdata->phydev) {
+		netdev_err(dev->net, "no PHY found\n");
+		ret = -ENODEV;
+		goto unregister_mdio;
+	}
+
+	dev->mii.dev = dev->net;
+	dev->mii.mdio_read = smsc95xx_mdio_read;
+	dev->mii.mdio_write = smsc95xx_mdio_write;
+	dev->mii.phy_id = pdata->phydev->mdio.addr;
+
 	/* detect device revision as different features may be available */
 	ret = smsc95xx_read_reg(dev, ID_REV, &val);
 	if (ret < 0)
-		goto free_pdata;
+		goto unregister_mdio;
 
 	val >>= 16;
 	pdata->chip_id = val;
-	pdata->mdix_ctrl = get_mdix_status(dev->net);
+	if (pdata->internal_phy)
+		pdata->mdix_ctrl = get_mdix_status(dev->net);
 
 	if ((val == ID_REV_CHIP_ID_9500A_) || (val == ID_REV_CHIP_ID_9530_) ||
 	    (val == ID_REV_CHIP_ID_89530_) || (val == ID_REV_CHIP_ID_9730_))
@@ -1322,6 +1340,12 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	return 0;
 
+unregister_mdio:
+	mdiobus_unregister(pdata->mdiobus);
+
+free_mdio:
+	mdiobus_free(pdata->mdiobus);
+
 free_pdata:
 	kfree(pdata);
 	return ret;
@@ -1332,10 +1356,47 @@ static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
 	struct smsc95xx_priv *pdata = dev->driver_priv;
 
 	cancel_delayed_work_sync(&pdata->carrier_check);
+	mdiobus_unregister(pdata->mdiobus);
+	mdiobus_free(pdata->mdiobus);
 	netif_dbg(dev, ifdown, dev->net, "free pdata\n");
 	kfree(pdata);
 }
 
+static void smsc95xx_handle_link_change(struct net_device *net)
+{
+	phy_print_status(net->phydev);
+}
+
+static int smsc95xx_start_phy(struct usbnet *dev)
+{
+	struct smsc95xx_priv *pdata = dev->driver_priv;
+	struct net_device *net = dev->net;
+	int ret;
+
+	ret = smsc95xx_reset(dev);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_connect_direct(net, pdata->phydev,
+				 &smsc95xx_handle_link_change,
+				 PHY_INTERFACE_MODE_MII);
+	if (ret) {
+		netdev_err(net, "can't attach PHY to %s\n", pdata->mdiobus->id);
+		return ret;
+	}
+
+	phy_attached_info(net->phydev);
+	phy_start(net->phydev);
+	return 0;
+}
+
+static int smsc95xx_disconnect_phy(struct usbnet *dev)
+{
+	phy_stop(dev->net->phydev);
+	phy_disconnect(dev->net->phydev);
+	return 0;
+}
+
 static u32 smsc_crc(const u8 *buffer, size_t len, int filter)
 {
 	u32 crc = bitrev16(crc16(0xFFFF, buffer, len));
@@ -1887,6 +1948,7 @@ static int smsc95xx_resume(struct usb_interface *intf)
 	if (ret < 0)
 		netdev_warn(dev->net, "usbnet_resume error\n");
 
+	phy_init_hw(pdata->phydev);
 	return ret;
 }
 
@@ -2092,7 +2154,8 @@ static const struct driver_info smsc95xx_info = {
 	.bind		= smsc95xx_bind,
 	.unbind		= smsc95xx_unbind,
 	.link_reset	= smsc95xx_link_reset,
-	.reset		= smsc95xx_reset,
+	.reset		= smsc95xx_start_phy,
+	.stop		= smsc95xx_disconnect_phy,
 	.rx_fixup	= smsc95xx_rx_fixup,
 	.tx_fixup	= smsc95xx_tx_fixup,
 	.status		= smsc95xx_status,
-- 
2.27.0

Re: [PATCH net-next v2 3/6] smsc95xx: add PAL support to use external PHY drivers

[Andrew Lunn]

On Thu, Jul 23, 2020 at 01:55:04PM +0200, Andre Edich wrote:
> Generally, each PHY has their own configuration and it can be done
> through an external PHY driver.  The smsc95xx driver uses only the
> hard-coded internal PHY configuration.
> 
> This patch adds PAL (PHY Abstraction Layer) support to probe external
> PHY drivers for configuring external PHYs.

Hi Andre

We call it phylib, not PAL.

>  static int __must_check smsc95xx_wait_eeprom(struct usbnet *dev)
>  {
>  	unsigned long start_time = jiffies;
> @@ -559,15 +580,20 @@ static int smsc95xx_link_reset(struct usbnet *dev)
>  	u16 lcladv, rmtadv;
>  	int ret;
>  
> -	/* clear interrupt status */
> -	ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
> -	if (ret < 0)
> -		return ret;
> -
>  	ret = smsc95xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL_);
>  	if (ret < 0)
>  		return ret;
>  
> +	if (pdata->internal_phy) {
> +		/* clear interrupt status */
> +		ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
> +		if (ret < 0)
> +			return ret;
> +
> +		smsc95xx_mdio_write(dev->net, mii->phy_id, PHY_INT_MASK,
> +				    PHY_INT_MASK_DEFAULT_);
> +	}

The PHY driver should do this, not the MAC driver.

Which PHY driver is used for the internal PHY? In theory, you should
not need to know if it is internal or external, it is just a PHY. That
might mean you need to move some code from this driver into the PHY
driver, if it is currently missing in the PHY driver.

> +
>  	mii_check_media(mii, 1, 1);
>  	mii_ethtool_gset(&dev->mii, &ecmd);
>  	lcladv = smsc95xx_mdio_read(dev->net, mii->phy_id, MII_ADVERTISE);
> @@ -851,10 +877,10 @@ static int smsc95xx_get_link_ksettings(struct net_device *net,
>  	int retval;
>  
>  	retval = usbnet_get_link_ksettings(net, cmd);
> -
> -	cmd->base.eth_tp_mdix = pdata->mdix_ctrl;
> -	cmd->base.eth_tp_mdix_ctrl = pdata->mdix_ctrl;
> -
> +	if (pdata->internal_phy) {
> +		cmd->base.eth_tp_mdix = pdata->mdix_ctrl;
> +		cmd->base.eth_tp_mdix_ctrl = pdata->mdix_ctrl;
> +	}

Again, they PHY driver should take care of this. You need to set
phydev->mdix_ctrl before starting the PHY. The PHY driver should set
phdev->mdix to the current status. 

> +static void smsc95xx_handle_link_change(struct net_device *net)
> +{
> +	phy_print_status(net->phydev);

So the MAC does not care about the speed? The pause configuration?
Duplex?

	Andrew

Re: [PATCH net-next v2 3/6] smsc95xx: add PAL support to use external PHY drivers

[Andre.Edich]

On Fri, 2020-07-24 at 00:39 +0200, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Thu, Jul 23, 2020 at 01:55:04PM +0200, Andre Edich wrote:
> > Generally, each PHY has their own configuration and it can be done
> > through an external PHY driver.  The smsc95xx driver uses only the
> > hard-coded internal PHY configuration.
> > 
> > This patch adds PAL (PHY Abstraction Layer) support to probe
> > external
> > PHY drivers for configuring external PHYs.
> 
> Hi Andre
> 
> We call it phylib, not PAL.

Hi Andrew,

thank you for the feedback. In the next version, I will correct these
wordings as well.

> 
> >  static int __must_check smsc95xx_wait_eeprom(struct usbnet *dev)
> >  {
> >       unsigned long start_time = jiffies;
> > @@ -559,15 +580,20 @@ static int smsc95xx_link_reset(struct usbnet
> > *dev)
> >       u16 lcladv, rmtadv;
> >       int ret;
> > 
> > -     /* clear interrupt status */
> > -     ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
> > -     if (ret < 0)
> > -             return ret;
> > -
> >       ret = smsc95xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL_);
> >       if (ret < 0)
> >               return ret;
> > 
> > +     if (pdata->internal_phy) {
> > +             /* clear interrupt status */
> > +             ret = smsc95xx_mdio_read(dev->net, mii->phy_id,
> > PHY_INT_SRC);
> > +             if (ret < 0)
> > +                     return ret;
> > +
> > +             smsc95xx_mdio_write(dev->net, mii->phy_id,
> > PHY_INT_MASK,
> > +                                 PHY_INT_MASK_DEFAULT_);
> > +     }
> 
> The PHY driver should do this, not the MAC driver.
> 
> Which PHY driver is used for the internal PHY? In theory, you should
> not need to know if it is internal or external, it is just a PHY. 

Yes sure, you are right.  I see the drivers/net/phy/smsc.c that is
probed for the internal PHY of the DUT's Ethernet controller.

> 
> That
> might mean you need to move some code from this driver into the PHY
> driver, if it is currently missing in the PHY driver.

Correct, the PHY driver does interrupt setup activities, so that they
can be removed from the smsc95xx.
 
> 
> > +
> >       mii_check_media(mii, 1, 1);
> >       mii_ethtool_gset(&dev->mii, &ecmd);
> >       lcladv = smsc95xx_mdio_read(dev->net, mii->phy_id,
> > MII_ADVERTISE);
> > @@ -851,10 +877,10 @@ static int smsc95xx_get_link_ksettings(struct
> > net_device *net,
> >       int retval;
> > 
> >       retval = usbnet_get_link_ksettings(net, cmd);
> > -
> > -     cmd->base.eth_tp_mdix = pdata->mdix_ctrl;
> > -     cmd->base.eth_tp_mdix_ctrl = pdata->mdix_ctrl;
> > -
> > +     if (pdata->internal_phy) {
> > +             cmd->base.eth_tp_mdix = pdata->mdix_ctrl;
> > +             cmd->base.eth_tp_mdix_ctrl = pdata->mdix_ctrl;
> > +     }
> 
> Again, they PHY driver should take care of this. You need to set
> phydev->mdix_ctrl before starting the PHY. The PHY driver should set
> phdev->mdix to the current status.

The SMSC Phy driver does not have any MDIX setup code, but I think I've
got the idea now.

> 
> > +static void smsc95xx_handle_link_change(struct net_device *net)
> > +{
> > +     phy_print_status(net->phydev);
> 
> So the MAC does not care about the speed? The pause configuration?
> Duplex?

Now, I'm wondering how those "care about speed", "pause", and "duplex"
work in the current smsc95xx.  I guess, we did not touch any of those
activities with our patches.

Thanks a lot.
Andre

> 
>         Andrew

Re: [PATCH net-next v2 3/6] smsc95xx: add PAL support to use external PHY drivers

[Andrew Lunn]

> > > +static void smsc95xx_handle_link_change(struct net_device *net)
> > > +{
> > > +     phy_print_status(net->phydev);
> > 
> > So the MAC does not care about the speed? The pause configuration?
> > Duplex?
> 
> Now, I'm wondering how those "care about speed", "pause", and "duplex"
> work in the current smsc95xx.  I guess, we did not touch any of those
> activities with our patches.

Yes, this patchset itself is not necessarily wrong. It seems more like
the driver could be broken with respect to these things. It is
something you might want to put on your TODO list to look at later.

	  Andrew

Re: [PATCH net-next v2 3/6] smsc95xx: add PAL support to use external PHY drivers

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8305 bytes --]

Hi Andre,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Andre-Edich/Add-PAL-support-to-smsc95xx/20200723-195824
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7fc3b978a8971305d456b32d3f2ac13191f5a0d7
config: x86_64-randconfig-a016-20200724 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_disconnect_phy':
>> drivers/net/usb/smsc95xx.c:1395: undefined reference to `phy_stop'
>> ld: drivers/net/usb/smsc95xx.c:1396: undefined reference to `phy_disconnect'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_unbind':
>> drivers/net/usb/smsc95xx.c:1359: undefined reference to `mdiobus_unregister'
>> ld: drivers/net/usb/smsc95xx.c:1360: undefined reference to `mdiobus_free'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_start_phy':
>> drivers/net/usb/smsc95xx.c:1380: undefined reference to `phy_connect_direct'
>> ld: drivers/net/usb/smsc95xx.c:1388: undefined reference to `phy_attached_info'
>> ld: drivers/net/usb/smsc95xx.c:1389: undefined reference to `phy_start'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_resume':
>> drivers/net/usb/smsc95xx.c:1951: undefined reference to `phy_init_hw'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_handle_link_change':
>> drivers/net/usb/smsc95xx.c:1367: undefined reference to `phy_print_status'
   ld: drivers/net/usb/smsc95xx.o: in function `mdiobus_alloc':
>> include/linux/phy.h:318: undefined reference to `mdiobus_alloc_size'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_bind':
>> drivers/net/usb/smsc95xx.c:1293: undefined reference to `__mdiobus_register'
>> ld: drivers/net/usb/smsc95xx.c:1299: undefined reference to `phy_find_first'
   ld: drivers/net/usb/smsc95xx.c:1344: undefined reference to `mdiobus_unregister'
   ld: drivers/net/usb/smsc95xx.c:1347: undefined reference to `mdiobus_free'

vim +1395 drivers/net/usb/smsc95xx.c

  1222	
  1223	static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
  1224	{
  1225		struct smsc95xx_priv *pdata;
  1226		u32 val;
  1227		int ret;
  1228	
  1229		printk(KERN_INFO SMSC_CHIPNAME " v" SMSC_DRIVER_VERSION "\n");
  1230	
  1231		ret = usbnet_get_endpoints(dev, intf);
  1232		if (ret < 0) {
  1233			netdev_warn(dev->net, "usbnet_get_endpoints failed: %d\n", ret);
  1234			return ret;
  1235		}
  1236	
  1237		pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
  1238		if (!pdata)
  1239			return -ENOMEM;
  1240	
  1241		dev->driver_priv = pdata;
  1242	
  1243		spin_lock_init(&pdata->mac_cr_lock);
  1244	
  1245		/* LAN95xx devices do not alter the computed checksum of 0 to 0xffff.
  1246		 * RFC 2460, ipv6 UDP calculated checksum yields a result of zero must
  1247		 * be changed to 0xffff. RFC 768, ipv4 UDP computed checksum is zero,
  1248		 * it is transmitted as all ones. The zero transmitted checksum means
  1249		 * transmitter generated no checksum. Hence, enable csum offload only
  1250		 * for ipv4 packets.
  1251		 */
  1252		if (DEFAULT_TX_CSUM_ENABLE)
  1253			dev->net->features |= NETIF_F_IP_CSUM;
  1254		if (DEFAULT_RX_CSUM_ENABLE)
  1255			dev->net->features |= NETIF_F_RXCSUM;
  1256	
  1257		dev->net->hw_features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM;
  1258		set_bit(EVENT_NO_IP_ALIGN, &dev->flags);
  1259	
  1260		smsc95xx_init_mac_address(dev);
  1261	
  1262		/* Init all registers */
  1263		ret = smsc95xx_reset(dev);
  1264		if (ret)
  1265			goto free_pdata;
  1266	
  1267		pdata->mdiobus = mdiobus_alloc();
  1268		if (!pdata->mdiobus) {
  1269			ret = -ENOMEM;
  1270			goto free_pdata;
  1271		}
  1272	
  1273		ret = smsc95xx_read_reg(dev, HW_CFG, &val);
  1274		if (ret < 0)
  1275			goto free_mdio;
  1276	
  1277		pdata->internal_phy = !(val & HW_CFG_PSEL_);
  1278		if (pdata->internal_phy)
  1279			pdata->mdiobus->phy_mask = ~(1u << SMSC95XX_INTERNAL_PHY_ID);
  1280	
  1281		pdata->mdiobus->priv = dev;
  1282		pdata->mdiobus->read = smsc95xx_mdiobus_read;
  1283		pdata->mdiobus->write = smsc95xx_mdiobus_write;
  1284		pdata->mdiobus->name = "smsc95xx-mdiobus";
  1285		pdata->mdiobus->parent = &dev->udev->dev;
  1286	
  1287		dev->mii.phy_id_mask = 0x1f;
  1288		dev->mii.reg_num_mask = 0x1f;
  1289	
  1290		snprintf(pdata->mdiobus->id, ARRAY_SIZE(pdata->mdiobus->id),
  1291			 "usb-%03d:%03d", dev->udev->bus->busnum, dev->udev->devnum);
  1292	
> 1293		ret = mdiobus_register(pdata->mdiobus);
  1294		if (ret) {
  1295			netdev_err(dev->net, "Could not register MDIO bus\n");
  1296			goto free_mdio;
  1297		}
  1298	
> 1299		pdata->phydev = phy_find_first(pdata->mdiobus);
  1300		if (!pdata->phydev) {
  1301			netdev_err(dev->net, "no PHY found\n");
  1302			ret = -ENODEV;
  1303			goto unregister_mdio;
  1304		}
  1305	
  1306		dev->mii.dev = dev->net;
  1307		dev->mii.mdio_read = smsc95xx_mdio_read;
  1308		dev->mii.mdio_write = smsc95xx_mdio_write;
  1309		dev->mii.phy_id = pdata->phydev->mdio.addr;
  1310	
  1311		/* detect device revision as different features may be available */
  1312		ret = smsc95xx_read_reg(dev, ID_REV, &val);
  1313		if (ret < 0)
  1314			goto unregister_mdio;
  1315	
  1316		val >>= 16;
  1317		pdata->chip_id = val;
  1318		if (pdata->internal_phy)
  1319			pdata->mdix_ctrl = get_mdix_status(dev->net);
  1320	
  1321		if ((val == ID_REV_CHIP_ID_9500A_) || (val == ID_REV_CHIP_ID_9530_) ||
  1322		    (val == ID_REV_CHIP_ID_89530_) || (val == ID_REV_CHIP_ID_9730_))
  1323			pdata->features = (FEATURE_8_WAKEUP_FILTERS |
  1324				FEATURE_PHY_NLP_CROSSOVER |
  1325				FEATURE_REMOTE_WAKEUP);
  1326		else if (val == ID_REV_CHIP_ID_9512_)
  1327			pdata->features = FEATURE_8_WAKEUP_FILTERS;
  1328	
  1329		dev->net->netdev_ops = &smsc95xx_netdev_ops;
  1330		dev->net->ethtool_ops = &smsc95xx_ethtool_ops;
  1331		dev->net->flags |= IFF_MULTICAST;
  1332		dev->net->hard_header_len += SMSC95XX_TX_OVERHEAD_CSUM;
  1333		dev->net->min_mtu = ETH_MIN_MTU;
  1334		dev->net->max_mtu = ETH_DATA_LEN;
  1335		dev->hard_mtu = dev->net->mtu + dev->net->hard_header_len;
  1336	
  1337		pdata->dev = dev;
  1338		INIT_DELAYED_WORK(&pdata->carrier_check, check_carrier);
  1339		schedule_delayed_work(&pdata->carrier_check, CARRIER_CHECK_DELAY);
  1340	
  1341		return 0;
  1342	
  1343	unregister_mdio:
  1344		mdiobus_unregister(pdata->mdiobus);
  1345	
  1346	free_mdio:
  1347		mdiobus_free(pdata->mdiobus);
  1348	
  1349	free_pdata:
  1350		kfree(pdata);
  1351		return ret;
  1352	}
  1353	
  1354	static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
  1355	{
  1356		struct smsc95xx_priv *pdata = dev->driver_priv;
  1357	
  1358		cancel_delayed_work_sync(&pdata->carrier_check);
> 1359		mdiobus_unregister(pdata->mdiobus);
> 1360		mdiobus_free(pdata->mdiobus);
  1361		netif_dbg(dev, ifdown, dev->net, "free pdata\n");
  1362		kfree(pdata);
  1363	}
  1364	
  1365	static void smsc95xx_handle_link_change(struct net_device *net)
  1366	{
> 1367		phy_print_status(net->phydev);
  1368	}
  1369	
  1370	static int smsc95xx_start_phy(struct usbnet *dev)
  1371	{
  1372		struct smsc95xx_priv *pdata = dev->driver_priv;
  1373		struct net_device *net = dev->net;
  1374		int ret;
  1375	
  1376		ret = smsc95xx_reset(dev);
  1377		if (ret < 0)
  1378			return ret;
  1379	
> 1380		ret = phy_connect_direct(net, pdata->phydev,
  1381					 &smsc95xx_handle_link_change,
  1382					 PHY_INTERFACE_MODE_MII);
  1383		if (ret) {
  1384			netdev_err(net, "can't attach PHY to %s\n", pdata->mdiobus->id);
  1385			return ret;
  1386		}
  1387	
> 1388		phy_attached_info(net->phydev);
> 1389		phy_start(net->phydev);
  1390		return 0;
  1391	}
  1392	
  1393	static int smsc95xx_disconnect_phy(struct usbnet *dev)
  1394	{
> 1395		phy_stop(dev->net->phydev);
> 1396		phy_disconnect(dev->net->phydev);
  1397		return 0;
  1398	}
  1399	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38364 bytes --]

[PATCH net-next v2 4/6] smsc95xx: remove redundant link status checking

[Andre Edich]

Current driver supports PAL that does link status checking anyway.

Signed-off-by: Andre Edich <andre.edich@microchip.com>
---
 drivers/net/usb/smsc95xx.c | 55 --------------------------------------
 1 file changed, 55 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 9d2710f6d396..8731724bf2c5 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -51,8 +51,6 @@
 #define SUSPEND_ALLMODES		(SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
 					 SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
 
-#define CARRIER_CHECK_DELAY (2 * HZ)
-
 struct smsc95xx_priv {
 	u32 chip_id;
 	u32 mac_cr;
@@ -64,8 +62,6 @@ struct smsc95xx_priv {
 	u8 suspend_flags;
 	u8 mdix_ctrl;
 	bool link_ok;
-	struct delayed_work carrier_check;
-	struct usbnet *dev;
 	struct mii_bus *mdiobus;
 	struct phy_device *phydev;
 	bool internal_phy;
@@ -644,44 +640,6 @@ static void smsc95xx_status(struct usbnet *dev, struct urb *urb)
 			    intdata);
 }
 
-static void set_carrier(struct usbnet *dev, bool link)
-{
-	struct smsc95xx_priv *pdata = dev->driver_priv;
-
-	if (pdata->link_ok == link)
-		return;
-
-	pdata->link_ok = link;
-
-	if (link)
-		usbnet_link_change(dev, 1, 0);
-	else
-		usbnet_link_change(dev, 0, 0);
-}
-
-static void check_carrier(struct work_struct *work)
-{
-	struct smsc95xx_priv *pdata = container_of(work, struct smsc95xx_priv,
-						carrier_check.work);
-	struct usbnet *dev = pdata->dev;
-	int ret;
-
-	if (pdata->suspend_flags != 0)
-		return;
-
-	ret = smsc95xx_mdio_read(dev->net, dev->mii.phy_id, MII_BMSR);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to read MII_BMSR\n");
-		return;
-	}
-	if (ret & BMSR_LSTATUS)
-		set_carrier(dev, 1);
-	else
-		set_carrier(dev, 0);
-
-	schedule_delayed_work(&pdata->carrier_check, CARRIER_CHECK_DELAY);
-}
-
 /* Enable or disable Tx & Rx checksum offload engines */
 static int smsc95xx_set_features(struct net_device *netdev,
 	netdev_features_t features)
@@ -1333,11 +1291,6 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->net->min_mtu = ETH_MIN_MTU;
 	dev->net->max_mtu = ETH_DATA_LEN;
 	dev->hard_mtu = dev->net->mtu + dev->net->hard_header_len;
-
-	pdata->dev = dev;
-	INIT_DELAYED_WORK(&pdata->carrier_check, check_carrier);
-	schedule_delayed_work(&pdata->carrier_check, CARRIER_CHECK_DELAY);
-
 	return 0;
 
 unregister_mdio:
@@ -1355,7 +1308,6 @@ static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
 	struct smsc95xx_priv *pdata = dev->driver_priv;
 
-	cancel_delayed_work_sync(&pdata->carrier_check);
 	mdiobus_unregister(pdata->mdiobus);
 	mdiobus_free(pdata->mdiobus);
 	netif_dbg(dev, ifdown, dev->net, "free pdata\n");
@@ -1649,8 +1601,6 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		return ret;
 	}
 
-	cancel_delayed_work_sync(&pdata->carrier_check);
-
 	if (pdata->suspend_flags) {
 		netdev_warn(dev->net, "error during last resume\n");
 		pdata->suspend_flags = 0;
@@ -1894,10 +1844,6 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	if (ret && PMSG_IS_AUTO(message))
 		usbnet_resume(intf);
 
-	if (ret)
-		schedule_delayed_work(&pdata->carrier_check,
-				      CARRIER_CHECK_DELAY);
-
 	return ret;
 }
 
@@ -1917,7 +1863,6 @@ static int smsc95xx_resume(struct usb_interface *intf)
 
 	/* do this first to ensure it's cleared even in error case */
 	pdata->suspend_flags = 0;
-	schedule_delayed_work(&pdata->carrier_check, CARRIER_CHECK_DELAY);
 
 	if (suspend_flags & SUSPEND_ALLMODES) {
 		/* clear wake-up sources */
-- 
2.27.0

Re: [PATCH net-next v2 4/6] smsc95xx: remove redundant link status checking

[Andrew Lunn]

On Thu, Jul 23, 2020 at 01:55:05PM +0200, Andre Edich wrote:
> Current driver supports PAL that does link status checking anyway.
> 
> Signed-off-by: Andre Edich <andre.edich@microchip.com>

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

    Andrew

[PATCH net-next v2 5/6] smsc95xx: use PAL framework read/write functions

[Andre Edich]

Use functions phy_read and phy_write instead of smsc95xx_mdio_read and
smsc95xx_mdio_write respectively.

Signed-off-by: Andre Edich <andre.edich@microchip.com>
---
 drivers/net/usb/smsc95xx.c | 43 +++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 8731724bf2c5..8add7109e661 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -582,22 +582,20 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 
 	if (pdata->internal_phy) {
 		/* clear interrupt status */
-		ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
+		ret = phy_read(pdata->phydev, PHY_INT_SRC);
 		if (ret < 0)
 			return ret;
 
-		smsc95xx_mdio_write(dev->net, mii->phy_id, PHY_INT_MASK,
-				    PHY_INT_MASK_DEFAULT_);
+		ret = phy_write(pdata->phydev, PHY_INT_MASK,
+				PHY_INT_MASK_DEFAULT_);
+		if (ret < 0)
+			return ret;
 	}
 
 	mii_check_media(mii, 1, 1);
 	mii_ethtool_gset(&dev->mii, &ecmd);
-	lcladv = smsc95xx_mdio_read(dev->net, mii->phy_id, MII_ADVERTISE);
-	rmtadv = smsc95xx_mdio_read(dev->net, mii->phy_id, MII_LPA);
-
-	netif_dbg(dev, link, dev->net,
-		  "speed: %u duplex: %d lcladv: %04x rmtadv: %04x\n",
-		  ethtool_cmd_speed(&ecmd), ecmd.duplex, lcladv, rmtadv);
+	lcladv = phy_read(pdata->phydev, MII_ADVERTISE);
+	rmtadv = phy_read(pdata->phydev, MII_LPA);
 
 	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
 	if (ecmd.duplex != DUPLEX_FULL) {
@@ -761,10 +759,11 @@ static int smsc95xx_ethtool_set_wol(struct net_device *net,
 static int get_mdix_status(struct net_device *net)
 {
 	struct usbnet *dev = netdev_priv(net);
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 	u32 val;
 	int buf;
 
-	buf = smsc95xx_mdio_read(dev->net, dev->mii.phy_id, SPECIAL_CTRL_STS);
+	buf = phy_read(pdata->phydev, SPECIAL_CTRL_STS);
 	if (buf & SPECIAL_CTRL_STS_OVRRD_AMDIX_) {
 		if (buf & SPECIAL_CTRL_STS_AMDIX_ENABLE_)
 			return ETH_TP_MDI_AUTO;
@@ -790,39 +789,31 @@ static void set_mdix_status(struct net_device *net, __u8 mdix_ctrl)
 	    (pdata->chip_id == ID_REV_CHIP_ID_89530_) ||
 	    (pdata->chip_id == ID_REV_CHIP_ID_9730_)) {
 		/* Extend Manual AutoMDIX timer for 9500A/9500Ai */
-		buf = smsc95xx_mdio_read(dev->net, dev->mii.phy_id,
-					 PHY_EDPD_CONFIG);
+		buf = phy_read(pdata->phydev, PHY_EDPD_CONFIG);
 		buf |= PHY_EDPD_CONFIG_EXT_CROSSOVER_;
-		smsc95xx_mdio_write(dev->net, dev->mii.phy_id,
-				    PHY_EDPD_CONFIG, buf);
+		phy_write(pdata->phydev, PHY_EDPD_CONFIG, buf);
 	}
 
 	if (mdix_ctrl == ETH_TP_MDI) {
-		buf = smsc95xx_mdio_read(dev->net, dev->mii.phy_id,
-					 SPECIAL_CTRL_STS);
+		buf = phy_read(pdata->phydev, SPECIAL_CTRL_STS);
 		buf |= SPECIAL_CTRL_STS_OVRRD_AMDIX_;
 		buf &= ~(SPECIAL_CTRL_STS_AMDIX_ENABLE_ |
 			 SPECIAL_CTRL_STS_AMDIX_STATE_);
-		smsc95xx_mdio_write(dev->net, dev->mii.phy_id,
-				    SPECIAL_CTRL_STS, buf);
+		phy_write(pdata->phydev, SPECIAL_CTRL_STS, buf);
 	} else if (mdix_ctrl == ETH_TP_MDI_X) {
-		buf = smsc95xx_mdio_read(dev->net, dev->mii.phy_id,
-					 SPECIAL_CTRL_STS);
+		buf = phy_read(pdata->phydev, SPECIAL_CTRL_STS);
 		buf |= SPECIAL_CTRL_STS_OVRRD_AMDIX_;
 		buf &= ~(SPECIAL_CTRL_STS_AMDIX_ENABLE_ |
 			 SPECIAL_CTRL_STS_AMDIX_STATE_);
 		buf |= SPECIAL_CTRL_STS_AMDIX_STATE_;
-		smsc95xx_mdio_write(dev->net, dev->mii.phy_id,
-				    SPECIAL_CTRL_STS, buf);
+		phy_write(pdata->phydev, SPECIAL_CTRL_STS, buf);
 	} else if (mdix_ctrl == ETH_TP_MDI_AUTO) {
-		buf = smsc95xx_mdio_read(dev->net, dev->mii.phy_id,
-					 SPECIAL_CTRL_STS);
+		buf = phy_read(pdata->phydev, SPECIAL_CTRL_STS);
 		buf &= ~SPECIAL_CTRL_STS_OVRRD_AMDIX_;
 		buf &= ~(SPECIAL_CTRL_STS_AMDIX_ENABLE_ |
 			 SPECIAL_CTRL_STS_AMDIX_STATE_);
 		buf |= SPECIAL_CTRL_STS_AMDIX_ENABLE_;
-		smsc95xx_mdio_write(dev->net, dev->mii.phy_id,
-				    SPECIAL_CTRL_STS, buf);
+		phy_write(pdata->phydev, SPECIAL_CTRL_STS, buf);
 	}
 	pdata->mdix_ctrl = mdix_ctrl;
 }
-- 
2.27.0

Re: [PATCH net-next v2 5/6] smsc95xx: use PAL framework read/write functions

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8435 bytes --]

Hi Andre,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Andre-Edich/Add-PAL-support-to-smsc95xx/20200723-195824
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7fc3b978a8971305d456b32d3f2ac13191f5a0d7
config: x86_64-randconfig-a016-20200724 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_disconnect_phy':
   drivers/net/usb/smsc95xx.c:1338: undefined reference to `phy_stop'
   ld: drivers/net/usb/smsc95xx.c:1339: undefined reference to `phy_disconnect'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_unbind':
   drivers/net/usb/smsc95xx.c:1302: undefined reference to `mdiobus_unregister'
   ld: drivers/net/usb/smsc95xx.c:1303: undefined reference to `mdiobus_free'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
>> include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
>> include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
>> include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
>> include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
>> include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
>> include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
>> include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
>> include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
>> include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
>> include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
>> include/linux/phy.h:804: undefined reference to `mdiobus_read'
>> ld: include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_start_phy':
   drivers/net/usb/smsc95xx.c:1323: undefined reference to `phy_connect_direct'
   ld: drivers/net/usb/smsc95xx.c:1331: undefined reference to `phy_attached_info'
   ld: drivers/net/usb/smsc95xx.c:1332: undefined reference to `phy_start'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_resume':
   drivers/net/usb/smsc95xx.c:1887: undefined reference to `phy_init_hw'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_handle_link_change':
   drivers/net/usb/smsc95xx.c:1310: undefined reference to `phy_print_status'
   ld: drivers/net/usb/smsc95xx.o: in function `mdiobus_alloc':
   include/linux/phy.h:318: undefined reference to `mdiobus_alloc_size'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_bind':
   drivers/net/usb/smsc95xx.c:1242: undefined reference to `__mdiobus_register'
   ld: drivers/net/usb/smsc95xx.c:1248: undefined reference to `phy_find_first'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
>> include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_bind':
   drivers/net/usb/smsc95xx.c:1288: undefined reference to `mdiobus_unregister'
   ld: drivers/net/usb/smsc95xx.c:1291: undefined reference to `mdiobus_free'

vim +804 include/linux/phy.h

8c5e850c0ce597 Russell King      2017-12-29  792  
2e888103295f47 Lennert Buytenhek 2008-09-29  793  /**
2e888103295f47 Lennert Buytenhek 2008-09-29  794   * phy_read - Convenience function for reading a given PHY register
2e888103295f47 Lennert Buytenhek 2008-09-29  795   * @phydev: the phy_device struct
2e888103295f47 Lennert Buytenhek 2008-09-29  796   * @regnum: register number to read
2e888103295f47 Lennert Buytenhek 2008-09-29  797   *
2e888103295f47 Lennert Buytenhek 2008-09-29  798   * NOTE: MUST NOT be called from interrupt context,
2e888103295f47 Lennert Buytenhek 2008-09-29  799   * because the bus read/write functions may wait for an interrupt
2e888103295f47 Lennert Buytenhek 2008-09-29  800   * to conclude the operation.
2e888103295f47 Lennert Buytenhek 2008-09-29  801   */
abf35df21513c5 Jason Gunthorpe   2010-03-09  802  static inline int phy_read(struct phy_device *phydev, u32 regnum)
2e888103295f47 Lennert Buytenhek 2008-09-29  803  {
e5a03bfd873c29 Andrew Lunn       2016-01-06 @804  	return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, regnum);
2e888103295f47 Lennert Buytenhek 2008-09-29  805  }
2e888103295f47 Lennert Buytenhek 2008-09-29  806  
fcbd30d09ba053 Dejin Zheng       2020-03-23  807  #define phy_read_poll_timeout(phydev, regnum, val, cond, sleep_us, \
fcbd30d09ba053 Dejin Zheng       2020-03-23  808  				timeout_us, sleep_before_read) \
fcbd30d09ba053 Dejin Zheng       2020-03-23  809  ({ \
fcbd30d09ba053 Dejin Zheng       2020-03-23  810  	int __ret = read_poll_timeout(phy_read, val, (cond) || val < 0, \
fcbd30d09ba053 Dejin Zheng       2020-03-23  811  		sleep_us, timeout_us, sleep_before_read, phydev, regnum); \
fcbd30d09ba053 Dejin Zheng       2020-03-23  812  	if (val <  0) \
fcbd30d09ba053 Dejin Zheng       2020-03-23  813  		__ret = val; \
fcbd30d09ba053 Dejin Zheng       2020-03-23  814  	if (__ret) \
fcbd30d09ba053 Dejin Zheng       2020-03-23  815  		phydev_err(phydev, "%s failed: %d\n", __func__, __ret); \
fcbd30d09ba053 Dejin Zheng       2020-03-23  816  	__ret; \
fcbd30d09ba053 Dejin Zheng       2020-03-23  817  })
fcbd30d09ba053 Dejin Zheng       2020-03-23  818  
fcbd30d09ba053 Dejin Zheng       2020-03-23  819  
788f9933db6172 Russell King      2018-01-02  820  /**
788f9933db6172 Russell King      2018-01-02  821   * __phy_read - convenience function for reading a given PHY register
788f9933db6172 Russell King      2018-01-02  822   * @phydev: the phy_device struct
788f9933db6172 Russell King      2018-01-02  823   * @regnum: register number to read
788f9933db6172 Russell King      2018-01-02  824   *
788f9933db6172 Russell King      2018-01-02  825   * The caller must have taken the MDIO bus lock.
788f9933db6172 Russell King      2018-01-02  826   */
788f9933db6172 Russell King      2018-01-02  827  static inline int __phy_read(struct phy_device *phydev, u32 regnum)
788f9933db6172 Russell King      2018-01-02  828  {
788f9933db6172 Russell King      2018-01-02  829  	return __mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, regnum);
788f9933db6172 Russell King      2018-01-02  830  }
788f9933db6172 Russell King      2018-01-02  831  
2e888103295f47 Lennert Buytenhek 2008-09-29  832  /**
2e888103295f47 Lennert Buytenhek 2008-09-29  833   * phy_write - Convenience function for writing a given PHY register
2e888103295f47 Lennert Buytenhek 2008-09-29  834   * @phydev: the phy_device struct
2e888103295f47 Lennert Buytenhek 2008-09-29  835   * @regnum: register number to write
2e888103295f47 Lennert Buytenhek 2008-09-29  836   * @val: value to write to @regnum
2e888103295f47 Lennert Buytenhek 2008-09-29  837   *
2e888103295f47 Lennert Buytenhek 2008-09-29  838   * NOTE: MUST NOT be called from interrupt context,
2e888103295f47 Lennert Buytenhek 2008-09-29  839   * because the bus read/write functions may wait for an interrupt
2e888103295f47 Lennert Buytenhek 2008-09-29  840   * to conclude the operation.
2e888103295f47 Lennert Buytenhek 2008-09-29  841   */
abf35df21513c5 Jason Gunthorpe   2010-03-09  842  static inline int phy_write(struct phy_device *phydev, u32 regnum, u16 val)
2e888103295f47 Lennert Buytenhek 2008-09-29  843  {
e5a03bfd873c29 Andrew Lunn       2016-01-06 @844  	return mdiobus_write(phydev->mdio.bus, phydev->mdio.addr, regnum, val);
2e888103295f47 Lennert Buytenhek 2008-09-29  845  }
2e888103295f47 Lennert Buytenhek 2008-09-29  846  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38364 bytes --]

[PATCH net-next v2 6/6] smsc95xx: use PHY framework instead of MII library

[Andre Edich]

Since the PHY framework is used anyway, replace the rest of the calls to
the MII library, including those from the USB Network driver
infrastructure, by the calls to the PHY framework.

Signed-off-by: Andre Edich <andre.edich@microchip.com>
---
 drivers/net/usb/smsc95xx.c | 78 +++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 48 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 8add7109e661..7de20c0aad36 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -174,10 +174,14 @@ static int __must_check __smsc95xx_phy_wait_not_busy(struct usbnet *dev,
 	return -EIO;
 }
 
-static int __smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx,
+static u32 mii_address(u16 op, int phy_id, int idx)
+{
+	return (phy_id & 0x1f) << 11 | (idx & 0x1f) << 6 | op | MII_BUSY_;
+}
+
+static int __smsc95xx_mdio_read(struct usbnet *dev, int phy_id, int idx,
 				int in_pm)
 {
-	struct usbnet *dev = netdev_priv(netdev);
 	u32 val, addr;
 	int ret;
 
@@ -191,9 +195,7 @@ static int __smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx,
 	}
 
 	/* set the address, index & direction (read from PHY) */
-	phy_id &= dev->mii.phy_id_mask;
-	idx &= dev->mii.reg_num_mask;
-	addr = (phy_id << 11) | (idx << 6) | MII_READ_ | MII_BUSY_;
+	addr = mii_address(MII_READ_, phy_id, idx);
 	ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm);
 	if (ret < 0) {
 		netdev_warn(dev->net, "Error writing MII_ADDR\n");
@@ -219,10 +221,9 @@ static int __smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx,
 	return ret;
 }
 
-static void __smsc95xx_mdio_write(struct net_device *netdev, int phy_id,
+static void __smsc95xx_mdio_write(struct usbnet *dev, int phy_id,
 				  int idx, int regval, int in_pm)
 {
-	struct usbnet *dev = netdev_priv(netdev);
 	u32 val, addr;
 	int ret;
 
@@ -243,9 +244,7 @@ static void __smsc95xx_mdio_write(struct net_device *netdev, int phy_id,
 	}
 
 	/* set the address, index & direction (write to PHY) */
-	phy_id &= dev->mii.phy_id_mask;
-	idx &= dev->mii.reg_num_mask;
-	addr = (phy_id << 11) | (idx << 6) | MII_WRITE_ | MII_BUSY_;
+	addr = mii_address(MII_WRITE_, phy_id, idx);
 	ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm);
 	if (ret < 0) {
 		netdev_warn(dev->net, "Error writing MII_ADDR\n");
@@ -264,34 +263,23 @@ static void __smsc95xx_mdio_write(struct net_device *netdev, int phy_id,
 
 static int smsc95xx_mdio_read_nopm(struct usbnet *dev, int idx)
 {
-	struct mii_if_info *mii = &dev->mii;
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 
-	return __smsc95xx_mdio_read(dev->net, mii->phy_id, idx, 1);
+	return __smsc95xx_mdio_read(dev, pdata->phydev->mdio.addr, idx, 1);
 }
 
 static void smsc95xx_mdio_write_nopm(struct usbnet *dev, int idx, int regval)
 {
-	struct mii_if_info *mii = &dev->mii;
-
-	__smsc95xx_mdio_write(dev->net, mii->phy_id, idx, regval, 1);
-}
-
-static int smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx)
-{
-	return __smsc95xx_mdio_read(netdev, phy_id, idx, 0);
-}
+	struct smsc95xx_priv *pdata = dev->driver_priv;
 
-static void smsc95xx_mdio_write(struct net_device *netdev, int phy_id, int idx,
-				int regval)
-{
-	__smsc95xx_mdio_write(netdev, phy_id, idx, regval, 0);
+	__smsc95xx_mdio_write(dev, pdata->phydev->mdio.addr, idx, regval, 1);
 }
 
 static int smsc95xx_mdiobus_read(struct mii_bus *bus, int phy_id, int idx)
 {
 	struct usbnet *dev = bus->priv;
 
-	return __smsc95xx_mdio_read(dev->net, phy_id, idx, 0);
+	return __smsc95xx_mdio_read(dev, phy_id, idx, 0);
 }
 
 static int smsc95xx_mdiobus_write(struct mii_bus *bus, int phy_id, int idx,
@@ -299,7 +287,7 @@ static int smsc95xx_mdiobus_write(struct mii_bus *bus, int phy_id, int idx,
 {
 	struct usbnet *dev = bus->priv;
 
-	__smsc95xx_mdio_write(dev->net, phy_id, idx, regval, 0);
+	__smsc95xx_mdio_write(dev, phy_id, idx, regval, 0);
 	return 0;
 }
 
@@ -570,8 +558,7 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
 static int smsc95xx_link_reset(struct usbnet *dev)
 {
 	struct smsc95xx_priv *pdata = dev->driver_priv;
-	struct mii_if_info *mii = &dev->mii;
-	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
+	struct ethtool_link_ksettings cmd;
 	unsigned long flags;
 	u16 lcladv, rmtadv;
 	int ret;
@@ -592,13 +579,12 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 			return ret;
 	}
 
-	mii_check_media(mii, 1, 1);
-	mii_ethtool_gset(&dev->mii, &ecmd);
+	phy_ethtool_ksettings_get(pdata->phydev, &cmd);
 	lcladv = phy_read(pdata->phydev, MII_ADVERTISE);
 	rmtadv = phy_read(pdata->phydev, MII_LPA);
 
 	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
-	if (ecmd.duplex != DUPLEX_FULL) {
+	if (cmd.base.duplex != DUPLEX_FULL) {
 		pdata->mac_cr &= ~MAC_CR_FDPX_;
 		pdata->mac_cr |= MAC_CR_RCVOWN_;
 	} else {
@@ -611,7 +597,8 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 	if (ret < 0)
 		return ret;
 
-	ret = smsc95xx_phy_update_flowcontrol(dev, ecmd.duplex, lcladv, rmtadv);
+	ret = smsc95xx_phy_update_flowcontrol(dev, cmd.base.duplex, lcladv,
+					      rmtadv);
 	if (ret < 0)
 		netdev_warn(dev->net, "Error updating PHY flow control\n");
 
@@ -825,7 +812,7 @@ static int smsc95xx_get_link_ksettings(struct net_device *net,
 	struct smsc95xx_priv *pdata = dev->driver_priv;
 	int retval;
 
-	retval = usbnet_get_link_ksettings(net, cmd);
+	retval = phy_ethtool_get_link_ksettings(net, cmd);
 	if (pdata->internal_phy) {
 		cmd->base.eth_tp_mdix = pdata->mdix_ctrl;
 		cmd->base.eth_tp_mdix_ctrl = pdata->mdix_ctrl;
@@ -842,13 +829,18 @@ static int smsc95xx_set_link_ksettings(struct net_device *net,
 
 	if (pdata->mdix_ctrl != mdix_ctrl && pdata->internal_phy)
 		set_mdix_status(net, mdix_ctrl);
+	return phy_ethtool_set_link_ksettings(net, cmd);
+}
 
-	return usbnet_set_link_ksettings(net, cmd);
+static u32 smsc95xx_get_link(struct net_device *net)
+{
+	phy_read_status(net->phydev);
+	return net->phydev->link;
 }
 
 static const struct ethtool_ops smsc95xx_ethtool_ops = {
-	.get_link	= usbnet_get_link,
-	.nway_reset	= usbnet_nway_reset,
+	.get_link	= smsc95xx_get_link,
+	.nway_reset	= phy_ethtool_nway_reset,
 	.get_drvinfo	= usbnet_get_drvinfo,
 	.get_msglevel	= usbnet_get_msglevel,
 	.set_msglevel	= usbnet_set_msglevel,
@@ -866,12 +858,10 @@ static const struct ethtool_ops smsc95xx_ethtool_ops = {
 
 static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
 {
-	struct usbnet *dev = netdev_priv(netdev);
-
 	if (!netif_running(netdev))
 		return -EINVAL;
 
-	return generic_mii_ioctl(&dev->mii, if_mii(rq), cmd, NULL);
+	return phy_mii_ioctl(netdev->phydev, rq, cmd);
 }
 
 static void smsc95xx_init_mac_address(struct usbnet *dev)
@@ -1233,9 +1223,6 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	pdata->mdiobus->name = "smsc95xx-mdiobus";
 	pdata->mdiobus->parent = &dev->udev->dev;
 
-	dev->mii.phy_id_mask = 0x1f;
-	dev->mii.reg_num_mask = 0x1f;
-
 	snprintf(pdata->mdiobus->id, ARRAY_SIZE(pdata->mdiobus->id),
 		 "usb-%03d:%03d", dev->udev->bus->busnum, dev->udev->devnum);
 
@@ -1252,11 +1239,6 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 		goto unregister_mdio;
 	}
 
-	dev->mii.dev = dev->net;
-	dev->mii.mdio_read = smsc95xx_mdio_read;
-	dev->mii.mdio_write = smsc95xx_mdio_write;
-	dev->mii.phy_id = pdata->phydev->mdio.addr;
-
 	/* detect device revision as different features may be available */
 	ret = smsc95xx_read_reg(dev, ID_REV, &val);
 	if (ret < 0)
-- 
2.27.0

Re: [PATCH net-next v2 6/6] smsc95xx: use PHY framework instead of MII library

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7151 bytes --]

Hi Andre,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Andre-Edich/Add-PAL-support-to-smsc95xx/20200723-195824
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7fc3b978a8971305d456b32d3f2ac13191f5a0d7
config: x86_64-randconfig-a016-20200724 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_disconnect_phy':
   drivers/net/usb/smsc95xx.c:1320: undefined reference to `phy_stop'
   ld: drivers/net/usb/smsc95xx.c:1321: undefined reference to `phy_disconnect'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_unbind':
   drivers/net/usb/smsc95xx.c:1284: undefined reference to `mdiobus_unregister'
   ld: drivers/net/usb/smsc95xx.c:1285: undefined reference to `mdiobus_free'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
   include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
   include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
   include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
   include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
   include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
   include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
   include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
   include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_get_link_ksettings':
>> drivers/net/usb/smsc95xx.c:815: undefined reference to `phy_ethtool_get_link_ksettings'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read_status':
>> include/linux/phy.h:1402: undefined reference to `genphy_read_status'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
   include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_write':
   include/linux/phy.h:844: undefined reference to `mdiobus_write'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_link_reset':
>> drivers/net/usb/smsc95xx.c:582: undefined reference to `phy_ethtool_ksettings_get'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
   include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_start_phy':
   drivers/net/usb/smsc95xx.c:1305: undefined reference to `phy_connect_direct'
   ld: drivers/net/usb/smsc95xx.c:1313: undefined reference to `phy_attached_info'
   ld: drivers/net/usb/smsc95xx.c:1314: undefined reference to `phy_start'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_resume':
   drivers/net/usb/smsc95xx.c:1869: undefined reference to `phy_init_hw'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_handle_link_change':
   drivers/net/usb/smsc95xx.c:1292: undefined reference to `phy_print_status'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_set_link_ksettings':
>> drivers/net/usb/smsc95xx.c:832: undefined reference to `phy_ethtool_set_link_ksettings'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_ioctl':
>> drivers/net/usb/smsc95xx.c:864: undefined reference to `phy_mii_ioctl'
>> ld: drivers/net/usb/smsc95xx.o:(.rodata+0x22e0): undefined reference to `phy_ethtool_nway_reset'
   ld: drivers/net/usb/smsc95xx.o: in function `mdiobus_alloc':
   include/linux/phy.h:318: undefined reference to `mdiobus_alloc_size'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_bind':
   drivers/net/usb/smsc95xx.c:1229: undefined reference to `__mdiobus_register'
   ld: drivers/net/usb/smsc95xx.c:1235: undefined reference to `phy_find_first'
   ld: drivers/net/usb/smsc95xx.o: in function `phy_read':
   include/linux/phy.h:804: undefined reference to `mdiobus_read'
   ld: drivers/net/usb/smsc95xx.o: in function `smsc95xx_bind':
   drivers/net/usb/smsc95xx.c:1270: undefined reference to `mdiobus_unregister'
   ld: drivers/net/usb/smsc95xx.c:1273: undefined reference to `mdiobus_free'

vim +815 drivers/net/usb/smsc95xx.c

   807	
   808	static int smsc95xx_get_link_ksettings(struct net_device *net,
   809					       struct ethtool_link_ksettings *cmd)
   810	{
   811		struct usbnet *dev = netdev_priv(net);
   812		struct smsc95xx_priv *pdata = dev->driver_priv;
   813		int retval;
   814	
 > 815		retval = phy_ethtool_get_link_ksettings(net, cmd);
   816		if (pdata->internal_phy) {
   817			cmd->base.eth_tp_mdix = pdata->mdix_ctrl;
   818			cmd->base.eth_tp_mdix_ctrl = pdata->mdix_ctrl;
   819		}
   820		return retval;
   821	}
   822	
   823	static int smsc95xx_set_link_ksettings(struct net_device *net,
   824					       const struct ethtool_link_ksettings *cmd)
   825	{
   826		struct usbnet *dev = netdev_priv(net);
   827		struct smsc95xx_priv *pdata = dev->driver_priv;
   828		u8 mdix_ctrl = cmd->base.eth_tp_mdix_ctrl;
   829	
   830		if (pdata->mdix_ctrl != mdix_ctrl && pdata->internal_phy)
   831			set_mdix_status(net, mdix_ctrl);
 > 832		return phy_ethtool_set_link_ksettings(net, cmd);
   833	}
   834	
   835	static u32 smsc95xx_get_link(struct net_device *net)
   836	{
   837		phy_read_status(net->phydev);
   838		return net->phydev->link;
   839	}
   840	
   841	static const struct ethtool_ops smsc95xx_ethtool_ops = {
   842		.get_link	= smsc95xx_get_link,
   843		.nway_reset	= phy_ethtool_nway_reset,
   844		.get_drvinfo	= usbnet_get_drvinfo,
   845		.get_msglevel	= usbnet_get_msglevel,
   846		.set_msglevel	= usbnet_set_msglevel,
   847		.get_eeprom_len	= smsc95xx_ethtool_get_eeprom_len,
   848		.get_eeprom	= smsc95xx_ethtool_get_eeprom,
   849		.set_eeprom	= smsc95xx_ethtool_set_eeprom,
   850		.get_regs_len	= smsc95xx_ethtool_getregslen,
   851		.get_regs	= smsc95xx_ethtool_getregs,
   852		.get_wol	= smsc95xx_ethtool_get_wol,
   853		.set_wol	= smsc95xx_ethtool_set_wol,
   854		.get_link_ksettings	= smsc95xx_get_link_ksettings,
   855		.set_link_ksettings	= smsc95xx_set_link_ksettings,
   856		.get_ts_info	= ethtool_op_get_ts_info,
   857	};
   858	
   859	static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
   860	{
   861		if (!netif_running(netdev))
   862			return -EINVAL;
   863	
 > 864		return phy_mii_ioctl(netdev->phydev, rq, cmd);
   865	}
   866	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38364 bytes --]