Re: [net-next,v3,1/2] r8152: Add support for 5Gbit Link Speeds and EEE
From: Paolo Abeni <pabeni@redhat.com>
Date: 2026-03-24 10:55:44
Also in:
linux-usb, lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- r8152: Add support for 5Gbit Link Speeds and EEE The RTL8157 supports 5GBit Link speeds. Add support for this speed in the setup and setting/getting through ethtool. Also add 5GBit EEE. Add functionality for setup and ethtool get/set methods.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 3b6d4252d34c..bab8e10e5f99 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -6343,15 +6356,20 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex, r8152_mdio_write(tp, MII_CTRL1000, new1); } - if (tp->support_2500full) { + if (tp->support_2500full || tp->support_5000full) { orig = ocp_reg_read(tp, OCP_10GBT_CTRL); - new1 = orig & ~MDIO_AN_10GBT_CTRL_ADV2_5G; + new1 = orig & ~(MDIO_AN_10GBT_CTRL_ADV2_5G | MDIO_AN_10GBT_CTRL_ADV5G); if (advertising & RTL_ADVERTISED_2500_FULL) { new1 |= MDIO_AN_10GBT_CTRL_ADV2_5G; tp->ups_info.speed_duplex = NWAY_2500M_FULL; } + if (advertising & RTL_ADVERTISED_5000_FULL) { + new1 |= MDIO_AN_10GBT_CTRL_ADV5G; + tp->ups_info.speed_duplex = NWAY_5000M_FULL; + } + if (orig != new1) ocp_reg_write(tp, OCP_10GBT_CTRL, new1); }
Does r8156_ups_flags() correctly handle the new NWAY_5000M_FULL value?
When 5000M is advertised, rtl8152_set_speed() stores NWAY_5000M_FULL
(enum value 11) in tp->ups_info.speed_duplex. However, r8156_ups_flags()
has a switch statement that only handles values up to NWAY_2500M_FULL
(value 10):
drivers/net/usb/r8152.c:r8156_ups_flags() {
switch (tp->ups_info.speed_duplex) {
case NWAY_10M_HALF:
return ups_flags_speed(UPS_FLAGS_SPEED_10);
case NWAY_10M_FULL:
return ups_flags_speed(UPS_FLAGS_SPEED_10);
case NWAY_100M_HALF:
return ups_flags_speed(UPS_FLAGS_SPEED_100);
case NWAY_100M_FULL:
return ups_flags_speed(UPS_FLAGS_SPEED_100);
case NWAY_1000M_FULL:
return ups_flags_speed(UPS_FLAGS_SPEED_1000);
case NWAY_2500M_FULL:
return ups_flags_speed(UPS_FLAGS_SPEED_2500);
default:
break;
}
}
The new NWAY_5000M_FULL falls through to 'default: break;', which means
ups_flags_speed() is never called. This results in incorrect UPS flags
(speed bits = 0) being written to the USB_UPS_FLAGS hardware register via
ocp_write_dword().
When the device enters UPS power saving mode at 5000M link speed, will the
hardware receive correct speed information?
The follow-up commit defca1127dd51 ('r8152: Add support for the RTL8157
hardware') enables support_5000full for RTL_VER_16 but also does not add a
NWAY_5000M_FULL case to r8156_ups_flags().