Thread (22 messages) flat view 22 messages, 4 authors, 1d ago

Re: [PATCH net-next v9 1/7] r8169: add speed in private struct

From: netdev-bot+sashiko@kernel.org
Date: 2026-09-04 22:25:18
Also in: lkml

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Claimed stale-cache regression: the patch converts
  rtl_coalesce_info() (r8169_main.c:2076,2080) and…

--

r8169: add speed in private struct

This adds an int speed field to struct rtl8169_private, initializes it to
SPEED_UNKNOWN in rtl_init_one() and updates it from r8169_phylink_handler().
The consumers rtl_link_chg_patch(), rtl_coalesce_info() and
r8169_get_tx_lpi_timer_us() are switched from tp->phydev->speed to the new
cached value.

No Link: tags in the commit header.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index ec4fc21fa21f..c60710f9bd21 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -750,6 +750,7 @@ struct rtl8169_private {
 	u32 irq_mask;
 	int irq;
 	struct clk *clk;
+	int speed;
 
 	struct {
 		DECLARE_BITMAP(flags, RTL_FLAG_MAX);
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2074,11 +2073,11 @@ rtl_coalesce_info(struct rtl8169_private *tp)
 		ci = rtl_coalesce_info_8168_8136;
 
 	/* if speed is unknown assume highest one */
-	if (tp->phydev->speed == SPEED_UNKNOWN)
+	if (tp->speed == SPEED_UNKNOWN)
 		return ci;
 
 	for (; ci->speed; ci++) {
-		if (tp->phydev->speed == ci->speed)
+		if (tp->speed == ci->speed)
 			return ci;
 	}
 
@@ -2236,7 +2235,7 @@ static void rtl_set_eee_txidle_timer(struct rtl8169_private *tp)
 
 static unsigned int r8169_get_tx_lpi_timer_us(struct rtl8169_private *tp)
 {
-	unsigned int speed = tp->phydev->speed;
+	unsigned int speed = tp->speed;
 	unsigned int timer = tp->tx_lpi_timer;
 
 	if (!timer || speed == SPEED_UNKNOWN)
@@ -4968,8 +4967,9 @@ static void r8169_phylink_handler(struct net_device *ndev)
 	struct rtl8169_private *tp = netdev_priv(ndev);
 	struct device *d = tp_to_dev(tp);
 
+	tp->speed = tp->phydev->speed;
[Severity: Low]
This isn't a bug, but the v3 entry in the changelog below the --- line says:

  Changes in v3:
   - update tp->speed in rtl8169_set_link_ksettings()

That hunk does not appear in this diff. After this patch the only runtime
writer of tp->speed is r8169_phylink_handler(), while the SFP branch of
rtl8169_set_link_ksettings() still writes phydev->speed directly and
reprograms the SerDes without touching tp->speed:

	if (!phy_check_valid(speed, duplex, phydev->supported))
		return -EINVAL;

	mutex_lock(&phydev->lock);

	phydev->autoneg = AUTONEG_DISABLE;
	phydev->speed = speed;
	phydev->duplex = duplex;

	rtl_sfp_init(tp);

	mutex_unlock(&phydev->lock);

Should the changelog entry be dropped, or was the intended
rtl8169_set_link_ksettings() update accidentally lost in a rebase?

Following the two converted consumers, the divergence does not look
observable today. tp->sfp_mode is only assigned inside
if (rtl_is_8125(tp)) in rtl_init_one(), and both rtl_get_coalesce() and
rtl_set_coalesce() start with:

	if (rtl_is_8125(tp))
		return -EOPNOTSUPP;

so rtl_coalesce_info() cannot be reached on the chips that take the SFP
write path. The SFP branch is also gated by phy_check_valid() against
phydev->supported, and rtlgen_sfp_get_features() in
drivers/net/phy/realtek/realtek_main.c only sets
ETHTOOL_LINK_MODE_10000baseT_Full_BIT, so no other speed can be forced
there, and r8169_get_tx_lpi_timer_us() keeps seeing the real link rate that
rtlgen_sfp_read_status() decodes from RTL_PHYSR.

The construct is also replaced later in this series by "r8169: add support
for phylink", where rtl8169_set_link_ksettings() only calls
phylink_ethtool_ksettings_set() and tp->speed is written from
rtl_mac_link_up() and rtl_mac_link_down(), so only the changelog/diff
mismatch remains.
quoted hunk ↗ jump to hunk
 	if (netif_carrier_ok(ndev)) {
-		rtl_link_chg_patch(tp);
+		rtl_link_chg_patch(tp, tp->speed);
 		rtl_enable_tx_lpi(tp, tp->phydev->enable_tx_lpi);
 		pm_request_resume(d);
 	} else {
@@ -5667,6 +5667,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 				     ext_xid_str, xid);
 	tp->mac_version = chip->mac_version;
 	tp->fw_name = chip->fw_name;
+	tp->speed = SPEED_UNKNOWN;
[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831053745.1197-1-javen_xu%40realsil.com.cn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help