2015-02-11 06:47:04

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 0/3] Adjust the settings about USB_RX_EARLY_AGG

The USB_RX_EARLY_AGG contains timeout and size. Separate them, and
they could be set independently. Then, the ethtool could be used to
change the timeout according to situation of the platform.

Hayes Wang (3):
r8152: separate USB_RX_EARLY_AGG
r8152: change rx early size when the mtu is changed
r8152: support setting rx coalesce

drivers/net/usb/r8152.c | 128 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 101 insertions(+), 27 deletions(-)

--
2.1.0


2015-02-11 06:47:03

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 1/3] r8152: separate USB_RX_EARLY_AGG

Separate USB_RX_EARLY_AGG into USB_RX_EARLY_TIMEOUT and USB_RX_EARLY_SIZE.

Replace r8153_set_rx_agg() with r8153_set_rx_early_timeout() and
r8153_set_rx_early_size().

Set the default timeout value according to the USB speed.

Signed-off-by: Hayes Wang <[email protected]>
---
drivers/net/usb/r8152.c | 55 ++++++++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 5980ac6..b043c7f 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -97,7 +97,8 @@
#define USB_TX_AGG 0xd40a
#define USB_RX_BUF_TH 0xd40c
#define USB_USB_TIMER 0xd428
-#define USB_RX_EARLY_AGG 0xd42c
+#define USB_RX_EARLY_TIMEOUT 0xd42c
+#define USB_RX_EARLY_SIZE 0xd42e
#define USB_PM_CTRL_STATUS 0xd432
#define USB_TX_DMA 0xd434
#define USB_TOLERANCE 0xd490
@@ -325,10 +326,10 @@
/* USB_MISC_0 */
#define PCUT_STATUS 0x0001

-/* USB_RX_EARLY_AGG */
-#define EARLY_AGG_SUPPER 0x0e832981
-#define EARLY_AGG_HIGH 0x0e837a12
-#define EARLY_AGG_SLOW 0x0e83ffff
+/* USB_RX_EARLY_TIMEOUT */
+#define COALESCE_SUPER 85000U
+#define COALESCE_HIGH 250000U
+#define COALESCE_SLOW 524280U

/* USB_WDT11_CTRL */
#define TIMER11_EN 0x0001
@@ -578,6 +579,7 @@ struct r8152 {
u32 saved_wolopts;
u32 msg_enable;
u32 tx_qlen;
+ u32 coalesce;
u16 ocp_base;
u8 *intr_buff;
u8 version;
@@ -2114,28 +2116,21 @@ static int rtl8152_enable(struct r8152 *tp)
return rtl_enable(tp);
}

-static void r8153_set_rx_agg(struct r8152 *tp)
+static void r8153_set_rx_early_timeout(struct r8152 *tp)
{
- u8 speed;
+ u32 ocp_data;

- speed = rtl8152_get_speed(tp);
- if (speed & _1000bps) {
- if (tp->udev->speed == USB_SPEED_SUPER) {
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH,
- RX_THR_SUPPER);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_EARLY_AGG,
- EARLY_AGG_SUPPER);
- } else {
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH,
- RX_THR_HIGH);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_EARLY_AGG,
- EARLY_AGG_HIGH);
- }
- } else {
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH, RX_THR_SLOW);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_EARLY_AGG,
- EARLY_AGG_SLOW);
- }
+ ocp_data = tp->coalesce / 8;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_TIMEOUT, ocp_data);
+}
+
+static void r8153_set_rx_early_size(struct r8152 *tp)
+{
+ struct net_device *dev = tp->netdev;
+ u32 ocp_data;
+
+ ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
}

static int rtl8153_enable(struct r8152 *tp)
@@ -2145,7 +2140,8 @@ static int rtl8153_enable(struct r8152 *tp)

set_tx_qlen(tp);
rtl_set_eee_plus(tp);
- r8153_set_rx_agg(tp);
+ r8153_set_rx_early_timeout(tp);
+ r8153_set_rx_early_size(tp);

