[PATCH net-next v2 0/8] port asix ax88772 to the PHYlib

[PATCH net-next v2 0/8] port asix ax88772 to the PHYlib

[Oleksij Rempel]

changes v2:
- add Reviewed-by: Andrew Lunn <andrew@lunn.ch> to some patches
- refactor asix_read_phy_addr() and add error handling for all callers
- refactor asix_mdio_bus_read()

Port ax88772 part of asix driver to the phylib to be able to use more
advanced external PHY attached to this controller.

Oleksij Rempel (8):
  net: usb: asix: ax88772_bind: use devm_kzalloc() instead of kzalloc()
  net: usb: asix: refactor asix_read_phy_addr() and handle errors on
    return
  net: usb/phy: asix: add support for ax88772A/C PHYs
  net: usb: asix: ax88772: add phylib support
  net: usb: asix: ax88772: add generic selftest support
  net: usb: asix: add error handling for asix_mdio_* functions
  net: phy: do not print dump stack if device was removed
  usbnet: run unbind() before unregister_netdev()

 drivers/net/phy/ax88796b.c     |  74 +++++++++++++++-
 drivers/net/phy/phy.c          |   3 +
 drivers/net/usb/Kconfig        |   2 +
 drivers/net/usb/asix.h         |  13 ++-
 drivers/net/usb/asix_common.c  | 106 ++++++++++++++++------
 drivers/net/usb/asix_devices.c | 157 +++++++++++++++++++++++----------
 drivers/net/usb/ax88172a.c     |  19 ++--
 drivers/net/usb/usbnet.c       |   6 +-
 8 files changed, 286 insertions(+), 94 deletions(-)

-- 
2.29.2

[PATCH net-next v2 1/8] net: usb: asix: ax88772_bind: use devm_kzalloc() instead of kzalloc()

[Oleksij Rempel]

Make resource management easier, use devm_kzalloc().

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/asix_devices.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 19a8fafb8f04..5f767a33264e 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -746,11 +746,11 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->rx_urb_size = 2048;
 	}
 
-	dev->driver_priv = kzalloc(sizeof(struct asix_common_private), GFP_KERNEL);
-	if (!dev->driver_priv)
+	priv = devm_kzalloc(&dev->udev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
 		return -ENOMEM;
 
-	priv = dev->driver_priv;
+	dev->driver_priv = priv;
 
 	priv->presvd_phy_bmcr = 0;
 	priv->presvd_phy_advertise = 0;
@@ -768,7 +768,6 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
 	asix_rx_fixup_common_free(dev->driver_priv);
-	kfree(dev->driver_priv);
 }
 
 static const struct ethtool_ops ax88178_ethtool_ops = {
-- 
2.29.2

[PATCH net-next v2 2/8] net: usb: asix: refactor asix_read_phy_addr() and handle errors on return

[Oleksij Rempel]

Refactor asix_read_phy_addr() to return usable error value directly and
make sure all callers handle this error.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/usb/asix.h         |  3 +--
 drivers/net/usb/asix_common.c  | 31 ++++++++++++++++---------------
 drivers/net/usb/asix_devices.c | 15 ++++++++++++---
 drivers/net/usb/ax88172a.c     |  5 +++++
 4 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index 3b53685301de..edb94efd265e 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -205,8 +205,7 @@ struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
 int asix_set_sw_mii(struct usbnet *dev, int in_pm);
 int asix_set_hw_mii(struct usbnet *dev, int in_pm);
 
-int asix_read_phy_addr(struct usbnet *dev, int internal);
-int asix_get_phy_addr(struct usbnet *dev);
+int asix_read_phy_addr(struct usbnet *dev, bool internal);
 
 int asix_sw_reset(struct usbnet *dev, u8 flags, int in_pm);
 
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 7bc6e8f856fe..e1109f1a8dd5 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -288,32 +288,33 @@ int asix_set_hw_mii(struct usbnet *dev, int in_pm)
 	return ret;
 }
 
-int asix_read_phy_addr(struct usbnet *dev, int internal)
+int asix_read_phy_addr(struct usbnet *dev, bool internal)
 {
-	int offset = (internal ? 1 : 0);
+	int ret, offset;
 	u8 buf[2];
-	int ret = asix_read_cmd(dev, AX_CMD_READ_PHY_ID, 0, 0, 2, buf, 0);
 
-	netdev_dbg(dev->net, "asix_get_phy_addr()\n");
+	ret = asix_read_cmd(dev, AX_CMD_READ_PHY_ID, 0, 0, 2, buf, 0);
+	if (ret < 0)
+		goto error;
 
 	if (ret < 2) {
-		netdev_err(dev->net, "Error reading PHYID register: %02x\n", ret);
-		goto out;
+		ret = -EIO;
+		goto error;
 	}
-	netdev_dbg(dev->net, "asix_get_phy_addr() returning 0x%04x\n",
-		   *((__le16 *)buf));
+
+	offset = (internal ? 1 : 0);
 	ret = buf[offset];
 
-out:
+	netdev_dbg(dev->net, "%s PHY address 0x%x\n",
+		   internal ? "internal" : "external", ret);
+
 	return ret;
-}
 
-int asix_get_phy_addr(struct usbnet *dev)
-{
-	/* return the address of the internal phy */
-	return asix_read_phy_addr(dev, 1);
-}
+error:
+	netdev_err(dev->net, "Error reading PHY_ID register: %02x\n", ret);
 
+	return ret;
+}
 
 int asix_sw_reset(struct usbnet *dev, u8 flags, int in_pm)
 {
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 5f767a33264e..00b6ac0570eb 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -262,7 +262,10 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->mii.mdio_write = asix_mdio_write;
 	dev->mii.phy_id_mask = 0x3f;
 	dev->mii.reg_num_mask = 0x1f;
-	dev->mii.phy_id = asix_get_phy_addr(dev);
+
+	dev->mii.phy_id = asix_read_phy_addr(dev, true);
+	if (dev->mii.phy_id < 0)
+		return dev->mii.phy_id;
 
 	dev->net->netdev_ops = &ax88172_netdev_ops;
 	dev->net->ethtool_ops = &ax88172_ethtool_ops;
@@ -717,7 +720,10 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->mii.mdio_write = asix_mdio_write;
 	dev->mii.phy_id_mask = 0x1f;
 	dev->mii.reg_num_mask = 0x1f;
-	dev->mii.phy_id = asix_get_phy_addr(dev);
+
+	dev->mii.phy_id = asix_read_phy_addr(dev, true);
+	if (dev->mii.phy_id < 0)
+		return dev->mii.phy_id;
 
 	dev->net->netdev_ops = &ax88772_netdev_ops;
 	dev->net->ethtool_ops = &ax88772_ethtool_ops;
@@ -1080,7 +1086,10 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->mii.phy_id_mask = 0x1f;
 	dev->mii.reg_num_mask = 0xff;
 	dev->mii.supports_gmii = 1;
-	dev->mii.phy_id = asix_get_phy_addr(dev);
+
+	dev->mii.phy_id = asix_read_phy_addr(dev, true);
+	if (dev->mii.phy_id < 0)
+		return dev->mii.phy_id;
 
 	dev->net->netdev_ops = &ax88178_netdev_ops;
 	dev->net->ethtool_ops = &ax88178_ethtool_ops;
diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
index b404c9462dce..c8ca5187eece 100644
--- a/drivers/net/usb/ax88172a.c
+++ b/drivers/net/usb/ax88172a.c
@@ -220,6 +220,11 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
 	}
 
 	priv->phy_addr = asix_read_phy_addr(dev, priv->use_embdphy);
+	if (priv->phy_addr < 0) {
+		ret = priv->phy_addr;
+		goto free;
+	}
+
 	ax88172a_reset_phy(dev, priv->use_embdphy);
 
 	/* Asix framing packs multiple eth frames into a 2K usb bulk transfer */
-- 
2.29.2

[PATCH net-next v2 3/8] net: usb/phy: asix: add support for ax88772A/C PHYs

[Oleksij Rempel]

Add support for build-in x88772A/C PHYs

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/ax88796b.c | 74 +++++++++++++++++++++++++++++++++++++-
 drivers/net/usb/Kconfig    |  1 +
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/ax88796b.c b/drivers/net/phy/ax88796b.c
index 79bf7ef1fcfd..457896337505 100644
--- a/drivers/net/phy/ax88796b.c
+++ b/drivers/net/phy/ax88796b.c
@@ -10,6 +10,8 @@
 #include <linux/mii.h>
 #include <linux/phy.h>
 
+#define PHY_ID_ASIX_AX88772A		0x003b1861
+#define PHY_ID_ASIX_AX88772C		0x003b1881
 #define PHY_ID_ASIX_AX88796B		0x003b1841
 
 MODULE_DESCRIPTION("Asix PHY driver");
@@ -39,7 +41,75 @@ static int asix_soft_reset(struct phy_device *phydev)
 	return genphy_soft_reset(phydev);
 }
 
-static struct phy_driver asix_driver[] = { {
+/* AX88772A is not working properly with some old switches (NETGEAR EN 108TP):
+ * after autoneg is done and the link status is reported as active, the MII_LPA
+ * register is 0. This issue is not reproducible on AX88772C.
+ */
+static int asix_ax88772a_read_status(struct phy_device *phydev)
+{
+	int ret, val;
+
+	ret = genphy_update_link(phydev);
+	if (ret)
+		return ret;
+
+	if (!phydev->link)
+		return 0;
+
+	/* If MII_LPA is 0, phy_resolve_aneg_linkmode() will fail to resolve
+	 * linkmode so use MII_BMCR as default values.
+	 */
+	val = phy_read(phydev, MII_BMCR);
+	if (val < 0)
+		return val;
+
+	if (val & BMCR_SPEED100)
+		phydev->speed = SPEED_100;
+	else
+		phydev->speed = SPEED_10;
+
+	if (val & BMCR_FULLDPLX)
+		phydev->duplex = DUPLEX_FULL;
+	else
+		phydev->duplex = DUPLEX_HALF;
+
+	ret = genphy_read_lpa(phydev);
+	if (ret < 0)
+		return ret;
+
+	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete)
+		phy_resolve_aneg_linkmode(phydev);
+
+	return 0;
+}
+
+static void asix_ax88772a_link_change_notify(struct phy_device *phydev)
+{
+	/* Reset PHY, otherwise MII_LPA will provide outdated information.
+	 * This issue is reproducible only with some link partner PHYs
+	 */
+	if (phydev->state == PHY_NOLINK && phydev->drv->soft_reset)
+		phydev->drv->soft_reset(phydev);
+}
+
+static struct phy_driver asix_driver[] = {
+{
+	PHY_ID_MATCH_EXACT(PHY_ID_ASIX_AX88772A),
+	.name		= "Asix Electronics AX88772A",
+	.flags		= PHY_IS_INTERNAL,
+	.read_status	= asix_ax88772a_read_status,
+	.suspend	= genphy_suspend,
+	.resume		= genphy_resume,
+	.soft_reset	= asix_soft_reset,
+	.link_change_notify	= asix_ax88772a_link_change_notify,
+}, {
+	PHY_ID_MATCH_EXACT(PHY_ID_ASIX_AX88772C),
+	.name		= "Asix Electronics AX88772C",
+	.flags		= PHY_IS_INTERNAL,
+	.suspend	= genphy_suspend,
+	.resume		= genphy_resume,
+	.soft_reset	= asix_soft_reset,
+}, {
 	.phy_id		= PHY_ID_ASIX_AX88796B,
 	.name		= "Asix Electronics AX88796B",
 	.phy_id_mask	= 0xfffffff0,
@@ -50,6 +120,8 @@ static struct phy_driver asix_driver[] = { {
 module_phy_driver(asix_driver);
 
 static struct mdio_device_id __maybe_unused asix_tbl[] = {
+	{ PHY_ID_MATCH_EXACT(PHY_ID_ASIX_AX88772A) },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_ASIX_AX88772C) },
 	{ PHY_ID_ASIX_AX88796B, 0xfffffff0 },
 	{ }
 };
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 179308782888..6f7be47974f6 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -164,6 +164,7 @@ config USB_NET_AX8817X
 	depends on USB_USBNET
 	select CRC32
 	select PHYLIB
+	select AX88796B_PHY
 	default y
 	help
 	  This option adds support for ASIX AX88xxx based USB 2.0
-- 
2.29.2

[PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Oleksij Rempel]

To be able to use ax88772 with external PHYs and use advantage of
existing PHY drivers, we need to port at least ax88772 part of asix
driver to the phylib framework.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/usb/asix.h         |   9 +++
 drivers/net/usb/asix_common.c  |  37 ++++++++++
 drivers/net/usb/asix_devices.c | 120 +++++++++++++++++++++------------
 drivers/net/usb/ax88172a.c     |  14 ----
 4 files changed, 122 insertions(+), 58 deletions(-)

diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index edb94efd265e..2122d302e643 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -25,6 +25,7 @@
 #include <linux/usb/usbnet.h>
 #include <linux/slab.h>
 #include <linux/if_vlan.h>
+#include <linux/phy.h>
 
 #define DRIVER_VERSION "22-Dec-2011"
 #define DRIVER_NAME "asix"
@@ -178,6 +179,10 @@ struct asix_common_private {
 	u16 presvd_phy_advertise;
 	u16 presvd_phy_bmcr;
 	struct asix_rx_fixup_info rx_fixup_info;
+	struct mii_bus *mdio;
+	struct phy_device *phydev;
+	u16 phy_addr;
+	char phy_name[20];
 };
 
 extern const struct driver_info ax88172a_info;
@@ -214,6 +219,7 @@ int asix_write_rx_ctl(struct usbnet *dev, u16 mode, int in_pm);
 
 u16 asix_read_medium_status(struct usbnet *dev, int in_pm);
 int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm);
+void asix_adjust_link(struct net_device *netdev);
 
 int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm);
 
@@ -222,6 +228,9 @@ void asix_set_multicast(struct net_device *net);
 int asix_mdio_read(struct net_device *netdev, int phy_id, int loc);
 void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val);
 
+int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum);
+int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val);
+
 int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc);
 void asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc,
 			  int val);
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index e1109f1a8dd5..085bc8281082 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -384,6 +384,27 @@ int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm)
 	return ret;
 }
 
+/* set MAC link settings according to information from phylib */
+void asix_adjust_link(struct net_device *netdev)
+{
+	struct phy_device *phydev = netdev->phydev;
+	struct usbnet *dev = netdev_priv(netdev);
+	u16 mode = 0;
+
+	if (phydev->link) {
+		mode = AX88772_MEDIUM_DEFAULT;
+
+		if (phydev->duplex == DUPLEX_HALF)
+			mode &= ~AX_MEDIUM_FD;
+
+		if (phydev->speed != SPEED_100)
+			mode &= ~AX_MEDIUM_PS;
+	}
+
+	asix_write_medium_mode(dev, mode, 0);
+	phy_print_status(phydev);
+}
+
 int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
 {
 	int ret;
@@ -506,6 +527,22 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
 	mutex_unlock(&dev->phy_mutex);
 }
 
+/* MDIO read and write wrappers for phylib */
+int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+	struct usbnet *priv = bus->priv;
+
+	return asix_mdio_read(priv->net, phy_id, regnum);
+}
+
+int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
+{
+	struct usbnet *priv = bus->priv;
+
+	asix_mdio_write(priv->net, phy_id, regnum, val);
+	return 0;
+}
+
 int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
 {
 	struct usbnet *dev = netdev_priv(netdev);
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 00b6ac0570eb..e4cd85e38edd 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -285,7 +285,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
 
 static const struct ethtool_ops ax88772_ethtool_ops = {
 	.get_drvinfo		= asix_get_drvinfo,
-	.get_link		= asix_get_link,
+	.get_link		= usbnet_get_link,
 	.get_msglevel		= usbnet_get_msglevel,
 	.set_msglevel		= usbnet_set_msglevel,
 	.get_wol		= asix_get_wol,
@@ -293,37 +293,15 @@ static const struct ethtool_ops ax88772_ethtool_ops = {
 	.get_eeprom_len		= asix_get_eeprom_len,
 	.get_eeprom		= asix_get_eeprom,
 	.set_eeprom		= asix_set_eeprom,
-	.nway_reset		= usbnet_nway_reset,
-	.get_link_ksettings	= usbnet_get_link_ksettings_mii,
-	.set_link_ksettings	= usbnet_set_link_ksettings_mii,
+	.nway_reset		= phy_ethtool_nway_reset,
+	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
+	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
 };
 
-static int ax88772_link_reset(struct usbnet *dev)
-{
-	u16 mode;
-	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
-
-	mii_check_media(&dev->mii, 1, 1);
-	mii_ethtool_gset(&dev->mii, &ecmd);
-	mode = AX88772_MEDIUM_DEFAULT;
-
-	if (ethtool_cmd_speed(&ecmd) != SPEED_100)
-		mode &= ~AX_MEDIUM_PS;
-
-	if (ecmd.duplex != DUPLEX_FULL)
-		mode &= ~AX_MEDIUM_FD;
-
-	netdev_dbg(dev->net, "ax88772_link_reset() speed: %u duplex: %d setting mode to 0x%04x\n",
-		   ethtool_cmd_speed(&ecmd), ecmd.duplex, mode);
-
-	asix_write_medium_mode(dev, mode, 0);
-
-	return 0;
-}
-
 static int ax88772_reset(struct usbnet *dev)
 {
 	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct asix_common_private *priv = dev->driver_priv;
 	int ret;
 
 	/* Rewrite MAC address */
@@ -342,6 +320,8 @@ static int ax88772_reset(struct usbnet *dev)
 	if (ret < 0)
 		goto out;
 
+	phy_start(priv->phydev);
+
 	return 0;
 
 out:
@@ -586,7 +566,7 @@ static const struct net_device_ops ax88772_netdev_ops = {
 	.ndo_get_stats64	= dev_get_tstats64,
 	.ndo_set_mac_address 	= asix_set_mac_address,
 	.ndo_validate_addr	= eth_validate_addr,
-	.ndo_do_ioctl		= asix_ioctl,
+	.ndo_do_ioctl		= phy_do_ioctl_running,
 	.ndo_set_rx_mode        = asix_set_multicast,
 };
 
@@ -677,12 +657,57 @@ static int asix_resume(struct usb_interface *intf)
 	return usbnet_resume(intf);
 }
 
+static int ax88772_init_mdio(struct usbnet *dev)
+{
+	struct asix_common_private *priv = dev->driver_priv;
+
+	priv->mdio = devm_mdiobus_alloc(&dev->udev->dev);
+	if (!priv->mdio)
+		return -ENOMEM;
+
+	priv->mdio->priv = dev;
+	priv->mdio->read = &asix_mdio_bus_read;
+	priv->mdio->write = &asix_mdio_bus_write;
+	priv->mdio->name = "Asix MDIO Bus";
+	/* mii bus name is usb-<usb bus number>-<usb device number> */
+	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
+		 dev->udev->bus->busnum, dev->udev->devnum);
+
+	return devm_mdiobus_register(&dev->udev->dev, priv->mdio);
+}
+
+static int ax88772_init_phy(struct usbnet *dev)
+{
+	struct asix_common_private *priv = dev->driver_priv;
+	int ret;
+
+	priv->phy_addr = asix_read_phy_addr(dev, true);
+	if (priv->phy_addr < 0)
+		return priv->phy_addr;
+
+	snprintf(priv->phy_name, sizeof(priv->phy_name), PHY_ID_FMT,
+		 priv->mdio->id, priv->phy_addr);
+
+	priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link,
+				   PHY_INTERFACE_MODE_INTERNAL);
+	if (IS_ERR(priv->phydev)) {
+		netdev_err(dev->net, "Could not connect to PHY device %s\n",
+			   priv->phy_name);
+		ret = PTR_ERR(priv->phydev);
+		return ret;
+	}
+
+	phy_attached_info(priv->phydev);
+
+	return 0;
+}
+
 static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-	int ret, i;
 	u8 buf[ETH_ALEN] = {0}, chipcode = 0;
-	u32 phyid;
 	struct asix_common_private *priv;
+	int ret, i;
+	u32 phyid;
 
 	usbnet_get_endpoints(dev, intf);
 
@@ -714,17 +739,6 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	asix_set_netdev_dev_addr(dev, buf);
 
