[PATCH net-next v3 0/8] r8152: refactor and extend RTL8157/8159 support

[PATCH net-next v3 0/8] r8152: refactor and extend RTL8157/8159 support

[Chih Kai Hsu]

v3:
Rebase on top of latest net-next to resolve merge conflicts.

v2:
- For patch #1, use read_poll_timeout() to replace the manual for-loop polling.
- For patch #2 and #3, rewrite the commit message.

v1:
RTL8157 and RTL8159 have different init, enable, up/down, hw_phy_cfg,
and PHY access sequences from RTL8156.  This series splits the shared
callbacks into per-chip variants and adds the missing RTL8157/8159
functionality: dedicated unload and change_mtu callbacks, TGPHY register
access via phy_read/phy_write pointers in struct rtl_ops, flow control
patch support through rtl_fc_pause_pkt_en(), and UPS support via
r8157_ups_en().

Chih Kai Hsu (8):
  r8152: refactor r8156_init
  r8152: support RTL8159 for different packages
  r8152: refactor rtl8156_enable, rtl8156_up, and rtl8156_down
  r8152: refactor r8157_hw_phy_cfg
  r8152: support rtl8157_unload and rtl8157_change_mtu
  r8152: add TGPHY access support
  r8152: support rtl_fc_pause_pkt_en()
  r8152: support UPS for RTL8157 and RTL8159

 drivers/net/usb/r8152.c | 1297 +++++++++++++++++++++++++++++++--------
 1 file changed, 1054 insertions(+), 243 deletions(-)

-- 
2.34.1

[PATCH net-next v3 1/8] r8152: refactor r8156_init

[Chih Kai Hsu]

RTL8156B, RTL8157 and RTL8159 have different init sequences from RTL8156.

Split r8156_init into per-chip functions:
- r8156_init (VER_10/11)
- r8156b_init (VER_12/13/15)
- r8157_init (VER_16)
- r8159_init (VER_17)

Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
 drivers/net/usb/r8152.c | 488 ++++++++++++++++++++++++++++------------
 1 file changed, 349 insertions(+), 139 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f61686433031c..013e8d1abfc24 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
@@ -4193,6 +4199,22 @@ 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,
+				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 ret;
+}
+
 static void r8153b_ups_en(struct r8152 *tp, bool enable)
 {
 	if (enable) {
@@ -4211,16 +4233,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);
 
@@ -4248,16 +4262,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);
 
@@ -7246,22 +7252,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);
 
@@ -7362,22 +7360,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);
 
@@ -7432,8 +7421,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;
 
@@ -7446,15 +7433,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);
 
@@ -8351,90 +8331,132 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
 	set_bit(PHY_RESET, &tp->flags);
 }
 
-static int r8159_wait_backup_restore(struct r8152 *tp)
+static void r8156_init(struct r8152 *tp)
 {
-	u32 ocp_data;
+	u16 data;
 
-	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
-	if (!(ocp_data & PCUT_STATUS))
-		return 0;
+	if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
+		return;
 
-	return poll_timeout_us(ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_GPHY_CTRL),
-			       ocp_data & BACKUP_RESTRORE, 200, 20000, false);
+	ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_ECM_OP, EN_ALL_SPEED);
+
+	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);
+
+	r8153b_u1u2en(tp, false);
+
+	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));
+
+	r8152_mdio_test_and_clr_bit(tp, MII_BMCR, BMCR_PDOWN);
+
+	data = r8153_phy_status(tp, PHY_STAT_LAN_ON);
+
+	r8153_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);
+
+	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);
+
+	usb_enable_lpm(tp->udev);
+
+	r8156_mac_clk_spd(tp, true);
+
+	ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
+			  PLA_MCU_SPDWN_EN);
+
+	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);
+
+	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL,
+			  RX_AGG_DISABLE | RX_ZERO_EN);
+
+	ocp_byte_set_bits(tp, MCU_TYPE_USB, USB_BMU_CONFIG, ACT_ODMA);
+
+	r8156_mdio_force_mode(tp);
+	rtl_tally_reset(tp);
+
+	tp->coalesce = 15000; /* 15 us */
 }
 
-static void r8156_init(struct r8152 *tp)
+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 r8156b_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)
-		ocp_word_set_bits(tp, MCU_TYPE_USB, USB_U2P3_CTRL, RX_DETECT8);
+	ocp_word_set_bits(tp, MCU_TYPE_USB, USB_U2P3_CTRL, RX_DETECT8);
 
 	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)
-			ocp_reg_clr_bits(tp, 0xa466, BIT(0));
+		ocp_reg_clr_bits(tp, 0xa466, BIT(0));
 	}
 
-	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);
+
+	/* Disable Auto Speed up */
+	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_FW_CTRL, AUTO_SPEEDUP);
 
 	/* MSC timer = 0xfff * 8ms = 32760 ms */
 	ocp_write_word(tp, MCU_TYPE_USB, USB_MSC_TIMER, 0x0fff);
@@ -8442,73 +8464,261 @@ 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);
+	r8156b_u2phy_backup(tp);
 
+	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);
 
 	usb_enable_lpm(tp->udev);
 
-	if (tp->version >= RTL_VER_12 && tp->version <= RTL_VER_15) {
-		ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR, SLOT_EN);
+	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);
+	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));
+	/* 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_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);
-	}
+	ocp_word_set_bits(tp, MCU_TYPE_USB, USB_FW_TASK, FC_PATCH_TASK);
 
 	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);
 
-	/* RX aggregation / 16 bytes RX descriptor
-	 * BIT(11) is specific to RTL8159, with unknown meaning
-	 */
-	if (tp->version == 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);
+	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL,
+			  RX_AGG_DISABLE | RX_ZERO_EN);
+
+	r8156_mdio_force_mode(tp);
+	rtl_tally_reset(tp);
+
+	tp->coalesce = 15000;	/* 15 us */
+}
+
+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);
+
+	if (wait_autoload_done(tp))
+		return;
+
+	r8156b_wait_loading_flash(tp);
+
+	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);
+
+	/* Disable Interrupt Mitigation */
+	ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xcf04,
+			  BIT(0) | BIT(1) | BIT(2) | BIT(7));
+
+	/* Disable Auto Speed up */
+	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_FW_CTRL, AUTO_SPEEDUP);
+
+	/* 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);
+
+	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));
+
+	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_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL, RX_AGG_DISABLE | RX_ZERO_EN);
+		ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_EXTRA_STATUS,
+			      CUR_LINK_OK, POLL_LINK_CHG);
 
-	if (tp->version < RTL_VER_12)
-		ocp_byte_set_bits(tp, MCU_TYPE_USB, USB_BMU_CONFIG, ACT_ODMA);
+	set_bit(GREEN_ETHERNET, &tp->flags);
 
-	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));
+	/* 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));
+
+	/* TX descriptor Signature */
+	ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd4ae, BIT(1));
+
+	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 r8159_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);
+
+	if (wait_autoload_done(tp))
+		return;
+
+	if (r8159_wait_backup_restore(tp)) {
+		rtl_set_inaccessible(tp);
+		dev_err(&tp->intf->dev,
+			"init failed, backup-restore timed out\n");
+		return;
 	}
 
+	r8156b_wait_loading_flash(tp);
+
+	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);
+
+	/* Disable Interrupt Mitigation */
+	ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xcf04,
+			  BIT(0) | BIT(1) | BIT(2) | BIT(7));
+
+	/* Disable Auto Speed up */
+	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_FW_CTRL, AUTO_SPEEDUP);
+
+	/* 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);
+
+	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));
+
+	/* Disable FW u1u2 patch option */
+	ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xb9a6, BIT(0));
+
+	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 / 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));
+
+	/* TX descriptor Signature */
+	ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd4ae, BIT(1));
+
+	/* Enable u3phy patch backup */
+	ocp_write_word(tp, MCU_TYPE_USB, 0xb9a2, 0x0448);
+
 	r8156_mdio_force_mode(tp);
 	rtl_tally_reset(tp);
 
@@ -9833,7 +10043,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;
-		ops->init		= r8156_init;
+		ops->init		= r8156b_init;
 		ops->enable		= rtl8156_enable;
 		ops->disable		= rtl8153_disable;
 		ops->up			= rtl8156_up;
@@ -9872,7 +10082,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;
@@ -9894,7 +10104,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		= r8159_init;
 		ops->enable		= rtl8156_enable;
 		ops->disable		= rtl8153_disable;
 		ops->up			= rtl8156_up;
-- 
2.34.1

[PATCH net-next v3 2/8] r8152: support RTL8159 for different packages

[Chih Kai Hsu]

RTL8159 comes in two packages, QFN68 and QFN100, which require different
handling. Split RTL_VER_17 into RTL_VER_17_QFN68 and RTL_VER_17_QFN100.

Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
 drivers/net/usb/r8152.c | 93 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 78 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 013e8d1abfc24..f01f442fa71a0 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
 };