return rtl_enable(tp);
}
@@ -3911,6 +3907,13 @@ static int rtl8152_probe(struct usb_interface *intf,
tp->mii.reg_num_mask = 0x1f;
tp->mii.phy_id = R8152_PHY_ID;

+ if (udev->speed == USB_SPEED_SUPER)
+ tp->coalesce = COALESCE_SUPER;
+ else if (udev->speed == USB_SPEED_HIGH)
+ tp->coalesce = COALESCE_HIGH;
+ else
+ tp->coalesce = COALESCE_SLOW;
+
intf->needs_remote_wakeup = 1;

tp->rtl_ops.init(tp);
--
2.1.0

2015-02-11 06:48:05

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 2/3] r8152: change rx early size when the mtu is changed

The rx early size is calculated with the mtu, so it has to be
re-calculated when the mtu is changed.

Signed-off-by: Hayes Wang <[email protected]>
---
drivers/net/usb/r8152.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index b043c7f..c5301ca 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3724,6 +3724,7 @@ out:
static int rtl8152_change_mtu(struct net_device *dev, int new_mtu)
{
struct r8152 *tp = netdev_priv(dev);
+ int ret;

switch (tp->version) {
case RTL_VER_01:
@@ -3736,9 +3737,22 @@ static int rtl8152_change_mtu(struct net_device *dev, int new_mtu)
if (new_mtu < 68 || new_mtu > RTL8153_MAX_MTU)
return -EINVAL;

+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ return ret;
+
+ mutex_lock(&tp->control);
+
dev->mtu = new_mtu;

- return 0;
+ if (netif_running(dev) && netif_carrier_ok(dev))
+ r8153_set_rx_early_size(tp);
+
+ mutex_unlock(&tp->control);
+
+ usb_autopm_put_interface(tp->intf);
+
+ return ret;
}

static const struct net_device_ops rtl8152_netdev_ops = {
--
2.1.0

2015-02-11 06:47:42

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 3/3] r8152: support setting rx coalesce

Support setting the rx coalesce. Then someone could change the rx
agg timeout value through ethtool.

Signed-off-by: Hayes Wang <[email protected]>
---
drivers/net/usb/r8152.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index c5301ca..e4e7238 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3660,6 +3660,61 @@ out:
return ret;
}

+static int rtl8152_get_coalesce(struct net_device *netdev,
+ struct ethtool_coalesce *coalesce)
+{
+ struct r8152 *tp = netdev_priv(netdev);
+
+ switch (tp->version) {
+ case RTL_VER_01:
+ case RTL_VER_02:
+ return -EOPNOTSUPP;
+ default:
+ break;
+ }
+
+ coalesce->rx_coalesce_usecs = tp->coalesce;
+
+ return 0;
+}
+
+static int rtl8152_set_coalesce(struct net_device *netdev,
+ struct ethtool_coalesce *coalesce)
+{
+ struct r8152 *tp = netdev_priv(netdev);
+ int ret;
+
+ switch (tp->version) {
+ case RTL_VER_01:
+ case RTL_VER_02:
+ return -EOPNOTSUPP;
+ default:
+ break;
+ }
+
+ if (coalesce->rx_coalesce_usecs > COALESCE_SLOW)
+ return -EINVAL;
+
+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ return ret;
+
+ mutex_lock(&tp->control);
+
+ if (tp->coalesce != coalesce->rx_coalesce_usecs) {
+ tp->coalesce = coalesce->rx_coalesce_usecs;
+
+ if (netif_running(tp->netdev) && netif_carrier_ok(netdev))
+ r8153_set_rx_early_timeout(tp);
+ }
+
+ mutex_unlock(&tp->control);
+
+ usb_autopm_put_interface(tp->intf);
+
+ return ret;
+}
+
static struct ethtool_ops ops = {
.get_drvinfo = rtl8152_get_drvinfo,
.get_settings = rtl8152_get_settings,
@@ -3673,6 +3728,8 @@ static struct ethtool_ops ops = {
.get_strings = rtl8152_get_strings,
.get_sset_count = rtl8152_get_sset_count,
.get_ethtool_stats = rtl8152_get_ethtool_stats,
+ .get_coalesce = rtl8152_get_coalesce,
+ .set_coalesce = rtl8152_set_coalesce,
.get_eee = rtl_ethtool_get_eee,
.set_eee = rtl_ethtool_set_eee,
};
--
2.1.0

2015-02-11 13:52:26

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH net-next 1/3] r8152: separate USB_RX_EARLY_AGG

