[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

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 <redacted>
---
 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

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

Signed-off-by: Hayes Wang <redacted>
---
 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

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 <redacted>
---
 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

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 <redacted>
---
  drivers/net/usb/r8152.c | 55 ++++++++++++++++++++++++++-----------------------
  1 file changed, 29 insertions(+), 26 deletions(-)

[...]

[...]

+	ocp_data = tp->coalesce / 8;

   Why not do it in the initializer?

   Not sure you actually need this variable.

   Why not in initializer?

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

[...]

   This is asking to be a *switch* statement.

[...]

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com] 
[...]

quoted

+	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.

quoted

+	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?

quoted

+	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.

quoted

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

[...]

quoted

@@ -3911,6 +3907,13 @@ static int rtl8152_probe(struct 

usb_interface *intf,

quoted

  	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

 Sergei Shtylyov; netdev@vger.kernel.org
[...]

quoted

quoted

+	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.

quoted

quoted

+	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?

quoted

quoted

+	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.

quoted

quoted

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

[...]

quoted

@@ -3911,6 +3907,13 @@ static int rtl8152_probe(struct

usb_interface *intf,

quoted

  	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

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

--

3 more lines

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 <redacted>
---
 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

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

Signed-off-by: Hayes Wang <redacted>
---
 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

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 <redacted>
---
 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

Hello.

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

[...]

quoted

quoted

+	ocp_data = tp->coalesce / 8;

quoted

     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;

quoted

quoted

+	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.

quoted

quoted

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

quoted

     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.

quoted

quoted

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

quoted

     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;

quoted

quoted

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

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

[...]

quoted

quoted

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

quoted

     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

From: Hayes Wang <redacted>
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.