-	/* Initialize MII structure */
-	dev->mii.dev = dev->net;
-	dev->mii.mdio_read = asix_mdio_read;
-	dev->mii.mdio_write = asix_mdio_write;
-	dev->mii.phy_id_mask = 0x1f;
-	dev->mii.reg_num_mask = 0x1f;
-
-	dev->mii.phy_id = asix_read_phy_addr(dev, true);
-	if (dev->mii.phy_id < 0)
-		return dev->mii.phy_id;
-
 	dev->net->netdev_ops = &ax88772_netdev_ops;
 	dev->net->ethtool_ops = &ax88772_ethtool_ops;
 	dev->net->needed_headroom = 4; /* cf asix_tx_fixup() */
@@ -768,11 +782,31 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 		priv->suspend = ax88772_suspend;
 	}
 
+	ret = ax88772_init_mdio(dev);
+	if (ret)
+		return ret;
+
+	return ax88772_init_phy(dev);
+}
+
+static int ax88772_stop(struct usbnet *dev)
+{
+	struct asix_common_private *priv = dev->driver_priv;
+
+	/* On unplugged USB, we will get MDIO communication errors and the
+	 * PHY will be set in to PHY_HALTED state.
+	 */
+	if (priv->phydev->state != PHY_HALTED)
+		phy_stop(priv->phydev);
+
 	return 0;
 }
 
 static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
+	struct asix_common_private *priv = dev->driver_priv;
+
+	phy_disconnect(priv->phydev);
 	asix_rx_fixup_common_free(dev->driver_priv);
 }
 
@@ -1161,8 +1195,8 @@ static const struct driver_info ax88772_info = {
 	.bind = ax88772_bind,
 	.unbind = ax88772_unbind,
 	.status = asix_status,
-	.link_reset = ax88772_link_reset,
 	.reset = ax88772_reset,
+	.stop = ax88772_stop,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
 	.rx_fixup = asix_rx_fixup_common,
 	.tx_fixup = asix_tx_fixup,
@@ -1173,7 +1207,6 @@ static const struct driver_info ax88772b_info = {
 	.bind = ax88772_bind,
 	.unbind = ax88772_unbind,
 	.status = asix_status,
-	.link_reset = ax88772_link_reset,
 	.reset = ax88772_reset,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
 	         FLAG_MULTI_PACKET,
@@ -1209,7 +1242,6 @@ static const struct driver_info hg20f9_info = {
 	.bind = ax88772_bind,
 	.unbind = ax88772_unbind,
 	.status = asix_status,
-	.link_reset = ax88772_link_reset,
 	.reset = ax88772_reset,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
 	         FLAG_MULTI_PACKET,
diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
index c8ca5187eece..2e2081346740 100644
--- a/drivers/net/usb/ax88172a.c
+++ b/drivers/net/usb/ax88172a.c
@@ -25,20 +25,6 @@ struct ax88172a_private {
 	struct asix_rx_fixup_info rx_fixup_info;
 };
 
-/* MDIO read and write wrappers for phylib */
-static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
-{
-	return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id,
-			      regnum);
-}
-
-static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum,
-			       u16 val)
-{
-	asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val);
-	return 0;
-}
-
 /* set MAC link settings according to information from phylib */
 static void ax88172a_adjust_link(struct net_device *netdev)
 {
-- 
2.29.2

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Marek Szyprowski]

Hi Oleksij,

On 07.06.2021 10:27, Oleksij Rempel wrote:
> To be able to use ax88772 with external PHYs and use advantage of
> existing PHY drivers, we need to port at least ax88772 part of asix
> driver to the phylib framework.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

This patch landed recently in linux-next as commit e532a096be0e ("net: 
usb: asix: ax88772: add phylib support"). I found that it causes some 
warnings on boards with those devices, see the following log:

root@target:~# time rtcwake -s10 -mmem
rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:16:41 2021
[  231.226579] PM: suspend entry (deep)
[  231.231697] Filesystems sync: 0.002 seconds
[  231.261761] Freezing user space processes ... (elapsed 0.002 seconds) 
done.
[  231.270526] OOM killer disabled.
[  231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 
seconds) done.
[  231.282229] printk: Suspending console(s) (use no_console_suspend to 
debug)
...
[  231.710852] Disabling non-boot CPUs ...
...
[  231.901794] Enabling non-boot CPUs ...
...
[  232.225640] usb usb3: root hub lost power or was reset
[  232.225746] usb usb1: root hub lost power or was reset
[  232.225864] usb usb5: root hub lost power or was reset
[  232.226206] usb usb6: root hub lost power or was reset
[  232.226207] usb usb4: root hub lost power or was reset
[  232.297749] usb usb2: root hub lost power or was reset
[  232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
[  232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access
[  232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
[  232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
[  232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 
returns -22
[  232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to 
resume: error -22
[  232.563712] usb 1-1: reset high-speed USB device number 2 using 
exynos-ehci
[  232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
[  233.730994] OOM killer enabled.
[  233.734122] Restarting tasks ... done.
[  233.754992] PM: suspend exit

real    0m11.546s
user    0m0.000s
sys     0m0.530s
root@target:~# sleep 2
root@target:~# time rtcwake -s10 -mmem
rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:17:02 2021
[  241.959608] PM: suspend entry (deep)
[  241.963446] Filesystems sync: 0.001 seconds
[  241.978619] Freezing user space processes ... (elapsed 0.004 seconds) 
done.
[  241.989199] OOM killer disabled.
[  241.992215] Freezing remaining freezable tasks ... (elapsed 0.005 
seconds) done.
[  242.003979] printk: Suspending console(s) (use no_console_suspend to 
debug)
...
[  242.592030] Disabling non-boot CPUs ...
...
[  242.879721] Enabling non-boot CPUs ...
...
[  243.145870] usb usb3: root hub lost power or was reset
[  243.145910] usb usb4: root hub lost power or was reset
[  243.147084] usb usb5: root hub lost power or was reset
[  243.147157] usb usb6: root hub lost power or was reset
[  243.147298] usb usb1: root hub lost power or was reset
[  243.217137] usb usb2: root hub lost power or was reset
[  243.283807] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
[  243.284005] asix 3-1:1.0 eth0: Failed to enable software MII access
[  243.285526] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
[  243.285676] asix 3-1:1.0 eth0: Failed to read reg index 0x0004: -22
[  243.285769] ------------[ cut here ]------------
[  243.286011] WARNING: CPU: 2 PID: 2069 at drivers/net/phy/phy.c:916 
phy_error+0x28/0x68
[  243.286115] Modules linked in: cmac bnep mwifiex_sdio mwifiex 
sha256_generic libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl 
bluetooth s5p_mfc uvcvideo s5p_jpeg exynos_gsc v
[  243.287490] CPU: 2 PID: 2069 Comm: kworker/2:5 Not tainted 
5.13.0-rc5-next-20210608 #10443
[  243.287555] Hardware name: Samsung Exynos (Flattened Device Tree)
[  243.287609] Workqueue: events_power_efficient phy_state_machine
[  243.287716] [<c0111920>] (unwind_backtrace) from [<c010d0cc>] 
(show_stack+0x10/0x14)
[  243.287807] [<c010d0cc>] (show_stack) from [<c0b62360>] 
(dump_stack_lvl+0xa0/0xc0)
[  243.287882] [<c0b62360>] (dump_stack_lvl) from [<c0127960>] 
(__warn+0x118/0x11c)
[  243.287954] [<c0127960>] (__warn) from [<c0127a18>] 
(warn_slowpath_fmt+0xb4/0xbc)
[  243.288021] [<c0127a18>] (warn_slowpath_fmt) from [<c0734968>] 
(phy_error+0x28/0x68)
[  243.288094] [<c0734968>] (phy_error) from [<c0735d6c>] 
(phy_state_machine+0x218/0x278)
[  243.288173] [<c0735d6c>] (phy_state_machine) from [<c014ae08>] 
(process_one_work+0x30c/0x884)
[  243.288254] [<c014ae08>] (process_one_work) from [<c014b3d8>] 
(worker_thread+0x58/0x594)
[  243.288333] [<c014b3d8>] (worker_thread) from [<c0153944>] 
(kthread+0x160/0x1c0)
[  243.288408] [<c0153944>] (kthread) from [<c010011c>] 
(ret_from_fork+0x14/0x38)
[  243.288475] Exception stack(0xc4683fb0 to 0xc4683ff8)
[  243.288531] 3fa0:                                     00000000 
00000000 00000000 00000000
[  243.288587] 3fc0: 00000000 00000000 00000000 00000000 00000000 
00000000 00000000 00000000
[  243.288641] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[  243.288690] irq event stamp: 1611
[  243.288744] hardirqs last  enabled at (1619): [<c01a6ef0>] 
vprintk_emit+0x230/0x290
[  243.288830] hardirqs last disabled at (1626): [<c01a6f2c>] 
vprintk_emit+0x26c/0x290
[  243.288906] softirqs last  enabled at (1012): [<c0101768>] 
__do_softirq+0x500/0x63c
[  243.288978] softirqs last disabled at (1007): [<c01315b4>] 
irq_exit+0x214/0x220
[  243.289055] ---[ end trace eeacda95eb7db60a ]---
[  243.289345] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
[  243.289466] asix 3-1:1.0 eth0: Failed to write Medium Mode mode to 
0x0000: ffffffea
[  243.289540] asix 3-1:1.0 eth0: Link is Down
[  243.482809] usb 1-1: reset high-speed USB device number 2 using 
exynos-ehci
[  243.647251] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
[  244.847161] OOM killer enabled.
[  244.850221] Restarting tasks ... done.
[  244.861372] PM: suspend exit

real    0m13.050s
user    0m0.000s
sys     0m1.152s
root@target:~#

It looks that some kind of system suspend/resume integration for phylib 
is not implemented.

> ---
>   drivers/net/usb/asix.h         |   9 +++
>   drivers/net/usb/asix_common.c  |  37 ++++++++++
>   drivers/net/usb/asix_devices.c | 120 +++++++++++++++++++++------------
>   drivers/net/usb/ax88172a.c     |  14 ----
>   4 files changed, 122 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
> index edb94efd265e..2122d302e643 100644
> --- a/drivers/net/usb/asix.h
> +++ b/drivers/net/usb/asix.h
> @@ -25,6 +25,7 @@
>   #include <linux/usb/usbnet.h>
>   #include <linux/slab.h>
>   #include <linux/if_vlan.h>
> +#include <linux/phy.h>
>   
>   #define DRIVER_VERSION "22-Dec-2011"
>   #define DRIVER_NAME "asix"
> @@ -178,6 +179,10 @@ struct asix_common_private {
>   	u16 presvd_phy_advertise;
>   	u16 presvd_phy_bmcr;
>   	struct asix_rx_fixup_info rx_fixup_info;
> +	struct mii_bus *mdio;
> +	struct phy_device *phydev;
> +	u16 phy_addr;
> +	char phy_name[20];
>   };
>   
>   extern const struct driver_info ax88172a_info;
> @@ -214,6 +219,7 @@ int asix_write_rx_ctl(struct usbnet *dev, u16 mode, int in_pm);
>   
>   u16 asix_read_medium_status(struct usbnet *dev, int in_pm);
>   int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm);
> +void asix_adjust_link(struct net_device *netdev);
>   
>   int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm);
>   
> @@ -222,6 +228,9 @@ void asix_set_multicast(struct net_device *net);
>   int asix_mdio_read(struct net_device *netdev, int phy_id, int loc);
>   void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val);
>   
> +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum);
> +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val);
> +
>   int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc);
>   void asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc,
>   			  int val);
> diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
> index e1109f1a8dd5..085bc8281082 100644
> --- a/drivers/net/usb/asix_common.c
> +++ b/drivers/net/usb/asix_common.c
> @@ -384,6 +384,27 @@ int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm)
>   	return ret;
>   }
>   
> +/* set MAC link settings according to information from phylib */
> +void asix_adjust_link(struct net_device *netdev)
> +{
> +	struct phy_device *phydev = netdev->phydev;
> +	struct usbnet *dev = netdev_priv(netdev);
> +	u16 mode = 0;
> +
> +	if (phydev->link) {
> +		mode = AX88772_MEDIUM_DEFAULT;
> +
> +		if (phydev->duplex == DUPLEX_HALF)
> +			mode &= ~AX_MEDIUM_FD;
> +
> +		if (phydev->speed != SPEED_100)
> +			mode &= ~AX_MEDIUM_PS;
> +	}
> +
> +	asix_write_medium_mode(dev, mode, 0);
> +	phy_print_status(phydev);
> +}
> +
>   int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
>   {
>   	int ret;
> @@ -506,6 +527,22 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
>   	mutex_unlock(&dev->phy_mutex);
>   }
>   
> +/* MDIO read and write wrappers for phylib */
> +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
> +{
> +	struct usbnet *priv = bus->priv;
> +
> +	return asix_mdio_read(priv->net, phy_id, regnum);
> +}
> +
> +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
> +{
> +	struct usbnet *priv = bus->priv;
> +
> +	asix_mdio_write(priv->net, phy_id, regnum, val);
> +	return 0;
> +}
> +
>   int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
>   {
>   	struct usbnet *dev = netdev_priv(netdev);
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index 00b6ac0570eb..e4cd85e38edd 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -285,7 +285,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
>   
>   static const struct ethtool_ops ax88772_ethtool_ops = {
>   	.get_drvinfo		= asix_get_drvinfo,
> -	.get_link		= asix_get_link,
> +	.get_link		= usbnet_get_link,
>   	.get_msglevel		= usbnet_get_msglevel,
>   	.set_msglevel		= usbnet_set_msglevel,
>   	.get_wol		= asix_get_wol,
> @@ -293,37 +293,15 @@ static const struct ethtool_ops ax88772_ethtool_ops = {
>   	.get_eeprom_len		= asix_get_eeprom_len,
>   	.get_eeprom		= asix_get_eeprom,
>   	.set_eeprom		= asix_set_eeprom,
> -	.nway_reset		= usbnet_nway_reset,
> -	.get_link_ksettings	= usbnet_get_link_ksettings_mii,
> -	.set_link_ksettings	= usbnet_set_link_ksettings_mii,
> +	.nway_reset		= phy_ethtool_nway_reset,
> +	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
> +	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
>   };
>   
> -static int ax88772_link_reset(struct usbnet *dev)
> -{
> -	u16 mode;
> -	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
> -
> -	mii_check_media(&dev->mii, 1, 1);
> -	mii_ethtool_gset(&dev->mii, &ecmd);
> -	mode = AX88772_MEDIUM_DEFAULT;
> -
> -	if (ethtool_cmd_speed(&ecmd) != SPEED_100)
> -		mode &= ~AX_MEDIUM_PS;
> -
> -	if (ecmd.duplex != DUPLEX_FULL)
> -		mode &= ~AX_MEDIUM_FD;
> -
> -	netdev_dbg(dev->net, "ax88772_link_reset() speed: %u duplex: %d setting mode to 0x%04x\n",
> -		   ethtool_cmd_speed(&ecmd), ecmd.duplex, mode);
> -
> -	asix_write_medium_mode(dev, mode, 0);
> -
> -	return 0;
> -}
> -
>   static int ax88772_reset(struct usbnet *dev)
>   {
>   	struct asix_data *data = (struct asix_data *)&dev->data;
> +	struct asix_common_private *priv = dev->driver_priv;
>   	int ret;
>   
>   	/* Rewrite MAC address */
> @@ -342,6 +320,8 @@ static int ax88772_reset(struct usbnet *dev)
>   	if (ret < 0)
>   		goto out;
>   
> +	phy_start(priv->phydev);
> +
>   	return 0;
>   
>   out:
> @@ -586,7 +566,7 @@ static const struct net_device_ops ax88772_netdev_ops = {
>   	.ndo_get_stats64	= dev_get_tstats64,
>   	.ndo_set_mac_address 	= asix_set_mac_address,
>   	.ndo_validate_addr	= eth_validate_addr,
> -	.ndo_do_ioctl		= asix_ioctl,
> +	.ndo_do_ioctl		= phy_do_ioctl_running,
>   	.ndo_set_rx_mode        = asix_set_multicast,
>   };
>   
> @@ -677,12 +657,57 @@ static int asix_resume(struct usb_interface *intf)
>   	return usbnet_resume(intf);
>   }
>   
> +static int ax88772_init_mdio(struct usbnet *dev)
> +{
> +	struct asix_common_private *priv = dev->driver_priv;
> +
> +	priv->mdio = devm_mdiobus_alloc(&dev->udev->dev);
> +	if (!priv->mdio)
> +		return -ENOMEM;
> +
> +	priv->mdio->priv = dev;
> +	priv->mdio->read = &asix_mdio_bus_read;
> +	priv->mdio->write = &asix_mdio_bus_write;
> +	priv->mdio->name = "Asix MDIO Bus";
> +	/* mii bus name is usb-<usb bus number>-<usb device number> */
> +	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
> +		 dev->udev->bus->busnum, dev->udev->devnum);
> +
> +	return devm_mdiobus_register(&dev->udev->dev, priv->mdio);
> +}
> +
> +static int ax88772_init_phy(struct usbnet *dev)
> +{
> +	struct asix_common_private *priv = dev->driver_priv;
> +	int ret;
> +
> +	priv->phy_addr = asix_read_phy_addr(dev, true);
> +	if (priv->phy_addr < 0)
> +		return priv->phy_addr;
> +
> +	snprintf(priv->phy_name, sizeof(priv->phy_name), PHY_ID_FMT,
> +		 priv->mdio->id, priv->phy_addr);
> +
> +	priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link,
> +				   PHY_INTERFACE_MODE_INTERNAL);
> +	if (IS_ERR(priv->phydev)) {
> +		netdev_err(dev->net, "Could not connect to PHY device %s\n",
> +			   priv->phy_name);
> +		ret = PTR_ERR(priv->phydev);
> +		return ret;
> +	}
> +
> +	phy_attached_info(priv->phydev);
> +
> +	return 0;
> +}
> +
>   static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>   {
> -	int ret, i;
>   	u8 buf[ETH_ALEN] = {0}, chipcode = 0;
> -	u32 phyid;
>   	struct asix_common_private *priv;
> +	int ret, i;
> +	u32 phyid;
>   
>   	usbnet_get_endpoints(dev, intf);
>   
> @@ -714,17 +739,6 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>   
>   	asix_set_netdev_dev_addr(dev, buf);
>   
> -	/* Initialize MII structure */
> -	dev->mii.dev = dev->net;
> -	dev->mii.mdio_read = asix_mdio_read;
> -	dev->mii.mdio_write = asix_mdio_write;
> -	dev->mii.phy_id_mask = 0x1f;
> -	dev->mii.reg_num_mask = 0x1f;
> -
> -	dev->mii.phy_id = asix_read_phy_addr(dev, true);
> -	if (dev->mii.phy_id < 0)
> -		return dev->mii.phy_id;
> -
>   	dev->net->netdev_ops = &ax88772_netdev_ops;
>   	dev->net->ethtool_ops = &ax88772_ethtool_ops;
>   	dev->net->needed_headroom = 4; /* cf asix_tx_fixup() */
> @@ -768,11 +782,31 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>   		priv->suspend = ax88772_suspend;
>   	}
>   
> +	ret = ax88772_init_mdio(dev);
> +	if (ret)
> +		return ret;
> +
> +	return ax88772_init_phy(dev);
> +}
> +
> +static int ax88772_stop(struct usbnet *dev)
> +{
> +	struct asix_common_private *priv = dev->driver_priv;
> +
> +	/* On unplugged USB, we will get MDIO communication errors and the
> +	 * PHY will be set in to PHY_HALTED state.
> +	 */
> +	if (priv->phydev->state != PHY_HALTED)
> +		phy_stop(priv->phydev);
> +
>   	return 0;
>   }
>   
>   static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
>   {
> +	struct asix_common_private *priv = dev->driver_priv;
> +
> +	phy_disconnect(priv->phydev);
>   	asix_rx_fixup_common_free(dev->driver_priv);
>   }
>   
> @@ -1161,8 +1195,8 @@ static const struct driver_info ax88772_info = {
>   	.bind = ax88772_bind,
>   	.unbind = ax88772_unbind,
>   	.status = asix_status,
> -	.link_reset = ax88772_link_reset,
>   	.reset = ax88772_reset,
> +	.stop = ax88772_stop,
>   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
>   	.rx_fixup = asix_rx_fixup_common,
>   	.tx_fixup = asix_tx_fixup,
> @@ -1173,7 +1207,6 @@ static const struct driver_info ax88772b_info = {
>   	.bind = ax88772_bind,
>   	.unbind = ax88772_unbind,
>   	.status = asix_status,
> -	.link_reset = ax88772_link_reset,
>   	.reset = ax88772_reset,
>   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
>   	         FLAG_MULTI_PACKET,
> @@ -1209,7 +1242,6 @@ static const struct driver_info hg20f9_info = {
>   	.bind = ax88772_bind,
>   	.unbind = ax88772_unbind,
>   	.status = asix_status,
> -	.link_reset = ax88772_link_reset,
>   	.reset = ax88772_reset,
>   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
>   	         FLAG_MULTI_PACKET,
> diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
> index c8ca5187eece..2e2081346740 100644
> --- a/drivers/net/usb/ax88172a.c
> +++ b/drivers/net/usb/ax88172a.c
> @@ -25,20 +25,6 @@ struct ax88172a_private {
>   	struct asix_rx_fixup_info rx_fixup_info;
>   };
>   
> -/* MDIO read and write wrappers for phylib */
> -static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
> -{
> -	return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id,
> -			      regnum);
> -}
> -
> -static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum,
> -			       u16 val)
> -{
> -	asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val);
> -	return 0;
> -}
> -
>   /* set MAC link settings according to information from phylib */
>   static void ax88172a_adjust_link(struct net_device *netdev)
>   {

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Oleksij Rempel]