Hello.

On 2/11/2015 9:46 AM, Hayes Wang wrote:

> Separate USB_RX_EARLY_AGG into USB_RX_EARLY_TIMEOUT and USB_RX_EARLY_SIZE.

> Replace r8153_set_rx_agg() with r8153_set_rx_early_timeout() and
> r8153_set_rx_early_size().

> Set the default timeout value according to the USB speed.

> Signed-off-by: Hayes Wang <[email protected]>
> ---
> drivers/net/usb/r8152.c | 55 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 29 insertions(+), 26 deletions(-)

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 5980ac6..b043c7f 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[...]
> @@ -2114,28 +2116,21 @@ static int rtl8152_enable(struct r8152 *tp)
> return rtl_enable(tp);
> }
>
> -static void r8153_set_rx_agg(struct r8152 *tp)
> +static void r8153_set_rx_early_timeout(struct r8152 *tp)
> {
> - u8 speed;
> + u32 ocp_data;
>
[...]
> + ocp_data = tp->coalesce / 8;

Why not do it in the initializer?

> + ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_TIMEOUT, ocp_data);
> +}
> +
> +static void r8153_set_rx_early_size(struct r8152 *tp)
> +{
> + struct net_device *dev = tp->netdev;

Not sure you actually need this variable.

> + u32 ocp_data;
> +
> + ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;

Why not in initializer?

> + ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
> }
[...]
> @@ -3911,6 +3907,13 @@ static int rtl8152_probe(struct usb_interface *intf,
> tp->mii.reg_num_mask = 0x1f;
> tp->mii.phy_id = R8152_PHY_ID;
>
> + if (udev->speed == USB_SPEED_SUPER)
> + tp->coalesce = COALESCE_SUPER;
> + else if (udev->speed == USB_SPEED_HIGH)
> + tp->coalesce = COALESCE_HIGH;
> + else
> + tp->coalesce = COALESCE_SLOW;

This is asking to be a *switch* statement.

[...]

WBR, Sergei

2015-02-12 02:36:26

by Hayes Wang

[permalink] [raw]
Subject: RE: [PATCH net-next 1/3] r8152: separate USB_RX_EARLY_AGG

Sergei Shtylyov [mailto:[email protected]]
[...]
> > + ocp_data = tp->coalesce / 8;
>
> Why not do it in the initializer?

This is for patch #3. The patch #3 would use this function.
The unit of the relative setting from the ethtool is 1 us.
However, the unit for the hw is 8 us. Therefore, I save the
value with the unit of 1 us, and transfer it to the unit of
the hw when setting.

> > + ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_TIMEOUT, ocp_data);
> > +}
> > +
> > +static void r8153_set_rx_early_size(struct r8152 *tp)
> > +{
> > + struct net_device *dev = tp->netdev;
>
> Not sure you actually need this variable.

If I replace dev->mtu with tp->netdev->mtu, the line would
more than 80 characters. This is used to avoid it. Should
I remove it?

> > + u32 ocp_data;
> > +
> > + ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
>
> Why not in initializer?

This is for patch #2. The patch #2 would use this function.
It has to be re-calculated when the mtu is changed, or the
function is called when the linking status changes to ON.

> > + ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
> > }
> [...]
> > @@ -3911,6 +3907,13 @@ static int rtl8152_probe(struct
> usb_interface *intf,
> > tp->mii.reg_num_mask = 0x1f;
> > tp->mii.phy_id = R8152_PHY_ID;
> >
> > + if (udev->speed == USB_SPEED_SUPER)
> > + tp->coalesce = COALESCE_SUPER;
> > + else if (udev->speed == USB_SPEED_HIGH)
> > + tp->coalesce = COALESCE_HIGH;
> > + else
> > + tp->coalesce = COALESCE_SLOW;
>
> This is asking to be a *switch* statement.

Excuse me. I don't understand what you mean.
The usb speed is determined when the device is plugged on
the usb host controller or usb hub. The usb speed wouldn't
chage unless you unplug the device and plug it to another
port with different usb speed. Therefore, I provide different
default values for different usb speed.

Best Regards,
Hayes

2015-02-12 06:04:15

by Hayes Wang

[permalink] [raw]
Subject: RE: [PATCH net-next 1/3] r8152: separate USB_RX_EARLY_AGG

Sergei Shtylyov; [email protected]
[...]
> > > + ocp_data = tp->coalesce / 8;
> >
> > Why not do it in the initializer?
>
> This is for patch #3. The patch #3 would use this function.
> The unit of the relative setting from the ethtool is 1 us.
> However, the unit for the hw is 8 us. Therefore, I save the
> value with the unit of 1 us, and transfer it to the unit of
> the hw when setting.

I think I misunderstand what you mean. I think you mean I
have to combine

u32 ocp_data;
ocp_data = tp->coalesce / 8;

into

u32 ocp_data = tp->coalesce / 8;

I would correct it.

> > > + ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_TIMEOUT, ocp_data);
> > > +}
> > > +
> > > +static void r8153_set_rx_early_size(struct r8152 *tp)
> > > +{
> > > + struct net_device *dev = tp->netdev;
> >
> > Not sure you actually need this variable.
>
> If I replace dev->mtu with tp->netdev->mtu, the line would
> more than 80 characters. This is used to avoid it. Should
> I remove it?
>
> > > + u32 ocp_data;
> > > +
> > > + ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
> >
> > Why not in initializer?
>
> This is for patch #2. The patch #2 would use this function.
> It has to be re-calculated when the mtu is changed, or the
> function is called when the linking status changes to ON.

I think you mean I have to combine

u32 ocp_data;
ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;

into

u32 ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;

I would correct it.

> > > + ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
> > > }
> > [...]
> > > @@ -3911,6 +3907,13 @@ static int rtl8152_probe(struct
> > usb_interface *intf,
> > > tp->mii.reg_num_mask = 0x1f;
> > > tp->mii.phy_id = R8152_PHY_ID;
> > >
> > > + if (udev->speed == USB_SPEED_SUPER)
> > > + tp->coalesce = COALESCE_SUPER;
> > > + else if (udev->speed == USB_SPEED_HIGH)
> > > + tp->coalesce = COALESCE_HIGH;
> > > + else
> > > + tp->coalesce = COALESCE_SLOW;
> >
> > This is asking to be a *switch* statement.
>
> Excuse me. I don't understand what you mean.
> The usb speed is determined when the device is plugged on
> the usb host controller or usb hub. The usb speed wouldn't
> chage unless you unplug the device and plug it to another
> port with different usb speed. Therefore, I provide different
> default values for different usb speed.

I think you mean

switch (udev->speed) {
case USB_SPEED_SUPER:
...

I would correct it.

Best Regards,
Hayes

2015-02-12 06:33:50

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next v2 0/3] Adjust the settings about USB_RX_EARLY_AGG

v2:
For patch #1, replace

u32 ocp_data;
ocp_data = tp->coalesce / 8;

with

u32 ocp_data = tp->coalesce / 8;

And replace

struct net_device *dev = tp->netdev;
u32 ocp_data;
ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;

with

u32 mtu = tp->netdev->mtu;
u32 ocp_data = (agg_buf_sz - mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;

Use *switch* statement to replace the checking of *if*.

v1:
The USB_RX_EARLY_AGG contains timeout and size. Separate them, and
they could be set independently. Then, the ethtool could be used to
change the timeout according to situation of the platform.

Hayes Wang (3):
r8152: separate USB_RX_EARLY_AGG
r8152: change rx early size when the mtu is changed
r8152: support setting rx coalesce

drivers/net/usb/r8152.c | 128 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 101 insertions(+), 27 deletions(-)

--
2.1.0

2015-02-12 06:33:52

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next v2 1/3] r8152: separate USB_RX_EARLY_AGG

Separate USB_RX_EARLY_AGG into USB_RX_EARLY_TIMEOUT and USB_RX_EARLY_SIZE.

Replace r8153_set_rx_agg() with r8153_set_rx_early_timeout() and
r8153_set_rx_early_size().

Set the default timeout value according to the USB speed.

Signed-off-by: Hayes Wang <[email protected]>
---
drivers/net/usb/r8152.c | 58 +++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 5980ac6..b2e6566 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -97,7 +97,8 @@
#define USB_TX_AGG 0xd40a
#define USB_RX_BUF_TH 0xd40c
#define USB_USB_TIMER 0xd428
-#define USB_RX_EARLY_AGG 0xd42c
+#define USB_RX_EARLY_TIMEOUT 0xd42c
+#define USB_RX_EARLY_SIZE 0xd42e
#define USB_PM_CTRL_STATUS 0xd432
#define USB_TX_DMA 0xd434
#define USB_TOLERANCE 0xd490
@@ -325,10 +326,10 @@
/* USB_MISC_0 */
#define PCUT_STATUS 0x0001

-/* USB_RX_EARLY_AGG */
-#define EARLY_AGG_SUPPER 0x0e832981
-#define EARLY_AGG_HIGH 0x0e837a12
-#define EARLY_AGG_SLOW 0x0e83ffff
+/* USB_RX_EARLY_TIMEOUT */
+#define COALESCE_SUPER 85000U
+#define COALESCE_HIGH 250000U
+#define COALESCE_SLOW 524280U

/* USB_WDT11_CTRL */
#define TIMER11_EN 0x0001
@@ -578,6 +579,7 @@ struct r8152 {
u32 saved_wolopts;
u32 msg_enable;
u32 tx_qlen;
+ u32 coalesce;
u16 ocp_base;
u8 *intr_buff;
u8 version;
@@ -2114,28 +2116,19 @@ static int rtl8152_enable(struct r8152 *tp)
return rtl_enable(tp);
}

-static void r8153_set_rx_agg(struct r8152 *tp)
+static void r8153_set_rx_early_timeout(struct r8152 *tp)
{
- u8 speed;
+ u32 ocp_data = tp->coalesce / 8;

- speed = rtl8152_get_speed(tp);
- if (speed & _1000bps) {
- if (tp->udev->speed == USB_SPEED_SUPER) {
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH,
- RX_THR_SUPPER);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_EARLY_AGG,
- EARLY_AGG_SUPPER);
- } else {
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH,
- RX_THR_HIGH);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_EARLY_AGG,
- EARLY_AGG_HIGH);
- }
- } else {
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH, RX_THR_SLOW);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_EARLY_AGG,
- EARLY_AGG_SLOW);
- }
+ ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_TIMEOUT, ocp_data);
+}
+
+static void r8153_set_rx_early_size(struct r8152 *tp)
+{
+ u32 mtu = tp->netdev->mtu;
+ u32 ocp_data = (agg_buf_sz - mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
+
+ ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
}

static int rtl8153_enable(struct r8152 *tp)
@@ -2145,7 +2138,8 @@ static int rtl8153_enable(struct r8152 *tp)

set_tx_qlen(tp);
rtl_set_eee_plus(tp);
- r8153_set_rx_agg(tp);
+ r8153_set_rx_early_timeout(tp);
+ r8153_set_rx_early_size(tp);

return rtl_enable(tp);
}
@@ -3911,6 +3905,18 @@ static int rtl8152_probe(struct usb_interface *intf,
tp->mii.reg_num_mask = 0x1f;
tp->mii.phy_id = R8152_PHY_ID;

+ switch (udev->speed) {
+ case USB_SPEED_SUPER:
+ tp->coalesce = COALESCE_SUPER;
+ break;
+ case USB_SPEED_HIGH:
+ tp->coalesce = COALESCE_HIGH;
+ break;
+ default:
+ tp->coalesce = COALESCE_SLOW;
+ break;
+ }
+
intf->needs_remote_wakeup = 1;

tp->rtl_ops.init(tp);
--
2.1.0

2015-02-12 06:34:36

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next v2 2/3] r8152: change rx early size when the mtu is changed

