[PATCH net 0/3] r8152: issues fix

[PATCH net 0/3] r8152: issues fix

[Hayes Wang]

These patches are used to fix issues.

Hayes Wang (3):
  r8152: fix the issue about U1/U2
  r8152: fix remote wakeup
  r8152: don't enable napi before rx ready

 drivers/net/usb/r8152.c | 103 ++++++++++++++++++++++++++++--------------------
 1 file changed, 60 insertions(+), 43 deletions(-)

-- 
2.4.2

[PATCH net 1/3] r8152: fix the issue about U1/U2

[Hayes Wang]

- Disable U1/U2 during initialization.
- Disable lpm when linking is on, and enable it when linking is off.
- Disable U1/U2 when enabling runtime suspend.

It is possible to let hw stop working, if the U1/U2 request occurs
during some situations. The patch is used to avoid it.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 94 ++++++++++++++++++++++++++++---------------------
 1 file changed, 54 insertions(+), 40 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 7f6419e..e3a0110 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2166,6 +2166,7 @@ static int rtl8153_enable(struct r8152 *tp)
 	if (test_bit(RTL8152_UNPLUG, &tp->flags))
 		return -ENODEV;
 
+	usb_disable_lpm(tp->udev);
 	set_tx_qlen(tp);
 	rtl_set_eee_plus(tp);
 	r8153_set_rx_early_timeout(tp);
@@ -2337,11 +2338,54 @@ static void __rtl_set_wol(struct r8152 *tp, u32 wolopts)
 		device_set_wakeup_enable(&tp->udev->dev, false);
 }
 
+static void r8153_u1u2en(struct r8152 *tp, bool enable)
+{
+	u8 u1u2[8];
+
+	if (enable)
+		memset(u1u2, 0xff, sizeof(u1u2));
+	else
+		memset(u1u2, 0x00, sizeof(u1u2));
+
+	usb_ocp_write(tp, USB_TOLERANCE, BYTE_EN_SIX_BYTES, sizeof(u1u2), u1u2);
+}
+
+static void r8153_u2p3en(struct r8152 *tp, bool enable)
+{
+	u32 ocp_data;
+
+	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL);
+	if (enable && tp->version != RTL_VER_03 && tp->version != RTL_VER_04)
+		ocp_data |= U2P3_ENABLE;
+	else
+		ocp_data &= ~U2P3_ENABLE;
+	ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
+}
+
+static void r8153_power_cut_en(struct r8152 *tp, bool enable)
+{
+	u32 ocp_data;
+
+	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_POWER_CUT);
+	if (enable)
+		ocp_data |= PWR_EN | PHASE2_EN;
+	else
+		ocp_data &= ~(PWR_EN | PHASE2_EN);
+	ocp_write_word(tp, MCU_TYPE_USB, USB_POWER_CUT, ocp_data);
+
+	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
+	ocp_data &= ~PCUT_STATUS;
+	ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
+}
+
 static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
 {
 	if (enable) {
 		u32 ocp_data;
 
+		r8153_u1u2en(tp, false);
+		r8153_u2p3en(tp, false);
+
 		__rtl_set_wol(tp, WAKE_ANY);
 
 		ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);
@@ -2353,6 +2397,8 @@ static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
 		ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
 	} else {
 		__rtl_set_wol(tp, tp->saved_wolopts);
+		r8153_u2p3en(tp, true);
+		r8153_u1u2en(tp, true);
 	}
 }
 
@@ -2599,46 +2645,6 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
 	set_bit(PHY_RESET, &tp->flags);
 }
 