Hi Marek,

On Wed, Jun 09, 2021 at 11:59:23AM +0200, Marek Szyprowski wrote:
> Hi Oleksij,
> 
> On 07.06.2021 10:27, Oleksij Rempel wrote:
> > To be able to use ax88772 with external PHYs and use advantage of
> > existing PHY drivers, we need to port at least ax88772 part of asix
> > driver to the phylib framework.
> >
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> This patch landed recently in linux-next as commit e532a096be0e ("net: 
> usb: asix: ax88772: add phylib support"). I found that it causes some 
> warnings on boards with those devices, see the following log:
> 
> root@target:~# time rtcwake -s10 -mmem
> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:16:41 2021
> [  231.226579] PM: suspend entry (deep)
> [  231.231697] Filesystems sync: 0.002 seconds
> [  231.261761] Freezing user space processes ... (elapsed 0.002 seconds) 
> done.
> [  231.270526] OOM killer disabled.
> [  231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 
> seconds) done.
> [  231.282229] printk: Suspending console(s) (use no_console_suspend to 
> debug)
> ...
> [  231.710852] Disabling non-boot CPUs ...
> ...
> [  231.901794] Enabling non-boot CPUs ...
> ...
> [  232.225640] usb usb3: root hub lost power or was reset
> [  232.225746] usb usb1: root hub lost power or was reset
> [  232.225864] usb usb5: root hub lost power or was reset
> [  232.226206] usb usb6: root hub lost power or was reset
> [  232.226207] usb usb4: root hub lost power or was reset
> [  232.297749] usb usb2: root hub lost power or was reset
> [  232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> [  232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access
> [  232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
> [  232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> [  232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 
> returns -22
> [  232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to 
> resume: error -22
> [  232.563712] usb 1-1: reset high-speed USB device number 2 using 
> exynos-ehci
> [  232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
> [  233.730994] OOM killer enabled.
> [  233.734122] Restarting tasks ... done.
> [  233.754992] PM: suspend exit
> 
> real    0m11.546s
> user    0m0.000s
> sys     0m0.530s
> root@target:~# sleep 2
> root@target:~# time rtcwake -s10 -mmem
> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:17:02 2021
> [  241.959608] PM: suspend entry (deep)
> [  241.963446] Filesystems sync: 0.001 seconds
> [  241.978619] Freezing user space processes ... (elapsed 0.004 seconds) 
> done.
> [  241.989199] OOM killer disabled.
> [  241.992215] Freezing remaining freezable tasks ... (elapsed 0.005 
> seconds) done.
> [  242.003979] printk: Suspending console(s) (use no_console_suspend to 
> debug)
> ...
> [  242.592030] Disabling non-boot CPUs ...
> ...
> [  242.879721] Enabling non-boot CPUs ...
> ...
> [  243.145870] usb usb3: root hub lost power or was reset
> [  243.145910] usb usb4: root hub lost power or was reset
> [  243.147084] usb usb5: root hub lost power or was reset
> [  243.147157] usb usb6: root hub lost power or was reset
> [  243.147298] usb usb1: root hub lost power or was reset
> [  243.217137] usb usb2: root hub lost power or was reset
> [  243.283807] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> [  243.284005] asix 3-1:1.0 eth0: Failed to enable software MII access
> [  243.285526] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
> [  243.285676] asix 3-1:1.0 eth0: Failed to read reg index 0x0004: -22
> [  243.285769] ------------[ cut here ]------------
> [  243.286011] WARNING: CPU: 2 PID: 2069 at drivers/net/phy/phy.c:916 
> phy_error+0x28/0x68
> [  243.286115] Modules linked in: cmac bnep mwifiex_sdio mwifiex 
> sha256_generic libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl 
> bluetooth s5p_mfc uvcvideo s5p_jpeg exynos_gsc v
> [  243.287490] CPU: 2 PID: 2069 Comm: kworker/2:5 Not tainted 
> 5.13.0-rc5-next-20210608 #10443
> [  243.287555] Hardware name: Samsung Exynos (Flattened Device Tree)
> [  243.287609] Workqueue: events_power_efficient phy_state_machine
> [  243.287716] [<c0111920>] (unwind_backtrace) from [<c010d0cc>] 
> (show_stack+0x10/0x14)
> [  243.287807] [<c010d0cc>] (show_stack) from [<c0b62360>] 
> (dump_stack_lvl+0xa0/0xc0)
> [  243.287882] [<c0b62360>] (dump_stack_lvl) from [<c0127960>] 
> (__warn+0x118/0x11c)
> [  243.287954] [<c0127960>] (__warn) from [<c0127a18>] 
> (warn_slowpath_fmt+0xb4/0xbc)
> [  243.288021] [<c0127a18>] (warn_slowpath_fmt) from [<c0734968>] 
> (phy_error+0x28/0x68)
> [  243.288094] [<c0734968>] (phy_error) from [<c0735d6c>] 
> (phy_state_machine+0x218/0x278)
> [  243.288173] [<c0735d6c>] (phy_state_machine) from [<c014ae08>] 
> (process_one_work+0x30c/0x884)
> [  243.288254] [<c014ae08>] (process_one_work) from [<c014b3d8>] 
> (worker_thread+0x58/0x594)
> [  243.288333] [<c014b3d8>] (worker_thread) from [<c0153944>] 
> (kthread+0x160/0x1c0)
> [  243.288408] [<c0153944>] (kthread) from [<c010011c>] 
> (ret_from_fork+0x14/0x38)
> [  243.288475] Exception stack(0xc4683fb0 to 0xc4683ff8)
> [  243.288531] 3fa0:                                     00000000 
> 00000000 00000000 00000000
> [  243.288587] 3fc0: 00000000 00000000 00000000 00000000 00000000 
> 00000000 00000000 00000000
> [  243.288641] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [  243.288690] irq event stamp: 1611
> [  243.288744] hardirqs last  enabled at (1619): [<c01a6ef0>] 
> vprintk_emit+0x230/0x290
> [  243.288830] hardirqs last disabled at (1626): [<c01a6f2c>] 
> vprintk_emit+0x26c/0x290
> [  243.288906] softirqs last  enabled at (1012): [<c0101768>] 
> __do_softirq+0x500/0x63c
> [  243.288978] softirqs last disabled at (1007): [<c01315b4>] 
> irq_exit+0x214/0x220
> [  243.289055] ---[ end trace eeacda95eb7db60a ]---
> [  243.289345] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> [  243.289466] asix 3-1:1.0 eth0: Failed to write Medium Mode mode to 
> 0x0000: ffffffea
> [  243.289540] asix 3-1:1.0 eth0: Link is Down
> [  243.482809] usb 1-1: reset high-speed USB device number 2 using 
> exynos-ehci
> [  243.647251] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
> [  244.847161] OOM killer enabled.
> [  244.850221] Restarting tasks ... done.
> [  244.861372] PM: suspend exit
> 
> real    0m13.050s
> user    0m0.000s
> sys     0m1.152s
> root@target:~#
> 
> It looks that some kind of system suspend/resume integration for phylib 
> is not implemented.

Probably it is should be handled only by the asix driver. I'll take a
look in to it. Did interface was able to resume after printing some
warnings?

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Heiner Kallweit]

On 09.06.2021 14:46, Oleksij Rempel wrote:
> Hi Marek,
> 
> On Wed, Jun 09, 2021 at 11:59:23AM +0200, Marek Szyprowski wrote:
>> Hi Oleksij,
>>
>> On 07.06.2021 10:27, Oleksij Rempel wrote:
>>> To be able to use ax88772 with external PHYs and use advantage of
>>> existing PHY drivers, we need to port at least ax88772 part of asix
>>> driver to the phylib framework.
>>>
>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>
>> This patch landed recently in linux-next as commit e532a096be0e ("net: 
>> usb: asix: ax88772: add phylib support"). I found that it causes some 
>> warnings on boards with those devices, see the following log:
>>
>> root@target:~# time rtcwake -s10 -mmem
>> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:16:41 2021
>> [  231.226579] PM: suspend entry (deep)
>> [  231.231697] Filesystems sync: 0.002 seconds
>> [  231.261761] Freezing user space processes ... (elapsed 0.002 seconds) 
>> done.
>> [  231.270526] OOM killer disabled.
>> [  231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 
>> seconds) done.
>> [  231.282229] printk: Suspending console(s) (use no_console_suspend to 
>> debug)
>> ...
>> [  231.710852] Disabling non-boot CPUs ...
>> ...
>> [  231.901794] Enabling non-boot CPUs ...
>> ...
>> [  232.225640] usb usb3: root hub lost power or was reset
>> [  232.225746] usb usb1: root hub lost power or was reset
>> [  232.225864] usb usb5: root hub lost power or was reset
>> [  232.226206] usb usb6: root hub lost power or was reset
>> [  232.226207] usb usb4: root hub lost power or was reset
>> [  232.297749] usb usb2: root hub lost power or was reset
>> [  232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
>> [  232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access
>> [  232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
>> [  232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
>> [  232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 
>> returns -22
>> [  232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to 
>> resume: error -22
>> [  232.563712] usb 1-1: reset high-speed USB device number 2 using 
>> exynos-ehci
>> [  232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
>> [  233.730994] OOM killer enabled.
>> [  233.734122] Restarting tasks ... done.
>> [  233.754992] PM: suspend exit
>>
>> real    0m11.546s
>> user    0m0.000s
>> sys     0m0.530s
>> root@target:~# sleep 2
>> root@target:~# time rtcwake -s10 -mmem
>> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:17:02 2021
>> [  241.959608] PM: suspend entry (deep)
>> [  241.963446] Filesystems sync: 0.001 seconds
>> [  241.978619] Freezing user space processes ... (elapsed 0.004 seconds) 
>> done.
>> [  241.989199] OOM killer disabled.
>> [  241.992215] Freezing remaining freezable tasks ... (elapsed 0.005 
>> seconds) done.
>> [  242.003979] printk: Suspending console(s) (use no_console_suspend to 
>> debug)
>> ...
>> [  242.592030] Disabling non-boot CPUs ...
>> ...
>> [  242.879721] Enabling non-boot CPUs ...
>> ...
>> [  243.145870] usb usb3: root hub lost power or was reset
>> [  243.145910] usb usb4: root hub lost power or was reset
>> [  243.147084] usb usb5: root hub lost power or was reset
>> [  243.147157] usb usb6: root hub lost power or was reset
>> [  243.147298] usb usb1: root hub lost power or was reset
>> [  243.217137] usb usb2: root hub lost power or was reset
>> [  243.283807] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
>> [  243.284005] asix 3-1:1.0 eth0: Failed to enable software MII access
>> [  243.285526] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
>> [  243.285676] asix 3-1:1.0 eth0: Failed to read reg index 0x0004: -22
>> [  243.285769] ------------[ cut here ]------------
>> [  243.286011] WARNING: CPU: 2 PID: 2069 at drivers/net/phy/phy.c:916 
>> phy_error+0x28/0x68
>> [  243.286115] Modules linked in: cmac bnep mwifiex_sdio mwifiex 
>> sha256_generic libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl 
>> bluetooth s5p_mfc uvcvideo s5p_jpeg exynos_gsc v
>> [  243.287490] CPU: 2 PID: 2069 Comm: kworker/2:5 Not tainted 
>> 5.13.0-rc5-next-20210608 #10443
>> [  243.287555] Hardware name: Samsung Exynos (Flattened Device Tree)
>> [  243.287609] Workqueue: events_power_efficient phy_state_machine
>> [  243.287716] [<c0111920>] (unwind_backtrace) from [<c010d0cc>] 
>> (show_stack+0x10/0x14)
>> [  243.287807] [<c010d0cc>] (show_stack) from [<c0b62360>] 
>> (dump_stack_lvl+0xa0/0xc0)
>> [  243.287882] [<c0b62360>] (dump_stack_lvl) from [<c0127960>] 
>> (__warn+0x118/0x11c)
>> [  243.287954] [<c0127960>] (__warn) from [<c0127a18>] 
>> (warn_slowpath_fmt+0xb4/0xbc)
>> [  243.288021] [<c0127a18>] (warn_slowpath_fmt) from [<c0734968>] 
>> (phy_error+0x28/0x68)
>> [  243.288094] [<c0734968>] (phy_error) from [<c0735d6c>] 
>> (phy_state_machine+0x218/0x278)
>> [  243.288173] [<c0735d6c>] (phy_state_machine) from [<c014ae08>] 
>> (process_one_work+0x30c/0x884)
>> [  243.288254] [<c014ae08>] (process_one_work) from [<c014b3d8>] 
>> (worker_thread+0x58/0x594)
>> [  243.288333] [<c014b3d8>] (worker_thread) from [<c0153944>] 
>> (kthread+0x160/0x1c0)
>> [  243.288408] [<c0153944>] (kthread) from [<c010011c>] 
>> (ret_from_fork+0x14/0x38)
>> [  243.288475] Exception stack(0xc4683fb0 to 0xc4683ff8)
>> [  243.288531] 3fa0:                                     00000000 
>> 00000000 00000000 00000000
>> [  243.288587] 3fc0: 00000000 00000000 00000000 00000000 00000000 
>> 00000000 00000000 00000000
>> [  243.288641] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>> [  243.288690] irq event stamp: 1611
>> [  243.288744] hardirqs last  enabled at (1619): [<c01a6ef0>] 
>> vprintk_emit+0x230/0x290
>> [  243.288830] hardirqs last disabled at (1626): [<c01a6f2c>] 
>> vprintk_emit+0x26c/0x290
>> [  243.288906] softirqs last  enabled at (1012): [<c0101768>] 
>> __do_softirq+0x500/0x63c
>> [  243.288978] softirqs last disabled at (1007): [<c01315b4>] 
>> irq_exit+0x214/0x220
>> [  243.289055] ---[ end trace eeacda95eb7db60a ]---
>> [  243.289345] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
>> [  243.289466] asix 3-1:1.0 eth0: Failed to write Medium Mode mode to 
>> 0x0000: ffffffea
>> [  243.289540] asix 3-1:1.0 eth0: Link is Down
>> [  243.482809] usb 1-1: reset high-speed USB device number 2 using 
>> exynos-ehci
>> [  243.647251] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
>> [  244.847161] OOM killer enabled.
>> [  244.850221] Restarting tasks ... done.
>> [  244.861372] PM: suspend exit
>>
>> real    0m13.050s
>> user    0m0.000s
>> sys     0m1.152s
>> root@target:~#
>>
>> It looks that some kind of system suspend/resume integration for phylib 
>> is not implemented.
> 
> Probably it is should be handled only by the asix driver. I'll take a
> look in to it. Did interface was able to resume after printing some
> warnings?
> 
> Regards,
> Oleksij
> 

Maybe it's a use case for the new mac_managed_pm flag, see
fba863b81604 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM")

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Oleksij Rempel]

On Wed, Jun 09, 2021 at 03:12:37PM +0200, Heiner Kallweit wrote:
> On 09.06.2021 14:46, Oleksij Rempel wrote:
> > Hi Marek,
> > 
> > On Wed, Jun 09, 2021 at 11:59:23AM +0200, Marek Szyprowski wrote:
> >> Hi Oleksij,
> >>
> >> On 07.06.2021 10:27, Oleksij Rempel wrote:
> >>> To be able to use ax88772 with external PHYs and use advantage of
> >>> existing PHY drivers, we need to port at least ax88772 part of asix
> >>> driver to the phylib framework.
> >>>
> >>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >>
> >> This patch landed recently in linux-next as commit e532a096be0e ("net: 
> >> usb: asix: ax88772: add phylib support"). I found that it causes some 
> >> warnings on boards with those devices, see the following log:
> >>
> >> root@target:~# time rtcwake -s10 -mmem
> >> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:16:41 2021
> >> [  231.226579] PM: suspend entry (deep)
> >> [  231.231697] Filesystems sync: 0.002 seconds
> >> [  231.261761] Freezing user space processes ... (elapsed 0.002 seconds) 
> >> done.
> >> [  231.270526] OOM killer disabled.
> >> [  231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 
> >> seconds) done.
> >> [  231.282229] printk: Suspending console(s) (use no_console_suspend to 
> >> debug)
> >> ...
> >> [  231.710852] Disabling non-boot CPUs ...
> >> ...
> >> [  231.901794] Enabling non-boot CPUs ...
> >> ...
> >> [  232.225640] usb usb3: root hub lost power or was reset
> >> [  232.225746] usb usb1: root hub lost power or was reset
> >> [  232.225864] usb usb5: root hub lost power or was reset
> >> [  232.226206] usb usb6: root hub lost power or was reset
> >> [  232.226207] usb usb4: root hub lost power or was reset
> >> [  232.297749] usb usb2: root hub lost power or was reset
> >> [  232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> >> [  232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access
> >> [  232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
> >> [  232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> >> [  232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 
> >> returns -22
> >> [  232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to 
> >> resume: error -22
> >> [  232.563712] usb 1-1: reset high-speed USB device number 2 using 
> >> exynos-ehci
> >> [  232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
> >> [  233.730994] OOM killer enabled.
> >> [  233.734122] Restarting tasks ... done.
> >> [  233.754992] PM: suspend exit
> >>
> >> real    0m11.546s
> >> user    0m0.000s
> >> sys     0m0.530s
> >> root@target:~# sleep 2
> >> root@target:~# time rtcwake -s10 -mmem
> >> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:17:02 2021
> >> [  241.959608] PM: suspend entry (deep)
> >> [  241.963446] Filesystems sync: 0.001 seconds
> >> [  241.978619] Freezing user space processes ... (elapsed 0.004 seconds) 
> >> done.
> >> [  241.989199] OOM killer disabled.
> >> [  241.992215] Freezing remaining freezable tasks ... (elapsed 0.005 
> >> seconds) done.
> >> [  242.003979] printk: Suspending console(s) (use no_console_suspend to 
> >> debug)
> >> ...
> >> [  242.592030] Disabling non-boot CPUs ...
> >> ...
> >> [  242.879721] Enabling non-boot CPUs ...
> >> ...
> >> [  243.145870] usb usb3: root hub lost power or was reset
> >> [  243.145910] usb usb4: root hub lost power or was reset
> >> [  243.147084] usb usb5: root hub lost power or was reset
> >> [  243.147157] usb usb6: root hub lost power or was reset
> >> [  243.147298] usb usb1: root hub lost power or was reset
> >> [  243.217137] usb usb2: root hub lost power or was reset
> >> [  243.283807] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> >> [  243.284005] asix 3-1:1.0 eth0: Failed to enable software MII access
> >> [  243.285526] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
> >> [  243.285676] asix 3-1:1.0 eth0: Failed to read reg index 0x0004: -22
> >> [  243.285769] ------------[ cut here ]------------
> >> [  243.286011] WARNING: CPU: 2 PID: 2069 at drivers/net/phy/phy.c:916 
> >> phy_error+0x28/0x68
> >> [  243.286115] Modules linked in: cmac bnep mwifiex_sdio mwifiex 
> >> sha256_generic libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl 
> >> bluetooth s5p_mfc uvcvideo s5p_jpeg exynos_gsc v
> >> [  243.287490] CPU: 2 PID: 2069 Comm: kworker/2:5 Not tainted 
> >> 5.13.0-rc5-next-20210608 #10443
> >> [  243.287555] Hardware name: Samsung Exynos (Flattened Device Tree)
> >> [  243.287609] Workqueue: events_power_efficient phy_state_machine
> >> [  243.287716] [<c0111920>] (unwind_backtrace) from [<c010d0cc>] 
> >> (show_stack+0x10/0x14)
> >> [  243.287807] [<c010d0cc>] (show_stack) from [<c0b62360>] 
> >> (dump_stack_lvl+0xa0/0xc0)
> >> [  243.287882] [<c0b62360>] (dump_stack_lvl) from [<c0127960>] 
> >> (__warn+0x118/0x11c)
> >> [  243.287954] [<c0127960>] (__warn) from [<c0127a18>] 
> >> (warn_slowpath_fmt+0xb4/0xbc)
> >> [  243.288021] [<c0127a18>] (warn_slowpath_fmt) from [<c0734968>] 
> >> (phy_error+0x28/0x68)
> >> [  243.288094] [<c0734968>] (phy_error) from [<c0735d6c>] 
> >> (phy_state_machine+0x218/0x278)
> >> [  243.288173] [<c0735d6c>] (phy_state_machine) from [<c014ae08>] 
> >> (process_one_work+0x30c/0x884)
> >> [  243.288254] [<c014ae08>] (process_one_work) from [<c014b3d8>] 
> >> (worker_thread+0x58/0x594)
> >> [  243.288333] [<c014b3d8>] (worker_thread) from [<c0153944>] 
> >> (kthread+0x160/0x1c0)
> >> [  243.288408] [<c0153944>] (kthread) from [<c010011c>] 
> >> (ret_from_fork+0x14/0x38)
> >> [  243.288475] Exception stack(0xc4683fb0 to 0xc4683ff8)
> >> [  243.288531] 3fa0:                                     00000000 
> >> 00000000 00000000 00000000
> >> [  243.288587] 3fc0: 00000000 00000000 00000000 00000000 00000000 
> >> 00000000 00000000 00000000
> >> [  243.288641] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> >> [  243.288690] irq event stamp: 1611
> >> [  243.288744] hardirqs last  enabled at (1619): [<c01a6ef0>] 
> >> vprintk_emit+0x230/0x290
> >> [  243.288830] hardirqs last disabled at (1626): [<c01a6f2c>] 
> >> vprintk_emit+0x26c/0x290
> >> [  243.288906] softirqs last  enabled at (1012): [<c0101768>] 
> >> __do_softirq+0x500/0x63c
> >> [  243.288978] softirqs last disabled at (1007): [<c01315b4>] 
> >> irq_exit+0x214/0x220
> >> [  243.289055] ---[ end trace eeacda95eb7db60a ]---
> >> [  243.289345] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> >> [  243.289466] asix 3-1:1.0 eth0: Failed to write Medium Mode mode to 
> >> 0x0000: ffffffea
> >> [  243.289540] asix 3-1:1.0 eth0: Link is Down
> >> [  243.482809] usb 1-1: reset high-speed USB device number 2 using 
> >> exynos-ehci
> >> [  243.647251] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
> >> [  244.847161] OOM killer enabled.
> >> [  244.850221] Restarting tasks ... done.
> >> [  244.861372] PM: suspend exit
> >>
> >> real    0m13.050s
> >> user    0m0.000s
> >> sys     0m1.152s
> >> root@target:~#
> >>
> >> It looks that some kind of system suspend/resume integration for phylib 
> >> is not implemented.
> > 
> > Probably it is should be handled only by the asix driver. I'll take a
> > look in to it. Did interface was able to resume after printing some
> > warnings?
> > 
> > Regards,
> > Oleksij
> > 
> 
> Maybe it's a use case for the new mac_managed_pm flag, see
> fba863b81604 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM")
> 

Thx! this is the right one :)

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Marek Szyprowski]

Hi Oleksij,

On 09.06.2021 14:46, Oleksij Rempel wrote:
> On Wed, Jun 09, 2021 at 11:59:23AM +0200, Marek Szyprowski wrote:
>> On 07.06.2021 10:27, Oleksij Rempel wrote:
>>> To be able to use ax88772 with external PHYs and use advantage of
>>> existing PHY drivers, we need to port at least ax88772 part of asix
>>> driver to the phylib framework.
>>>
>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> This patch landed recently in linux-next as commit e532a096be0e ("net:
>> usb: asix: ax88772: add phylib support"). I found that it causes some
>> warnings on boards with those devices, see the following log:
>>
>> root@target:~# time rtcwake -s10 -mmem
>> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:16:41 2021
>> [  231.226579] PM: suspend entry (deep)
>> [  231.231697] Filesystems sync: 0.002 seconds
>> [  231.261761] Freezing user space processes ... (elapsed 0.002 seconds)
>> done.
>> [  231.270526] OOM killer disabled.
>> [  231.273557] Freezing remaining freezable tasks ... (elapsed 0.002
>> seconds) done.
>> [  231.282229] printk: Suspending console(s) (use no_console_suspend to
>> debug)
>> ...
>> [  231.710852] Disabling non-boot CPUs ...
>> ...
>> [  231.901794] Enabling non-boot CPUs ...
>> ...
>> [  232.225640] usb usb3: root hub lost power or was reset
>> [  232.225746] usb usb1: root hub lost power or was reset
>> [  232.225864] usb usb5: root hub lost power or was reset
>> [  232.226206] usb usb6: root hub lost power or was reset
>> [  232.226207] usb usb4: root hub lost power or was reset
>> [  232.297749] usb usb2: root hub lost power or was reset
>> [  232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
>> [  232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access
>> [  232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
>> [  232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
>> [  232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78
>> returns -22
>> [  232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to
>> resume: error -22
>> [  232.563712] usb 1-1: reset high-speed USB device number 2 using
>> exynos-ehci
>> [  232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
>> [  233.730994] OOM killer enabled.
>> [  233.734122] Restarting tasks ... done.
>> [  233.754992] PM: suspend exit
>>
>> real    0m11.546s
>> user    0m0.000s
>> sys     0m0.530s
>> root@target:~# sleep 2
>> root@target:~# time rtcwake -s10 -mmem
>> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:17:02 2021
>> [  241.959608] PM: suspend entry (deep)
>> [  241.963446] Filesystems sync: 0.001 seconds
>> [  241.978619] Freezing user space processes ... (elapsed 0.004 seconds)
>> done.
>> [  241.989199] OOM killer disabled.
>> [  241.992215] Freezing remaining freezable tasks ... (elapsed 0.005
>> seconds) done.
>> [  242.003979] printk: Suspending console(s) (use no_console_suspend to
>> debug)
>> ...
>> [  242.592030] Disabling non-boot CPUs ...
>> ...
>> [  242.879721] Enabling non-boot CPUs ...
>> ...
>> [  243.145870] usb usb3: root hub lost power or was reset
>> [  243.145910] usb usb4: root hub lost power or was reset
>> [  243.147084] usb usb5: root hub lost power or was reset
>> [  243.147157] usb usb6: root hub lost power or was reset
>> [  243.147298] usb usb1: root hub lost power or was reset
>> [  243.217137] usb usb2: root hub lost power or was reset
>> [  243.283807] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
>> [  243.284005] asix 3-1:1.0 eth0: Failed to enable software MII access
>> [  243.285526] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
>> [  243.285676] asix 3-1:1.0 eth0: Failed to read reg index 0x0004: -22
>> [  243.285769] ------------[ cut here ]------------
>> [  243.286011] WARNING: CPU: 2 PID: 2069 at drivers/net/phy/phy.c:916
>> phy_error+0x28/0x68
>> [  243.286115] Modules linked in: cmac bnep mwifiex_sdio mwifiex
>> sha256_generic libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl
>> bluetooth s5p_mfc uvcvideo s5p_jpeg exynos_gsc v
>> [  243.287490] CPU: 2 PID: 2069 Comm: kworker/2:5 Not tainted
>> 5.13.0-rc5-next-20210608 #10443
>> [  243.287555] Hardware name: Samsung Exynos (Flattened Device Tree)
>> [  243.287609] Workqueue: events_power_efficient phy_state_machine
>> [  243.287716] [<c0111920>] (unwind_backtrace) from [<c010d0cc>]
>> (show_stack+0x10/0x14)
>> [  243.287807] [<c010d0cc>] (show_stack) from [<c0b62360>]
>> (dump_stack_lvl+0xa0/0xc0)
>> [  243.287882] [<c0b62360>] (dump_stack_lvl) from [<c0127960>]
>> (__warn+0x118/0x11c)
>> [  243.287954] [<c0127960>] (__warn) from [<c0127a18>]
>> (warn_slowpath_fmt+0xb4/0xbc)
>> [  243.288021] [<c0127a18>] (warn_slowpath_fmt) from [<c0734968>]
>> (phy_error+0x28/0x68)
>> [  243.288094] [<c0734968>] (phy_error) from [<c0735d6c>]
>> (phy_state_machine+0x218/0x278)
>> [  243.288173] [<c0735d6c>] (phy_state_machine) from [<c014ae08>]
>> (process_one_work+0x30c/0x884)
>> [  243.288254] [<c014ae08>] (process_one_work) from [<c014b3d8>]
>> (worker_thread+0x58/0x594)
>> [  243.288333] [<c014b3d8>] (worker_thread) from [<c0153944>]
>> (kthread+0x160/0x1c0)
>> [  243.288408] [<c0153944>] (kthread) from [<c010011c>]
>> (ret_from_fork+0x14/0x38)
>> [  243.288475] Exception stack(0xc4683fb0 to 0xc4683ff8)
>> [  243.288531] 3fa0:                                     00000000
>> 00000000 00000000 00000000
>> [  243.288587] 3fc0: 00000000 00000000 00000000 00000000 00000000
>> 00000000 00000000 00000000
>> [  243.288641] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>> [  243.288690] irq event stamp: 1611
>> [  243.288744] hardirqs last  enabled at (1619): [<c01a6ef0>]
>> vprintk_emit+0x230/0x290
>> [  243.288830] hardirqs last disabled at (1626): [<c01a6f2c>]
>> vprintk_emit+0x26c/0x290
>> [  243.288906] softirqs last  enabled at (1012): [<c0101768>]
>> __do_softirq+0x500/0x63c
>> [  243.288978] softirqs last disabled at (1007): [<c01315b4>]
>> irq_exit+0x214/0x220
>> [  243.289055] ---[ end trace eeacda95eb7db60a ]---
>> [  243.289345] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
>> [  243.289466] asix 3-1:1.0 eth0: Failed to write Medium Mode mode to
>> 0x0000: ffffffea
>> [  243.289540] asix 3-1:1.0 eth0: Link is Down
>> [  243.482809] usb 1-1: reset high-speed USB device number 2 using
>> exynos-ehci
>> [  243.647251] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
>> [  244.847161] OOM killer enabled.
>> [  244.850221] Restarting tasks ... done.
>> [  244.861372] PM: suspend exit
>>
>> real    0m13.050s
>> user    0m0.000s
>> sys     0m1.152s
>> root@target:~#
>>
>> It looks that some kind of system suspend/resume integration for phylib
>> is not implemented.
> Probably it is should be handled only by the asix driver. I'll take a
> look in to it. Did interface was able to resume after printing some
> warnings?

Nope. The network is not operational after suspend/resume cycle after 
applying this patch.

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Jon Hunter]

On 09/06/2021 10:59, Marek Szyprowski wrote:
> Hi Oleksij,
> 
> On 07.06.2021 10:27, Oleksij Rempel wrote:
>> To be able to use ax88772 with external PHYs and use advantage of
>> existing PHY drivers, we need to port at least ax88772 part of asix
>> driver to the phylib framework.
>>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> This patch landed recently in linux-next as commit e532a096be0e ("net: 
> usb: asix: ax88772: add phylib support"). I found that it causes some 
> warnings on boards with those devices, see the following log:
> 
> root@target:~# time rtcwake -s10 -mmem
> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:16:41 2021
> [  231.226579] PM: suspend entry (deep)
> [  231.231697] Filesystems sync: 0.002 seconds
> [  231.261761] Freezing user space processes ... (elapsed 0.002 seconds) 
> done.
> [  231.270526] OOM killer disabled.
> [  231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 
> seconds) done.
> [  231.282229] printk: Suspending console(s) (use no_console_suspend to 
> debug)
> ...
> [  231.710852] Disabling non-boot CPUs ...
> ...
> [  231.901794] Enabling non-boot CPUs ...
> ...
> [  232.225640] usb usb3: root hub lost power or was reset
> [  232.225746] usb usb1: root hub lost power or was reset
> [  232.225864] usb usb5: root hub lost power or was reset
> [  232.226206] usb usb6: root hub lost power or was reset
> [  232.226207] usb usb4: root hub lost power or was reset
> [  232.297749] usb usb2: root hub lost power or was reset
> [  232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> [  232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access
> [  232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
> [  232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> [  232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 
> returns -22
> [  232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to 
> resume: error -22
> [  232.563712] usb 1-1: reset high-speed USB device number 2 using 
> exynos-ehci
> [  232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
> [  233.730994] OOM killer enabled.
> [  233.734122] Restarting tasks ... done.
> [  233.754992] PM: suspend exit


I am seeing a similar problem on a couple of our Tegra boards that
use AX88772A device. When resuming from suspend I see ...

[   54.733266] PM: suspend entry (deep)

[   54.737179] Filesystems sync: 0.000 seconds

[   54.741904] Freezing user space processes ... (elapsed 0.001 seconds) done.

[   54.750895] OOM killer disabled.

[   54.754452] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.

[   54.763505] printk: Suspending console(s) (use no_console_suspend to debug)

[   54.898334] Disabling non-boot CPUs ...

[   54.899546] IRQ 26: no longer affine to CPU1

[   54.924373] Entering suspend state LP1

[   54.924493] Enabling non-boot CPUs ...

[   54.933164] CPU1 is up

[   55.005166] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -113

[   55.005226] asix 3-1:1.0 eth0: Failed to enable software MII access

[   55.006579] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -113

[   55.006722] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -113

[   55.006762] asix 3-1:1.0 eth0: Failed to enable software MII access


Interestingly once commit d275afb66371 ("net: usb: asix: add error
handling for asix_mdio_* functions") is applied, then resume from
suspend completely fails because the error is propagated. Bisect
is pointing to that patch, however, it is this patch that is
causing the problem.

Cheers
Jon

-- 
nvpublic

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Oleksij Rempel]

Hi Marek and Jon,

I just send a patch to fix suspend/resume. It was tested on ax88772A and ax88772C
on iMX6 host. Can you please confirm if it works for you?

Regards,
Oleksij

net: usb: asix: ax88772: manage PHY PM from MAC 

On Thu, Jun 10, 2021 at 01:54:12PM +0100, Jon Hunter wrote:
> 
> On 09/06/2021 10:59, Marek Szyprowski wrote:
> > Hi Oleksij,
> > 
> > On 07.06.2021 10:27, Oleksij Rempel wrote:
> >> To be able to use ax88772 with external PHYs and use advantage of
> >> existing PHY drivers, we need to port at least ax88772 part of asix
> >> driver to the phylib framework.
> >>
> >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > 
> > This patch landed recently in linux-next as commit e532a096be0e ("net: 
> > usb: asix: ax88772: add phylib support"). I found that it causes some 
> > warnings on boards with those devices, see the following log:
> > 
> > root@target:~# time rtcwake -s10 -mmem
> > rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun  9 08:16:41 2021
> > [  231.226579] PM: suspend entry (deep)
> > [  231.231697] Filesystems sync: 0.002 seconds
> > [  231.261761] Freezing user space processes ... (elapsed 0.002 seconds) 
> > done.
> > [  231.270526] OOM killer disabled.
> > [  231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 
> > seconds) done.
> > [  231.282229] printk: Suspending console(s) (use no_console_suspend to 
> > debug)
> > ...
> > [  231.710852] Disabling non-boot CPUs ...
> > ...
> > [  231.901794] Enabling non-boot CPUs ...
> > ...
> > [  232.225640] usb usb3: root hub lost power or was reset
> > [  232.225746] usb usb1: root hub lost power or was reset
> > [  232.225864] usb usb5: root hub lost power or was reset
> > [  232.226206] usb usb6: root hub lost power or was reset
> > [  232.226207] usb usb4: root hub lost power or was reset
> > [  232.297749] usb usb2: root hub lost power or was reset
> > [  232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> > [  232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access
> > [  232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22
> > [  232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22
> > [  232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 
> > returns -22
> > [  232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to 
> > resume: error -22
> > [  232.563712] usb 1-1: reset high-speed USB device number 2 using 
> > exynos-ehci
> > [  232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd
> > [  233.730994] OOM killer enabled.
> > [  233.734122] Restarting tasks ... done.
> > [  233.754992] PM: suspend exit
> 
> 
> I am seeing a similar problem on a couple of our Tegra boards that
> use AX88772A device. When resuming from suspend I see ...
> 
> [   54.733266] PM: suspend entry (deep)
> 
> [   54.737179] Filesystems sync: 0.000 seconds
> 
> [   54.741904] Freezing user space processes ... (elapsed 0.001 seconds) done.
> 
> [   54.750895] OOM killer disabled.
> 
> [   54.754452] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> 
> [   54.763505] printk: Suspending console(s) (use no_console_suspend to debug)
> 
> [   54.898334] Disabling non-boot CPUs ...
> 
> [   54.899546] IRQ 26: no longer affine to CPU1
> 
> [   54.924373] Entering suspend state LP1
> 
> [   54.924493] Enabling non-boot CPUs ...
> 
> [   54.933164] CPU1 is up
> 
> [   55.005166] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -113
> 
> [   55.005226] asix 3-1:1.0 eth0: Failed to enable software MII access
> 
> [   55.006579] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -113
> 
> [   55.006722] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -113
> 
> [   55.006762] asix 3-1:1.0 eth0: Failed to enable software MII access
> 
> 
> Interestingly once commit d275afb66371 ("net: usb: asix: add error
> handling for asix_mdio_* functions") is applied, then resume from
> suspend completely fails because the error is propagated. Bisect
> is pointing to that patch, however, it is this patch that is
> causing the problem.
> 
> Cheers
> Jon
> 
> -- 
> nvpublic
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Marek Szyprowski]

Hi Oleksij,

On 07.06.2021 10:27, Oleksij Rempel wrote:
> To be able to use ax88772 with external PHYs and use advantage of
> existing PHY drivers, we need to port at least ax88772 part of asix
> driver to the phylib framework.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

I found one more issue with this patch. On one of my test boards 
(Samsung Exynos5250 SoC based Arndale) system fails to establish network 
connection just after starting the kernel when the driver is build-in.

--->8---
# dmesg | grep asix
[    2.761928] usbcore: registered new interface driver asix
[    5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): 
invalid hw address, using random
[    6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at 
usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 7a:9b:9a:f2:94:8e
[   14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
control off
# ping -c2  host
PING host (192.168.100.1) 56(84) bytes of data.
 From 192.168.100.20 icmp_seq=1 Destination Host Unreachable
 From 192.168.100.20 icmp_seq=2 Destination Host Unreachable

--- host ping statistics ---
2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 59ms
--->8---

Calling ifup eth0 && ifdown eth0 fixes the network status:

--->8---
# ifdown eth0 && ifup eth0
[   60.474929] asix 1-3.2.4:1.0 eth0: Link is Down
[   60.623516] asix 1-3.2.4:1.0 eth0: Link is Down
[   62.774304] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[   62.786354] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
control off
# ping -c2 host
PING host (192.168.100.1) 56(84) bytes of data.
64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=1.25 ms
64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.853 ms

--- host ping statistics ---
2 packets transmitted, 2 received, 0% packet loss, time 3ms
rtt min/avg/max/mdev = 0.853/1.053/1.254/0.203 ms
--->8---

When driver is loaded as a module (and without any other modules, so 
this is not a dependency issue), the connection is established properly 
just after the boot:

--->8---
# dmesg | grep asix
[   13.633284] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): 
invalid hw address, using random
[   15.390350] asix 1-3.2.4:1.0 eth0: register 'asix' at 
usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 3a:51:11:08:aa:ea
[   15.414052] usbcore: registered new interface driver asix
[   15.832564] asix 1-3.2.4:1.0 eth0: Link is Down
[   18.053747] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
control off
# ping -c2 host
PING host (192.168.100.1) 56(84) bytes of data.
64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=0.545 ms
64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.742 ms

--- host ping statistics ---
2 packets transmitted, 2 received, 0% packet loss, time 3ms
rtt min/avg/max/mdev = 0.545/0.643/0.742/0.101 ms

--->8---

Let me know if I can make any other tests that would help fixing this issue.

> ---
>   drivers/net/usb/asix.h         |   9 +++
>   drivers/net/usb/asix_common.c  |  37 ++++++++++
>   drivers/net/usb/asix_devices.c | 120 +++++++++++++++++++++------------
>   drivers/net/usb/ax88172a.c     |  14 ----
>   4 files changed, 122 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
> index edb94efd265e..2122d302e643 100644
> --- a/drivers/net/usb/asix.h
> +++ b/drivers/net/usb/asix.h
> @@ -25,6 +25,7 @@
>   #include <linux/usb/usbnet.h>
>   #include <linux/slab.h>
>   #include <linux/if_vlan.h>
> +#include <linux/phy.h>
>   
>   #define DRIVER_VERSION "22-Dec-2011"
>   #define DRIVER_NAME "asix"
> @@ -178,6 +179,10 @@ struct asix_common_private {
>   	u16 presvd_phy_advertise;
>   	u16 presvd_phy_bmcr;
>   	struct asix_rx_fixup_info rx_fixup_info;
> +	struct mii_bus *mdio;
> +	struct phy_device *phydev;
> +	u16 phy_addr;
> +	char phy_name[20];
>   };
>   
>   extern const struct driver_info ax88172a_info;
> @@ -214,6 +219,7 @@ int asix_write_rx_ctl(struct usbnet *dev, u16 mode, int in_pm);
>   
>   u16 asix_read_medium_status(struct usbnet *dev, int in_pm);
>   int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm);
> +void asix_adjust_link(struct net_device *netdev);
>   
>   int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm);
>   
> @@ -222,6 +228,9 @@ void asix_set_multicast(struct net_device *net);
>   int asix_mdio_read(struct net_device *netdev, int phy_id, int loc);
>   void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val);
>   
> +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum);
> +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val);
> +
>   int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc);
>   void asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc,
>   			  int val);
> diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
> index e1109f1a8dd5..085bc8281082 100644
> --- a/drivers/net/usb/asix_common.c
> +++ b/drivers/net/usb/asix_common.c
> @@ -384,6 +384,27 @@ int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm)
>   	return ret;
>   }
>   
> +/* set MAC link settings according to information from phylib */
> +void asix_adjust_link(struct net_device *netdev)
> +{
> +	struct phy_device *phydev = netdev->phydev;
> +	struct usbnet *dev = netdev_priv(netdev);
> +	u16 mode = 0;
> +
> +	if (phydev->link) {
> +		mode = AX88772_MEDIUM_DEFAULT;
> +
> +		if (phydev->duplex == DUPLEX_HALF)
> +			mode &= ~AX_MEDIUM_FD;
> +
> +		if (phydev->speed != SPEED_100)
> +			mode &= ~AX_MEDIUM_PS;
> +	}
> +
> +	asix_write_medium_mode(dev, mode, 0);
> +	phy_print_status(phydev);
> +}
> +
>   int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
>   {
>   	int ret;
> @@ -506,6 +527,22 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
>   	mutex_unlock(&dev->phy_mutex);
>   }
>   
> +/* MDIO read and write wrappers for phylib */
> +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
> +{
> +	struct usbnet *priv = bus->priv;
> +
> +	return asix_mdio_read(priv->net, phy_id, regnum);
> +}
> +
> +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
> +{
> +	struct usbnet *priv = bus->priv;
> +
> +	asix_mdio_write(priv->net, phy_id, regnum, val);
> +	return 0;
> +}
> +
>   int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
>   {
>   	struct usbnet *dev = netdev_priv(netdev);
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index 00b6ac0570eb..e4cd85e38edd 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -285,7 +285,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
>   
>   static const struct ethtool_ops ax88772_ethtool_ops = {
>   	.get_drvinfo		= asix_get_drvinfo,
> -	.get_link		= asix_get_link,
> +	.get_link		= usbnet_get_link,
>   	.get_msglevel		= usbnet_get_msglevel,
>   	.set_msglevel		= usbnet_set_msglevel,
>   	.get_wol		= asix_get_wol,
> @@ -293,37 +293,15 @@ static const struct ethtool_ops ax88772_ethtool_ops = {
>   	.get_eeprom_len		= asix_get_eeprom_len,
>   	.get_eeprom		= asix_get_eeprom,
>   	.set_eeprom		= asix_set_eeprom,
> -	.nway_reset		= usbnet_nway_reset,
> -	.get_link_ksettings	= usbnet_get_link_ksettings_mii,
> -	.set_link_ksettings	= usbnet_set_link_ksettings_mii,
> +	.nway_reset		= phy_ethtool_nway_reset,
> +	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
> +	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
>   };
>   
> -static int ax88772_link_reset(struct usbnet *dev)
> -{
> -	u16 mode;
> -	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
> -
> -	mii_check_media(&dev->mii, 1, 1);
> -	mii_ethtool_gset(&dev->mii, &ecmd);
> -	mode = AX88772_MEDIUM_DEFAULT;
> -
> -	if (ethtool_cmd_speed(&ecmd) != SPEED_100)
> -		mode &= ~AX_MEDIUM_PS;
> -
> -	if (ecmd.duplex != DUPLEX_FULL)
> -		mode &= ~AX_MEDIUM_FD;
> -
> -	netdev_dbg(dev->net, "ax88772_link_reset() speed: %u duplex: %d setting mode to 0x%04x\n",
> -		   ethtool_cmd_speed(&ecmd), ecmd.duplex, mode);
> -
> -	asix_write_medium_mode(dev, mode, 0);
> -
> -	return 0;
> -}
> -
>   static int ax88772_reset(struct usbnet *dev)
>   {
>   	struct asix_data *data = (struct asix_data *)&dev->data;
> +	struct asix_common_private *priv = dev->driver_priv;
>   	int ret;
>   
>   	/* Rewrite MAC address */
> @@ -342,6 +320,8 @@ static int ax88772_reset(struct usbnet *dev)
>   	if (ret < 0)
>   		goto out;
>   
> +	phy_start(priv->phydev);
> +
>   	return 0;
>   
>   out:
> @@ -586,7 +566,7 @@ static const struct net_device_ops ax88772_netdev_ops = {
>   	.ndo_get_stats64	= dev_get_tstats64,
>   	.ndo_set_mac_address 	= asix_set_mac_address,
>   	.ndo_validate_addr	= eth_validate_addr,
> -	.ndo_do_ioctl		= asix_ioctl,
> +	.ndo_do_ioctl		= phy_do_ioctl_running,
>   	.ndo_set_rx_mode        = asix_set_multicast,
>   };
>   
> @@ -677,12 +657,57 @@ static int asix_resume(struct usb_interface *intf)
>   	return usbnet_resume(intf);
>   }
>   
> +static int ax88772_init_mdio(struct usbnet *dev)
> +{
> +	struct asix_common_private *priv = dev->driver_priv;
> +
> +	priv->mdio = devm_mdiobus_alloc(&dev->udev->dev);
> +	if (!priv->mdio)
> +		return -ENOMEM;
> +
> +	priv->mdio->priv = dev;
> +	priv->mdio->read = &asix_mdio_bus_read;
> +	priv->mdio->write = &asix_mdio_bus_write;
> +	priv->mdio->name = "Asix MDIO Bus";
> +	/* mii bus name is usb-<usb bus number>-<usb device number> */
> +	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
> +		 dev->udev->bus->busnum, dev->udev->devnum);
> +
> +	return devm_mdiobus_register(&dev->udev->dev, priv->mdio);
> +}
> +
> +static int ax88772_init_phy(struct usbnet *dev)
> +{
> +	struct asix_common_private *priv = dev->driver_priv;
> +	int ret;
> +
> +	priv->phy_addr = asix_read_phy_addr(dev, true);
> +	if (priv->phy_addr < 0)
> +		return priv->phy_addr;
> +
> +	snprintf(priv->phy_name, sizeof(priv->phy_name), PHY_ID_FMT,
> +		 priv->mdio->id, priv->phy_addr);
> +
> +	priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link,
> +				   PHY_INTERFACE_MODE_INTERNAL);
> +	if (IS_ERR(priv->phydev)) {
> +		netdev_err(dev->net, "Could not connect to PHY device %s\n",
> +			   priv->phy_name);
> +		ret = PTR_ERR(priv->phydev);
> +		return ret;
> +	}
> +
> +	phy_attached_info(priv->phydev);
> +
> +	return 0;
> +}
> +
>   static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>   {
> -	int ret, i;
>   	u8 buf[ETH_ALEN] = {0}, chipcode = 0;
> -	u32 phyid;
>   	struct asix_common_private *priv;
> +	int ret, i;
> +	u32 phyid;
>   
>   	usbnet_get_endpoints(dev, intf);
>   
> @@ -714,17 +739,6 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>   
>   	asix_set_netdev_dev_addr(dev, buf);
>   
> -	/* Initialize MII structure */
> -	dev->mii.dev = dev->net;
> -	dev->mii.mdio_read = asix_mdio_read;
> -	dev->mii.mdio_write = asix_mdio_write;
> -	dev->mii.phy_id_mask = 0x1f;
> -	dev->mii.reg_num_mask = 0x1f;
> -
> -	dev->mii.phy_id = asix_read_phy_addr(dev, true);
> -	if (dev->mii.phy_id < 0)
> -		return dev->mii.phy_id;
> -
>   	dev->net->netdev_ops = &ax88772_netdev_ops;
>   	dev->net->ethtool_ops = &ax88772_ethtool_ops;
>   	dev->net->needed_headroom = 4; /* cf asix_tx_fixup() */
> @@ -768,11 +782,31 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>   		priv->suspend = ax88772_suspend;
>   	}
>   
> +	ret = ax88772_init_mdio(dev);
> +	if (ret)
> +		return ret;
> +
> +	return ax88772_init_phy(dev);
> +}
> +
> +static int ax88772_stop(struct usbnet *dev)
> +{
> +	struct asix_common_private *priv = dev->driver_priv;
> +
> +	/* On unplugged USB, we will get MDIO communication errors and the
> +	 * PHY will be set in to PHY_HALTED state.
> +	 */
> +	if (priv->phydev->state != PHY_HALTED)
> +		phy_stop(priv->phydev);
> +
>   	return 0;
>   }
>   
>   static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
>   {
> +	struct asix_common_private *priv = dev->driver_priv;
> +
> +	phy_disconnect(priv->phydev);
>   	asix_rx_fixup_common_free(dev->driver_priv);
>   }
>   
> @@ -1161,8 +1195,8 @@ static const struct driver_info ax88772_info = {
>   	.bind = ax88772_bind,
>   	.unbind = ax88772_unbind,
>   	.status = asix_status,
> -	.link_reset = ax88772_link_reset,
>   	.reset = ax88772_reset,
> +	.stop = ax88772_stop,
>   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
>   	.rx_fixup = asix_rx_fixup_common,
>   	.tx_fixup = asix_tx_fixup,
> @@ -1173,7 +1207,6 @@ static const struct driver_info ax88772b_info = {
>   	.bind = ax88772_bind,
>   	.unbind = ax88772_unbind,
>   	.status = asix_status,
> -	.link_reset = ax88772_link_reset,
>   	.reset = ax88772_reset,
>   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
>   	         FLAG_MULTI_PACKET,
> @@ -1209,7 +1242,6 @@ static const struct driver_info hg20f9_info = {
>   	.bind = ax88772_bind,
>   	.unbind = ax88772_unbind,
>   	.status = asix_status,
> -	.link_reset = ax88772_link_reset,
>   	.reset = ax88772_reset,
>   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
>   	         FLAG_MULTI_PACKET,
> diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
> index c8ca5187eece..2e2081346740 100644
> --- a/drivers/net/usb/ax88172a.c
> +++ b/drivers/net/usb/ax88172a.c
> @@ -25,20 +25,6 @@ struct ax88172a_private {
>   	struct asix_rx_fixup_info rx_fixup_info;
>   };
>   
> -/* MDIO read and write wrappers for phylib */
> -static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
> -{
> -	return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id,
> -			      regnum);
> -}
> -
> -static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum,
> -			       u16 val)
> -{
> -	asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val);
> -	return 0;
> -}
> -
>   /* set MAC link settings according to information from phylib */
>   static void ax88172a_adjust_link(struct net_device *netdev)
>   {

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Oleksij Rempel]

Hi Marek,

thank you for your feedback.

On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
> Hi Oleksij,
> 
> On 07.06.2021 10:27, Oleksij Rempel wrote:
> > To be able to use ax88772 with external PHYs and use advantage of
> > existing PHY drivers, we need to port at least ax88772 part of asix
> > driver to the phylib framework.
> >
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> I found one more issue with this patch. On one of my test boards 
> (Samsung Exynos5250 SoC based Arndale) system fails to establish network 
> connection just after starting the kernel when the driver is build-in.
> 
> --->8---
> # dmesg | grep asix
> [    2.761928] usbcore: registered new interface driver asix
> [    5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): 
> invalid hw address, using random
> [    6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at 
> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 7a:9b:9a:f2:94:8e
> [   14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
> control off
> # ping -c2  host
> PING host (192.168.100.1) 56(84) bytes of data.
>  From 192.168.100.20 icmp_seq=1 Destination Host Unreachable
>  From 192.168.100.20 icmp_seq=2 Destination Host Unreachable
> 
> --- host ping statistics ---
> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 59ms
> --->8---

Hm... it looks like different chip variant. My is registered as
"ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the
difference. Can you please tell me more about this adapter and if possible open
tell the real part name.

I can imagine that this adapter may using generic PHY driver.
Can you please confirm it by dmesg | grep PHY?
In my case i'll get:
Asix Electronics AX88772C usb-001:003:10: attached PHY driver (mii_bus:phy_addr=usb-001:003:10, irq=POLL)

If you have a different PHY, can you please send me the PHY id:
cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id

Your usb path will probably be different.

> Calling ifup eth0 && ifdown eth0 fixes the network status:
> 
> --->8---
> # ifdown eth0 && ifup eth0
> [   60.474929] asix 1-3.2.4:1.0 eth0: Link is Down
> [   60.623516] asix 1-3.2.4:1.0 eth0: Link is Down
> [   62.774304] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> [   62.786354] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
> control off
> # ping -c2 host
> PING host (192.168.100.1) 56(84) bytes of data.
> 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=1.25 ms
> 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.853 ms
> 
> --- host ping statistics ---
> 2 packets transmitted, 2 received, 0% packet loss, time 3ms
> rtt min/avg/max/mdev = 0.853/1.053/1.254/0.203 ms
> --->8---
> 
> When driver is loaded as a module (and without any other modules, so 
> this is not a dependency issue), the connection is established properly 
> just after the boot:
> 
> --->8---
> # dmesg | grep asix
> [   13.633284] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): 
> invalid hw address, using random
> [   15.390350] asix 1-3.2.4:1.0 eth0: register 'asix' at 
> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 3a:51:11:08:aa:ea
> [   15.414052] usbcore: registered new interface driver asix
> [   15.832564] asix 1-3.2.4:1.0 eth0: Link is Down
> [   18.053747] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
> control off
> # ping -c2 host
> PING host (192.168.100.1) 56(84) bytes of data.
> 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=0.545 ms
> 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.742 ms
> 
> --- host ping statistics ---
> 2 packets transmitted, 2 received, 0% packet loss, time 3ms
> rtt min/avg/max/mdev = 0.545/0.643/0.742/0.101 ms
> 
> --->8---
> 
> Let me know if I can make any other tests that would help fixing this issue.
> 
> > ---
> >   drivers/net/usb/asix.h         |   9 +++
> >   drivers/net/usb/asix_common.c  |  37 ++++++++++
> >   drivers/net/usb/asix_devices.c | 120 +++++++++++++++++++++------------
> >   drivers/net/usb/ax88172a.c     |  14 ----
> >   4 files changed, 122 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
> > index edb94efd265e..2122d302e643 100644
> > --- a/drivers/net/usb/asix.h
> > +++ b/drivers/net/usb/asix.h
> > @@ -25,6 +25,7 @@
> >   #include <linux/usb/usbnet.h>
> >   #include <linux/slab.h>
> >   #include <linux/if_vlan.h>
> > +#include <linux/phy.h>
> >   
> >   #define DRIVER_VERSION "22-Dec-2011"
> >   #define DRIVER_NAME "asix"
> > @@ -178,6 +179,10 @@ struct asix_common_private {
> >   	u16 presvd_phy_advertise;
> >   	u16 presvd_phy_bmcr;
> >   	struct asix_rx_fixup_info rx_fixup_info;
> > +	struct mii_bus *mdio;
> > +	struct phy_device *phydev;
> > +	u16 phy_addr;
> > +	char phy_name[20];
> >   };
> >   
> >   extern const struct driver_info ax88172a_info;
> > @@ -214,6 +219,7 @@ int asix_write_rx_ctl(struct usbnet *dev, u16 mode, int in_pm);
> >   
> >   u16 asix_read_medium_status(struct usbnet *dev, int in_pm);
> >   int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm);
> > +void asix_adjust_link(struct net_device *netdev);
> >   
> >   int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm);
> >   
> > @@ -222,6 +228,9 @@ void asix_set_multicast(struct net_device *net);
> >   int asix_mdio_read(struct net_device *netdev, int phy_id, int loc);
> >   void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val);
> >   
> > +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum);
> > +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val);
> > +
> >   int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc);
> >   void asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc,
> >   			  int val);
> > diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
> > index e1109f1a8dd5..085bc8281082 100644
> > --- a/drivers/net/usb/asix_common.c
> > +++ b/drivers/net/usb/asix_common.c
> > @@ -384,6 +384,27 @@ int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm)
> >   	return ret;
> >   }
> >   
> > +/* set MAC link settings according to information from phylib */
> > +void asix_adjust_link(struct net_device *netdev)
> > +{
> > +	struct phy_device *phydev = netdev->phydev;
> > +	struct usbnet *dev = netdev_priv(netdev);
> > +	u16 mode = 0;
> > +
> > +	if (phydev->link) {
> > +		mode = AX88772_MEDIUM_DEFAULT;
> > +
> > +		if (phydev->duplex == DUPLEX_HALF)
> > +			mode &= ~AX_MEDIUM_FD;
> > +
> > +		if (phydev->speed != SPEED_100)
> > +			mode &= ~AX_MEDIUM_PS;
> > +	}
> > +
> > +	asix_write_medium_mode(dev, mode, 0);
> > +	phy_print_status(phydev);
> > +}
> > +
> >   int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
> >   {
> >   	int ret;
> > @@ -506,6 +527,22 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
> >   	mutex_unlock(&dev->phy_mutex);
> >   }
> >   
> > +/* MDIO read and write wrappers for phylib */
> > +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
> > +{
> > +	struct usbnet *priv = bus->priv;
> > +
> > +	return asix_mdio_read(priv->net, phy_id, regnum);
> > +}
> > +
> > +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
> > +{
> > +	struct usbnet *priv = bus->priv;
> > +
> > +	asix_mdio_write(priv->net, phy_id, regnum, val);
> > +	return 0;
> > +}
> > +
> >   int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
> >   {
> >   	struct usbnet *dev = netdev_priv(netdev);
> > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> > index 00b6ac0570eb..e4cd85e38edd 100644
> > --- a/drivers/net/usb/asix_devices.c
> > +++ b/drivers/net/usb/asix_devices.c
> > @@ -285,7 +285,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
> >   
> >   static const struct ethtool_ops ax88772_ethtool_ops = {
> >   	.get_drvinfo		= asix_get_drvinfo,
> > -	.get_link		= asix_get_link,
> > +	.get_link		= usbnet_get_link,
> >   	.get_msglevel		= usbnet_get_msglevel,
> >   	.set_msglevel		= usbnet_set_msglevel,
> >   	.get_wol		= asix_get_wol,
> > @@ -293,37 +293,15 @@ static const struct ethtool_ops ax88772_ethtool_ops = {
> >   	.get_eeprom_len		= asix_get_eeprom_len,
> >   	.get_eeprom		= asix_get_eeprom,
> >   	.set_eeprom		= asix_set_eeprom,
> > -	.nway_reset		= usbnet_nway_reset,
> > -	.get_link_ksettings	= usbnet_get_link_ksettings_mii,
> > -	.set_link_ksettings	= usbnet_set_link_ksettings_mii,
> > +	.nway_reset		= phy_ethtool_nway_reset,
> > +	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
> > +	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
> >   };
> >   
> > -static int ax88772_link_reset(struct usbnet *dev)
> > -{
> > -	u16 mode;
> > -	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
> > -
> > -	mii_check_media(&dev->mii, 1, 1);
> > -	mii_ethtool_gset(&dev->mii, &ecmd);
> > -	mode = AX88772_MEDIUM_DEFAULT;
> > -
> > -	if (ethtool_cmd_speed(&ecmd) != SPEED_100)
> > -		mode &= ~AX_MEDIUM_PS;
> > -
> > -	if (ecmd.duplex != DUPLEX_FULL)
> > -		mode &= ~AX_MEDIUM_FD;
> > -
> > -	netdev_dbg(dev->net, "ax88772_link_reset() speed: %u duplex: %d setting mode to 0x%04x\n",
> > -		   ethtool_cmd_speed(&ecmd), ecmd.duplex, mode);
> > -
> > -	asix_write_medium_mode(dev, mode, 0);
> > -
> > -	return 0;
> > -}
> > -
> >   static int ax88772_reset(struct usbnet *dev)
> >   {
> >   	struct asix_data *data = (struct asix_data *)&dev->data;
> > +	struct asix_common_private *priv = dev->driver_priv;
> >   	int ret;
> >   
> >   	/* Rewrite MAC address */
> > @@ -342,6 +320,8 @@ static int ax88772_reset(struct usbnet *dev)
> >   	if (ret < 0)
> >   		goto out;
> >   
> > +	phy_start(priv->phydev);
> > +
> >   	return 0;
> >   
> >   out:
> > @@ -586,7 +566,7 @@ static const struct net_device_ops ax88772_netdev_ops = {
> >   	.ndo_get_stats64	= dev_get_tstats64,
> >   	.ndo_set_mac_address 	= asix_set_mac_address,
> >   	.ndo_validate_addr	= eth_validate_addr,
> > -	.ndo_do_ioctl		= asix_ioctl,
> > +	.ndo_do_ioctl		= phy_do_ioctl_running,
> >   	.ndo_set_rx_mode        = asix_set_multicast,
> >   };
> >   
> > @@ -677,12 +657,57 @@ static int asix_resume(struct usb_interface *intf)
> >   	return usbnet_resume(intf);
> >   }
> >   
> > +static int ax88772_init_mdio(struct usbnet *dev)
> > +{
> > +	struct asix_common_private *priv = dev->driver_priv;
> > +
> > +	priv->mdio = devm_mdiobus_alloc(&dev->udev->dev);
> > +	if (!priv->mdio)
> > +		return -ENOMEM;
> > +
> > +	priv->mdio->priv = dev;
> > +	priv->mdio->read = &asix_mdio_bus_read;
> > +	priv->mdio->write = &asix_mdio_bus_write;
> > +	priv->mdio->name = "Asix MDIO Bus";
> > +	/* mii bus name is usb-<usb bus number>-<usb device number> */
> > +	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
> > +		 dev->udev->bus->busnum, dev->udev->devnum);
> > +
> > +	return devm_mdiobus_register(&dev->udev->dev, priv->mdio);
> > +}
> > +
> > +static int ax88772_init_phy(struct usbnet *dev)
> > +{
> > +	struct asix_common_private *priv = dev->driver_priv;
> > +	int ret;
> > +
> > +	priv->phy_addr = asix_read_phy_addr(dev, true);
> > +	if (priv->phy_addr < 0)
> > +		return priv->phy_addr;
> > +
> > +	snprintf(priv->phy_name, sizeof(priv->phy_name), PHY_ID_FMT,
> > +		 priv->mdio->id, priv->phy_addr);
> > +
> > +	priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link,
> > +				   PHY_INTERFACE_MODE_INTERNAL);
> > +	if (IS_ERR(priv->phydev)) {
> > +		netdev_err(dev->net, "Could not connect to PHY device %s\n",
> > +			   priv->phy_name);
> > +		ret = PTR_ERR(priv->phydev);
> > +		return ret;
> > +	}
> > +
> > +	phy_attached_info(priv->phydev);
> > +
> > +	return 0;
> > +}
> > +
> >   static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
> >   {
> > -	int ret, i;
> >   	u8 buf[ETH_ALEN] = {0}, chipcode = 0;
> > -	u32 phyid;
> >   	struct asix_common_private *priv;
> > +	int ret, i;
> > +	u32 phyid;
> >   
> >   	usbnet_get_endpoints(dev, intf);
> >   
> > @@ -714,17 +739,6 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
> >   
> >   	asix_set_netdev_dev_addr(dev, buf);
> >   
> > -	/* Initialize MII structure */
> > -	dev->mii.dev = dev->net;
> > -	dev->mii.mdio_read = asix_mdio_read;
> > -	dev->mii.mdio_write = asix_mdio_write;
> > -	dev->mii.phy_id_mask = 0x1f;
> > -	dev->mii.reg_num_mask = 0x1f;
> > -
> > -	dev->mii.phy_id = asix_read_phy_addr(dev, true);
> > -	if (dev->mii.phy_id < 0)
> > -		return dev->mii.phy_id;
> > -
> >   	dev->net->netdev_ops = &ax88772_netdev_ops;
> >   	dev->net->ethtool_ops = &ax88772_ethtool_ops;
> >   	dev->net->needed_headroom = 4; /* cf asix_tx_fixup() */
> > @@ -768,11 +782,31 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
> >   		priv->suspend = ax88772_suspend;
> >   	}
> >   
> > +	ret = ax88772_init_mdio(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return ax88772_init_phy(dev);
> > +}
> > +
> > +static int ax88772_stop(struct usbnet *dev)
> > +{
> > +	struct asix_common_private *priv = dev->driver_priv;
> > +
> > +	/* On unplugged USB, we will get MDIO communication errors and the
> > +	 * PHY will be set in to PHY_HALTED state.
> > +	 */
> > +	if (priv->phydev->state != PHY_HALTED)
> > +		phy_stop(priv->phydev);
> > +
> >   	return 0;
> >   }
> >   
> >   static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
> >   {
> > +	struct asix_common_private *priv = dev->driver_priv;
> > +
> > +	phy_disconnect(priv->phydev);
> >   	asix_rx_fixup_common_free(dev->driver_priv);
> >   }
> >   
> > @@ -1161,8 +1195,8 @@ static const struct driver_info ax88772_info = {
> >   	.bind = ax88772_bind,
> >   	.unbind = ax88772_unbind,
> >   	.status = asix_status,
> > -	.link_reset = ax88772_link_reset,
> >   	.reset = ax88772_reset,
> > +	.stop = ax88772_stop,
> >   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
> >   	.rx_fixup = asix_rx_fixup_common,
> >   	.tx_fixup = asix_tx_fixup,
> > @@ -1173,7 +1207,6 @@ static const struct driver_info ax88772b_info = {
> >   	.bind = ax88772_bind,
> >   	.unbind = ax88772_unbind,
> >   	.status = asix_status,
> > -	.link_reset = ax88772_link_reset,
> >   	.reset = ax88772_reset,
> >   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
> >   	         FLAG_MULTI_PACKET,
> > @@ -1209,7 +1242,6 @@ static const struct driver_info hg20f9_info = {
> >   	.bind = ax88772_bind,
> >   	.unbind = ax88772_unbind,
> >   	.status = asix_status,
> > -	.link_reset = ax88772_link_reset,
> >   	.reset = ax88772_reset,
> >   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
> >   	         FLAG_MULTI_PACKET,
> > diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
> > index c8ca5187eece..2e2081346740 100644
> > --- a/drivers/net/usb/ax88172a.c
> > +++ b/drivers/net/usb/ax88172a.c
> > @@ -25,20 +25,6 @@ struct ax88172a_private {
> >   	struct asix_rx_fixup_info rx_fixup_info;
> >   };
> >   
> > -/* MDIO read and write wrappers for phylib */
> > -static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
> > -{
> > -	return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id,
> > -			      regnum);
> > -}
> > -
> > -static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum,
> > -			       u16 val)
> > -{
> > -	asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val);
> > -	return 0;
> > -}
> > -
> >   /* set MAC link settings according to information from phylib */
> >   static void ax88172a_adjust_link(struct net_device *netdev)
> >   {
> 
> Best regards
> -- 
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Marek Szyprowski]

Hi Oleksij,

On 18.06.2021 12:13, Oleksij Rempel wrote:
> thank you for your feedback.
>
> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
>> On 07.06.2021 10:27, Oleksij Rempel wrote:
>>> To be able to use ax88772 with external PHYs and use advantage of
>>> existing PHY drivers, we need to port at least ax88772 part of asix
>>> driver to the phylib framework.
>>>
>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> I found one more issue with this patch. On one of my test boards
>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network
>> connection just after starting the kernel when the driver is build-in.
>>
>> --->8---
>> # dmesg | grep asix
>> [    2.761928] usbcore: registered new interface driver asix
>> [    5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized):
>> invalid hw address, using random
>> [    6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at
>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 7a:9b:9a:f2:94:8e
>> [   14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
>> control off
>> # ping -c2  host
>> PING host (192.168.100.1) 56(84) bytes of data.
>>   From 192.168.100.20 icmp_seq=1 Destination Host Unreachable
>>   From 192.168.100.20 icmp_seq=2 Destination Host Unreachable
>>
>> --- host ping statistics ---
>> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 59ms
>> --->8---
> Hm... it looks like different chip variant. My is registered as
> "ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the
> difference. Can you please tell me more about this adapter and if possible open
> tell the real part name.
Well, currently I have only remote access to that board. The network 
chip is soldered on board. Maybe you can read something from the photo 
on the wiki page: https://en.wikipedia.org/wiki/Arndale_Board
> I can imagine that this adapter may using generic PHY driver.
> Can you please confirm it by dmesg | grep PHY?
> In my case i'll get:
> Asix Electronics AX88772C usb-001:003:10: attached PHY driver (mii_bus:phy_addr=usb-001:003:10, irq=POLL)
# dmesg | grep PHY
[    5.700274] Asix Electronics AX88772A usb-001:004:10: attached PHY 
driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL)
> If you have a different PHY, can you please send me the PHY id:
> cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id
>
> Your usb path will probably be different.

# cat /sys/bus/mdio_bus/devices/usb-001\:004\:10/phy_id
0x003b1861

 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Marek Szyprowski]

On 18.06.2021 12:45, Marek Szyprowski wrote:
> On 18.06.2021 12:13, Oleksij Rempel wrote:
>> thank you for your feedback.
>>
>> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
>>> On 07.06.2021 10:27, Oleksij Rempel wrote:
>>>> To be able to use ax88772 with external PHYs and use advantage of
>>>> existing PHY drivers, we need to port at least ax88772 part of asix
>>>> driver to the phylib framework.
>>>>
>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>> I found one more issue with this patch. On one of my test boards
>>> (Samsung Exynos5250 SoC based Arndale) system fails to establish 
>>> network
>>> connection just after starting the kernel when the driver is build-in.
>>>
>>> --->8---
>>> # dmesg | grep asix
>>> [    2.761928] usbcore: registered new interface driver asix
>>> [    5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized):
>>> invalid hw address, using random
>>> [    6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at
>>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 
>>> 7a:9b:9a:f2:94:8e
>>> [   14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
>>> control off
>>> # ping -c2  host
>>> PING host (192.168.100.1) 56(84) bytes of data.
>>>   From 192.168.100.20 icmp_seq=1 Destination Host Unreachable
>>>   From 192.168.100.20 icmp_seq=2 Destination Host Unreachable
>>>
>>> --- host ping statistics ---
>>> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 
>>> 59ms
>>> --->8---
>> Hm... it looks like different chip variant. My is registered as
>> "ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the
>> difference. Can you please tell me more about this adapter and if 
>> possible open
>> tell the real part name.
> Well, currently I have only remote access to that board. The network 
> chip is soldered on board. Maybe you can read something from the photo 
> on the wiki page: https://en.wikipedia.org/wiki/Arndale_Board
>> I can imagine that this adapter may using generic PHY driver.
>> Can you please confirm it by dmesg | grep PHY?
>> In my case i'll get:
>> Asix Electronics AX88772C usb-001:003:10: attached PHY driver 
>> (mii_bus:phy_addr=usb-001:003:10, irq=POLL)
> # dmesg | grep PHY
> [    5.700274] Asix Electronics AX88772A usb-001:004:10: attached PHY 
> driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL)
>> If you have a different PHY, can you please send me the PHY id:
>> cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id
>>
>> Your usb path will probably be different.
>
> # cat /sys/bus/mdio_bus/devices/usb-001\:004\:10/phy_id
> 0x003b1861
>
> > ...

Just for the record, I also have a board with external USB Ethernet 
dongle based on ASIX chip, which works fine with this patch, both when 
driver is built-in or as a module. Here is the log:

# dmesg | grep -i Asix
[    1.718349] usbcore: registered new interface driver asix
[    2.608596] usb 3-1: Manufacturer: ASIX Elec. Corp.
[    3.876279] libphy: Asix MDIO Bus: probed
[    3.958105] Asix Electronics AX88772C usb-003:002:10: attached PHY 
driver (mii_bus:phy_addr=usb-003:002:10, irq=POLL)
[    3.962728] asix 3-1:1.0 eth0: register 'asix' at 
usb-xhci-hcd.6.auto-1, ASIX AX88772B USB 2.0 Ethernet, 00:50:b6:18:92:f0
[   17.488532] asix 3-1:1.0 eth0: Link is Down
[   19.557233] asix 3-1:1.0 eth0: Link is Up - 100Mbps/Full - flow 
control off

# cat /sys/bus/mdio_bus/devices/usb-003\:002\:10/phy_id
0x003b1881


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Oleksij Rempel]

On Fri, Jun 18, 2021 at 12:57:13PM +0200, Marek Szyprowski wrote:
> On 18.06.2021 12:45, Marek Szyprowski wrote:
> > On 18.06.2021 12:13, Oleksij Rempel wrote:
> >> thank you for your feedback.
> >>
> >> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
> >>> On 07.06.2021 10:27, Oleksij Rempel wrote:
> >>>> To be able to use ax88772 with external PHYs and use advantage of
> >>>> existing PHY drivers, we need to port at least ax88772 part of asix
> >>>> driver to the phylib framework.
> >>>>
> >>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >>> I found one more issue with this patch. On one of my test boards
> >>> (Samsung Exynos5250 SoC based Arndale) system fails to establish 
> >>> network
> >>> connection just after starting the kernel when the driver is build-in.
> >>>
> >>> --->8---
> >>> # dmesg | grep asix
> >>> [    2.761928] usbcore: registered new interface driver asix
> >>> [    5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized):
> >>> invalid hw address, using random
> >>> [    6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at
> >>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 
> >>> 7a:9b:9a:f2:94:8e
> >>> [   14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
> >>> control off
> >>> # ping -c2  host
> >>> PING host (192.168.100.1) 56(84) bytes of data.
> >>>   From 192.168.100.20 icmp_seq=1 Destination Host Unreachable
> >>>   From 192.168.100.20 icmp_seq=2 Destination Host Unreachable
> >>>
> >>> --- host ping statistics ---
> >>> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 
> >>> 59ms
> >>> --->8---
> >> Hm... it looks like different chip variant. My is registered as
> >> "ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the
> >> difference. Can you please tell me more about this adapter and if 
> >> possible open
> >> tell the real part name.
> > Well, currently I have only remote access to that board. The network 
> > chip is soldered on board. Maybe you can read something from the photo 
> > on the wiki page: https://en.wikipedia.org/wiki/Arndale_Board
> >> I can imagine that this adapter may using generic PHY driver.
> >> Can you please confirm it by dmesg | grep PHY?
> >> In my case i'll get:
> >> Asix Electronics AX88772C usb-001:003:10: attached PHY driver 
> >> (mii_bus:phy_addr=usb-001:003:10, irq=POLL)
> > # dmesg | grep PHY
> > [    5.700274] Asix Electronics AX88772A usb-001:004:10: attached PHY 
> > driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL)
> >> If you have a different PHY, can you please send me the PHY id:
> >> cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id
> >>
> >> Your usb path will probably be different.
> >
> > # cat /sys/bus/mdio_bus/devices/usb-001\:004\:10/phy_id
> > 0x003b1861
> >
> > > ...
> 
> Just for the record, I also have a board with external USB Ethernet 
> dongle based on ASIX chip, which works fine with this patch, both when 
> driver is built-in or as a module. Here is the log:
> 
> # dmesg | grep -i Asix
> [    1.718349] usbcore: registered new interface driver asix
> [    2.608596] usb 3-1: Manufacturer: ASIX Elec. Corp.
> [    3.876279] libphy: Asix MDIO Bus: probed
> [    3.958105] Asix Electronics AX88772C usb-003:002:10: attached PHY 
> driver (mii_bus:phy_addr=usb-003:002:10, irq=POLL)
> [    3.962728] asix 3-1:1.0 eth0: register 'asix' at 
> usb-xhci-hcd.6.auto-1, ASIX AX88772B USB 2.0 Ethernet, 00:50:b6:18:92:f0
> [   17.488532] asix 3-1:1.0 eth0: Link is Down
> [   19.557233] asix 3-1:1.0 eth0: Link is Up - 100Mbps/Full - flow 
> control off
> 
> # cat /sys/bus/mdio_bus/devices/usb-003\:002\:10/phy_id
> 0x003b1881

Ok, this one is different. It is AX88772C variant.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Heiner Kallweit]

On 18.06.2021 12:13, Oleksij Rempel wrote:
> Hi Marek,
> 
> thank you for your feedback.
> 
> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
>> Hi Oleksij,
>>
>> On 07.06.2021 10:27, Oleksij Rempel wrote:
>>> To be able to use ax88772 with external PHYs and use advantage of
>>> existing PHY drivers, we need to port at least ax88772 part of asix
>>> driver to the phylib framework.
>>>
>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>
>> I found one more issue with this patch. On one of my test boards 
>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network 
>> connection just after starting the kernel when the driver is build-in.
>>
If you build in the MAC driver, do you also build in the PHY driver?
If the PHY driver is still a module this could explain why genphy
driver is used.
And your dmesg filtering suppresses the phy_attached_info() output
that would tell us the truth.

>> --->8---
>> # dmesg | grep asix
>> [    2.761928] usbcore: registered new interface driver asix
>> [    5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): 
>> invalid hw address, using random
>> [    6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at 
>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 7a:9b:9a:f2:94:8e
>> [   14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
>> control off
>> # ping -c2  host
>> PING host (192.168.100.1) 56(84) bytes of data.
>>  From 192.168.100.20 icmp_seq=1 Destination Host Unreachable
>>  From 192.168.100.20 icmp_seq=2 Destination Host Unreachable
>>
>> --- host ping statistics ---
>> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 59ms
>> --->8---
> 
> Hm... it looks like different chip variant. My is registered as
> "ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the
> difference. Can you please tell me more about this adapter and if possible open
> tell the real part name.
> 
> I can imagine that this adapter may using generic PHY driver.
> Can you please confirm it by dmesg | grep PHY?
> In my case i'll get:
> Asix Electronics AX88772C usb-001:003:10: attached PHY driver (mii_bus:phy_addr=usb-001:003:10, irq=POLL)
> 
> If you have a different PHY, can you please send me the PHY id:
> cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id
> 
> Your usb path will probably be different.
> 
>> Calling ifup eth0 && ifdown eth0 fixes the network status:
>>
>> --->8---
>> # ifdown eth0 && ifup eth0
>> [   60.474929] asix 1-3.2.4:1.0 eth0: Link is Down
>> [   60.623516] asix 1-3.2.4:1.0 eth0: Link is Down
>> [   62.774304] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>> [   62.786354] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
>> control off
>> # ping -c2 host
>> PING host (192.168.100.1) 56(84) bytes of data.
>> 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=1.25 ms
>> 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.853 ms
>>
>> --- host ping statistics ---
>> 2 packets transmitted, 2 received, 0% packet loss, time 3ms
>> rtt min/avg/max/mdev = 0.853/1.053/1.254/0.203 ms
>> --->8---
>>
>> When driver is loaded as a module (and without any other modules, so 
>> this is not a dependency issue), the connection is established properly 
>> just after the boot:
>>
>> --->8---
>> # dmesg | grep asix
>> [   13.633284] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): 
>> invalid hw address, using random
>> [   15.390350] asix 1-3.2.4:1.0 eth0: register 'asix' at 
>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 3a:51:11:08:aa:ea
>> [   15.414052] usbcore: registered new interface driver asix
>> [   15.832564] asix 1-3.2.4:1.0 eth0: Link is Down
>> [   18.053747] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
>> control off
>> # ping -c2 host
>> PING host (192.168.100.1) 56(84) bytes of data.
>> 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=0.545 ms
>> 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.742 ms
>>
>> --- host ping statistics ---
>> 2 packets transmitted, 2 received, 0% packet loss, time 3ms
>> rtt min/avg/max/mdev = 0.545/0.643/0.742/0.101 ms
>>
>> --->8---
>>
>> Let me know if I can make any other tests that would help fixing this issue.
>>
>>> ---
>>>   drivers/net/usb/asix.h         |   9 +++
>>>   drivers/net/usb/asix_common.c  |  37 ++++++++++
>>>   drivers/net/usb/asix_devices.c | 120 +++++++++++++++++++++------------
>>>   drivers/net/usb/ax88172a.c     |  14 ----
>>>   4 files changed, 122 insertions(+), 58 deletions(-)
>>>
>>> diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
>>> index edb94efd265e..2122d302e643 100644
>>> --- a/drivers/net/usb/asix.h
>>> +++ b/drivers/net/usb/asix.h
>>> @@ -25,6 +25,7 @@
>>>   #include <linux/usb/usbnet.h>
>>>   #include <linux/slab.h>
>>>   #include <linux/if_vlan.h>
>>> +#include <linux/phy.h>
>>>   
>>>   #define DRIVER_VERSION "22-Dec-2011"
>>>   #define DRIVER_NAME "asix"
>>> @@ -178,6 +179,10 @@ struct asix_common_private {
>>>   	u16 presvd_phy_advertise;
>>>   	u16 presvd_phy_bmcr;
>>>   	struct asix_rx_fixup_info rx_fixup_info;
>>> +	struct mii_bus *mdio;
>>> +	struct phy_device *phydev;
>>> +	u16 phy_addr;
>>> +	char phy_name[20];
>>>   };
>>>   
>>>   extern const struct driver_info ax88172a_info;
>>> @@ -214,6 +219,7 @@ int asix_write_rx_ctl(struct usbnet *dev, u16 mode, int in_pm);
>>>   
>>>   u16 asix_read_medium_status(struct usbnet *dev, int in_pm);
>>>   int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm);
>>> +void asix_adjust_link(struct net_device *netdev);
>>>   
>>>   int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm);
>>>   
>>> @@ -222,6 +228,9 @@ void asix_set_multicast(struct net_device *net);
>>>   int asix_mdio_read(struct net_device *netdev, int phy_id, int loc);
>>>   void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val);
>>>   
>>> +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum);
>>> +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val);
>>> +
>>>   int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc);
>>>   void asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc,
>>>   			  int val);
>>> diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
>>> index e1109f1a8dd5..085bc8281082 100644
>>> --- a/drivers/net/usb/asix_common.c
>>> +++ b/drivers/net/usb/asix_common.c
>>> @@ -384,6 +384,27 @@ int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm)
>>>   	return ret;
>>>   }
>>>   
>>> +/* set MAC link settings according to information from phylib */
>>> +void asix_adjust_link(struct net_device *netdev)
>>> +{
>>> +	struct phy_device *phydev = netdev->phydev;
>>> +	struct usbnet *dev = netdev_priv(netdev);
>>> +	u16 mode = 0;
>>> +
>>> +	if (phydev->link) {
>>> +		mode = AX88772_MEDIUM_DEFAULT;
>>> +
>>> +		if (phydev->duplex == DUPLEX_HALF)
>>> +			mode &= ~AX_MEDIUM_FD;
>>> +
>>> +		if (phydev->speed != SPEED_100)
>>> +			mode &= ~AX_MEDIUM_PS;
>>> +	}
>>> +
>>> +	asix_write_medium_mode(dev, mode, 0);
>>> +	phy_print_status(phydev);
>>> +}
>>> +
>>>   int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
>>>   {
>>>   	int ret;
>>> @@ -506,6 +527,22 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
>>>   	mutex_unlock(&dev->phy_mutex);
>>>   }
>>>   
>>> +/* MDIO read and write wrappers for phylib */
>>> +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
>>> +{
>>> +	struct usbnet *priv = bus->priv;
>>> +
>>> +	return asix_mdio_read(priv->net, phy_id, regnum);
>>> +}
>>> +
>>> +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
>>> +{
>>> +	struct usbnet *priv = bus->priv;
>>> +
>>> +	asix_mdio_write(priv->net, phy_id, regnum, val);
>>> +	return 0;
>>> +}
>>> +
>>>   int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
>>>   {
>>>   	struct usbnet *dev = netdev_priv(netdev);
>>> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
>>> index 00b6ac0570eb..e4cd85e38edd 100644
>>> --- a/drivers/net/usb/asix_devices.c
>>> +++ b/drivers/net/usb/asix_devices.c
>>> @@ -285,7 +285,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
>>>   
>>>   static const struct ethtool_ops ax88772_ethtool_ops = {
>>>   	.get_drvinfo		= asix_get_drvinfo,
>>> -	.get_link		= asix_get_link,
>>> +	.get_link		= usbnet_get_link,
>>>   	.get_msglevel		= usbnet_get_msglevel,
>>>   	.set_msglevel		= usbnet_set_msglevel,
>>>   	.get_wol		= asix_get_wol,
>>> @@ -293,37 +293,15 @@ static const struct ethtool_ops ax88772_ethtool_ops = {
>>>   	.get_eeprom_len		= asix_get_eeprom_len,
>>>   	.get_eeprom		= asix_get_eeprom,
>>>   	.set_eeprom		= asix_set_eeprom,
>>> -	.nway_reset		= usbnet_nway_reset,
>>> -	.get_link_ksettings	= usbnet_get_link_ksettings_mii,
>>> -	.set_link_ksettings	= usbnet_set_link_ksettings_mii,
>>> +	.nway_reset		= phy_ethtool_nway_reset,
>>> +	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
>>> +	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
>>>   };
>>>   
>>> -static int ax88772_link_reset(struct usbnet *dev)
>>> -{
>>> -	u16 mode;
>>> -	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
>>> -
>>> -	mii_check_media(&dev->mii, 1, 1);
>>> -	mii_ethtool_gset(&dev->mii, &ecmd);
>>> -	mode = AX88772_MEDIUM_DEFAULT;
>>> -
>>> -	if (ethtool_cmd_speed(&ecmd) != SPEED_100)
>>> -		mode &= ~AX_MEDIUM_PS;
>>> -
>>> -	if (ecmd.duplex != DUPLEX_FULL)
>>> -		mode &= ~AX_MEDIUM_FD;
>>> -
>>> -	netdev_dbg(dev->net, "ax88772_link_reset() speed: %u duplex: %d setting mode to 0x%04x\n",
>>> -		   ethtool_cmd_speed(&ecmd), ecmd.duplex, mode);
>>> -
>>> -	asix_write_medium_mode(dev, mode, 0);
>>> -
>>> -	return 0;
>>> -}
>>> -
>>>   static int ax88772_reset(struct usbnet *dev)
>>>   {
>>>   	struct asix_data *data = (struct asix_data *)&dev->data;
>>> +	struct asix_common_private *priv = dev->driver_priv;
>>>   	int ret;
>>>   
>>>   	/* Rewrite MAC address */
>>> @@ -342,6 +320,8 @@ static int ax88772_reset(struct usbnet *dev)
>>>   	if (ret < 0)
>>>   		goto out;
>>>   
>>> +	phy_start(priv->phydev);
>>> +
>>>   	return 0;
>>>   
>>>   out:
>>> @@ -586,7 +566,7 @@ static const struct net_device_ops ax88772_netdev_ops = {
>>>   	.ndo_get_stats64	= dev_get_tstats64,
>>>   	.ndo_set_mac_address 	= asix_set_mac_address,
>>>   	.ndo_validate_addr	= eth_validate_addr,
>>> -	.ndo_do_ioctl		= asix_ioctl,
>>> +	.ndo_do_ioctl		= phy_do_ioctl_running,
>>>   	.ndo_set_rx_mode        = asix_set_multicast,
>>>   };
>>>   
>>> @@ -677,12 +657,57 @@ static int asix_resume(struct usb_interface *intf)
>>>   	return usbnet_resume(intf);
>>>   }
>>>   
>>> +static int ax88772_init_mdio(struct usbnet *dev)
>>> +{
>>> +	struct asix_common_private *priv = dev->driver_priv;
>>> +
>>> +	priv->mdio = devm_mdiobus_alloc(&dev->udev->dev);
>>> +	if (!priv->mdio)
>>> +		return -ENOMEM;
>>> +
>>> +	priv->mdio->priv = dev;
>>> +	priv->mdio->read = &asix_mdio_bus_read;
>>> +	priv->mdio->write = &asix_mdio_bus_write;
>>> +	priv->mdio->name = "Asix MDIO Bus";
>>> +	/* mii bus name is usb-<usb bus number>-<usb device number> */
>>> +	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
>>> +		 dev->udev->bus->busnum, dev->udev->devnum);
>>> +
>>> +	return devm_mdiobus_register(&dev->udev->dev, priv->mdio);
>>> +}
>>> +
>>> +static int ax88772_init_phy(struct usbnet *dev)
>>> +{
>>> +	struct asix_common_private *priv = dev->driver_priv;
>>> +	int ret;
>>> +
>>> +	priv->phy_addr = asix_read_phy_addr(dev, true);
>>> +	if (priv->phy_addr < 0)
>>> +		return priv->phy_addr;
>>> +
>>> +	snprintf(priv->phy_name, sizeof(priv->phy_name), PHY_ID_FMT,
>>> +		 priv->mdio->id, priv->phy_addr);
>>> +
>>> +	priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link,
>>> +				   PHY_INTERFACE_MODE_INTERNAL);
>>> +	if (IS_ERR(priv->phydev)) {
>>> +		netdev_err(dev->net, "Could not connect to PHY device %s\n",
>>> +			   priv->phy_name);
>>> +		ret = PTR_ERR(priv->phydev);
>>> +		return ret;
>>> +	}
>>> +
>>> +	phy_attached_info(priv->phydev);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>   static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>>>   {
>>> -	int ret, i;
>>>   	u8 buf[ETH_ALEN] = {0}, chipcode = 0;
>>> -	u32 phyid;
>>>   	struct asix_common_private *priv;
>>> +	int ret, i;
>>> +	u32 phyid;
>>>   
>>>   	usbnet_get_endpoints(dev, intf);
>>>   
>>> @@ -714,17 +739,6 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>>>   
>>>   	asix_set_netdev_dev_addr(dev, buf);
>>>   
>>> -	/* Initialize MII structure */
>>> -	dev->mii.dev = dev->net;
>>> -	dev->mii.mdio_read = asix_mdio_read;
>>> -	dev->mii.mdio_write = asix_mdio_write;
>>> -	dev->mii.phy_id_mask = 0x1f;
>>> -	dev->mii.reg_num_mask = 0x1f;
>>> -
>>> -	dev->mii.phy_id = asix_read_phy_addr(dev, true);
>>> -	if (dev->mii.phy_id < 0)
>>> -		return dev->mii.phy_id;
>>> -
>>>   	dev->net->netdev_ops = &ax88772_netdev_ops;
>>>   	dev->net->ethtool_ops = &ax88772_ethtool_ops;
>>>   	dev->net->needed_headroom = 4; /* cf asix_tx_fixup() */
>>> @@ -768,11 +782,31 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>>>   		priv->suspend = ax88772_suspend;
>>>   	}
>>>   
>>> +	ret = ax88772_init_mdio(dev);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	return ax88772_init_phy(dev);
>>> +}
>>> +
>>> +static int ax88772_stop(struct usbnet *dev)
>>> +{
>>> +	struct asix_common_private *priv = dev->driver_priv;
>>> +
>>> +	/* On unplugged USB, we will get MDIO communication errors and the
>>> +	 * PHY will be set in to PHY_HALTED state.
>>> +	 */
>>> +	if (priv->phydev->state != PHY_HALTED)
>>> +		phy_stop(priv->phydev);
>>> +
>>>   	return 0;
>>>   }
>>>   
>>>   static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
>>>   {
>>> +	struct asix_common_private *priv = dev->driver_priv;
>>> +
>>> +	phy_disconnect(priv->phydev);
>>>   	asix_rx_fixup_common_free(dev->driver_priv);
>>>   }
>>>   
>>> @@ -1161,8 +1195,8 @@ static const struct driver_info ax88772_info = {
>>>   	.bind = ax88772_bind,
>>>   	.unbind = ax88772_unbind,
>>>   	.status = asix_status,
>>> -	.link_reset = ax88772_link_reset,
>>>   	.reset = ax88772_reset,
>>> +	.stop = ax88772_stop,
>>>   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
>>>   	.rx_fixup = asix_rx_fixup_common,
>>>   	.tx_fixup = asix_tx_fixup,
>>> @@ -1173,7 +1207,6 @@ static const struct driver_info ax88772b_info = {
>>>   	.bind = ax88772_bind,
>>>   	.unbind = ax88772_unbind,
>>>   	.status = asix_status,
>>> -	.link_reset = ax88772_link_reset,
>>>   	.reset = ax88772_reset,
>>>   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
>>>   	         FLAG_MULTI_PACKET,
>>> @@ -1209,7 +1242,6 @@ static const struct driver_info hg20f9_info = {
>>>   	.bind = ax88772_bind,
>>>   	.unbind = ax88772_unbind,
>>>   	.status = asix_status,
>>> -	.link_reset = ax88772_link_reset,
>>>   	.reset = ax88772_reset,
>>>   	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
>>>   	         FLAG_MULTI_PACKET,
>>> diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
>>> index c8ca5187eece..2e2081346740 100644
>>> --- a/drivers/net/usb/ax88172a.c
>>> +++ b/drivers/net/usb/ax88172a.c
>>> @@ -25,20 +25,6 @@ struct ax88172a_private {
>>>   	struct asix_rx_fixup_info rx_fixup_info;
>>>   };
>>>   
>>> -/* MDIO read and write wrappers for phylib */
>>> -static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
>>> -{
>>> -	return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id,
>>> -			      regnum);
>>> -}
>>> -
>>> -static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum,
>>> -			       u16 val)
>>> -{
>>> -	asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val);
>>> -	return 0;
>>> -}
>>> -
>>>   /* set MAC link settings according to information from phylib */
>>>   static void ax88172a_adjust_link(struct net_device *netdev)
>>>   {
>>
>> Best regards
>> -- 
>> Marek Szyprowski, PhD
>> Samsung R&D Institute Poland
>>
>>
> 

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Marek Szyprowski]

Hi Heiner,

On 18.06.2021 13:04, Heiner Kallweit wrote:
> On 18.06.2021 12:13, Oleksij Rempel wrote:
>> thank you for your feedback.
>>
>> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
>>> On 07.06.2021 10:27, Oleksij Rempel wrote:
>>>> To be able to use ax88772 with external PHYs and use advantage of
>>>> existing PHY drivers, we need to port at least ax88772 part of asix
>>>> driver to the phylib framework.
>>>>
>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>> I found one more issue with this patch. On one of my test boards
>>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network
>>> connection just after starting the kernel when the driver is build-in.
>>>
> If you build in the MAC driver, do you also build in the PHY driver?
> If the PHY driver is still a module this could explain why genphy
> driver is used.
> And your dmesg filtering suppresses the phy_attached_info() output
> that would tell us the truth.

Here is a bit more complete log:

# dmesg | grep -i Asix
[    2.412966] usbcore: registered new interface driver asix
[    4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp.
[    4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): 
invalid hw address, using random
[    5.657009] libphy: Asix MDIO Bus: probed
[    5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY 
driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL)
[    5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at 
usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e
[    9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
control off

This seems to be something different than missing PHY driver.

>>> --->8---
>>> # dmesg | grep asix
>>> [    2.761928] usbcore: registered new interface driver asix
>>> [    5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized):
>>> invalid hw address, using random
>>> [    6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at
>>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 7a:9b:9a:f2:94:8e
>>> [   14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
>>> control off
>>> # ping -c2  host
>>> PING host (192.168.100.1) 56(84) bytes of data.
>>>   From 192.168.100.20 icmp_seq=1 Destination Host Unreachable
>>>   From 192.168.100.20 icmp_seq=2 Destination Host Unreachable
>>>
>>> --- host ping statistics ---
>>> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 59ms
>>> --->8---
>> Hm... it looks like different chip variant. My is registered as
>> "ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the
>> difference. Can you please tell me more about this adapter and if possible open
>> tell the real part name.
>>
>> I can imagine that this adapter may using generic PHY driver.
>> Can you please confirm it by dmesg | grep PHY?
>> In my case i'll get:
>> Asix Electronics AX88772C usb-001:003:10: attached PHY driver (mii_bus:phy_addr=usb-001:003:10, irq=POLL)
>>
>> If you have a different PHY, can you please send me the PHY id:
>> cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id
>>
>> Your usb path will probably be different.
>>
>>> Calling ifup eth0 && ifdown eth0 fixes the network status:
>>>
>>> --->8---
>>> # ifdown eth0 && ifup eth0
>>> [   60.474929] asix 1-3.2.4:1.0 eth0: Link is Down
>>> [   60.623516] asix 1-3.2.4:1.0 eth0: Link is Down
>>> [   62.774304] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>>> [   62.786354] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
>>> control off
>>> # ping -c2 host
>>> PING host (192.168.100.1) 56(84) bytes of data.
>>> 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=1.25 ms
>>> 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.853 ms
>>>
>>> --- host ping statistics ---
>>> 2 packets transmitted, 2 received, 0% packet loss, time 3ms
>>> rtt min/avg/max/mdev = 0.853/1.053/1.254/0.203 ms
>>> --->8---
>>>
>>> When driver is loaded as a module (and without any other modules, so
>>> this is not a dependency issue), the connection is established properly
>>> just after the boot:
>>>
>>> --->8---
>>> # dmesg | grep asix
>>> [   13.633284] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized):
>>> invalid hw address, using random
>>> [   15.390350] asix 1-3.2.4:1.0 eth0: register 'asix' at
>>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 3a:51:11:08:aa:ea
>>> [   15.414052] usbcore: registered new interface driver asix
>>> [   15.832564] asix 1-3.2.4:1.0 eth0: Link is Down
>>> [   18.053747] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
>>> control off
>>> # ping -c2 host
>>> PING host (192.168.100.1) 56(84) bytes of data.
>>> 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=0.545 ms
>>> 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.742 ms
>>>
>>> --- host ping statistics ---
>>> 2 packets transmitted, 2 received, 0% packet loss, time 3ms
>>> rtt min/avg/max/mdev = 0.545/0.643/0.742/0.101 ms
>>>
>>> --->8---
>>>
>>> Let me know if I can make any other tests that would help fixing this issue.
>>> [...]

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Oleksij Rempel]

On Fri, Jun 18, 2021 at 01:11:41PM +0200, Marek Szyprowski wrote:
> Hi Heiner,
> 
> On 18.06.2021 13:04, Heiner Kallweit wrote:
> > On 18.06.2021 12:13, Oleksij Rempel wrote:
> >> thank you for your feedback.
> >>
> >> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
> >>> On 07.06.2021 10:27, Oleksij Rempel wrote:
> >>>> To be able to use ax88772 with external PHYs and use advantage of
> >>>> existing PHY drivers, we need to port at least ax88772 part of asix
> >>>> driver to the phylib framework.
> >>>>
> >>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >>> I found one more issue with this patch. On one of my test boards
> >>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network
> >>> connection just after starting the kernel when the driver is build-in.
> >>>
> > If you build in the MAC driver, do you also build in the PHY driver?
> > If the PHY driver is still a module this could explain why genphy
> > driver is used.
> > And your dmesg filtering suppresses the phy_attached_info() output
> > that would tell us the truth.
> 
> Here is a bit more complete log:
> 
> # dmesg | grep -i Asix
> [    2.412966] usbcore: registered new interface driver asix
> [    4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp.
> [    4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): 
> invalid hw address, using random
> [    5.657009] libphy: Asix MDIO Bus: probed
> [    5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY 
> driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL)
> [    5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at 
> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e
> [    9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow 
> control off
> 
> This seems to be something different than missing PHY driver.

Can you please test it:

diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index aec97b021a73..7897108a1a42 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -453,6 +453,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
 	u16 rx_ctl, phy14h, phy15h, phy16h;
 	u8 chipcode = 0;
 
+	netdev_info(dev->net, "ax88772a_hw_reset\n");
 	ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm);
 	if (ret < 0)
 		goto out;
@@ -509,31 +510,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
 			goto out;
 		}
 	} else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) {
-		/* Check if the PHY registers have default settings */
-		phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
-					     AX88772A_PHY14H);
-		phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
-					     AX88772A_PHY15H);
-		phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
-					     AX88772A_PHY16H);
-
-		netdev_dbg(dev->net,
-			   "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n",
-			   phy14h, phy15h, phy16h);
-
-		/* Restore PHY registers default setting if not */
-		if (phy14h != AX88772A_PHY14H_DEFAULT)
-			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
-					     AX88772A_PHY14H,
-					     AX88772A_PHY14H_DEFAULT);
-		if (phy15h != AX88772A_PHY15H_DEFAULT)
-			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
-					     AX88772A_PHY15H,
-					     AX88772A_PHY15H_DEFAULT);
-		if (phy16h != AX88772A_PHY16H_DEFAULT)
-			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
-					     AX88772A_PHY16H,
-					     AX88772A_PHY16H_DEFAULT);
+		netdev_info(dev->net, "do not touch PHY regs\n");
 	}
 
 	ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0,
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Marek Szyprowski]

Hi Oleksij,

On 18.06.2021 15:20, Oleksij Rempel wrote:
> On Fri, Jun 18, 2021 at 01:11:41PM +0200, Marek Szyprowski wrote:
>> On 18.06.2021 13:04, Heiner Kallweit wrote:
>>> On 18.06.2021 12:13, Oleksij Rempel wrote:
>>>> thank you for your feedback.
>>>>
>>>> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
>>>>> On 07.06.2021 10:27, Oleksij Rempel wrote:
>>>>>> To be able to use ax88772 with external PHYs and use advantage of
>>>>>> existing PHY drivers, we need to port at least ax88772 part of asix
>>>>>> driver to the phylib framework.
>>>>>>
>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>>> I found one more issue with this patch. On one of my test boards
>>>>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network
>>>>> connection just after starting the kernel when the driver is build-in.
>>>>>
>>> If you build in the MAC driver, do you also build in the PHY driver?
>>> If the PHY driver is still a module this could explain why genphy
>>> driver is used.
>>> And your dmesg filtering suppresses the phy_attached_info() output
>>> that would tell us the truth.
>> Here is a bit more complete log:
>>
>> # dmesg | grep -i Asix
>> [    2.412966] usbcore: registered new interface driver asix
>> [    4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp.
>> [    4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized):
>> invalid hw address, using random
>> [    5.657009] libphy: Asix MDIO Bus: probed
>> [    5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY
>> driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL)
>> [    5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at
>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e
>> [    9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
>> control off
>>
>> This seems to be something different than missing PHY driver.
> Can you please test it:
>
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index aec97b021a73..7897108a1a42 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -453,6 +453,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
>   	u16 rx_ctl, phy14h, phy15h, phy16h;
>   	u8 chipcode = 0;
>   
> +	netdev_info(dev->net, "ax88772a_hw_reset\n");
>   	ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm);
>   	if (ret < 0)
>   		goto out;
> @@ -509,31 +510,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
>   			goto out;
>   		}
>   	} else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) {
> -		/* Check if the PHY registers have default settings */
> -		phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> -					     AX88772A_PHY14H);
> -		phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> -					     AX88772A_PHY15H);
> -		phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> -					     AX88772A_PHY16H);
> -
> -		netdev_dbg(dev->net,
> -			   "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n",
> -			   phy14h, phy15h, phy16h);
> -
> -		/* Restore PHY registers default setting if not */
> -		if (phy14h != AX88772A_PHY14H_DEFAULT)
> -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> -					     AX88772A_PHY14H,
> -					     AX88772A_PHY14H_DEFAULT);
> -		if (phy15h != AX88772A_PHY15H_DEFAULT)
> -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> -					     AX88772A_PHY15H,
> -					     AX88772A_PHY15H_DEFAULT);
> -		if (phy16h != AX88772A_PHY16H_DEFAULT)
> -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> -					     AX88772A_PHY16H,
> -					     AX88772A_PHY16H_DEFAULT);
> +		netdev_info(dev->net, "do not touch PHY regs\n");
>   	}
>   
>   	ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0,

This doesn't help for this issue.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Oleksij Rempel]

Hi Marek,

On Mon, Jun 21, 2021 at 08:05:49AM +0200, Marek Szyprowski wrote:
> Hi Oleksij,
> 
> On 18.06.2021 15:20, Oleksij Rempel wrote:
> > On Fri, Jun 18, 2021 at 01:11:41PM +0200, Marek Szyprowski wrote:
> >> On 18.06.2021 13:04, Heiner Kallweit wrote:
> >>> On 18.06.2021 12:13, Oleksij Rempel wrote:
> >>>> thank you for your feedback.
> >>>>
> >>>> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
> >>>>> On 07.06.2021 10:27, Oleksij Rempel wrote:
> >>>>>> To be able to use ax88772 with external PHYs and use advantage of
> >>>>>> existing PHY drivers, we need to port at least ax88772 part of asix
> >>>>>> driver to the phylib framework.
> >>>>>>
> >>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >>>>> I found one more issue with this patch. On one of my test boards
> >>>>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network
> >>>>> connection just after starting the kernel when the driver is build-in.
> >>>>>
> >>> If you build in the MAC driver, do you also build in the PHY driver?
> >>> If the PHY driver is still a module this could explain why genphy
> >>> driver is used.
> >>> And your dmesg filtering suppresses the phy_attached_info() output
> >>> that would tell us the truth.
> >> Here is a bit more complete log:
> >>
> >> # dmesg | grep -i Asix
> >> [    2.412966] usbcore: registered new interface driver asix
> >> [    4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp.
> >> [    4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized):
> >> invalid hw address, using random
> >> [    5.657009] libphy: Asix MDIO Bus: probed
> >> [    5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY
> >> driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL)
> >> [    5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at
> >> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e
> >> [    9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
> >> control off
> >>
> >> This seems to be something different than missing PHY driver.
> > Can you please test it:
> >
> > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> > index aec97b021a73..7897108a1a42 100644
> > --- a/drivers/net/usb/asix_devices.c
> > +++ b/drivers/net/usb/asix_devices.c
> > @@ -453,6 +453,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
> >   	u16 rx_ctl, phy14h, phy15h, phy16h;
> >   	u8 chipcode = 0;
> >   
> > +	netdev_info(dev->net, "ax88772a_hw_reset\n");
> >   	ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm);
> >   	if (ret < 0)
> >   		goto out;
> > @@ -509,31 +510,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
> >   			goto out;
> >   		}
> >   	} else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) {
> > -		/* Check if the PHY registers have default settings */
> > -		phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY14H);
> > -		phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY15H);
> > -		phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY16H);
> > -
> > -		netdev_dbg(dev->net,
> > -			   "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n",
> > -			   phy14h, phy15h, phy16h);
> > -
> > -		/* Restore PHY registers default setting if not */
> > -		if (phy14h != AX88772A_PHY14H_DEFAULT)
> > -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY14H,
> > -					     AX88772A_PHY14H_DEFAULT);
> > -		if (phy15h != AX88772A_PHY15H_DEFAULT)
> > -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY15H,
> > -					     AX88772A_PHY15H_DEFAULT);
> > -		if (phy16h != AX88772A_PHY16H_DEFAULT)
> > -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY16H,
> > -					     AX88772A_PHY16H_DEFAULT);
> > +		netdev_info(dev->net, "do not touch PHY regs\n");
> >   	}
> >   
> >   	ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0,
> 
> This doesn't help for this issue.

