| Message ID | 20210607082727.26045-5-o.rempel@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | port asix ax88772 to the PHYlib | expand |
Hi Oleksij, On 07.06.2021 10:27, Oleksij Rempel wrote: > To be able to use ax88772 with external PHYs and use advantage of > existing PHY drivers, we need to port at least ax88772 part of asix > driver to the phylib framework. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> This patch landed recently in linux-next as commit e532a096be0e ("net: usb: asix: ax88772: add phylib support"). I found that it causes some warnings on boards with those devices, see the following log: root@target:~# time rtcwake -s10 -mmem rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun 9 08:16:41 2021 [ 231.226579] PM: suspend entry (deep) [ 231.231697] Filesystems sync: 0.002 seconds [ 231.261761] Freezing user space processes ... (elapsed 0.002 seconds) done. [ 231.270526] OOM killer disabled. [ 231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done. [ 231.282229] printk: Suspending console(s) (use no_console_suspend to debug) ... [ 231.710852] Disabling non-boot CPUs ... ... [ 231.901794] Enabling non-boot CPUs ... ... [ 232.225640] usb usb3: root hub lost power or was reset [ 232.225746] usb usb1: root hub lost power or was reset [ 232.225864] usb usb5: root hub lost power or was reset [ 232.226206] usb usb6: root hub lost power or was reset [ 232.226207] usb usb4: root hub lost power or was reset [ 232.297749] usb usb2: root hub lost power or was reset [ 232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 [ 232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access [ 232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22 [ 232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 [ 232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 returns -22 [ 232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to resume: error -22 [ 232.563712] usb 1-1: reset high-speed USB device number 2 using exynos-ehci [ 232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd [ 233.730994] OOM killer enabled. [ 233.734122] Restarting tasks ... done. [ 233.754992] PM: suspend exit real 0m11.546s user 0m0.000s sys 0m0.530s root@target:~# sleep 2 root@target:~# time rtcwake -s10 -mmem rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun 9 08:17:02 2021 [ 241.959608] PM: suspend entry (deep) [ 241.963446] Filesystems sync: 0.001 seconds [ 241.978619] Freezing user space processes ... (elapsed 0.004 seconds) done. [ 241.989199] OOM killer disabled. [ 241.992215] Freezing remaining freezable tasks ... (elapsed 0.005 seconds) done. [ 242.003979] printk: Suspending console(s) (use no_console_suspend to debug) ... [ 242.592030] Disabling non-boot CPUs ... ... [ 242.879721] Enabling non-boot CPUs ... ... [ 243.145870] usb usb3: root hub lost power or was reset [ 243.145910] usb usb4: root hub lost power or was reset [ 243.147084] usb usb5: root hub lost power or was reset [ 243.147157] usb usb6: root hub lost power or was reset [ 243.147298] usb usb1: root hub lost power or was reset [ 243.217137] usb usb2: root hub lost power or was reset [ 243.283807] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 [ 243.284005] asix 3-1:1.0 eth0: Failed to enable software MII access [ 243.285526] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22 [ 243.285676] asix 3-1:1.0 eth0: Failed to read reg index 0x0004: -22 [ 243.285769] ------------[ cut here ]------------ [ 243.286011] WARNING: CPU: 2 PID: 2069 at drivers/net/phy/phy.c:916 phy_error+0x28/0x68 [ 243.286115] Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl bluetooth s5p_mfc uvcvideo s5p_jpeg exynos_gsc v [ 243.287490] CPU: 2 PID: 2069 Comm: kworker/2:5 Not tainted 5.13.0-rc5-next-20210608 #10443 [ 243.287555] Hardware name: Samsung Exynos (Flattened Device Tree) [ 243.287609] Workqueue: events_power_efficient phy_state_machine [ 243.287716] [<c0111920>] (unwind_backtrace) from [<c010d0cc>] (show_stack+0x10/0x14) [ 243.287807] [<c010d0cc>] (show_stack) from [<c0b62360>] (dump_stack_lvl+0xa0/0xc0) [ 243.287882] [<c0b62360>] (dump_stack_lvl) from [<c0127960>] (__warn+0x118/0x11c) [ 243.287954] [<c0127960>] (__warn) from [<c0127a18>] (warn_slowpath_fmt+0xb4/0xbc) [ 243.288021] [<c0127a18>] (warn_slowpath_fmt) from [<c0734968>] (phy_error+0x28/0x68) [ 243.288094] [<c0734968>] (phy_error) from [<c0735d6c>] (phy_state_machine+0x218/0x278) [ 243.288173] [<c0735d6c>] (phy_state_machine) from [<c014ae08>] (process_one_work+0x30c/0x884) [ 243.288254] [<c014ae08>] (process_one_work) from [<c014b3d8>] (worker_thread+0x58/0x594) [ 243.288333] [<c014b3d8>] (worker_thread) from [<c0153944>] (kthread+0x160/0x1c0) [ 243.288408] [<c0153944>] (kthread) from [<c010011c>] (ret_from_fork+0x14/0x38) [ 243.288475] Exception stack(0xc4683fb0 to 0xc4683ff8) [ 243.288531] 3fa0: 00000000 00000000 00000000 00000000 [ 243.288587] 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 243.288641] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 243.288690] irq event stamp: 1611 [ 243.288744] hardirqs last enabled at (1619): [<c01a6ef0>] vprintk_emit+0x230/0x290 [ 243.288830] hardirqs last disabled at (1626): [<c01a6f2c>] vprintk_emit+0x26c/0x290 [ 243.288906] softirqs last enabled at (1012): [<c0101768>] __do_softirq+0x500/0x63c [ 243.288978] softirqs last disabled at (1007): [<c01315b4>] irq_exit+0x214/0x220 [ 243.289055] ---[ end trace eeacda95eb7db60a ]--- [ 243.289345] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 [ 243.289466] asix 3-1:1.0 eth0: Failed to write Medium Mode mode to 0x0000: ffffffea [ 243.289540] asix 3-1:1.0 eth0: Link is Down [ 243.482809] usb 1-1: reset high-speed USB device number 2 using exynos-ehci [ 243.647251] usb 3-1: reset high-speed USB device number 2 using xhci-hcd [ 244.847161] OOM killer enabled. [ 244.850221] Restarting tasks ... done. [ 244.861372] PM: suspend exit real 0m13.050s user 0m0.000s sys 0m1.152s root@target:~# It looks that some kind of system suspend/resume integration for phylib is not implemented. > --- > drivers/net/usb/asix.h | 9 +++ > drivers/net/usb/asix_common.c | 37 ++++++++++ > drivers/net/usb/asix_devices.c | 120 +++++++++++++++++++++------------ > drivers/net/usb/ax88172a.c | 14 ---- > 4 files changed, 122 insertions(+), 58 deletions(-) > > diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h > index edb94efd265e..2122d302e643 100644 > --- a/drivers/net/usb/asix.h > +++ b/drivers/net/usb/asix.h > @@ -25,6 +25,7 @@ > #include <linux/usb/usbnet.h> > #include <linux/slab.h> > #include <linux/if_vlan.h> > +#include <linux/phy.h> > > #define DRIVER_VERSION "22-Dec-2011" > #define DRIVER_NAME "asix" > @@ -178,6 +179,10 @@ struct asix_common_private { > u16 presvd_phy_advertise; > u16 presvd_phy_bmcr; > struct asix_rx_fixup_info rx_fixup_info; > + struct mii_bus *mdio; > + struct phy_device *phydev; > + u16 phy_addr; > + char phy_name[20]; > }; > > extern const struct driver_info ax88172a_info; > @@ -214,6 +219,7 @@ int asix_write_rx_ctl(struct usbnet *dev, u16 mode, int in_pm); > > u16 asix_read_medium_status(struct usbnet *dev, int in_pm); > int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm); > +void asix_adjust_link(struct net_device *netdev); > > int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm); > > @@ -222,6 +228,9 @@ void asix_set_multicast(struct net_device *net); > int asix_mdio_read(struct net_device *netdev, int phy_id, int loc); > void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val); > > +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum); > +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val); > + > int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc); > void asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc, > int val); > diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c > index e1109f1a8dd5..085bc8281082 100644 > --- a/drivers/net/usb/asix_common.c > +++ b/drivers/net/usb/asix_common.c > @@ -384,6 +384,27 @@ int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm) > return ret; > } > > +/* set MAC link settings according to information from phylib */ > +void asix_adjust_link(struct net_device *netdev) > +{ > + struct phy_device *phydev = netdev->phydev; > + struct usbnet *dev = netdev_priv(netdev); > + u16 mode = 0; > + > + if (phydev->link) { > + mode = AX88772_MEDIUM_DEFAULT; > + > + if (phydev->duplex == DUPLEX_HALF) > + mode &= ~AX_MEDIUM_FD; > + > + if (phydev->speed != SPEED_100) > + mode &= ~AX_MEDIUM_PS; > + } > + > + asix_write_medium_mode(dev, mode, 0); > + phy_print_status(phydev); > +} > + > int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm) > { > int ret; > @@ -506,6 +527,22 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val) > mutex_unlock(&dev->phy_mutex); > } > > +/* MDIO read and write wrappers for phylib */ > +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum) > +{ > + struct usbnet *priv = bus->priv; > + > + return asix_mdio_read(priv->net, phy_id, regnum); > +} > + > +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val) > +{ > + struct usbnet *priv = bus->priv; > + > + asix_mdio_write(priv->net, phy_id, regnum, val); > + return 0; > +} > + > int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc) > { > struct usbnet *dev = netdev_priv(netdev); > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c > index 00b6ac0570eb..e4cd85e38edd 100644 > --- a/drivers/net/usb/asix_devices.c > +++ b/drivers/net/usb/asix_devices.c > @@ -285,7 +285,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf) > > static const struct ethtool_ops ax88772_ethtool_ops = { > .get_drvinfo = asix_get_drvinfo, > - .get_link = asix_get_link, > + .get_link = usbnet_get_link, > .get_msglevel = usbnet_get_msglevel, > .set_msglevel = usbnet_set_msglevel, > .get_wol = asix_get_wol, > @@ -293,37 +293,15 @@ static const struct ethtool_ops ax88772_ethtool_ops = { > .get_eeprom_len = asix_get_eeprom_len, > .get_eeprom = asix_get_eeprom, > .set_eeprom = asix_set_eeprom, > - .nway_reset = usbnet_nway_reset, > - .get_link_ksettings = usbnet_get_link_ksettings_mii, > - .set_link_ksettings = usbnet_set_link_ksettings_mii, > + .nway_reset = phy_ethtool_nway_reset, > + .get_link_ksettings = phy_ethtool_get_link_ksettings, > + .set_link_ksettings = phy_ethtool_set_link_ksettings, > }; > > -static int ax88772_link_reset(struct usbnet *dev) > -{ > - u16 mode; > - struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET }; > - > - mii_check_media(&dev->mii, 1, 1); > - mii_ethtool_gset(&dev->mii, &ecmd); > - mode = AX88772_MEDIUM_DEFAULT; > - > - if (ethtool_cmd_speed(&ecmd) != SPEED_100) > - mode &= ~AX_MEDIUM_PS; > - > - if (ecmd.duplex != DUPLEX_FULL) > - mode &= ~AX_MEDIUM_FD; > - > - netdev_dbg(dev->net, "ax88772_link_reset() speed: %u duplex: %d setting mode to 0x%04x\n", > - ethtool_cmd_speed(&ecmd), ecmd.duplex, mode); > - > - asix_write_medium_mode(dev, mode, 0); > - > - return 0; > -} > - > static int ax88772_reset(struct usbnet *dev) > { > struct asix_data *data = (struct asix_data *)&dev->data; > + struct asix_common_private *priv = dev->driver_priv; > int ret; > > /* Rewrite MAC address */ > @@ -342,6 +320,8 @@ static int ax88772_reset(struct usbnet *dev) > if (ret < 0) > goto out; > > + phy_start(priv->phydev); > + > return 0; > > out: > @@ -586,7 +566,7 @@ static const struct net_device_ops ax88772_netdev_ops = { > .ndo_get_stats64 = dev_get_tstats64, > .ndo_set_mac_address = asix_set_mac_address, > .ndo_validate_addr = eth_validate_addr, > - .ndo_do_ioctl = asix_ioctl, > + .ndo_do_ioctl = phy_do_ioctl_running, > .ndo_set_rx_mode = asix_set_multicast, > }; > > @@ -677,12 +657,57 @@ static int asix_resume(struct usb_interface *intf) > return usbnet_resume(intf); > } > > +static int ax88772_init_mdio(struct usbnet *dev) > +{ > + struct asix_common_private *priv = dev->driver_priv; > + > + priv->mdio = devm_mdiobus_alloc(&dev->udev->dev); > + if (!priv->mdio) > + return -ENOMEM; > + > + priv->mdio->priv = dev; > + priv->mdio->read = &asix_mdio_bus_read; > + priv->mdio->write = &asix_mdio_bus_write; > + priv->mdio->name = "Asix MDIO Bus"; > + /* mii bus name is usb-<usb bus number>-<usb device number> */ > + snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d", > + dev->udev->bus->busnum, dev->udev->devnum); > + > + return devm_mdiobus_register(&dev->udev->dev, priv->mdio); > +} > + > +static int ax88772_init_phy(struct usbnet *dev) > +{ > + struct asix_common_private *priv = dev->driver_priv; > + int ret; > + > + priv->phy_addr = asix_read_phy_addr(dev, true); > + if (priv->phy_addr < 0) > + return priv->phy_addr; > + > + snprintf(priv->phy_name, sizeof(priv->phy_name), PHY_ID_FMT, > + priv->mdio->id, priv->phy_addr); > + > + priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link, > + PHY_INTERFACE_MODE_INTERNAL); > + if (IS_ERR(priv->phydev)) { > + netdev_err(dev->net, "Could not connect to PHY device %s\n", > + priv->phy_name); > + ret = PTR_ERR(priv->phydev); > + return ret; > + } > + > + phy_attached_info(priv->phydev); > + > + return 0; > +} > + > static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) > { > - int ret, i; > u8 buf[ETH_ALEN] = {0}, chipcode = 0; > - u32 phyid; > struct asix_common_private *priv; > + int ret, i; > + u32 phyid; > > usbnet_get_endpoints(dev, intf); > > @@ -714,17 +739,6 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) > > asix_set_netdev_dev_addr(dev, buf); > > - /* Initialize MII structure */ > - dev->mii.dev = dev->net; > - dev->mii.mdio_read = asix_mdio_read; > - dev->mii.mdio_write = asix_mdio_write; > - dev->mii.phy_id_mask = 0x1f; > - dev->mii.reg_num_mask = 0x1f; > - > - dev->mii.phy_id = asix_read_phy_addr(dev, true); > - if (dev->mii.phy_id < 0) > - return dev->mii.phy_id; > - > dev->net->netdev_ops = &ax88772_netdev_ops; > dev->net->ethtool_ops = &ax88772_ethtool_ops; > dev->net->needed_headroom = 4; /* cf asix_tx_fixup() */ > @@ -768,11 +782,31 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) > priv->suspend = ax88772_suspend; > } > > + ret = ax88772_init_mdio(dev); > + if (ret) > + return ret; > + > + return ax88772_init_phy(dev); > +} > + > +static int ax88772_stop(struct usbnet *dev) > +{ > + struct asix_common_private *priv = dev->driver_priv; > + > + /* On unplugged USB, we will get MDIO communication errors and the > + * PHY will be set in to PHY_HALTED state. > + */ > + if (priv->phydev->state != PHY_HALTED) > + phy_stop(priv->phydev); > + > return 0; > } > > static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf) > { > + struct asix_common_private *priv = dev->driver_priv; > + > + phy_disconnect(priv->phydev); > asix_rx_fixup_common_free(dev->driver_priv); > } > > @@ -1161,8 +1195,8 @@ static const struct driver_info ax88772_info = { > .bind = ax88772_bind, > .unbind = ax88772_unbind, > .status = asix_status, > - .link_reset = ax88772_link_reset, > .reset = ax88772_reset, > + .stop = ax88772_stop, > .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET, > .rx_fixup = asix_rx_fixup_common, > .tx_fixup = asix_tx_fixup, > @@ -1173,7 +1207,6 @@ static const struct driver_info ax88772b_info = { > .bind = ax88772_bind, > .unbind = ax88772_unbind, > .status = asix_status, > - .link_reset = ax88772_link_reset, > .reset = ax88772_reset, > .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | > FLAG_MULTI_PACKET, > @@ -1209,7 +1242,6 @@ static const struct driver_info hg20f9_info = { > .bind = ax88772_bind, > .unbind = ax88772_unbind, > .status = asix_status, > - .link_reset = ax88772_link_reset, > .reset = ax88772_reset, > .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | > FLAG_MULTI_PACKET, > diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c > index c8ca5187eece..2e2081346740 100644 > --- a/drivers/net/usb/ax88172a.c > +++ b/drivers/net/usb/ax88172a.c > @@ -25,20 +25,6 @@ struct ax88172a_private { > struct asix_rx_fixup_info rx_fixup_info; > }; > > -/* MDIO read and write wrappers for phylib */ > -static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum) > -{ > - return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id, > - regnum); > -} > - > -static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, > - u16 val) > -{ > - asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val); > - return 0; > -} > - > /* set MAC link settings according to information from phylib */ > static void ax88172a_adjust_link(struct net_device *netdev) > { Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
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
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")
Hi Oleksij, On 09.06.2021 14:46, Oleksij Rempel wrote: > On Wed, Jun 09, 2021 at 11:59:23AM +0200, Marek Szyprowski wrote: >> On 07.06.2021 10:27, Oleksij Rempel wrote: >>> To be able to use ax88772 with external PHYs and use advantage of >>> existing PHY drivers, we need to port at least ax88772 part of asix >>> driver to the phylib framework. >>> >>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> This patch landed recently in linux-next as commit e532a096be0e ("net: >> usb: asix: ax88772: add phylib support"). I found that it causes some >> warnings on boards with those devices, see the following log: >> >> root@target:~# time rtcwake -s10 -mmem >> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun 9 08:16:41 2021 >> [ 231.226579] PM: suspend entry (deep) >> [ 231.231697] Filesystems sync: 0.002 seconds >> [ 231.261761] Freezing user space processes ... (elapsed 0.002 seconds) >> done. >> [ 231.270526] OOM killer disabled. >> [ 231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 >> seconds) done. >> [ 231.282229] printk: Suspending console(s) (use no_console_suspend to >> debug) >> ... >> [ 231.710852] Disabling non-boot CPUs ... >> ... >> [ 231.901794] Enabling non-boot CPUs ... >> ... >> [ 232.225640] usb usb3: root hub lost power or was reset >> [ 232.225746] usb usb1: root hub lost power or was reset >> [ 232.225864] usb usb5: root hub lost power or was reset >> [ 232.226206] usb usb6: root hub lost power or was reset >> [ 232.226207] usb usb4: root hub lost power or was reset >> [ 232.297749] usb usb2: root hub lost power or was reset >> [ 232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 >> [ 232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access >> [ 232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22 >> [ 232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 >> [ 232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 >> returns -22 >> [ 232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to >> resume: error -22 >> [ 232.563712] usb 1-1: reset high-speed USB device number 2 using >> exynos-ehci >> [ 232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd >> [ 233.730994] OOM killer enabled. >> [ 233.734122] Restarting tasks ... done. >> [ 233.754992] PM: suspend exit >> >> real 0m11.546s >> user 0m0.000s >> sys 0m0.530s >> root@target:~# sleep 2 >> root@target:~# time rtcwake -s10 -mmem >> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun 9 08:17:02 2021 >> [ 241.959608] PM: suspend entry (deep) >> [ 241.963446] Filesystems sync: 0.001 seconds >> [ 241.978619] Freezing user space processes ... (elapsed 0.004 seconds) >> done. >> [ 241.989199] OOM killer disabled. >> [ 241.992215] Freezing remaining freezable tasks ... (elapsed 0.005 >> seconds) done. >> [ 242.003979] printk: Suspending console(s) (use no_console_suspend to >> debug) >> ... >> [ 242.592030] Disabling non-boot CPUs ... >> ... >> [ 242.879721] Enabling non-boot CPUs ... >> ... >> [ 243.145870] usb usb3: root hub lost power or was reset >> [ 243.145910] usb usb4: root hub lost power or was reset >> [ 243.147084] usb usb5: root hub lost power or was reset >> [ 243.147157] usb usb6: root hub lost power or was reset >> [ 243.147298] usb usb1: root hub lost power or was reset >> [ 243.217137] usb usb2: root hub lost power or was reset >> [ 243.283807] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 >> [ 243.284005] asix 3-1:1.0 eth0: Failed to enable software MII access >> [ 243.285526] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22 >> [ 243.285676] asix 3-1:1.0 eth0: Failed to read reg index 0x0004: -22 >> [ 243.285769] ------------[ cut here ]------------ >> [ 243.286011] WARNING: CPU: 2 PID: 2069 at drivers/net/phy/phy.c:916 >> phy_error+0x28/0x68 >> [ 243.286115] Modules linked in: cmac bnep mwifiex_sdio mwifiex >> sha256_generic libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl >> bluetooth s5p_mfc uvcvideo s5p_jpeg exynos_gsc v >> [ 243.287490] CPU: 2 PID: 2069 Comm: kworker/2:5 Not tainted >> 5.13.0-rc5-next-20210608 #10443 >> [ 243.287555] Hardware name: Samsung Exynos (Flattened Device Tree) >> [ 243.287609] Workqueue: events_power_efficient phy_state_machine >> [ 243.287716] [<c0111920>] (unwind_backtrace) from [<c010d0cc>] >> (show_stack+0x10/0x14) >> [ 243.287807] [<c010d0cc>] (show_stack) from [<c0b62360>] >> (dump_stack_lvl+0xa0/0xc0) >> [ 243.287882] [<c0b62360>] (dump_stack_lvl) from [<c0127960>] >> (__warn+0x118/0x11c) >> [ 243.287954] [<c0127960>] (__warn) from [<c0127a18>] >> (warn_slowpath_fmt+0xb4/0xbc) >> [ 243.288021] [<c0127a18>] (warn_slowpath_fmt) from [<c0734968>] >> (phy_error+0x28/0x68) >> [ 243.288094] [<c0734968>] (phy_error) from [<c0735d6c>] >> (phy_state_machine+0x218/0x278) >> [ 243.288173] [<c0735d6c>] (phy_state_machine) from [<c014ae08>] >> (process_one_work+0x30c/0x884) >> [ 243.288254] [<c014ae08>] (process_one_work) from [<c014b3d8>] >> (worker_thread+0x58/0x594) >> [ 243.288333] [<c014b3d8>] (worker_thread) from [<c0153944>] >> (kthread+0x160/0x1c0) >> [ 243.288408] [<c0153944>] (kthread) from [<c010011c>] >> (ret_from_fork+0x14/0x38) >> [ 243.288475] Exception stack(0xc4683fb0 to 0xc4683ff8) >> [ 243.288531] 3fa0: 00000000 >> 00000000 00000000 00000000 >> [ 243.288587] 3fc0: 00000000 00000000 00000000 00000000 00000000 >> 00000000 00000000 00000000 >> [ 243.288641] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 >> [ 243.288690] irq event stamp: 1611 >> [ 243.288744] hardirqs last enabled at (1619): [<c01a6ef0>] >> vprintk_emit+0x230/0x290 >> [ 243.288830] hardirqs last disabled at (1626): [<c01a6f2c>] >> vprintk_emit+0x26c/0x290 >> [ 243.288906] softirqs last enabled at (1012): [<c0101768>] >> __do_softirq+0x500/0x63c >> [ 243.288978] softirqs last disabled at (1007): [<c01315b4>] >> irq_exit+0x214/0x220 >> [ 243.289055] ---[ end trace eeacda95eb7db60a ]--- >> [ 243.289345] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 >> [ 243.289466] asix 3-1:1.0 eth0: Failed to write Medium Mode mode to >> 0x0000: ffffffea >> [ 243.289540] asix 3-1:1.0 eth0: Link is Down >> [ 243.482809] usb 1-1: reset high-speed USB device number 2 using >> exynos-ehci >> [ 243.647251] usb 3-1: reset high-speed USB device number 2 using xhci-hcd >> [ 244.847161] OOM killer enabled. >> [ 244.850221] Restarting tasks ... done. >> [ 244.861372] PM: suspend exit >> >> real 0m13.050s >> user 0m0.000s >> sys 0m1.152s >> root@target:~# >> >> It looks that some kind of system suspend/resume integration for phylib >> is not implemented. > Probably it is should be handled only by the asix driver. I'll take a > look in to it. Did interface was able to resume after printing some > warnings? Nope. The network is not operational after suspend/resume cycle after applying this patch. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
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
On Wed, Jun 09, 2021 at 03:12:37PM +0200, Heiner Kallweit wrote: > On 09.06.2021 14:46, Oleksij Rempel wrote: > > Hi Marek, > > > > On Wed, Jun 09, 2021 at 11:59:23AM +0200, Marek Szyprowski wrote: > >> Hi Oleksij, > >> > >> On 07.06.2021 10:27, Oleksij Rempel wrote: > >>> To be able to use ax88772 with external PHYs and use advantage of > >>> existing PHY drivers, we need to port at least ax88772 part of asix > >>> driver to the phylib framework. > >>> > >>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > >> > >> This patch landed recently in linux-next as commit e532a096be0e ("net: > >> usb: asix: ax88772: add phylib support"). I found that it causes some > >> warnings on boards with those devices, see the following log: > >> > >> root@target:~# time rtcwake -s10 -mmem > >> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun 9 08:16:41 2021 > >> [ 231.226579] PM: suspend entry (deep) > >> [ 231.231697] Filesystems sync: 0.002 seconds > >> [ 231.261761] Freezing user space processes ... (elapsed 0.002 seconds) > >> done. > >> [ 231.270526] OOM killer disabled. > >> [ 231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 > >> seconds) done. > >> [ 231.282229] printk: Suspending console(s) (use no_console_suspend to > >> debug) > >> ... > >> [ 231.710852] Disabling non-boot CPUs ... > >> ... > >> [ 231.901794] Enabling non-boot CPUs ... > >> ... > >> [ 232.225640] usb usb3: root hub lost power or was reset > >> [ 232.225746] usb usb1: root hub lost power or was reset > >> [ 232.225864] usb usb5: root hub lost power or was reset > >> [ 232.226206] usb usb6: root hub lost power or was reset > >> [ 232.226207] usb usb4: root hub lost power or was reset > >> [ 232.297749] usb usb2: root hub lost power or was reset > >> [ 232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 > >> [ 232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access > >> [ 232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22 > >> [ 232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 > >> [ 232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 > >> returns -22 > >> [ 232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to > >> resume: error -22 > >> [ 232.563712] usb 1-1: reset high-speed USB device number 2 using > >> exynos-ehci > >> [ 232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd > >> [ 233.730994] OOM killer enabled. > >> [ 233.734122] Restarting tasks ... done. > >> [ 233.754992] PM: suspend exit > >> > >> real 0m11.546s > >> user 0m0.000s > >> sys 0m0.530s > >> root@target:~# sleep 2 > >> root@target:~# time rtcwake -s10 -mmem > >> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun 9 08:17:02 2021 > >> [ 241.959608] PM: suspend entry (deep) > >> [ 241.963446] Filesystems sync: 0.001 seconds > >> [ 241.978619] Freezing user space processes ... (elapsed 0.004 seconds) > >> done. > >> [ 241.989199] OOM killer disabled. > >> [ 241.992215] Freezing remaining freezable tasks ... (elapsed 0.005 > >> seconds) done. > >> [ 242.003979] printk: Suspending console(s) (use no_console_suspend to > >> debug) > >> ... > >> [ 242.592030] Disabling non-boot CPUs ... > >> ... > >> [ 242.879721] Enabling non-boot CPUs ... > >> ... > >> [ 243.145870] usb usb3: root hub lost power or was reset > >> [ 243.145910] usb usb4: root hub lost power or was reset > >> [ 243.147084] usb usb5: root hub lost power or was reset > >> [ 243.147157] usb usb6: root hub lost power or was reset > >> [ 243.147298] usb usb1: root hub lost power or was reset > >> [ 243.217137] usb usb2: root hub lost power or was reset > >> [ 243.283807] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 > >> [ 243.284005] asix 3-1:1.0 eth0: Failed to enable software MII access > >> [ 243.285526] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22 > >> [ 243.285676] asix 3-1:1.0 eth0: Failed to read reg index 0x0004: -22 > >> [ 243.285769] ------------[ cut here ]------------ > >> [ 243.286011] WARNING: CPU: 2 PID: 2069 at drivers/net/phy/phy.c:916 > >> phy_error+0x28/0x68 > >> [ 243.286115] Modules linked in: cmac bnep mwifiex_sdio mwifiex > >> sha256_generic libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl > >> bluetooth s5p_mfc uvcvideo s5p_jpeg exynos_gsc v > >> [ 243.287490] CPU: 2 PID: 2069 Comm: kworker/2:5 Not tainted > >> 5.13.0-rc5-next-20210608 #10443 > >> [ 243.287555] Hardware name: Samsung Exynos (Flattened Device Tree) > >> [ 243.287609] Workqueue: events_power_efficient phy_state_machine > >> [ 243.287716] [<c0111920>] (unwind_backtrace) from [<c010d0cc>] > >> (show_stack+0x10/0x14) > >> [ 243.287807] [<c010d0cc>] (show_stack) from [<c0b62360>] > >> (dump_stack_lvl+0xa0/0xc0) > >> [ 243.287882] [<c0b62360>] (dump_stack_lvl) from [<c0127960>] > >> (__warn+0x118/0x11c) > >> [ 243.287954] [<c0127960>] (__warn) from [<c0127a18>] > >> (warn_slowpath_fmt+0xb4/0xbc) > >> [ 243.288021] [<c0127a18>] (warn_slowpath_fmt) from [<c0734968>] > >> (phy_error+0x28/0x68) > >> [ 243.288094] [<c0734968>] (phy_error) from [<c0735d6c>] > >> (phy_state_machine+0x218/0x278) > >> [ 243.288173] [<c0735d6c>] (phy_state_machine) from [<c014ae08>] > >> (process_one_work+0x30c/0x884) > >> [ 243.288254] [<c014ae08>] (process_one_work) from [<c014b3d8>] > >> (worker_thread+0x58/0x594) > >> [ 243.288333] [<c014b3d8>] (worker_thread) from [<c0153944>] > >> (kthread+0x160/0x1c0) > >> [ 243.288408] [<c0153944>] (kthread) from [<c010011c>] > >> (ret_from_fork+0x14/0x38) > >> [ 243.288475] Exception stack(0xc4683fb0 to 0xc4683ff8) > >> [ 243.288531] 3fa0: 00000000 > >> 00000000 00000000 00000000 > >> [ 243.288587] 3fc0: 00000000 00000000 00000000 00000000 00000000 > >> 00000000 00000000 00000000 > >> [ 243.288641] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 > >> [ 243.288690] irq event stamp: 1611 > >> [ 243.288744] hardirqs last enabled at (1619): [<c01a6ef0>] > >> vprintk_emit+0x230/0x290 > >> [ 243.288830] hardirqs last disabled at (1626): [<c01a6f2c>] > >> vprintk_emit+0x26c/0x290 > >> [ 243.288906] softirqs last enabled at (1012): [<c0101768>] > >> __do_softirq+0x500/0x63c > >> [ 243.288978] softirqs last disabled at (1007): [<c01315b4>] > >> irq_exit+0x214/0x220 > >> [ 243.289055] ---[ end trace eeacda95eb7db60a ]--- > >> [ 243.289345] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 > >> [ 243.289466] asix 3-1:1.0 eth0: Failed to write Medium Mode mode to > >> 0x0000: ffffffea > >> [ 243.289540] asix 3-1:1.0 eth0: Link is Down > >> [ 243.482809] usb 1-1: reset high-speed USB device number 2 using > >> exynos-ehci > >> [ 243.647251] usb 3-1: reset high-speed USB device number 2 using xhci-hcd > >> [ 244.847161] OOM killer enabled. > >> [ 244.850221] Restarting tasks ... done. > >> [ 244.861372] PM: suspend exit > >> > >> real 0m13.050s > >> user 0m0.000s > >> sys 0m1.152s > >> root@target:~# > >> > >> It looks that some kind of system suspend/resume integration for phylib > >> is not implemented. > > > > Probably it is should be handled only by the asix driver. I'll take a > > look in to it. Did interface was able to resume after printing some > > warnings? > > > > Regards, > > Oleksij > > > > Maybe it's a use case for the new mac_managed_pm flag, see > fba863b81604 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM") > Thx! this is the right one :) Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Marek and Jon, I just send a patch to fix suspend/resume. It was tested on ax88772A and ax88772C on iMX6 host. Can you please confirm if it works for you? Regards, Oleksij net: usb: asix: ax88772: manage PHY PM from MAC On Thu, Jun 10, 2021 at 01:54:12PM +0100, Jon Hunter wrote: > > On 09/06/2021 10:59, Marek Szyprowski wrote: > > Hi Oleksij, > > > > On 07.06.2021 10:27, Oleksij Rempel wrote: > >> To be able to use ax88772 with external PHYs and use advantage of > >> existing PHY drivers, we need to port at least ax88772 part of asix > >> driver to the phylib framework. > >> > >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > > This patch landed recently in linux-next as commit e532a096be0e ("net: > > usb: asix: ax88772: add phylib support"). I found that it causes some > > warnings on boards with those devices, see the following log: > > > > root@target:~# time rtcwake -s10 -mmem > > rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Jun 9 08:16:41 2021 > > [ 231.226579] PM: suspend entry (deep) > > [ 231.231697] Filesystems sync: 0.002 seconds > > [ 231.261761] Freezing user space processes ... (elapsed 0.002 seconds) > > done. > > [ 231.270526] OOM killer disabled. > > [ 231.273557] Freezing remaining freezable tasks ... (elapsed 0.002 > > seconds) done. > > [ 231.282229] printk: Suspending console(s) (use no_console_suspend to > > debug) > > ... > > [ 231.710852] Disabling non-boot CPUs ... > > ... > > [ 231.901794] Enabling non-boot CPUs ... > > ... > > [ 232.225640] usb usb3: root hub lost power or was reset > > [ 232.225746] usb usb1: root hub lost power or was reset > > [ 232.225864] usb usb5: root hub lost power or was reset > > [ 232.226206] usb usb6: root hub lost power or was reset > > [ 232.226207] usb usb4: root hub lost power or was reset > > [ 232.297749] usb usb2: root hub lost power or was reset > > [ 232.343227] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 > > [ 232.343293] asix 3-1:1.0 eth0: Failed to enable software MII access > > [ 232.344486] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -22 > > [ 232.344512] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -22 > > [ 232.344529] PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x78 > > returns -22 > > [ 232.344554] Asix Electronics AX88772C usb-003:002:10: PM: failed to > > resume: error -22 > > [ 232.563712] usb 1-1: reset high-speed USB device number 2 using > > exynos-ehci > > [ 232.757653] usb 3-1: reset high-speed USB device number 2 using xhci-hcd > > [ 233.730994] OOM killer enabled. > > [ 233.734122] Restarting tasks ... done. > > [ 233.754992] PM: suspend exit > > > I am seeing a similar problem on a couple of our Tegra boards that > use AX88772A device. When resuming from suspend I see ... > > [ 54.733266] PM: suspend entry (deep) > > [ 54.737179] Filesystems sync: 0.000 seconds > > [ 54.741904] Freezing user space processes ... (elapsed 0.001 seconds) done. > > [ 54.750895] OOM killer disabled. > > [ 54.754452] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. > > [ 54.763505] printk: Suspending console(s) (use no_console_suspend to debug) > > [ 54.898334] Disabling non-boot CPUs ... > > [ 54.899546] IRQ 26: no longer affine to CPU1 > > [ 54.924373] Entering suspend state LP1 > > [ 54.924493] Enabling non-boot CPUs ... > > [ 54.933164] CPU1 is up > > [ 55.005166] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -113 > > [ 55.005226] asix 3-1:1.0 eth0: Failed to enable software MII access > > [ 55.006579] asix 3-1:1.0 eth0: Failed to read reg index 0x0000: -113 > > [ 55.006722] asix 3-1:1.0 eth0: Failed to write reg index 0x0000: -113 > > [ 55.006762] asix 3-1:1.0 eth0: Failed to enable software MII access > > > Interestingly once commit d275afb66371 ("net: usb: asix: add error > handling for asix_mdio_* functions") is applied, then resume from > suspend completely fails because the error is propagated. Bisect > is pointing to that patch, however, it is this patch that is > causing the problem. > > Cheers > Jon > > -- > nvpublic > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Oleksij, On 07.06.2021 10:27, Oleksij Rempel wrote: > To be able to use ax88772 with external PHYs and use advantage of > existing PHY drivers, we need to port at least ax88772 part of asix > driver to the phylib framework. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> I found one more issue with this patch. On one of my test boards (Samsung Exynos5250 SoC based Arndale) system fails to establish network connection just after starting the kernel when the driver is build-in. --->8--- # dmesg | grep asix [ 2.761928] usbcore: registered new interface driver asix [ 5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): invalid hw address, using random [ 6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 7a:9b:9a:f2:94:8e [ 14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow control off # ping -c2 host PING host (192.168.100.1) 56(84) bytes of data. From 192.168.100.20 icmp_seq=1 Destination Host Unreachable From 192.168.100.20 icmp_seq=2 Destination Host Unreachable --- host ping statistics --- 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 59ms --->8--- Calling ifup eth0 && ifdown eth0 fixes the network status: --->8--- # ifdown eth0 && ifup eth0 [ 60.474929] asix 1-3.2.4:1.0 eth0: Link is Down [ 60.623516] asix 1-3.2.4:1.0 eth0: Link is Down [ 62.774304] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready [ 62.786354] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow control off # ping -c2 host PING host (192.168.100.1) 56(84) bytes of data. 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=1.25 ms 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.853 ms --- host ping statistics --- 2 packets transmitted, 2 received, 0% packet loss, time 3ms rtt min/avg/max/mdev = 0.853/1.053/1.254/0.203 ms --->8--- When driver is loaded as a module (and without any other modules, so this is not a dependency issue), the connection is established properly just after the boot: --->8--- # dmesg | grep asix [ 13.633284] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): invalid hw address, using random [ 15.390350] asix 1-3.2.4:1.0 eth0: register 'asix' at usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 3a:51:11:08:aa:ea [ 15.414052] usbcore: registered new interface driver asix [ 15.832564] asix 1-3.2.4:1.0 eth0: Link is Down [ 18.053747] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow control off # ping -c2 host PING host (192.168.100.1) 56(84) bytes of data. 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=0.545 ms 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.742 ms --- host ping statistics --- 2 packets transmitted, 2 received, 0% packet loss, time 3ms rtt min/avg/max/mdev = 0.545/0.643/0.742/0.101 ms --->8--- Let me know if I can make any other tests that would help fixing this issue. > --- > drivers/net/usb/asix.h | 9 +++ > drivers/net/usb/asix_common.c | 37 ++++++++++ > drivers/net/usb/asix_devices.c | 120 +++++++++++++++++++++------------ > drivers/net/usb/ax88172a.c | 14 ---- > 4 files changed, 122 insertions(+), 58 deletions(-) > > diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h > index edb94efd265e..2122d302e643 100644 > --- a/drivers/net/usb/asix.h > +++ b/drivers/net/usb/asix.h > @@ -25,6 +25,7 @@ > #include <linux/usb/usbnet.h> > #include <linux/slab.h> > #include <linux/if_vlan.h> > +#include <linux/phy.h> > > #define DRIVER_VERSION "22-Dec-2011" > #define DRIVER_NAME "asix" > @@ -178,6 +179,10 @@ struct asix_common_private { > u16 presvd_phy_advertise; > u16 presvd_phy_bmcr; > struct asix_rx_fixup_info rx_fixup_info; > + struct mii_bus *mdio; > + struct phy_device *phydev; > + u16 phy_addr; > + char phy_name[20]; > }; > > extern const struct driver_info ax88172a_info; > @@ -214,6 +219,7 @@ int asix_write_rx_ctl(struct usbnet *dev, u16 mode, int in_pm); > > u16 asix_read_medium_status(struct usbnet *dev, int in_pm); > int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm); > +void asix_adjust_link(struct net_device *netdev); > > int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm); > > @@ -222,6 +228,9 @@ void asix_set_multicast(struct net_device *net); > int asix_mdio_read(struct net_device *netdev, int phy_id, int loc); > void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val); > > +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum); > +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val); > + > int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc); > void asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc, > int val); > diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c > index e1109f1a8dd5..085bc8281082 100644 > --- a/drivers/net/usb/asix_common.c > +++ b/drivers/net/usb/asix_common.c > @@ -384,6 +384,27 @@ int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm) > return ret; > } > > +/* set MAC link settings according to information from phylib */ > +void asix_adjust_link(struct net_device *netdev) > +{ > + struct phy_device *phydev = netdev->phydev; > + struct usbnet *dev = netdev_priv(netdev); > + u16 mode = 0; > + > + if (phydev->link) { > + mode = AX88772_MEDIUM_DEFAULT; > + > + if (phydev->duplex == DUPLEX_HALF) > + mode &= ~AX_MEDIUM_FD; > + > + if (phydev->speed != SPEED_100) > + mode &= ~AX_MEDIUM_PS; > + } > + > + asix_write_medium_mode(dev, mode, 0); > + phy_print_status(phydev); > +} > + > int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm) > { > int ret; > @@ -506,6 +527,22 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val) > mutex_unlock(&dev->phy_mutex); > } > > +/* MDIO read and write wrappers for phylib */ > +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum) > +{ > + struct usbnet *priv = bus->priv; > + > + return asix_mdio_read(priv->net, phy_id, regnum); > +} > + > +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val) > +{ > + struct usbnet *priv = bus->priv; > + > + asix_mdio_write(priv->net, phy_id, regnum, val); > + return 0; > +} > + > int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc) > { > struct usbnet *dev = netdev_priv(netdev); > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c > index 00b6ac0570eb..e4cd85e38edd 100644 > --- a/drivers/net/usb/asix_devices.c > +++ b/drivers/net/usb/asix_devices.c > @@ -285,7 +285,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf) > > static const struct ethtool_ops ax88772_ethtool_ops = { > .get_drvinfo = asix_get_drvinfo, > - .get_link = asix_get_link, > + .get_link = usbnet_get_link, > .get_msglevel = usbnet_get_msglevel, > .set_msglevel = usbnet_set_msglevel, > .get_wol = asix_get_wol, > @@ -293,37 +293,15 @@ static const struct ethtool_ops ax88772_ethtool_ops = { > .get_eeprom_len = asix_get_eeprom_len, > .get_eeprom = asix_get_eeprom, > .set_eeprom = asix_set_eeprom, > - .nway_reset = usbnet_nway_reset, > - .get_link_ksettings = usbnet_get_link_ksettings_mii, > - .set_link_ksettings = usbnet_set_link_ksettings_mii, > + .nway_reset = phy_ethtool_nway_reset, > + .get_link_ksettings = phy_ethtool_get_link_ksettings, > + .set_link_ksettings = phy_ethtool_set_link_ksettings, > }; > > -static int ax88772_link_reset(struct usbnet *dev) > -{ > - u16 mode; > - struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET }; > - > - mii_check_media(&dev->mii, 1, 1); > - mii_ethtool_gset(&dev->mii, &ecmd); > - mode = AX88772_MEDIUM_DEFAULT; > - > - if (ethtool_cmd_speed(&ecmd) != SPEED_100) > - mode &= ~AX_MEDIUM_PS; > - > - if (ecmd.duplex != DUPLEX_FULL) > - mode &= ~AX_MEDIUM_FD; > - > - netdev_dbg(dev->net, "ax88772_link_reset() speed: %u duplex: %d setting mode to 0x%04x\n", > - ethtool_cmd_speed(&ecmd), ecmd.duplex, mode); > - > - asix_write_medium_mode(dev, mode, 0); > - > - return 0; > -} > - > static int ax88772_reset(struct usbnet *dev) > { > struct asix_data *data = (struct asix_data *)&dev->data; > + struct asix_common_private *priv = dev->driver_priv; > int ret; > > /* Rewrite MAC address */ > @@ -342,6 +320,8 @@ static int ax88772_reset(struct usbnet *dev) > if (ret < 0) > goto out; > > + phy_start(priv->phydev); > + > return 0; > > out: > @@ -586,7 +566,7 @@ static const struct net_device_ops ax88772_netdev_ops = { > .ndo_get_stats64 = dev_get_tstats64, > .ndo_set_mac_address = asix_set_mac_address, > .ndo_validate_addr = eth_validate_addr, > - .ndo_do_ioctl = asix_ioctl, > + .ndo_do_ioctl = phy_do_ioctl_running, > .ndo_set_rx_mode = asix_set_multicast, > }; > > @@ -677,12 +657,57 @@ static int asix_resume(struct usb_interface *intf) > return usbnet_resume(intf); > } > > +static int ax88772_init_mdio(struct usbnet *dev) > +{ > + struct asix_common_private *priv = dev->driver_priv; > + > + priv->mdio = devm_mdiobus_alloc(&dev->udev->dev); > + if (!priv->mdio) > + return -ENOMEM; > + > + priv->mdio->priv = dev; > + priv->mdio->read = &asix_mdio_bus_read; > + priv->mdio->write = &asix_mdio_bus_write; > + priv->mdio->name = "Asix MDIO Bus"; > + /* mii bus name is usb-<usb bus number>-<usb device number> */ > + snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d", > + dev->udev->bus->busnum, dev->udev->devnum); > + > + return devm_mdiobus_register(&dev->udev->dev, priv->mdio); > +} > + > +static int ax88772_init_phy(struct usbnet *dev) > +{ > + struct asix_common_private *priv = dev->driver_priv; > + int ret; > + > + priv->phy_addr = asix_read_phy_addr(dev, true); > + if (priv->phy_addr < 0) > + return priv->phy_addr; > + > + snprintf(priv->phy_name, sizeof(priv->phy_name), PHY_ID_FMT, > + priv->mdio->id, priv->phy_addr); > + > + priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link, > + PHY_INTERFACE_MODE_INTERNAL); > + if (IS_ERR(priv->phydev)) { > + netdev_err(dev->net, "Could not connect to PHY device %s\n", > + priv->phy_name); > + ret = PTR_ERR(priv->phydev); > + return ret; > + } > + > + phy_attached_info(priv->phydev); > + > + return 0; > +} > + > static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) > { > - int ret, i; > u8 buf[ETH_ALEN] = {0}, chipcode = 0; > - u32 phyid; > struct asix_common_private *priv; > + int ret, i; > + u32 phyid; > > usbnet_get_endpoints(dev, intf); > > @@ -714,17 +739,6 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) > > asix_set_netdev_dev_addr(dev, buf); > > - /* Initialize MII structure */ > - dev->mii.dev = dev->net; > - dev->mii.mdio_read = asix_mdio_read; > - dev->mii.mdio_write = asix_mdio_write; > - dev->mii.phy_id_mask = 0x1f; > - dev->mii.reg_num_mask = 0x1f; > - > - dev->mii.phy_id = asix_read_phy_addr(dev, true); > - if (dev->mii.phy_id < 0) > - return dev->mii.phy_id; > - > dev->net->netdev_ops = &ax88772_netdev_ops; > dev->net->ethtool_ops = &ax88772_ethtool_ops; > dev->net->needed_headroom = 4; /* cf asix_tx_fixup() */ > @@ -768,11 +782,31 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) > priv->suspend = ax88772_suspend; > } > > + ret = ax88772_init_mdio(dev); > + if (ret) > + return ret; > + > + return ax88772_init_phy(dev); > +} > + > +static int ax88772_stop(struct usbnet *dev) > +{ > + struct asix_common_private *priv = dev->driver_priv; > + > + /* On unplugged USB, we will get MDIO communication errors and the > + * PHY will be set in to PHY_HALTED state. > + */ > + if (priv->phydev->state != PHY_HALTED) > + phy_stop(priv->phydev); > + > return 0; > } > > static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf) > { > + struct asix_common_private *priv = dev->driver_priv; > + > + phy_disconnect(priv->phydev); > asix_rx_fixup_common_free(dev->driver_priv); > } > > @@ -1161,8 +1195,8 @@ static const struct driver_info ax88772_info = { > .bind = ax88772_bind, > .unbind = ax88772_unbind, > .status = asix_status, > - .link_reset = ax88772_link_reset, > .reset = ax88772_reset, > + .stop = ax88772_stop, > .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET, > .rx_fixup = asix_rx_fixup_common, > .tx_fixup = asix_tx_fixup, > @@ -1173,7 +1207,6 @@ static const struct driver_info ax88772b_info = { > .bind = ax88772_bind, > .unbind = ax88772_unbind, > .status = asix_status, > - .link_reset = ax88772_link_reset, > .reset = ax88772_reset, > .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | > FLAG_MULTI_PACKET, > @@ -1209,7 +1242,6 @@ static const struct driver_info hg20f9_info = { > .bind = ax88772_bind, > .unbind = ax88772_unbind, > .status = asix_status, > - .link_reset = ax88772_link_reset, > .reset = ax88772_reset, > .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | > FLAG_MULTI_PACKET, > diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c > index c8ca5187eece..2e2081346740 100644 > --- a/drivers/net/usb/ax88172a.c > +++ b/drivers/net/usb/ax88172a.c > @@ -25,20 +25,6 @@ struct ax88172a_private { > struct asix_rx_fixup_info rx_fixup_info; > }; > > -/* MDIO read and write wrappers for phylib */ > -static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum) > -{ > - return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id, > - regnum); > -} > - > -static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, > - u16 val) > -{ > - asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val); > - return 0; > -} > - > /* set MAC link settings according to information from phylib */ > static void ax88172a_adjust_link(struct net_device *netdev) > { Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
Hi Marek, thank you for your feedback. On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote: > Hi Oleksij, > > On 07.06.2021 10:27, Oleksij Rempel wrote: > > To be able to use ax88772 with external PHYs and use advantage of > > existing PHY drivers, we need to port at least ax88772 part of asix > > driver to the phylib framework. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > I found one more issue with this patch. On one of my test boards > (Samsung Exynos5250 SoC based Arndale) system fails to establish network > connection just after starting the kernel when the driver is build-in. > > --->8--- > # dmesg | grep asix > [ 2.761928] usbcore: registered new interface driver asix > [ 5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): > invalid hw address, using random > [ 6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 7a:9b:9a:f2:94:8e > [ 14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow > control off > # ping -c2 host > PING host (192.168.100.1) 56(84) bytes of data. > From 192.168.100.20 icmp_seq=1 Destination Host Unreachable > From 192.168.100.20 icmp_seq=2 Destination Host Unreachable > > --- host ping statistics --- > 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 59ms > --->8--- Hm... it looks like different chip variant. My is registered as "ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the difference. Can you please tell me more about this adapter and if possible open tell the real part name. I can imagine that this adapter may using generic PHY driver. Can you please confirm it by dmesg | grep PHY? In my case i'll get: Asix Electronics AX88772C usb-001:003:10: attached PHY driver (mii_bus:phy_addr=usb-001:003:10, irq=POLL) If you have a different PHY, can you please send me the PHY id: cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id Your usb path will probably be different. > Calling ifup eth0 && ifdown eth0 fixes the network status: > > --->8--- > # ifdown eth0 && ifup eth0 > [ 60.474929] asix 1-3.2.4:1.0 eth0: Link is Down > [ 60.623516] asix 1-3.2.4:1.0 eth0: Link is Down > [ 62.774304] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready > [ 62.786354] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow > control off > # ping -c2 host > PING host (192.168.100.1) 56(84) bytes of data. > 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=1.25 ms > 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.853 ms > > --- host ping statistics --- > 2 packets transmitted, 2 received, 0% packet loss, time 3ms > rtt min/avg/max/mdev = 0.853/1.053/1.254/0.203 ms > --->8--- > > When driver is loaded as a module (and without any other modules, so > this is not a dependency issue), the connection is established properly > just after the boot: > > --->8--- > # dmesg | grep asix > [ 13.633284] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): > invalid hw address, using random > [ 15.390350] asix 1-3.2.4:1.0 eth0: register 'asix' at > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 3a:51:11:08:aa:ea > [ 15.414052] usbcore: registered new interface driver asix > [ 15.832564] asix 1-3.2.4:1.0 eth0: Link is Down > [ 18.053747] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow > control off > # ping -c2 host > PING host (192.168.100.1) 56(84) bytes of data. > 64 bytes from host (192.168.100.1): icmp_seq=1 ttl=64 time=0.545 ms > 64 bytes from host (192.168.100.1): icmp_seq=2 ttl=64 time=0.742 ms > > --- host ping statistics --- > 2 packets transmitted, 2 received, 0% packet loss, time 3ms > rtt min/avg/max/mdev = 0.545/0.643/0.742/0.101 ms > > --->8--- > > Let me know if I can make any other tests that would help fixing this issue. > > > --- > > drivers/net/usb/asix.h | 9 +++ > > drivers/net/usb/asix_common.c | 37 ++++++++++ > > drivers/net/usb/asix_devices.c | 120 +++++++++++++++++++++------------ > > drivers/net/usb/ax88172a.c | 14 ---- > > 4 files changed, 122 insertions(+), 58 deletions(-) > > > > diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h > > index edb94efd265e..2122d302e643 100644 > > --- a/drivers/net/usb/asix.h > > +++ b/drivers/net/usb/asix.h > > @@ -25,6 +25,7 @@ > > #include <linux/usb/usbnet.h> > > #include <linux/slab.h> > > #include <linux/if_vlan.h> > > +#include <linux/phy.h> > > > > #define DRIVER_VERSION "22-Dec-2011" > > #define DRIVER_NAME "asix" > > @@ -178,6 +179,10 @@ struct asix_common_private { > > u16 presvd_phy_advertise; > > u16 presvd_phy_bmcr; > > struct asix_rx_fixup_info rx_fixup_info; > > + struct mii_bus *mdio; > > + struct phy_device *phydev; > > + u16 phy_addr; > > + char phy_name[20]; > > }; > > > > extern const struct driver_info ax88172a_info; > > @@ -214,6 +219,7 @@ int asix_write_rx_ctl(struct usbnet *dev, u16 mode, int in_pm); > > > > u16 asix_read_medium_status(struct usbnet *dev, int in_pm); > > int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm); > > +void asix_adjust_link(struct net_device *netdev); > > > > int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm); > > > > @@ -222,6 +228,9 @@ void asix_set_multicast(struct net_device *net); > > int asix_mdio_read(struct net_device *netdev, int phy_id, int loc); > > void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val); > > > > +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum); > > +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val); > > + > > int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc); > > void asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc, > > int val); > > diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c > > index e1109f1a8dd5..085bc8281082 100644 > > --- a/drivers/net/usb/asix_common.c > > +++ b/drivers/net/usb/asix_common.c > > @@ -384,6 +384,27 @@ int asix_write_medium_mode(struct usbnet *dev, u16 mode, int in_pm) > > return ret; > > } > > > > +/* set MAC link settings according to information from phylib */ > > +void asix_adjust_link(struct net_device *netdev) > > +{ > > + struct phy_device *phydev = netdev->phydev; > > + struct usbnet *dev = netdev_priv(netdev); > > + u16 mode = 0; > > + > > + if (phydev->link) { > > + mode = AX88772_MEDIUM_DEFAULT; > > + > > + if (phydev->duplex == DUPLEX_HALF) > > + mode &= ~AX_MEDIUM_FD; > > + > > + if (phydev->speed != SPEED_100) > > + mode &= ~AX_MEDIUM_PS; > > + } > > + > > + asix_write_medium_mode(dev, mode, 0); > > + phy_print_status(phydev); > > +} > > + > > int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm) > > { > > int ret; > > @@ -506,6 +527,22 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val) > > mutex_unlock(&dev->phy_mutex); > > } > > > > +/* MDIO read and write wrappers for phylib */ > > +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum) > > +{ > > + struct usbnet *priv = bus->priv; > > + > > + return asix_mdio_read(priv->net, phy_id, regnum); > > +} > > + > > +int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val) > > +{ > > + struct usbnet *priv = bus->priv; > > + > > + asix_mdio_write(priv->net, phy_id, regnum, val); > > + return 0; > > +} > > + > > int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc) > > { > > struct usbnet *dev = netdev_priv(netdev); > > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c > > index 00b6ac0570eb..e4cd85e38edd 100644 > > --- a/drivers/net/usb/asix_devices.c > > +++ b/drivers/net/usb/asix_devices.c > > @@ -285,7 +285,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf) > > > > static const struct ethtool_ops ax88772_ethtool_ops = { > > .get_drvinfo = asix_get_drvinfo, > > - .get_link = asix_get_link, > > + .get_link = usbnet_get_link, > > .get_msglevel = usbnet_get_msglevel, > > .set_msglevel = usbnet_set_msglevel, > > .get_wol = asix_get_wol, > > @@ -293,37 +293,15 @@ static const struct ethtool_ops ax88772_ethtool_ops = { > > .get_eeprom_len = asix_get_eeprom_len, > > .get_eeprom = asix_get_eeprom, > > .set_eeprom = asix_set_eeprom, > > - .nway_reset = usbnet_nway_reset, > > - .get_link_ksettings = usbnet_get_link_ksettings_mii, > > - .set_link_ksettings = usbnet_set_link_ksettings_mii, > > + .nway_reset = phy_ethtool_nway_reset, > > + .get_link_ksettings = phy_ethtool_get_link_ksettings, > > + .set_link_ksettings = phy_ethtool_set_link_ksettings, > > }; > > > > -static int ax88772_link_reset(struct usbnet *dev) > > -{ > > - u16 mode; > > - struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET }; > > - > > - mii_check_media(&dev->mii, 1, 1); > > - mii_ethtool_gset(&dev->mii, &ecmd); > > - mode = AX88772_MEDIUM_DEFAULT; > > - > > - if (ethtool_cmd_speed(&ecmd) != SPEED_100) > > - mode &= ~AX_MEDIUM_PS; > > - > > - if (ecmd.duplex != DUPLEX_FULL) > > - mode &= ~AX_MEDIUM_FD; > > - > > - netdev_dbg(dev->net, "ax88772_link_reset() speed: %u duplex: %d setting mode to 0x%04x\n", > > - ethtool_cmd_speed(&ecmd), ecmd.duplex, mode); > > - > > - asix_write_medium_mode(dev, mode, 0); > > - > > - return 0; > > -} > > - > > static int ax88772_reset(struct usbnet *dev) > > { > > struct asix_data *data = (struct asix_data *)&dev->data; > > + struct asix_common_private *priv = dev->driver_priv; > > int ret; > > > > /* Rewrite MAC address */ > > @@ -342,6 +320,8 @@ static int ax88772_reset(struct usbnet *dev) > > if (ret < 0) > > goto out; > > > > + phy_start(priv->phydev); > > + > > return 0; > > > > out: > > @@ -586,7 +566,7 @@ static const struct net_device_ops ax88772_netdev_ops = { > > .ndo_get_stats64 = dev_get_tstats64, > > .ndo_set_mac_address = asix_set_mac_address, > > .ndo_validate_addr = eth_validate_addr, > > - .ndo_do_ioctl = asix_ioctl, > > + .ndo_do_ioctl = phy_do_ioctl_running, > > .ndo_set_rx_mode = asix_set_multicast, > > }; > > > > @@ -677,12 +657,57 @@ static int asix_resume(struct usb_interface *intf) > > return usbnet_resume(intf); > > } > > > > +static int ax88772_init_mdio(struct usbnet *dev) > > +{ > > + struct asix_common_private *priv = dev->driver_priv; > > + > > + priv->mdio = devm_mdiobus_alloc(&dev->udev->dev); > > + if (!priv->mdio) > > + return -ENOMEM; > > + > > + priv->mdio->priv = dev; > > + priv->mdio->read = &asix_mdio_bus_read; > > + priv->mdio->write = &asix_mdio_bus_write; > > + priv->mdio->name = "Asix MDIO Bus"; > > + /* mii bus name is usb-<usb bus number>-<usb device number> */ > > + snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d", > > + dev->udev->bus->busnum, dev->udev->devnum); > > + > > + return devm_mdiobus_register(&dev->udev->dev, priv->mdio); > > +} > > + > > +static int ax88772_init_phy(struct usbnet *dev) > > +{ > > + struct asix_common_private *priv = dev->driver_priv; > > + int ret; > > + > > + priv->phy_addr = asix_read_phy_addr(dev, true); > > + if (priv->phy_addr < 0) > > + return priv->phy_addr; > > + > > + snprintf(priv->phy_name, sizeof(priv->phy_name), PHY_ID_FMT, > > + priv->mdio->id, priv->phy_addr); > > + > > + priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link, > > + PHY_INTERFACE_MODE_INTERNAL); > > + if (IS_ERR(priv->phydev)) { > > + netdev_err(dev->net, "Could not connect to PHY device %s\n", > > + priv->phy_name); > > + ret = PTR_ERR(priv->phydev); > > + return ret; > > + } > > + > > + phy_attached_info(priv->phydev); > > + > > + return 0; > > +} > > + > > static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) > > { > > - int ret, i; > > u8 buf[ETH_ALEN] = {0}, chipcode = 0; > > - u32 phyid; > > struct asix_common_private *priv; > > + int ret, i; > > + u32 phyid; > > > > usbnet_get_endpoints(dev, intf); > > > > @@ -714,17 +739,6 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) > > > > asix_set_netdev_dev_addr(dev, buf); > > > > - /* Initialize MII structure */ > > - dev->mii.dev = dev->net; > > - dev->mii.mdio_read = asix_mdio_read; > > - dev->mii.mdio_write = asix_mdio_write; > > - dev->mii.phy_id_mask = 0x1f; > > - dev->mii.reg_num_mask = 0x1f; > > - > > - dev->mii.phy_id = asix_read_phy_addr(dev, true); > > - if (dev->mii.phy_id < 0) > > - return dev->mii.phy_id; > > - > > dev->net->netdev_ops = &ax88772_netdev_ops; > > dev->net->ethtool_ops = &ax88772_ethtool_ops; > > dev->net->needed_headroom = 4; /* cf asix_tx_fixup() */ > > @@ -768,11 +782,31 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) > > priv->suspend = ax88772_suspend; > > } > > > > + ret = ax88772_init_mdio(dev); > > + if (ret) > > + return ret; > > + > > + return ax88772_init_phy(dev); > > +} > > + > > +static int ax88772_stop(struct usbnet *dev) > > +{ > > + struct asix_common_private *priv = dev->driver_priv; > > + > > + /* On unplugged USB, we will get MDIO communication errors and the > > + * PHY will be set in to PHY_HALTED state. > > + */ > > + if (priv->phydev->state != PHY_HALTED) > > + phy_stop(priv->phydev); > > + > > return 0; > > } > > > > static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf) > > { > > + struct asix_common_private *priv = dev->driver_priv; > > + > > + phy_disconnect(priv->phydev); > > asix_rx_fixup_common_free(dev->driver_priv); > > } > > > > @@ -1161,8 +1195,8 @@ static const struct driver_info ax88772_info = { > > .bind = ax88772_bind, > > .unbind = ax88772_unbind, > > .status = asix_status, > > - .link_reset = ax88772_link_reset, > > .reset = ax88772_reset, > > + .stop = ax88772_stop, > > .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET, > > .rx_fixup = asix_rx_fixup_common, > > .tx_fixup = asix_tx_fixup, > > @@ -1173,7 +1207,6 @@ static const struct driver_info ax88772b_info = { > > .bind = ax88772_bind, > > .unbind = ax88772_unbind, > > .status = asix_status, > > - .link_reset = ax88772_link_reset, > > .reset = ax88772_reset, > > .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | > > FLAG_MULTI_PACKET, > > @@ -1209,7 +1242,6 @@ static const struct driver_info hg20f9_info = { > > .bind = ax88772_bind, > > .unbind = ax88772_unbind, > > .status = asix_status, > > - .link_reset = ax88772_link_reset, > > .reset = ax88772_reset, > > .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | > > FLAG_MULTI_PACKET, > > diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c > > index c8ca5187eece..2e2081346740 100644 > > --- a/drivers/net/usb/ax88172a.c > > +++ b/drivers/net/usb/ax88172a.c > > @@ -25,20 +25,6 @@ struct ax88172a_private { > > struct asix_rx_fixup_info rx_fixup_info; > > }; > > > > -/* MDIO read and write wrappers for phylib */ > > -static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum) > > -{ > > - return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id, > > - regnum); > > -} > > - > > -static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, > > - u16 val) > > -{ > > - asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val); > > - return 0; > > -} > > - > > /* set MAC link settings according to information from phylib */ > > static void ax88172a_adjust_link(struct net_device *netdev) > > { > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Oleksij, On 18.06.2021 12:13, Oleksij Rempel wrote: > thank you for your feedback. > > On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote: >> On 07.06.2021 10:27, Oleksij Rempel wrote: >>> To be able to use ax88772 with external PHYs and use advantage of >>> existing PHY drivers, we need to port at least ax88772 part of asix >>> driver to the phylib framework. >>> >>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> I found one more issue with this patch. On one of my test boards >> (Samsung Exynos5250 SoC based Arndale) system fails to establish network >> connection just after starting the kernel when the driver is build-in. >> >> --->8--- >> # dmesg | grep asix >> [ 2.761928] usbcore: registered new interface driver asix >> [ 5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): >> invalid hw address, using random >> [ 6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at >> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 7a:9b:9a:f2:94:8e >> [ 14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow >> control off >> # ping -c2 host >> PING host (192.168.100.1) 56(84) bytes of data. >> From 192.168.100.20 icmp_seq=1 Destination Host Unreachable >> From 192.168.100.20 icmp_seq=2 Destination Host Unreachable >> >> --- host ping statistics --- >> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time 59ms >> --->8--- > Hm... it looks like different chip variant. My is registered as > "ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the > difference. Can you please tell me more about this adapter and if possible open > tell the real part name. Well, currently I have only remote access to that board. The network chip is soldered on board. Maybe you can read something from the photo on the wiki page: https://en.wikipedia.org/wiki/Arndale_Board > I can imagine that this adapter may using generic PHY driver. > Can you please confirm it by dmesg | grep PHY? > In my case i'll get: > Asix Electronics AX88772C usb-001:003:10: attached PHY driver (mii_bus:phy_addr=usb-001:003:10, irq=POLL) # dmesg | grep PHY [ 5.700274] Asix Electronics AX88772A usb-001:004:10: attached PHY driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL) > If you have a different PHY, can you please send me the PHY id: > cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id > > Your usb path will probably be different. # cat /sys/bus/mdio_bus/devices/usb-001\:004\:10/phy_id 0x003b1861 > ... Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
On 18.06.2021 12:45, Marek Szyprowski wrote: > On 18.06.2021 12:13, Oleksij Rempel wrote: >> thank you for your feedback. >> >> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote: >>> On 07.06.2021 10:27, Oleksij Rempel wrote: >>>> To be able to use ax88772 with external PHYs and use advantage of >>>> existing PHY drivers, we need to port at least ax88772 part of asix >>>> driver to the phylib framework. >>>> >>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>> I found one more issue with this patch. On one of my test boards >>> (Samsung Exynos5250 SoC based Arndale) system fails to establish >>> network >>> connection just after starting the kernel when the driver is build-in. >>> >>> --->8--- >>> # dmesg | grep asix >>> [ 2.761928] usbcore: registered new interface driver asix >>> [ 5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): >>> invalid hw address, using random >>> [ 6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at >>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, >>> 7a:9b:9a:f2:94:8e >>> [ 14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow >>> control off >>> # ping -c2 host >>> PING host (192.168.100.1) 56(84) bytes of data. >>> From 192.168.100.20 icmp_seq=1 Destination Host Unreachable >>> From 192.168.100.20 icmp_seq=2 Destination Host Unreachable >>> >>> --- host ping statistics --- >>> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time >>> 59ms >>> --->8--- >> Hm... it looks like different chip variant. My is registered as >> "ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the >> difference. Can you please tell me more about this adapter and if >> possible open >> tell the real part name. > Well, currently I have only remote access to that board. The network > chip is soldered on board. Maybe you can read something from the photo > on the wiki page: https://en.wikipedia.org/wiki/Arndale_Board >> I can imagine that this adapter may using generic PHY driver. >> Can you please confirm it by dmesg | grep PHY? >> In my case i'll get: >> Asix Electronics AX88772C usb-001:003:10: attached PHY driver >> (mii_bus:phy_addr=usb-001:003:10, irq=POLL) > # dmesg | grep PHY > [ 5.700274] Asix Electronics AX88772A usb-001:004:10: attached PHY > driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL) >> If you have a different PHY, can you please send me the PHY id: >> cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id >> >> Your usb path will probably be different. > > # cat /sys/bus/mdio_bus/devices/usb-001\:004\:10/phy_id > 0x003b1861 > > > ... Just for the record, I also have a board with external USB Ethernet dongle based on ASIX chip, which works fine with this patch, both when driver is built-in or as a module. Here is the log: # dmesg | grep -i Asix [ 1.718349] usbcore: registered new interface driver asix [ 2.608596] usb 3-1: Manufacturer: ASIX Elec. Corp. [ 3.876279] libphy: Asix MDIO Bus: probed [ 3.958105] Asix Electronics AX88772C usb-003:002:10: attached PHY driver (mii_bus:phy_addr=usb-003:002:10, irq=POLL) [ 3.962728] asix 3-1:1.0 eth0: register 'asix' at usb-xhci-hcd.6.auto-1, ASIX AX88772B USB 2.0 Ethernet, 00:50:b6:18:92:f0 [ 17.488532] asix 3-1:1.0 eth0: Link is Down [ 19.557233] asix 3-1:1.0 eth0: Link is Up - 100Mbps/Full - flow control off # cat /sys/bus/mdio_bus/devices/usb-003\:002\:10/phy_id 0x003b1881 Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
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 >> >> >
On Fri, Jun 18, 2021 at 12:57:13PM +0200, Marek Szyprowski wrote: > On 18.06.2021 12:45, Marek Szyprowski wrote: > > On 18.06.2021 12:13, Oleksij Rempel wrote: > >> thank you for your feedback. > >> > >> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote: > >>> On 07.06.2021 10:27, Oleksij Rempel wrote: > >>>> To be able to use ax88772 with external PHYs and use advantage of > >>>> existing PHY drivers, we need to port at least ax88772 part of asix > >>>> driver to the phylib framework. > >>>> > >>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > >>> I found one more issue with this patch. On one of my test boards > >>> (Samsung Exynos5250 SoC based Arndale) system fails to establish > >>> network > >>> connection just after starting the kernel when the driver is build-in. > >>> > >>> --->8--- > >>> # dmesg | grep asix > >>> [ 2.761928] usbcore: registered new interface driver asix > >>> [ 5.003110] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): > >>> invalid hw address, using random > >>> [ 6.065400] asix 1-3.2.4:1.0 eth0: register 'asix' at > >>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, > >>> 7a:9b:9a:f2:94:8e > >>> [ 14.043868] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow > >>> control off > >>> # ping -c2 host > >>> PING host (192.168.100.1) 56(84) bytes of data. > >>> From 192.168.100.20 icmp_seq=1 Destination Host Unreachable > >>> From 192.168.100.20 icmp_seq=2 Destination Host Unreachable > >>> > >>> --- host ping statistics --- > >>> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time > >>> 59ms > >>> --->8--- > >> Hm... it looks like different chip variant. My is registered as > >> "ASIX AX88772B USB", yours is "ASIX AX88772 USB 2.0" - "B" is the > >> difference. Can you please tell me more about this adapter and if > >> possible open > >> tell the real part name. > > Well, currently I have only remote access to that board. The network > > chip is soldered on board. Maybe you can read something from the photo > > on the wiki page: https://en.wikipedia.org/wiki/Arndale_Board > >> I can imagine that this adapter may using generic PHY driver. > >> Can you please confirm it by dmesg | grep PHY? > >> In my case i'll get: > >> Asix Electronics AX88772C usb-001:003:10: attached PHY driver > >> (mii_bus:phy_addr=usb-001:003:10, irq=POLL) > > # dmesg | grep PHY > > [ 5.700274] Asix Electronics AX88772A usb-001:004:10: attached PHY > > driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL) > >> If you have a different PHY, can you please send me the PHY id: > >> cat /sys/bus/mdio_bus/devices/usb-001\:003\:10/phy_id > >> > >> Your usb path will probably be different. > > > > # cat /sys/bus/mdio_bus/devices/usb-001\:004\:10/phy_id > > 0x003b1861 > > > > > ... > > Just for the record, I also have a board with external USB Ethernet > dongle based on ASIX chip, which works fine with this patch, both when > driver is built-in or as a module. Here is the log: > > # dmesg | grep -i Asix > [ 1.718349] usbcore: registered new interface driver asix > [ 2.608596] usb 3-1: Manufacturer: ASIX Elec. Corp. > [ 3.876279] libphy: Asix MDIO Bus: probed > [ 3.958105] Asix Electronics AX88772C usb-003:002:10: attached PHY > driver (mii_bus:phy_addr=usb-003:002:10, irq=POLL) > [ 3.962728] asix 3-1:1.0 eth0: register 'asix' at > usb-xhci-hcd.6.auto-1, ASIX AX88772B USB 2.0 Ethernet, 00:50:b6:18:92:f0 > [ 17.488532] asix 3-1:1.0 eth0: Link is Down > [ 19.557233] asix 3-1:1.0 eth0: Link is Up - 100Mbps/Full - flow > control off > > # cat /sys/bus/mdio_bus/devices/usb-003\:002\:10/phy_id > 0x003b1881 Ok, this one is different. It is AX88772C variant. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, Jun 18, 2021 at 01:11:41PM +0200, Marek Szyprowski wrote: > Hi Heiner, > > On 18.06.2021 13:04, Heiner Kallweit wrote: > > On 18.06.2021 12:13, Oleksij Rempel wrote: > >> thank you for your feedback. > >> > >> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote: > >>> On 07.06.2021 10:27, Oleksij Rempel wrote: > >>>> To be able to use ax88772 with external PHYs and use advantage of > >>>> existing PHY drivers, we need to port at least ax88772 part of asix > >>>> driver to the phylib framework. > >>>> > >>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > >>> I found one more issue with this patch. On one of my test boards > >>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network > >>> connection just after starting the kernel when the driver is build-in. > >>> > > If you build in the MAC driver, do you also build in the PHY driver? > > If the PHY driver is still a module this could explain why genphy > > driver is used. > > And your dmesg filtering suppresses the phy_attached_info() output > > that would tell us the truth. > > Here is a bit more complete log: > > # dmesg | grep -i Asix > [ 2.412966] usbcore: registered new interface driver asix > [ 4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp. > [ 4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): > invalid hw address, using random > [ 5.657009] libphy: Asix MDIO Bus: probed > [ 5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY > driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL) > [ 5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e > [ 9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow > control off > > This seems to be something different than missing PHY driver. Can you please test it: diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c index aec97b021a73..7897108a1a42 100644 --- a/drivers/net/usb/asix_devices.c +++ b/drivers/net/usb/asix_devices.c @@ -453,6 +453,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm) u16 rx_ctl, phy14h, phy15h, phy16h; u8 chipcode = 0; + netdev_info(dev->net, "ax88772a_hw_reset\n"); ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm); if (ret < 0) goto out; @@ -509,31 +510,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm) goto out; } } else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) { - /* Check if the PHY registers have default settings */ - phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, - AX88772A_PHY14H); - phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, - AX88772A_PHY15H); - phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, - AX88772A_PHY16H); - - netdev_dbg(dev->net, - "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n", - phy14h, phy15h, phy16h); - - /* Restore PHY registers default setting if not */ - if (phy14h != AX88772A_PHY14H_DEFAULT) - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, - AX88772A_PHY14H, - AX88772A_PHY14H_DEFAULT); - if (phy15h != AX88772A_PHY15H_DEFAULT) - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, - AX88772A_PHY15H, - AX88772A_PHY15H_DEFAULT); - if (phy16h != AX88772A_PHY16H_DEFAULT) - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, - AX88772A_PHY16H, - AX88772A_PHY16H_DEFAULT); + netdev_info(dev->net, "do not touch PHY regs\n"); } ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0, -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Oleksij, On 18.06.2021 15:20, Oleksij Rempel wrote: > On Fri, Jun 18, 2021 at 01:11:41PM +0200, Marek Szyprowski wrote: >> On 18.06.2021 13:04, Heiner Kallweit wrote: >>> On 18.06.2021 12:13, Oleksij Rempel wrote: >>>> thank you for your feedback. >>>> >>>> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote: >>>>> On 07.06.2021 10:27, Oleksij Rempel wrote: >>>>>> To be able to use ax88772 with external PHYs and use advantage of >>>>>> existing PHY drivers, we need to port at least ax88772 part of asix >>>>>> driver to the phylib framework. >>>>>> >>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>> I found one more issue with this patch. On one of my test boards >>>>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network >>>>> connection just after starting the kernel when the driver is build-in. >>>>> >>> If you build in the MAC driver, do you also build in the PHY driver? >>> If the PHY driver is still a module this could explain why genphy >>> driver is used. >>> And your dmesg filtering suppresses the phy_attached_info() output >>> that would tell us the truth. >> Here is a bit more complete log: >> >> # dmesg | grep -i Asix >> [ 2.412966] usbcore: registered new interface driver asix >> [ 4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp. >> [ 4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): >> invalid hw address, using random >> [ 5.657009] libphy: Asix MDIO Bus: probed >> [ 5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY >> driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL) >> [ 5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at >> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e >> [ 9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow >> control off >> >> This seems to be something different than missing PHY driver. > Can you please test it: > > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c > index aec97b021a73..7897108a1a42 100644 > --- a/drivers/net/usb/asix_devices.c > +++ b/drivers/net/usb/asix_devices.c > @@ -453,6 +453,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm) > u16 rx_ctl, phy14h, phy15h, phy16h; > u8 chipcode = 0; > > + netdev_info(dev->net, "ax88772a_hw_reset\n"); > ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm); > if (ret < 0) > goto out; > @@ -509,31 +510,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm) > goto out; > } > } else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) { > - /* Check if the PHY registers have default settings */ > - phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, > - AX88772A_PHY14H); > - phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, > - AX88772A_PHY15H); > - phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, > - AX88772A_PHY16H); > - > - netdev_dbg(dev->net, > - "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n", > - phy14h, phy15h, phy16h); > - > - /* Restore PHY registers default setting if not */ > - if (phy14h != AX88772A_PHY14H_DEFAULT) > - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, > - AX88772A_PHY14H, > - AX88772A_PHY14H_DEFAULT); > - if (phy15h != AX88772A_PHY15H_DEFAULT) > - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, > - AX88772A_PHY15H, > - AX88772A_PHY15H_DEFAULT); > - if (phy16h != AX88772A_PHY16H_DEFAULT) > - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, > - AX88772A_PHY16H, > - AX88772A_PHY16H_DEFAULT); > + netdev_info(dev->net, "do not touch PHY regs\n"); > } > > ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0, This doesn't help for this issue. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
Hi Marek, On Mon, Jun 21, 2021 at 08:05:49AM +0200, Marek Szyprowski wrote: > Hi Oleksij, > > On 18.06.2021 15:20, Oleksij Rempel wrote: > > On Fri, Jun 18, 2021 at 01:11:41PM +0200, Marek Szyprowski wrote: > >> On 18.06.2021 13:04, Heiner Kallweit wrote: > >>> On 18.06.2021 12:13, Oleksij Rempel wrote: > >>>> thank you for your feedback. > >>>> > >>>> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote: > >>>>> On 07.06.2021 10:27, Oleksij Rempel wrote: > >>>>>> To be able to use ax88772 with external PHYs and use advantage of > >>>>>> existing PHY drivers, we need to port at least ax88772 part of asix > >>>>>> driver to the phylib framework. > >>>>>> > >>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > >>>>> I found one more issue with this patch. On one of my test boards > >>>>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network > >>>>> connection just after starting the kernel when the driver is build-in. > >>>>> > >>> If you build in the MAC driver, do you also build in the PHY driver? > >>> If the PHY driver is still a module this could explain why genphy > >>> driver is used. > >>> And your dmesg filtering suppresses the phy_attached_info() output > >>> that would tell us the truth. > >> Here is a bit more complete log: > >> > >> # dmesg | grep -i Asix > >> [ 2.412966] usbcore: registered new interface driver asix > >> [ 4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp. > >> [ 4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): > >> invalid hw address, using random > >> [ 5.657009] libphy: Asix MDIO Bus: probed > >> [ 5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY > >> driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL) > >> [ 5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at > >> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e > >> [ 9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow > >> control off > >> > >> This seems to be something different than missing PHY driver. > > Can you please test it: > > > > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c > > index aec97b021a73..7897108a1a42 100644 > > --- a/drivers/net/usb/asix_devices.c > > +++ b/drivers/net/usb/asix_devices.c > > @@ -453,6 +453,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm) > > u16 rx_ctl, phy14h, phy15h, phy16h; > > u8 chipcode = 0; > > > > + netdev_info(dev->net, "ax88772a_hw_reset\n"); > > ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm); > > if (ret < 0) > > goto out; > > @@ -509,31 +510,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm) > > goto out; > > } > > } else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) { > > - /* Check if the PHY registers have default settings */ > > - phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, > > - AX88772A_PHY14H); > > - phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, > > - AX88772A_PHY15H); > > - phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, > > - AX88772A_PHY16H); > > - > > - netdev_dbg(dev->net, > > - "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n", > > - phy14h, phy15h, phy16h); > > - > > - /* Restore PHY registers default setting if not */ > > - if (phy14h != AX88772A_PHY14H_DEFAULT) > > - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, > > - AX88772A_PHY14H, > > - AX88772A_PHY14H_DEFAULT); > > - if (phy15h != AX88772A_PHY15H_DEFAULT) > > - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, > > - AX88772A_PHY15H, > > - AX88772A_PHY15H_DEFAULT); > > - if (phy16h != AX88772A_PHY16H_DEFAULT) > > - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, > > - AX88772A_PHY16H, > > - AX88772A_PHY16H_DEFAULT); > > + netdev_info(dev->net, "do not touch PHY regs\n"); > > } > > > > ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0, > > This doesn't help for this issue. Ok. So far I was not able to see obvious differences between: probe -> ip link set dev eth1 up and probe -> ip link set dev eth1 up; ip link set dev eth1 down; ip link set dev eth1 up Except of PHY sate. By default the PHY is in resumed state after probe and is able to negotiate the link even if the MAC is down. After ip link set dev eth1 down, the PHY is in suspend state, as expected. Can you please test this change? diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c index aec97b021a73..2c115216420a 100644 --- a/drivers/net/usb/asix_devices.c +++ b/drivers/net/usb/asix_devices.c @@ -701,6 +701,7 @@ static int ax88772_init_phy(struct usbnet *dev) return ret; } + phy_suspend(priv->phydev); priv->phydev->mac_managed_pm = 1; phy_attached_info(priv->phydev); Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Oleksij, On 23.06.2021 09:06, Oleksij Rempel wrote: > On Mon, Jun 21, 2021 at 08:05:49AM +0200, Marek Szyprowski wrote: >> On 18.06.2021 15:20, Oleksij Rempel wrote: >>> On Fri, Jun 18, 2021 at 01:11:41PM +0200, Marek Szyprowski wrote: >>>> On 18.06.2021 13:04, Heiner Kallweit wrote: >>>>> On 18.06.2021 12:13, Oleksij Rempel wrote: >>>>>> thank you for your feedback. >>>>>> >>>>>> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote: >>>>>>> On 07.06.2021 10:27, Oleksij Rempel wrote: >>>>>>>> To be able to use ax88772 with external PHYs and use advantage of >>>>>>>> existing PHY drivers, we need to port at least ax88772 part of asix >>>>>>>> driver to the phylib framework. >>>>>>>> >>>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>>>> I found one more issue with this patch. On one of my test boards >>>>>>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network >>>>>>> connection just after starting the kernel when the driver is build-in. >>>>>>> >>>>> If you build in the MAC driver, do you also build in the PHY driver? >>>>> If the PHY driver is still a module this could explain why genphy >>>>> driver is used. >>>>> And your dmesg filtering suppresses the phy_attached_info() output >>>>> that would tell us the truth. >>>> Here is a bit more complete log: >>>> >>>> # dmesg | grep -i Asix >>>> [ 2.412966] usbcore: registered new interface driver asix >>>> [ 4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp. >>>> [ 4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized): >>>> invalid hw address, using random >>>> [ 5.657009] libphy: Asix MDIO Bus: probed >>>> [ 5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY >>>> driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL) >>>> [ 5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at >>>> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e >>>> [ 9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow >>>> control off >>>> >>>> This seems to be something different than missing PHY driver. >>> Can you please test it: >>> >>> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c >>> index aec97b021a73..7897108a1a42 100644 >>> --- a/drivers/net/usb/asix_devices.c >>> +++ b/drivers/net/usb/asix_devices.c >>> @@ -453,6 +453,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm) >>> u16 rx_ctl, phy14h, phy15h, phy16h; >>> u8 chipcode = 0; >>> >>> + netdev_info(dev->net, "ax88772a_hw_reset\n"); >>> ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm); >>> if (ret < 0) >>> goto out; >>> @@ -509,31 +510,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm) >>> goto out; >>> } >>> } else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) { >>> - /* Check if the PHY registers have default settings */ >>> - phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, >>> - AX88772A_PHY14H); >>> - phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, >>> - AX88772A_PHY15H); >>> - phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id, >>> - AX88772A_PHY16H); >>> - >>> - netdev_dbg(dev->net, >>> - "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n", >>> - phy14h, phy15h, phy16h); >>> - >>> - /* Restore PHY registers default setting if not */ >>> - if (phy14h != AX88772A_PHY14H_DEFAULT) >>> - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, >>> - AX88772A_PHY14H, >>> - AX88772A_PHY14H_DEFAULT); >>> - if (phy15h != AX88772A_PHY15H_DEFAULT) >>> - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, >>> - AX88772A_PHY15H, >>> - AX88772A_PHY15H_DEFAULT); >>> - if (phy16h != AX88772A_PHY16H_DEFAULT) >>> - asix_mdio_write_nopm(dev->net, dev->mii.phy_id, >>> - AX88772A_PHY16H, >>> - AX88772A_PHY16H_DEFAULT); >>> + netdev_info(dev->net, "do not touch PHY regs\n"); >>> } >>> >>> ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0, >> This doesn't help for this issue. > Ok. > So far I was not able to see obvious differences between: > probe -> ip link set dev eth1 up > > and > > probe -> ip link set dev eth1 up; > ip link set dev eth1 down; > ip link set dev eth1 up > > > Except of PHY sate. By default the PHY is in resumed state after probe > and is able to negotiate the link even if the MAC is down. > After ip link set dev eth1 down, the PHY is in suspend state, as > expected. > > Can you please test this change? > > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c > index aec97b021a73..2c115216420a 100644 > --- a/drivers/net/usb/asix_devices.c > +++ b/drivers/net/usb/asix_devices.c > @@ -701,6 +701,7 @@ static int ax88772_init_phy(struct usbnet *dev) > return ret; > } > > + phy_suspend(priv->phydev); > priv->phydev->mac_managed_pm = 1; > > phy_attached_info(priv->phydev); > I'm sorry for the late reply, I've just got back from vacations. The above change fixes the issue. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
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) {
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(-)