-static void r8153_u1u2en(struct r8152 *tp, bool enable)
-{
-	u8 u1u2[8];
-
-	if (enable)
-		memset(u1u2, 0xff, sizeof(u1u2));
-	else
-		memset(u1u2, 0x00, sizeof(u1u2));
-
-	usb_ocp_write(tp, USB_TOLERANCE, BYTE_EN_SIX_BYTES, sizeof(u1u2), u1u2);
-}
-
-static void r8153_u2p3en(struct r8152 *tp, bool enable)
-{
-	u32 ocp_data;
-
-	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL);
-	if (enable)
-		ocp_data |= U2P3_ENABLE;
-	else
-		ocp_data &= ~U2P3_ENABLE;
-	ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
-}
-
-static void r8153_power_cut_en(struct r8152 *tp, bool enable)
-{
-	u32 ocp_data;
-
-	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_POWER_CUT);
-	if (enable)
-		ocp_data |= PWR_EN | PHASE2_EN;
-	else
-		ocp_data &= ~(PWR_EN | PHASE2_EN);
-	ocp_write_word(tp, MCU_TYPE_USB, USB_POWER_CUT, ocp_data);
-
-	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
-	ocp_data &= ~PCUT_STATUS;
-	ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
-}
-
 static void r8153_first_init(struct r8152 *tp)
 {
 	u32 ocp_data;
@@ -2781,6 +2787,7 @@ static void rtl8153_disable(struct r8152 *tp)
 	r8153_disable_aldps(tp);
 	rtl_disable(tp);
 	r8153_enable_aldps(tp);
+	usb_enable_lpm(tp->udev);
 }
 
 static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u16 speed, u8 duplex)
@@ -2901,9 +2908,13 @@ static void rtl8153_up(struct r8152 *tp)
 	if (test_bit(RTL8152_UNPLUG, &tp->flags))
 		return;
 
+	r8153_u1u2en(tp, false);
 	r8153_disable_aldps(tp);
 	r8153_first_init(tp);
 	r8153_enable_aldps(tp);
+	r8153_u2p3en(tp, true);
+	r8153_u1u2en(tp, true);
+	usb_enable_lpm(tp->udev);
 }
 
 static void rtl8153_down(struct r8152 *tp)
@@ -2914,6 +2925,7 @@ static void rtl8153_down(struct r8152 *tp)
 	}
 
 	r8153_u1u2en(tp, false);
+	r8153_u2p3en(tp, false);
 	r8153_power_cut_en(tp, false);
 	r8153_disable_aldps(tp);
 	r8153_enter_oob(tp);
@@ -3252,6 +3264,7 @@ static void r8153_init(struct r8152 *tp)
 		msleep(20);
 	}
 
+	usb_disable_lpm(tp->udev);
 	r8153_u2p3en(tp, false);
 
 	if (tp->version == RTL_VER_04) {
@@ -3319,6 +3332,7 @@ static void r8153_init(struct r8152 *tp)
 	r8153_enable_aldps(tp);
 	r8152b_enable_fc(tp);
 	rtl_tally_reset(tp);
+	r8153_u2p3en(tp, true);
 }
 
 static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
-- 
2.4.2

[PATCH net 2/3] r8152: fix remote wakeup

[Hayes Wang]

Set needs_remote_wakeup only when the device supports it.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e3a0110..eff1f25 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -4059,7 +4059,8 @@ static int rtl8152_probe(struct usb_interface *intf,
 		break;
 	}
 
-	intf->needs_remote_wakeup = 1;
+	if (udev->actconfig->desc.bmAttributes & USB_CONFIG_ATT_WAKEUP)
+		intf->needs_remote_wakeup = 1;
 
 	tp->rtl_ops.init(tp);
 	set_ethernet_addr(tp);
-- 
2.4.2

Re: [PATCH net 2/3] r8152: fix remote wakeup

[Oliver Neukum]

On Thu, 2015-07-23 at 15:09 +0800, Hayes Wang wrote:
> Set needs_remote_wakeup only when the device supports it.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>  drivers/net/usb/r8152.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index e3a0110..eff1f25 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -4059,7 +4059,8 @@ static int rtl8152_probe(struct usb_interface *intf,
>  		break;
>  	}
>  
> -	intf->needs_remote_wakeup = 1;
> +	if (udev->actconfig->desc.bmAttributes & USB_CONFIG_ATT_WAKEUP)
> +		intf->needs_remote_wakeup = 1;

Hi,

this is most likely wrong. Usbcore does check for a device's ability to
do remote wakeup and will block a runtime suspend if it detects that
a remote wakeup would be required but the device cannot deliver.
(static int autosuspend_check())

So by removing the flag in the probe() method means that devices will
suspend during operations without remote wakeup requested. Thus an
incoming packet cannot wake them up.

If you remove setting the flag on probe() you need to set it at open()
[and reset on close()], as devices which cannot do remote wakeup must
only be suspended when they are down.

	Regards
		Oliver

Re: [PATCH net 2/3] r8152: fix remote wakeup

[Oliver Neukum]