Ok.
So far I was not able to see obvious differences between:
probe -> ip link set dev eth1 up

and

probe -> ip link set dev eth1 up;
	 ip link set dev eth1 down;
	 ip link set dev eth1 up


Except of PHY sate. By default the PHY is in resumed state after probe
and is able to negotiate the link even if the MAC is down.
After ip link set dev eth1 down, the PHY is in suspend state, as
expected.

Can you please test this change?

diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index aec97b021a73..2c115216420a 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -701,6 +701,7 @@ static int ax88772_init_phy(struct usbnet *dev)
 		return ret;
 	}
 
+	phy_suspend(priv->phydev);
 	priv->phydev->mac_managed_pm = 1;
 
 	phy_attached_info(priv->phydev);

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support

[Marek Szyprowski]

Hi Oleksij,

On 23.06.2021 09:06, Oleksij Rempel wrote:
> On Mon, Jun 21, 2021 at 08:05:49AM +0200, Marek Szyprowski wrote:
>> On 18.06.2021 15:20, Oleksij Rempel wrote:
>>> On Fri, Jun 18, 2021 at 01:11:41PM +0200, Marek Szyprowski wrote:
>>>> On 18.06.2021 13:04, Heiner Kallweit wrote:
>>>>> On 18.06.2021 12:13, Oleksij Rempel wrote:
>>>>>> thank you for your feedback.
>>>>>>
>>>>>> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
>>>>>>> On 07.06.2021 10:27, Oleksij Rempel wrote:
>>>>>>>> To be able to use ax88772 with external PHYs and use advantage of
>>>>>>>> existing PHY drivers, we need to port at least ax88772 part of asix
>>>>>>>> driver to the phylib framework.
>>>>>>>>
>>>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>>>>> I found one more issue with this patch. On one of my test boards
>>>>>>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network
>>>>>>> connection just after starting the kernel when the driver is build-in.
>>>>>>>
>>>>> If you build in the MAC driver, do you also build in the PHY driver?
>>>>> If the PHY driver is still a module this could explain why genphy
>>>>> driver is used.
>>>>> And your dmesg filtering suppresses the phy_attached_info() output
>>>>> that would tell us the truth.
>>>> Here is a bit more complete log:
>>>>
>>>> # dmesg | grep -i Asix
>>>> [    2.412966] usbcore: registered new interface driver asix
>>>> [    4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp.
>>>> [    4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized):
>>>> invalid hw address, using random
>>>> [    5.657009] libphy: Asix MDIO Bus: probed
>>>> [    5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY
>>>> driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL)
>>>> [    5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at
>>>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e
>>>> [    9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
>>>> control off
>>>>
>>>> This seems to be something different than missing PHY driver.
>>> Can you please test it:
>>>
>>> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
>>> index aec97b021a73..7897108a1a42 100644
>>> --- a/drivers/net/usb/asix_devices.c
>>> +++ b/drivers/net/usb/asix_devices.c
>>> @@ -453,6 +453,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
>>>    	u16 rx_ctl, phy14h, phy15h, phy16h;
>>>    	u8 chipcode = 0;
>>>    
>>> +	netdev_info(dev->net, "ax88772a_hw_reset\n");
>>>    	ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm);
>>>    	if (ret < 0)
>>>    		goto out;
>>> @@ -509,31 +510,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
>>>    			goto out;
>>>    		}
>>>    	} else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) {
>>> -		/* Check if the PHY registers have default settings */
>>> -		phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
>>> -					     AX88772A_PHY14H);
>>> -		phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
>>> -					     AX88772A_PHY15H);
>>> -		phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
>>> -					     AX88772A_PHY16H);
>>> -
>>> -		netdev_dbg(dev->net,
>>> -			   "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n",
>>> -			   phy14h, phy15h, phy16h);
>>> -
>>> -		/* Restore PHY registers default setting if not */
>>> -		if (phy14h != AX88772A_PHY14H_DEFAULT)
>>> -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
>>> -					     AX88772A_PHY14H,
>>> -					     AX88772A_PHY14H_DEFAULT);
>>> -		if (phy15h != AX88772A_PHY15H_DEFAULT)
>>> -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
>>> -					     AX88772A_PHY15H,
>>> -					     AX88772A_PHY15H_DEFAULT);
>>> -		if (phy16h != AX88772A_PHY16H_DEFAULT)
>>> -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
>>> -					     AX88772A_PHY16H,
>>> -					     AX88772A_PHY16H_DEFAULT);
>>> +		netdev_info(dev->net, "do not touch PHY regs\n");
>>>    	}
>>>    
>>>    	ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0,
>> This doesn't help for this issue.
> Ok.
> So far I was not able to see obvious differences between:
> probe -> ip link set dev eth1 up
>
> and
>
> probe -> ip link set dev eth1 up;
> 	 ip link set dev eth1 down;
> 	 ip link set dev eth1 up
>
>
> Except of PHY sate. By default the PHY is in resumed state after probe
> and is able to negotiate the link even if the MAC is down.
> After ip link set dev eth1 down, the PHY is in suspend state, as
> expected.
>
> Can you please test this change?
>
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index aec97b021a73..2c115216420a 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -701,6 +701,7 @@ static int ax88772_init_phy(struct usbnet *dev)
>   		return ret;
>   	}
>   
> +	phy_suspend(priv->phydev);
>   	priv->phydev->mac_managed_pm = 1;
>   
>   	phy_attached_info(priv->phydev);
>
I'm sorry for the late reply, I've just got back from vacations. The 
above change fixes the issue.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH net-next v2 5/8] net: usb: asix: ax88772: add generic selftest support

