* [PATCH net-next v7 01/10] r8152: split r8156_init by generation and factor out wait_autoload_done
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
@ 2026-10-14 8:18 ` Chih Kai Hsu
2026-10-21 0:52 ` Jakub Kicinski
2026-10-14 8:18 ` [PATCH net-next v7 02/10] r8152: add new init writes for RTL8156/8156B/8157/8159 Chih Kai Hsu
` (9 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:18 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
RTL8156, RTL8156B, RTL8157 and RTL8159 shared a single r8156_init(),
gated by a dense set of tp->version range checks. That made the init
sequence hard to follow.
Split r8156_init() into two functions by chip generation:
- r8156_init (VER_10/11 and VER_12/13/15, RTL8156 and RTL8156B)
- r8157_init (VER_16/17, RTL8157 and RTL8159)
The members of each generation have near-identical sequences, so they
share a function and branch on tp->version only for the few real
differences, such as the RTL8156B-only flow-control setup or the
RTL8159-only backup-restore wait.
Also factor the repeated "poll PLA_BOOT_CTRL for AUTOLOAD_DONE" loop
into wait_autoload_done(), reusing read_poll_timeout() instead of the
500 * msleep(20) busy loops. r8153_init(), r8153b_init(),
r8153c_init(), r8153b_ups_en() and r8153c_ups_en() are converted to
use it too.
wait_autoload_done() only reports failure (nonzero) when
RTL8152_INACCESSIBLE is set; a plain AUTOLOAD_DONE timeout is logged
but otherwise treated as success, same as every one of the loops it
replaces, which only bailed out early on RTL8152_INACCESSIBLE and
otherwise fell through to the rest of the init sequence. The two
failure modes are not equivalent: once RTL8152_INACCESSIBLE is set,
generic_ocp_read()/generic_ocp_write() short-circuit to -ENODEV
without touching the device, so returning early costs nothing. A plain
timeout means the device is still reachable, so skipping the rest of
the bring-up would leave real hardware state half-configured.
No functional change intended.
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 307 +++++++++++++++++++++++-----------------
1 file changed, 180 insertions(+), 127 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index af3d7dcb2f141..518a2b42c2200 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -4172,6 +4172,23 @@ static u16 r8153_phy_status(struct r8152 *tp, u16 desired)
return data;
}
+static int wait_autoload_done(struct r8152 *tp)
+{
+ u16 ocp_data;
+ int ret;
+
+ ret = read_poll_timeout(ocp_read_word, ocp_data,
+ test_bit(RTL8152_INACCESSIBLE, &tp->flags) ||
+ (ocp_data & AUTOLOAD_DONE),
+ 20000, 10 * USEC_PER_SEC, false, tp,
+ MCU_TYPE_PLA, PLA_BOOT_CTRL);
+
+ if (ret)
+ dev_err(&tp->intf->dev, "autoload done timeout\n");
+
+ return test_bit(RTL8152_INACCESSIBLE, &tp->flags) ? -ENODEV : 0;
+}
+
static void r8153b_ups_en(struct r8152 *tp, bool enable)
{
if (enable) {
@@ -4190,16 +4207,8 @@ static void r8153b_ups_en(struct r8152 *tp, bool enable)
UPS_FORCE_PWR_DOWN);
if (ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0) & PCUT_STATUS) {
- int i;
-
- for (i = 0; i < 500; i++) {
- if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
- return;
- if (ocp_read_word(tp, MCU_TYPE_PLA, PLA_BOOT_CTRL) &
- AUTOLOAD_DONE)
- break;
- msleep(20);
- }
+ if (wait_autoload_done(tp))
+ return;
tp->rtl_ops.hw_phy_cfg(tp);
@@ -4227,16 +4236,8 @@ static void r8153c_ups_en(struct r8152 *tp, bool enable)
UPS_FORCE_PWR_DOWN);
if (ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0) & PCUT_STATUS) {
- int i;
-
- for (i = 0; i < 500; i++) {
- if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
- return;
- if (ocp_read_word(tp, MCU_TYPE_PLA, PLA_BOOT_CTRL) &
- AUTOLOAD_DONE)
- break;
- msleep(20);
- }
+ if (wait_autoload_done(tp))
+ return;
tp->rtl_ops.hw_phy_cfg(tp);
@@ -7225,22 +7226,14 @@ static void r8152b_init(struct r8152 *tp)
static void r8153_init(struct r8152 *tp)
{
u32 ocp_data;
- int i;
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
return;
r8153_u1u2en(tp, false);
- for (i = 0; i < 500; i++) {
- if (ocp_read_word(tp, MCU_TYPE_PLA, PLA_BOOT_CTRL) &
- AUTOLOAD_DONE)
- break;
-
- msleep(20);
- if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
- break;
- }
+ if (wait_autoload_done(tp))
+ return;
r8153_phy_status(tp, 0);
@@ -7341,22 +7334,13 @@ static void r8153_init(struct r8152 *tp)
static void r8153b_init(struct r8152 *tp)
{
- int i;
-
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
return;
r8153b_u1u2en(tp, false);
- for (i = 0; i < 500; i++) {
- if (ocp_read_word(tp, MCU_TYPE_PLA, PLA_BOOT_CTRL) &
- AUTOLOAD_DONE)
- break;
-
- msleep(20);
- if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
- break;
- }
+ if (wait_autoload_done(tp))
+ return;
r8153_phy_status(tp, 0);
@@ -7411,8 +7395,6 @@ static void r8153b_init(struct r8152 *tp)
static void r8153c_init(struct r8152 *tp)
{
- int i;
-
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
return;
@@ -7425,15 +7407,8 @@ static void r8153c_init(struct r8152 *tp)
ocp_word_set_bits(tp, MCU_TYPE_USB, 0xcbf0, BIT(1));
- for (i = 0; i < 500; i++) {
- if (ocp_read_word(tp, MCU_TYPE_PLA, PLA_BOOT_CTRL) &
- AUTOLOAD_DONE)
- break;
-
- msleep(20);
- if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
- return;
- }
+ if (wait_autoload_done(tp))
+ return;
r8153_phy_status(tp, 0);
@@ -8330,90 +8305,64 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
set_bit(PHY_RESET, &tp->flags);
}
-static int r8159_wait_backup_restore(struct r8152 *tp)
-{
- u32 ocp_data;
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
- if (!(ocp_data & PCUT_STATUS))
- return 0;
-
- return poll_timeout_us(ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_GPHY_CTRL),
- ocp_data & BACKUP_RESTRORE, 200, 20000, false);
-}
-
static void r8156_init(struct r8152 *tp)
{
u32 ocp_data;
u16 data;
- int i;
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
return;
- if (tp->version == RTL_VER_16 || tp->version == RTL_VER_17) {
- ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xcffe, BIT(3));
- ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd3ca, BIT(0));
- }
-
ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_ECM_OP, EN_ALL_SPEED);
- if (tp->version < RTL_VER_16)
- ocp_write_word(tp, MCU_TYPE_USB, USB_SPEED_OPTION, 0);
+ ocp_write_word(tp, MCU_TYPE_USB, USB_SPEED_OPTION, 0);
ocp_word_set_bits(tp, MCU_TYPE_USB, USB_ECM_OPTION, BYPASS_MAC_RESET);
- if (tp->version >= RTL_VER_12 && tp->version <= RTL_VER_15)
+ switch (tp->version) {
+ case RTL_VER_12:
+ case RTL_VER_13:
+ case RTL_VER_15:
ocp_word_set_bits(tp, MCU_TYPE_USB, USB_U2P3_CTRL, RX_DETECT8);
+ break;
+ default:
+ break;
+ }
r8153b_u1u2en(tp, false);
switch (tp->version) {
case RTL_VER_13:
case RTL_VER_15:
- case RTL_VER_16:
- case RTL_VER_17:
r8156b_wait_loading_flash(tp);
break;
default:
break;
}
- for (i = 0; i < 500; i++) {
- if (ocp_read_word(tp, MCU_TYPE_PLA, PLA_BOOT_CTRL) &
- AUTOLOAD_DONE)
- break;
-
- msleep(20);
- if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
- return;
- }
-
- if (tp->version == RTL_VER_17 && r8159_wait_backup_restore(tp)) {
- rtl_set_inaccessible(tp);
- dev_err(&tp->intf->dev, "init failed, backup-restore timed out\n");
+ if (wait_autoload_done(tp))
return;
- }
data = r8153_phy_status(tp, 0);
if (data == PHY_STAT_EXT_INIT) {
ocp_reg_clr_bits(tp, 0xa468, BIT(3) | BIT(1));
- if (tp->version >= RTL_VER_12)
+
+ switch (tp->version) {
+ case RTL_VER_12:
+ case RTL_VER_13:
+ case RTL_VER_15:
ocp_reg_clr_bits(tp, 0xa466, BIT(0));
+ break;
+ default:
+ break;
+ }
}
- data = r8152_mdio_read(tp, MII_BMCR);
- if (data & BMCR_PDOWN) {
- data &= ~BMCR_PDOWN;
- r8152_mdio_write(tp, MII_BMCR, data);
- }
+ r8152_mdio_test_and_clr_bit(tp, MII_BMCR, BMCR_PDOWN);
data = r8153_phy_status(tp, PHY_STAT_LAN_ON);
- if (tp->version >= RTL_VER_16)
- r8157_u2p3en(tp, false);
- else
- r8153_u2p3en(tp, false);
+ r8153_u2p3en(tp, false);
/* MSC timer = 0xfff * 8ms = 32760 ms */
ocp_write_word(tp, MCU_TYPE_USB, USB_MSC_TIMER, 0x0fff);
@@ -8421,11 +8370,7 @@ static void r8156_init(struct r8152 *tp)
/* U1/U2/L1 idle timer. 500 us */
ocp_write_word(tp, MCU_TYPE_USB, USB_U1U2_TIMER, 500);
- if (tp->version >= RTL_VER_16)
- r8157_power_cut_en(tp, false);
- else
- r8153b_power_cut_en(tp, false);
-
+ r8153b_power_cut_en(tp, false);
r8156_ups_en(tp, false);
r8153_queue_wake(tp, false);
rtl_runtime_suspend_enable(tp, false);
@@ -8435,7 +8380,10 @@ static void r8156_init(struct r8152 *tp)
usb_enable_lpm(tp->udev);
- if (tp->version >= RTL_VER_12 && tp->version <= RTL_VER_15) {
+ switch (tp->version) {
+ case RTL_VER_12:
+ case RTL_VER_13:
+ case RTL_VER_15:
ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR, SLOT_EN);
ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_CPCR, FLOW_CTRL_EN);
@@ -8450,43 +8398,148 @@ static void r8156_init(struct r8152 *tp)
ocp_write_word(tp, MCU_TYPE_USB, USB_FW_CTRL, ocp_data);
ocp_word_set_bits(tp, MCU_TYPE_USB, USB_FW_TASK, FC_PATCH_TASK);
+ break;
+ default:
+ break;
}
r8156_mac_clk_spd(tp, true);
- if (tp->version < RTL_VER_16)
- ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, PLA_MCU_SPDWN_EN);
+ ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, PLA_MCU_SPDWN_EN);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EXTRA_STATUS);
if (rtl8152_get_speed(tp) & LINK_STATUS)
- ocp_data |= CUR_LINK_OK;
+ ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_EXTRA_STATUS,
+ CUR_LINK_OK | POLL_LINK_CHG);
else
- ocp_data &= ~CUR_LINK_OK;
- ocp_data |= POLL_LINK_CHG;
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_EXTRA_STATUS, ocp_data);
+ ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_EXTRA_STATUS, CUR_LINK_OK,
+ POLL_LINK_CHG);
+
+ set_bit(GREEN_ETHERNET, &tp->flags);
+
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL,
+ RX_AGG_DISABLE | RX_ZERO_EN);
+
+ switch (tp->version) {
+ case RTL_TEST_01:
+ case RTL_VER_10:
+ case RTL_VER_11:
+ ocp_byte_set_bits(tp, MCU_TYPE_USB, USB_BMU_CONFIG, ACT_ODMA);
+ break;
+ default:
+ break;
+ }
+
+ r8156_mdio_force_mode(tp);
+ rtl_tally_reset(tp);
+
+ tp->coalesce = 15000; /* 15 us */
+}
+
+static int r8159_wait_backup_restore(struct r8152 *tp)
+{
+ u32 ocp_data;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
+ if (!(ocp_data & PCUT_STATUS))
+ return 0;
+
+ return poll_timeout_us(ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_GPHY_CTRL),
+ ocp_data & BACKUP_RESTRORE, 200, 20000, false);
+}
+
+static void r8157_init(struct r8152 *tp)
+{
+ u16 data;
+
+ if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
+ return;
+
+ /* Enable SW reset */
+ ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xcffe, BIT(3));
+ ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd3ca, BIT(0));
+
+ ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_ECM_OP, EN_ALL_SPEED);
+
+ ocp_word_set_bits(tp, MCU_TYPE_USB, USB_ECM_OPTION, BYPASS_MAC_RESET);
+
+ r8153b_u1u2en(tp, false);
+
+ r8156b_wait_loading_flash(tp);
+
+ if (wait_autoload_done(tp))
+ return;
+
+ switch (tp->version) {
+ case RTL_VER_17:
+ if (r8159_wait_backup_restore(tp)) {
+ rtl_set_inaccessible(tp);
+ dev_err(&tp->intf->dev,
+ "init failed, backup-restore timed out\n");
+ return;
+ }
+ break;
+ default:
+ break;
+ }
+
+ data = r8153_phy_status(tp, 0);
+ if (data == PHY_STAT_EXT_INIT) {
+ ocp_reg_clr_bits(tp, 0xa468, BIT(3) | BIT(1));
+ ocp_reg_clr_bits(tp, 0xa466, BIT(0));
+ }
+
+ r8152_mdio_test_and_clr_bit(tp, MII_BMCR, BMCR_PDOWN);
+
+ data = r8153_phy_status(tp, PHY_STAT_LAN_ON);
+
+ r8157_u2p3en(tp, false);
+
+ /* MSC timer = 0xfff * 8ms = 32760 ms */
+ ocp_write_word(tp, MCU_TYPE_USB, USB_MSC_TIMER, 0x0fff);
+
+ /* U1/U2/L1 idle timer. 500 us */
+ ocp_write_word(tp, MCU_TYPE_USB, USB_U1U2_TIMER, 500);
+
+ r8157_power_cut_en(tp, false);
+ r8156_ups_en(tp, false);
+ r8153_queue_wake(tp, false);
+ rtl_runtime_suspend_enable(tp, false);
+
+ if (tp->udev->speed >= USB_SPEED_SUPER)
+ r8153b_u1u2en(tp, true);
+
+ usb_enable_lpm(tp->udev);
+
+ r8156_mac_clk_spd(tp, true);
+
+ if (rtl8152_get_speed(tp) & LINK_STATUS)
+ ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_EXTRA_STATUS,
+ CUR_LINK_OK | POLL_LINK_CHG);
+ else
+ ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_EXTRA_STATUS, CUR_LINK_OK,
+ POLL_LINK_CHG);
set_bit(GREEN_ETHERNET, &tp->flags);
/* RX aggregation / 16 bytes RX descriptor
* BIT(11) is specific to RTL8159, with unknown meaning
*/
- if (tp->version == RTL_VER_17)
+ switch (tp->version) {
+ case RTL_VER_17:
ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL,
RX_AGG_DISABLE | RX_DESC_16B | BIT(11));
- else if (tp->version == RTL_VER_16)
- ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL, RX_AGG_DISABLE | RX_DESC_16B);
- else
- ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL, RX_AGG_DISABLE | RX_ZERO_EN);
+ break;
+ default:
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL,
+ RX_AGG_DISABLE | RX_DESC_16B);
+ break;
+ }
- if (tp->version < RTL_VER_12)
- ocp_byte_set_bits(tp, MCU_TYPE_USB, USB_BMU_CONFIG, ACT_ODMA);
+ /* Disable Rx Zero Len */
+ rtl_bmu_clr_bits(tp, 0x2300, BIT(3));
- if (tp->version >= RTL_VER_16) {
- /* Disable Rx Zero Len */
- rtl_bmu_clr_bits(tp, 0x2300, BIT(3));
- /* TX descriptor Signature */
- ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd4ae, BIT(1));
- }
+ /* TX descriptor Signature */
+ ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd4ae, BIT(1));
r8156_mdio_force_mode(tp);
rtl_tally_reset(tp);
@@ -9851,7 +9904,7 @@ static int rtl_ops_init(struct r8152 *tp)
tp->eee_en = true;
tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
- ops->init = r8156_init;
+ ops->init = r8157_init;
ops->enable = rtl8156_enable;
ops->disable = rtl8153_disable;
ops->up = rtl8156_up;
@@ -9873,7 +9926,7 @@ static int rtl_ops_init(struct r8152 *tp)
tp->eee_en = true;
tp->eee_adv = MDIO_EEE_100TX | MDIO_EEE_1000T | MDIO_EEE_10GT;
tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
- ops->init = r8156_init;
+ ops->init = r8157_init;
ops->enable = rtl8156_enable;
ops->disable = rtl8153_disable;
ops->up = rtl8156_up;
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v7 01/10] r8152: split r8156_init by generation and factor out wait_autoload_done
2026-10-14 8:18 ` [PATCH net-next v7 01/10] r8152: split r8156_init by generation and factor out wait_autoload_done Chih Kai Hsu
@ 2026-10-21 0:52 ` Jakub Kicinski
0 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2026-10-21 0:52 UTC (permalink / raw)
To: hsu.chih.kai
Cc: Jakub Kicinski, davem, netdev, nic_swsd, linux-kernel, linux-usb,
edumazet, bjorn, pabeni, andrew+netdev, mail
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: split r8156_init by generation and factor out wait_autoload_done
The patch splits r8156_init() into two functions. r8156_init() now covers
RTL8156/RTL8156B, and the new r8157_init() covers RTL8157/RTL8159. It also
moves the repeated PLA_BOOT_CTRL AUTOLOAD_DONE polling loop into a new
wait_autoload_done() helper built on read_poll_timeout(), and converts the
r8153 init and ups_en paths to use it.
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index af3d7dcb2f141..518a2b42c2200 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -4172,6 +4172,23 @@ static u16 r8153_phy_status(struct r8152 *tp, u16 desired)
> return data;
> }
>
> +static int wait_autoload_done(struct r8152 *tp)
> +{
> + u16 ocp_data;
> + int ret;
> +
> + ret = read_poll_timeout(ocp_read_word, ocp_data,
> + test_bit(RTL8152_INACCESSIBLE, &tp->flags) ||
> + (ocp_data & AUTOLOAD_DONE),
> + 20000, 10 * USEC_PER_SEC, false, tp,
> + MCU_TYPE_PLA, PLA_BOOT_CTRL);
> +
> + if (ret)
> + dev_err(&tp->intf->dev, "autoload done timeout\n");
> +
> + return test_bit(RTL8152_INACCESSIBLE, &tp->flags) ? -ENODEV : 0;
> +}
> +
[Severity: Medium]
Can this end up reading uninitialized stack memory in ocp_read_word()?
read_poll_timeout() runs op before it evaluates cond, on the first
iteration, after every sleep, and once more after the timeout expires.
So once RTL8152_INACCESSIBLE gets set, the helper still issues another
ocp_read_word() on PLA_BOOT_CTRL before the test_bit() in the condition
gets a chance to stop the loop.
In that case generic_ocp_read() returns -ENODEV without writing to the
local tmp. ocp_read_word() ignores that return value and still does
__le32_to_cpu(tmp), then hands the result back as ocp_data.
The loops being replaced did not do this. r8153b_ups_en() and
r8153c_ups_en() tested the flag before each read. The init functions
tested it right after msleep(20) and left the loop before the next
read.
The test_bit() short-circuit means the garbage is never acted on, but
the read itself is still undefined behaviour of the sort KMSAN is meant
to catch, and it now happens whenever the device goes away while we are
waiting for AUTOLOAD_DONE.
A flag check before the poll would only cover the entry case, since
the flag can be set during the sleep. Would it be better to initialize
tmp in ocp_read_word() and the other ocp_read_*() helpers, given that
plenty of existing callers already read without checking the flag?
Or is an open-coded loop that tests the flag first preferable here?
[ ... ]
> @@ -7225,22 +7226,14 @@ static void r8152b_init(struct r8152 *tp)
> static void r8153_init(struct r8152 *tp)
> {
> u32 ocp_data;
> - int i;
>
> if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
> return;
>
> r8153_u1u2en(tp, false);
>
> - for (i = 0; i < 500; i++) {
> - if (ocp_read_word(tp, MCU_TYPE_PLA, PLA_BOOT_CTRL) &
> - AUTOLOAD_DONE)
> - break;
> -
> - msleep(20);
> - if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
> - break;
> - }
> + if (wait_autoload_done(tp))
> + return;
>
> r8153_phy_status(tp, 0);
>
> @@ -7341,22 +7334,13 @@ static void r8153_init(struct r8152 *tp)
>
> static void r8153b_init(struct r8152 *tp)
> {
> - int i;
> -
> if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
> return;
>
> r8153b_u1u2en(tp, false);
>
> - for (i = 0; i < 500; i++) {
> - if (ocp_read_word(tp, MCU_TYPE_PLA, PLA_BOOT_CTRL) &
> - AUTOLOAD_DONE)
> - break;
> -
> - msleep(20);
> - if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
> - break;
> - }
> + if (wait_autoload_done(tp))
> + return;
>
> r8153_phy_status(tp, 0);
>
[Severity: Medium]
Is the commit message accurate for r8153_init() and r8153b_init()? It
says:
a plain AUTOLOAD_DONE timeout is logged but otherwise treated as
success, same as every one of the loops it replaces, which only bailed
out early on RTL8152_INACCESSIBLE and otherwise fell through to the
rest of the init sequence.
and:
No functional change intended.
In these two functions the old loops did a break, not a return, when
RTL8152_INACCESSIBLE was set:
msleep(20);
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
break;
So init always ran to the end of the function. Only r8153c_init(), the
old r8156_init() and the two *_ups_en() loops returned early.
wait_autoload_done() returns -ENODEV whenever RTL8152_INACCESSIBLE is
set, so both functions now return early instead.
The commit message says the early return costs nothing because
generic_ocp_read()/generic_ocp_write() short-circuit. However, the
skipped tail also does work that does not go through OCP accesses:
r8153_init():
usb_enable_lpm(tp->udev);
...
tp->coalesce = COALESCE_SUPER; (or COALESCE_HIGH / COALESCE_SLOW)
r8153b_init():
usb_enable_lpm(tp->udev);
...
set_bit(GREEN_ETHERNET, &tp->flags);
...
tp->coalesce = 15000;
usb_enable_lpm() is a counted USB core call. It decrements
udev->lpm_disable_count and does not check the driver's
RTL8152_INACCESSIBLE flag.
The polling interval also changes. The old loops used msleep(20), while
read_poll_timeout() with a sleep_us of 20000 ends up in
usleep_range(5001, 20000).
These differences only show up on error paths. Still, could the commit
message describe the new early return in r8153_init() and r8153b_init()?
Or should these two functions keep the old break-and-continue behaviour?
--
pw-bot: cr
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v7 02/10] r8152: add new init writes for RTL8156/8156B/8157/8159
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
2026-10-14 8:18 ` [PATCH net-next v7 01/10] r8152: split r8156_init by generation and factor out wait_autoload_done Chih Kai Hsu
@ 2026-10-14 8:18 ` Chih Kai Hsu
2026-10-14 8:18 ` [PATCH net-next v7 03/10] r8152: split RTL_VER_17 into QFN68 and QFN100 package variants Chih Kai Hsu
` (8 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:18 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
Bring the per-generation init functions in line with the vendor init
sequence by adding writes the shared r8156_init() never performed.
- Clear PREBOOT_OPTION and RMT_WAKE_EN in r8156_init() and
r8157_init().
- Add r8156b_u2phy_backup() for RTL8156B (VER_12/13/15).
- Clear AUTO_SPEEDUP in r8157_init().
- Disable interrupt mitigation (0xcf04) in r8157_init().
- Clear the Warm RST / Bus RST event flag (0xcd06 BIT(11)) in
r8157_init().
- Clear the FW u1u2 patch option (0xb9a6 BIT(0)) and enable the u3phy
patch backup (0xb9a2) for RTL8159 (VER_17) only.
- Clear RX_END_TRANSFER_EN (0xd406 BIT(11)) for RTL8157 (VER_16), too.
While it is set, incoming packets are buffered in the RX FIFO and
Bulk-in transfers are suspended. The bit is zero by default and
isn't used now. Clear it unconditionally for safety.
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 77 ++++++++++++++++++++++++++++++++++-------
1 file changed, 64 insertions(+), 13 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 518a2b42c2200..43a3f09b7c29b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -78,6 +78,7 @@
#define PLA_EEE_TXTWSYS_2P5G 0xe058
#define PLA_EEEP_CR 0xe080
#define PLA_MAC_PWR_CTRL 0xe0c0
+#define PLA_RMT_WAKE 0xe0c8
#define PLA_MAC_PWR_CTRL2 0xe0ca
#define PLA_MAC_PWR_CTRL3 0xe0cc
#define PLA_MAC_PWR_CTRL4 0xe0ce
@@ -371,6 +372,9 @@
#define MCU_CLK_RATIO_MASK 0x0f0f0f0f
#define ALDPS_SPDWN_RATIO 0x0f87
+/* PLA_RMT_WAKE */
+#define RMT_WAKE_EN BIT(0)
+
/* PLA_MAC_PWR_CTRL2 */
#define EEE_SPDWN_RATIO 0x8007
#define MAC_CLK_SPDWN_EN BIT(15)
@@ -417,6 +421,7 @@
/* PLA_INDICATE_FALG */
#define UPCOMING_RUNTIME_D3 BIT(0)
+#define PREBOOT_OPTION BIT(1)
/* PLA_MACDBG_PRE and PLA_MACDBG_POST */
#define DEBUG_OE BIT(0)
@@ -542,6 +547,7 @@
#define RX_AGG_DISABLE 0x0010
#define RX_ZERO_EN 0x0080
#define RX_DESC_16B 0x0400
+#define RX_END_TRANSFER_EN BIT(11)
/* USB_U2P3_CTRL */
#define U2P3_ENABLE 0x0001
@@ -8305,6 +8311,15 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
set_bit(PHY_RESET, &tp->flags);
}
+static void r8156b_u2phy_backup(struct r8152 *tp)
+{
+ ocp_write_word(tp, MCU_TYPE_USB, 0xd3ce, 0x181b);
+ ocp_write_dword(tp, MCU_TYPE_USB, 0xd3d0, 0x616ccd99);
+ ocp_write_dword(tp, MCU_TYPE_USB, 0xd3d4, 0x08fc8101);
+ ocp_write_dword(tp, MCU_TYPE_USB, 0xd3d8, 0x159b1100);
+ ocp_write_word(tp, MCU_TYPE_USB, 0xd3dc, 0x0a00);
+}
+
static void r8156_init(struct r8152 *tp)
{
u32 ocp_data;
@@ -8370,11 +8385,25 @@ static void r8156_init(struct r8152 *tp)
/* U1/U2/L1 idle timer. 500 us */
ocp_write_word(tp, MCU_TYPE_USB, USB_U1U2_TIMER, 500);
+ switch (tp->version) {
+ case RTL_VER_12:
+ case RTL_VER_13:
+ case RTL_VER_15:
+ r8156b_u2phy_backup(tp);
+ break;
+ default:
+ break;
+ }
+
r8153b_power_cut_en(tp, false);
r8156_ups_en(tp, false);
r8153_queue_wake(tp, false);
rtl_runtime_suspend_enable(tp, false);
+ ocp_byte_clr_bits(tp, MCU_TYPE_PLA, PLA_INDICATE_FALG, PREBOOT_OPTION);
+
+ ocp_byte_clr_bits(tp, MCU_TYPE_PLA, PLA_RMT_WAKE, RMT_WAKE_EN);
+
if (tp->udev->speed >= USB_SPEED_SUPER)
r8153b_u1u2en(tp, true);
@@ -8494,6 +8523,10 @@ static void r8157_init(struct r8152 *tp)
r8157_u2p3en(tp, false);
+ /* Disable Interrupt Mitigation */
+ ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xcf04,
+ BIT(0) | BIT(1) | BIT(2) | BIT(7));
+
/* MSC timer = 0xfff * 8ms = 32760 ms */
ocp_write_word(tp, MCU_TYPE_USB, USB_MSC_TIMER, 0x0fff);
@@ -8505,11 +8538,30 @@ static void r8157_init(struct r8152 *tp)
r8153_queue_wake(tp, false);
rtl_runtime_suspend_enable(tp, false);
+ ocp_byte_clr_bits(tp, MCU_TYPE_PLA, PLA_INDICATE_FALG, PREBOOT_OPTION);
+
+ ocp_byte_clr_bits(tp, MCU_TYPE_PLA, PLA_RMT_WAKE, RMT_WAKE_EN);
+
+ /* Clear Warm RST / Bus RST event flag */
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, 0xcd06, BIT(11));
+
+ switch (tp->version) {
+ case RTL_VER_17:
+ /* Disable FW u1u2 patch option */
+ ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xb9a6, BIT(0));
+ break;
+ default:
+ break;
+ }
+
if (tp->udev->speed >= USB_SPEED_SUPER)
r8153b_u1u2en(tp, true);
usb_enable_lpm(tp->udev);
+ /* Disable Auto Speed up */
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_FW_CTRL, AUTO_SPEEDUP);
+
r8156_mac_clk_spd(tp, true);
if (rtl8152_get_speed(tp) & LINK_STATUS)
@@ -8521,19 +8573,9 @@ static void r8157_init(struct r8152 *tp)
set_bit(GREEN_ETHERNET, &tp->flags);
- /* RX aggregation / 16 bytes RX descriptor
- * BIT(11) is specific to RTL8159, with unknown meaning
- */
- switch (tp->version) {
- case RTL_VER_17:
- ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL,
- RX_AGG_DISABLE | RX_DESC_16B | BIT(11));
- break;
- default:
- ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL,
- RX_AGG_DISABLE | RX_DESC_16B);
- break;
- }
+ /* RX aggregation / 16 bytes RX descriptor / Bulk In End transfer */
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL,
+ RX_AGG_DISABLE | RX_DESC_16B | RX_END_TRANSFER_EN);
/* Disable Rx Zero Len */
rtl_bmu_clr_bits(tp, 0x2300, BIT(3));
@@ -8541,6 +8583,15 @@ static void r8157_init(struct r8152 *tp)
/* TX descriptor Signature */
ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd4ae, BIT(1));
+ switch (tp->version) {
+ case RTL_VER_17:
+ /* Enable u3phy patch backup */
+ ocp_write_word(tp, MCU_TYPE_USB, 0xb9a2, 0x0448);
+ break;
+ default:
+ break;
+ }
+
r8156_mdio_force_mode(tp);
rtl_tally_reset(tp);
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH net-next v7 03/10] r8152: split RTL_VER_17 into QFN68 and QFN100 package variants
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
2026-10-14 8:18 ` [PATCH net-next v7 01/10] r8152: split r8156_init by generation and factor out wait_autoload_done Chih Kai Hsu
2026-10-14 8:18 ` [PATCH net-next v7 02/10] r8152: add new init writes for RTL8156/8156B/8157/8159 Chih Kai Hsu
@ 2026-10-14 8:18 ` Chih Kai Hsu
2026-10-14 8:18 ` [PATCH net-next v7 04/10] r8152: split rtl8156_enable/up/down into per-generation functions Chih Kai Hsu
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:18 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
RTL8159 is sold in QFN68 and QFN100 packages, and only QFN68 needs
the u2phy backup-restore patch and enables UPS (in a later patch).
There was previously no way to tell the two apart at runtime, since
both used the same RTL_VER_17.
Split RTL_VER_17 into RTL_VER_17_QFN68 and RTL_VER_17_QFN100 and
update every switch/case on RTL_VER_17 in the driver to handle both.
Detect the package by reading the new PLA_PKG_DET register
(0xdc48, masked with PKG_MASK = 0x1e) via a new __rtl_get_pkg_det()
helper, called from __rtl_get_hw_ver() when the chip ID is 0x2020:
pkg_det value 0x1e/0x1c selects QFN68, 0x18/0x1a selects QFN100, and
any other value logs "Unknown package" and falls back to
RTL_VER_UNKNOWN.
r8157_init() gains a QFN68-only write ("Enable u2phy backup restore
patch", 0xb99c BIT(0)) that does not apply to QFN100.
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 97 +++++++++++++++++++++++++++++++++--------
1 file changed, 80 insertions(+), 17 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 43a3f09b7c29b..25fce499f7938 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -64,6 +64,7 @@
#define PLA_MACDBG_POST 0xd38e /* RTL_VER_04 only */
#define PLA_EXTRA_STATUS 0xd398
#define PLA_GPHY_CTRL 0xd3ae
+#define PLA_PKG_DET 0xdc48
#define PLA_POL_GPIO_CTRL 0xdc6a
#define PLA_EFUSE_DATA 0xdd00
#define PLA_EFUSE_CMD 0xdd02
@@ -290,6 +291,9 @@
#define IFG_144NS BIT(9)
#define IFG_96NS (BIT(9) | BIT(8))
+/* PLA_PKG_DET */
+#define PKG_MASK 0x1e
+
/* PLA_MTPS */
#define MTPS_JUMBO (12 * 1024 / 64)
#define MTPS_DEFAULT (6 * 1024 / 64)
@@ -1253,7 +1257,8 @@ enum rtl_version {
RTL_VER_14,
RTL_VER_15,
RTL_VER_16,
- RTL_VER_17,
+ RTL_VER_17_QFN68,
+ RTL_VER_17_QFN100,
RTL_VER_MAX
};
@@ -3418,7 +3423,8 @@ static void rtl8152_nic_reset(struct r8152 *tp)
break;
case RTL_VER_16:
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
ocp_byte_clr_bits(tp, MCU_TYPE_PLA, PLA_CR, CR_RE | CR_TE);
break;
@@ -3458,7 +3464,7 @@ static void rtl_eee_plus_en(struct r8152 *tp, bool enable)
static void rtl_set_eee_plus(struct r8152 *tp)
{
- if (tp->version == RTL_VER_17)
+ if (tp->version == RTL_VER_17_QFN68 || tp->version == RTL_VER_17_QFN100)
return rtl_eee_plus_en(tp, false);
if (rtl8152_get_speed(tp) & _10bps)
@@ -3646,7 +3652,8 @@ static void r8153_set_rx_early_timeout(struct r8152 *tp)
case RTL_VER_13:
case RTL_VER_15:
case RTL_VER_16:
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_TIMEOUT,
640 / 8);
ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EXTRA_AGGR_TMR,
@@ -3691,7 +3698,8 @@ static void r8153_set_rx_early_size(struct r8152 *tp)
ocp_data / 8);
break;
case RTL_VER_16:
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE,
ocp_data / 16);
break;
@@ -3807,6 +3815,8 @@ static void rtl_rx_vlan_en(struct r8152 *tp, bool enable)
case RTL_VER_13:
case RTL_VER_15:
case RTL_VER_16:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
default:
if (enable)
ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_RCR1,
@@ -4487,6 +4497,8 @@ static void r8153_teredo_off(struct r8152 *tp)
case RTL_VER_14:
case RTL_VER_15:
case RTL_VER_16:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
default:
/* The bit 0 ~ 7 are relative with teredo settings. They are
* W1C (write 1 to clear), so set all 1 to disable it.
@@ -4541,7 +4553,8 @@ static void rtl_clear_bp(struct r8152 *tp, u16 type)
break;
case RTL_VER_14:
case RTL_VER_16:
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
default:
ocp_write_word(tp, type, USB_BP2_EN, 0);
bp_num = 16;
@@ -4653,7 +4666,8 @@ static bool rtl8152_is_fw_phy_speed_up_ok(struct r8152 *tp, struct fw_phy_speed_
case RTL_VER_13:
case RTL_VER_15:
case RTL_VER_16:
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
default:
break;
}
@@ -5813,7 +5827,8 @@ static void rtl_eee_enable(struct r8152 *tp, bool enable)
case RTL_VER_13:
case RTL_VER_15:
case RTL_VER_16:
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
if (enable) {
r8156_eee_en(tp, true);
ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
@@ -6404,7 +6419,7 @@ static int rtl8156_enable(struct r8152 *tp)
set_tx_qlen(tp);
rtl_set_eee_plus(tp);
- if (tp->version >= RTL_VER_12 && tp->version <= RTL_VER_17)
+ if (tp->version >= RTL_VER_12 && tp->version <= RTL_VER_17_QFN100)
ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_RX_AGGR_NUM, RX_AGGR_NUM_MASK);
r8153_set_rx_early_timeout(tp);
@@ -8093,7 +8108,8 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
sram2_write_w0w1(tp, 0x809d, 0xff00, 0x5000);
break;
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
/* Disable bypass turn off clk in ALDPS */
ocp_byte_clr_bits(tp, MCU_TYPE_PLA, 0xd3c8, BIT(0));
@@ -8499,7 +8515,8 @@ static void r8157_init(struct r8152 *tp)
return;
switch (tp->version) {
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
if (r8159_wait_backup_restore(tp)) {
rtl_set_inaccessible(tp);
dev_err(&tp->intf->dev,
@@ -8546,7 +8563,8 @@ static void r8157_init(struct r8152 *tp)
ocp_word_clr_bits(tp, MCU_TYPE_USB, 0xcd06, BIT(11));
switch (tp->version) {
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
/* Disable FW u1u2 patch option */
ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xb9a6, BIT(0));
break;
@@ -8584,7 +8602,11 @@ static void r8157_init(struct r8152 *tp)
ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd4ae, BIT(1));
switch (tp->version) {
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ /* Enable u2phy backup restore patch */
+ ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xb99c, BIT(0));
+ fallthrough;
+ case RTL_VER_17_QFN100:
/* Enable u3phy patch backup */
ocp_write_word(tp, MCU_TYPE_USB, 0xb9a2, 0x0448);
break;
@@ -9973,7 +9995,8 @@ static int rtl_ops_init(struct r8152 *tp)
r8157_desc_init(tp);
break;
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
tp->eee_en = true;
tp->eee_adv = MDIO_EEE_100TX | MDIO_EEE_1000T | MDIO_EEE_10GT;
tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
@@ -10065,7 +10088,8 @@ static int rtl_fw_init(struct r8152 *tp)
case RTL_VER_16:
rtl_fw->fw_name = FIRMWARE_8157_1;
break;
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
rtl_fw->fw_name = FIRMWARE_8159_1;
break;
default:
@@ -10075,9 +10099,38 @@ static int rtl_fw_init(struct r8152 *tp)
return 0;
}
+static u32 __rtl_get_pkg_det(struct usb_device *udev)
+{
+ u32 pkg_det = 0;
+ __le32 *tmp;
+ int ret, i;
+
+ tmp = kmalloc_obj(*tmp);
+ if (!tmp)
+ return 0;
+
+ for (i = 0, ret = 0; i < 3 && ret != 4; i++)
+ ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+ RTL8152_REQ_GET_REGS, RTL8152_REQT_READ,
+ PLA_PKG_DET, MCU_TYPE_PLA, tmp,
+ sizeof(*tmp), USB_CTRL_GET_TIMEOUT);
+
+ if (ret == sizeof(*tmp)) {
+ if (i != 1)
+ dev_warn(&udev->dev,
+ "Needed %d retries to read package\n", i - 1);
+
+ pkg_det = __le32_to_cpu(*tmp) & PKG_MASK;
+ }
+
+ kfree(tmp);
+ return pkg_det;
+}
+
static u8 __rtl_get_hw_ver(struct usb_device *udev)
{
u32 ocp_data = 0;
+ u32 pkg_det = 0;
__le32 *tmp;
u8 version;
int ret;
@@ -10160,7 +10213,16 @@ static u8 __rtl_get_hw_ver(struct usb_device *udev)
version = RTL_VER_16;
break;
case 0x2020:
- version = RTL_VER_17;
+ pkg_det = __rtl_get_pkg_det(udev);
+ if (pkg_det == 0x1e || pkg_det == 0x1c) {
+ version = RTL_VER_17_QFN68;
+ } else if (pkg_det == 0x18 || pkg_det == 0x1a) {
+ version = RTL_VER_17_QFN100;
+ } else {
+ version = RTL_VER_UNKNOWN;
+ dev_info(&udev->dev, "Unknown package 0x%04x\n",
+ pkg_det);
+ }
break;
default:
version = RTL_VER_UNKNOWN;
@@ -10319,7 +10381,8 @@ static int rtl8152_probe_once(struct usb_interface *intf,
case RTL_VER_13:
case RTL_VER_15:
case RTL_VER_16:
- case RTL_VER_17:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
netdev->max_mtu = size_to_mtu(16 * 1024);
break;
case RTL_VER_01:
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH net-next v7 04/10] r8152: split rtl8156_enable/up/down into per-generation functions
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
` (2 preceding siblings ...)
2026-10-14 8:18 ` [PATCH net-next v7 03/10] r8152: split RTL_VER_17 into QFN68 and QFN100 package variants Chih Kai Hsu
@ 2026-10-14 8:18 ` Chih Kai Hsu
2026-10-14 8:18 ` [PATCH net-next v7 05/10] r8152: fix up and down register settings for RTL8156/8156B/8157/8159 Chih Kai Hsu
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:18 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
rtl8156_enable(), rtl8156_up() and rtl8156_down() had accumulated
tp->version range checks to cover RTL8156, RTL8156B, and
RTL8157/RTL8159, including an early bail-out in rtl8156_enable()
that skipped most of the function body for RTL8157/RTL8159. The
result hides which settings belong to which chip, so chip-specific
register differences cannot be reviewed, and cannot be added without
touching the other families.
Give each generation its own ops callback: rtl8156_enable() for
VER_10/11/12/13/15, and rtl8157_enable()/rtl8157_up()/rtl8157_down()
for VER_16/17. rtl8156_up()/rtl8156_down() keep serving RTL8156 and
RTL8156B (VER_10/11/12/13/15).
RTL8156/8156B/8157/8159 share most of the up/down sequence, so move
it into common helpers, as RTL8152B/8153 do with exit_oob/enter_oob.
Each extracted function reproduces what its chips already did, with
the guards that were true for them inlined and the ones that were
false dropped as unreachable.
No functional change intended.
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 166 +++++++++++++++++++++++++++++-----------
1 file changed, 123 insertions(+), 43 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 25fce499f7938..b245b63fb6aaf 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -6413,14 +6413,28 @@ static int rtl8156_enable(struct r8152 *tp)
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
return -ENODEV;
- if (tp->version < RTL_VER_12)
+ switch (tp->version) {
+ case RTL_TEST_01:
+ case RTL_VER_10:
+ case RTL_VER_11:
r8156_fc_parameter(tp);
+ break;
+ default:
+ break;
+ }
set_tx_qlen(tp);
rtl_set_eee_plus(tp);
- if (tp->version >= RTL_VER_12 && tp->version <= RTL_VER_17_QFN100)
+ switch (tp->version) {
+ case RTL_VER_12:
+ case RTL_VER_13:
+ case RTL_VER_15:
ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_RX_AGGR_NUM, RX_AGGR_NUM_MASK);
+ break;
+ default:
+ break;
+ }
r8153_set_rx_early_timeout(tp);
r8153_set_rx_early_size(tp);
@@ -6428,9 +6442,6 @@ static int rtl8156_enable(struct r8152 *tp)
speed = rtl8152_get_speed(tp);
rtl_set_ifg(tp, speed);
- if (tp->version >= RTL_VER_16)
- return rtl_enable(tp);
-
if (speed & _2500bps)
ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4,
IDLE_SPDWN_EN);
@@ -6438,11 +6449,17 @@ static int rtl8156_enable(struct r8152 *tp)
ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4,
IDLE_SPDWN_EN);
- if (tp->version < RTL_VER_12) {
+ switch (tp->version) {
+ case RTL_TEST_01:
+ case RTL_VER_10:
+ case RTL_VER_11:
if (speed & _1000bps)
ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_TXTWSYS, 0x11);
else if (speed & _500bps)
ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_TXTWSYS, 0x3d);
+ break;
+ default:
+ break;
}
if (tp->udev->speed == USB_SPEED_HIGH) {
@@ -6468,6 +6485,27 @@ static void rtl8156_disable(struct r8152 *tp)
rtl8153_disable(tp);
}
+static int rtl8157_enable(struct r8152 *tp)
+{
+ u16 speed;
+
+ if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
+ return -ENODEV;
+
+ set_tx_qlen(tp);
+ rtl_set_eee_plus(tp);
+
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_RX_AGGR_NUM, RX_AGGR_NUM_MASK);
+
+ r8153_set_rx_early_timeout(tp);
+ r8153_set_rx_early_size(tp);
+
+ speed = rtl8152_get_speed(tp);
+ rtl_set_ifg(tp, speed);
+
+ return rtl_enable(tp);
+}
+
static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
u32 advertising)
{
@@ -6822,16 +6860,8 @@ static void rtl8156_change_mtu(struct r8152 *tp)
ALIGN(rx_max_size + tp->tx_desc.size, 1024) / 16);
}
-static void rtl8156_up(struct r8152 *tp)
+static void r8156_exit_oob(struct r8152 *tp)
{
- if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
- return;
-
- r8153b_u1u2en(tp, false);
- if (tp->version < RTL_VER_16)
- r8153_u2p3en(tp, false);
- r8153_aldps_en(tp, false);
-
rxdy_gated_en(tp, true);
r8153_teredo_off(tp);
@@ -6864,12 +6894,23 @@ static void rtl8156_up(struct r8152 *tp)
/* share FIFO settings */
ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_RXFIFO_FULL, RXFIFO_FULL_MASK,
0x08);
+}
+
+static void rtl8156_up(struct r8152 *tp)
+{
+ if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
+ return;
+
+ r8153b_u1u2en(tp, false);
+ r8153_u2p3en(tp, false);
+ r8153_aldps_en(tp, false);
+
+ r8156_exit_oob(tp);
ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, PLA_MCU_SPDWN_EN);
- if (tp->version < RTL_VER_16)
- ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_SPEED_OPTION,
- RG_PWRDN_EN | ALL_SPEED_OFF);
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_SPEED_OPTION,
+ RG_PWRDN_EN | ALL_SPEED_OFF);
ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH, 0x00600400);
@@ -6879,30 +6920,14 @@ static void rtl8156_up(struct r8152 *tp)
}
r8153_aldps_en(tp, true);
- if (tp->version < RTL_VER_16)
- r8153_u2p3en(tp, true);
+ r8153_u2p3en(tp, true);
- if (tp->version < RTL_VER_16 && tp->udev->speed >= USB_SPEED_SUPER)
+ if (tp->udev->speed >= USB_SPEED_SUPER)
r8153b_u1u2en(tp, true);
}
-static void rtl8156_down(struct r8152 *tp)
+static void r8156_enter_oob(struct r8152 *tp)
{
- if (test_bit(RTL8152_INACCESSIBLE, &tp->flags)) {
- rtl_drop_queued_tx(tp);
- return;
- }
-
- ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
- PLA_MCU_SPDWN_EN);
-
- r8153b_u1u2en(tp, false);
- if (tp->version < RTL_VER_16) {
- r8153_u2p3en(tp, false);
- r8153b_power_cut_en(tp, false);
- }
- r8153_aldps_en(tp, false);
-
ocp_byte_clr_bits(tp, MCU_TYPE_PLA, PLA_OOB_CTRL, NOW_IS_OOB);
/* RX FIFO settings for OOB */
@@ -6931,7 +6956,62 @@ static void rtl8156_down(struct r8152 *tp)
ocp_dword_set_bits(tp, MCU_TYPE_PLA, PLA_RCR,
RCR_APM | RCR_AM | RCR_AB);
+}
+static void rtl8156_down(struct r8152 *tp)
+{
+ if (test_bit(RTL8152_INACCESSIBLE, &tp->flags)) {
+ rtl_drop_queued_tx(tp);
+ return;
+ }
+
+ ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
+ PLA_MCU_SPDWN_EN);
+
+ r8153b_u1u2en(tp, false);
+ r8153_u2p3en(tp, false);
+ r8153b_power_cut_en(tp, false);
+ r8153_aldps_en(tp, false);
+ r8156_enter_oob(tp);
+ r8153_aldps_en(tp, true);
+}
+
+static void rtl8157_up(struct r8152 *tp)
+{
+ if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
+ return;
+
+ r8153b_u1u2en(tp, false);
+ r8153_aldps_en(tp, false);
+
+ r8156_exit_oob(tp);
+
+ ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
+ PLA_MCU_SPDWN_EN);
+
+ ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH, 0x00600400);
+
+ if (tp->saved_wolopts != __rtl_get_wol(tp)) {
+ netif_warn(tp, ifup, tp->netdev, "wol setting is changed\n");
+ __rtl_set_wol(tp, tp->saved_wolopts);
+ }
+
+ r8153_aldps_en(tp, true);
+}
+
+static void rtl8157_down(struct r8152 *tp)
+{
+ if (test_bit(RTL8152_INACCESSIBLE, &tp->flags)) {
+ rtl_drop_queued_tx(tp);
+ return;
+ }
+
+ ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
+ PLA_MCU_SPDWN_EN);
+
+ r8153b_u1u2en(tp, false);
+ r8153_aldps_en(tp, false);
+ r8156_enter_oob(tp);
r8153_aldps_en(tp, true);
}
@@ -9978,10 +10058,10 @@ static int rtl_ops_init(struct r8152 *tp)
tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
ops->init = r8157_init;
- ops->enable = rtl8156_enable;
+ ops->enable = rtl8157_enable;
ops->disable = rtl8153_disable;
- ops->up = rtl8156_up;
- ops->down = rtl8156_down;
+ ops->up = rtl8157_up;
+ ops->down = rtl8157_down;
ops->unload = rtl8153_unload;
ops->eee_get = r8153_get_eee;
ops->eee_set = r8152_set_eee;
@@ -10001,10 +10081,10 @@ static int rtl_ops_init(struct r8152 *tp)
tp->eee_adv = MDIO_EEE_100TX | MDIO_EEE_1000T | MDIO_EEE_10GT;
tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
ops->init = r8157_init;
- ops->enable = rtl8156_enable;
+ ops->enable = rtl8157_enable;
ops->disable = rtl8153_disable;
- ops->up = rtl8156_up;
- ops->down = rtl8156_down;
+ ops->up = rtl8157_up;
+ ops->down = rtl8157_down;
ops->unload = rtl8153_unload;
ops->eee_get = r8153_get_eee;
ops->eee_set = r8152_set_eee;
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH net-next v7 05/10] r8152: fix up and down register settings for RTL8156/8156B/8157/8159
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
` (3 preceding siblings ...)
2026-10-14 8:18 ` [PATCH net-next v7 04/10] r8152: split rtl8156_enable/up/down into per-generation functions Chih Kai Hsu
@ 2026-10-14 8:18 ` Chih Kai Hsu
2026-10-21 0:52 ` Jakub Kicinski
2026-10-14 8:19 ` [PATCH net-next v7 06/10] r8152: split r8157_hw_phy_cfg into RTL8157 and RTL8159 variants Chih Kai Hsu
` (5 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:18 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
The per-generation split was kept behaviour-neutral, so the new
callbacks still carry the settings that the shared functions applied
under tp->version range checks. Several of those were wrong or
missing for the chips that now have functions of their own, and were
invisible as long as one function served four chip families.
Per the datasheet:
- RTL8156/8156B need PLA_RCR1 BIT(3) cleared on up. The shared
function had this clear under "version >= RTL_VER_16", so the
RTL8156/8156B path never reached it.
- RTL8156B (VER_13/15) need Clear_SDR enabled on up, which nothing
ever did for them.
- RTL8157/8159 need USB_SPEED_OPTION's RG_PWRDN_EN and ALL_SPEED_OFF
cleared on up. The shared function had this under
"version < RTL_VER_16", which excluded precisely the chips that
need it.
- RTL8157/8159 need their own Clear_SDR enables on up: USB 0xd378
BIT(7) set and USB 0xcd06 BIT(15) cleared.
- Entering OOB on RTL8156/8156B/8157/8159 also needs PLA_BDC_CR's
ALDPS_PROXY_MODE and PLA_OOB_CTRL's DIS_MCU_CLROOB. Only
r8152b_enter_oob() and r8153_enter_oob() set these today; the
RTL8156 family down paths set NOW_IS_OOB alone.
- RTL8157/8159 OOB RX settings are PLA_RMS 1526 and PLA_MTPS 10K,
not the 1522 and 6K they inherited from the shared function.
PLA_MCU_SPDWN_EN is not used by RTL8157/8159, so drop the up/down
management of it that rtl8157_up()/rtl8157_down() carried over;
keeping it would suggest the bit is meaningful on these chips.
Fixes: 195aae321c82 ("r8152: support new chips")
Fixes: fd3c7d080df5 ("r8152: Add support for the RTL8157 hardware")
Fixes: ad2a55b63b41 ("r8152: Add support for the RTL8159 chip")
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index b245b63fb6aaf..045460a35f383 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -6872,8 +6872,7 @@ static void r8156_exit_oob(struct r8152 *tp)
ocp_byte_clr_bits(tp, MCU_TYPE_PLA, PLA_OOB_CTRL, NOW_IS_OOB);
- if (tp->version >= RTL_VER_16)
- ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR1, BIT(3));
+ ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR1, BIT(3));
ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_SFF_STS_7, MCU_BORW_EN);
@@ -6938,8 +6937,18 @@ static void r8156_enter_oob(struct r8152 *tp)
rtl_disable(tp);
rtl_reset_bmu(tp);
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, 1522);
- ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, MTPS_DEFAULT);
+ switch (tp->version) {
+ case RTL_VER_16:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, 1526);
+ ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, 10 * 1024 / 64);
+ break;
+ default:
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, 1522);
+ ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, MTPS_DEFAULT);
+ break;
+ }
/* Clear teredo wake event. bit[15:8] is the teredo wakeup
* type. Set it to zero. bits[7:0] are the W1C bits about
@@ -6947,7 +6956,10 @@ static void r8156_enter_oob(struct r8152 *tp)
*/
ocp_write_word(tp, MCU_TYPE_PLA, PLA_TEREDO_WAKE_BASE, 0x00ff);
- ocp_byte_set_bits(tp, MCU_TYPE_PLA, PLA_OOB_CTRL, NOW_IS_OOB);
+ ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_BDC_CR, ALDPS_PROXY_MODE);
+
+ ocp_byte_set_bits(tp, MCU_TYPE_PLA, PLA_OOB_CTRL,
+ NOW_IS_OOB | DIS_MCU_CLROOB);
ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_SFF_STS_7, MCU_BORW_EN);
@@ -6983,11 +6995,10 @@ static void rtl8157_up(struct r8152 *tp)
r8153b_u1u2en(tp, false);
r8153_aldps_en(tp, false);
-
r8156_exit_oob(tp);
- ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
- PLA_MCU_SPDWN_EN);
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_SPEED_OPTION,
+ RG_PWRDN_EN | ALL_SPEED_OFF);
ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH, 0x00600400);
@@ -6997,6 +7008,11 @@ static void rtl8157_up(struct r8152 *tp)
}
r8153_aldps_en(tp, true);
+
+ /* Enable Clear_SDR */
+ ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xd378, BIT(7));
+ /* Clear SDR bit */
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, 0xcd06, BIT(15));
}
static void rtl8157_down(struct r8152 *tp)
@@ -7006,9 +7022,6 @@ static void rtl8157_down(struct r8152 *tp)
return;
}
- ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
- PLA_MCU_SPDWN_EN);
-
r8153b_u1u2en(tp, false);
r8153_aldps_en(tp, false);
r8156_enter_oob(tp);
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v7 05/10] r8152: fix up and down register settings for RTL8156/8156B/8157/8159
2026-10-14 8:18 ` [PATCH net-next v7 05/10] r8152: fix up and down register settings for RTL8156/8156B/8157/8159 Chih Kai Hsu
@ 2026-10-21 0:52 ` Jakub Kicinski
0 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2026-10-21 0:52 UTC (permalink / raw)
To: hsu.chih.kai
Cc: Jakub Kicinski, davem, netdev, nic_swsd, linux-kernel, linux-usb,
edumazet, bjorn, pabeni, andrew+netdev, mail
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: fix up and down register settings for RTL8156/8156B/8157/8159
This commit corrects several up/down and OOB register settings for the
RTL8156, RTL8156B, RTL8157 and RTL8159 chips, following the earlier
per-generation split of the up/down callbacks. It changes the PLA_RCR1,
USB_SPEED_OPTION, Clear_SDR, PLA_BDC_CR/PLA_OOB_CTRL and OOB
PLA_RMS/PLA_MTPS settings, and drops PLA_MCU_SPDWN_EN handling from
rtl8157_up()/rtl8157_down().
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index b245b63fb6aaf..045460a35f383 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[ ... ]
> @@ -6938,8 +6937,18 @@ static void r8156_enter_oob(struct r8152 *tp)
> rtl_disable(tp);
> rtl_reset_bmu(tp);
>
> - ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, 1522);
> - ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, MTPS_DEFAULT);
> + switch (tp->version) {
> + case RTL_VER_16:
> + case RTL_VER_17_QFN68:
> + case RTL_VER_17_QFN100:
[Severity: Medium]
This patch has three Fixes: tags:
Fixes: 195aae321c82 ("r8152: support new chips")
Fixes: fd3c7d080df5 ("r8152: Add support for the RTL8157 hardware")
Fixes: ad2a55b63b41 ("r8152: Add support for the RTL8159 chip")
Will it apply and build on the trees those tags point to?
RTL_VER_17_QFN68 and RTL_VER_17_QFN100 come from an earlier patch in this
series, "r8152: split RTL_VER_17 into QFN68 and QFN100 package variants".
Before that patch the enum only has RTL_VER_17.
rtl8157_up() and rtl8157_down() also only exist after "r8152: split
rtl8156_enable/up/down into per-generation functions", which is described
as no functional change. Before that patch there is one shared
rtl8156_up() with tp->version >= RTL_VER_16 and tp->version < RTL_VER_16
checks.
So a stable backport would either conflict or need both non-fix patches
as prerequisites. One of them is the package detection change, which
makes an unrecognised PLA_PKG_DET value fall back to RTL_VER_UNKNOWN.
Could these fixes go to net separately, written against the pre-refactor
code? Or could the commit message at least name the prerequisites?
> + ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, 1526);
> + ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, 10 * 1024 / 64);
> + break;
[ ... ]
> @@ -6997,6 +7008,11 @@ static void rtl8157_up(struct r8152 *tp)
> }
>
> r8153_aldps_en(tp, true);
> +
> + /* Enable Clear_SDR */
> + ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xd378, BIT(7));
> + /* Clear SDR bit */
> + ocp_word_clr_bits(tp, MCU_TYPE_USB, 0xcd06, BIT(15));
> }
[Severity: High]
The commit message says:
- RTL8156B (VER_13/15) need Clear_SDR enabled on up, which nothing
ever did for them.
Is the RTL8156B part missing from this patch? These are the only
Clear_SDR writes the patch adds. rtl8157_up() is only used for
RTL_VER_16, RTL_VER_17_QFN68 and RTL_VER_17_QFN100.
In rtl_ops_init(), RTL_VER_13 and RTL_VER_15 are bound to rtl8156_up():
case RTL_VER_12:
case RTL_VER_13:
tp->support_2500full = 1;
fallthrough;
case RTL_VER_15:
...
ops->up = rtl8156_up;
This patch does not change rtl8156_up(), and it still writes no
Clear_SDR register:
r8156_exit_oob(tp);
ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, PLA_MCU_SPDWN_EN);
ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_SPEED_OPTION,
RG_PWRDN_EN | ALL_SPEED_OFF);
ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH, 0x00600400);
...
r8153_aldps_en(tp, true);
r8153_u2p3en(tp, true);
So every open or resume on an RTL8156B goes through ops->up ->
rtl8156_up() without the Clear_SDR enable. The later patches in the
series don't add it to rtl8156_up() either.
Should rtl8156_up() get the RTL8156B Clear_SDR writes under an
RTL_VER_13/RTL_VER_15 check, or should this bullet come out of the commit
message? The 0xd378/0xcd06 writes used for RTL8157/8159 can't be assumed
to apply to RTL8156B.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v7 06/10] r8152: split r8157_hw_phy_cfg into RTL8157 and RTL8159 variants
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
` (4 preceding siblings ...)
2026-10-14 8:18 ` [PATCH net-next v7 05/10] r8152: fix up and down register settings for RTL8156/8156B/8157/8159 Chih Kai Hsu
@ 2026-10-14 8:19 ` Chih Kai Hsu
2026-10-14 8:19 ` [PATCH net-next v7 07/10] r8152: add rtl8157_unload and rtl8157_change_mtu Chih Kai Hsu
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:19 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
r8157_hw_phy_cfg() served both RTL8157 (VER_16) and RTL8159
(VER_17_QFN68/QFN100) via a tp->version switch inside a single
function, but per the datasheet RTL8159's PHY tuning differs from
RTL8157's in more than the per-version blocks already captured.
Split it into r8157_hw_phy_cfg() (VER_16 only) and a new
r8159_hw_phy_cfg() (VER_17_QFN68/QFN100), each with its own copy of
the shared prologue/epilogue. rtl_ops_init() assigns each one to its
own chips only. Per the datasheet, RTL8159 diverges from RTL8157 in
this split in several ways:
- RTL8159 doesn't need EEE disabled before reprogramming PHY
parameters, so r8159_hw_phy_cfg() drops the rtl_eee_enable(tp,
false) call that r8157_hw_phy_cfg() keeps.
- Since EEE isn't forced off up front, r8159_hw_phy_cfg() ends with
rtl_eee_enable(tp, tp->eee_en) instead of only enabling EEE when
eee_en is set, so the configured state is applied in both
directions.
- RTL8159's EEE clock-divider and 0xa5ea encoding differ from
RTL8157's: r8159_hw_phy_cfg() clears EN_10M_CLKDIV (ups_info.
_10m_ckdiv = false) instead of setting it, and clears an extra bit
(BIT(2)) of 0xa5ea.
- "Disable bypass_turn_off_clk_in_aldps" (0xd3c8 BIT(0)) is needed by
both chips, so it moves from the VER_17-only switch case into the
shared prologue and now also runs for RTL8157.
Newly added PHY tuning parameters for RTL8159:
- shorten CLKS latency
- CMP_Timer on MP_Timer=333
- spdchg_pga1_lpf_cap
- ENET PLL jitter improvement
Moreover, change sram2 0x8ffe[15:0] = 0xf450 to 0xf4ff.
Along the way, add sram2_set_bits()/sram2_clr_bits() helpers
mirroring the existing sram_set_bits()/sram_clr_bits(), and use the
named OCP_POWER_CFG/EN_10M_PLLOFF/EN_ALDPS_PLLOFF macros in place of
the raw "ocp_reg_set_bits(tp, 0xa430, BIT(0) | BIT(1))".
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 168 ++++++++++++++++++++++++++++++++--------
1 file changed, 137 insertions(+), 31 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 045460a35f383..c31ce65d6a586 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -655,6 +655,7 @@ enum spd_duplex {
/* OCP_POWER_CFG */
#define EEE_CLKDIV_EN 0x8000
#define EN_ALDPS 0x0004
+#define EN_ALDPS_PLLOFF 0x0002
#define EN_10M_PLLOFF 0x0001
/* OCP_EEE_CONFIG1 */
@@ -1964,6 +1965,16 @@ static void sram2_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
ocp_reg_write(tp, OCP_SRAM2_DATA, data);
}
+static void sram2_set_bits(struct r8152 *tp, u16 addr, u16 set)
+{
+ sram2_write_w0w1(tp, addr, 0, set);
+}
+
+static void sram2_clr_bits(struct r8152 *tp, u16 addr, u16 clear)
+{
+ sram2_write_w0w1(tp, addr, clear, 0);
+}
+
static void r8152_mdio_clr_bit(struct r8152 *tp, u16 addr, u16 clear)
{
int data;
@@ -8074,11 +8085,14 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_PHY_PWR, PFM_PWM_SWITCH);
/* Advanced Power Saving parameter */
- ocp_reg_set_bits(tp, 0xa430, BIT(0) | BIT(1));
+ ocp_reg_set_bits(tp, OCP_POWER_CFG, EN_10M_PLLOFF | EN_ALDPS_PLLOFF);
/* Disable ALDPS force mode */
ocp_reg_clr_bits(tp, 0xa44a, BIT(2));
+ /* Disable bypass_turn_off_clk_in_aldps */
+ ocp_byte_clr_bits(tp, MCU_TYPE_PLA, 0xd3c8, BIT(0));
+
switch (tp->version) {
case RTL_VER_16:
/* XG_INRX parameter */
@@ -8094,7 +8108,7 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
sram2_write_w0w1(tp, 0x8078, 0xff00, 0x3000);
/* green mode */
- sram2_write_w0w1(tp, 0x89e9, 0xff00, 0);
+ sram2_clr_bits(tp, 0x89e9, 0xff00);
sram2_write_w0w1(tp, 0x8ffd, 0xff00, 0x0100);
sram2_write_w0w1(tp, 0x8ffe, 0xff00, 0x0200);
sram2_write_w0w1(tp, 0x8fff, 0xff00, 0x0400);
@@ -8200,12 +8214,85 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
sram2_write_w0w1(tp, 0x807c, 0xff00, 0x5000);
sram2_write_w0w1(tp, 0x809d, 0xff00, 0x5000);
break;
+ default:
+ break;
+ }
+
+ if (rtl_phy_patch_request(tp, true, true))
+ return;
+
+ ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4, EEE_SPDWN_EN);
+
+ ocp_reg_w0w1(tp, OCP_DOWN_SPEED, EN_EEE_100 | EN_EEE_1000, EN_10M_CLKDIV);
+
+ tp->ups_info._10m_ckdiv = true;
+ tp->ups_info.eee_plloff_100 = false;
+ tp->ups_info.eee_plloff_giga = false;
+
+ ocp_reg_set_bits(tp, OCP_POWER_CFG, EEE_CLKDIV_EN);
+ tp->ups_info.eee_ckdiv = true;
+
+ rtl_phy_patch_request(tp, false, true);
+
+ rtl_green_en(tp, test_bit(GREEN_ETHERNET, &tp->flags));
+
+ ocp_reg_clr_bits(tp, 0xa428, BIT(9));
+ ocp_reg_clr_bits(tp, 0xa5ea, BIT(0) | BIT(1));
+ tp->ups_info.lite_mode = 0;
+
+ if (tp->eee_en)
+ rtl_eee_enable(tp, true);
+
+ r8153_aldps_en(tp, true);
+ r8152b_enable_fc(tp);
+
+ set_bit(PHY_RESET, &tp->flags);
+}
+
+static void r8159_hw_phy_cfg(struct r8152 *tp)
+{
+ u16 data;
+
+ r8156b_wait_loading_flash(tp);
+
+ ocp_word_test_and_clr_bits(tp, MCU_TYPE_USB, USB_MISC_0, PCUT_STATUS);
+
+ data = r8153_phy_status(tp, 0);
+ switch (data) {
+ case PHY_STAT_EXT_INIT:
+ rtl8152_apply_firmware(tp, true);
+ ocp_reg_clr_bits(tp, 0xa466, BIT(0));
+ ocp_reg_clr_bits(tp, 0xa468, BIT(3) | BIT(1));
+ break;
+ case PHY_STAT_LAN_ON:
+ case PHY_STAT_PWRDN:
+ default:
+ rtl8152_apply_firmware(tp, false);
+ break;
+ }
+
+ r8152_mdio_test_and_clr_bit(tp, MII_BMCR, BMCR_PDOWN);
+
+ r8153_aldps_en(tp, false);
+
+ data = r8153_phy_status(tp, PHY_STAT_LAN_ON);
+ WARN_ON_ONCE(data != PHY_STAT_LAN_ON);
+ /* PFM mode */
+ ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_PHY_PWR, PFM_PWM_SWITCH);
+
+ /* Advanced Power Saving parameter */
+ ocp_reg_set_bits(tp, OCP_POWER_CFG, EN_10M_PLLOFF | EN_ALDPS_PLLOFF);
+
+ /* Disable ALDPS force mode */
+ ocp_reg_clr_bits(tp, 0xa44a, BIT(2));
+
+ /* Disable bypass_turn_off_clk_in_aldps */
+ ocp_byte_clr_bits(tp, MCU_TYPE_PLA, 0xd3c8, BIT(0));
+
+ switch (tp->version) {
case RTL_VER_17_QFN68:
case RTL_VER_17_QFN100:
- /* Disable bypass turn off clk in ALDPS */
- ocp_byte_clr_bits(tp, MCU_TYPE_PLA, 0xd3c8, BIT(0));
-
/* Power level tuning
* test mode power level
*/
@@ -8215,22 +8302,35 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
sram_write_w0w1(tp, 0x81ae, 0xff00, 0x0f00);
sram_write_w0w1(tp, 0x81b9, 0xff00, 0xb900);
/* normal link TX filter */
- sram2_write_w0w1(tp, 0x83b0, 0x0e00, 0);
- sram2_write_w0w1(tp, 0x83c5, 0x0e00, 0);
- sram2_write_w0w1(tp, 0x83da, 0x0e00, 0);
- sram2_write_w0w1(tp, 0x83ef, 0x0e00, 0);
+ sram2_clr_bits(tp, 0x83b0, 0x0e00);
+ sram2_clr_bits(tp, 0x83c5, 0x0e00);
+ sram2_clr_bits(tp, 0x83da, 0x0e00);
+ sram2_clr_bits(tp, 0x83ef, 0x0e00);
+
+ ocp_reg_w0w1(tp, 0xbf38, 0x01f0, 0x0160);
+ ocp_reg_w0w1(tp, 0xbf3a, 0x001f, 0x0014);
+ /* shorten CLKS latency */
+ ocp_reg_clr_bits(tp, 0xbf28, BIT(14) | BIT(13));
+ ocp_reg_clr_bits(tp, 0xbf2c, BIT(15) | BIT(14));
+ /* CMP_Timer on MP_Timer=333
+ * GPHY OCP 0xbf28 bit[0] = 0x1
+ * GPHY OCP 0xbf28 bit[6:1] = 0x3
+ * GPHY OCP 0xbf28 bit[12:7] = 0x3
+ */
+ ocp_reg_w0w1(tp, 0xbf28, 0x1fff, 0x0187);
+ ocp_reg_w0w1(tp, 0xbf2a, 0x3f, 0x03);
/* AFE power saving for 2.5G & 5G */
sram_write(tp, 0x8173, 0x8620);
sram_write(tp, 0x8175, 0x8671);
- sram_write_w0w1(tp, 0x817c, 0, BIT(13));
- sram_write_w0w1(tp, 0x8187, 0, BIT(13));
- sram_write_w0w1(tp, 0x8192, 0, BIT(13));
- sram_write_w0w1(tp, 0x819d, 0, BIT(13));
- sram_write_w0w1(tp, 0x81a8, BIT(13), 0);
- sram_write_w0w1(tp, 0x81b3, BIT(13), 0);
- sram_write_w0w1(tp, 0x81be, 0, BIT(13));
+ sram_set_bits(tp, 0x817c, BIT(13));
+ sram_set_bits(tp, 0x8187, BIT(13));
+ sram_set_bits(tp, 0x8192, BIT(13));
+ sram_set_bits(tp, 0x819d, BIT(13));
+ sram_clr_bits(tp, 0x81a8, BIT(13));
+ sram_clr_bits(tp, 0x81b3, BIT(13));
+ sram_set_bits(tp, 0x81be, BIT(13));
sram_write_w0w1(tp, 0x817d, 0xff00, 0xa600);
sram_write_w0w1(tp, 0x8188, 0xff00, 0xa600);
@@ -8294,10 +8394,10 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
sram2_write_w0w1(tp, 0x84b2, 0xff00, 0x6000);
/* Training AAGC PAR (with uc2 patch) */
sram2_write(tp, 0x8ffc, 0x6008);
- sram2_write(tp, 0x8ffe, 0xf450);
+ sram2_write(tp, 0x8ffe, 0xf4ff);
/* DAC BGK */
- sram2_write_w0w1(tp, 0x8015, 0, BIT(9));
- sram2_write_w0w1(tp, 0x8016, 0, BIT(11));
+ sram2_set_bits(tp, 0x8015, BIT(9));
+ sram2_set_bits(tp, 0x8016, BIT(11));
sram2_write_w0w1(tp, 0x8fe6, 0xff00, 0x0800);
sram2_write(tp, 0x8fe4, 0x2114);
/* 10G PBO table */
@@ -8306,14 +8406,14 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
sram2_write_w0w1(tp, 0x864b, 0xff00, 0xdc00);
/* 2.5G ado power window size */
sram2_write_w0w1(tp, 0x8154, 0xc000, 0x4000);
- sram2_write_w0w1(tp, 0x8158, 0xc000, 0);
+ sram2_clr_bits(tp, 0x8158, 0xc000);
/* 10G lock far */
sram2_write(tp, 0x826c, 0xffff);
sram2_write(tp, 0x826e, 0xffff);
/* XG INRX parameter */
sram2_write_w0w1(tp, 0x8872, 0xff00, 0x0e00);
- sram_write_w0w1(tp, 0x8012, 0, BIT(11));
- sram_write_w0w1(tp, 0x8012, 0, BIT(14));
+ sram_set_bits(tp, 0x8012, BIT(11));
+ sram_set_bits(tp, 0x8012, BIT(14));
ocp_reg_set_bits(tp, 0xb576, BIT(0));
sram_write_w0w1(tp, 0x834a, 0xff00, 0x0700);
sram2_write_w0w1(tp, 0x8217, 0x3f00, 0x2a00);
@@ -8324,7 +8424,7 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
/* improve UBE */
ocp_reg_set_bits(tp, 0xbf0c, 0x7 << 11);
/* close Sparse NEC, improve connect 5EUU cable performance */
- sram2_write_w0w1(tp, 0x88de, 0xff00, 0);
+ sram2_clr_bits(tp, 0x88de, 0xff00);
/* 5G slave compatibility issue */
sram2_write(tp, 0x80b4, 0x5195);
@@ -8383,8 +8483,15 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
sram2_write(tp, 0x8ff8, 0xaa5a);
sram2_write_w0w1(tp, 0x88d5, 0xff00, 0x0200);
- break;
+ /* spdchg_pga1_lpf_cap */
+ sram_write_w0w1(tp, 0x84bb, 0xff00, 0x0a00);
+ sram_write_w0w1(tp, 0x84c0, 0xff00, 0x1600);
+
+ /* ENET PLL jitter improvement */
+ ocp_reg_w0w1(tp, 0xbf8a, 0xfc00, 0x2000);
+ ocp_reg_set_bits(tp, 0xbf88, BIT(2));
+ break;
default:
break;
}
@@ -8394,9 +8501,9 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4, EEE_SPDWN_EN);
- ocp_reg_w0w1(tp, OCP_DOWN_SPEED, EN_EEE_100 | EN_EEE_1000, EN_10M_CLKDIV);
-
- tp->ups_info._10m_ckdiv = true;
+ ocp_reg_clr_bits(tp, OCP_DOWN_SPEED,
+ EN_EEE_100 | EN_EEE_1000 | EN_10M_CLKDIV);
+ tp->ups_info._10m_ckdiv = false;
tp->ups_info.eee_plloff_100 = false;
tp->ups_info.eee_plloff_giga = false;
@@ -8408,11 +8515,10 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
rtl_green_en(tp, test_bit(GREEN_ETHERNET, &tp->flags));
ocp_reg_clr_bits(tp, 0xa428, BIT(9));
- ocp_reg_clr_bits(tp, 0xa5ea, BIT(0) | BIT(1));
+ ocp_reg_clr_bits(tp, 0xa5ea, BIT(0) | BIT(1) | BIT(2));
tp->ups_info.lite_mode = 0;
- if (tp->eee_en)
- rtl_eee_enable(tp, true);
+ rtl_eee_enable(tp, tp->eee_en);
r8153_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -10102,7 +10208,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->eee_get = r8153_get_eee;
ops->eee_set = r8152_set_eee;
ops->in_nway = rtl8153_in_nway;
- ops->hw_phy_cfg = r8157_hw_phy_cfg;
+ ops->hw_phy_cfg = r8159_hw_phy_cfg;
ops->autosuspend_en = rtl8157_runtime_enable;
ops->change_mtu = rtl8156_change_mtu;
tp->rx_buf_sz = 48 * 1024;
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH net-next v7 07/10] r8152: add rtl8157_unload and rtl8157_change_mtu
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
` (5 preceding siblings ...)
2026-10-14 8:19 ` [PATCH net-next v7 06/10] r8152: split r8157_hw_phy_cfg into RTL8157 and RTL8159 variants Chih Kai Hsu
@ 2026-10-14 8:19 ` Chih Kai Hsu
2026-10-14 8:19 ` [PATCH net-next v7 08/10] r8152: add TGPHY register access for RTL8157 and RTL8159 Chih Kai Hsu
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:19 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
RTL8157 and RTL8159 used rtl8153_unload() and rtl8156_change_mtu(),
which program power-cut and MTPS the way earlier chips require. Per
the datasheet, RTL8157/RTL8159 need r8157_power_cut_en() (already
used by r8157_init()) instead of r8153_power_cut_en(),
and an MTPS scaled from the configured MTU (10K-16K, max_pkt_size /
64 capped at 0xff) instead of a fixed MTPS_JUMBO.
Add rtl8157_unload(), used only by RTL8157/RTL8159: calls
r8157_power_cut_en(tp, false), then the Interrupt Mitigation disable
that rtl8153_unload() used to gate on "tp->version >= RTL_VER_16"
unconditionally, since this function now only runs for those
versions.
Add rtl8157_change_mtu(), also RTL8157/RTL8159-only, and wire it up
via ops->change_mtu in place of rtl8156_change_mtu().
Fixes: fd3c7d080df5 ("r8152: Add support for the RTL8157 hardware")
Fixes: ad2a55b63b41 ("r8152: Add support for the RTL8159 chip")
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 47 ++++++++++++++++++++++++++++++++---------
1 file changed, 37 insertions(+), 10 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index c31ce65d6a586..558ccb4682c27 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -6889,7 +6889,7 @@ static void r8156_exit_oob(struct r8152 *tp)
rtl_rx_vlan_en(tp, tp->netdev->features & NETIF_F_HW_VLAN_CTAG_RX);
- rtl8156_change_mtu(tp);
+ tp->rtl_ops.change_mtu(tp);
switch (tp->version) {
case RTL_TEST_01:
@@ -6999,6 +6999,28 @@ static void rtl8156_down(struct r8152 *tp)
r8153_aldps_en(tp, true);
}
+static void rtl8157_change_mtu(struct r8152 *tp)
+{
+ u32 max_pkt_size = mtu_to_size(tp->netdev->mtu);
+ u32 ocp_data;
+
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, max_pkt_size);
+
+ /* Use at least 10K for MTPS */
+ ocp_data = max_t(u32, max_pkt_size, 10 * 1024) / 64;
+
+ /* 16 * 1024 / 64 = 0x100, so the max is 0xff for 8 bits data */
+ ocp_data = min_t(u32, ocp_data, 0xff);
+
+ ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, ocp_data);
+ r8156_fc_parameter(tp);
+
+ /* TX share fifo free credit full threshold */
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_TXFIFO_CTRL, 512 / 64);
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_TXFIFO_FULL,
+ ALIGN(max_pkt_size + tp->tx_desc.size, 1024) / 16);
+}
+
static void rtl8157_up(struct r8152 *tp)
{
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
@@ -9988,11 +10010,6 @@ static void rtl8153_unload(struct r8152 *tp)
return;
r8153_power_cut_en(tp, false);
-
- if (tp->version >= RTL_VER_16) {
- /* Disable Interrupt Mitigation */
- ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xcf04, BIT(0) | BIT(1) | BIT(2) | BIT(7));
- }
}
static void rtl8153b_unload(struct r8152 *tp)
@@ -10003,6 +10020,16 @@ static void rtl8153b_unload(struct r8152 *tp)
r8153b_power_cut_en(tp, false);
}
+static void rtl8157_unload(struct r8152 *tp)
+{
+ if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
+ return;
+
+ r8157_power_cut_en(tp, false);
+ ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xcf04,
+ BIT(0) | BIT(1) | BIT(2) | BIT(7));
+}
+
static int r8152_desc_init(struct r8152 *tp)
{
tp->rx_desc.size = sizeof(struct rx_desc);
@@ -10181,13 +10208,13 @@ static int rtl_ops_init(struct r8152 *tp)
ops->disable = rtl8153_disable;
ops->up = rtl8157_up;
ops->down = rtl8157_down;
- ops->unload = rtl8153_unload;
+ ops->unload = rtl8157_unload;
ops->eee_get = r8153_get_eee;
ops->eee_set = r8152_set_eee;
ops->in_nway = rtl8153_in_nway;
ops->hw_phy_cfg = r8157_hw_phy_cfg;
ops->autosuspend_en = rtl8157_runtime_enable;
- ops->change_mtu = rtl8156_change_mtu;
+ ops->change_mtu = rtl8157_change_mtu;
tp->rx_buf_sz = 32 * 1024;
tp->support_2500full = 1;
tp->support_5000full = 1;
@@ -10204,13 +10231,13 @@ static int rtl_ops_init(struct r8152 *tp)
ops->disable = rtl8153_disable;
ops->up = rtl8157_up;
ops->down = rtl8157_down;
- ops->unload = rtl8153_unload;
+ ops->unload = rtl8157_unload;
ops->eee_get = r8153_get_eee;
ops->eee_set = r8152_set_eee;
ops->in_nway = rtl8153_in_nway;
ops->hw_phy_cfg = r8159_hw_phy_cfg;
ops->autosuspend_en = rtl8157_runtime_enable;
- ops->change_mtu = rtl8156_change_mtu;
+ ops->change_mtu = rtl8157_change_mtu;
tp->rx_buf_sz = 48 * 1024;
tp->support_2500full = 1;
tp->support_5000full = 1;
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH net-next v7 08/10] r8152: add TGPHY register access for RTL8157 and RTL8159
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
` (6 preceding siblings ...)
2026-10-14 8:19 ` [PATCH net-next v7 07/10] r8152: add rtl8157_unload and rtl8157_change_mtu Chih Kai Hsu
@ 2026-10-14 8:19 ` Chih Kai Hsu
2026-10-21 0:52 ` Jakub Kicinski
2026-10-14 8:19 ` [PATCH net-next v7 09/10] r8152: extract rtl_fc_pause_pkt_en() and apply it to RTL8156/8157/8159 Chih Kai Hsu
` (2 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:19 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
RTL8157 and RTL8159 can reach PHY registers through the same
OCP-base-switching path as earlier chips, but they also provide a
TGPHY command channel (USB_TGPHY_ADDR/DATA/CMD) that needs no base
switching. Use it for these chips.
Make ocp_reg_read()/ocp_reg_write() wrappers over new
rtl_ops.phy_read/phy_write hooks, set per chip in rtl_ops_init().
The existing code becomes r8152_phy_read()/r8152_phy_write(), and the
TGPHY code is r8157_phy_read()/r8157_phy_write(). A busy timeout
abandons the access and returns the poll error, since ADDR/DATA/CMD
form a single transaction. Like the ADV command channel, it only
logs the timeout and does not mark the device inaccessible. If the
device is already marked RTL8152_INACCESSIBLE, the wait returns
-ENODEV.
A TGPHY access can fail, so the PHY accessors and the helpers built
on them (sram_*, sram2_*, ocp_reg_*_bits, MDIO bit helpers) now
return int. This patch propagates the status only where an errno
would otherwise be taken for register content or reported as
success: the pause parameters, rtl8152_set_speed(),
SIOCGMIIREG/SIOCSMIIREG, the PHY patch handshake and the PHY
firmware version checks.
Not handled here, and left to a follow-up series: the remaining
callers, notably the hw_phy_cfg and init sequences,
r8153_phy_status(), r8152_mmd_read(), EEE handling and the MII
library callbacks, still ignore the status. The MII library
callbacks need particular care, as read_mii_word() can now return an
errno that the library would take for a register value. Recovery
from a TGPHY timeout is left to the same series.
Older chips are unaffected, as r8152_phy_read() always succeeds.
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 475 +++++++++++++++++++++++++++++++---------
1 file changed, 366 insertions(+), 109 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 558ccb4682c27..7c0de56266963 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -162,6 +162,9 @@
#define USB_ADV_ADDR 0xd5d6
#define USB_ADV_DATA 0xd5d8
#define USB_ADV_CMD 0xd5dc
+#define USB_TGPHY_ADDR 0xd630
+#define USB_TGPHY_DATA 0xd632
+#define USB_TGPHY_CMD 0xd634
#define USB_UPS_CTRL 0xd800
#define USB_POWER_CUT 0xd80a
#define USB_MISC_0 0xd81a
@@ -511,6 +514,10 @@
#define ADV_CMD_WR BIT(1)
#define ADV_CMD_IP BIT(2)
+/* USB_TGPHY_CMD */
+#define TGPHY_CMD_BUSY BIT(0)
+#define TGPHY_CMD_WR BIT(1)
+
/* USB_UPS_CTRL */
#define POWER_CUT 0x0100
@@ -959,6 +966,8 @@ struct r8152 {
void (*hw_phy_cfg)(struct r8152 *tp);
void (*autosuspend_en)(struct r8152 *tp, bool enable);
void (*change_mtu)(struct r8152 *tp);
+ int (*phy_read)(struct r8152 *tp, u16 addr, u16 *data);
+ int (*phy_write)(struct r8152 *tp, u16 addr, u16 data);
} rtl_ops;
struct ups_info {
@@ -1638,7 +1647,7 @@ static void ocp_write_byte(struct r8152 *tp, u16 type, u16 index, u32 data)
generic_ocp_write(tp, index, byen, sizeof(tmp), &tmp, type);
}
-static u16 ocp_reg_read(struct r8152 *tp, u16 addr)
+static int r8152_phy_read(struct r8152 *tp, u16 addr, u16 *data)
{
u16 ocp_base, ocp_index;
@@ -1649,10 +1658,12 @@ static u16 ocp_reg_read(struct r8152 *tp, u16 addr)
}
ocp_index = (addr & 0x0fff) | 0xb000;
- return ocp_read_word(tp, MCU_TYPE_PLA, ocp_index);
+ *data = ocp_read_word(tp, MCU_TYPE_PLA, ocp_index);
+
+ return 0;
}
-static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
+static int r8152_phy_write(struct r8152 *tp, u16 addr, u16 data)
{
u16 ocp_base, ocp_index;
@@ -1664,16 +1675,33 @@ static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
ocp_index = (addr & 0x0fff) | 0xb000;
ocp_write_word(tp, MCU_TYPE_PLA, ocp_index, data);
+
+ return 0;
}
-static inline void r8152_mdio_write(struct r8152 *tp, u32 reg_addr, u32 value)
+static int ocp_reg_read(struct r8152 *tp, u16 addr, u16 *data)
{
- ocp_reg_write(tp, OCP_BASE_MII + reg_addr * 2, value);
+ return tp->rtl_ops.phy_read(tp, addr, data);
}
-static inline int r8152_mdio_read(struct r8152 *tp, u32 reg_addr)
+static int ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
{
- return ocp_reg_read(tp, OCP_BASE_MII + reg_addr * 2);
+ return tp->rtl_ops.phy_write(tp, addr, data);
+}
+
+static inline int r8152_mdio_write(struct r8152 *tp, u32 reg_addr, u32 value)
+{
+ return ocp_reg_write(tp, OCP_BASE_MII + reg_addr * 2, value);
+}
+
+static int r8152_mdio_read(struct r8152 *tp, u32 reg_addr)
+{
+ u16 data;
+ int ret;
+
+ ret = ocp_reg_read(tp, OCP_BASE_MII + reg_addr * 2, &data);
+
+ return ret < 0 ? ret : data;
}
static int wait_cmd_ready(struct r8152 *tp, u16 cmd)
@@ -1792,16 +1820,32 @@ static int rtl_ip_set_bits(struct r8152 *tp, u16 addr, u32 set)
return rtl_ip_w0w1(tp, addr, 0, set);
}
-static void sram_write(struct r8152 *tp, u16 addr, u16 data)
+static int sram_write(struct r8152 *tp, u16 addr, u16 data)
{
- ocp_reg_write(tp, OCP_SRAM_ADDR, addr);
- ocp_reg_write(tp, OCP_SRAM_DATA, data);
+ int ret;
+
+ ret = ocp_reg_write(tp, OCP_SRAM_ADDR, addr);
+ if (ret < 0)
+ goto out;
+
+ ret = ocp_reg_write(tp, OCP_SRAM_DATA, data);
+
+out:
+ return ret;
}
-static u16 sram_read(struct r8152 *tp, u16 addr)
+static int sram_read(struct r8152 *tp, u16 addr, u16 *data)
{
- ocp_reg_write(tp, OCP_SRAM_ADDR, addr);
- return ocp_reg_read(tp, OCP_SRAM_DATA);
+ int ret;
+
+ ret = ocp_reg_write(tp, OCP_SRAM_ADDR, addr);
+ if (ret < 0)
+ goto out;
+
+ ret = ocp_reg_read(tp, OCP_SRAM_DATA, data);
+
+out:
+ return ret;
}
static int read_mii_word(struct net_device *netdev, int phy_id, int reg)
@@ -1906,100 +1950,217 @@ static void ocp_byte_set_bits(struct r8152 *tp, u16 type, u16 index, u8 set)
ocp_byte_w0w1(tp, type, index, 0, set);
}
-static void ocp_reg_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
+static int ocp_reg_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
{
u16 data;
+ int ret;
+
+ ret = ocp_reg_read(tp, addr, &data);
+ if (ret < 0)
+ goto out;
- data = ocp_reg_read(tp, addr);
data = (data & ~clear) | set;
- ocp_reg_write(tp, addr, data);
+ ret = ocp_reg_write(tp, addr, data);
+
+out:
+ return ret;
}
-static void ocp_reg_clr_bits(struct r8152 *tp, u16 addr, u16 clear)
+static int ocp_reg_clr_bits(struct r8152 *tp, u16 addr, u16 clear)
{
- ocp_reg_w0w1(tp, addr, clear, 0);
+ return ocp_reg_w0w1(tp, addr, clear, 0);
}
-static void ocp_reg_set_bits(struct r8152 *tp, u16 addr, u16 set)
+static int ocp_reg_set_bits(struct r8152 *tp, u16 addr, u16 set)
{
- ocp_reg_w0w1(tp, addr, 0, set);
+ return ocp_reg_w0w1(tp, addr, 0, set);
}
-static void sram_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
+static int sram_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
{
u16 data;
+ int ret;
+
+ ret = sram_read(tp, addr, &data);
+ if (ret < 0)
+ goto out;
- data = sram_read(tp, addr);
data = (data & ~clear) | set;
- ocp_reg_write(tp, OCP_SRAM_DATA, data);
+ ret = ocp_reg_write(tp, OCP_SRAM_DATA, data);
+
+out:
+ return ret;
}
-static void sram_clr_bits(struct r8152 *tp, u16 addr, u16 clear)
+static int sram_clr_bits(struct r8152 *tp, u16 addr, u16 clear)
{
- sram_write_w0w1(tp, addr, clear, 0);
+ return sram_write_w0w1(tp, addr, clear, 0);
}
-static void sram_set_bits(struct r8152 *tp, u16 addr, u16 set)
+static int sram_set_bits(struct r8152 *tp, u16 addr, u16 set)
{
- sram_write_w0w1(tp, addr, 0, set);
+ return sram_write_w0w1(tp, addr, 0, set);
}
-static void sram2_write(struct r8152 *tp, u16 addr, u16 data)
+static int sram2_write(struct r8152 *tp, u16 addr, u16 data)
{
- ocp_reg_write(tp, OCP_SRAM2_ADDR, addr);
- ocp_reg_write(tp, OCP_SRAM2_DATA, data);
+ int ret;
+
+ ret = ocp_reg_write(tp, OCP_SRAM2_ADDR, addr);
+ if (ret < 0)
+ goto out;
+
+ ret = ocp_reg_write(tp, OCP_SRAM2_DATA, data);
+
+out:
+ return ret;
}
-static u16 sram2_read(struct r8152 *tp, u16 addr)
+static int sram2_read(struct r8152 *tp, u16 addr, u16 *data)
{
- ocp_reg_write(tp, OCP_SRAM2_ADDR, addr);
- return ocp_reg_read(tp, OCP_SRAM2_DATA);
+ int ret;
+
+ ret = ocp_reg_write(tp, OCP_SRAM2_ADDR, addr);
+ if (ret < 0)
+ goto out;
+
+ ret = ocp_reg_read(tp, OCP_SRAM2_DATA, data);
+
+out:
+ return ret;
}
-static void sram2_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
+static int sram2_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
{
u16 data;
+ int ret;
+
+ ret = sram2_read(tp, addr, &data);
+ if (ret < 0)
+ goto out;
- data = sram2_read(tp, addr);
data = (data & ~clear) | set;
- ocp_reg_write(tp, OCP_SRAM2_DATA, data);
+ ret = ocp_reg_write(tp, OCP_SRAM2_DATA, data);
+
+out:
+ return ret;
}
-static void sram2_set_bits(struct r8152 *tp, u16 addr, u16 set)
+static int sram2_set_bits(struct r8152 *tp, u16 addr, u16 set)
{
- sram2_write_w0w1(tp, addr, 0, set);
+ return sram2_write_w0w1(tp, addr, 0, set);
}
-static void sram2_clr_bits(struct r8152 *tp, u16 addr, u16 clear)
+static int sram2_clr_bits(struct r8152 *tp, u16 addr, u16 clear)
{
- sram2_write_w0w1(tp, addr, clear, 0);
+ return sram2_write_w0w1(tp, addr, clear, 0);
}
-static void r8152_mdio_clr_bit(struct r8152 *tp, u16 addr, u16 clear)
+static int r8152_mdio_clr_bit(struct r8152 *tp, u16 addr, u16 clear)
{
int data;
data = r8152_mdio_read(tp, addr);
- r8152_mdio_write(tp, addr, data & ~clear);
+ if (data < 0)
+ goto out;
+
+ data = r8152_mdio_write(tp, addr, data & ~clear);
+
+out:
+ return data;
}
-static void r8152_mdio_set_bit(struct r8152 *tp, u16 addr, u16 set)
+static int r8152_mdio_set_bit(struct r8152 *tp, u16 addr, u16 set)
{
int data;
data = r8152_mdio_read(tp, addr);
- r8152_mdio_write(tp, addr, data | set);
+ if (data < 0)
+ goto out;
+
+ data = r8152_mdio_write(tp, addr, data | set);
+
+out:
+ return data;
}
static int r8152_mdio_test_and_clr_bit(struct r8152 *tp, u16 addr, u16 clear)
{
- int data;
+ int data, ret;
- data = r8152_mdio_read(tp, addr);
- if (data & clear)
- r8152_mdio_write(tp, addr, data & ~clear);
+ ret = r8152_mdio_read(tp, addr);
+ if (ret < 0)
+ goto out;
+
+ data = ret;
+ if (data & clear) {
+ ret = r8152_mdio_write(tp, addr, data & ~clear);
+ if (ret < 0)
+ goto out;
+ }
+
+ ret = !!(data & clear);
- return data & clear;
+out:
+ return ret;
+}
+
+static int wait_tgphy_cmd_ready(struct r8152 *tp)
+{
+ u16 ocp_data;
+ int ret;
+
+ ret = read_poll_timeout(ocp_read_word, ocp_data,
+ test_bit(RTL8152_INACCESSIBLE, &tp->flags) ||
+ !(ocp_data & TGPHY_CMD_BUSY),
+ 2000, 20000, false, tp,
+ MCU_TYPE_USB, USB_TGPHY_CMD);
+
+ if (ret)
+ dev_err(&tp->intf->dev, "TGPHY cmd busy timeout\n");
+
+ return test_bit(RTL8152_INACCESSIBLE, &tp->flags) ? -ENODEV : ret;
+}
+
+static int rtl_tgphy_access(struct r8152 *tp, u16 addr, u16 *data, bool write)
+{
+ u16 cmd = 0;
+ int ret;
+
+ ret = wait_tgphy_cmd_ready(tp);
+ if (ret < 0)
+ goto out;
+
+ if (write) {
+ cmd |= TGPHY_CMD_WR;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_TGPHY_DATA, *data);
+ }
+
+ ocp_write_word(tp, MCU_TYPE_USB, USB_TGPHY_ADDR, addr);
+
+ cmd |= TGPHY_CMD_BUSY;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_TGPHY_CMD, cmd);
+
+ if (!write) {
+ ret = wait_tgphy_cmd_ready(tp);
+ if (ret < 0)
+ goto out;
+
+ *data = ocp_read_word(tp, MCU_TYPE_USB, USB_TGPHY_DATA);
+ }
+
+out:
+ return ret;
+}
+
+static int r8157_phy_read(struct r8152 *tp, u16 addr, u16 *data)
+{
+ return rtl_tgphy_access(tp, addr, data, false);
+}
+
+static int r8157_phy_write(struct r8152 *tp, u16 addr, u16 data)
+{
+ return rtl_tgphy_access(tp, addr, &data, true);
}
static int
@@ -4177,11 +4338,13 @@ static void r8153b_green_en(struct r8152 *tp, bool enable)
static u16 r8153_phy_status(struct r8152 *tp, u16 desired)
{
- u16 data;
+ u16 data = 0;
int i;
for (i = 0; i < 500; i++) {
- data = ocp_reg_read(tp, OCP_PHY_STATUS);
+ if (ocp_reg_read(tp, OCP_PHY_STATUS, &data) < 0)
+ break;
+
data &= PHY_STAT_MASK;
if (desired) {
if (data == desired)
@@ -4587,7 +4750,8 @@ static inline void rtl_reset_ocp_base(struct r8152 *tp)
static int rtl_phy_patch_request(struct r8152 *tp, bool request, bool wait)
{
u16 check;
- int i;
+ u16 ocp_data;
+ int i, ret;
if (request) {
ocp_reg_set_bits(tp, OCP_PHY_PATCH_CMD, PATCH_REQUEST);
@@ -4598,25 +4762,29 @@ static int rtl_phy_patch_request(struct r8152 *tp, bool request, bool wait)
}
for (i = 0; wait && i < 5000; i++) {
- u32 ocp_data;
-
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
return -ENODEV;
usleep_range(1000, 2000);
- ocp_data = ocp_reg_read(tp, OCP_PHY_PATCH_STAT);
- if ((ocp_data & PATCH_READY) ^ check)
+ ret = ocp_reg_read(tp, OCP_PHY_PATCH_STAT, &ocp_data);
+ if (ret < 0)
+ return ret;
+ else if ((ocp_data & PATCH_READY) ^ check)
break;
}
- if (request && wait &&
- !(ocp_reg_read(tp, OCP_PHY_PATCH_STAT) & PATCH_READY)) {
- dev_err(&tp->intf->dev, "PHY patch request fail\n");
- rtl_phy_patch_request(tp, false, false);
- return -ETIME;
- } else {
- return 0;
+ if (request && wait) {
+ ret = ocp_reg_read(tp, OCP_PHY_PATCH_STAT, &ocp_data);
+ if (ret < 0) {
+ return ret;
+ } else if (!(ocp_data & PATCH_READY)) {
+ dev_err(&tp->intf->dev, "PHY patch request fail\n");
+ rtl_phy_patch_request(tp, false, false);
+ return -ETIME;
+ }
}
+
+ return 0;
}
static void rtl_patch_key_set(struct r8152 *tp, u16 key_addr, u16 patch_key)
@@ -5331,10 +5499,14 @@ static void rtl_ram_code_speed_up(struct r8152 *tp, struct fw_phy_speed_up *phy,
{
u32 len;
u8 *data;
+ u16 ver;
rtl_reset_ocp_base(tp);
- if (sram_read(tp, SRAM_GPHY_FW_VER) >= __le16_to_cpu(phy->version)) {
+ if (sram_read(tp, SRAM_GPHY_FW_VER, &ver) < 0)
+ return;
+
+ if (ver >= __le16_to_cpu(phy->version)) {
dev_dbg(&tp->intf->dev, "PHY firmware has been the newest\n");
return;
}
@@ -5381,7 +5553,8 @@ static void rtl_ram_code_speed_up(struct r8152 *tp, struct fw_phy_speed_up *phy,
rtl_phy_patch_request(tp, false, wait);
- if (sram_read(tp, SRAM_GPHY_FW_VER) == __le16_to_cpu(phy->version))
+ if (sram_read(tp, SRAM_GPHY_FW_VER, &ver) >= 0 &&
+ ver == __le16_to_cpu(phy->version))
dev_dbg(&tp->intf->dev, "successfully applied %s\n", phy->info);
else
dev_err(&tp->intf->dev, "ram code speedup mode fail\n");
@@ -5389,14 +5562,17 @@ static void rtl_ram_code_speed_up(struct r8152 *tp, struct fw_phy_speed_up *phy,
static int rtl8152_fw_phy_ver(struct r8152 *tp, struct fw_phy_ver *phy_ver)
{
- u16 ver_addr, ver;
+ u16 ver_addr, ver, cur_ver;
ver_addr = __le16_to_cpu(phy_ver->ver.addr);
ver = __le16_to_cpu(phy_ver->ver.data);
rtl_reset_ocp_base(tp);
- if (sram_read(tp, ver_addr) >= ver) {
+ if (sram_read(tp, ver_addr, &cur_ver) < 0)
+ return 0;
+
+ if (cur_ver >= ver) {
dev_dbg(&tp->intf->dev, "PHY firmware has been the newest\n");
return 0;
}
@@ -5415,7 +5591,8 @@ static void rtl8152_fw_phy_fixup(struct r8152 *tp, struct fw_phy_fixup *fix)
rtl_reset_ocp_base(tp);
addr = __le16_to_cpu(fix->setting.addr);
- data = ocp_reg_read(tp, addr);
+ if (ocp_reg_read(tp, addr, &data) < 0)
+ return;
switch (__le16_to_cpu(fix->bit_cmd)) {
case FW_FIXUP_AND:
@@ -5719,10 +5896,10 @@ static inline void r8152_mmd_indirect(struct r8152 *tp, u16 dev, u16 reg)
static u16 r8152_mmd_read(struct r8152 *tp, u16 dev, u16 reg)
{
- u16 data;
+ u16 data = 0;
r8152_mmd_indirect(tp, dev, reg);
- data = ocp_reg_read(tp, OCP_EEE_DATA);
+ ocp_reg_read(tp, OCP_EEE_DATA, &data);
ocp_reg_write(tp, OCP_EEE_AR, 0x0000);
return data;
@@ -5787,7 +5964,8 @@ static void r8156_eee_en(struct r8152 *tp, bool enable)
r8153_eee_en(tp, enable);
- config = ocp_reg_read(tp, OCP_EEE_ADV2);
+ if (ocp_reg_read(tp, OCP_EEE_ADV2, &config) < 0)
+ return;
if (enable && (tp->eee_adv2 & MDIO_EEE_2_5GT))
config |= MDIO_EEE_2_5GT;
@@ -6243,8 +6421,8 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
* rg_saw_cnt = OCP reg 0xC426 Bit[13:0]
* swr_cnt_1ms_ini = 16000000 / rg_saw_cnt
*/
- ocp_data = ocp_reg_read(tp, 0xc426);
- ocp_data &= 0x3fff;
+ ocp_reg_read(tp, 0xc426, &data);
+ ocp_data = data & 0x3fff;
if (ocp_data) {
u32 swr_cnt_1ms_ini;
@@ -6588,7 +6766,11 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
if (!advertising)
return -EINVAL;
- orig = r8152_mdio_read(tp, MII_ADVERTISE);
+ ret = r8152_mdio_read(tp, MII_ADVERTISE);
+ if (ret < 0)
+ goto out;
+
+ orig = ret;
new1 = orig & ~(ADVERTISE_10HALF | ADVERTISE_10FULL |
ADVERTISE_100HALF | ADVERTISE_100FULL);
if (advertising & RTL_ADVERTISED_10_HALF) {
@@ -6615,7 +6797,11 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
}
if (tp->mii.supports_gmii) {
- orig = r8152_mdio_read(tp, MII_CTRL1000);
+ ret = r8152_mdio_read(tp, MII_CTRL1000);
+ if (ret < 0)
+ goto out;
+
+ orig = ret;
new1 = orig & ~(ADVERTISE_1000FULL |
ADVERTISE_1000HALF);
@@ -6629,7 +6815,10 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
}
if (tp->support_2500full || tp->support_5000full || tp->support_10000full) {
- orig = ocp_reg_read(tp, OCP_10GBT_CTRL);
+ ret = ocp_reg_read(tp, OCP_10GBT_CTRL, &orig);
+ if (ret < 0)
+ goto out;
+
new1 = orig & ~(MDIO_AN_10GBT_CTRL_ADV2_5G | MDIO_AN_10GBT_CTRL_ADV5G
| MDIO_AN_10GBT_CTRL_ADV10G);
@@ -6660,20 +6849,23 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
if (test_and_clear_bit(PHY_RESET, &tp->flags))
bmcr |= BMCR_RESET;
- r8152_mdio_write(tp, MII_BMCR, bmcr);
+ ret = r8152_mdio_write(tp, MII_BMCR, bmcr);
+ if (ret < 0)
+ goto out;
if (bmcr & BMCR_RESET) {
int i;
for (i = 0; i < 50; i++) {
msleep(20);
- if ((r8152_mdio_read(tp, MII_BMCR) & BMCR_RESET) == 0)
+ ret = r8152_mdio_read(tp, MII_BMCR);
+ if (ret < 0 || !(ret & BMCR_RESET))
break;
}
}
out:
- return ret;
+ return ret < 0 ? ret : 0;
}
static void rtl8152_up(struct r8152 *tp)
@@ -7079,7 +7271,12 @@ static bool rtl8152_in_nway(struct r8152 *tp)
static bool rtl8153_in_nway(struct r8152 *tp)
{
- u16 phy_state = ocp_reg_read(tp, OCP_PHY_STATE) & 0xff;
+ u16 phy_state;
+
+ if (ocp_reg_read(tp, OCP_PHY_STATE, &phy_state) < 0)
+ return false;
+
+ phy_state &= 0xff;
if (phy_state == TXDIS_STATE || phy_state == ABD_STATE)
return false;
@@ -7095,7 +7292,9 @@ static void r8156_mdio_force_mode(struct r8152 *tp)
* 0: MDIO force mode
* 1: MMD force mode
*/
- data = ocp_reg_read(tp, 0xa5b4);
+ if (ocp_reg_read(tp, 0xa5b4, &data) < 0)
+ return;
+
if (data & BIT(15)) {
data &= ~BIT(15);
ocp_reg_write(tp, 0xa5b4, data);
@@ -7734,19 +7933,20 @@ static void r8156_hw_phy_cfg(struct r8152 *tp)
ocp_reg_clr_bits(tp, 0xa86a, BIT(0));
/* MDI SWAP */
+ ocp_reg_read(tp, 0xd068, &data);
if ((ocp_read_word(tp, MCU_TYPE_USB, USB_UPS_CFG) & MID_REVERSE) &&
- (ocp_reg_read(tp, 0xd068) & BIT(1))) {
+ (data & BIT(1))) {
u16 swap_a, swap_b;
- data = ocp_reg_read(tp, 0xd068);
+ ocp_reg_read(tp, 0xd068, &data);
data &= ~0x1f;
data |= 0x1; /* p0 */
ocp_reg_write(tp, 0xd068, data);
- swap_a = ocp_reg_read(tp, 0xd06a);
+ ocp_reg_read(tp, 0xd06a, &swap_a);
data &= ~0x18;
data |= 0x18; /* p3 */
ocp_reg_write(tp, 0xd068, data);
- swap_b = ocp_reg_read(tp, 0xd06a);
+ ocp_reg_read(tp, 0xd06a, &swap_b);
data &= ~0x18; /* p0 */
ocp_reg_write(tp, 0xd068, data);
ocp_reg_write(tp, 0xd06a,
@@ -7758,11 +7958,11 @@ static void r8156_hw_phy_cfg(struct r8152 *tp)
data &= ~0x18;
data |= 0x08; /* p1 */
ocp_reg_write(tp, 0xd068, data);
- swap_a = ocp_reg_read(tp, 0xd06a);
+ ocp_reg_read(tp, 0xd06a, &swap_a);
data &= ~0x18;
data |= 0x10; /* p2 */
ocp_reg_write(tp, 0xd068, data);
- swap_b = ocp_reg_read(tp, 0xd06a);
+ ocp_reg_read(tp, 0xd06a, &swap_b);
data &= ~0x18;
data |= 0x08; /* p1 */
ocp_reg_write(tp, 0xd068, data);
@@ -7773,16 +7973,16 @@ static void r8156_hw_phy_cfg(struct r8152 *tp)
ocp_reg_write(tp, 0xd068, data);
ocp_reg_write(tp, 0xd06a,
(swap_b & ~0x7ff) | (swap_a & 0x7ff));
- swap_a = ocp_reg_read(tp, 0xbd5a);
- swap_b = ocp_reg_read(tp, 0xbd5c);
+ ocp_reg_read(tp, 0xbd5a, &swap_a);
+ ocp_reg_read(tp, 0xbd5c, &swap_b);
ocp_reg_write(tp, 0xbd5a, (swap_a & ~0x1f1f) |
((swap_b & 0x1f) << 8) |
((swap_b >> 8) & 0x1f));
ocp_reg_write(tp, 0xbd5c, (swap_b & ~0x1f1f) |
((swap_a & 0x1f) << 8) |
((swap_a >> 8) & 0x1f));
- swap_a = ocp_reg_read(tp, 0xbc18);
- swap_b = ocp_reg_read(tp, 0xbc1a);
+ ocp_reg_read(tp, 0xbc18, &swap_a);
+ ocp_reg_read(tp, 0xbc1a, &swap_b);
ocp_reg_write(tp, 0xbc18, (swap_a & ~0x1f1f) |
((swap_b & 0x1f) << 8) |
((swap_b >> 8) & 0x1f));
@@ -9273,8 +9473,16 @@ int rtl8152_get_link_ksettings(struct net_device *netdev,
cmd->link_modes.supported, tp->support_10000full);
if (tp->support_2500full || tp->support_5000full || tp->support_10000full) {
- u16 ocp_10gbt_ctrl = ocp_reg_read(tp, OCP_10GBT_CTRL);
- u16 ocp_10gbt_stat = ocp_reg_read(tp, OCP_10GBT_STAT);
+ u16 ocp_10gbt_ctrl;
+ u16 ocp_10gbt_stat;
+
+ ret = ocp_reg_read(tp, OCP_10GBT_CTRL, &ocp_10gbt_ctrl);
+ if (ret < 0)
+ goto out_unlock;
+
+ ret = ocp_reg_read(tp, OCP_10GBT_STAT, &ocp_10gbt_stat);
+ if (ret < 0)
+ goto out_unlock;
if (tp->support_2500full) {
linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
@@ -9316,12 +9524,13 @@ int rtl8152_get_link_ksettings(struct net_device *netdev,
}
}
+out_unlock:
mutex_unlock(&tp->control);
usb_autopm_put_interface(tp->intf);
out:
- return ret;
+ return ret < 0 ? ret : 0;
}
static int rtl8152_set_link_ksettings(struct net_device *dev,
@@ -9502,21 +9711,37 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_keee *eee)
__ETHTOOL_DECLARE_LINK_MODE_MASK(common) = {};
u16 speed = rtl8152_get_speed(tp);
u16 val;
+ int ret;
+
+ ret = ocp_reg_read(tp, OCP_EEE_ABLE, &val);
+ if (ret < 0)
+ goto out;
- val = ocp_reg_read(tp, OCP_EEE_ABLE);
mii_eee_cap1_mod_linkmode_t(eee->supported, val);
- val = ocp_reg_read(tp, OCP_EEE_ADV);
+ ret = ocp_reg_read(tp, OCP_EEE_ADV, &val);
+ if (ret < 0)
+ goto out;
+
mii_eee_cap1_mod_linkmode_t(eee->advertised, val);
- val = ocp_reg_read(tp, OCP_EEE_LPABLE);
+ ret = ocp_reg_read(tp, OCP_EEE_LPABLE, &val);
+ if (ret < 0)
+ goto out;
+
mii_eee_cap1_mod_linkmode_t(eee->lp_advertised, val);
if (tp->support_2500full || tp->support_5000full) {
- val = ocp_reg_read(tp, OCP_EEE_ADV2);
+ ret = ocp_reg_read(tp, OCP_EEE_ADV2, &val);
+ if (ret < 0)
+ goto out;
+
mii_eee_cap2_mod_linkmode_adv_t(eee->advertised, val);
- val = ocp_reg_read(tp, OCP_EEE_LPABLE2);
+ ret = ocp_reg_read(tp, OCP_EEE_LPABLE2, &val);
+ if (ret < 0)
+ goto out;
+
mii_eee_cap2_mod_linkmode_adv_t(eee->lp_advertised, val);
}
@@ -9552,7 +9777,8 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_keee *eee)
linkmode_and(common, common, eee->lp_advertised);
eee->eee_active = !linkmode_empty(common);
- return 0;
+out:
+ return ret < 0 ? ret : 0;
}
static int
@@ -9792,7 +10018,7 @@ static int rtl8152_set_ringparam(struct net_device *netdev,
static void rtl8152_get_pauseparam(struct net_device *netdev, struct ethtool_pauseparam *pause)
{
struct r8152 *tp = netdev_priv(netdev);
- u16 bmcr, lcladv, rmtadv;
+ int bmcr, lcladv, rmtadv;
u8 cap;
if (usb_autopm_get_interface(tp->intf) < 0)
@@ -9808,7 +10034,10 @@ static void rtl8152_get_pauseparam(struct net_device *netdev, struct ethtool_pau
usb_autopm_put_interface(tp->intf);
- if (!(bmcr & BMCR_ANENABLE)) {
+ /* Without a readable PHY, or with autoneg off, there is no flow
+ * control to report.
+ */
+ if (bmcr < 0 || lcladv < 0 || rmtadv < 0 || !(bmcr & BMCR_ANENABLE)) {
pause->autoneg = 0;
pause->rx_pause = 0;
pause->tx_pause = 0;
@@ -9839,7 +10068,11 @@ static int rtl8152_set_pauseparam(struct net_device *netdev, struct ethtool_paus
mutex_lock(&tp->control);
- if (pause->autoneg && !(r8152_mdio_read(tp, MII_BMCR) & BMCR_ANENABLE)) {
+ ret = r8152_mdio_read(tp, MII_BMCR);
+ if (ret < 0)
+ goto out;
+
+ if (pause->autoneg && !(ret & BMCR_ANENABLE)) {
ret = -EINVAL;
goto out;
}
@@ -9850,16 +10083,20 @@ static int rtl8152_set_pauseparam(struct net_device *netdev, struct ethtool_paus
if (pause->tx_pause)
cap |= FLOW_CTRL_TX;
- old = r8152_mdio_read(tp, MII_ADVERTISE);
+ ret = r8152_mdio_read(tp, MII_ADVERTISE);
+ if (ret < 0)
+ goto out;
+
+ old = ret;
new1 = (old & ~(ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM)) | mii_advertise_flowctrl(cap);
if (old != new1)
- r8152_mdio_write(tp, MII_ADVERTISE, new1);
+ ret = r8152_mdio_write(tp, MII_ADVERTISE, new1);
out:
mutex_unlock(&tp->control);
usb_autopm_put_interface(tp->intf);
- return ret;
+ return ret < 0 ? ret : 0;
}
static const struct ethtool_ops ops = {
@@ -9893,7 +10130,7 @@ static int rtl8152_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
{
struct r8152 *tp = netdev_priv(netdev);
struct mii_ioctl_data *data = if_mii(rq);
- int res;
+ int res, val;
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
return -ENODEV;
@@ -9909,7 +10146,11 @@ static int rtl8152_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
case SIOCGMIIREG:
mutex_lock(&tp->control);
- data->val_out = r8152_mdio_read(tp, data->reg_num);
+ val = r8152_mdio_read(tp, data->reg_num);
+ if (val < 0)
+ res = val;
+ else
+ data->val_out = val;
mutex_unlock(&tp->control);
break;
@@ -9919,7 +10160,7 @@ static int rtl8152_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
break;
}
mutex_lock(&tp->control);
- r8152_mdio_write(tp, data->reg_num, data->val_in);
+ res = r8152_mdio_write(tp, data->reg_num, data->val_in);
mutex_unlock(&tp->control);
break;
@@ -10082,6 +10323,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->in_nway = rtl8152_in_nway;
ops->hw_phy_cfg = r8152b_hw_phy_cfg;
ops->autosuspend_en = rtl_runtime_suspend_enable;
+ ops->phy_read = r8152_phy_read;
+ ops->phy_write = r8152_phy_write;
tp->rx_buf_sz = 16 * 1024;
tp->eee_en = true;
tp->eee_adv = MDIO_EEE_100TX;
@@ -10104,6 +10347,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8153_hw_phy_cfg;
ops->autosuspend_en = rtl8153_runtime_enable;
ops->change_mtu = rtl8153_change_mtu;
+ ops->phy_read = r8152_phy_read;
+ ops->phy_write = r8152_phy_write;
if (tp->udev->speed < USB_SPEED_SUPER)
tp->rx_buf_sz = 16 * 1024;
else
@@ -10127,6 +10372,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8153b_hw_phy_cfg;
ops->autosuspend_en = rtl8153b_runtime_enable;
ops->change_mtu = rtl8153_change_mtu;
+ ops->phy_read = r8152_phy_read;
+ ops->phy_write = r8152_phy_write;
tp->rx_buf_sz = 32 * 1024;
tp->eee_en = true;
tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
@@ -10151,6 +10398,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8156_hw_phy_cfg;
ops->autosuspend_en = rtl8156_runtime_enable;
ops->change_mtu = rtl8156_change_mtu;
+ ops->phy_read = r8152_phy_read;
+ ops->phy_write = r8152_phy_write;
tp->rx_buf_sz = 48 * 1024;
tp->support_2500full = 1;
r8152_desc_init(tp);
@@ -10176,6 +10425,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8156b_hw_phy_cfg;
ops->autosuspend_en = rtl8156_runtime_enable;
ops->change_mtu = rtl8156_change_mtu;
+ ops->phy_read = r8152_phy_read;
+ ops->phy_write = r8152_phy_write;
tp->rx_buf_sz = 48 * 1024;
r8152_desc_init(tp);
break;
@@ -10193,6 +10444,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8153c_hw_phy_cfg;
ops->autosuspend_en = rtl8153c_runtime_enable;
ops->change_mtu = rtl8153c_change_mtu;
+ ops->phy_read = r8152_phy_read;
+ ops->phy_write = r8152_phy_write;
tp->rx_buf_sz = 32 * 1024;
tp->eee_en = true;
tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
@@ -10215,6 +10468,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8157_hw_phy_cfg;
ops->autosuspend_en = rtl8157_runtime_enable;
ops->change_mtu = rtl8157_change_mtu;
+ ops->phy_read = r8157_phy_read;
+ ops->phy_write = r8157_phy_write;
tp->rx_buf_sz = 32 * 1024;
tp->support_2500full = 1;
tp->support_5000full = 1;
@@ -10238,6 +10493,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8159_hw_phy_cfg;
ops->autosuspend_en = rtl8157_runtime_enable;
ops->change_mtu = rtl8157_change_mtu;
+ ops->phy_read = r8157_phy_read;
+ ops->phy_write = r8157_phy_write;
tp->rx_buf_sz = 48 * 1024;
tp->support_2500full = 1;
tp->support_5000full = 1;
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v7 08/10] r8152: add TGPHY register access for RTL8157 and RTL8159
2026-10-14 8:19 ` [PATCH net-next v7 08/10] r8152: add TGPHY register access for RTL8157 and RTL8159 Chih Kai Hsu
@ 2026-10-21 0:52 ` Jakub Kicinski
0 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2026-10-21 0:52 UTC (permalink / raw)
To: hsu.chih.kai
Cc: Jakub Kicinski, davem, netdev, nic_swsd, linux-kernel, linux-usb,
edumazet, bjorn, pabeni, andrew+netdev, mail
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: add TGPHY register access for RTL8157 and RTL8159
This adds a TGPHY command channel backend for PHY register access on
RTL8157 and RTL8159. It is selected through new rtl_ops.phy_read and
phy_write hooks. The helpers built on ocp_reg_read()/ocp_reg_write() now
return int, and a subset of callers propagate the errors.
> This patch propagates the status only where an errno
> would otherwise be taken for register content or reported as
> success: the pause parameters, rtl8152_set_speed(),
> SIOCGMIIREG/SIOCSMIIREG, the PHY patch handshake and the PHY
> firmware version checks.
>
> Not handled here, and left to a follow-up series: the remaining
> callers, notably the hw_phy_cfg and init sequences,
> r8153_phy_status(), r8152_mmd_read(), EEE handling and the MII
> library callbacks, still ignore the status.
[Severity: Medium]
Is this description accurate? Some of the callers listed as still
ignoring the status do act on it in this diff.
r8153_get_eee() returns an errno from each of its ocp_reg_read() calls:
ret = ocp_reg_read(tp, OCP_EEE_ABLE, &val);
if (ret < 0)
goto out;
r8153_phy_status() now leaves its poll on the first failed read:
if (ocp_reg_read(tp, OCP_PHY_STATUS, &data) < 0)
break;
rtl8152_get_link_ksettings() is not in either list. It now fails the
whole request when the OCP_10GBT_CTRL or OCP_10GBT_STAT read fails.
rtl8153_in_nway(), r8156_mdio_force_mode(), r8156_eee_en() and
rtl8152_fw_phy_fixup() also return early on a read failure now.
Could the commit message be updated to cover these changes?
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 558ccb4682c27..7c0de56266963 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[ ... ]
> @@ -1664,16 +1675,33 @@ static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
[ ... ]
> +static int r8152_mdio_read(struct r8152 *tp, u32 reg_addr)
> +{
> + u16 data;
> + int ret;
> +
> + ret = ocp_reg_read(tp, OCP_BASE_MII + reg_addr * 2, &data);
> +
> + return ret < 0 ? ret : data;
> }
[Severity: Medium]
On RTL8157/RTL8159, can this pass -ETIMEDOUT to the generic MII library
as if it were a BMCR value? read_mii_word() is the mii_if_info.mdio_read
callback, and it returns the result of r8152_mdio_read() unchanged.
mii_nway_restart() in drivers/net/mii.c does:
bmcr = mii->mdio_read(mii->dev, mii->phy_id, MII_BMCR);
if (bmcr & BMCR_ANENABLE) {
bmcr |= BMCR_ANRESTART;
mii->mdio_write(mii->dev, mii->phy_id, MII_BMCR, bmcr);
r = 0;
}
-ETIMEDOUT is 0xffffff92, which has BMCR_ANENABLE set. write_mii_word()
only filters RTL8152_INACCESSIBLE, and a TGPHY timeout does not set that
flag. So 0xff92 would be written to BMCR. That sets BMCR_RESET,
BMCR_LOOPBACK, BMCR_PDOWN and BMCR_ISOLATE, and the call still returns 0.
The path is:
rtl8152_nway_reset()
mii_nway_restart()
read_mii_word()
r8152_mdio_read()
r8157_phy_read()
rtl_tgphy_access()
wait_tgphy_cmd_ready() returns -ETIMEDOUT
rtl_ethtool_set_eee() also calls mii_nway_restart(). In addition,
mii_ethtool_get_link_ksettings() would decode link modes from the errno.
Before this patch, the only errno read_mii_word() could return was
-ENODEV, and write_mii_word() filters that case.
The commit message mentions this case, and the later patches in the series
do not seem to address it. rtl_ops_init() switches RTL_VER_16/17 to
r8157_phy_read() in this patch. Could read_mii_word()/write_mii_word()
handle the errno before or in this patch?
> @@ -1906,100 +1950,217 @@ static void ocp_byte_set_bits(struct r8152 *tp, u16 type, u16 index, u8 set)
[ ... ]
> +static int wait_tgphy_cmd_ready(struct r8152 *tp)
> +{
> + u16 ocp_data;
> + int ret;
> +
> + ret = read_poll_timeout(ocp_read_word, ocp_data,
> + test_bit(RTL8152_INACCESSIBLE, &tp->flags) ||
> + !(ocp_data & TGPHY_CMD_BUSY),
> + 2000, 20000, false, tp,
> + MCU_TYPE_USB, USB_TGPHY_CMD);
> +
> + if (ret)
> + dev_err(&tp->intf->dev, "TGPHY cmd busy timeout\n");
[Severity: Medium]
What happens if TGPHY_CMD_BUSY stays set while USB transfers still work?
Each PHY access then waits up to 20ms here, or 40ms for a read. It also
prints another error that is not ratelimited. There is no recovery, and
RTL8152_INACCESSIBLE is never set.
Most callers ignore the status and keep going. r8157_hw_phy_cfg() makes
well over a hundred PHY accesses, and the firmware loaders do one write
per firmware word:
rtl8152_fw_phy_nc_apply() {
...
for (i = 0; i < num; i++)
ocp_reg_write(tp, OCP_SRAM_DATA, __le16_to_cpu(data[i]));
...
}
rtl8152_fw_phy_union_apply() has the same loop.
All of this runs with tp->control held in the probe, resume, open and
reset paths. Could this stall for seconds or minutes and flood the log?
Would ratelimiting the message, or stopping the sequence after the first
timeout, avoid that?
> +
> + return test_bit(RTL8152_INACCESSIBLE, &tp->flags) ? -ENODEV : ret;
> +}
> +
> +static int rtl_tgphy_access(struct r8152 *tp, u16 addr, u16 *data, bool write)
> +{
> + u16 cmd = 0;
> + int ret;
> +
> + ret = wait_tgphy_cmd_ready(tp);
> + if (ret < 0)
> + goto out;
> +
> + if (write) {
> + cmd |= TGPHY_CMD_WR;
> + ocp_write_word(tp, MCU_TYPE_USB, USB_TGPHY_DATA, *data);
> + }
> +
> + ocp_write_word(tp, MCU_TYPE_USB, USB_TGPHY_ADDR, addr);
> +
> + cmd |= TGPHY_CMD_BUSY;
> + ocp_write_word(tp, MCU_TYPE_USB, USB_TGPHY_CMD, cmd);
[Severity: Low]
This is a minor point. The status of the USB_TGPHY_DATA, ADDR and CMD
transfers is not checked here.
A misdirected command does not seem possible. r8152_control_msg() calls
rtl_set_inaccessible() when a control transfer fails, so a CMD write that
follows a failed ADDR write is never sent.
rtl_tgphy_access() can still return 0 once after a failed transfer,
either for a write or when the final USB_TGPHY_DATA read fails. Would it
be cleaner to return -ENODEV when RTL8152_INACCESSIBLE is set at the end
of the transaction?
> +
> + if (!write) {
> + ret = wait_tgphy_cmd_ready(tp);
> + if (ret < 0)
> + goto out;
> +
> + *data = ocp_read_word(tp, MCU_TYPE_USB, USB_TGPHY_DATA);
[Severity: Medium]
This isn't a bug introduced by this patch, but ocp_read_word() ignores
the return value of generic_ocp_read():
ocp_read_word() {
...
generic_ocp_read(tp, index, sizeof(tmp), &tmp, type | byen);
data = __le32_to_cpu(tmp);
...
}
RTL8152_INACCESSIBLE could be set between the second
wait_tgphy_cmd_ready() and this read, for example by
rtl8152_disconnect()->rtl_set_unplug(). In that case generic_ocp_read()
returns -ENODEV early, tmp stays uninitialized, and rtl_tgphy_access()
returns 0. SIOCGMIIREG could then copy that stack data into
data->val_out.
dev_ioctl() requires CAP_NET_ADMIN for SIOCGMIIREG, so unprivileged users
cannot reach this. r8152_phy_read() already has the same flaw.
[ ... ]
> @@ -4177,11 +4338,13 @@ static void r8153b_green_en(struct r8152 *tp, bool enable)
>
> static u16 r8153_phy_status(struct r8152 *tp, u16 desired)
> {
> - u16 data;
> + u16 data = 0;
> int i;
>
> for (i = 0; i < 500; i++) {
> - data = ocp_reg_read(tp, OCP_PHY_STATUS);
> + if (ocp_reg_read(tp, OCP_PHY_STATUS, &data) < 0)
> + break;
> +
[Severity: Medium]
On RTL8157/8159, one 20ms TGPHY busy timeout now ends this poll, which
previously ran for up to 500 x 20ms. The function then returns 0 or a
stale masked value. Is that safe for callers that switch on the result?
r8157_hw_phy_cfg() {
...
data = r8153_phy_status(tp, 0);
switch (data) {
case PHY_STAT_EXT_INIT:
rtl8152_apply_firmware(tp, true);
ocp_reg_clr_bits(tp, 0xa466, BIT(0));
ocp_reg_clr_bits(tp, 0xa468, BIT(3) | BIT(1));
break;
case PHY_STAT_LAN_ON:
case PHY_STAT_PWRDN:
default:
rtl8152_apply_firmware(tp, false);
break;
}
...
}
A return of 0 falls into the default case. That calls
rtl8152_apply_firmware(tp, false) and skips the EXT_INIT release writes,
even when the PHY really is in PHY_STAT_EXT_INIT. r8159_hw_phy_cfg() and
r8157_init() follow the same pattern.
The later r8153_phy_status(tp, PHY_STAT_LAN_ON) call also stops after its
first failed read. The PFM, ALDPS and related settings are then
programmed before the PHY reaches LAN_ON, and WARN_ON_ONCE() fires.
Before this patch, the loop always ran to its full timeout on these
chips.
[ ... ]
> @@ -4598,25 +4762,29 @@ static int rtl_phy_patch_request(struct r8152 *tp, bool request, bool wait)
> }
>
> for (i = 0; wait && i < 5000; i++) {
> - u32 ocp_data;
> -
> if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
> return -ENODEV;
>
> usleep_range(1000, 2000);
> - ocp_data = ocp_reg_read(tp, OCP_PHY_PATCH_STAT);
> - if ((ocp_data & PATCH_READY) ^ check)
> + ret = ocp_reg_read(tp, OCP_PHY_PATCH_STAT, &ocp_data);
> + if (ret < 0)
> + return ret;
> + else if ((ocp_data & PATCH_READY) ^ check)
> break;
> }
>
> - if (request && wait &&
> - !(ocp_reg_read(tp, OCP_PHY_PATCH_STAT) & PATCH_READY)) {
> - dev_err(&tp->intf->dev, "PHY patch request fail\n");
> - rtl_phy_patch_request(tp, false, false);
> - return -ETIME;
> - } else {
> - return 0;
> + if (request && wait) {
> + ret = ocp_reg_read(tp, OCP_PHY_PATCH_STAT, &ocp_data);
> + if (ret < 0) {
> + return ret;
> + } else if (!(ocp_data & PATCH_READY)) {
> + dev_err(&tp->intf->dev, "PHY patch request fail\n");
> + rtl_phy_patch_request(tp, false, false);
> + return -ETIME;
> + }
> }
[Severity: Medium]
When request is true, PATCH_REQUEST is already set in OCP_PHY_PATCH_CMD
by the time these two new return ret paths run. Do they leave it
asserted? The -ETIME path releases it with
rtl_phy_patch_request(tp, false, false), but the errno paths return
without doing that.
Callers treat any non-zero return as "not acquired" and do not release:
r8157_hw_phy_cfg() {
...
if (rtl_phy_patch_request(tp, true, true))
return;
...
}
r8159_hw_phy_cfg() and rtl_ram_code_speed_up() do the same. A TGPHY
timeout on the OCP_PHY_PATCH_STAT read would leave the PHY in patch
request mode. r8157_hw_phy_cfg() would also skip the rest of its
configuration.
[ ... ]
> @@ -6629,7 +6815,10 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
> }
>
> if (tp->support_2500full || tp->support_5000full || tp->support_10000full) {
> - orig = ocp_reg_read(tp, OCP_10GBT_CTRL);
> + ret = ocp_reg_read(tp, OCP_10GBT_CTRL, &orig);
> + if (ret < 0)
> + goto out;
> +
> new1 = orig & ~(MDIO_AN_10GBT_CTRL_ADV2_5G | MDIO_AN_10GBT_CTRL_ADV5G
> | MDIO_AN_10GBT_CTRL_ADV10G);
>
[Severity: Medium]
The reads are checked now, but the writes in rtl8152_set_speed() still
are not. The commit message names this function as one where an errno is
no longer reported as success:
if (orig != new1) {
r8152_mdio_write(tp, MII_ADVERTISE, new1);
tp->mii.advertising = new1;
}
...
if (orig != new1)
r8152_mdio_write(tp, MII_CTRL1000, new1);
...
if (orig != new1)
ocp_reg_write(tp, OCP_10GBT_CTRL, new1);
If one of these writes times out on RTL8157/8159, tp->mii.advertising is
still updated. The function returns 0 as long as the BMCR write succeeds.
rtl8152_set_link_ksettings() then saves tp->advertising, tp->speed and
tp->duplex as if they had been applied.
The PHY patch handshake has the same gap. rtl_phy_patch_request() does
not check the result of the PATCH_REQUEST write:
if (request) {
ocp_reg_set_bits(tp, OCP_PHY_PATCH_CMD, PATCH_REQUEST);
check = 0;
} else {
ocp_reg_clr_bits(tp, OCP_PHY_PATCH_CMD, PATCH_REQUEST);
check = PATCH_READY;
}
This has three effects:
- With wait false, a failed request or release returns 0. The firmware
path uses this mode through rtl_pre_ram_code(..., !power_cut).
- With request true, a failed write polls for more than 5 seconds and
then returns -ETIME instead of the real errno.
- With request false and wait true, a failed release runs all 5000
iterations and still returns 0.
rtl_pre_ram_code() also turns any error into -ETIME, and
rtl8152_apply_firmware() ignores its return value.
Should these write results be checked as well?
> @@ -6660,20 +6849,23 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
> if (test_and_clear_bit(PHY_RESET, &tp->flags))
> bmcr |= BMCR_RESET;
>
> - r8152_mdio_write(tp, MII_BMCR, bmcr);
> + ret = r8152_mdio_write(tp, MII_BMCR, bmcr);
> + if (ret < 0)
> + goto out;
[Severity: Medium]
Is the pending PHY reset lost when this write fails? PHY_RESET is cleared
before the write. On RTL8157/8159, r8152_mdio_write() can now return
-ETIMEDOUT before anything is sent to the hardware:
rtl_tgphy_access() {
...
ret = wait_tgphy_cmd_ready(tp);
if (ret < 0)
goto out;
...
}
This can happen if the preceding OCP_10GBT_CTRL write is still busy.
r8157_hw_phy_cfg() and r8159_hw_phy_cfg() end with
set_bit(PHY_RESET, &tp->flags) so that the next rtl8152_set_speed()
resets the PHY. If this write fails, BMCR_RESET never reaches the PHY and
the flag is not set again.
Should PHY_RESET be restored on this error path?
[ ... ]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v7 09/10] r8152: extract rtl_fc_pause_pkt_en() and apply it to RTL8156/8157/8159
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
` (7 preceding siblings ...)
2026-10-14 8:19 ` [PATCH net-next v7 08/10] r8152: add TGPHY register access for RTL8157 and RTL8159 Chih Kai Hsu
@ 2026-10-14 8:19 ` Chih Kai Hsu
2026-10-14 8:19 ` [PATCH net-next v7 10/10] r8152: enable UPS for RTL8157 and RTL8159 QFN68 Chih Kai Hsu
2026-10-14 8:25 ` [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support netdev-bot+sinfo
10 siblings, 0 replies; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:19 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
The flow-control configuration keeps the device from sending pause
frames without limit. Only RTL8156B (VER_12/13/15) got the full
sequence from r8156_init(). RTL8156A (VER_10/11) toggled
FC_PATCH_TASK in rtl8156_enable() without the FC timer and
FLOW_CTRL_PATCH_OPT setup that has to precede it, and RTL8157/RTL8159
(VER_16/17) got no flow-control configuration at all, so they could
keep sending pause frames indefinitely.
Collect the per-family sequence into rtl_fc_pause_pkt_en() and call
it for all of them.
- VER_10/11: the FC timer + FLOW_CTRL_PATCH_OPT sequence that
RTL8153B (VER_09) already runs from its pre_fw/post_fw hooks,
applied directly instead.
- VER_16/17: RTL8157/RTL8159 limit pause frames with the new
PLA_PAUSE_LIMIT register instead of the FW patch. The datasheet
gives a fixed pause-packet count per link speed, reproduced here
by a log2-ratio computation. Because the count follows the
negotiated speed, this family is called from rtl8157_enable()
rather than from init.
The speed decode covers every rate the driver advertises. The
hardware can also report 500 and 1250 Mbit/s, which the kernel has
no EEE link-mode encoding for and so never negotiates; if the PHY
still reports such a rate, pause limiting is disarmed rather than
programmed from a guessed speed.
In r8156_init(), split AUTO_SPEEDUP clearing from the flow-control FW
patch; it still applies to RTL8156B only.
Fixes: 195aae321c82 ("r8152: support new chips")
Fixes: fd3c7d080df5 ("r8152: Add support for the RTL8157 hardware")
Fixes: ad2a55b63b41 ("r8152: Add support for the RTL8159 chip")
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 124 +++++++++++++++++++++++++++++++++++-----
1 file changed, 109 insertions(+), 15 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 7c0de56266963..95f510da376de 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -89,6 +89,7 @@
#define PLA_MTPS 0xe615
#define PLA_TXFIFO_CTRL 0xe618
#define PLA_TXFIFO_FULL 0xe61a
+#define PLA_PAUSE_LIMIT 0xe61e
#define PLA_RSTTALLY 0xe800
#define PLA_CR 0xe813
#define PLA_CRWECR 0xe81c
@@ -301,6 +302,10 @@
#define MTPS_JUMBO (12 * 1024 / 64)
#define MTPS_DEFAULT (6 * 1024 / 64)
+/* PLA_PAUSE_LIMIT */
+#define PAUSE_LIMIT_EN BIT(3)
+#define PAUSE_LIMIT_MASK 0xf0
+
/* PLA_RSTTALLY */
#define TALLY_RESET 0x0001
@@ -6172,6 +6177,104 @@ static void r8152b_enter_oob(struct r8152 *tp)
RCR_APM | RCR_AM | RCR_AB);
}
+static void rtl_fc_pause_pkt_en(struct r8152 *tp, u16 speed)
+{
+ int log2_ratio, ratio;
+ u16 num_pause_pkts;
+ u32 ocp_data;
+
+ switch (tp->version) {
+ case RTL_VER_10:
+ case RTL_VER_11:
+ ocp_write_word(tp, MCU_TYPE_USB, USB_FC_TIMER,
+ CTRL_TIMER_EN | (1000 / 8));
+
+ ocp_word_set_bits(tp, MCU_TYPE_USB, USB_FW_CTRL,
+ FLOW_CTRL_PATCH_OPT);
+
+ ocp_word_set_bits(tp, MCU_TYPE_USB, USB_FW_TASK, FC_PATCH_TASK);
+ break;
+ case RTL_VER_12:
+ case RTL_VER_13:
+ case RTL_VER_15:
+ ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR, SLOT_EN);
+
+ ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_CPCR, FLOW_CTRL_EN);
+
+ /* enable fc timer and set timer to 600 ms. */
+ ocp_write_word(tp, MCU_TYPE_USB, USB_FC_TIMER,
+ CTRL_TIMER_EN | (600 / 8));
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_POL_GPIO_CTRL);
+ if (!(ocp_data & DACK_DET_EN))
+ ocp_word_set_bits(tp, MCU_TYPE_USB, USB_FW_CTRL,
+ FLOW_CTRL_PATCH_2);
+
+ ocp_word_set_bits(tp, MCU_TYPE_USB, USB_FW_TASK, FC_PATCH_TASK);
+ break;
+ case RTL_VER_16:
+ case RTL_VER_17_QFN68:
+ case RTL_VER_17_QFN100:
+ ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR, SLOT_EN);
+
+ num_pause_pkts = 0xa;
+ ratio = 10000;
+
+ if (speed & _10bps) {
+ ratio /= 10;
+ } else if (speed & _100bps) {
+ ratio /= 100;
+ } else if (speed & _1000bps) {
+ ratio /= 1000;
+ } else if (speed & _2500bps) {
+ ratio /= 2500;
+ } else if (speed & _5000bps) {
+ ratio /= 5000;
+ } else if (speed & _10000bps) {
+ ratio /= 10000;
+ } else {
+ /* No rate bit is set, e.g. a 500 or 1250 Mbit/s
+ * sub-rate, which the driver does not advertise.
+ * Disarm the limit.
+ */
+ dev_err(&tp->intf->dev, "Unknown link speed\n");
+ ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_PAUSE_LIMIT,
+ PAUSE_LIMIT_EN);
+ break;
+ }
+
+ log2_ratio = ilog2(ratio);
+ num_pause_pkts -= log2_ratio;
+
+ /* Round ratio up to the next power of two if it is
+ * more than halfway there. Floating-point is avoided
+ * by rewriting ratio > 1.5 * 2^log2_ratio as
+ * 2 * ratio > 3 * 2^log2_ratio.
+ */
+ if (2 * ratio > 3 * (1 << log2_ratio))
+ num_pause_pkts--;
+
+ /* At most 2^num_pause_pkts pause packets would be sent.
+ *
+ * 10 Mbit/s -> 2^0
+ * 100 Mbit/s -> 2^3
+ * 1 Gbit/s -> 2^7
+ * 2.5 Gbit/s -> 2^8
+ * 5 Gbit/s -> 2^9
+ * 10 Gbit/s -> 2^10
+ */
+ ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_PAUSE_LIMIT,
+ PAUSE_LIMIT_MASK | PAUSE_LIMIT_EN,
+ num_pause_pkts << 4);
+
+ ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_PAUSE_LIMIT,
+ PAUSE_LIMIT_EN);
+ break;
+ default:
+ break;
+ }
+}
+
static int r8153_pre_firmware_1(struct r8152 *tp)
{
int i;
@@ -6690,6 +6793,8 @@ static int rtl8157_enable(struct r8152 *tp)
r8153_set_rx_early_size(tp);
speed = rtl8152_get_speed(tp);
+ rtl_fc_pause_pkt_en(tp, speed);
+
rtl_set_ifg(tp, speed);
return rtl_enable(tp);
@@ -8759,7 +8864,6 @@ static void r8156b_u2phy_backup(struct r8152 *tp)
static void r8156_init(struct r8152 *tp)
{
- u32 ocp_data;
u16 data;
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
@@ -8846,24 +8950,14 @@ static void r8156_init(struct r8152 *tp)
usb_enable_lpm(tp->udev);
+ rtl_fc_pause_pkt_en(tp, 0);
+
switch (tp->version) {
case RTL_VER_12:
case RTL_VER_13:
case RTL_VER_15:
- ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR, SLOT_EN);
-
- ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_CPCR, FLOW_CTRL_EN);
-
- /* enable fc timer and set timer to 600 ms. */
- ocp_write_word(tp, MCU_TYPE_USB, USB_FC_TIMER, CTRL_TIMER_EN | (600 / 8));
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_FW_CTRL);
- if (!(ocp_read_word(tp, MCU_TYPE_PLA, PLA_POL_GPIO_CTRL) & DACK_DET_EN))
- ocp_data |= FLOW_CTRL_PATCH_2;
- ocp_data &= ~AUTO_SPEEDUP;
- ocp_write_word(tp, MCU_TYPE_USB, USB_FW_CTRL, ocp_data);
-
- ocp_word_set_bits(tp, MCU_TYPE_USB, USB_FW_TASK, FC_PATCH_TASK);
+ /* Disable Auto Speed up */
+ ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_FW_CTRL, AUTO_SPEEDUP);
break;
default:
break;
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH net-next v7 10/10] r8152: enable UPS for RTL8157 and RTL8159 QFN68
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
` (8 preceding siblings ...)
2026-10-14 8:19 ` [PATCH net-next v7 09/10] r8152: extract rtl_fc_pause_pkt_en() and apply it to RTL8156/8157/8159 Chih Kai Hsu
@ 2026-10-14 8:19 ` Chih Kai Hsu
2026-10-21 0:52 ` Jakub Kicinski
2026-10-14 8:25 ` [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support netdev-bot+sinfo
10 siblings, 1 reply; 16+ messages in thread
From: Chih Kai Hsu @ 2026-10-14 8:19 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
pabeni, hsu.chih.kai, andrew+netdev, mail
RTL8157 and RTL8159 QFN68 support UPS, but rtl8157_runtime_enable()
never toggled it on runtime suspend/resume, and r8156_ups_flags()
used the older per-chip EEE/lite_mode encoding instead of the
per-speed EEE flags their UPS firmware expects. RTL8159 QFN100 does
not support UPS.
Add r8157_ups_en(), mirroring r8156_ups_en(), and call it from
rtl8157_runtime_enable() for VER_16/QFN68 only; QFN100 keeps its
existing no-UPS behaviour there. Use r8157_ups_en(tp, false) in
r8157_init() in place of r8156_ups_en(tp, false).
Extend r8156_ups_flags() with per-speed EEE flags
(UPS_FLAGS_EN_100M_EEE/1000M_EEE/2500M_EEE/5000M_EEE/10G_EEE) and
skip the lite_mode bits[6:5] encoding for VER_16/QFN68. Add
ups_flags_speed() entries 10 and 11 for NWAY_5000M_FULL and
NWAY_10000M_FULL, previously unmapped.
Add the USB_FW_USB_VER register (0xcfd7), used by the new
r8157_ups_en() disable path.
If the backup-restore wait in r8157_ups_en() times out, it marks the
device inaccessible and logs, without queuing a reset, as
r8157_init() does for the same wait. During runtime resume the
interface then stays unusable until a replug or an unrelated reset,
since r8157_ups_en() returns void and the error is not passed back.
Recovery for these timeouts is left to a follow-up series.
Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
drivers/net/usb/r8152.c | 122 ++++++++++++++++++++++++++++++++++++----
1 file changed, 111 insertions(+), 11 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 95f510da376de..7a1b34db1d88b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -132,6 +132,7 @@
#define USB_BURST_SIZE 0xcfc0
#define USB_FW_FIX_EN0 0xcfca
#define USB_FW_FIX_EN1 0xcfcc
+#define USB_FW_USB_VER 0xcfd7
#define USB_LPM_CONFIG 0xcfd8
#define USB_ECM_OPTION 0xcfee
#define USB_CSTMR 0xcfef /* RTL8153A */
@@ -619,6 +620,11 @@
#define UPS_FLAGS_250M_CKDIV BIT(2)
#define UPS_FLAGS_EN_ALDPS BIT(3)
#define UPS_FLAGS_CTAP_SHORT_DIS BIT(4)
+#define UPS_FLAGS_EN_100M_EEE BIT(9)
+#define UPS_FLAGS_EN_1000M_EEE BIT(10)
+#define UPS_FLAGS_EN_2500M_EEE BIT(11)
+#define UPS_FLAGS_EN_5000M_EEE BIT(12)
+#define UPS_FLAGS_EN_10G_EEE BIT(13)
#define UPS_FLAGS_SPEED_MASK (0xf << 16)
#define ups_flags_speed(x) ((x) << 16)
#define UPS_FLAGS_EN_EEE BIT(20)
@@ -4244,8 +4250,27 @@ static void r8156_ups_flags(struct r8152 *tp)
if (tp->ups_info.aldps)
ups_flags |= UPS_FLAGS_EN_ALDPS;
- if (tp->ups_info.eee)
- ups_flags |= UPS_FLAGS_EN_EEE;
+ if (tp->ups_info.eee) {
+ switch (tp->version) {
+ case RTL_VER_17_QFN68:
+ if (tp->eee_adv & MDIO_EEE_10GT)
+ ups_flags |= UPS_FLAGS_EN_10G_EEE;
+ fallthrough;
+ case RTL_VER_16:
+ if (tp->eee_adv & MDIO_EEE_100TX)
+ ups_flags |= UPS_FLAGS_EN_100M_EEE;
+ if (tp->eee_adv & MDIO_EEE_1000T)
+ ups_flags |= UPS_FLAGS_EN_1000M_EEE;
+ if (tp->eee_adv2 & MDIO_EEE_2_5GT)
+ ups_flags |= UPS_FLAGS_EN_2500M_EEE;
+ if (tp->eee_adv2 & MDIO_EEE_5GT)
+ ups_flags |= UPS_FLAGS_EN_5000M_EEE;
+ break;
+ default:
+ ups_flags |= UPS_FLAGS_EN_EEE;
+ break;
+ }
+ }
if (tp->ups_info.flow_control)
ups_flags |= UPS_FLAGS_EN_FLOW_CTR;
@@ -4296,20 +4321,33 @@ static void r8156_ups_flags(struct r8152 *tp)
case NWAY_2500M_FULL:
ups_flags |= ups_flags_speed(9);
break;
+ case NWAY_5000M_FULL:
+ ups_flags |= ups_flags_speed(10);
+ break;
+ case NWAY_10000M_FULL:
+ ups_flags |= ups_flags_speed(11);
+ break;
default:
break;
}
- switch (tp->ups_info.lite_mode) {
- case 1:
- ups_flags |= 0 << 5;
- break;
- case 2:
- ups_flags |= 2 << 5;
+ switch (tp->version) {
+ case RTL_VER_16:
+ case RTL_VER_17_QFN68:
break;
- case 0:
default:
- ups_flags |= 1 << 5;
+ switch (tp->ups_info.lite_mode) {
+ case 1:
+ ups_flags |= 0 << 5;
+ break;
+ case 2:
+ ups_flags |= 2 << 5;
+ break;
+ case 0:
+ default:
+ ups_flags |= 1 << 5;
+ break;
+ }
break;
}
@@ -4484,6 +4522,49 @@ static void r8156_ups_en(struct r8152 *tp, bool enable)
}
}
+static void r8157_ups_en(struct r8152 *tp, bool enable)
+{
+ if (enable) {
+ r8156_ups_flags(tp);
+
+ ocp_byte_set_bits(tp, MCU_TYPE_USB, USB_POWER_CUT,
+ UPS_EN | USP_PREWAKE | PHASE2_EN);
+
+ ocp_byte_set_bits(tp, MCU_TYPE_USB, USB_MISC_2,
+ UPS_FORCE_PWR_DOWN);
+ } else {
+ ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_POWER_CUT,
+ UPS_EN | USP_PREWAKE);
+
+ ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_MISC_2,
+ UPS_FORCE_PWR_DOWN);
+
+ if (ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0) & PCUT_STATUS) {
+ u32 ocp_data;
+ int ret;
+
+ ret = read_poll_timeout(ocp_read_word, ocp_data,
+ ocp_data & BACKUP_RESTRORE, 200,
+ 20000, false, tp, MCU_TYPE_USB,
+ USB_GPHY_CTRL);
+ if (ret) {
+ rtl_set_inaccessible(tp);
+ dev_err(&tp->intf->dev,
+ "backup-restore timed out\n");
+ return;
+ }
+
+ /* clear USB fw_ver_reg */
+ ocp_write_byte(tp, MCU_TYPE_USB, USB_FW_USB_VER, 0);
+
+ tp->rtl_ops.hw_phy_cfg(tp);
+
+ rtl8152_set_speed(tp, tp->autoneg, tp->speed,
+ tp->duplex, tp->advertising);
+ }
+ }
+}
+
static void r8153_power_cut_en(struct r8152 *tp, bool enable)
{
if (enable)
@@ -4642,9 +4723,28 @@ static void rtl8157_runtime_enable(struct r8152 *tp, bool enable)
r8153b_u1u2en(tp, false);
r8157_u2p3en(tp, false);
rtl_runtime_suspend_enable(tp, true);
+
+ switch (tp->version) {
+ case RTL_VER_16:
+ case RTL_VER_17_QFN68:
+ r8157_ups_en(tp, true);
+ break;
+ default:
+ break;
+ }
} else {
r8153_queue_wake(tp, false);
rtl_runtime_suspend_enable(tp, false);
+
+ switch (tp->version) {
+ case RTL_VER_16:
+ case RTL_VER_17_QFN68:
+ r8157_ups_en(tp, false);
+ break;
+ default:
+ break;
+ }
+
r8157_u2p3en(tp, true);
if (tp->udev->speed >= USB_SPEED_SUPER)
r8153b_u1u2en(tp, true);
@@ -9066,7 +9166,7 @@ static void r8157_init(struct r8152 *tp)
ocp_write_word(tp, MCU_TYPE_USB, USB_U1U2_TIMER, 500);
r8157_power_cut_en(tp, false);
- r8156_ups_en(tp, false);
+ r8157_ups_en(tp, false);
r8153_queue_wake(tp, false);
rtl_runtime_suspend_enable(tp, false);
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v7 10/10] r8152: enable UPS for RTL8157 and RTL8159 QFN68
2026-10-14 8:19 ` [PATCH net-next v7 10/10] r8152: enable UPS for RTL8157 and RTL8159 QFN68 Chih Kai Hsu
@ 2026-10-21 0:52 ` Jakub Kicinski
0 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2026-10-21 0:52 UTC (permalink / raw)
To: hsu.chih.kai
Cc: Jakub Kicinski, davem, netdev, nic_swsd, linux-kernel, linux-usb,
edumazet, bjorn, pabeni, andrew+netdev, mail
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: enable UPS for RTL8157 and RTL8159 QFN68
This patch adds r8157_ups_en() and calls it from rtl8157_runtime_enable(),
so UPS is now switched on and off during runtime suspend and resume for
RTL_VER_16 and RTL_VER_17_QFN68. It also adds per-speed EEE flags and
5G/10G speed encodings to r8156_ups_flags(), and skips the lite_mode
encoding for those chips.
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 95f510da376de..7a1b34db1d88b 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[ ... ]
> @@ -4484,6 +4522,49 @@ static void r8156_ups_en(struct r8152 *tp, bool enable)
> }
> }
>
> +static void r8157_ups_en(struct r8152 *tp, bool enable)
> +{
> + if (enable) {
> + r8156_ups_flags(tp);
> +
> + ocp_byte_set_bits(tp, MCU_TYPE_USB, USB_POWER_CUT,
> + UPS_EN | USP_PREWAKE | PHASE2_EN);
[Severity: Medium]
This is a pre-existing issue, but this patch adds more callers of the
ocp_byte_w0w1() read/modify/write path, and that path depends on
get_registers():
drivers/net/usb/r8152.c:get_registers() {
...
if (ret < 0)
memset(data, 0xff, size);
else
memcpy(data, tmp, size);
...
}
usb_control_msg() accepts short IN transfers. Could a malicious device
reply with fewer than size bytes and leave stale kmalloc() data in the
part of tmp it never wrote?
ocp_byte_w0w1() then writes that byte back with only the masked bits
changed:
ocp_data = ocp_read_byte(tp, type, index);
ocp_data = (ocp_data & ~clear) | set;
ocp_write_byte(tp, type, index, ocp_data);
That would let the device read back up to 8 bits of stale heap data per
read/modify/write. This does not happen with init_on_alloc. Every
read/modify/write in the driver has the same pattern, and this patch does
not change get_registers().
> +
> + ocp_byte_set_bits(tp, MCU_TYPE_USB, USB_MISC_2,
> + UPS_FORCE_PWR_DOWN);
> + } else {
> + ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_POWER_CUT,
> + UPS_EN | USP_PREWAKE);
> +
> + ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_MISC_2,
> + UPS_FORCE_PWR_DOWN);
> +
> + if (ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0) & PCUT_STATUS) {
[Severity: Low]
This isn't a bug introduced by this patch, but ocp_read_word() ignores
the return value of generic_ocp_read() and uses its local tmp anyway:
drivers/net/usb/r8152.c:ocp_read_word() {
__le32 tmp;
...
generic_ocp_read(tp, index, sizeof(tmp), &tmp, type | byen);
data = __le32_to_cpu(tmp);
...
}
If RTL8152_INACCESSIBLE is already set, generic_ocp_read() returns
-ENODEV without writing the buffer. The same happens if kmalloc() fails
in get_registers(). Would the PCUT_STATUS check here, and the
BACKUP_RESTRORE poll below, then test an uninitialized stack value?
When the device is inaccessible, the writes that follow do nothing, so
the most likely visible effect is a KMSAN report. Every ocp_read_*
caller is affected.
> + u32 ocp_data;
> + int ret;
> +
> + ret = read_poll_timeout(ocp_read_word, ocp_data,
> + ocp_data & BACKUP_RESTRORE, 200,
> + 20000, false, tp, MCU_TYPE_USB,
> + USB_GPHY_CTRL);
> + if (ret) {
> + rtl_set_inaccessible(tp);
> + dev_err(&tp->intf->dev,
> + "backup-restore timed out\n");
> + return;
> + }
[Severity: Medium]
Can this leave the interface unusable after a runtime resume, with
nothing to recover it?
With this patch, runtime suspend on RTL_VER_16 and RTL_VER_17_QFN68
enables UPS power cut by default. That means any runtime resume that
finds PCUT_STATUS set will run this wait:
rtl8152_runtime_resume()
tp->rtl_ops.autosuspend_en(tp, false)
rtl8157_runtime_enable(tp, false)
r8157_ups_en(tp, false)
read_poll_timeout() times out
rtl_set_inaccessible(tp)
r8157_ups_en() returns void, so rtl8157_runtime_enable() carries on, and
r8157_u2p3en() and r8153b_u1u2en() quietly do nothing.
rtl8152_runtime_resume() then runs napi_disable()/napi_enable(),
rtl8152_get_speed() and usb_submit_urb(), and returns 0.
rtl8152_resume() has a reset fallback, but it is skipped for runtime
resume:
if (!runtime_resume && test_bit(RTL8152_INACCESSIBLE, &tp->flags))
usb_reset_device(tp->udev);
Once the flag is set, r8152_control_msg() also returns early:
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
return -ENODEV;
This happens before it gets to the usb_queue_reset_device() logic, so
later register accesses cannot start a recovery either.
The netdev stays registered and up while every hardware access is
skipped. That lasts until a replug, a system suspend/resume, or an
unrelated reset. Before this patch, rtl8157_runtime_enable() had no such
wait.
The commit message mentions this and defers recovery to a follow-up
series. Since this path is on by default for these chips, should this
patch at least queue a USB reset here (for example with
usb_queue_reset_device(tp->intf)) or pass the error back?
> +
> + /* clear USB fw_ver_reg */
> + ocp_write_byte(tp, MCU_TYPE_USB, USB_FW_USB_VER, 0);
> +
> + tp->rtl_ops.hw_phy_cfg(tp);
[Severity: Low]
This isn't a bug introduced by this patch, but r8157_hw_phy_cfg() can
return early after it has already turned EEE and ALDPS off:
drivers/net/usb/r8152.c:r8157_hw_phy_cfg() {
...
r8153_aldps_en(tp, false);
rtl_eee_enable(tp, false);
...
if (rtl_phy_patch_request(tp, true, true))
return;
...
}
If the PHY patch handshake times out (for example -ETIME when
PATCH_READY never appears), the code skips re-enabling EEE and ALDPS and
skips set_bit(PHY_RESET). By then r8153_eee_en() has already cleared
tp->ups_info.eee.
hw_phy_cfg returns void, so r8157_ups_en() goes straight on to
rtl8152_set_speed() and runtime resume reports success. Does EEE and
ALDPS then stay off until the PHY is reconfigured? Would the next UPS
suspend also leave out the new per-speed EEE flags in r8156_ups_flags()?
The hw_phy_work path and r8153b_ups_en()/r8156_ups_en() share the same
early return. This patch adds one more caller.
> +
> + rtl8152_set_speed(tp, tp->autoneg, tp->speed,
> + tp->duplex, tp->advertising);
> + }
> + }
> +}
[ ... ]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support
2026-10-14 8:18 [PATCH net-next v7 00/10] r8152: refactor and extend RTL8157/8159 support Chih Kai Hsu
` (9 preceding siblings ...)
2026-10-14 8:19 ` [PATCH net-next v7 10/10] r8152: enable UPS for RTL8157 and RTL8159 QFN68 Chih Kai Hsu
@ 2026-10-14 8:25 ` netdev-bot+sinfo
10 siblings, 0 replies; 16+ messages in thread
From: netdev-bot+sinfo @ 2026-10-14 8:25 UTC (permalink / raw)
To: Chih Kai Hsu
Cc: davem, kuba, netdev, nic_swsd, linux-kernel, linux-usb, edumazet,
bjorn, pabeni, andrew+netdev, mail
Hi!
This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:
- Whether the issue was actually triggered, or is only theoretical
(e.g. found by code inspection). If it was triggered please include
the symptoms, like the stack trace or error messages.
Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.
^ permalink raw reply [flat|nested] 16+ messages in thread