On Thu, 2015-07-23 at 15:09 +0800, Hayes Wang wrote:
> Set needs_remote_wakeup only when the device supports it.
> 
> Signed-off-by: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/net/usb/r8152.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index e3a0110..eff1f25 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -4059,7 +4059,8 @@ static int rtl8152_probe(struct usb_interface *intf,
>  		break;
>  	}
>  
> -	intf->needs_remote_wakeup = 1;
> +	if (udev->actconfig->desc.bmAttributes & USB_CONFIG_ATT_WAKEUP)
> +		intf->needs_remote_wakeup = 1;

Hi,

this is most likely wrong. Usbcore does check for a device's ability to
do remote wakeup and will block a runtime suspend if it detects that
a remote wakeup would be required but the device cannot deliver.
(static int autosuspend_check())

So by removing the flag in the probe() method means that devices will
suspend during operations without remote wakeup requested. Thus an
incoming packet cannot wake them up.

If you remove setting the flag on probe() you need to set it at open()
[and reset on close()], as devices which cannot do remote wakeup must
only be suspended when they are down.

	Regards
		Oliver



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

RE: [PATCH net 2/3] r8152: fix remote wakeup

[Hayes Wang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1251 bytes --]

> Hi,
> 
> this is most likely wrong. Usbcore does check for a device's ability to
> do remote wakeup and will block a runtime suspend if it detects that
> a remote wakeup would be required but the device cannot deliver.
> (static int autosuspend_check())
> 
> So by removing the flag in the probe() method means that devices will
> suspend during operations without remote wakeup requested. Thus an
> incoming packet cannot wake them up.
> 
> If you remove setting the flag on probe() you need to set it at open()
> [and reset on close()], as devices which cannot do remote wakeup must
> only be suspended when they are down.

Hi,

I don't think I understand your description clearly. My idea is that if the
device doesn't support wakeup, we don't need set " needs_remote_wakeup".
We allow the device could be suspended and couldn't be waked up by
incoming packet. The system could be waked up by other methods except
by the device.

I don't understand why I have to set it at open() and reset it at close(). And
why must the device only be suspended when it is down?

Best Regards,
Hayes

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH net 2/3] r8152: fix remote wakeup

[Hayes Wang]

> Hi,
> 
> this is most likely wrong. Usbcore does check for a device's ability to
> do remote wakeup and will block a runtime suspend if it detects that
> a remote wakeup would be required but the device cannot deliver.
> (static int autosuspend_check())
> 
> So by removing the flag in the probe() method means that devices will
> suspend during operations without remote wakeup requested. Thus an
> incoming packet cannot wake them up.
> 
> If you remove setting the flag on probe() you need to set it at open()
> [and reset on close()], as devices which cannot do remote wakeup must
> only be suspended when they are down.

Hi,

I don't think I understand your description clearly. My idea is that if the
device doesn't support wakeup, we don't need set " needs_remote_wakeup".
We allow the device could be suspended and couldn't be waked up by
incoming packet. The system could be waked up by other methods except
by the device.

I don't understand why I have to set it at open() and reset it at close(). And
why must the device only be suspended when it is down?

Best Regards,
Hayes

Re: [PATCH net 2/3] r8152: fix remote wakeup

[Oliver Neukum]

On Thu, 2015-07-23 at 09:55 +0000, Hayes Wang wrote:
> > Hi,
> > 
> > this is most likely wrong. Usbcore does check for a device's ability to
> > do remote wakeup and will block a runtime suspend if it detects that
> > a remote wakeup would be required but the device cannot deliver.
> > (static int autosuspend_check())
> > 
> > So by removing the flag in the probe() method means that devices will
> > suspend during operations without remote wakeup requested. Thus an
> > incoming packet cannot wake them up.
> > 
> > If you remove setting the flag on probe() you need to set it at open()
> > [and reset on close()], as devices which cannot do remote wakeup must
> > only be suspended when they are down.
> 
> Hi,
> 
> I don't think I understand your description clearly. My idea is that if the

Sorry, I will try to be clearer.

> device doesn't support wakeup, we don't need set " needs_remote_wakeup".

The algorithm for power saving needs remote wakeup.

> We allow the device could be suspended and couldn't be waked up by
> incoming packet. The system could be waked up by other methods except
> by the device.

The system is not the problem. I was talking about device level power
management at runtime.

> I don't understand why I have to set it at open() and reset it at close(). And
> why must the device only be suspended when it is down?

OK,

