Re: [PATCH net-next v2 1/4] r8152: Add support for 10Gbit Link Speeds and EEE

From: Birger Koblitz

Date: Sat May 02 2026 - 00:19:22 EST

Thanks a lot for reviewing this patch, Jakub!
On 01/05/2026 3:15 am, Jakub Kicinski wrote:

+ if (enable && (tp->eee_adv2 & MDIO_EEE_10GT))
+ config |= MDIO_EEE_10GT;
+ else
+ config &= ~MDIO_EEE_10GT;
+
ocp_reg_write(tp, OCP_EEE_ADV2, config);
}

Is MDIO_EEE_10GT actually a bit of the EEE advertisement 2 register
(7.62, cap2 / OCP_EEE_ADV2)?

No, and it should have been clear to me. The entire bit of code is actually unneccessary, as the MDIO_EEE_10GT-BIT is part of OCP_EEE_ADV and is set in tp->eee_adv, so that rtl_eee_enable() correctly already sets it:
static void rtl_eee_enable(struct r8152 *tp, bool enable)
{
case RTL_VER_17:
if (enable) {
r8156_eee_en(tp, true);
ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
}
...
}
Will be fixed in v3.

[ ... ]

@@ -8968,6 +9008,13 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_keee *eee)
linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, common);
}
+ if (tp->support_10000full) {
+ linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, eee->supported);
+
+ if (speed & _10000bps)
+ linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common);
+ }
+
eee->eee_enabled = tp->eee_en;

Should this hunk also extend the earlier gating inside r8153_get_eee()?

A few lines above, the cap2 register reads are still gated on:

if (tp->support_2500full || tp->support_5000full) {
val = ocp_reg_read(tp, OCP_EEE_ADV2);
mii_eee_cap2_mod_linkmode_adv_t(eee->advertised, val);

val = ocp_reg_read(tp, OCP_EEE_LPABLE2);
mii_eee_cap2_mod_linkmode_adv_t(eee->lp_advertised, val);
}

The equivalent disjunction in rtl8152_set_speed() and
rtl8152_get_link_ksettings() was updated to include support_10000full,
but r8153_get_eee() (and r8152_set_eee() at the analogous spot) still
only checks support_2500full || support_5000full.

A hypothetical chip that sets only support_10000full would bypass the
OCP_EEE_ADV2 / OCP_EEE_LPABLE2 reads and never reflect 10G EEE into
eee->advertised. Should the condition be made consistent with the
other sites?

The AI here actually contradicts itself with the previous issue. The 10GBit modes are part of OCP_EEE_ADV (MMD 7.0x003C) and OCP_EEE_LPABLE (MMD 7.0x003D). A hypothetical chip that sets only support_10000full would still reflect the correct LP-ability 10GBit setting.

@@ -9982,6 +10029,11 @@ static int rtl8152_probe_once(struct usb_interface *intf,
tp->speed = SPEED_5000;
tp->advertising |= RTL_ADVERTISED_5000_FULL;
}
+ if (tp->support_10000full &&
+ tp->udev->speed >= USB_SPEED_SUPER) {
+ tp->speed = SPEED_10000;
+ tp->advertising |= RTL_ADVERTISED_10000_FULL;
+ }
tp->advertising |= RTL_ADVERTISED_1000_FULL;
}
tp->duplex = DUPLEX_FULL;

Is USB_SPEED_SUPER the correct gate for defaulting to a 10 Gbit link?

I believe this is the correct gate. In order to get the full speed out of the 10GBit Ethernet connection, a 20GBit USB link would be needed, anyway, with 10GBit USB-C, i.e. USB_SPEED_SUPER_PLUS, I see only about 7.6GBit throughput. The difference of 10GBit wire-speed to 5GBit Speed for USB 3.0 is not that bad, and the overhead does not seem to make a problem.

But I think this is not the point: an important aspect of the 10GBit USB-Ethernet adapters is that they are used for testing 10GBit connections with network equipment. The RTL8159 is even put in connection with a media converter and an SFP+ cage in the same housing and advertised for testing 10GBit fiber lines. It should be possible to test and use 10GBit connectivity even on a 5GBit USB port.

Birger