[PATCH net 0/3] r8152: fix 2.5G devices

Hayes Wang posted 3 patches 2 years, 9 months ago
There is a newer version of this series
drivers/net/usb/r8152.c | 80 +++++++++++++++++++++++++++--------------
1 file changed, 54 insertions(+), 26 deletions(-)
[PATCH net 0/3] r8152: fix 2.5G devices
Posted by Hayes Wang 2 years, 9 months ago
These patches are used to fix some issues of RTL8156.

Hayes Wang (3):
  r8152: fix flow control issue of RTL8156A
  r8152: fix the poor throughput for 2.5G devices
  r8152: move setting r8153b_rx_agg_chg_indicate()

 drivers/net/usb/r8152.c | 80 +++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 26 deletions(-)

-- 
2.40.0
[PATCH net v2 0/3] r8152: fix 2.5G devices
Posted by Hayes Wang 2 years, 9 months ago
v2:
For patch #1, Remove inline for fc_pause_on_auto() and fc_pause_off_auto(),
and update the commit message.

For patch #2, define the magic value for OCP register 0xa424.

v1:
These patches are used to fix some issues of RTL8156.

Hayes Wang (3):
  r8152: fix flow control issue of RTL8156A
  r8152: fix the poor throughput for 2.5G devices
  r8152: move setting r8153b_rx_agg_chg_indicate()

 drivers/net/usb/r8152.c | 84 ++++++++++++++++++++++++++++-------------
 1 file changed, 58 insertions(+), 26 deletions(-)

-- 
2.40.0
Re: [PATCH net v2 0/3] r8152: fix 2.5G devices
Posted by Andrew Lunn 2 years, 9 months ago
On Thu, Apr 27, 2023 at 08:10:54PM +0800, Hayes Wang wrote:
> v2:
> For patch #1, Remove inline for fc_pause_on_auto() and fc_pause_off_auto(),
> and update the commit message.
> 
> For patch #2, define the magic value for OCP register 0xa424.
> 
> v1:
> These patches are used to fix some issues of RTL8156.

Please always create a new thread. The patch automation bots can get
confused if you append to an existing thread, they assume it is just
comments to the original patchset.

	 Andrew
Re: [PATCH net v2 0/3] r8152: fix 2.5G devices
Posted by Hayes Wang 2 years, 9 months ago
On 4/27/2023 8:27 PM, Andrew Lunn wrote:
[...]
> Please always create a new thread. The patch automation bots can get
> confused if you append to an existing thread, they assume it is just
> comments to the original patchset.
It 's my mistake. Sorry.

Best Regards,
Hayes
[PATCH net v2 1/3] r8152: fix flow control issue of RTL8156A
Posted by Hayes Wang 2 years, 9 months ago
The feature of flow control becomes abnormal, if the device sends a
pause frame and the tx/rx is disabled before sending a release frame. It
causes the lost of packets.

Set PLA_RX_FIFO_FULL and PLA_RX_FIFO_EMPTY to zeros before disabling the
tx/rx. And, toggle FC_PATCH_TASK before enabling tx/rx to reset the flow
control patch and timer. Then, the hardware could clear the state and
the flow control becomes normal after enabling tx/rx.

Besides, remove inline for fc_pause_on_auto() and fc_pause_off_auto().

Fixes: 195aae321c82 ("r8152: support new chips")
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 56 ++++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0fc4b959edc1..afd50e90d1fe 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -5986,6 +5986,25 @@ static void rtl8153_disable(struct r8152 *tp)
 	r8153_aldps_en(tp, true);
 }
 
+static u32 fc_pause_on_auto(struct r8152 *tp)
+{
+	return (ALIGN(mtu_to_size(tp->netdev->mtu), 1024) + 6 * 1024);
+}
+
+static u32 fc_pause_off_auto(struct r8152 *tp)
+{
+	return (ALIGN(mtu_to_size(tp->netdev->mtu), 1024) + 14 * 1024);
+}
+
+static void r8156_fc_parameter(struct r8152 *tp)
+{
+	u32 pause_on = tp->fc_pause_on ? tp->fc_pause_on : fc_pause_on_auto(tp);
+	u32 pause_off = tp->fc_pause_off ? tp->fc_pause_off : fc_pause_off_auto(tp);
+
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RX_FIFO_FULL, pause_on / 16);
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RX_FIFO_EMPTY, pause_off / 16);
+}
+
 static int rtl8156_enable(struct r8152 *tp)
 {
 	u32 ocp_data;
@@ -5994,6 +6013,7 @@ static int rtl8156_enable(struct r8152 *tp)
 	if (test_bit(RTL8152_UNPLUG, &tp->flags))
 		return -ENODEV;
 
+	r8156_fc_parameter(tp);
 	set_tx_qlen(tp);
 	rtl_set_eee_plus(tp);
 	r8153_set_rx_early_timeout(tp);
@@ -6025,9 +6045,24 @@ static int rtl8156_enable(struct r8152 *tp)
 		ocp_write_word(tp, MCU_TYPE_USB, USB_L1_CTRL, ocp_data);
 	}
 