The rx early size is calculated with the mtu, so it has to be
re-calculated when the mtu is changed.

Signed-off-by: Hayes Wang <[email protected]>
---
drivers/net/usb/r8152.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index b2e6566..46b99c6 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3722,6 +3722,7 @@ out:
static int rtl8152_change_mtu(struct net_device *dev, int new_mtu)
{
struct r8152 *tp = netdev_priv(dev);
+ int ret;

switch (tp->version) {
case RTL_VER_01:
@@ -3734,9 +3735,22 @@ static int rtl8152_change_mtu(struct net_device *dev, int new_mtu)
if (new_mtu < 68 || new_mtu > RTL8153_MAX_MTU)
return -EINVAL;

+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ return ret;
+
+ mutex_lock(&tp->control);
+
dev->mtu = new_mtu;

- return 0;
+ if (netif_running(dev) && netif_carrier_ok(dev))
+ r8153_set_rx_early_size(tp);
+
+ mutex_unlock(&tp->control);
+
+ usb_autopm_put_interface(tp->intf);
+
+ return ret;
}

static const struct net_device_ops rtl8152_netdev_ops = {
--
2.1.0

2015-02-12 06:34:02

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next v2 3/3] r8152: support setting rx coalesce

Support setting the rx coalesce. Then someone could change the rx
agg timeout value through ethtool.

Signed-off-by: Hayes Wang <[email protected]>
---
drivers/net/usb/r8152.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 46b99c6..1f8921b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3658,6 +3658,61 @@ out:
return ret;
}