the driver right now requests that runtime power management be done:

static struct usb_driver rtl8152_driver = {
	.name =		MODULENAME,
	.id_table =	rtl8152_table,
	.probe =	rtl8152_probe,
	.disconnect =	rtl8152_disconnect,
	.suspend =	rtl8152_suspend,
	.resume =	rtl8152_resume,
	.reset_resume =	rtl8152_resume,
	.supports_autosuspend = 1,

	^ the crucial flag

	.disable_hub_initiated_lpm = 1,
};

It implements an advanced form of runtime power management which
requires remote wakeup.
For the device to work correctly it must not be suspended while

a - a setting is changed
b - a packet is transmitted
c - a packet is received

Cases a and b are covered by the driver on its own.
Case a see for example rtl8152_set_mac_address()
Case b is handled in rtl8152_start_xmit()

		if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
			set_bit(SCHEDULE_NAPI, &tp->flags);
			schedule_delayed_work(&tp->schedule, 0);
		} else {
			usb_mark_last_busy(tp->udev);

The scheduled work will resume the device if necessary.

Case c is handled in rtl8152_resume()

	if (netif_running(tp->netdev)) {
		if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
			rtl_runtime_suspend_enable(tp, false);
			clear_bit(SELECTIVE_SUSPEND, &tp->flags);
			set_bit(WORK_ENABLE, &tp->flags);
			if (netif_carrier_ok(tp->netdev))
				rtl_start_rx(tp);

But that code path only runs if remote wakeup is enabled.
So to operate with runtime power management is is necessary
that the device support remote wakeup and the driver enable it.

If the device does not support remote wakeup and the driver
enables it, runtime power management will be switched off.
That is the current state and it means that devices which
don't support remote wakeup cannot do runtime power management
at all. But the driver is correct.

The only time a device that doesn't support remote wakeup can
do runtime power managent is when no packets can be received
that is while the interface is down. If you want to allow that
you must not set needs_remote_wakeup in probe(), but you must
set it in open() because it is necessary for runtime power
management as I explained above.

Sorry for the length of this mail, but I wanted to make sure
I am absolutely clear this time.

	HTH
		Oliver

RE: [PATCH net 2/3] r8152: fix remote wakeup

[Hayes Wang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1123 bytes --]

Oliver Neukum [mailto:oneukum@suse.com]
[...]
> If the device does not support remote wakeup and the driver enables it, runtime
> power management will be switched off.
> That is the current state and it means that devices which don't support remote
> wakeup cannot do runtime power management at all. But the driver is correct.
> 
> The only time a device that doesn't support remote wakeup can do runtime power
> managent is when no packets can be received that is while the interface is down. If
> you want to allow that you must not set needs_remote_wakeup in probe(), but you
> must set it in open() because it is necessary for runtime power management as I
> explained above.
> 
> Sorry for the length of this mail, but I wanted to make sure I am absolutely clear
> this time.

Thanks for you explanation. I focus on the wrong point. You mean this would cause
the problem for runtime suspend. I would rethink my method and correct it.

Best Regards,
Hayes

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH net 2/3] r8152: fix remote wakeup

[Hayes Wang]

Oliver Neukum [mailto:oneukum@suse.com]
[...]
> If the device does not support remote wakeup and the driver enables it, runtime
> power management will be switched off.
> That is the current state and it means that devices which don't support remote
> wakeup cannot do runtime power management at all. But the driver is correct.
> 
> The only time a device that doesn't support remote wakeup can do runtime power
> managent is when no packets can be received that is while the interface is down. If
> you want to allow that you must not set needs_remote_wakeup in probe(), but you
> must set it in open() because it is necessary for runtime power management as I
> explained above.
> 
> Sorry for the length of this mail, but I wanted to make sure I am absolutely clear
> this time.

Thanks for you explanation. I focus on the wrong point. You mean this would cause
the problem for runtime suspend. I would rethink my method and correct it.

Best Regards,
Hayes

[PATCH net 3/3] r8152: don't enable napi before rx ready

[Hayes Wang]

Adjust napi_disable() and napi_enable() to avoid r8152_poll() start
working before rx ready. Otherwise, it may have race condition for
rx_agg.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index eff1f25..fe2cd8a 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2075,7 +2075,6 @@ static int rtl_start_rx(struct r8152 *tp)
 {
 	int i, ret = 0;
 
-	napi_disable(&tp->napi);
 	INIT_LIST_HEAD(&tp->rx_done);
 	for (i = 0; i < RTL8152_MAX_RX; i++) {
 		INIT_LIST_HEAD(&tp->rx_info[i].list);
@@ -2083,7 +2082,6 @@ static int rtl_start_rx(struct r8152 *tp)
 		if (ret)
 			break;
 	}
-	napi_enable(&tp->napi);
 
 	if (ret && ++i < RTL8152_MAX_RX) {
 		struct list_head rx_queue;
@@ -2944,8 +2942,10 @@ static void set_carrier(struct r8152 *tp)
 		if (!netif_carrier_ok(netdev)) {
 			tp->rtl_ops.enable(tp);
 			set_bit(RTL8152_SET_RX_MODE, &tp->flags);
+			napi_disable(&tp->napi);
 			netif_carrier_on(netdev);
 			rtl_start_rx(tp);
+			napi_enable(&tp->napi);
 		}
 	} else {
 		if (netif_carrier_ok(netdev)) {
@@ -3388,9 +3388,11 @@ static int rtl8152_resume(struct usb_interface *intf)
 		if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
 			rtl_runtime_suspend_enable(tp, false);
 			clear_bit(SELECTIVE_SUSPEND, &tp->flags);
+			napi_disable(&tp->napi);
 			set_bit(WORK_ENABLE, &tp->flags);
 			if (netif_carrier_ok(tp->netdev))
 				rtl_start_rx(tp);
+			napi_enable(&tp->napi);
 		} else {
 			tp->rtl_ops.up(tp);
 			rtl8152_set_speed(tp, AUTONEG_ENABLE,
-- 
2.4.2

[PATCH net v2 0/3] r8152: issues fix

[Hayes Wang]

v2:
Replace patch #2 with "r8152: fix wakeup settings".

v1:
These patches are used to fix issues.

Hayes Wang (3):
  r8152: fix the issue about U1/U2
-  r8152: fix remote wakeup
+  r8152: fix wakeup settings
  r8152: don't enable napi before rx ready

 drivers/net/usb/r8152.c | 103 ++++++++++++++++++++++++++++--------------------
 1 file changed, 60 insertions(+), 43 deletions(-)

-- 
2.4.2

[PATCH net v2 1/3] r8152: fix the issue about U1/U2

[Hayes Wang]

- Disable U1/U2 during initialization.
- Disable lpm when linking is on, and enable it when linking is off.
- Disable U1/U2 when enabling runtime suspend.

It is possible to let hw stop working, if the U1/U2 request occurs
during some situations. The patch is used to avoid it.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 94 ++++++++++++++++++++++++++++---------------------
 1 file changed, 54 insertions(+), 40 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 7f6419e..e3a0110 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2166,6 +2166,7 @@ static int rtl8153_enable(struct r8152 *tp)
 	if (test_bit(RTL8152_UNPLUG, &tp->flags))
 		return -ENODEV;
 
+	usb_disable_lpm(tp->udev);
 	set_tx_qlen(tp);
 	rtl_set_eee_plus(tp);
 	r8153_set_rx_early_timeout(tp);
@@ -2337,11 +2338,54 @@ static void __rtl_set_wol(struct r8152 *tp, u32 wolopts)
 		device_set_wakeup_enable(&tp->udev->dev, false);
 }
 
+static void r8153_u1u2en(struct r8152 *tp, bool enable)
+{
+	u8 u1u2[8];
+
+	if (enable)
+		memset(u1u2, 0xff, sizeof(u1u2));
+	else
+		memset(u1u2, 0x00, sizeof(u1u2));
+
+	usb_ocp_write(tp, USB_TOLERANCE, BYTE_EN_SIX_BYTES, sizeof(u1u2), u1u2);
+}
+
+static void r8153_u2p3en(struct r8152 *tp, bool enable)
+{
+	u32 ocp_data;
+
+	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL);
+	if (enable && tp->version != RTL_VER_03 && tp->version != RTL_VER_04)
+		ocp_data |= U2P3_ENABLE;
+	else
+		ocp_data &= ~U2P3_ENABLE;
+	ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
+}
+
+static void r8153_power_cut_en(struct r8152 *tp, bool enable)
+{
+	u32 ocp_data;
+
+	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_POWER_CUT);
+	if (enable)
+		ocp_data |= PWR_EN | PHASE2_EN;
+	else
+		ocp_data &= ~(PWR_EN | PHASE2_EN);
+	ocp_write_word(tp, MCU_TYPE_USB, USB_POWER_CUT, ocp_data);
+
+	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
+	ocp_data &= ~PCUT_STATUS;
+	ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
+}
+
 static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
 {
 	if (enable) {
 		u32 ocp_data;
 
+		r8153_u1u2en(tp, false);
+		r8153_u2p3en(tp, false);
+
 		__rtl_set_wol(tp, WAKE_ANY);
 
 		ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);
@@ -2353,6 +2397,8 @@ static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
 		ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
 	} else {
 		__rtl_set_wol(tp, tp->saved_wolopts);
+		r8153_u2p3en(tp, true);
+		r8153_u1u2en(tp, true);
 	}
 }
 
@@ -2599,46 +2645,6 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
 	set_bit(PHY_RESET, &tp->flags);
 }
 