+	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_FW_TASK);
+	ocp_data &= ~FC_PATCH_TASK;
+	ocp_write_word(tp, MCU_TYPE_USB, USB_FW_TASK, ocp_data);
+	usleep_range(1000, 2000);
+	ocp_data |= FC_PATCH_TASK;
+	ocp_write_word(tp, MCU_TYPE_USB, USB_FW_TASK, ocp_data);
+
 	return rtl_enable(tp);
 }
 
+static void rtl8156_disable(struct r8152 *tp)
+{
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RX_FIFO_FULL, 0);
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RX_FIFO_EMPTY, 0);
+
+	rtl8153_disable(tp);
+}
+
 static int rtl8156b_enable(struct r8152 *tp)
 {
 	u32 ocp_data;
@@ -6429,25 +6464,6 @@ static void rtl8153c_up(struct r8152 *tp)
 	r8153b_u1u2en(tp, true);
 }
 
-static inline u32 fc_pause_on_auto(struct r8152 *tp)
-{
-	return (ALIGN(mtu_to_size(tp->netdev->mtu), 1024) + 6 * 1024);
-}
-
-static inline u32 fc_pause_off_auto(struct r8152 *tp)
-{
-	return (ALIGN(mtu_to_size(tp->netdev->mtu), 1024) + 14 * 1024);
-}
-
-static void r8156_fc_parameter(struct r8152 *tp)
-{
-	u32 pause_on = tp->fc_pause_on ? tp->fc_pause_on : fc_pause_on_auto(tp);
-	u32 pause_off = tp->fc_pause_off ? tp->fc_pause_off : fc_pause_off_auto(tp);
-
-	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RX_FIFO_FULL, pause_on / 16);
-	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RX_FIFO_EMPTY, pause_off / 16);
-}
-
 static void rtl8156_change_mtu(struct r8152 *tp)
 {
 	u32 rx_max_size = mtu_to_size(tp->netdev->mtu);
@@ -9340,7 +9356,7 @@ static int rtl_ops_init(struct r8152 *tp)
 	case RTL_VER_10:
 		ops->init		= r8156_init;
 		ops->enable		= rtl8156_enable;
-		ops->disable		= rtl8153_disable;
+		ops->disable		= rtl8156_disable;
 		ops->up			= rtl8156_up;
 		ops->down		= rtl8156_down;
 		ops->unload		= rtl8153_unload;
-- 
2.40.0
Re: [PATCH net v2 1/3] r8152: fix flow control issue of RTL8156A
Posted by Andrew Lunn 2 years, 9 months ago
On Thu, Apr 27, 2023 at 08:10:55PM +0800, Hayes Wang wrote:
> The feature of flow control becomes abnormal, if the device sends a
> pause frame and the tx/rx is disabled before sending a release frame. It
> causes the lost of packets.
> 
> Set PLA_RX_FIFO_FULL and PLA_RX_FIFO_EMPTY to zeros before disabling the
> tx/rx. And, toggle FC_PATCH_TASK before enabling tx/rx to reset the flow
> control patch and timer. Then, the hardware could clear the state and
> the flow control becomes normal after enabling tx/rx.
> 
> Besides, remove inline for fc_pause_on_auto() and fc_pause_off_auto().
> 
> Fixes: 195aae321c82 ("r8152: support new chips")
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
[PATCH net v2 2/3] r8152: fix the poor throughput for 2.5G devices
Posted by Hayes Wang 2 years, 9 months ago
Fix the poor throughput for 2.5G devices, when changing the speed from
auto mode to force mode. This patch is used to notify the MAC when the
mode is changed.

Fixes: 195aae321c82 ("r8152: support new chips")
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index afd50e90d1fe..0846ceb72162 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -199,6 +199,7 @@
 #define OCP_EEE_AR		0xa41a
 #define OCP_EEE_DATA		0xa41c
 #define OCP_PHY_STATUS		0xa420
+#define OCP_INTR_EN		0xa424
 #define OCP_NCTL_CFG		0xa42c
 #define OCP_POWER_CFG		0xa430
 #define OCP_EEE_CFG		0xa432
@@ -620,6 +621,9 @@ enum spd_duplex {
 #define PHY_STAT_LAN_ON		3
 #define PHY_STAT_PWRDN		5
 
+/* OCP_INTR_EN */
+#define INTR_SPEED_FORCE	BIT(3)
+
 /* OCP_NCTL_CFG */
 #define PGA_RETURN_EN		BIT(1)
 
@@ -7554,6 +7558,11 @@ static void r8156_hw_phy_cfg(struct r8152 *tp)
 				      ((swap_a & 0x1f) << 8) |
 				      ((swap_a >> 8) & 0x1f));
 		}