+static int rtl8152_get_coalesce(struct net_device *netdev,
+ struct ethtool_coalesce *coalesce)
+{
+ struct r8152 *tp = netdev_priv(netdev);
+
+ switch (tp->version) {
+ case RTL_VER_01:
+ case RTL_VER_02:
+ return -EOPNOTSUPP;
+ default:
+ break;
+ }
+
+ coalesce->rx_coalesce_usecs = tp->coalesce;
+
+ return 0;
+}
+
+static int rtl8152_set_coalesce(struct net_device *netdev,
+ struct ethtool_coalesce *coalesce)
+{
+ struct r8152 *tp = netdev_priv(netdev);
+ int ret;
+
+ switch (tp->version) {
+ case RTL_VER_01:
+ case RTL_VER_02:
+ return -EOPNOTSUPP;
+ default:
+ break;
+ }
+
+ if (coalesce->rx_coalesce_usecs > COALESCE_SLOW)
+ return -EINVAL;
+
+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ return ret;
+
+ mutex_lock(&tp->control);
+
+ if (tp->coalesce != coalesce->rx_coalesce_usecs) {
+ tp->coalesce = coalesce->rx_coalesce_usecs;
+
+ if (netif_running(tp->netdev) && netif_carrier_ok(netdev))
+ r8153_set_rx_early_timeout(tp);
+ }
+
+ mutex_unlock(&tp->control);
+
+ usb_autopm_put_interface(tp->intf);
+
+ return ret;
+}
+
static struct ethtool_ops ops = {
.get_drvinfo = rtl8152_get_drvinfo,
.get_settings = rtl8152_get_settings,
@@ -3671,6 +3726,8 @@ static struct ethtool_ops ops = {
.get_strings = rtl8152_get_strings,
.get_sset_count = rtl8152_get_sset_count,
.get_ethtool_stats = rtl8152_get_ethtool_stats,
+ .get_coalesce = rtl8152_get_coalesce,
+ .set_coalesce = rtl8152_set_coalesce,
.get_eee = rtl_ethtool_get_eee,
.set_eee = rtl_ethtool_set_eee,
};
--
2.1.0

2015-02-12 11:37:23

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH net-next 1/3] r8152: separate USB_RX_EARLY_AGG