@@ -3439,7 +3444,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;
 
@@ -3479,7 +3485,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)
@@ -3667,7 +3673,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,
@@ -3712,7 +3719,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;
@@ -3828,6 +3836,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,
@@ -4507,6 +4517,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.
@@ -4561,7 +4573,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;
@@ -4673,7 +4686,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;
 	}
@@ -5833,7 +5847,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);
@@ -6424,8 +6439,15 @@ 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)
-		ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_RX_AGGR_NUM, RX_AGGR_NUM_MASK);
+	switch (tp->version) {
+	case RTL_VER_10:
+	case RTL_VER_11:
+		break;
+	default:
+		ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_RX_AGGR_NUM,
+				  RX_AGGR_NUM_MASK);
+		break;
+	}
 
 	r8153_set_rx_early_timeout(tp);
 	r8153_set_rx_early_size(tp);
@@ -8113,7 +8135,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));
 
@@ -8716,6 +8739,10 @@ static void r8159_init(struct r8152 *tp)
 	/* TX descriptor Signature */
 	ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd4ae, BIT(1));
 
+	/* Enable u2phy backup restore patch */
+	if (tp->version == RTL_VER_17_QFN68)
+		ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xb99c, BIT(0));
+
 	/* Enable u3phy patch backup */
 	ocp_write_word(tp, MCU_TYPE_USB, 0xb9a2, 0x0448);
 
@@ -10100,7 +10127,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;
@@ -10192,7 +10220,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:
@@ -10202,9 +10231,33 @@ 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 > 0)
+		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;
@@ -10287,7 +10340,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 %#02x\n",
+				 pkg_det);
+		}
 		break;
 	default:
 		version = RTL_VER_UNKNOWN;
@@ -10446,7 +10508,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

[PATCH net-next v3 3/8] r8152: refactor rtl8156_enable, rtl8156_up, and rtl8156_down

[Chih Kai Hsu]

RTL8156B and RTL8157/8159 have different enable, up, and down sequences
from RTL8156. Add dedicated rtl8156b_enable (VER_12/13/15),
rtl8157_enable, rtl8157_up, and rtl8157_down (VER_16/17) instead of
handling per-version differences with inline version guards.

Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
 drivers/net/usb/r8152.c | 235 ++++++++++++++++++++++++++++++++--------
 1 file changed, 189 insertions(+), 46 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f01f442fa71a0..6c189790b0bab 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -6433,31 +6433,16 @@ static int rtl8156_enable(struct r8152 *tp)
 	if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
 		return -ENODEV;
 
-	if (tp->version < RTL_VER_12)
-		r8156_fc_parameter(tp);
-
+	r8156_fc_parameter(tp);
 	set_tx_qlen(tp);
 	rtl_set_eee_plus(tp);
 
-	switch (tp->version) {
-	case RTL_VER_10:
-	case RTL_VER_11:
-		break;
-	default:
-		ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_RX_AGGR_NUM,
-				  RX_AGGR_NUM_MASK);
-		break;
-	}
-
 	r8153_set_rx_early_timeout(tp);
 	r8153_set_rx_early_size(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);
@@ -6465,12 +6450,10 @@ 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) {
-		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);
-	}
+	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);
 
 	if (tp->udev->speed == USB_SPEED_HIGH) {
 		/* USB 0xb45e[3:0] l1_nyet_hird */
@@ -6495,6 +6478,67 @@ static void rtl8156_disable(struct r8152 *tp)
 	rtl8153_disable(tp);
 }
 
+static int rtl8156b_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);
+
+	if (speed & _2500bps)
+		ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4,
+				  IDLE_SPDWN_EN);
+	else
+		ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4,
+				  IDLE_SPDWN_EN);
+
+	if (tp->udev->speed == USB_SPEED_HIGH) {
+		/* USB 0xb45e[3:0] l1_nyet_hird */
+		if (is_flow_control(speed))
+			ocp_word_w0w1(tp, MCU_TYPE_USB, USB_L1_CTRL, 0xf, 0xf);
+		else
+			ocp_word_w0w1(tp, MCU_TYPE_USB, USB_L1_CTRL, 0xf, 0x1);
+	}
+
+	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_FW_TASK, FC_PATCH_TASK);
+	usleep_range(1000, 2000);
+	ocp_word_set_bits(tp, MCU_TYPE_USB, USB_FW_TASK, FC_PATCH_TASK);
+
+	return rtl_enable(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)
 {
@@ -6855,8 +6899,7 @@ static void rtl8156_up(struct r8152 *tp)
 		return;
 
 	r8153b_u1u2en(tp, false);
-	if (tp->version < RTL_VER_16)
-		r8153_u2p3en(tp, false);
+	r8153_u2p3en(tp, false);
 	r8153_aldps_en(tp, false);
 
 	rxdy_gated_en(tp, true);
@@ -6869,8 +6912,7 @@ static void rtl8156_up(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);
 
@@ -6892,11 +6934,11 @@ static void rtl8156_up(struct r8152 *tp)
 	ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_RXFIFO_FULL, RXFIFO_FULL_MASK,
 		      0x08);
 
-	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);
 
-	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);
 
@@ -6906,10 +6948,19 @@ 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)
+	switch (tp->version) {
+	case RTL_VER_13:
+	case RTL_VER_15:
+		/* Enable Clear_SDR */
+		ocp_word_set_bits(tp, MCU_TYPE_USB, 0xd3ca, BIT(15));
+		break;
+	default:
+		break;
+	}
+
+	if (tp->udev->speed >= USB_SPEED_SUPER)
 		r8153b_u1u2en(tp, true);
 }
 
@@ -6922,12 +6973,9 @@ static void rtl8156_down(struct r8152 *tp)
 
 	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_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);
@@ -6949,7 +6997,102 @@ static void rtl8156_down(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);
+
+	rtl_rx_vlan_en(tp, true);
+	rxdy_gated_en(tp, false);
+
+	ocp_dword_set_bits(tp, MCU_TYPE_PLA, PLA_RCR,
+			   RCR_APM | RCR_AM | RCR_AB);
+
+	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);
+
+	rxdy_gated_en(tp, true);
+	r8153_teredo_off(tp);
+
+	ocp_dword_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR, RCR_ACPT_ALL);
+
+	rtl8152_nic_reset(tp);
+	rtl_reset_bmu(tp);
+
+	ocp_byte_clr_bits(tp, MCU_TYPE_PLA, PLA_OOB_CTRL, NOW_IS_OOB);
+
+	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);
+
+	rtl_rx_vlan_en(tp, tp->netdev->features & NETIF_F_HW_VLAN_CTAG_RX);
+
+	rtl8156_change_mtu(tp);
+
+	/* share FIFO settings */
+	ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_RXFIFO_FULL, RXFIFO_FULL_MASK,
+		      0x08);
+
+	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);
+
+	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);
+
+	/* Clear_SDR */
+	ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xd378, BIT(7));
+	ocp_word_clr_bits(tp, MCU_TYPE_USB, 0xcd06, BIT(15));
+}
+
+static void rtl8157_down(struct r8152 *tp)
+{
+	if (test_bit(RTL8152_INACCESSIBLE, &tp->flags)) {
+		rtl_drop_queued_tx(tp);
+		return;
+	}
+
+	r8153b_u1u2en(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 */
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RXFIFO_FULL, 64 / 16);
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RX_FIFO_FULL, 1024 / 16);
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RX_FIFO_EMPTY, 4096 / 16);
+
+	rtl_disable(tp);
+	rtl_reset_bmu(tp);
+
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, 1526);
+	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, 10 * 1024 / 64);
+
+	/* Clear teredo wake event. bit[15:8] is the teredo wakeup
+	 * type. Set it to zero. bits[7:0] are the W1C bits about
+	 * the events. Set them to all 1 to clear them.
+	 */
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_TEREDO_WAKE_BASE, 0x00ff);
+
+	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);
 
@@ -10071,7 +10214,7 @@ static int rtl_ops_init(struct r8152 *tp)
 		tp->eee_adv		= MDIO_EEE_1000T | MDIO_EEE_100TX;
 		tp->eee_adv2		= MDIO_EEE_2_5GT;
 		ops->init		= r8156b_init;
-		ops->enable		= rtl8156_enable;
+		ops->enable		= rtl8156b_enable;
 		ops->disable		= rtl8153_disable;
 		ops->up			= rtl8156_up;
 		ops->down		= rtl8156_down;
@@ -10110,10 +10253,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;
@@ -10133,10 +10276,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		= r8159_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

[PATCH net-next v3 4/8] r8152: refactor r8157_hw_phy_cfg

[Chih Kai Hsu]

