* [PATCH REPOST] usbnet: asix: leave the carrier control to phylink
@ 2025-04-07 12:08 Krzysztof Hałasa
2025-04-07 13:38 ` Oleksij Rempel
2025-04-08 13:29 ` [PATCH REPOST] usbnet: asix: " Paolo Abeni
0 siblings, 2 replies; 9+ messages in thread
From: Krzysztof Hałasa @ 2025-04-07 12:08 UTC (permalink / raw)
To: netdev
Cc: Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-usb, linux-kernel,
Jose Ignacio Tornos Martinez, Ming Lei, Oleksij Rempel
[added Oleksij - the author of the phylink code for this driver]
ASIX AX88772B based USB 10/100 Ethernet adapter doesn't come
up ("carrier off"), despite the built-in 100BASE-FX PHY positive link
indication. The internal PHY is configured (using EEPROM) in fixed
100 Mbps full duplex mode.
The primary problem appears to be using carrier_netif_{on,off}() while,
at the same time, delegating carrier management to phylink. Use only the
latter and remove "manual control" in the asix driver.
I don't have any other AX88772 board here, but the problem doesn't seem
specific to a particular board or settings - it's probably
timing-dependent.
Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index 74162190bccc..8531b804021a 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -224,7 +224,6 @@ 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);
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 72ffc89b477a..7fd763917ae2 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -414,28 +414,6 @@ 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);
- usbnet_link_change(dev, phydev->link, 0);
-}
-
int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
{
int ret;
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 57d6e5abc30e..af91fc947f40 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -40,22 +40,6 @@ struct ax88172_int_data {
__le16 res3;
} __packed;
-static void asix_status(struct usbnet *dev, struct urb *urb)
-{
- struct ax88172_int_data *event;
- int link;
-
- if (urb->actual_length < 8)
- return;
-
- event = urb->transfer_buffer;
- link = event->link & 0x01;
- if (netif_carrier_ok(dev->net) != link) {
- usbnet_link_change(dev, link, 1);
- netdev_dbg(dev->net, "Link Status is: %d\n", link);
- }
-}
-
static void asix_set_netdev_dev_addr(struct usbnet *dev, u8 *addr)
{
if (is_valid_ether_addr(addr)) {
@@ -752,7 +736,6 @@ static void ax88772_mac_link_down(struct phylink_config *config,
struct usbnet *dev = netdev_priv(to_net_dev(config->dev));
asix_write_medium_mode(dev, 0, 0);
- usbnet_link_change(dev, false, false);
}
static void ax88772_mac_link_up(struct phylink_config *config,
@@ -783,7 +766,6 @@ static void ax88772_mac_link_up(struct phylink_config *config,
m |= AX_MEDIUM_RFC;
asix_write_medium_mode(dev, m, 0);
- usbnet_link_change(dev, true, false);
}
static const struct phylink_mac_ops ax88772_phylink_mac_ops = {
@@ -1309,40 +1291,36 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
static const struct driver_info ax8817x_info = {
.description = "ASIX AX8817x USB 2.0 Ethernet",
.bind = ax88172_bind,
- .status = asix_status,
.link_reset = ax88172_link_reset,
.reset = ax88172_link_reset,
- .flags = FLAG_ETHER | FLAG_LINK_INTR,
+ .flags = FLAG_ETHER,
.data = 0x00130103,
};
static const struct driver_info dlink_dub_e100_info = {
.description = "DLink DUB-E100 USB Ethernet",
.bind = ax88172_bind,
- .status = asix_status,
.link_reset = ax88172_link_reset,
.reset = ax88172_link_reset,
- .flags = FLAG_ETHER | FLAG_LINK_INTR,
+ .flags = FLAG_ETHER,
.data = 0x009f9d9f,
};
static const struct driver_info netgear_fa120_info = {
.description = "Netgear FA-120 USB Ethernet",
.bind = ax88172_bind,
- .status = asix_status,
.link_reset = ax88172_link_reset,
.reset = ax88172_link_reset,
- .flags = FLAG_ETHER | FLAG_LINK_INTR,
+ .flags = FLAG_ETHER,
.data = 0x00130103,
};
static const struct driver_info hawking_uf200_info = {
.description = "Hawking UF200 USB Ethernet",
.bind = ax88172_bind,
- .status = asix_status,
.link_reset = ax88172_link_reset,
.reset = ax88172_link_reset,
- .flags = FLAG_ETHER | FLAG_LINK_INTR,
+ .flags = FLAG_ETHER,
.data = 0x001f1d1f,
};
@@ -1350,10 +1328,9 @@ static const struct driver_info ax88772_info = {
.description = "ASIX AX88772 USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
- .status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
};
@@ -1362,11 +1339,9 @@ static const struct driver_info ax88772b_info = {
.description = "ASIX AX88772B USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
- .status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
- FLAG_MULTI_PACKET,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
.data = FLAG_EEPROM_MAC,
@@ -1376,11 +1351,9 @@ static const struct driver_info lxausb_t1l_info = {
.description = "Linux Automation GmbH USB 10Base-T1L",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
- .status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
- FLAG_MULTI_PACKET,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
.data = FLAG_EEPROM_MAC,
@@ -1390,11 +1363,9 @@ static const struct driver_info ax88178_info = {
.description = "ASIX AX88178 USB 2.0 Ethernet",
.bind = ax88178_bind,
.unbind = ax88178_unbind,
- .status = asix_status,
.link_reset = ax88178_link_reset,
.reset = ax88178_reset,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
- FLAG_MULTI_PACKET,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
};
@@ -1412,10 +1383,8 @@ static const struct driver_info hg20f9_info = {
.description = "HG20F9 USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
- .status = asix_status,
.reset = ax88772_reset,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
- FLAG_MULTI_PACKET,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
.data = FLAG_EEPROM_MAC,
--
Krzysztof "Chris" Hałasa
Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH REPOST] usbnet: asix: leave the carrier control to phylink
2025-04-07 12:08 [PATCH REPOST] usbnet: asix: leave the carrier control to phylink Krzysztof Hałasa
@ 2025-04-07 13:38 ` Oleksij Rempel
2025-04-08 11:55 ` Krzysztof Hałasa
2025-04-08 13:29 ` [PATCH REPOST] usbnet: asix: " Paolo Abeni
1 sibling, 1 reply; 9+ messages in thread
From: Oleksij Rempel @ 2025-04-07 13:38 UTC (permalink / raw)
To: Krzysztof Hałasa
Cc: netdev, Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-usb, linux-kernel,
Jose Ignacio Tornos Martinez, Ming Lei
Hi Krzysztof,
On Mon, Apr 07, 2025 at 02:08:22PM +0200, Krzysztof Hałasa wrote:
> [added Oleksij - the author of the phylink code for this driver]
>
> ASIX AX88772B based USB 10/100 Ethernet adapter doesn't come
> up ("carrier off"), despite the built-in 100BASE-FX PHY positive link
> indication. The internal PHY is configured (using EEPROM) in fixed
> 100 Mbps full duplex mode.
>
> The primary problem appears to be using carrier_netif_{on,off}() while,
> at the same time, delegating carrier management to phylink. Use only the
> latter and remove "manual control" in the asix driver.
Good point, this artifact should be partially removed, but not for all
devices. Only ax88772 are converted to PHYlink. ax88178 are not
converted.
> I don't have any other AX88772 board here, but the problem doesn't seem
> specific to a particular board or settings - it's probably
> timing-dependent.
The AX88772 portion of the driver, is not forwarding the interrupt to
the PHY driver. It means, PHY is in polling mode. As long as PHY
provides proper information, it will work.
On other hand, you seems to use AX88772B in 100BASE-FX mode. I'm sure,
current PHY driver for this device do not know anything about FX mode:
drivers/net/phy/ax88796b.c
Which 100BASE-FX PHY capable device do you use? Is it possible to buy
it some where?
> Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
>
> diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
> index 74162190bccc..8531b804021a 100644
> --- a/drivers/net/usb/asix.h
> +++ b/drivers/net/usb/asix.h
> @@ -224,7 +224,6 @@ 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);
>
> diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
> index 72ffc89b477a..7fd763917ae2 100644
> --- a/drivers/net/usb/asix_common.c
> +++ b/drivers/net/usb/asix_common.c
> @@ -414,28 +414,6 @@ 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);
> - usbnet_link_change(dev, phydev->link, 0);
> -}
> -
> int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
> {
> int ret;
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index 57d6e5abc30e..af91fc947f40 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -40,22 +40,6 @@ struct ax88172_int_data {
> __le16 res3;
> } __packed;
>
> -static void asix_status(struct usbnet *dev, struct urb *urb)
> -{
> - struct ax88172_int_data *event;
> - int link;
> -
> - if (urb->actual_length < 8)
> - return;
> -
> - event = urb->transfer_buffer;
> - link = event->link & 0x01;
> - if (netif_carrier_ok(dev->net) != link) {
> - usbnet_link_change(dev, link, 1);
> - netdev_dbg(dev->net, "Link Status is: %d\n", link);
> - }
> -}
> -
> static void asix_set_netdev_dev_addr(struct usbnet *dev, u8 *addr)
> {
> if (is_valid_ether_addr(addr)) {
> @@ -752,7 +736,6 @@ static void ax88772_mac_link_down(struct phylink_config *config,
> struct usbnet *dev = netdev_priv(to_net_dev(config->dev));
>
> asix_write_medium_mode(dev, 0, 0);
> - usbnet_link_change(dev, false, false);
> }
>
> static void ax88772_mac_link_up(struct phylink_config *config,
> @@ -783,7 +766,6 @@ static void ax88772_mac_link_up(struct phylink_config *config,
> m |= AX_MEDIUM_RFC;
>
> asix_write_medium_mode(dev, m, 0);
> - usbnet_link_change(dev, true, false);
> }
>
> static const struct phylink_mac_ops ax88772_phylink_mac_ops = {
> @@ -1309,40 +1291,36 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
> static const struct driver_info ax8817x_info = {
> .description = "ASIX AX8817x USB 2.0 Ethernet",
> .bind = ax88172_bind,
> - .status = asix_status,
> .link_reset = ax88172_link_reset,
> .reset = ax88172_link_reset,
> - .flags = FLAG_ETHER | FLAG_LINK_INTR,
> + .flags = FLAG_ETHER,
> .data = 0x00130103,
> };
>
> static const struct driver_info dlink_dub_e100_info = {
> .description = "DLink DUB-E100 USB Ethernet",
> .bind = ax88172_bind,
> - .status = asix_status,
> .link_reset = ax88172_link_reset,
> .reset = ax88172_link_reset,
> - .flags = FLAG_ETHER | FLAG_LINK_INTR,
> + .flags = FLAG_ETHER,
> .data = 0x009f9d9f,
> };
>
> static const struct driver_info netgear_fa120_info = {
> .description = "Netgear FA-120 USB Ethernet",
> .bind = ax88172_bind,
> - .status = asix_status,
> .link_reset = ax88172_link_reset,
> .reset = ax88172_link_reset,
> - .flags = FLAG_ETHER | FLAG_LINK_INTR,
> + .flags = FLAG_ETHER,
> .data = 0x00130103,
> };
>
> static const struct driver_info hawking_uf200_info = {
> .description = "Hawking UF200 USB Ethernet",
> .bind = ax88172_bind,
> - .status = asix_status,
> .link_reset = ax88172_link_reset,
> .reset = ax88172_link_reset,
> - .flags = FLAG_ETHER | FLAG_LINK_INTR,
> + .flags = FLAG_ETHER,
> .data = 0x001f1d1f,
> };
>
> @@ -1350,10 +1328,9 @@ static const struct driver_info ax88772_info = {
> .description = "ASIX AX88772 USB 2.0 Ethernet",
> .bind = ax88772_bind,
> .unbind = ax88772_unbind,
> - .status = asix_status,
> .reset = ax88772_reset,
> .stop = ax88772_stop,
> - .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
> + .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
> .rx_fixup = asix_rx_fixup_common,
> .tx_fixup = asix_tx_fixup,
> };
> @@ -1362,11 +1339,9 @@ static const struct driver_info ax88772b_info = {
> .description = "ASIX AX88772B USB 2.0 Ethernet",
> .bind = ax88772_bind,
> .unbind = ax88772_unbind,
> - .status = asix_status,
> .reset = ax88772_reset,
> .stop = ax88772_stop,
> - .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
> - FLAG_MULTI_PACKET,
> + .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
> .rx_fixup = asix_rx_fixup_common,
> .tx_fixup = asix_tx_fixup,
> .data = FLAG_EEPROM_MAC,
> @@ -1376,11 +1351,9 @@ static const struct driver_info lxausb_t1l_info = {
> .description = "Linux Automation GmbH USB 10Base-T1L",
> .bind = ax88772_bind,
> .unbind = ax88772_unbind,
> - .status = asix_status,
> .reset = ax88772_reset,
> .stop = ax88772_stop,
> - .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
> - FLAG_MULTI_PACKET,
> + .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
> .rx_fixup = asix_rx_fixup_common,
> .tx_fixup = asix_tx_fixup,
> .data = FLAG_EEPROM_MAC,
> @@ -1390,11 +1363,9 @@ static const struct driver_info ax88178_info = {
> .description = "ASIX AX88178 USB 2.0 Ethernet",
> .bind = ax88178_bind,
> .unbind = ax88178_unbind,
> - .status = asix_status,
> .link_reset = ax88178_link_reset,
> .reset = ax88178_reset,
> - .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
> - FLAG_MULTI_PACKET,
> + .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
> .rx_fixup = asix_rx_fixup_common,
> .tx_fixup = asix_tx_fixup,
> };
> @@ -1412,10 +1383,8 @@ static const struct driver_info hg20f9_info = {
> .description = "HG20F9 USB 2.0 Ethernet",
> .bind = ax88772_bind,
> .unbind = ax88772_unbind,
> - .status = asix_status,
> .reset = ax88772_reset,
> - .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
> - FLAG_MULTI_PACKET,
> + .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
> .rx_fixup = asix_rx_fixup_common,
> .tx_fixup = asix_tx_fixup,
> .data = FLAG_EEPROM_MAC,
>
> --
> Krzysztof "Chris" Hałasa
>
> Sieć Badawcza Łukasiewicz
> Przemysłowy Instytut Automatyki i Pomiarów PIAP
> Al. Jerozolimskie 202, 02-486 Warszawa
>
>
--
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 |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH REPOST] usbnet: asix: leave the carrier control to phylink
2025-04-07 13:38 ` Oleksij Rempel
@ 2025-04-08 11:55 ` Krzysztof Hałasa
2025-04-08 11:59 ` [PATCH v2] usbnet: asix AX88772: " Krzysztof Hałasa
0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Hałasa @ 2025-04-08 11:55 UTC (permalink / raw)
To: Oleksij Rempel
Cc: netdev, Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-usb, linux-kernel,
Jose Ignacio Tornos Martinez, Ming Lei
Oleksij,
thanks for your fast response.
Oleksij Rempel <o.rempel@pengutronix.de> writes:
> Good point, this artifact should be partially removed, but not for all
> devices. Only ax88772 are converted to PHYlink. ax88178 are not
> converted.
There is also AX88172. I assume the situation with 172 and 178 is
similar.
> The AX88772 portion of the driver, is not forwarding the interrupt to
> the PHY driver. It means, PHY is in polling mode. As long as PHY
> provides proper information, it will work.
It does, yes.
> On other hand, you seems to use AX88772B in 100BASE-FX mode. I'm sure,
> current PHY driver for this device do not know anything about FX mode:
> drivers/net/phy/ax88796b.c
>
> Which 100BASE-FX PHY capable device do you use? Is it possible to buy
> it some where?
No, it's a part of custom hw, but the carrier problem seems to be
independent of the actual PHY type. The PHY code needs a bit of fixing
as well, though (one can't really enable autoneg with 100BASE-FX).
Will attach a version with 8817x parts removed.
--
Krzysztof "Chris" Hałasa
Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] usbnet: asix AX88772: leave the carrier control to phylink
2025-04-08 11:55 ` Krzysztof Hałasa
@ 2025-04-08 11:59 ` Krzysztof Hałasa
2025-04-08 15:22 ` Jakub Kicinski
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Krzysztof Hałasa @ 2025-04-08 11:59 UTC (permalink / raw)
To: Oleksij Rempel
Cc: netdev, Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-usb, linux-kernel,
Jose Ignacio Tornos Martinez, Ming Lei
ASIX AX88772B based USB 10/100 Ethernet adapter doesn't come
up ("carrier off"), despite the built-in 100BASE-FX PHY positive link
indication. The internal PHY is configured (using EEPROM) in fixed
100 Mbps full duplex mode.
The primary problem appears to be using carrier_netif_{on,off}() while,
at the same time, delegating carrier management to phylink. Use only the
latter and remove "manual control" in the asix driver.
I don't have any other AX88772 board here, but the problem doesn't seem
specific to a particular board or settings - it's probably
timing-dependent.
Remove unused asix_adjust_link() as well.
Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -224,7 +224,6 @@ 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);
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -414,28 +414,6 @@ 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);
- usbnet_link_change(dev, phydev->link, 0);
-}
-
int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
{
int ret;
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -752,7 +736,6 @@ static void ax88772_mac_link_down(struct phylink_config *config,
struct usbnet *dev = netdev_priv(to_net_dev(config->dev));
asix_write_medium_mode(dev, 0, 0);
- usbnet_link_change(dev, false, false);
}
static void ax88772_mac_link_up(struct phylink_config *config,
@@ -783,7 +766,6 @@ static void ax88772_mac_link_up(struct phylink_config *config,
m |= AX_MEDIUM_RFC;
asix_write_medium_mode(dev, m, 0);
- usbnet_link_change(dev, true, false);
}
static const struct phylink_mac_ops ax88772_phylink_mac_ops = {
@@ -1350,10 +1328,9 @@ static const struct driver_info ax88772_info = {
.description = "ASIX AX88772 USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
- .status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
};
@@ -1362,11 +1339,9 @@ static const struct driver_info ax88772b_info = {
.description = "ASIX AX88772B USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
- .status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
- FLAG_MULTI_PACKET,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
.data = FLAG_EEPROM_MAC,
@@ -1376,11 +1351,9 @@ static const struct driver_info lxausb_t1l_info = {
.description = "Linux Automation GmbH USB 10Base-T1L",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
- .status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
- FLAG_MULTI_PACKET,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
.data = FLAG_EEPROM_MAC,
@@ -1412,10 +1383,8 @@ static const struct driver_info hg20f9_info = {
.description = "HG20F9 USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
- .status = asix_status,
.reset = ax88772_reset,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
- FLAG_MULTI_PACKET,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
.data = FLAG_EEPROM_MAC,
--
Krzysztof "Chris" Hałasa
Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH REPOST] usbnet: asix: leave the carrier control to phylink
2025-04-07 12:08 [PATCH REPOST] usbnet: asix: leave the carrier control to phylink Krzysztof Hałasa
2025-04-07 13:38 ` Oleksij Rempel
@ 2025-04-08 13:29 ` Paolo Abeni
2025-04-09 9:07 ` Krzysztof Hałasa
1 sibling, 1 reply; 9+ messages in thread
From: Paolo Abeni @ 2025-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Hałasa, netdev
Cc: Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-usb, linux-kernel,
Jose Ignacio Tornos Martinez, Ming Lei, Oleksij Rempel
On 4/7/25 2:08 PM, Krzysztof Hałasa wrote:
> [added Oleksij - the author of the phylink code for this driver]
>
> ASIX AX88772B based USB 10/100 Ethernet adapter doesn't come
> up ("carrier off"), despite the built-in 100BASE-FX PHY positive link
> indication. The internal PHY is configured (using EEPROM) in fixed
> 100 Mbps full duplex mode.
>
> The primary problem appears to be using carrier_netif_{on,off}() while,
> at the same time, delegating carrier management to phylink. Use only the
> latter and remove "manual control" in the asix driver.
>
> I don't have any other AX88772 board here, but the problem doesn't seem
> specific to a particular board or settings - it's probably
> timing-dependent.
>
> Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
Does not build:
../drivers/net/usb/asix_devices.c:1396:19: error: ‘asix_status’
undeclared here (not in a function); did you mean ‘si_status’?
1396 | .status = asix_status,
| ^~~~~~~~~~~
| si_status
/P
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] usbnet: asix AX88772: leave the carrier control to phylink
2025-04-08 11:59 ` [PATCH v2] usbnet: asix AX88772: " Krzysztof Hałasa
@ 2025-04-08 15:22 ` Jakub Kicinski
2025-04-10 12:14 ` Oleksij Rempel
2025-04-11 2:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2025-04-08 15:22 UTC (permalink / raw)
To: Krzysztof Hałasa
Cc: Oleksij Rempel, netdev, Oliver Neukum, Andrew Lunn,
David S. Miller, Eric Dumazet, Paolo Abeni, linux-usb,
linux-kernel, Jose Ignacio Tornos Martinez, Ming Lei
On Tue, 08 Apr 2025 13:59:41 +0200 Krzysztof Hałasa wrote:
> ASIX AX88772B based USB 10/100 Ethernet adapter doesn't come
> up ("carrier off"), despite the built-in 100BASE-FX PHY positive link
> indication. The internal PHY is configured (using EEPROM) in fixed
> 100 Mbps full duplex mode.
>
> The primary problem appears to be using carrier_netif_{on,off}() while,
> at the same time, delegating carrier management to phylink. Use only the
> latter and remove "manual control" in the asix driver.
>
> I don't have any other AX88772 board here, but the problem doesn't seem
> specific to a particular board or settings - it's probably
> timing-dependent.
>
> Remove unused asix_adjust_link() as well.
>
> Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
In the future, if you don't mind, please add a lore link here, like
this:
---
v1: https://lore.kernel.org/all/m35xjgdvih.fsf@t19.piap.pl/
Sending in-reply-to is discouraged, it messes up patch ordering for
reviewers:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#resending-after-review
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH REPOST] usbnet: asix: leave the carrier control to phylink
2025-04-08 13:29 ` [PATCH REPOST] usbnet: asix: " Paolo Abeni
@ 2025-04-09 9:07 ` Krzysztof Hałasa
0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Hałasa @ 2025-04-09 9:07 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-usb, linux-kernel,
Jose Ignacio Tornos Martinez, Ming Lei, Oleksij Rempel
Paolo Abeni <pabeni@redhat.com> writes:
> Does not build:
>
> ../drivers/net/usb/asix_devices.c:1396:19: error: ‘asix_status’
> undeclared here (not in a function); did you mean ‘si_status’?
> 1396 | .status = asix_status,
Right, thanks - somehow I didn't realize the recent addition in net-next
will break it. Should have been obvious.
Not a factor in v2, though.
--
Krzysztof "Chris" Hałasa
Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] usbnet: asix AX88772: leave the carrier control to phylink
2025-04-08 11:59 ` [PATCH v2] usbnet: asix AX88772: " Krzysztof Hałasa
2025-04-08 15:22 ` Jakub Kicinski
@ 2025-04-10 12:14 ` Oleksij Rempel
2025-04-11 2:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 9+ messages in thread
From: Oleksij Rempel @ 2025-04-10 12:14 UTC (permalink / raw)
To: Krzysztof Hałasa
Cc: netdev, Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-usb, linux-kernel,
Jose Ignacio Tornos Martinez, Ming Lei
On Tue, Apr 08, 2025 at 01:59:41PM +0200, Krzysztof Hałasa wrote:
> ASIX AX88772B based USB 10/100 Ethernet adapter doesn't come
> up ("carrier off"), despite the built-in 100BASE-FX PHY positive link
> indication. The internal PHY is configured (using EEPROM) in fixed
> 100 Mbps full duplex mode.
>
> The primary problem appears to be using carrier_netif_{on,off}() while,
> at the same time, delegating carrier management to phylink. Use only the
> latter and remove "manual control" in the asix driver.
>
> I don't have any other AX88772 board here, but the problem doesn't seem
> specific to a particular board or settings - it's probably
> timing-dependent.
>
> Remove unused asix_adjust_link() as well.
>
> Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Link detection is still working on variants with copper interface.
Thank you!
Best 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 |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] usbnet: asix AX88772: leave the carrier control to phylink
2025-04-08 11:59 ` [PATCH v2] usbnet: asix AX88772: " Krzysztof Hałasa
2025-04-08 15:22 ` Jakub Kicinski
2025-04-10 12:14 ` Oleksij Rempel
@ 2025-04-11 2:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-04-11 2:20 UTC (permalink / raw)
To: =?utf-8?q?Krzysztof_Ha=C5=82asa_=3Ckhalasa=40piap=2Epl=3E?=
Cc: o.rempel, netdev, oneukum, andrew+netdev, davem, edumazet, kuba,
pabeni, linux-usb, linux-kernel, jtornosm, ming.lei
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 08 Apr 2025 13:59:41 +0200 you wrote:
> ASIX AX88772B based USB 10/100 Ethernet adapter doesn't come
> up ("carrier off"), despite the built-in 100BASE-FX PHY positive link
> indication. The internal PHY is configured (using EEPROM) in fixed
> 100 Mbps full duplex mode.
>
> The primary problem appears to be using carrier_netif_{on,off}() while,
> at the same time, delegating carrier management to phylink. Use only the
> latter and remove "manual control" in the asix driver.
>
> [...]
Here is the summary with links:
- [v2] usbnet: asix AX88772: leave the carrier control to phylink
https://git.kernel.org/netdev/net-next/c/4145f00227ee
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-04-11 2:19 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-07 12:08 [PATCH REPOST] usbnet: asix: leave the carrier control to phylink Krzysztof Hałasa
2025-04-07 13:38 ` Oleksij Rempel
2025-04-08 11:55 ` Krzysztof Hałasa
2025-04-08 11:59 ` [PATCH v2] usbnet: asix AX88772: " Krzysztof Hałasa
2025-04-08 15:22 ` Jakub Kicinski
2025-04-10 12:14 ` Oleksij Rempel
2025-04-11 2:20 ` patchwork-bot+netdevbpf
2025-04-08 13:29 ` [PATCH REPOST] usbnet: asix: " Paolo Abeni
2025-04-09 9:07 ` Krzysztof Hałasa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).