[Oleksij Rempel]

With working phylib support we are able now to use generic selftests.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/Kconfig        |  1 +
 drivers/net/usb/asix.h         |  1 +
 drivers/net/usb/asix_devices.c | 23 +++++++++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 6f7be47974f6..4c5d69732a7e 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -165,6 +165,7 @@ config USB_NET_AX8817X
 	select CRC32
 	select PHYLIB
 	select AX88796B_PHY
+	imply NET_SELFTESTS
 	default y
 	help
 	  This option adds support for ASIX AX88xxx based USB 2.0
diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index 2122d302e643..e1994a246122 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -26,6 +26,7 @@
 #include <linux/slab.h>
 #include <linux/if_vlan.h>
 #include <linux/phy.h>
+#include <net/selftests.h>
 
 #define DRIVER_VERSION "22-Dec-2011"
 #define DRIVER_NAME "asix"
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index e4cd85e38edd..57dafb3262d9 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -283,6 +283,26 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
 	return ret;
 }
 
+static void ax88772_ethtool_get_strings(struct net_device *netdev, u32 sset,
+					u8 *data)
+{
+	switch (sset) {
+	case ETH_SS_TEST:
+		net_selftest_get_strings(data);
+		break;
+	}
+}
+
+static int ax88772_ethtool_get_sset_count(struct net_device *ndev, int sset)
+{
+	switch (sset) {
+	case ETH_SS_TEST:
+		return net_selftest_get_count();
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static const struct ethtool_ops ax88772_ethtool_ops = {
 	.get_drvinfo		= asix_get_drvinfo,
 	.get_link		= usbnet_get_link,
@@ -296,6 +316,9 @@ static const struct ethtool_ops ax88772_ethtool_ops = {
 	.nway_reset		= phy_ethtool_nway_reset,
 	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
 	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
+	.self_test		= net_selftest,
+	.get_strings		= ax88772_ethtool_get_strings,
+	.get_sset_count		= ax88772_ethtool_get_sset_count,
 };
 
 static int ax88772_reset(struct usbnet *dev)
-- 
2.29.2

[PATCH net-next v2 6/8] net: usb: asix: add error handling for asix_mdio_* functions

[Oleksij Rempel]

This usb devices can be removed at any time, so we need to forward
correct error value if device was detached.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/asix_common.c | 42 +++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 085bc8281082..ac92bc52a85e 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -485,18 +485,23 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)
 		return ret;
 	}
 
-	asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
-				(__u16)loc, 2, &res, 0);
-	asix_set_hw_mii(dev, 0);
+	ret = asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id, (__u16)loc, 2,
+			    &res, 0);
+	if (ret < 0)
+		goto out;
+
+	ret = asix_set_hw_mii(dev, 0);
+out:
 	mutex_unlock(&dev->phy_mutex);
 
 	netdev_dbg(dev->net, "asix_mdio_read() phy_id=0x%02x, loc=0x%02x, returns=0x%04x\n",
 			phy_id, loc, le16_to_cpu(res));
 