RTL8159 has a different hw_phy_cfg sequence from RTL8157. Split
r8157_hw_phy_cfg into r8157_hw_phy_cfg (VER_16) and r8159_hw_phy_cfg
(VER_17_*), update PHY parameters for RTL8156B/57/59, and add sram2
bitwise operation helpers.

Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
 drivers/net/usb/r8152.c | 168 +++++++++++++++++++++++++++++++++-------
 1 file changed, 139 insertions(+), 29 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 6c189790b0bab..679aead731f73 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 */
@@ -1985,6 +1986,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;
@@ -8075,6 +8086,9 @@ static void r8156b_hw_phy_cfg(struct r8152 *tp)
 		sram_write(tp, 0x8074, 0x2417);
 		sram_write(tp, 0x807a, 0x2417);
 
+		/* Nway DACONB parameters */
+		ocp_reg_w0w1(tp, 0xa4ca, 0x6000, 0x0040);
+
 		/* XG PLL */
 		ocp_reg_w0w1(tp, 0xbf84, 0xe000, 0xa000);
 		break;
@@ -8151,11 +8165,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 */
@@ -8171,7 +8188,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);
@@ -8277,12 +8294,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
 		 */
@@ -8292,22 +8382,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);
@@ -8371,10 +8474,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 */
@@ -8383,14 +8486,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);
@@ -8401,7 +8504,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);
 
@@ -8460,8 +8563,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;
 	}
@@ -8471,9 +8581,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;
 
@@ -8485,7 +8595,7 @@ 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)
@@ -10284,7 +10394,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

[PATCH net-next v3 5/8] r8152: support rtl8157_unload and rtl8157_change_mtu

[Chih Kai Hsu]

RTL8157 and RTL8159 need a dedicated unload callback to disable interrupt
mitigation and use r8157_power_cut_en instead of r8153_power_cut_en. They
also need a dedicated change_mtu that scales MTPS between 10K and 16K
depending on the MTU, unlike rtl8156_change_mtu which uses a fixed MTPS.

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 679aead731f73..06fad895fce09 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -7024,6 +7024,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))
@@ -7048,7 +7070,7 @@ static void rtl8157_up(struct r8152 *tp)
 
 	rtl_rx_vlan_en(tp, tp->netdev->features & NETIF_F_HW_VLAN_CTAG_RX);
 
-	rtl8156_change_mtu(tp);
+	rtl8157_change_mtu(tp);
 
 	/* share FIFO settings */
 	ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_RXFIFO_FULL, RXFIFO_FULL_MASK,
@@ -10174,11 +10196,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)
@@ -10189,6 +10206,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);
@@ -10367,13 +10394,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;
@@ -10390,13 +10417,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

[PATCH net-next v3 6/8] r8152: add TGPHY access support

[Chih Kai Hsu]

RTL8157 and RTL8159 support a TGPHY interface via USB_TGPHY_CMD/ADDR/DATA,
which allows reading/writing PHY registers without changing the OCP base
address at 0xe86c. Add r8157_phy_read/phy_write implementing this path,
and add phy_read/phy_write function pointers to struct rtl_ops to dispatch
the correct access method per chip.

Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
 drivers/net/usb/r8152.c | 94 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 06fad895fce09..1fcb1cc5b4a18 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);
+		u16 (*phy_read)(struct r8152 *tp, u16 addr);
+		void (*phy_write)(struct r8152 *tp, u16 addr, u16 data);
 	} rtl_ops;
 
 	struct ups_info {
@@ -1659,7 +1668,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 u16 r8152_phy_read(struct r8152 *tp, u16 addr)
 {
 	u16 ocp_base, ocp_index;
 
@@ -1673,7 +1682,7 @@ static u16 ocp_reg_read(struct r8152 *tp, u16 addr)
 	return ocp_read_word(tp, MCU_TYPE_PLA, ocp_index);
 }
 
-static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
+static void r8152_phy_write(struct r8152 *tp, u16 addr, u16 data)
 {
 	u16 ocp_base, ocp_index;
 
@@ -1687,6 +1696,16 @@ static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
 	ocp_write_word(tp, MCU_TYPE_PLA, ocp_index, data);
 }
 
+static u16 ocp_reg_read(struct r8152 *tp, u16 addr)
+{
+	return tp->rtl_ops.phy_read(tp, addr);
+}
+
+static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
+{
+	tp->rtl_ops.phy_write(tp, addr, data);
+}
+
 static inline void r8152_mdio_write(struct r8152 *tp, u32 reg_addr, u32 value)
 {
 	ocp_reg_write(tp, OCP_BASE_MII + reg_addr * 2, value);
@@ -2023,6 +2042,61 @@ static int r8152_mdio_test_and_clr_bit(struct r8152 *tp, u16 addr, u16 clear)
 	return data & clear;
 }
 
+static int wait_tgphy_cmd_ready(struct r8152 *tp)
+{
+	u16 ocp_data;
+
+	return poll_timeout_us(ocp_data = ocp_read_word(tp, MCU_TYPE_USB,
+							USB_TGPHY_CMD),
+			       !(ocp_data & TGPHY_CMD_BUSY), 2000, 20000,
+			       false);
+}
+
+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 u16 r8157_phy_read(struct r8152 *tp, u16 addr)
+{
+	u16 data = 0;
+
+	rtl_tgphy_access(tp, addr, &data, false);
+
+	return data;
+}
+
+static void r8157_phy_write(struct r8152 *tp, u16 addr, u16 data)
+{
+	rtl_tgphy_access(tp, addr, &data, true);
+}
+
 static int
 r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags);
 
@@ -10268,6 +10342,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;
@@ -10290,6 +10366,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
@@ -10313,6 +10391,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;
@@ -10337,6 +10417,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);
@@ -10362,6 +10444,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;
@@ -10379,6 +10463,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;
@@ -10401,6 +10487,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;
@@ -10424,6 +10512,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

[PATCH net-next v3 7/8] r8152: support rtl_fc_pause_pkt_en()

[Chih Kai Hsu]

The flow control patch was inline in r8156b_init and missing for RTL8156
and RTL8157/8159. Extract it as rtl_fc_pause_pkt_en(), call it from
r8156_init and r8156b_init during chip init, and call it from
rtl8157_enable with the current link speed for RTL8157 and RTL8159.

Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
 drivers/net/usb/r8152.c | 112 ++++++++++++++++++++++++++++++++++------
 1 file changed, 97 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 1fcb1cc5b4a18..2b0972b967385 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
 
@@ -6088,6 +6093,93 @@ 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 & LINK_STATUS)) {
+			dev_dbg(&tp->intf->dev, "No link\n");
+			goto no_link;
+		} else 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 {
+			dev_err(&tp->intf->dev, "Unknown link speed\n");
+			goto no_link;
+		}
+
+		log2_ratio = ilog2(ratio);
+		num_pause_pkts -= log2_ratio;
+
+		/* Round up if ratio is more than halfway to the next power of 2.
+		 * 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--;
+
+no_link:
+		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;
@@ -6619,6 +6711,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);
@@ -8751,6 +8845,8 @@ static void r8156_init(struct r8152 *tp)
 
 	usb_enable_lpm(tp->udev);
 
+	rtl_fc_pause_pkt_en(tp, 0);
+
 	r8156_mac_clk_spd(tp, true);
 
 	ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
@@ -8787,7 +8883,6 @@ static void r8156b_u2phy_backup(struct r8152 *tp)
 
 static void r8156b_init(struct r8152 *tp)
 {
-	u32 ocp_data;
 	u16 data;
 
 	if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
@@ -8852,20 +8947,7 @@ static void r8156b_init(struct r8152 *tp)
 
 	usb_enable_lpm(tp->udev);
 
-	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);
+	rtl_fc_pause_pkt_en(tp, 0);
 
 	r8156_mac_clk_spd(tp, true);
 
-- 
2.34.1

[PATCH net-next v3 8/8] r8152: support UPS for RTL8157 and RTL8159

[Chih Kai Hsu]

RTL8157 and RTL8159 support UPS but need different enable logic and EEE
flag handling than RTL8156. Add r8157_ups_en() and extend
r8156_ups_flags() with per-speed EEE flags (100M through 10G) and
5G/10G speed entries for VER_16/17.

Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
 drivers/net/usb/r8152.c | 110 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 98 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2b0972b967385..cc18b1c5a17c3 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_USE_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)
@@ -4178,8 +4184,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;
@@ -4230,20 +4255,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;
 	}
 
@@ -4415,6 +4453,35 @@ 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) {
+			/* clear USB fw_ver_reg */
+			ocp_write_byte(tp, MCU_TYPE_USB, USB_FW_USE_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)
@@ -4573,9 +4640,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);
@@ -9020,7 +9106,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);
 
@@ -9130,7 +9216,7 @@ static void r8159_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

Re: [PATCH net-next v3 1/8] r8152: refactor r8156_init

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 · Medium: 2 (1) · Low: 0

- [High] wait_autoload_done() drops the per-iteration
  test_bit(RTL8152_INACCESSIBLE, &tp->flags) abort that every one of the…
