Re: [PATCH net-next v2 1/4] r8152: Add support for 10Gbit Link Speeds and EEE
From: Birger Koblitz <hidden>
Date: 2026-05-02 04:19:04
Also in:
linux-usb, lkml
Thanks a lot for reviewing this patch, Jakub! On 01/05/2026 3:15 am, Jakub Kicinski wrote:
quoted
+ 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.
[ ... ]quoted
@@ -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.
quoted
@@ -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