-	return le16_to_cpu(res);
+	return ret < 0 ? ret : le16_to_cpu(res);
 }
 
-void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
+static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc,
+			     int val)
 {
 	struct usbnet *dev = netdev_priv(netdev);
 	__le16 res = cpu_to_le16(val);
@@ -516,15 +521,25 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
 		ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
 				    0, 0, 1, &smsr, 0);
 	} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
-	if (ret == -ENODEV) {
-		mutex_unlock(&dev->phy_mutex);
-		return;
-	}
 
-	asix_write_cmd(dev, AX_CMD_WRITE_MII_REG, phy_id,
-		       (__u16)loc, 2, &res, 0);
-	asix_set_hw_mii(dev, 0);
+	if (ret == -ENODEV)
+		goto out;
+
+	ret = asix_write_cmd(dev, AX_CMD_WRITE_MII_REG, phy_id, (__u16)loc, 2,
+			     &res, 0);
+	if (ret < 0)
+		goto out;
+
+	ret = asix_set_hw_mii(dev, 0);
+out:
 	mutex_unlock(&dev->phy_mutex);
+
+	return ret < 0 ? ret : 0;
+}
+
+void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
+{
+	__asix_mdio_write(netdev, phy_id, loc, val);
 }
 
 /* MDIO read and write wrappers for phylib */