-static void r8153_u1u2en(struct r8152 *tp, bool enable)
-{
-	u8 u1u2[8];
-
-	if (enable)
-		memset(u1u2, 0xff, sizeof(u1u2));
-	else
-		memset(u1u2, 0x00, sizeof(u1u2));
-
-	usb_ocp_write(tp, USB_TOLERANCE, BYTE_EN_SIX_BYTES, sizeof(u1u2), u1u2);
-}
-
-static void r8153_u2p3en(struct r8152 *tp, bool enable)
-{
-	u32 ocp_data;
-
-	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL);
-	if (enable)
-		ocp_data |= U2P3_ENABLE;
-	else
-		ocp_data &= ~U2P3_ENABLE;
-	ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
-}
-
-static void r8153_power_cut_en(struct r8152 *tp, bool enable)
-{
-	u32 ocp_data;
-
-	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_POWER_CUT);
-	if (enable)
-		ocp_data |= PWR_EN | PHASE2_EN;
-	else
-		ocp_data &= ~(PWR_EN | PHASE2_EN);
-	ocp_write_word(tp, MCU_TYPE_USB, USB_POWER_CUT, ocp_data);
-
-	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
-	ocp_data &= ~PCUT_STATUS;
-	ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
-}
-
 static void r8153_first_init(struct r8152 *tp)
 {
 	u32 ocp_data;
@@ -2781,6 +2787,7 @@ static void rtl8153_disable(struct r8152 *tp)
 	r8153_disable_aldps(tp);
 	rtl_disable(tp);
 	r8153_enable_aldps(tp);
+	usb_enable_lpm(tp->udev);
 }
 
 static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u16 speed, u8 duplex)