- [High] On a genuine autoload timeout the new `if
  (wait_autoload_done(tp)) return;` aborts initialization mid-sequence…
- [Medium] Commit message mischaracterizes the change: a patch titled
  "refactor r8156_init" adds brand-new hardware programming and changes…
- [Medium] The new wait_autoload_done() helper is applied to five open-
  coded autoload-done loops that belong to chips outside the patch's…

Pre-existing issues:
- [Medium] `tp->coalesce = 15000;` carries the comment `/* 15 us */`, but
  tp->coalesce is exported verbatim to userspace as…

--

r8152: refactor r8156_init

This splits the shared r8156_init() into per-chip init functions for
RTL8156, RTL8156B, RTL8157 and RTL8159, and introduces a new
wait_autoload_done() helper that replaces the open-coded autoload-done
poll loops in several init and UPS paths.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index f61686433031c..013e8d1abfc24 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)

[Severity: Medium]

The changelog says:

    Split r8156_init into per-chip functions:
    - r8156_init (VER_10/11)
    - r8156b_init (VER_12/13/15)
    - r8157_init (VER_16)
    - r8159_init (VER_17)

Is that a complete description of what the patch does?  These new register
and bit definitions have no user in the baseline, so the clears that use
them are new hardware programming rather than code motion:

  PLA_RMT_WAKE (0xe0c8) / RMT_WAKE_EN
  PLA_INDICATE_FALG / PREBOOT_OPTION

The same applies to several other writes that appear only after this patch:

  r8156b_u2phy_backup(): USB 0xd3ce/0xd3d0/0xd3d4/0xd3d8/0xd3dc
  r8157_init() and r8159_init(): USB 0xcd06 BIT(11) clear
  r8159_init(): USB 0xb9a6 BIT(0) clear and the write 0xb9a2 = 0x0448

A few existing sequences also change behaviour: AUTO_SPEEDUP is now cleared
for VER_16/VER_17 (previously only inside the VER_12..VER_15 block), the
0xcf04 interrupt-mitigation clear now also runs during init (previously
only in the unload path), and r8156b_wait_loading_flash() moved from before
the autoload poll to after it for VER_16/17.

Would it be possible to split the mechanical function split from the new
chip programming, so that a regression on RTL8156/8156B/8157/8159 does not
bisect to a commit that describes itself as a pure refactor?

[ ... ]

> @@ -4193,6 +4199,22 @@ 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,
> +				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 ret;
> +}
> +

[Severity: High]

Every loop this helper replaces tested RTL8152_INACCESSIBLE on each
iteration, but the new poll has no exit other than the condition or the
10 second deadline.  Can this keep sleeping for 10 seconds under
tp->control after the device is gone?

Once RTL8152_INACCESSIBLE is set (rtl_set_unplug() on disconnect, pre-reset,
or a failed control transfer), the read never touches the caller's buffer:

drivers/net/usb/r8152.c:generic_ocp_read() {
	if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
		return -ENODEV;
	...
}

and ocp_read_word() ignores that error while converting an uninitialized
variable:

drivers/net/usb/r8152.c:ocp_read_word() {
	u32 data;
	__le32 tmp;
	...
	generic_ocp_read(tp, index, sizeof(tmp), &tmp, type | byen);

	data = __le32_to_cpu(tmp);
	...
}

So is the poll condition ocp_data & AUTOLOAD_DONE decided by stack garbage
in that state?  If bit 1 happens to be set, wait_autoload_done() reports
success and the caller programs the whole bring-up sequence, including
tp->rtl_ops.hw_phy_cfg() from r8153b_ups_en()/r8153c_ups_en(), against a
device that is already gone.  If bit 1 is clear, the caller sleeps the full
10 seconds and then logs a timeout that did not really happen.

There is a related case on plain transfer failures:

drivers/net/usb/r8152.c:get_registers() {
	...
	if (ret < 0)
		memset(data, 0xff, size);
	...
}

0xffff & AUTOLOAD_DONE (0x0002) is non-zero, so the poll reports completion
on read errors too.

All the other wait loops in this file (r8153_phy_status(),
r8156b_wait_loading_flash(), rtl_phy_patch_request(), rtl_disable(),
r8153_aldps_en()) still honour RTL8152_INACCESSIBLE.  Should the poll
condition terminate on that flag as well, and should the read path preserve
the error status instead of returning an undefined value?

>  static void r8153b_ups_en(struct r8152 *tp, bool enable)
>  {
>  	if (enable) {
> @@ -4211,16 +4233,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);
>  

[ ... ]

> @@ -7246,22 +7252,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);
>  

[Severity: Medium]

The helper is also applied to five call sites outside the chips named in
the changelog: r8153_init() (VER_03..06), r8153b_init() (VER_08/09),
r8153c_init() (VER_14), r8153b_ups_en() and r8153c_ups_en().

For r8153_init() and r8153b_init() the previous poll only did break on
expiry and then executed the rest of the bring-up (power-cut disable,
u1u2 re-enable, LPM, RX aggregation, rtl_tally_reset(), and the
tp->coalesce assignment at the tail of r8153_init).  With the new

	if (wait_autoload_done(tp))
		return;

all of that is skipped instead.  The helper also newly emits
dev_err("autoload done timeout") on paths that were previously silent, and
drops the per-iteration test_bit(RTL8152_INACCESSIBLE, &tp->flags) abort.

Is this change of failure semantics on the RTL8153/8153B/8153C families
intended?  Right now a regression there would bisect to a commit titled
"r8152: refactor r8156_init".

[ ... ]

> @@ -8351,90 +8331,132 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
>  	set_bit(PHY_RESET, &tp->flags);
>  }
>  
> -static int r8159_wait_backup_restore(struct r8152 *tp)
> +static void r8156_init(struct r8152 *tp)
>  {

[ ... ]

> +	r8156_mdio_force_mode(tp);
> +	rtl_tally_reset(tp);
> +
> +	tp->coalesce = 15000; /* 15 us */
>  }

[Severity: Medium]

This is a pre-existing issue and not introduced by this patch, but the
comment and the value disagree with the unit that is exported to
userspace.  tp->coalesce is handed to ethtool verbatim as
ETHTOOL_COALESCE_USECS:

drivers/net/usb/r8152.c:rtl8152_get_coalesce() {
	...
	coalesce->rx_coalesce_usecs = tp->coalesce;
	...
}

drivers/net/usb/r8152.c:rtl8152_set_coalesce() {
	...
	tp->coalesce = coalesce->rx_coalesce_usecs;
	...
}

while the internal unit is nanoseconds, one register tick being 8 ns:

drivers/net/usb/r8152.c:r8153_set_rx_early_timeout() {
	u32 ocp_data = tp->coalesce / 8;
	...
	/* ... For USB_RX_EARLY_TIMEOUT, we fix it to 128ns. */
	ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_TIMEOUT, 128 / 8);
	...
}

So ethtool -c reports rx-usecs 15000 for a 15 us setting, and
ethtool -C rx-usecs 15 programs about 15 ns.  Since this patch replicates
the same constant and comment into r8156b_init(), r8157_init() and
r8159_init(), would it make sense to fix the unit (or at least the comment)
while these functions are being created?

> +
> +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);
> +
> +	if (wait_autoload_done(tp))
> +		return;

[Severity: High]

On a real autoload timeout this returns in the middle of the sequence, after
the SW reset bits (0xcffe BIT(3), 0xd3ca BIT(0)) and the U1/U2 disable have
already been applied, and without marking the device inaccessible.

What state does the adapter end up in?  The rest of the function is skipped:
r8153b_u1u2en(tp, true), usb_enable_lpm(), r8156_mac_clk_spd(), the
PLA_MCU_SPDWN_EN clear, the PLA_EXTRA_STATUS link polling setup, the RX
aggregation / RX_DESC_16B / BMU programming, r8156_mdio_force_mode(),
rtl_tally_reset() and tp->coalesce.

tp->coalesce then stays 0 in the freshly allocated private area, and:

drivers/net/usb/r8152.c:r8153_set_rx_early_timeout() {
	u32 ocp_data = tp->coalesce / 8;
	...
}

programs USB_RX_EARLY_TIMEOUT / USB_RX_EXTRA_AGGR_TMR with 0.  The same
applies to r8153_init(), where the tp->coalesce = COALESCE_SUPER/HIGH/SLOW
switch sits at the very end of the function.

Since ops->init is void and the timeout path sets neither
RTL8152_INACCESSIBLE nor PROBE_SHOULD_RETRY, does anything upstream learn
about the failure?  rtl8152_probe_once() still registers the netdev and
rtl8152_reset_resume() still continues into the normal resume path.

The intended contract seems to be the one used a few lines below in the
same patch for the other failure of the same class:

