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
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.
Fixes: 195aae321c82 ("r8152: support new chips")
Signed-off-by: Hayes Wang <[email protected]>
---
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..08d1786135f2 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 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 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
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 <[email protected]>
---
drivers/net/usb/r8152.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 08d1786135f2..3ecd4651ae29 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -7554,6 +7554,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, 0xa424);
+ data |= BIT(3);
+ ocp_reg_write(tp, 0xa424, data);
break;
default:
break;
@@ -7949,6 +7954,11 @@ static void r8156b_hw_phy_cfg(struct r8152 *tp)
break;
}
+ /* set intr_en[3] */
+ data = ocp_reg_read(tp, 0xa424);
+ data |= BIT(3);
+ ocp_reg_write(tp, 0xa424, data);
+
if (rtl_phy_patch_request(tp, true, true))
return;
--
2.40.0
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 <[email protected]>
---
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 3ecd4651ae29..c464da385511 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3023,12 +3023,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;
}
@@ -3082,7 +3086,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:
@@ -3116,7 +3119,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
On Wed, Apr 26, 2023 at 08:28:03PM +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.
>
> Fixes: 195aae321c82 ("r8152: support new chips")
> Signed-off-by: Hayes Wang <[email protected]>
> ---
> 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..08d1786135f2 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 inline u32 fc_pause_on_auto(struct r8152 *tp)
> +{
> + return (ALIGN(mtu_to_size(tp->netdev->mtu), 1024) + 6 * 1024);
> +}
No inline functions in .c files. Let the compiler decide. I see you
are just moving functions around, they were already inline, but now is
a good time to fix this.
Andrew
On Wed, Apr 26, 2023 at 08:28:04PM +0800, Hayes Wang wrote:
> 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 <[email protected]>
> ---
> drivers/net/usb/r8152.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 08d1786135f2..3ecd4651ae29 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -7554,6 +7554,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, 0xa424);
> + data |= BIT(3);
> + ocp_reg_write(tp, 0xa424, data);
Please add #define for 0xa424. And a #define for BIT(3), just to
document what bit 3 is. Once we understand what bit 3 is, we might
then understand why this change makes 2.5G perform better. At the
moment all i see is magic.
Andrew
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
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 <[email protected]>
---
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
> @@ -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
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
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