@@ -2901,9 +2908,13 @@ static void rtl8153_up(struct r8152 *tp)
 	if (test_bit(RTL8152_UNPLUG, &tp->flags))
 		return;
 
+	r8153_u1u2en(tp, false);
 	r8153_disable_aldps(tp);
 	r8153_first_init(tp);
 	r8153_enable_aldps(tp);
+	r8153_u2p3en(tp, true);
+	r8153_u1u2en(tp, true);
+	usb_enable_lpm(tp->udev);
 }
 
 static void rtl8153_down(struct r8152 *tp)
@@ -2914,6 +2925,7 @@ static void rtl8153_down(struct r8152 *tp)
 	}
 
 	r8153_u1u2en(tp, false);
+	r8153_u2p3en(tp, false);
 	r8153_power_cut_en(tp, false);
 	r8153_disable_aldps(tp);
 	r8153_enter_oob(tp);
@@ -3252,6 +3264,7 @@ static void r8153_init(struct r8152 *tp)
 		msleep(20);
 	}
 
+	usb_disable_lpm(tp->udev);
 	r8153_u2p3en(tp, false);
 
 	if (tp->version == RTL_VER_04) {
@@ -3319,6 +3332,7 @@ static void r8153_init(struct r8152 *tp)
 	r8153_enable_aldps(tp);
 	r8152b_enable_fc(tp);
 	rtl_tally_reset(tp);
+	r8153_u2p3en(tp, true);
 }
 
 static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
-- 
2.4.2

[PATCH net v2 2/3] r8152: fix wakeup settings

[Hayes Wang]

Avoid the driver to enable WOL if the device doesn't support it.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e3a0110..d537c30 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2378,6 +2378,13 @@ static void r8153_power_cut_en(struct r8152 *tp, bool enable)
 	ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
 }
 