+
+		/*  set intr_en[3] */
+		data = ocp_reg_read(tp, OCP_INTR_EN);
+		data |= INTR_SPEED_FORCE;
+		ocp_reg_write(tp, OCP_INTR_EN, data);
 		break;
 	default:
 		break;
@@ -7949,6 +7958,11 @@ static void r8156b_hw_phy_cfg(struct r8152 *tp)
 		break;
 	}
 
+	/*  set intr_en[3] */
+	data = ocp_reg_read(tp, OCP_INTR_EN);
+	data |= INTR_SPEED_FORCE;
+	ocp_reg_write(tp, OCP_INTR_EN, data);
+
 	if (rtl_phy_patch_request(tp, true, true))
 		return;
 
-- 
2.40.0
Re: [PATCH net v2 2/3] r8152: fix the poor throughput for 2.5G devices
Posted by Andrew Lunn 2 years, 9 months ago
> @@ -7554,6 +7558,11 @@ static void r8156_hw_phy_cfg(struct r8152 *tp)
>  				      ((swap_a & 0x1f) << 8) |
>  				      ((swap_a >> 8) & 0x1f));
>  		}
> +
> +		/*  set intr_en[3] */
> +		data = ocp_reg_read(tp, OCP_INTR_EN);
> +		data |= INTR_SPEED_FORCE;

Which is more meaningful, set intr_en[3], or data |= INTR_SPEED_FORCE
I would say the second. The code is now pretty readable, it says
what it is doing. So if you want to add a comment, you really should
be commenting on why, not what.

   Andrew
[PATCH net v2 3/3] r8152: move setting r8153b_rx_agg_chg_indicate()
Posted by Hayes Wang 2 years, 9 months ago
Move setting r8153b_rx_agg_chg_indicate() for 2.5G devices. The
r8153b_rx_agg_chg_indicate() has to be called after enabling tx/rx.
Otherwise, the coalescing settings are useless.

Fixes: 195aae321c82 ("r8152: support new chips")
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0846ceb72162..59498aec8f16 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3027,12 +3027,16 @@ static int rtl_enable(struct r8152 *tp)
 	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CR, ocp_data);
 
 	switch (tp->version) {
-	case RTL_VER_08:
-	case RTL_VER_09:
-	case RTL_VER_14:
-		r8153b_rx_agg_chg_indicate(tp);
+	case RTL_VER_01:
+	case RTL_VER_02:
+	case RTL_VER_03:
+	case RTL_VER_04:
+	case RTL_VER_05:
+	case RTL_VER_06:
+	case RTL_VER_07:
 		break;
 	default:
+		r8153b_rx_agg_chg_indicate(tp);
 		break;
 	}
 
@@ -3086,7 +3090,6 @@ static void r8153_set_rx_early_timeout(struct r8152 *tp)
 			       640 / 8);
 		ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EXTRA_AGGR_TMR,
 			       ocp_data);
-		r8153b_rx_agg_chg_indicate(tp);
 		break;
 
 	default:
@@ -3120,7 +3123,6 @@ static void r8153_set_rx_early_size(struct r8152 *tp)
 	case RTL_VER_15:
 		ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE,
 			       ocp_data / 8);
-		r8153b_rx_agg_chg_indicate(tp);
 		break;
 	default:
 		WARN_ON_ONCE(1);
-- 
2.40.0
Re: [PATCH net v2 3/3] r8152: move setting r8153b_rx_agg_chg_indicate()
Posted by Andrew Lunn 2 years, 9 months ago
On Thu, Apr 27, 2023 at 08:10:57PM +0800, Hayes Wang wrote:
> Move setting r8153b_rx_agg_chg_indicate() for 2.5G devices. The
> r8153b_rx_agg_chg_indicate() has to be called after enabling tx/rx.
> Otherwise, the coalescing settings are useless.
> 
> Fixes: 195aae321c82 ("r8152: support new chips")
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew