Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support
From: Marek Szyprowski <m.szyprowski@samsung.com>
Date: 2021-06-09 09:59:31
Also in:
linux-usb, lkml
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.
quoted hunk ↗ jump to hunk
--- 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