Hello.

On 2/12/2015 5:36 AM, Hayes Wang wrote:

> [...]
>>> + ocp_data = tp->coalesce / 8;

>> Why not do it in the initializer?

> This is for patch #3. The patch #3 would use this function.

The new function is already called in this patch.

> The unit of the relative setting from the ethtool is 1 us.
> However, the unit for the hw is 8 us. Therefore, I save the
> value with the unit of 1 us, and transfer it to the unit of
> the hw when setting.

You're replying to the question I didn't ask. I was just suggesting:

u32 ocp_data = tp->coalesce / 8;

>>> + ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_TIMEOUT, ocp_data);

... if you don't want to pass 'tp->coalesce / 8' directly here.

>>> +}
>>> +
>>> +static void r8153_set_rx_early_size(struct r8152 *tp)
>>> +{
>>> + struct net_device *dev = tp->netdev;

>> Not sure you actually need this variable.

> If I replace dev->mtu with tp->netdev->mtu, the line would
> more than 80 characters. This is used to avoid it. Should
> I remove it?

OK, you can keep it.

>>> + u32 ocp_data;
>>> +
>>> + ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;

>> Why not in initializer?

> This is for patch #2. The patch #2 would use this function.

The new function is again used in this patch already.

> It has to be re-calculated when the mtu is changed, or the
> function is called when the linking status changes to ON.

You're again replying to the question I didn't ask. I was just suggesting:

u32 ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;

>>> + ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);