@@ -539,8 +554,7 @@ int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
 {
 	struct usbnet *priv = bus->priv;
 
-	asix_mdio_write(priv->net, phy_id, regnum, val);
-	return 0;
+	return __asix_mdio_write(priv->net, phy_id, regnum, val);
 }
 
 int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
-- 
2.29.2

[PATCH net-next v2 7/8] net: phy: do not print dump stack if device was removed

[Oleksij Rempel]

In case phy_state_machine() works on top of USB device, we can get -ENODEV
at any point. So, be less noisy if device was removed.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1f0512e39c65..1089a93d12f6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1136,6 +1136,9 @@ void phy_state_machine(struct work_struct *work)
 	else if (do_suspend)
 		phy_suspend(phydev);
 
+	if (err == -ENODEV)
+		return;
+
 	if (err < 0)
 		phy_error(phydev);
 
-- 
2.29.2

[PATCH net-next v2 8/8] usbnet: run unbind() before unregister_netdev()

[Oleksij Rempel]

unbind() is the proper place to disconnect PHY, but it will fail if
netdev is already unregistered.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/usb/usbnet.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index ecf62849f4c1..57a5a025255c 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1597,6 +1597,9 @@ void usbnet_disconnect (struct usb_interface *intf)
 		   xdev->bus->bus_name, xdev->devpath,
 		   dev->driver_info->description);
 