+static bool rtl_can_wakeup(struct r8152 *tp)
+{
+	struct usb_device *udev = tp->udev;
+
+	return (udev->actconfig->desc.bmAttributes & USB_CONFIG_ATT_WAKEUP);
+}
+
 static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
 {
 	if (enable) {
@@ -3417,12 +3424,15 @@ static void rtl8152_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 	if (usb_autopm_get_interface(tp->intf) < 0)
 		return;
 
-	mutex_lock(&tp->control);
-
-	wol->supported = WAKE_ANY;
-	wol->wolopts = __rtl_get_wol(tp);
-
-	mutex_unlock(&tp->control);
+	if (!rtl_can_wakeup(tp)) {
+		wol->supported = 0;
+		wol->wolopts = 0;
+	} else {
+		mutex_lock(&tp->control);
+		wol->supported = WAKE_ANY;
+		wol->wolopts = __rtl_get_wol(tp);
+		mutex_unlock(&tp->control);
+	}
 
 	usb_autopm_put_interface(tp->intf);
 }
@@ -3432,6 +3442,9 @@ static int rtl8152_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 	struct r8152 *tp = netdev_priv(dev);
 	int ret;
 
+	if (!rtl_can_wakeup(tp))
+		return -EOPNOTSUPP;
+
 	ret = usb_autopm_get_interface(tp->intf);
 	if (ret < 0)
 		goto out_set_wol;
@@ -4073,6 +4086,9 @@ static int rtl8152_probe(struct usb_interface *intf,
 		goto out1;
 	}
 
+	if (!rtl_can_wakeup(tp))
+		__rtl_set_wol(tp, 0);
+
 	tp->saved_wolopts = __rtl_get_wol(tp);
 	if (tp->saved_wolopts)
 		device_set_wakeup_enable(&udev->dev, true);
-- 
2.4.2

[PATCH net v2 3/3] r8152: don't enable napi before rx ready

[Hayes Wang]

Adjust napi_disable() and napi_enable() to avoid r8152_poll() start
working before rx ready. Otherwise, it may have race condition for
rx_agg.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index d537c30..144dc64 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2075,7 +2075,6 @@ static int rtl_start_rx(struct r8152 *tp)
 {
 	int i, ret = 0;
 
-	napi_disable(&tp->napi);
 	INIT_LIST_HEAD(&tp->rx_done);
 	for (i = 0; i < RTL8152_MAX_RX; i++) {
 		INIT_LIST_HEAD(&tp->rx_info[i].list);
@@ -2083,7 +2082,6 @@ static int rtl_start_rx(struct r8152 *tp)
 		if (ret)
 			break;
 	}
-	napi_enable(&tp->napi);
 
 	if (ret && ++i < RTL8152_MAX_RX) {
 		struct list_head rx_queue;
@@ -2951,8 +2949,10 @@ static void set_carrier(struct r8152 *tp)
 		if (!netif_carrier_ok(netdev)) {
 			tp->rtl_ops.enable(tp);
 			set_bit(RTL8152_SET_RX_MODE, &tp->flags);
+			napi_disable(&tp->napi);
 			netif_carrier_on(netdev);
 			rtl_start_rx(tp);
+			napi_enable(&tp->napi);
 		}
 	} else {
 		if (netif_carrier_ok(netdev)) {
@@ -3395,9 +3395,11 @@ static int rtl8152_resume(struct usb_interface *intf)
 		if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
 			rtl_runtime_suspend_enable(tp, false);
 			clear_bit(SELECTIVE_SUSPEND, &tp->flags);
+			napi_disable(&tp->napi);
 			set_bit(WORK_ENABLE, &tp->flags);
 			if (netif_carrier_ok(tp->netdev))
 				rtl_start_rx(tp);
+			napi_enable(&tp->napi);
 		} else {
 			tp->rtl_ops.up(tp);
 			rtl8152_set_speed(tp, AUTONEG_ENABLE,
-- 
2.4.2

Re: [PATCH net v2 0/3] r8152: issues fix

[David Miller]

From: Hayes Wang <hayeswang@realtek.com>
Date: Fri, 24 Jul 2015 13:54:22 +0800

> v2:
> Replace patch #2 with "r8152: fix wakeup settings".
> 
> v1:
> These patches are used to fix issues.

Series applied, thank you.