> +	if (r8159_wait_backup_restore(tp)) {
> +		rtl_set_inaccessible(tp);
> +		dev_err(&tp->intf->dev,
> +			"init failed, backup-restore timed out\n");
> +		return;
>  	}

Should the autoload timeout do the same (rtl_set_inaccessible() plus an
error), so the device is not registered and used half-programmed?

The same early return in r8153b_ups_en()/r8153c_ups_en() skips
hw_phy_cfg() and rtl8152_set_speed() right after the chip left power-cut,
and in r8153c_ups_en() also the trailing PLA_CRWECR/PLA_CONFIG34 writes
that previously always ran.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908075634.23740-1-nic_swsd%40realtek.com

Re: [PATCH net-next v3 2/8] r8152: support RTL8159 for different packages

[netdev-bot+sashiko]

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

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

- [Low] The changelog describes the patch as a pure enum split ("RTL8159
  comes in two packages, QFN68 and QFN100, which require different…

--

r8152: support RTL8159 for different packages

This splits RTL_VER_17 into RTL_VER_17_QFN68 and RTL_VER_17_QFN100 and
updates every version switch accordingly.  Probe now reads PLA_PKG_DET
over a USB control transfer to decide which of the two packages is
present, and r8159_init() gains one package-specific register write.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 013e8d1abfc24..f01f442fa71a0 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c

[ ... ]

> @@ -8716,6 +8739,10 @@ static void r8159_init(struct r8152 *tp)
>  	/* TX descriptor Signature */
>  	ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd4ae, BIT(1));
>  
> +	/* Enable u2phy backup restore patch */
> +	if (tp->version == RTL_VER_17_QFN68)
> +		ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xb99c, BIT(0));
> +

[Severity: Low]
This isn't a bug, but could the changelog mention this write?  The commit
message describes the change as an enum split:

    RTL8159 comes in two packages, QFN68 and QFN100, which require different
    handling. Split RTL_VER_17 into RTL_VER_17_QFN68 and RTL_VER_17_QFN100.

Every other hunk in the patch is a mechanical rename, and this
ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xb99c, BIT(0)) in r8159_init() is the
only place where the two packages actually behave differently.

RTL8159 has been driven as RTL_VER_17 since commit ad2a55b63b41af ("r8152:
Add support for the RTL8159 chip"), so QFN68 parts are running today without
this u2phy backup restore bit.

Is this write correcting misbehaviour on QFN68 hardware that is already
supported, or is it enablement for silicon that was not previously in the
field?  If it is the former, would a Fixes: ad2a55b63b41af ("r8152: Add
support for the RTL8159 chip") tag be appropriate so it reaches stable, and
could the changelog describe the symptom it addresses?

>  	/* Enable u3phy patch backup */
>  	ocp_write_word(tp, MCU_TYPE_USB, 0xb9a2, 0x0448);
>  

[ ... ]

> @@ -10287,7 +10340,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 %#02x\n",
> +				 pkg_det);
> +		}
>  		break;

[Severity: Low]
Along the same lines, the changelog does not mention that hw version 0x2020
now requires an extra PLA_PKG_DET control read at probe time to be
classified at all.  Could a sentence be added covering that?

Was this new detection path validated against both packages, and which
PLA_PKG_DET values were observed on each?

>  	default:
>  		version = RTL_VER_UNKNOWN;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908075634.23740-1-nic_swsd%40realtek.com

Re: [PATCH net-next v3 3/8] r8152: refactor rtl8156_enable, rtl8156_up, and rtl8156_down

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 · Medium: 3 · Low: 0

- [High] Version guard collapsed in the wrong direction:
  `ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR1, BIT(3))` in…
