diff mbox series

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

Message ID 20210607082727.26045-5-o.rempel@pengutronix.de (mailing list archive)
State Accepted
Commit e532a096be0e5e570b383e71d4560e7f04384e0f
Delegated to: Netdev Maintainers
Headers show
Series port asix ax88772 to the PHYlib | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 5 maintainers not CCed: oneukum@suse.com george.kennedy@oracle.com linux@rempel-privat.de bjorn@mork.no himadrispandya@gmail.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 297 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Oleksij Rempel June 7, 2021, 8:27 a.m. UTC
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(-)

Comments

Marek Szyprowski June 9, 2021, 9:59 a.m. UTC | #1
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
Oleksij Rempel June 9, 2021, 12:46 p.m. UTC | #2
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
Heiner Kallweit June 9, 2021, 1:12 p.m. UTC | #3
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")
Marek Szyprowski June 10, 2021, 10:31 a.m. UTC | #4
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
Jon Hunter June 10, 2021, 12:54 p.m. UTC | #5
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
Oleksij Rempel June 10, 2021, 1:36 p.m. UTC | #6
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
Oleksij Rempel June 10, 2021, 2:22 p.m. UTC | #7
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
>
Marek Szyprowski June 18, 2021, 8:39 a.m. UTC | #8
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
Oleksij Rempel June 18, 2021, 10:13 a.m. UTC | #9
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
> 
>
Marek Szyprowski June 18, 2021, 10:45 a.m. UTC | #10
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 June 18, 2021, 10:57 a.m. UTC | #11
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
Heiner Kallweit June 18, 2021, 11:04 a.m. UTC | #12
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
>>
>>
>
Marek Szyprowski June 18, 2021, 11:11 a.m. UTC | #13
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
Oleksij Rempel June 18, 2021, 1:10 p.m. UTC | #14
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.
Oleksij Rempel June 18, 2021, 1:20 p.m. UTC | #15
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,
Marek Szyprowski June 21, 2021, 6:05 a.m. UTC | #16
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
Oleksij Rempel June 23, 2021, 7:06 a.m. UTC | #17
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
Marek Szyprowski June 28, 2021, 8:27 a.m. UTC | #18
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
diff mbox series

Patch

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)
 {