... if you don't want to pass that expression directly here.

[...]
>>> @@ -3911,6 +3907,13 @@ static int rtl8152_probe(struct
>>> usb_interface *intf,
>>> tp->mii.reg_num_mask = 0x1f;
>>> tp->mii.phy_id = R8152_PHY_ID;
>>>
>>> + if (udev->speed == USB_SPEED_SUPER)
>>> + tp->coalesce = COALESCE_SUPER;
>>> + else if (udev->speed == USB_SPEED_HIGH)
>>> + tp->coalesce = COALESCE_HIGH;
>>> + else
>>> + tp->coalesce = COALESCE_SLOW;

>> This is asking to be a *switch* statement.

> Excuse me. I don't understand what you mean.

switch (udev->speed) {
case USB_SPEED_SUPER:
tp->coalesce = COALESCE_SUPER;
break;
case USB_SPEED_HIGH:
tp->coalesce = COALESCE_HIGH;
break;
default:
tp->coalesce = COALESCE_SLOW;
}

> The usb speed is determined when the device is plugged on
> the usb host controller or usb hub. The usb speed wouldn't
> chage unless you unplug the device and plug it to another
> port with different usb speed. Therefore, I provide different
> default values for different usb speed.

I didn't ask for explanations here either. :-)

> Best Regards,
> Hayes

WBR, Sergei

2015-02-19 20:09:14

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/3] Adjust the settings about USB_RX_EARLY_AGG

From: Hayes Wang <[email protected]>
Date: Thu, 12 Feb 2015 14:33:45 +0800

> v2:
> For patch #1, replace
>
> u32 ocp_data;
> ocp_data = tp->coalesce / 8;
>
> with
>
> u32 ocp_data = tp->coalesce / 8;
>
> And replace
>
> struct net_device *dev = tp->netdev;
> u32 ocp_data;
> ocp_data = (agg_buf_sz - dev->mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
>
> with
>
> u32 mtu = tp->netdev->mtu;
> u32 ocp_data = (agg_buf_sz - mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
>
> Use *switch* statement to replace the checking of *if*.
>
> v1:
> The USB_RX_EARLY_AGG contains timeout and size. Separate them, and
> they could be set independently. Then, the ethtool could be used to
> change the timeout according to situation of the platform.

Series applied, thanks.