Thread (12 messages) 12 messages, 3 authors, 2026-05-02

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help