- [High] During the split of rtl8156_up()/rtl8156_down(), the two power-
  management writes for RTL8157/RTL8159 (RTL_VER_16,…
- [Medium] The commit message describes a pure code-motion refactor ("Add
  dedicated rtl8156b_enable (VER_12/13/15), rtl8157_enable, rtl8157_up,…
- [Medium] rtl8156_down() (RTL_VER_10/11 and RTL_VER_12/13/15) and the
  new rtl8157_down() (RTL_VER_16/17) both add two OOB hand-off register…
- [Medium] New 'Clear_SDR' hardware programming is introduced with raw
  register addresses and no symbolic defines: `ocp_word_set_bits(tp,…

--

r8152: refactor rtl8156_enable, rtl8156_up, and rtl8156_down

This splits the shared RTL8156 enable/up/down handlers into per-family
variants: rtl8156b_enable() for RTL_VER_12/13/15 and rtl8157_enable(),
rtl8157_up(), rtl8157_down() for RTL_VER_16/17.  The inline version
guards inside rtl8156_enable(), rtl8156_up() and rtl8156_down() are
dropped and rtl_ops_init() is updated to install the new callbacks.

[Severity: Medium]
The changelog reads as a pure code-motion refactor:

  "Add dedicated rtl8156b_enable (VER_12/13/15), rtl8157_enable,
   rtl8157_up, and rtl8157_down (VER_16/17) instead of handling
   per-version differences with inline version guards."

But rtl8156_up() and rtl8156_down() themselves are also modified, and
several register writes change for every family involved:

  - PLA_RCR1 BIT(3) clear becomes unconditional in rtl8156_up(), so it
    is now a new write on RTL8156/RTL8156B.
  - a new "Enable Clear_SDR" write to USB 0xd3ca BIT(15) for
    RTL_VER_13/15 in rtl8156_up().
  - rtl8156_down() replaces the bare PLA_OOB_CTRL |= NOW_IS_OOB with
    PLA_BDC_CR |= ALDPS_PROXY_MODE plus
    PLA_OOB_CTRL |= NOW_IS_OOB | DIS_MCU_CLROOB.
  - rtl8157_up() performs the USB_SPEED_OPTION clear that the baseline
    skipped for RTL_VER_16+, drops the PLA_MCU_SPDWN_EN clear, and adds
    two new writes (USB 0xd378 BIT(7) set, USB 0xcd06 BIT(15) clear).
  - rtl8157_down() changes PLA_RMS/PLA_MTPS and drops the
    PLA_MCU_SPDWN_EN set.

Could the changelog describe these hardware programming changes, or could
they be split into separate patches so they can be reverted or bisected
independently of the refactor?

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index f01f442fa71a0..6c189790b0bab 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c

[ ... ]

> @@ -6869,8 +6912,7 @@ static void rtl8156_up(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));

[Severity: High]
Was this guard collapsed in the wrong direction?  After the split,
rtl_ops_init() installs rtl8156_up() only for RTL_VER_10/11 and
RTL_VER_12/13/15, and gives RTL_VER_16/17 the new rtl8157_up():

	ops->up			= rtl8156_up;	/* VER_10/11, VER_12/13/15 */
	...
	ops->up			= rtl8157_up;	/* VER_16, VER_17_QFN68/100 */

So the "tp->version >= RTL_VER_16" condition can no longer be true in
rtl8156_up(), which suggests the write should have been removed together
with its guard rather than made unconditional.  The new rtl8157_up()
already contains the identical write:

	ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR1, BIT(3));

Does this mean RTL8156/RTL8156B now clear bit 3 of PLA_RCR1 (0xc012) on
every bring-up and resume, a write those parts never received before?
The driver only names OUTER_VLAN BIT(7) and INNER_VLAN BIT(6) for that
register, so the effect of BIT(3) on those chips is not obvious from the
code.

>  
>  	ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_SFF_STS_7, MCU_BORW_EN);
>  
> @@ -6892,11 +6934,11 @@ static void rtl8156_up(struct r8152 *tp)
>  	ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_RXFIFO_FULL, RXFIFO_FULL_MASK,
>  		      0x08);
>  
> -	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);
>  
> -	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);
>  
> @@ -6906,10 +6948,19 @@ 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)
> +	switch (tp->version) {
> +	case RTL_VER_13:
> +	case RTL_VER_15:
> +		/* Enable Clear_SDR */
> +		ocp_word_set_bits(tp, MCU_TYPE_USB, 0xd3ca, BIT(15));
> +		break;
> +	default:
> +		break;
> +	}

[Severity: Medium]
This "Clear_SDR" programming is new, has no baseline counterpart, and
uses a raw address with no #define.  Two questions about it:

The same USB address is accessed as a byte register elsewhere in this
file, in r8157_init() and r8159_init():

	ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd3ca, BIT(0));

while the new code does a 16-bit read-modify-write covering
0xd3ca..0xd3cb and sets BIT(15).  Is the upper byte a plain read/write
field?  If any of it is W1C or status, can this read-modify-write clobber
unrelated state?

Also, RTL_VER_12 is RTL8156B silicon as well but is excluded here.  Is
VER_12 genuinely unaffected, or was it missed?

There is no matching clear of this bit in rtl8156_down().  Is the bit
expected to persist across an ifdown/ifup cycle?

> +
> +	if (tp->udev->speed >= USB_SPEED_SUPER)
>  		r8153b_u1u2en(tp, true);
>  }
>  
> @@ -6922,12 +6973,9 @@ static void rtl8156_down(struct r8152 *tp)
>  
>  	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_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);
> @@ -6949,7 +6997,102 @@ static void rtl8156_down(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);

[Severity: Medium]
These two writes are new for the chips rtl8156_down() still serves
(RTL_VER_10/11 and RTL_VER_12/13/15 per rtl_ops_init()).  The baseline
ended the OOB entry with only:

	ocp_byte_set_bits(tp, MCU_TYPE_PLA, PLA_OOB_CTRL, NOW_IS_OOB);

ALDPS_PROXY_MODE in PLA_BDC_CR and DIS_MCU_CLROOB in PLA_OOB_CTRL change
who owns OOB state and how the MCU proxies during ALDPS, so does this
alter WoL and ARP-proxy behaviour on RTL8156/RTL8156B after an ifdown or
suspend?  The same two writes are added to rtl8157_down() below.

It looks like this brings 8156 in line with r8152b_enter_oob() and
r8153_enter_oob(), which do program both bits.  If that is the intent,
could it be stated in the changelog or split out so a WoL regression
report can be traced back to it?

> +
> +	ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_SFF_STS_7, MCU_BORW_EN);
> +
> +	rtl_rx_vlan_en(tp, true);
> +	rxdy_gated_en(tp, false);
> +
> +	ocp_dword_set_bits(tp, MCU_TYPE_PLA, PLA_RCR,
> +			   RCR_APM | RCR_AM | RCR_AB);
> +
> +	r8153_aldps_en(tp, true);
> +}
> +
> +static void rtl8157_up(struct r8152 *tp)
> +{

[ ... ]

> +	/* share FIFO settings */
> +	ocp_word_w0w1(tp, MCU_TYPE_PLA, PLA_RXFIFO_FULL, RXFIFO_FULL_MASK,
> +		      0x08);
> +
> +	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_SPEED_OPTION,
> +			  RG_PWRDN_EN | ALL_SPEED_OFF);

[Severity: High]
Are the two power-management writes for RTL8157/RTL8159 swapped here?

The baseline guarded this USB_SPEED_OPTION clear specifically to exclude
these versions:

	if (tp->version < RTL_VER_16)
		ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_SPEED_OPTION,
				  RG_PWRDN_EN | ALL_SPEED_OFF);

and rtl8157_up() is installed only for RTL_VER_16 and
RTL_VER_17_QFN68/QFN100, so this now runs on exactly the parts the guard
excluded.  Does clearing RG_PWRDN_EN | ALL_SPEED_OFF change PHY power
management on 8157/8159?

In the other direction, the PLA_MCU_SPDWN_EN clear that rtl8156_up()
does just before this point is missing from rtl8157_up(), and the paired
set is missing from rtl8157_down().  The baseline did both
unconditionally for RTL_VER_16/17, and grepping the file after this patch
the remaining PLA_MCU_SPDWN_EN sites are rtl8153b_up/down, rtl8153c_up,
rtl8156_up/down, r8153b_init, r8156_init and r8156b_init - none of which
are reachable for VER_16/17, and r8157_init()/r8159_init() only clear it
"if (tp->version < RTL_VER_16)".

Does that leave PLA_MAC_PWR_CTRL3 PLA_MCU_SPDWN_EN at whatever value the
ROM, OOB firmware or a previous driver instance left while the interface
is up on those chips?

> +
> +	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);
> +
> +	/* Clear_SDR */
> +	ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xd378, BIT(7));
> +	ocp_word_clr_bits(tp, MCU_TYPE_USB, 0xcd06, BIT(15));

[Severity: Medium]
These two writes have no baseline counterpart, no symbolic defines, and
no matching undo in rtl8157_down().  Could they get names, and is the
comment "Clear_SDR" enough to explain what the two different addresses
do?

> +}
> +
> +static void rtl8157_down(struct r8152 *tp)
> +{

[ ... ]

> +	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, 1526);
> +	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, 10 * 1024 / 64);

[Severity: Medium]
The baseline down path applied 1522 and MTPS_DEFAULT for RTL_VER_16/17.
Is the change to 1526 and 10 * 1024 / 64 intentional, and could it be
mentioned in the changelog?

> +
> +	/* Clear teredo wake event. bit[15:8] is the teredo wakeup
> +	 * type. Set it to zero. bits[7:0] are the W1C bits about
> +	 * the events. Set them to all 1 to clear them.
> +	 */
> +	ocp_write_word(tp, MCU_TYPE_PLA, PLA_TEREDO_WAKE_BASE, 0x00ff);
> +
> +	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);
>  
> @@ -10071,7 +10214,7 @@ static int rtl_ops_init(struct r8152 *tp)
>  		tp->eee_adv		= MDIO_EEE_1000T | MDIO_EEE_100TX;
>  		tp->eee_adv2		= MDIO_EEE_2_5GT;
>  		ops->init		= r8156b_init;
> -		ops->enable		= rtl8156_enable;
> +		ops->enable		= rtl8156b_enable;
>  		ops->disable		= rtl8153_disable;
>  		ops->up			= rtl8156_up;
>  		ops->down		= rtl8156_down;
> @@ -10110,10 +10253,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;

[ ... ]

For reference, none of the above is changed later in the series; the
same code is still present in rtl8156_up(), rtl8156_down(),
rtl8157_up() and rtl8157_down() at the end of the series.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908075634.23740-1-nic_swsd%40realtek.com

Re: [PATCH net-next v3 4/8] r8152: refactor r8157_hw_phy_cfg

[netdev-bot+sashiko]

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

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

- [Medium] r8159_hw_phy_cfg() (drivers/net/usb/r8152.c:8332), newly bound
  as the hw_phy_cfg handler for RTL_VER_17_QFN68/RTL_VER_17_QFN100 in…
- [Low] The commit is presented as 'refactor r8157_hw_phy_cfg' with
  'update PHY parameters for RTL8156B/57/59', but it silently changes…

--

r8152: refactor r8157_hw_phy_cfg

This splits the shared r8157_hw_phy_cfg() into r8157_hw_phy_cfg() for
RTL_VER_16 and a new r8159_hw_phy_cfg() for RTL_VER_17_QFN68/QFN100,
rebinds the VER_17 ops entry to the new function, adds sram2_set_bits()
and sram2_clr_bits() helpers, and adjusts PHY parameters for
RTL8156B/57/59.

Two questions below.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 6c189790b0bab..679aead731f73 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c

[ ... ]

> @@ -8075,6 +8086,9 @@ static void r8156b_hw_phy_cfg(struct r8152 *tp)
>  		sram_write(tp, 0x8074, 0x2417);
>  		sram_write(tp, 0x807a, 0x2417);
>  
> +		/* Nway DACONB parameters */
> +		ocp_reg_w0w1(tp, 0xa4ca, 0x6000, 0x0040);
> +
>  		/* XG PLL */
>  		ocp_reg_w0w1(tp, 0xbf84, 0xe000, 0xa000);
>  		break;
> @@ -8151,11 +8165,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));
> +

[Severity: Low]
This isn't a bug, but the commit message describes this as a refactor plus
an "update PHY parameters for RTL8156B/57/59", and some of the register
programming changes are hard to classify from the message alone.

In r8157_hw_phy_cfg() this 0xd3c8 BIT(0) clear was previously inside the
"case RTL_VER_17_QFN68/RTL_VER_17_QFN100:" arm only, and it is now above
the version switch, so RTL8157 (RTL_VER_16) executes it for the first
time.

In r8156b_hw_phy_cfg() the new "Nway DACONB parameters" write:

	ocp_reg_w0w1(tp, 0xa4ca, 0x6000, 0x0040);

retunes RTL8156B (RTL_VER_13/RTL_VER_15) in a patch whose subject is an
r8157 refactor.

Can the changelog say which of these register changes fix an observed
symptom (and therefore want a Fixes: tag and stable backporting) versus
which are vendor tuning?  Would the RTL8156B change be better as its own
patch?

>  	switch (tp->version) {
>  	case RTL_VER_16:
>  		/* XG_INRX parameter */

[ ... ]

> @@ -8277,12 +8294,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;
> +	}
> +

[ ... ]

> +	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);

[Severity: Medium]
Was dropping the EEE disable here intentional for RTL8159?

Before the split, RTL_VER_17_QFN68/QFN100 went through the shared
r8157_hw_phy_cfg(), which quiesced both ALDPS and EEE before touching PHY
parameters, and the RTL_VER_16 half still does:

	r8153_aldps_en(tp, false);
	rtl_eee_enable(tp, false);

r8159_hw_phy_cfg() only calls r8153_aldps_en(tp, false) and then starts
reprogramming PHY/SRAM/OCP parameters, including the EEE related ones
(PLA_MAC_PWR_CTRL4 EEE_SPDWN_EN, OCP_DOWN_SPEED and OCP_POWER_CFG
EEE_CLKDIV_EN).  The other handlers in this driver comment on that
ordering explicitly, for example in r8156b_hw_phy_cfg():

	/* disable EEE before updating the PHY parameters */
	rtl_eee_enable(tp, false);

The remaining half of the pair is only the conditional enable at the tail
of r8159_hw_phy_cfg():

	if (tp->eee_en)
		rtl_eee_enable(tp, true);

With tp->eee_en set to false through r8152_set_eee(), nothing turns
hardware EEE off any more, so after a re-run of hw_phy_cfg (the
PCUT_STATUS resume paths, or rtl_hw_phy_change_work_func() calling
tp->rtl_ops.hw_phy_cfg(tp)) does the PHY come back with its default EEE
state while ups_info still records EEE as disabled, silently undoing an
earlier "ethtool --set-eee eee off"?

This still looks the same at the end of the series, so no later patch in
the set restores the disable.

> +
> +	/* 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
>  		 */

[ ... ]

> @@ -8471,9 +8581,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;
>  
> @@ -8485,7 +8595,7 @@ 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)

These two hunks land in the RTL8159 epilogue and also change behaviour
rather than move code: EN_10M_CLKDIV goes from set to cleared with
ups_info._10m_ckdiv flipping from true to false (that field feeds the
UPS/suspend programming), and 0xa5ea BIT(2) is newly cleared.  Could the
changelog mention what these two changes do for RTL8159?

> @@ -10284,7 +10394,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;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908075634.23740-1-nic_swsd%40realtek.com

Re: [PATCH net-next v3 5/8] r8152: support rtl8157_unload and rtl8157_change_mtu

[netdev-bot+sashiko]

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

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

- [Low] Commit message understates the impact of the change and carries
  no Fixes: tag, even though it silently alters the register programming…

--

r8152: support rtl8157_unload and rtl8157_change_mtu

This adds rtl8157_unload() and rtl8157_change_mtu() and wires them into
the RTL_VER_16 (RTL8157) and RTL_VER_17 (RTL8159) op tables, replacing
rtl8153_unload() and rtl8156_change_mtu().  The interrupt mitigation clear
of USB register 0xcf04 is moved out of rtl8153_unload() into the new
callback.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 679aead731f73..06fad895fce09 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -7024,6 +7024,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);

[ ... ]

> @@ -10174,11 +10196,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 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));
> +}

[Severity: Low]
Could the changelog say what misbehaves today on these chips, and carry
Fixes: tags?  RTL_VER_16 and RTL_VER_17 are already supported in-tree
(added by fd3c7d080df531 and ad2a55b63b41af), and both assigned
ops->unload = rtl8153_unload and ops->change_mtu = rtl8156_change_mtu, so
this changes the register programming of shipped hardware rather than
enabling something new.

The commit message says the dedicated unload is needed "to disable
interrupt mitigation", but rtl8153_unload() already did that for
tp->version >= RTL_VER_16:

	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));
	}

so that part is only relocated.  The behaviour that actually changes is the
power cut disable sequence:

r8153_power_cut_en(tp, false):
	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_POWER_CUT, PWR_EN | PHASE2_EN);
	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_MISC_0, PCUT_STATUS);

r8157_power_cut_en(tp, false):
	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_POWER_CUT, PWR_EN);
	ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_MISC_0, PCUT_STATUS);
	ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_MISC_2, BIT(1));

PHASE2_EN is now left set on the unload/disconnect path for these chips.
Is that intentional, and is there an observable symptom with the old
sequence?

Similarly, rtl8157_change_mtu() replaces the fixed MTPS_JUMBO (12 * 1024 /
64 = 0xc0) that rtl8156_change_mtu() programmed with an MTU derived value:
at the default 1500 byte MTU, max_t(u32, mtu_to_size(1500), 10 * 1024) / 64
gives 0xa0, and it saturates at 0xff for a 16K MTU.  Both paths are live at
runtime, via rtl8157_up() on every bring-up and via rtl8152_change_mtu() ->
tp->rtl_ops.change_mtu() on every MTU change.  What does the old MTPS value
break on these chips?

As written it is hard to tell whether this is net-next tuning or a fix for a
user visible defect on released kernels, and there is nothing for stable
backports to key on.  Note that this wording is unchanged at the end of the
series, so no later patch adds the missing justification.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908075634.23740-1-nic_swsd%40realtek.com

Re: [PATCH net-next v3 6/8] r8152: add TGPHY access support

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · Low: 0

- [Medium] r8157_phy_read() (drivers/net/usb/r8152.c:2086) and
  r8157_phy_write() (:2095) discard rtl_tgphy_access()'s return value.
- [Medium] wait_tgphy_cmd_ready() (drivers/net/usb/r8152.c:2045) makes
  its loop-exit decision on the value returned by ocp_read_word(), which…

--

r8152: add TGPHY access support

This adds a TGPHY register access path for RTL8157/RTL8159 using
USB_TGPHY_CMD/ADDR/DATA, so PHY registers can be reached without
reprogramming the PLA OCP base. It also introduces phy_read/phy_write
function pointers in struct rtl_ops so each chip gets the right access
method, with ocp_reg_read()/ocp_reg_write() becoming thin dispatchers.

A couple of questions about error propagation in the new path below.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 06fad895fce09..1fcb1cc5b4a18 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c

[ ... ]

> @@ -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);
> +		u16 (*phy_read)(struct r8152 *tp, u16 addr);
> +		void (*phy_write)(struct r8152 *tp, u16 addr, u16 data);
>  	} rtl_ops;

[ ... ]

> @@ -1687,6 +1696,16 @@ static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
>  	ocp_write_word(tp, MCU_TYPE_PLA, ocp_index, data);
>  }
>  
> +static u16 ocp_reg_read(struct r8152 *tp, u16 addr)
> +{
> +	return tp->rtl_ops.phy_read(tp, addr);
> +}
> +
> +static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
> +{
> +	tp->rtl_ops.phy_write(tp, addr, data);
> +}
> +

[ ... ]

> @@ -2023,6 +2042,61 @@ static int r8152_mdio_test_and_clr_bit(struct r8152 *tp, u16 addr, u16 clear)
>  	return data & clear;
>  }
>  
> +static int wait_tgphy_cmd_ready(struct r8152 *tp)
> +{
> +	u16 ocp_data;
> +
> +	return poll_timeout_us(ocp_data = ocp_read_word(tp, MCU_TYPE_USB,
> +							USB_TGPHY_CMD),
> +			       !(ocp_data & TGPHY_CMD_BUSY), 2000, 20000,
> +			       false);
> +}

[Severity: Medium]

wait_tgphy_cmd_ready() makes its loop-exit decision purely on the value
returned by ocp_read_word(), which is indeterminate if the underlying
transfer never happens:

  ocp_read_word() {
	u32 data;
	__le32 tmp;
	...
	generic_ocp_read(tp, index, sizeof(tmp), &tmp, type | byen);
	data = __le32_to_cpu(tmp);
	...
  }

generic_ocp_read() starts with:

	if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
		return -ENODEV;

so tmp is left untouched and the u16 handed to the poll loop is stale
stack content.  If bit 0 happens to be set, does this helper then sleep
for its whole 20 ms budget instead of returning right away?

The USB-error case looks deterministic rather than random, since
get_registers() does:

	if (ret < 0)
		memset(data, 0xff, size);

USB_TGPHY_CMD then reads back as 0xffff, TGPHY_CMD_BUSY always appears
asserted, and every TGPHY access consumes the full timeout.

Since every PHY register access on RTL8157/RTL8159 now funnels through
rtl_tgphy_access() (one wait per write, two per read), would a device
marked inaccessible mid-sequence make r8157_hw_phy_cfg()/hw_phy_work, the
up/down paths and the mdio ioctl sleep 20-40 ms per register while holding
tp->control?  Would an is-accessible check in the helper be preferable to
polling a value that may never have been read?

> +
> +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 u16 r8157_phy_read(struct r8152 *tp, u16 addr)
> +{
> +	u16 data = 0;
> +
> +	rtl_tgphy_access(tp, addr, &data, false);
> +
> +	return data;
> +}
> +
> +static void r8157_phy_write(struct r8152 *tp, u16 addr, u16 data)
> +{
> +	rtl_tgphy_access(tp, addr, &data, true);
> +}

[Severity: Medium]

Both wrappers discard rtl_tgphy_access()'s status.  On a
wait_tgphy_cmd_ready() timeout, r8157_phy_read() returns the
pre-initialised 0, which is indistinguishable from real register content.

Can that fabricated 0 get written back to the PHY?  ocp_reg_read() now
dispatches to r8157_phy_read() for RTL8157/RTL8159, and the
read-modify-write helpers use it directly:

  ocp_reg_w0w1() {
	u16 data;

	data = ocp_reg_read(tp, addr);
	data = (data & ~clear) | set;
	ocp_reg_write(tp, addr, data);
  }

so a timed-out read makes the driver write (0 & ~clear) | set, dropping
every other bit of the MII/PHY register (advertisement, EEE, PLL/ALDPS).
r8152_mdio_set_bit(), r8152_mdio_test_and_clr_bit() and sram_write_w0w1()
have the same shape.

For the write direction, a first-wait timeout jumps to out before any of
the ADDR/DATA/CMD writes are issued, so the PHY write is dropped
entirely.  With the void signature there is no way for the caller to
notice, and no dev_err()/dev_warn() is emitted, so should this at least
be logged?

Note that the existing global mitigation does not cover a plain BUSY
timeout: rtl_set_inaccessible() is only reached when a USB control
transfer fails in r8152_control_msg(), so here the control transfers
succeed and generic_ocp_write() still pushes the clobbered value to the
chip.

The sibling ADV interface in the same file returns int for exactly this
reason:

  static int ocp_adv_read(struct r8152 *tp, u16 cmd, u16 addr, u32 *data)
  ...
  static int rtl_bmu_w0w1(struct r8152 *tp, u16 addr, u32 clear, u32 set)
  {
	ret = rtl_bmu_read(tp, addr, &bmu);
	if (ret < 0)
		goto out;
	...
  }

and wait_tgphy_cmd_ready() duplicates wait_cmd_ready() almost verbatim.
Would it make sense for the new phy_read/phy_write ops in struct rtl_ops
to carry a status back, or to reuse the existing wait_cmd_ready() helper,
so the TGPHY timeout can stop the read-modify-write sequence instead of
being swallowed?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908075634.23740-1-nic_swsd%40realtek.com

Re: [PATCH net-next v3 7/8] r8152: support rtl_fc_pause_pkt_en()

[netdev-bot+sashiko]

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

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

- [Medium] Commit message mischaracterizes the change as an extraction
  and omits the new register programming it introduces.

--

r8152: support rtl_fc_pause_pkt_en()

This moves the flow control programming out of r8156b_init() into a new
helper rtl_fc_pause_pkt_en(), and calls it from r8156_init(), r8156b_init()
and rtl8157_enable(). The helper switches on tp->version and, for RTL8157
and RTL8159, programs a new pause limit register based on the link speed.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 1fcb1cc5b4a18..2b0972b967385 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
>  
> @@ -6088,6 +6093,93 @@ 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 & LINK_STATUS)) {
> +			dev_dbg(&tp->intf->dev, "No link\n");
> +			goto no_link;
> +		} else 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 {
> +			dev_err(&tp->intf->dev, "Unknown link speed\n");
> +			goto no_link;
> +		}
> +
> +		log2_ratio = ilog2(ratio);
> +		num_pause_pkts -= log2_ratio;
> +
> +		/* Round up if ratio is more than halfway to the next power of 2.
> +		 * 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--;
> +
> +no_link:
> +		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;
> +	}
> +}
> +

[Severity: Medium]
Is the changelog description of "Extract it as rtl_fc_pause_pkt_en()"
accurate for the RTL_VER_10/RTL_VER_11 and RTL_VER_16/RTL_VER_17 arms of
rtl_fc_pause_pkt_en()?  The three arms are not the same sequence that was
inline in r8156b_init().

For RTL_VER_10/RTL_VER_11 the helper programs:

	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);

At the series baseline the flow control block in r8156_init() was gated as:

	if (tp->version >= RTL_VER_12 && tp->version <= 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);
		...
	}

so RTL_VER_10/RTL_VER_11 received no flow control programming, and
FLOW_CTRL_PATCH_OPT was only set from the RTL8153B firmware path.  Could
the changelog say why RTL8156A now gets a 1000 ms timer plus
FLOW_CTRL_PATCH_OPT, while RTL8156B keeps the 600 ms timer,
FLOW_CTRL_PATCH_2 and PLA_CPCR FLOW_CTRL_EN?

For RTL_VER_16/RTL_VER_17_QFN68/RTL_VER_17_QFN100 the helper adds a new
register, PLA_PAUSE_LIMIT (0xe61e), the new PAUSE_LIMIT_EN/PAUSE_LIMIT_MASK
bits, and a link-speed dependent pause packet count, but it sets neither
PLA_CPCR FLOW_CTRL_EN nor USB_FC_TIMER even though the changelog says the
flow control patch was "missing for ... RTL8157/8159".  None of the new
register or the pause budget calculation is described in the changelog.

Could the changelog describe the observed malfunction, the link speeds and
traffic conditions it was seen under, and what was tested, so the behaviour
change on already supported RTL8156A hardware and the new pause limit
programming can be judged?

>  static int r8153_pre_firmware_1(struct r8152 *tp)
>  {
>  	int i;
> @@ -6619,6 +6711,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);
> @@ -8751,6 +8845,8 @@ static void r8156_init(struct r8152 *tp)
>  
>  	usb_enable_lpm(tp->udev);
>  
> +	rtl_fc_pause_pkt_en(tp, 0);
> +
>  	r8156_mac_clk_spd(tp, true);
>  

[ ... ]

> @@ -8852,20 +8947,7 @@ static void r8156b_init(struct r8152 *tp)
>  
>  	usb_enable_lpm(tp->udev);
>  
> -	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);
> +	rtl_fc_pause_pkt_en(tp, 0);
>  
>  	r8156_mac_clk_spd(tp, true);
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908075634.23740-1-nic_swsd%40realtek.com

Re: [PATCH net-next v3 8/8] r8152: support UPS for RTL8157 and RTL8159

[netdev-bot+sashiko]

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

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

- [Medium] Incomplete version dispatch: the four new switch statements
  added by this patch enumerate only RTL_VER_16 and RTL_VER_17_QFN68 and…

--

r8152: support UPS for RTL8157 and RTL8159

This adds r8157_ups_en() for the RTL8157/RTL8159 UPS enable sequence,
extends r8156_ups_flags() with per-speed EEE flags and 5G/10G speed
encodings, and switches r8157_init()/r8159_init() over to the new
enable helper.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 2b0972b967385..cc18b1c5a17c3 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c

[ ... ]

> @@ -4178,8 +4184,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:

[Severity: Medium]

Should RTL_VER_17_QFN100 be listed here as well?

In rtl_ops_init() the two RTL8159 packages share one identical block:

	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;
		ops->init		= r8159_init;
		...
		ops->hw_phy_cfg		= r8159_hw_phy_cfg;
		ops->autosuspend_en	= rtl8157_runtime_enable;

So QFN100 advertises the same 100M through 10G EEE capabilities, but with
only RTL_VER_17_QFN68 matched above it takes the default arm and gets the
old UPS_FLAGS_EN_EEE encoding that this patch replaces for this chip
generation.

> +			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;
> @@ -4230,20 +4255,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;

[Severity: Medium]

Same question for the lite_mode suppression: RTL_VER_17_QFN100 is not
listed, so it still gets the bit 5/6 lite_mode field that this hunk treats
as not applicable to this generation.

> +		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;
>  	}
>  

[ ... ]

> @@ -4573,9 +4640,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;
> +		}

[Severity: Medium]

Does this leave RTL_VER_17_QFN100 without UPS entirely?

rtl_ops_init() assigns ops->autosuspend_en = rtl8157_runtime_enable for
both RTL_VER_17_QFN68 and RTL_VER_17_QFN100, and __rtl_get_pkg_det()
results of 0x18/0x1a select RTL_VER_17_QFN100. On runtime suspend the
chain is:

	rtl8152_runtime_suspend()
	  -> tp->rtl_ops.autosuspend_en(tp, true)
	     == rtl8157_runtime_enable(tp, true)
	        -> switch (tp->version) ... default: break;

so r8157_ups_en() is never reached for that package, while the commit
message says "support UPS for RTL8157 and RTL8159".

>  	} 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;
> +		}

[Severity: Medium]

Mirror of the above on the resume path.

Note that r8159_init(), shared by both RTL8159 packages, was converted in
this same patch:

	r8157_power_cut_en(tp, false);
	r8157_ups_en(tp, false);

so the QFN100 path does get the new disable call at probe/init time, but
never the enable/disable dispatch during runtime suspend and resume.

There is a QFN68-only quirk elsewhere in r8159_init() ("Enable u2phy
backup restore patch"), so a package-specific restriction is plausible.
Should case RTL_VER_17_QFN100 be added to all four new switches, or should
the commit message be scoped to the QFN68 package with a note on the
hardware difference?

>  		r8157_u2p3en(tp, true);
>  		if (tp->udev->speed >= USB_SPEED_SUPER)
>  			r8153b_u1u2en(tp, true);

[ ... ]

> @@ -9130,7 +9216,7 @@ static void r8159_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);
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908075634.23740-1-nic_swsd%40realtek.com