+	if (dev->driver_info->unbind)
+		dev->driver_info->unbind(dev, intf);
+
 	net = dev->net;
 	unregister_netdev (net);
 
@@ -1604,9 +1607,6 @@ void usbnet_disconnect (struct usb_interface *intf)
 
 	usb_scuttle_anchored_urbs(&dev->deferred);
 
-	if (dev->driver_info->unbind)
-		dev->driver_info->unbind (dev, intf);
-
 	usb_kill_urb(dev->interrupt);
 	usb_free_urb(dev->interrupt);
 	kfree(dev->padding_pkt);
-- 
2.29.2

Re: [PATCH net-next v2 0/8] port asix ax88772 to the PHYlib

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Mon,  7 Jun 2021 10:27:19 +0200 you wrote:
> changes v2:
> - add Reviewed-by: Andrew Lunn <andrew@lunn.ch> to some patches
> - refactor asix_read_phy_addr() and add error handling for all callers
> - refactor asix_mdio_bus_read()
> 
> Port ax88772 part of asix driver to the phylib to be able to use more
> advanced external PHY attached to this controller.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/8] net: usb: asix: ax88772_bind: use devm_kzalloc() instead of kzalloc()
    https://git.kernel.org/netdev/net-next/c/218d154f540a
  - [net-next,v2,2/8] net: usb: asix: refactor asix_read_phy_addr() and handle errors on return
    https://git.kernel.org/netdev/net-next/c/7e88b11a862a
  - [net-next,v2,3/8] net: usb/phy: asix: add support for ax88772A/C PHYs
    https://git.kernel.org/netdev/net-next/c/dde258469257
  - [net-next,v2,4/8] net: usb: asix: ax88772: add phylib support
    https://git.kernel.org/netdev/net-next/c/e532a096be0e
  - [net-next,v2,5/8] net: usb: asix: ax88772: add generic selftest support
    https://git.kernel.org/netdev/net-next/c/34a1dee6bc44
  - [net-next,v2,6/8] net: usb: asix: add error handling for asix_mdio_* functions
    https://git.kernel.org/netdev/net-next/c/d275afb66371
  - [net-next,v2,7/8] net: phy: do not print dump stack if device was removed
    https://git.kernel.org/netdev/net-next/c/06edf1a940be
  - [net-next,v2,8/8] usbnet: run unbind() before unregister_netdev()
    https://git.kernel.org/netdev/net-next/c/2c9d6c2b871d

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html