RE: Hangs in r8152 connected to power management in kernels at least up v4.17-rc4

[Hayes Wang]

Oliver Neukum [mailto:oneu...@suse.com]
> Sent: Wednesday, May 16, 2018 6:10 PM
[...]
> > Besides, I find a similar issue as following.
> > https://www.spinics.net/lists/netdev/msg493512.html
> 
> Well, if we have an imbalance in NAPI it should strike whereever
> it is used, not just in suspend(). Is there debugging for NAPI
> we could activate?

No. The driver doesn't embed such debugging about it.
And I don't find an imbalance between napi_disable() and napi_enable().


Best Regards,
Hayes

RE: Hangs in r8152 connected to power management in kernels at least up v4.17-rc4

[Hayes Wang]

Oliver Neukum [mailto:oneu...@suse.com]
> Sent: Wednesday, May 16, 2018 4:27 PM
[...]
> >
> > Would usb_autopm_get_interface() take a long time?
> > The driver would wake the device if it has suspended.
> > I have no idea about how usb_autopm_get_interface() works, so I don't know
> how to help.
> 
> Hi,
> 
> it basically calls r8152_resume() and makes a control request to the
> hub. I think we are spinning in rtl8152_runtime_resume(), but where?
> It has a lot of NAPI stuff. Any suggestions on how to instrument or
> trace this?

Is rtl8152_runtime_resume() called? I don't see the name in the trace.

I guess the relative API in rtl8152_runtime_resume() are
ops->disable= rtl8153_disable;
ops->autosuspend_en = rtl8153_runtime_enable;

And I don't find any possible dead lock in rtl8152_runtime_resume().

Besides, I find a similar issue as following.
https://www.spinics.net/lists/netdev/msg493512.html


Best Regards,
Hayes

RE: Hangs in r8152 connected to power management in kernels at least up v4.17-rc4

[Hayes Wang]

Oliver Neukum [mailto:oneu...@suse.com]
> 
> Hi,
> 
> I got reports about hangs with this trace:
> 
> May 13 01:36:55 neroon kernel: INFO: task kworker/0:0:4 blocked for more
> than 60 seconds.
> May 13 01:36:55 neroon kernel:   Tainted: G U
> 4.17.0-rc4-1.g8257a00-vanilla #1
> May 13 01:36:55 neroon kernel: "echo 0 >
> /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> May 13 01:36:55 neroon kernel: kworker/0:0 D0 4  2
> 0x8000
> May 13 01:36:55 neroon kernel: Workqueue: events rtl_work_func_t [r8152]
> May 13 01:36:55 neroon kernel: Call Trace:
> May 13 01:36:55 neroon kernel:  ? __schedule+0x289/0x880
> May 13 01:36:55 neroon kernel:  schedule+0x2f/0x90
> May 13 01:36:55 neroon kernel:  rpm_resume+0xf9/0x7a0
> May 13 01:36:55 neroon kernel:  ? wait_woken+0x80/0x80
> May 13 01:36:55 neroon kernel:  rpm_resume+0x547/0x7a0
> May 13 01:36:55 neroon kernel:  ? __switch_to_asm+0x40/0x70
> May 13 01:36:55 neroon kernel:  ? __switch_to_asm+0x34/0x70
> May 13 01:36:55 neroon kernel:  ? __switch_to_asm+0x40/0x70
> May 13 01:36:55 neroon kernel:  ? __switch_to_asm+0x34/0x70
> May 13 01:36:55 neroon kernel:  ? __switch_to_asm+0x40/0x70
> May 13 01:36:55 neroon kernel:  __pm_runtime_resume+0x3a/0x50
> May 13 01:36:55 neroon kernel:  usb_autopm_get_interface+0x1d/0x50 [usbcore]

Would usb_autopm_get_interface() take a long time?
The driver would wake the device if it has suspended.
I have no idea about how usb_autopm_get_interface() works, so I don't know how 
to help.

> May 13 01:36:55 neroon kernel:  rtl_work_func_t+0x3c/0x27c [r8152]
> May 13 01:36:55 neroon kernel:  process_one_work+0x1d4/0x3f0
> May 13 01:36:55 neroon kernel:  worker_thread+0x2b/0x3d0
> May 13 01:36:55 neroon kernel:  ? process_one_work+0x3f0/0x3f0
> May 13 01:36:55 neroon kernel:  kthread+0x113/0x130
> May 13 01:36:55 neroon kernel:  ?
> kthread_create_worker_on_cpu+0x50/0x50
> May 13 01:36:55 neroon kernel:  ret_from_fork+0x3a/0x50

Best Regards,
Hayes

[PATCH net 0/2] r8152: fix rx issues

[Hayes Wang]

The two patched are used to fix rx issues.

Hayes Wang (2):
  r8152: fix wrong checksum status for received IPv4 packets
  r8152: set rx mode early when linking on

 drivers/net/usb/r8152.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

-- 
2.13.6

[PATCH net 2/2] r8152: set rx mode early when linking on

[Hayes Wang]

Set rx mode before calling netif_wake_queue() when linking on to avoid
the device missing the receiving packets.

The transmission may start after calling netif_wake_queue(), and the
packets of resopnse may reach before calling rtl8152_set_rx_mode()
which let the device could receive packets. Then, the packets of
response would be missed.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 756de9ea8d2e..958b2e8b90f6 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3795,11 +3795,12 @@ static void set_carrier(struct r8152 *tp)
if (speed & LINK_STATUS) {
if (!netif_carrier_ok(netdev)) {
tp->rtl_ops.enable(tp);
-   set_bit(RTL8152_SET_RX_MODE, >flags);
netif_stop_queue(netdev);
napi_disable(napi);
netif_carrier_on(netdev);
rtl_start_rx(tp);
+   clear_bit(RTL8152_SET_RX_MODE, >flags);
+   _rtl8152_set_rx_mode(netdev);
napi_enable(>napi);
netif_wake_queue(netdev);
netif_info(tp, link, netdev, "carrier on\n");
@@ -4259,7 +4260,7 @@ static int rtl8152_post_reset(struct usb_interface *intf)
mutex_lock(>control);
tp->rtl_ops.enable(tp);
rtl_start_rx(tp);
-   rtl8152_set_rx_mode(netdev);
+   _rtl8152_set_rx_mode(netdev);
mutex_unlock(>control);
}
 
-- 
2.13.6

[PATCH net 1/2] r8152: fix wrong checksum status for received IPv4 packets

[Hayes Wang]

The device could only check the checksum of TCP and UDP packets. Therefore,
for the IPv4 packets excluding TCP and UDP, the check of checksum is necessary,
even though the IP checksum is correct.

Take ICMP for example, The IP checksum may be correct, but the ICMP checksum
may be wrong.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0657203ffb91..756de9ea8d2e 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1848,11 +1848,9 @@ static u8 r8152_rx_csum(struct r8152 *tp, struct rx_desc 
*rx_desc)
if (opts2 & RD_IPV4_CS) {
if (opts3 & IPF)
checksum = CHECKSUM_NONE;
-   else if ((opts2 & RD_UDP_CS) && (opts3 & UDPF))
-   checksum = CHECKSUM_NONE;
-   else if ((opts2 & RD_TCP_CS) && (opts3 & TCPF))
-   checksum = CHECKSUM_NONE;
-   else
+   else if ((opts2 & RD_UDP_CS) && !(opts3 & UDPF))
+   checksum = CHECKSUM_UNNECESSARY;
+   else if ((opts2 & RD_TCP_CS) && !(opts3 & TCPF))
checksum = CHECKSUM_UNNECESSARY;
} else if (opts2 & RD_IPV6_CS) {
if ((opts2 & RD_UDP_CS) && !(opts3 & UDPF))
-- 
2.13.6

RE: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[Hayes Wang]

[...]
> > r8153 on Dell TB15/16 dock corrupts rx packets.
> >
> > This change is suggested by Realtek. They guess that the XHCI
> > controller doesn't have enough buffer, and their guesswork is correct,
> > once the RX aggregation gets disabled, the issue is gone.
> >
> > ASMedia is currently working on a real sulotion for this issue.
> >
> > Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
> >
> > Note that TB15 has different bcdDevice and iSerialNumber, which are
> > not unique values. If you still have TB15, please contact Dell to
> > replace it with TB16.

Excuse me. I don't understand why this patch is for specific USB nic rather 
than xHCI.
It seems to make the specific USB nic working and the other ones keeping error.


Best Regards,
Hayes

RE: [PATCH net-next 3/3] r8169: improve runtime pm in general and suspend unused ports

[Hayes Wang]

> Heiner Kallweit [mailto:hkallwe...@gmail.com]
> > Sent: Tuesday, January 09, 2018 4:39 AM
> [...]
> > - Change rtl8169_runtime_suspend to power down the chip if the
> >   interface is down.
> 
> The original driver has done it. I don't think you have to do it again in
> rtl8169_runtime_suspend().

It's my mistake. If ndo_stop() doesn't be called after probe(),
rtl_pll_power_down() wouldn't be called, either.
You are right.

Best Regards,
Hayes

RE: [PATCH net-next 3/3] r8169: improve runtime pm in general and suspend unused ports

[Hayes Wang]

Heiner Kallweit [mailto:hkallwe...@gmail.com]
> Sent: Tuesday, January 09, 2018 4:39 AM
[...]
> - Change rtl8169_runtime_suspend to power down the chip if the
>   interface is down.

The original driver has done it. I don't think you have to do it again in 
rtl8169_runtime_suspend().

[...]
> - if (!tp->TxDescArray)
> + if (!tp->TxDescArray) {
> + rtl_pll_power_down(tp);
>   return 0;
> + }

It is unnecessary.

Best Regards,
Hayes

[PATCH net-next] r8152: correct the definition

[Hayes Wang]

Replace VLAN_HLEN and CRC_SIZE with ETH_FCS_LEN.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 8bc4573..6cfffef 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -569,7 +569,6 @@ enum rtl_register_content {
 #define RTL8152_MAX_TX 4
 #define RTL8152_MAX_RX 10
 #define INTBUFSIZE 2
-#define CRC_SIZE   4
 #define TX_ALIGN   4
 #define RX_ALIGN   8
 
@@ -588,12 +587,13 @@ enum rtl_register_content {
 #define BYTE_EN_END_MASK   0xf0
 
 #define RTL8153_MAX_PACKET 9216 /* 9K */
-#define RTL8153_MAX_MTU(RTL8153_MAX_PACKET - VLAN_ETH_HLEN - 
VLAN_HLEN)
-#define RTL8152_RMS(VLAN_ETH_FRAME_LEN + VLAN_HLEN)
+#define RTL8153_MAX_MTU(RTL8153_MAX_PACKET - VLAN_ETH_HLEN - \
+ETH_FCS_LEN)
+#define RTL8152_RMS(VLAN_ETH_FRAME_LEN + ETH_FCS_LEN)
 #define RTL8153_RMSRTL8153_MAX_PACKET
 #define RTL8152_TX_TIMEOUT (5 * HZ)
 #define RTL8152_NAPI_WEIGHT64
-#define rx_reserved_size(x)((x) + VLAN_ETH_HLEN + CRC_SIZE + \
+#define rx_reserved_size(x)((x) + VLAN_ETH_HLEN + ETH_FCS_LEN + \
 sizeof(struct rx_desc) + RX_ALIGN)
 
 /* rtl8152 flags */
@@ -770,7 +770,7 @@ static const int multicast_filter_limit = 32;
 static unsigned int agg_buf_sz = 16384;
 
 #define RTL_LIMITED_TSO_SIZE   (agg_buf_sz - sizeof(struct tx_desc) - \
-VLAN_ETH_HLEN - VLAN_HLEN)
+VLAN_ETH_HLEN - ETH_FCS_LEN)
 
 static
 int get_registers(struct r8152 *tp, u16 value, u16 index, u16 size, void *data)
@@ -1928,7 +1928,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
if (urb->actual_length < len_used)
break;
 
-   pkt_len -= CRC_SIZE;
+   pkt_len -= ETH_FCS_LEN;
rx_data += sizeof(struct rx_desc);
 
skb = napi_alloc_skb(napi, pkt_len);
@@ -1952,7 +1952,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
}
 
 find_next_rx:
-   rx_data = rx_agg_align(rx_data + pkt_len + CRC_SIZE);
+   rx_data = rx_agg_align(rx_data + pkt_len + ETH_FCS_LEN);
rx_desc = (struct rx_desc *)rx_data;
len_used = (int)(rx_data - (u8 *)agg->head);
len_used += sizeof(struct rx_desc);
@@ -2242,7 +2242,7 @@ static void set_tx_qlen(struct r8152 *tp)
 {
struct net_device *netdev = tp->netdev;
 
-   tp->tx_qlen = agg_buf_sz / (netdev->mtu + VLAN_ETH_HLEN + VLAN_HLEN +
+   tp->tx_qlen = agg_buf_sz / (netdev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN +
sizeof(struct tx_desc));
 }
 
@@ -3439,7 +3439,7 @@ static void r8153_first_init(struct r8152 *tp)
 
rtl_rx_vlan_en(tp, tp->netdev->features & NETIF_F_HW_VLAN_CTAG_RX);
 
-   ocp_data = tp->netdev->mtu + VLAN_ETH_HLEN + CRC_SIZE;
+   ocp_data = tp->netdev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, ocp_data);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, MTPS_JUMBO);
 
@@ -3489,7 +3489,7 @@ static void r8153_enter_oob(struct r8152 *tp)
usleep_range(1000, 2000);
}
 
-   ocp_data = tp->netdev->mtu + VLAN_ETH_HLEN + CRC_SIZE;
+   ocp_data = tp->netdev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, ocp_data);
 
switch (tp->version) {
@@ -4957,7 +4957,7 @@ static int rtl8152_change_mtu(struct net_device *dev, int 
new_mtu)
dev->mtu = new_mtu;
 
if (netif_running(dev)) {
-   u32 rms = new_mtu + VLAN_ETH_HLEN + CRC_SIZE;
+   u32 rms = new_mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
 
ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, rms);
 
-- 
2.7.4

RE: [PATCH net-next v2 0/2] r8152: adjust runtime suspend/resume

[Hayes Wang]

David Miller [mailto:da...@davemloft.net]
> Sent: Wednesday, June 14, 2017 1:02 AM
> > v2:
> > For #1, replace GFP_KERNEL with GFP_NOIO for usb_submit_urb().
> >
> > v1:
> > Improve the flow about runtime suspend/resume and make the code
> > easy to read.
> 
> Series applied.

Excuse me. I don't see these patches in net-next repository. Where could I find 
them?

Best Regards,
Hayes

[PATCH net-next 0/3] r8152: support new chips

[Hayes Wang]

These patches are used to support new chips.

Hayes Wang (3):
  r8152: support new chip 8050
  r8152: support RTL8153B
  r8152: add byte_enable for ocp_read_word function

 drivers/net/usb/r8152.c | 687 ++--
 1 file changed, 671 insertions(+), 16 deletions(-)

-- 
2.7.4

[PATCH net-next 1/3] r8152: support new chip 8050

[Hayes Wang]

The settings of the new chip are the same with RTL8152, except that
its product ID is 0x8050.

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 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 5a02053..2744405 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -659,6 +659,7 @@ enum rtl_version {
RTL_VER_04,
RTL_VER_05,
RTL_VER_06,
+   RTL_VER_07,
RTL_VER_MAX
 };
 
@@ -4183,6 +4184,7 @@ static int rtl8152_get_coalesce(struct net_device *netdev,
switch (tp->version) {
case RTL_VER_01:
case RTL_VER_02:
+   case RTL_VER_07:
return -EOPNOTSUPP;
default:
break;
@@ -4202,6 +4204,7 @@ static int rtl8152_set_coalesce(struct net_device *netdev,
switch (tp->version) {
case RTL_VER_01:
case RTL_VER_02:
+   case RTL_VER_07:
return -EOPNOTSUPP;
default:
break;
@@ -4301,6 +4304,7 @@ static int rtl8152_change_mtu(struct net_device *dev, int 
new_mtu)
switch (tp->version) {
case RTL_VER_01:
case RTL_VER_02:
+   case RTL_VER_07:
dev->mtu = new_mtu;
return 0;
default:
@@ -4370,6 +4374,7 @@ static int rtl_ops_init(struct r8152 *tp)
switch (tp->version) {
case RTL_VER_01:
case RTL_VER_02:
+   case RTL_VER_07:
ops->init   = r8152b_init;
ops->enable = rtl8152_enable;
ops->disable= rtl8152_disable;
@@ -4448,6 +4453,9 @@ static u8 rtl_get_version(struct usb_interface *intf)
case 0x5c30:
version = RTL_VER_06;
break;
+   case 0x4800:
+   version = RTL_VER_07;
+   break;
default:
version = RTL_VER_UNKNOWN;
dev_info(>dev, "Unknown version 0x%04x\n", ocp_data);
@@ -4493,6 +4501,7 @@ static int rtl8152_probe(struct usb_interface *intf,
switch (version) {
case RTL_VER_01:
case RTL_VER_02:
+   case RTL_VER_07:
tp->mii.supports_gmii = 0;
break;
default:
@@ -4627,6 +4636,7 @@ static void rtl8152_disconnect(struct usb_interface *intf)
 
 /* table of devices that work with this driver */
 static struct usb_device_id rtl8152_table[] = {
+   {REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8050)},
{REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8152)},
{REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8153)},
{REALTEK_USB_DEVICE(VENDOR_ID_MICROSOFT, 0x07ab)},
-- 
2.7.4

[PATCH net-next 3/3] r8152: add byte_enable for ocp_read_word function

[Hayes Wang]

Add byte_enable for ocp_read_word() to replace reading 4
bytes data with reading the desired 2 bytes data.

This is used to avoid the issue which is described in
commit b4d99def0938 ("r8152: remove sram_read"). The
original method always reads 4 bytes data, and it may
have problem when reading the PHY registers.

The new method is supported since RTL8153B, but it
doesn't influence the previous chips. The bits of the
byte_enable for the previous chips are the reserved
bits, and the hw would ignore them.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 4c197da..184b680 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -962,11 +962,13 @@ static u16 ocp_read_word(struct r8152 *tp, u16 type, u16 
index)
 {
u32 data;
__le32 tmp;
+   u16 byen = BYTE_EN_WORD;
u8 shift = index & 2;
 
index &= ~3;
+   byen <<= shift;
 
-   generic_ocp_read(tp, index, sizeof(tmp), , type);
+   generic_ocp_read(tp, index, sizeof(tmp), , type | byen);
 
data = __le32_to_cpu(tmp);
data >>= (shift * 8);
-- 
2.7.4

[PATCH net-next 2/3] r8152: support RTL8153B

[Hayes Wang]

This patch supports two new chips for RTL8153B.

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 673 ++--
 1 file changed, 658 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2744405..4c197da 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -29,7 +29,7 @@
 #include 
 
 /* Information for net-next */
-#define NETNEXT_VERSION"08"
+#define NETNEXT_VERSION"09"
 
 /* Information for net */
 #define NET_VERSION"9"
@@ -51,11 +51,14 @@
 #define PLA_FMC0xc0b4
 #define PLA_CFG_WOL0xc0b6
 #define PLA_TEREDO_CFG 0xc0bc
+#define PLA_TEREDO_WAKE_BASE   0xc0c4
 #define PLA_MAR0xcd00
 #define PLA_BACKUP 0xd000
 #define PAL_BDC_CR 0xd1a0
 #define PLA_TEREDO_TIMER   0xd2cc
 #define PLA_REALWOW_TIMER  0xd2e8
+#define PLA_EFUSE_DATA 0xdd00
+#define PLA_EFUSE_CMD  0xdd02
 #define PLA_LEDSEL 0xdd90
 #define PLA_LED_FEATURE0xdd92
 #define PLA_PHYAR  0xde00
@@ -105,7 +108,9 @@
 #define USB_CSR_DUMMY2 0xb466
 #define USB_DEV_STAT   0xb808
 #define USB_CONNECT_TIMER  0xcbf8
+#define USB_MSC_TIMER  0xcbfc
 #define USB_BURST_SIZE 0xcfc0
+#define USB_LPM_CONFIG 0xcfd8
 #define USB_USB_CTRL   0xd406
 #define USB_PHY_CTRL   0xd408
 #define USB_TX_AGG 0xd40a
@@ -113,15 +118,20 @@
 #define USB_USB_TIMER  0xd428
 #define USB_RX_EARLY_TIMEOUT   0xd42c
 #define USB_RX_EARLY_SIZE  0xd42e
-#define USB_PM_CTRL_STATUS 0xd432
+#define USB_PM_CTRL_STATUS 0xd432  /* RTL8153A */
+#define USB_RX_EXTRA_AGGR_TMR  0xd432  /* RTL8153B */
 #define USB_TX_DMA 0xd434
+#define USB_UPT_RXDMA_OWN  0xd437
 #define USB_TOLERANCE  0xd490
 #define USB_LPM_CTRL   0xd41a
 #define USB_BMU_RESET  0xd4b0
+#define USB_U1U2_TIMER 0xd4da
 #define USB_UPS_CTRL   0xd800
-#define USB_MISC_0 0xd81a
 #define USB_POWER_CUT  0xd80a
+#define USB_MISC_0 0xd81a
 #define USB_AFE_CTRL2  0xd824
+#define USB_UPS_CFG0xd842
+#define USB_UPS_FLAGS  0xd848
 #define USB_WDT11_CTRL 0xe43c
 #define USB_BP_BA  0xfc26
 #define USB_BP_0   0xfc28
@@ -133,6 +143,15 @@
 #define USB_BP_6   0xfc34
 #define USB_BP_7   0xfc36
 #define USB_BP_EN  0xfc38
+#define USB_BP_8   0xfc38
+#define USB_BP_9   0xfc3a
+#define USB_BP_10  0xfc3c
+#define USB_BP_11  0xfc3e
+#define USB_BP_12  0xfc40
+#define USB_BP_13  0xfc42
+#define USB_BP_14  0xfc44
+#define USB_BP_15  0xfc46
+#define USB_BP2_EN 0xfc48
 
 /* OCP Registers */
 #define OCP_ALDPS_CONFIG   0x2010
@@ -143,6 +162,7 @@
 #define OCP_EEE_AR 0xa41a
 #define OCP_EEE_DATA   0xa41c
 #define OCP_PHY_STATUS 0xa420
+#define OCP_NCTL_CFG   0xa42c
 #define OCP_POWER_CFG  0xa430
 #define OCP_EEE_CFG0xa432
 #define OCP_SRAM_ADDR  0xa436
@@ -152,9 +172,14 @@
 #define OCP_EEE_ADV0xa5d0
 #define OCP_EEE_LPABLE 0xa5d2
 #define OCP_PHY_STATE  0xa708  /* nway state for 8153 */
+#define OCP_PHY_PATCH_STAT 0xb800
+#define OCP_PHY_PATCH_CMD  0xb820
+#define OCP_ADC_IOFFSET0xbcfc
 #define OCP_ADC_CFG0xbc06
+#define OCP_SYSCLK_CFG 0xc416
 
 /* SRAM Register */
+#define SRAM_GREEN_CFG 0x8011
 #define SRAM_LPF_CFG   0x8012
 #define SRAM_10M_AMP1  0x8080
 #define SRAM_10M_AMP2  0x8082
@@ -252,6 +277,10 @@
 /* PAL_BDC_CR */
 #define ALDPS_PROXY_MODE   0x0001
 
+/* PLA_EFUSE_CMD */
+#define EFUSE_READ_CMD BIT(15)
+#define EFUSE_DATA_BIT16   BIT(7)
+
 /* PLA_CONFIG34 */
 #define LINK_ON_WAKE_EN0x0010
 #define LINK_OFF_WAKE_EN   0x0008
@@ -277,6 +306,7 @@
 
 /* PLA_MAC_PWR_CTRL2 */
 #define EEE_SPDWN_RATIO0x8007
+#define MAC_CLK_SPDWN_EN   BIT(15)
 
 /* PLA_MAC_PWR_CTRL3 */
 #define PKT_AVAIL_SPDWN_EN 0x0100
@@ -328,6 +358,9 @@
 #define STAT_SPEED_HIGH0x
 #define STAT_SPEED_FULL0x0002
 
+/* USB_LPM_CONFIG */
+#define LPM_U1U2_ENBIT(0)
+
 /* USB_TX_AGG */
 #define TX_AGG_MAX_THRESHOLD   0x03
 
@@ -335,6 +368,7 @@
 #define RX_THR_SUPPER  0x0c350180
 #define RX_THR_HIGH0x7a120180
 #define RX_THR_SLOW0x0180
+#define RX_THR_B   0x00010001
 
 /* USB_TX_DMA */
 #define TEST_MODE_DISABLE  0x0001
@@ -344,6 +378,10 @@
 #define BMU_RESET_EP_IN0x01
 #define BMU_RESET_EP_OUT   0x02

[PATCH net-next v2 0/2] r8152: adjust runtime suspend/resume

[Hayes Wang]

v2:
For #1, replace GFP_KERNEL with GFP_NOIO for usb_submit_urb().

v1:
Improve the flow about runtime suspend/resume and make the code
easy to read.

Hayes Wang (2):
  r8152: split rtl8152_resume function
  r8152: move calling delay_autosuspend function

 drivers/net/usb/r8152.c | 107 
 1 file changed, 62 insertions(+), 45 deletions(-)

-- 
2.7.4

[PATCH net-next v2 1/2] r8152: split rtl8152_resume function

[Hayes Wang]

Split rtl8152_resume() into rtl8152_runtime_resume() and
rtl8152_system_resume().

Besides, replace GFP_KERNEL with GFP_NOIO for usb_submit_urb().

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 99 ++---
 1 file changed, 61 insertions(+), 38 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 5a02053..2d238b5 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3686,6 +3686,61 @@ static bool delay_autosuspend(struct r8152 *tp)
return false;
 }
 
+static int rtl8152_runtime_resume(struct r8152 *tp)
+{
+   struct net_device *netdev = tp->netdev;
+
+   if (netif_running(netdev) && netdev->flags & IFF_UP) {
+   struct napi_struct *napi = >napi;
+
+   tp->rtl_ops.autosuspend_en(tp, false);
+   napi_disable(napi);
+   set_bit(WORK_ENABLE, >flags);
+
+   if (netif_carrier_ok(netdev)) {
+   if (rtl8152_get_speed(tp) & LINK_STATUS) {
+   rtl_start_rx(tp);
+   } else {
+   netif_carrier_off(netdev);
+   tp->rtl_ops.disable(tp);
+   netif_info(tp, link, netdev, "linking down\n");
+   }
+   }
+
+   napi_enable(napi);
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
+
+   if (!list_empty(>rx_done))
+   napi_schedule(>napi);
+
+   usb_submit_urb(tp->intr_urb, GFP_NOIO);
+   } else {
+   if (netdev->flags & IFF_UP)
+   tp->rtl_ops.autosuspend_en(tp, false);
+
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   }
+
+   return 0;
+}
+
+static int rtl8152_system_resume(struct r8152 *tp)
+{
+   struct net_device *netdev = tp->netdev;
+
+   netif_device_attach(netdev);
+
+   if (netif_running(netdev) && netdev->flags & IFF_UP) {
+   tp->rtl_ops.up(tp);
+   netif_carrier_off(netdev);
+   set_bit(WORK_ENABLE, >flags);
+   usb_submit_urb(tp->intr_urb, GFP_NOIO);
+   }
+
+   return 0;
+}
+
 static int rtl8152_runtime_suspend(struct r8152 *tp)
 {
struct net_device *netdev = tp->netdev;
@@ -3784,50 +3839,18 @@ static int rtl8152_suspend(struct usb_interface *intf, 
pm_message_t message)
 static int rtl8152_resume(struct usb_interface *intf)
 {
struct r8152 *tp = usb_get_intfdata(intf);
-   struct net_device *netdev = tp->netdev;
+   int ret;
 
mutex_lock(>control);
 
-   if (!test_bit(SELECTIVE_SUSPEND, >flags))
-   netif_device_attach(netdev);
-
-   if (netif_running(netdev) && netdev->flags & IFF_UP) {
-   if (test_bit(SELECTIVE_SUSPEND, >flags)) {
-   struct napi_struct *napi = >napi;
-
-   tp->rtl_ops.autosuspend_en(tp, false);
-   napi_disable(napi);
-   set_bit(WORK_ENABLE, >flags);
-   if (netif_carrier_ok(netdev)) {
-   if (rtl8152_get_speed(tp) & LINK_STATUS) {
-   rtl_start_rx(tp);
-   } else {
-   netif_carrier_off(netdev);
-   tp->rtl_ops.disable(tp);
-   netif_info(tp, link, netdev,
-  "linking down\n");
-   }
-   }
-   napi_enable(napi);
-   clear_bit(SELECTIVE_SUSPEND, >flags);
-   smp_mb__after_atomic();
-   if (!list_empty(>rx_done))
-   napi_schedule(>napi);
-   } else {
-   tp->rtl_ops.up(tp);
-   netif_carrier_off(netdev);
-   set_bit(WORK_ENABLE, >flags);
-   }
-   usb_submit_urb(tp->intr_urb, GFP_KERNEL);
-   } else if (test_bit(SELECTIVE_SUSPEND, >flags)) {
-   if (netdev->flags & IFF_UP)
-   tp->rtl_ops.autosuspend_en(tp, false);
-   clear_bit(SELECTIVE_SUSPEND, >flags);
-   }
+   if (test_bit(SELECTIVE_SUSPEND, >flags))
+   ret = rtl8152_runtime_resume(tp);
+   else
+   ret = rtl8152_system_resume(tp);
 
mutex_unlock(>control);
 
-   return 0;
+   return ret;
 }
 
 static int rtl8152_reset_resume(struct usb_interface *intf)
-- 
2.7.4

[PATCH net-next v2 2/2] r8152: move calling delay_autosuspend function

[Hayes Wang]

Move calling delay_autosuspend() in rtl8152_runtime_suspend(). Calling
delay_autosuspend() as late as possible.

The original flows are
   1. check if the driver/device is busy now.
   2. set wake events.
   3. enter runtime suspend.

If the wake event occurs between (1) and (2), the device may miss it. Besides,
to avoid the runtime resume occurs after runtime suspend immediately, move the
checking to the end of rtl8152_runtime_suspend().

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2d238b5..b916418 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3752,13 +3752,6 @@ static int rtl8152_runtime_suspend(struct r8152 *tp)
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
u32 rcr = 0;
 
-   if (delay_autosuspend(tp)) {
-   clear_bit(SELECTIVE_SUSPEND, >flags);
-   smp_mb__after_atomic();
-   ret = -EBUSY;
-   goto out1;
-   }
-
if (netif_carrier_ok(netdev)) {
u32 ocp_data;
 
@@ -3792,6 +3785,11 @@ static int rtl8152_runtime_suspend(struct r8152 *tp)
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
napi_enable(napi);
}
+
+   if (delay_autosuspend(tp)) {
+   rtl8152_runtime_resume(tp);
+   ret = -EBUSY;
+   }
}
 
 out1:
-- 
2.7.4

RE: [PATCH net-next 1/2] r8152: split rtl8152_resume function

[Hayes Wang]

Oliver Neukum [mailto:oneu...@suse.com]
> Sent: Monday, June 12, 2017 8:33 PM
[...]
> > +   usb_submit_urb(tp->intr_urb, GFP_KERNEL);
> 
> If you ever built a device with included storage, this can deadlock,
> as you may want to wake up a device for memory that is needed to wake
> up a device. Use GFP_NOIO in resume() and reset_resume(), always.

I would change it. Thanks.

Best Regards,
Hayes

[PATCH net-next 0/2] r8152: adjust runtime suspend/resume

[Hayes Wang]

Improve the flow about runtime suspend/resume and make the code
easy to read.

Hayes Wang (2):
  r8152: split rtl8152_resume function
  r8152: move calling delay_autosuspend function

 drivers/net/usb/r8152.c | 107 
 1 file changed, 62 insertions(+), 45 deletions(-)

-- 
2.7.4

[PATCH net-next 1/2] r8152: split rtl8152_resume function

[Hayes Wang]

Split rtl8152_resume() into rtl8152_runtime_resume() and
rtl8152_system_resume().

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 99 ++---
 1 file changed, 61 insertions(+), 38 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 5a02053..3257955 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3686,6 +3686,61 @@ static bool delay_autosuspend(struct r8152 *tp)
return false;
 }
 
+static int rtl8152_runtime_resume(struct r8152 *tp)
+{
+   struct net_device *netdev = tp->netdev;
+
+   if (netif_running(netdev) && netdev->flags & IFF_UP) {
+   struct napi_struct *napi = >napi;
+
+   tp->rtl_ops.autosuspend_en(tp, false);
+   napi_disable(napi);
+   set_bit(WORK_ENABLE, >flags);
+
+   if (netif_carrier_ok(netdev)) {
+   if (rtl8152_get_speed(tp) & LINK_STATUS) {
+   rtl_start_rx(tp);
+   } else {
+   netif_carrier_off(netdev);
+   tp->rtl_ops.disable(tp);
+   netif_info(tp, link, netdev, "linking down\n");
+   }
+   }
+
+   napi_enable(napi);
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
+
+   if (!list_empty(>rx_done))
+   napi_schedule(>napi);
+
+   usb_submit_urb(tp->intr_urb, GFP_KERNEL);
+   } else {
+   if (netdev->flags & IFF_UP)
+   tp->rtl_ops.autosuspend_en(tp, false);
+
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   }
+
+   return 0;
+}
+
+static int rtl8152_system_resume(struct r8152 *tp)
+{
+   struct net_device *netdev = tp->netdev;
+
+   netif_device_attach(netdev);
+
+   if (netif_running(netdev) && netdev->flags & IFF_UP) {
+   tp->rtl_ops.up(tp);
+   netif_carrier_off(netdev);
+   set_bit(WORK_ENABLE, >flags);
+   usb_submit_urb(tp->intr_urb, GFP_KERNEL);
+   }
+
+   return 0;
+}
+
 static int rtl8152_runtime_suspend(struct r8152 *tp)
 {
struct net_device *netdev = tp->netdev;
@@ -3784,50 +3839,18 @@ static int rtl8152_suspend(struct usb_interface *intf, 
pm_message_t message)
 static int rtl8152_resume(struct usb_interface *intf)
 {
struct r8152 *tp = usb_get_intfdata(intf);
-   struct net_device *netdev = tp->netdev;
+   int ret;
 
mutex_lock(>control);
 
-   if (!test_bit(SELECTIVE_SUSPEND, >flags))
-   netif_device_attach(netdev);
-
-   if (netif_running(netdev) && netdev->flags & IFF_UP) {
-   if (test_bit(SELECTIVE_SUSPEND, >flags)) {
-   struct napi_struct *napi = >napi;
-
-   tp->rtl_ops.autosuspend_en(tp, false);
-   napi_disable(napi);
-   set_bit(WORK_ENABLE, >flags);
-   if (netif_carrier_ok(netdev)) {
-   if (rtl8152_get_speed(tp) & LINK_STATUS) {
-   rtl_start_rx(tp);
-   } else {
-   netif_carrier_off(netdev);
-   tp->rtl_ops.disable(tp);
-   netif_info(tp, link, netdev,
-  "linking down\n");
-   }
-   }
-   napi_enable(napi);
-   clear_bit(SELECTIVE_SUSPEND, >flags);
-   smp_mb__after_atomic();
-   if (!list_empty(>rx_done))
-   napi_schedule(>napi);
-   } else {
-   tp->rtl_ops.up(tp);
-   netif_carrier_off(netdev);
-   set_bit(WORK_ENABLE, >flags);
-   }
-   usb_submit_urb(tp->intr_urb, GFP_KERNEL);
-   } else if (test_bit(SELECTIVE_SUSPEND, >flags)) {
-   if (netdev->flags & IFF_UP)
-   tp->rtl_ops.autosuspend_en(tp, false);
-   clear_bit(SELECTIVE_SUSPEND, >flags);
-   }
+   if (test_bit(SELECTIVE_SUSPEND, >flags))
+   ret = rtl8152_runtime_resume(tp);
+   else
+   ret = rtl8152_system_resume(tp);
 
mutex_unlock(>control);
 
-   return 0;
+   return ret;
 }
 
 static int rtl8152_reset_resume(struct usb_interface *intf)
-- 
2.7.4

[PATCH net-next 2/2] r8152: move calling delay_autosuspend function

[Hayes Wang]

Move calling delay_autosuspend() in rtl8152_runtime_suspend(). Calling
delay_autosuspend() as late as possible.

The original flows are
   1. check if the driver/device is busy now.
   2. set wake events.
   3. enter runtime suspend.

If the wake event occurs between (1) and (2), the device may miss it. Besides,
to avoid the runtime resume occurs after runtime suspend immediately, move the
checking to the end of rtl8152_runtime_suspend().

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 3257955..fb6fa6e 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3752,13 +3752,6 @@ static int rtl8152_runtime_suspend(struct r8152 *tp)
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
u32 rcr = 0;
 
-   if (delay_autosuspend(tp)) {
-   clear_bit(SELECTIVE_SUSPEND, >flags);
-   smp_mb__after_atomic();
-   ret = -EBUSY;
-   goto out1;
-   }
-
if (netif_carrier_ok(netdev)) {
u32 ocp_data;
 
@@ -3792,6 +3785,11 @@ static int rtl8152_runtime_suspend(struct r8152 *tp)
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
napi_enable(napi);
}
+
+   if (delay_autosuspend(tp)) {
+   rtl8152_runtime_resume(tp);
+   ret = -EBUSY;
+   }
}
 
 out1:
-- 
2.7.4

[PATCH net-next 00/11] r8152: minor adjustment

[Hayes Wang]

Adjust some code to make it reasonable or satisfy the suggestion from
the engineers.

Hayes Wang (11):
  r8152: add r8153_phy_status function
  r8152: adjust lpm settings for RTL8153
  r8152: adjust the settings about MAC clock speed down for RTL8153
  r8152: move the setting of rx aggregation
  r8152: adjust rtl8153_runtime_enable function
  r8152: adjust U2P3 for RTL8153
  r8152: move the default coalesce setting for RTL8153
  r8152: move the initialization to reset_resume function
  r8152: check if disabling ALDPS is finished
  r8152: avoid rx queue more than 1000 packets
  r8152: replace napi_complete with napi_complete_done

 drivers/net/usb/r8152.c | 181 ++--
 1 file changed, 130 insertions(+), 51 deletions(-)

-- 
2.7.4

[PATCH net-next 02/11] r8152: adjust lpm settings for RTL8153

[Hayes Wang]

Enable lpm after r8153_init() and remove other enable/disable lpm.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 9239dfb..b8c904f 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2263,7 +2263,6 @@ static int rtl8153_enable(struct r8152 *tp)
if (test_bit(RTL8152_UNPLUG, >flags))
return -ENODEV;
 
-   usb_disable_lpm(tp->udev);
set_tx_qlen(tp);
rtl_set_eee_plus(tp);
r8153_set_rx_early_timeout(tp);
@@ -3003,7 +3002,6 @@ static void rtl8153_disable(struct r8152 *tp)
rtl_disable(tp);
rtl_reset_bmu(tp);
r8153_aldps_en(tp, true);
-   usb_enable_lpm(tp->udev);
 }
 
 static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u16 speed, u8 
duplex)
@@ -3127,7 +3125,6 @@ static void rtl8153_up(struct r8152 *tp)
r8153_aldps_en(tp, true);
r8153_u2p3en(tp, true);
r8153_u1u2en(tp, true);
-   usb_enable_lpm(tp->udev);
 }
 
 static void rtl8153_down(struct r8152 *tp)
@@ -3457,7 +3454,6 @@ static void r8153_init(struct r8152 *tp)
 
data = r8153_phy_status(tp, PHY_STAT_LAN_ON);
 
-   usb_disable_lpm(tp->udev);
r8153_u2p3en(tp, false);
 
if (tp->version == RTL_VER_04) {
@@ -3517,6 +3513,7 @@ static void r8153_init(struct r8152 *tp)
 
r8153_power_cut_en(tp, false);
r8153_u1u2en(tp, true);
+   usb_enable_lpm(tp->udev);
 
/* MAC clock speed down */
ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL, 0);
-- 
2.7.4

[PATCH net-next 01/11] r8152: add r8153_phy_status function

[Hayes Wang]

Use r8153_phy_status() to check phy status of RTL8153.

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 37 +
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fd31fab..9239dfb 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -394,6 +394,7 @@
 
 /* OCP_PHY_STATUS */
 #define PHY_STAT_MASK  0x0007
+#define PHY_STAT_EXT_INIT  2
 #define PHY_STAT_LAN_ON3
 #define PHY_STAT_PWRDN 5
 
@@ -2452,6 +2453,28 @@ static void r8153_u2p3en(struct r8152 *tp, bool enable)
ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
 }
 
+static u16 r8153_phy_status(struct r8152 *tp, u16 desired)
+{
+   u16 data;
+   int i;
+
+   for (i = 0; i < 500; i++) {
+   data = ocp_reg_read(tp, OCP_PHY_STATUS);
+   data &= PHY_STAT_MASK;
+   if (desired) {
+   if (data == desired)
+   break;
+   } else if (data == PHY_STAT_LAN_ON || data == PHY_STAT_PWRDN ||
+  data == PHY_STAT_EXT_INIT) {
+   break;
+   }
+
+   msleep(20);
+   }
+
+   return data;
+}
+
 static void r8153_power_cut_en(struct r8152 *tp, bool enable)
 {
u32 ocp_data;
@@ -3420,12 +3443,7 @@ static void r8153_init(struct r8152 *tp)
msleep(20);
}
 
-   for (i = 0; i < 500; i++) {
-   ocp_data = ocp_reg_read(tp, OCP_PHY_STATUS) & PHY_STAT_MASK;
-   if (ocp_data == PHY_STAT_LAN_ON || ocp_data == PHY_STAT_PWRDN)
-   break;
-   msleep(20);
-   }
+   data = r8153_phy_status(tp, 0);
 
if (tp->version == RTL_VER_03 || tp->version == RTL_VER_04 ||
tp->version == RTL_VER_05)
@@ -3437,12 +3455,7 @@ static void r8153_init(struct r8152 *tp)
r8152_mdio_write(tp, MII_BMCR, data);
}
 
-   for (i = 0; i < 500; i++) {
-   ocp_data = ocp_reg_read(tp, OCP_PHY_STATUS) & PHY_STAT_MASK;
-   if (ocp_data == PHY_STAT_LAN_ON)
-   break;
-   msleep(20);
-   }
+   data = r8153_phy_status(tp, PHY_STAT_LAN_ON);
 
usb_disable_lpm(tp->udev);
r8153_u2p3en(tp, false);
-- 
2.7.4

[PATCH net-next 03/11] r8152: adjust the settings about MAC clock speed down for RTL8153

[Hayes Wang]

The MAC clock speed down could be enabled if the U1/U2 is disabled.

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index b8c904f..9a794db 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2428,6 +2428,29 @@ static void __rtl_set_wol(struct r8152 *tp, u32 wolopts)
device_set_wakeup_enable(>udev->dev, false);
 }
 
+static void r8153_mac_clk_spd(struct r8152 *tp, bool enable)
+{
+   /* MAC clock speed down */
+   if (enable) {
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL,
+  ALDPS_SPDWN_RATIO);
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL2,
+  EEE_SPDWN_RATIO);
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
+  PKT_AVAIL_SPDWN_EN | SUSPEND_SPDWN_EN |
+  U1U2_SPDWN_EN | L1_SPDWN_EN);
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4,
+  PWRSAVE_SPDWN_EN | RXDV_SPDWN_EN | TX10MIDLE_EN |
+  TP100_SPDWN_EN | TP500_SPDWN_EN | EEE_SPDWN_EN |
+  TP1000_SPDWN_EN);
+   } else {
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL, 0);
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL2, 0);
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, 0);
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4, 0);
+   }
+}
+
 static void r8153_u1u2en(struct r8152 *tp, bool enable)
 {
u8 u1u2[8];
@@ -2533,7 +2556,9 @@ static void rtl8153_runtime_enable(struct r8152 *tp, bool 
enable)
if (enable) {
r8153_u1u2en(tp, false);
r8153_u2p3en(tp, false);
+   r8153_mac_clk_spd(tp, true);
} else {
+   r8153_mac_clk_spd(tp, false);
r8153_u2p3en(tp, true);
r8153_u1u2en(tp, true);
}
@@ -2881,6 +2906,7 @@ static void r8153_first_init(struct r8152 *tp)
u32 ocp_data;
int i;
 
+   r8153_mac_clk_spd(tp, false);
rxdy_gated_en(tp, true);
r8153_teredo_off(tp);
 
@@ -2947,6 +2973,8 @@ static void r8153_enter_oob(struct r8152 *tp)
u32 ocp_data;
int i;
 
+   r8153_mac_clk_spd(tp, true);
+
ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL);
ocp_data &= ~NOW_IS_OOB;
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL, ocp_data);
@@ -3513,13 +3541,9 @@ static void r8153_init(struct r8152 *tp)
 
r8153_power_cut_en(tp, false);
r8153_u1u2en(tp, true);
+   r8153_mac_clk_spd(tp, false);
usb_enable_lpm(tp->udev);
 
-   /* MAC clock speed down */
-   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL, 0);
-   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL2, 0);
-   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, 0);
-   ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL4, 0);
 
rtl_tally_reset(tp);
r8153_u2p3en(tp, true);
-- 
2.7.4

[PATCH net-next 04/11] r8152: move the setting of rx aggregation

[Hayes Wang]

Move the setting from r8153_first_init() to r8153_init(). It only needs to
be set once.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 9a794db..e569c48 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2961,11 +2961,6 @@ static void r8153_first_init(struct r8152 *tp)
ocp_write_word(tp, MCU_TYPE_PLA, PLA_RXFIFO_CTRL2, RXFIFO_THR3_NORMAL);
/* TX share fifo free credit full threshold */
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_TXFIFO_CTRL, TXFIFO_THR_NORMAL2);
-
-   /* rx aggregation */
-   ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
-   ocp_data &= ~(RX_AGG_DISABLE | RX_ZERO_EN);
-   ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
 }
 
 static void r8153_enter_oob(struct r8152 *tp)
@@ -3544,6 +3539,10 @@ static void r8153_init(struct r8152 *tp)
r8153_mac_clk_spd(tp, false);
usb_enable_lpm(tp->udev);
 
+   /* rx aggregation */
+   ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
+   ocp_data &= ~(RX_AGG_DISABLE | RX_ZERO_EN);
+   ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
 
rtl_tally_reset(tp);
r8153_u2p3en(tp, true);
-- 
2.7.4

[PATCH net-next 06/11] r8152: adjust U2P3 for RTL8153

[Hayes Wang]

Use another way to keep disabling the U2P3 for both RTL_VER_03 and
RTL_VER_04.

Move enabling U2P3 from r8153_init() to r8153_hw_phy_cfg(). The
engineer ask the setting should be done after PHY settings.

Disable U2P3 first in rtl8153_up().

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 32e83fd..565ac5b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2468,7 +2468,7 @@ 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)
+   if (enable)
ocp_data |= U2P3_ENABLE;
else
ocp_data &= ~U2P3_ENABLE;
@@ -2559,7 +2559,18 @@ static void rtl8153_runtime_enable(struct r8152 *tp, 
bool enable)
} else {
rtl_runtime_suspend_enable(tp, false);
r8153_mac_clk_spd(tp, false);
-   r8153_u2p3en(tp, true);
+
+   switch (tp->version) {
+   case RTL_VER_03:
+   case RTL_VER_04:
+   break;
+   case RTL_VER_05:
+   case RTL_VER_06:
+   default:
+   r8153_u2p3en(tp, true);
+   break;
+   }
+
r8153_u1u2en(tp, true);
}
 }
@@ -2898,6 +2909,17 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
r8153_aldps_en(tp, true);
r8152b_enable_fc(tp);
 
+   switch (tp->version) {
+   case RTL_VER_03:
+   case RTL_VER_04:
+   break;
+   case RTL_VER_05:
+   case RTL_VER_06:
+   default:
+   r8153_u2p3en(tp, true);
+   break;
+   }
+
set_bit(PHY_RESET, >flags);
 }
 
@@ -3143,10 +3165,22 @@ static void rtl8153_up(struct r8152 *tp)
return;
 
r8153_u1u2en(tp, false);
+   r8153_u2p3en(tp, false);
r8153_aldps_en(tp, false);
r8153_first_init(tp);
r8153_aldps_en(tp, true);
-   r8153_u2p3en(tp, true);
+
+   switch (tp->version) {
+   case RTL_VER_03:
+   case RTL_VER_04:
+   break;
+   case RTL_VER_05:
+   case RTL_VER_06:
+   default:
+   r8153_u2p3en(tp, true);
+   break;
+   }
+
r8153_u1u2en(tp, true);
 }
 
@@ -3545,7 +3579,6 @@ static void r8153_init(struct r8152 *tp)
ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
 
rtl_tally_reset(tp);
-   r8153_u2p3en(tp, true);
 }
 
 static int rtl8152_pre_reset(struct usb_interface *intf)
-- 
2.7.4

[PATCH net-next 05/11] r8152: adjust rtl8153_runtime_enable function

[Hayes Wang]

Adjust the order of rtl8153_runtime_enable() according to the
suggestion from the engineer.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e569c48..32e83fd 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2551,13 +2551,13 @@ static void rtl_runtime_suspend_enable(struct r8152 
*tp, bool enable)
 
 static void rtl8153_runtime_enable(struct r8152 *tp, bool enable)
 {
-   rtl_runtime_suspend_enable(tp, enable);
-
if (enable) {
r8153_u1u2en(tp, false);
r8153_u2p3en(tp, false);
r8153_mac_clk_spd(tp, true);
+   rtl_runtime_suspend_enable(tp, true);
} else {
+   rtl_runtime_suspend_enable(tp, false);
r8153_mac_clk_spd(tp, false);
r8153_u2p3en(tp, true);
r8153_u1u2en(tp, true);
-- 
2.7.4

[PATCH net-next 07/11] r8152: move the default coalesce setting for RTL8153

[Hayes Wang]

Only RTL8153 could set coalesce, so move the default setting for
rtl8152_probe() to r8153_init().

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 565ac5b..f78111c 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3579,6 +3579,19 @@ static void r8153_init(struct r8152 *tp)
ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
 
rtl_tally_reset(tp);
+
+   switch (tp->udev->speed) {
+   case USB_SPEED_SUPER:
+   case USB_SPEED_SUPER_PLUS:
+   tp->coalesce = COALESCE_SUPER;
+   break;
+   case USB_SPEED_HIGH:
+   tp->coalesce = COALESCE_HIGH;
+   break;
+   default:
+   tp->coalesce = COALESCE_SLOW;
+   break;
+   }
 }
 
 static int rtl8152_pre_reset(struct usb_interface *intf)
@@ -4524,19 +4537,6 @@ 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:
-   case USB_SPEED_SUPER_PLUS:
-   tp->coalesce = COALESCE_SUPER;
-   break;
-   case USB_SPEED_HIGH:
-   tp->coalesce = COALESCE_HIGH;
-   break;
-   default:
-   tp->coalesce = COALESCE_SLOW;
-   break;
-   }
-
tp->autoneg = AUTONEG_ENABLE;
tp->speed = tp->mii.supports_gmii ? SPEED_1000 : SPEED_100;
tp->duplex = DUPLEX_FULL;
-- 
2.7.4

[PATCH net-next 08/11] r8152: move the initialization to reset_resume function

[Hayes Wang]

Move tp->rtl_ops.init() from rtl8152_resume() to rtl8152_reset_resume().
The initialization is only necessary for reset_resume().

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f78111c..f43b7a8 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3776,11 +3776,8 @@ static int rtl8152_resume(struct usb_interface *intf)
 
mutex_lock(>control);
 
-   if (!test_bit(SELECTIVE_SUSPEND, >flags)) {
-   tp->rtl_ops.init(tp);
-   queue_delayed_work(system_long_wq, >hw_phy_work, 0);
+   if (!test_bit(SELECTIVE_SUSPEND, >flags))
netif_device_attach(netdev);
-   }
 
if (netif_running(netdev) && netdev->flags & IFF_UP) {
if (test_bit(SELECTIVE_SUSPEND, >flags)) {
@@ -3826,6 +3823,10 @@ static int rtl8152_reset_resume(struct usb_interface 
*intf)
struct r8152 *tp = usb_get_intfdata(intf);
 
clear_bit(SELECTIVE_SUSPEND, >flags);
+   mutex_lock(>control);
+   tp->rtl_ops.init(tp);
+   queue_delayed_work(system_long_wq, >hw_phy_work, 0);
+   mutex_unlock(>control);
return rtl8152_resume(intf);
 }
 
-- 
2.7.4

[PATCH net-next 09/11] r8152: check if disabling ALDPS is finished

[Hayes Wang]

Use PLA 0xe000 bit 8 to check if disabling ALDPS is finished.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f43b7a8..204f4b2 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2836,9 +2836,15 @@ static void r8153_aldps_en(struct r8152 *tp, bool enable)
data |= EN_ALDPS;
ocp_reg_write(tp, OCP_POWER_CFG, data);
} else {
+   int i;
+
data &= ~EN_ALDPS;
ocp_reg_write(tp, OCP_POWER_CFG, data);
-   msleep(20);
+   for (i = 0; i < 20; i++) {
+   usleep_range(1000, 2000);
+   if (ocp_read_word(tp, MCU_TYPE_PLA, 0xe000) & 0x0100)
+   break;
+   }
}
 }
 
-- 
2.7.4

[PATCH net-next 10/11] r8152: avoid rx queue more than 1000 packets

[Hayes Wang]

Stop queuing rx packets if it is more than 1000.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 204f4b2..fa29583 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1813,6 +1813,10 @@ static int rx_bottom(struct r8152 *tp, int budget)
unsigned int pkt_len;
struct sk_buff *skb;
 
+   /* limite the skb numbers for rx_queue */
+   if (unlikely(skb_queue_len(>rx_queue) >= 1000))
+   break;
+
pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
if (pkt_len < ETH_ZLEN)
break;
-- 
2.7.4

[PATCH net-next 11/11] r8152: replace napi_complete with napi_complete_done

[Hayes Wang]

Change from using napi_complete to napi_complete_done to allow for the
use of gro_flush_timeout in tuning network processing.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fa29583..5a02053 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1938,7 +1938,8 @@ static int r8152_poll(struct napi_struct *napi, int 
budget)
bottom_half(tp);
 
if (work_done < budget) {
-   napi_complete(napi);
+   if (!napi_complete_done(napi, work_done))
+   goto out;
if (!list_empty(>rx_done))
napi_schedule(napi);
else if (!skb_queue_empty(>tx_queue) &&
@@ -1946,6 +1947,7 @@ static int r8152_poll(struct napi_struct *napi, int 
budget)
napi_schedule(napi);
}
 
+out:
return work_done;
 }
 
-- 
2.7.4

RE: [PATCH] Fix for new version of realtek r8153

[Hayes Wang]

jake Briggs [mailto:nexus...@gmail.com]
> Sent: Wednesday, May 03, 2017 7:21 AM
[...]
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 07f788c49d57..2a55459fdfac 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -4277,6 +4277,7 @@ static void r8152b_get_version(struct r8152 *tp)
>   tp->mii.supports_gmii = 1;
>   break;
>   case 0x5c30:
> + case 0x6010:

The two chips are different. I don't think it is a good idea.
Maybe you could use the driver from the Realtek website first.

>   tp->version = RTL_VER_06;
>   tp->mii.supports_gmii = 1;
>   break;

[PATCH net] r8152: prevent the driver from transmitting packets with carrier off

[Hayes Wang]

The linking status may be changed when autosuspend. And, after
autoresume, the driver may try to transmit packets when the device
is carrier off, because the interrupt transfer doesn't update the
linking status, yet. And, if the device is in ALDPS mode, the device
would stop working.

The another similar case is
 1. unplug the cable.
 2. interrupt transfer queue a work_queue for linking change.
 3. device enters the ALDPS mode.
 4. a tx occurs before the work_queue is called.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0b1b918..c34df33 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1294,6 +1294,7 @@ static void intr_callback(struct urb *urb)
}
} else {
if (netif_carrier_ok(tp->netdev)) {
+   netif_stop_queue(tp->netdev);
set_bit(RTL8152_LINK_CHG, >flags);
schedule_delayed_work(>schedule, 0);
}
@@ -3169,6 +3170,9 @@ static void set_carrier(struct r8152 *tp)
napi_enable(>napi);
netif_wake_queue(netdev);
netif_info(tp, link, netdev, "carrier on\n");
+   } else if (netif_queue_stopped(netdev) &&
+  skb_queue_len(>tx_queue) < tp->tx_qlen) {
+   netif_wake_queue(netdev);
}
} else {
if (netif_carrier_ok(netdev)) {
@@ -3702,8 +3706,18 @@ static int rtl8152_resume(struct usb_interface *intf)
tp->rtl_ops.autosuspend_en(tp, false);
napi_disable(>napi);
set_bit(WORK_ENABLE, >flags);
-   if (netif_carrier_ok(tp->netdev))
-   rtl_start_rx(tp);
+
+   if (netif_carrier_ok(tp->netdev)) {
+   if (rtl8152_get_speed(tp) & LINK_STATUS) {
+   rtl_start_rx(tp);
+   } else {
+   netif_carrier_off(tp->netdev);
+   tp->rtl_ops.disable(tp);
+   netif_info(tp, link, tp->netdev,
+  "linking down\n");
+   }
+   }
+
napi_enable(>napi);
clear_bit(SELECTIVE_SUSPEND, >flags);
smp_mb__after_atomic();
-- 
2.7.4

[PATCH net 0/2] r8152: fix the rx settings of RTL8153

[Hayes Wang]

The RMS and the rx early size should base on the same rx size. However,
the RMS is set to 9K bytes now and the rx early depends on mtu. For using
the rx buffer effectively, sync the two settings according to the mtu.

Hayes Wang (2):
  r8152: set the RMS of RTL8153 according to the mtu
  r8152: fix the rx early size of RTL8153

 drivers/net/usb/r8152.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

-- 
2.7.4

[PATCH net 1/2] r8152: set the RMS of RTL8153 according to the mtu

[Hayes Wang]

Set the received maximum size (RMS) according to the mtu size. It is
unnecessary to receive a packet which is more than the size we could
transmit. Besides, this could let the rx buffer be used effectively.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 986243c..3a892fa 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2898,7 +2898,8 @@ static void r8153_first_init(struct r8152 *tp)
 
rtl_rx_vlan_en(tp, tp->netdev->features & NETIF_F_HW_VLAN_CTAG_RX);
 
-   ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, RTL8153_RMS);
+   ocp_data = tp->netdev->mtu + VLAN_ETH_HLEN + CRC_SIZE;
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, ocp_data);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, MTPS_JUMBO);
 
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_TCR0);
@@ -2950,7 +2951,8 @@ static void r8153_enter_oob(struct r8152 *tp)
usleep_range(1000, 2000);
}
 
-   ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, RTL8153_RMS);
+   ocp_data = tp->netdev->mtu + VLAN_ETH_HLEN + CRC_SIZE;
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, ocp_data);
 
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_TEREDO_CFG);
ocp_data &= ~TEREDO_WAKE_MASK;
@@ -4200,8 +4202,14 @@ static int rtl8152_change_mtu(struct net_device *dev, 
int new_mtu)
 
dev->mtu = new_mtu;
 
-   if (netif_running(dev) && netif_carrier_ok(dev))
-   r8153_set_rx_early_size(tp);
+   if (netif_running(dev)) {
+   u32 rms = new_mtu + VLAN_ETH_HLEN + CRC_SIZE;
+
+   ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, rms);
+
+   if (netif_carrier_ok(dev))
+   r8153_set_rx_early_size(tp);
+   }
 
mutex_unlock(>control);
 
-- 
2.7.4

[PATCH net 2/2] r8152: fix the rx early size of RTL8153

[Hayes Wang]

revert commit a59e6d815226 ("r8152: correct the rx early size") and
fix the rx early size as

(rx buffer size - rx packet size - rx desc size - alignment) / 4

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 3a892fa..800be78 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -32,7 +32,7 @@
 #define NETNEXT_VERSION"08"
 
 /* Information for net */
-#define NET_VERSION"8"
+#define NET_VERSION"9"
 
 #define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION
 #define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_s...@realtek.com>"
@@ -501,6 +501,8 @@ enum rtl_register_content {
 #define RTL8153_RMSRTL8153_MAX_PACKET
 #define RTL8152_TX_TIMEOUT (5 * HZ)
 #define RTL8152_NAPI_WEIGHT64
+#define rx_reserved_size(x)((x) + VLAN_ETH_HLEN + CRC_SIZE + \
+sizeof(struct rx_desc) + RX_ALIGN)
 
 /* rtl8152 flags */
 enum rtl8152_flags {
@@ -2252,8 +2254,7 @@ static void r8153_set_rx_early_timeout(struct r8152 *tp)
 
 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) / 8;
+   u32 ocp_data = (agg_buf_sz - rx_reserved_size(tp->netdev->mtu)) / 4;
 
ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
 }
-- 
2.7.4

RE: [PATCH net-next] r8152: simply the arguments

[Hayes Wang]

David Laight [mailto:david.lai...@aculab.com]
> Sent: Thursday, March 16, 2017 10:28 PM
[...]
> If you are really lucky the compiler will optimise it away.
> Otherwise it will generate another local variable and possibly
> a register spill to stack.

However, I could reduce the time for calculating the address,
because I only calculate it once and save the result to a variable.
Right?

Best Regards,
Hayes

[PATCH net-next] r8152: check hw version first

[Hayes Wang]

Check hw version first in probe(). Do nothing if the driver doesn't
support the chip.

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 102 ++--
 1 file changed, 63 insertions(+), 39 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 4b85e95..3262a32 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -4236,44 +4236,6 @@ static const struct net_device_ops rtl8152_netdev_ops = {
.ndo_features_check = rtl8152_features_check,
 };
 
-static void r8152b_get_version(struct r8152 *tp)
-{
-   u32 ocp_data;
-   u16 version;
-
-   ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_TCR1);
-   version = (u16)(ocp_data & VERSION_MASK);
-
-   switch (version) {
-   case 0x4c00:
-   tp->version = RTL_VER_01;
-   break;
-   case 0x4c10:
-   tp->version = RTL_VER_02;
-   break;
-   case 0x5c00:
-   tp->version = RTL_VER_03;
-   tp->mii.supports_gmii = 1;
-   break;
-   case 0x5c10:
-   tp->version = RTL_VER_04;
-   tp->mii.supports_gmii = 1;
-   break;
-   case 0x5c20:
-   tp->version = RTL_VER_05;
-   tp->mii.supports_gmii = 1;
-   break;
-   case 0x5c30:
-   tp->version = RTL_VER_06;
-   tp->mii.supports_gmii = 1;
-   break;
-   default:
-   netif_info(tp, probe, tp->netdev,
-  "Unknown version 0x%04x\n", version);
-   break;
-   }
-}
-
 static void rtl8152_unload(struct r8152 *tp)
 {
if (test_bit(RTL8152_UNPLUG, >flags))
@@ -4338,14 +4300,66 @@ static int rtl_ops_init(struct r8152 *tp)
return ret;
 }
 
+static u8 rtl_get_version(struct usb_interface *intf)
+{
+   struct usb_device *udev = interface_to_usbdev(intf);
+   u32 ocp_data = 0;
+   __le32 *tmp;
+   u8 version;
+   int ret;
+
+   tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+   if (!tmp)
+   return 0;
+
+   ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+ RTL8152_REQ_GET_REGS, RTL8152_REQT_READ,
+ PLA_TCR0, MCU_TYPE_PLA, tmp, sizeof(*tmp), 500);
+   if (ret > 0)
+   ocp_data = (__le32_to_cpu(*tmp) >> 16) & VERSION_MASK;
+
+   kfree(tmp);
+
+   switch (ocp_data) {
+   case 0x4c00:
+   version = RTL_VER_01;
+   break;
+   case 0x4c10:
+   version = RTL_VER_02;
+   break;
+   case 0x5c00:
+   version = RTL_VER_03;
+   break;
+   case 0x5c10:
+   version = RTL_VER_04;
+   break;
+   case 0x5c20:
+   version = RTL_VER_05;
+   break;
+   case 0x5c30:
+   version = RTL_VER_06;
+   break;
+   default:
+   version = RTL_VER_UNKNOWN;
+   dev_info(>dev, "Unknown version 0x%04x\n", ocp_data);
+   break;
+   }
+
+   return version;
+}
+
 static int rtl8152_probe(struct usb_interface *intf,
 const struct usb_device_id *id)
 {
struct usb_device *udev = interface_to_usbdev(intf);
+   u8 version = rtl_get_version(intf);
struct r8152 *tp;
struct net_device *netdev;
int ret;
 
+   if (version == RTL_VER_UNKNOWN)
+   return -ENODEV;
+
if (udev->actconfig->desc.bConfigurationValue != 1) {
usb_driver_set_configuration(udev, 1);
return -ENODEV;
@@ -4365,8 +4379,18 @@ static int rtl8152_probe(struct usb_interface *intf,
tp->udev = udev;
tp->netdev = netdev;
tp->intf = intf;
+   tp->version = version;
+
+   switch (version) {
+   case RTL_VER_01:
+   case RTL_VER_02:
+   tp->mii.supports_gmii = 0;
+   break;
+   default:
+   tp->mii.supports_gmii = 1;
+   break;
+   }
 
-   r8152b_get_version(tp);
ret = rtl_ops_init(tp);
if (ret)
goto out;
-- 
2.7.4

[PATCH v2 net-next] r8152: simply the arguments

[Hayes Wang]

Replace >napi with napi and tp->netdev with netdev.

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 43 ++-
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 227e1fd..4b85e95 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1761,6 +1761,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
unsigned long flags;
struct list_head *cursor, *next, rx_queue;
int ret = 0, work_done = 0;
+   struct napi_struct *napi = >napi;
 
if (!skb_queue_empty(>rx_queue)) {
while (work_done < budget) {
@@ -1773,7 +1774,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
break;
 
pkt_len = skb->len;
-   napi_gro_receive(>napi, skb);
+   napi_gro_receive(napi, skb);
work_done++;
stats->rx_packets++;
stats->rx_bytes += pkt_len;
@@ -1823,7 +1824,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
pkt_len -= CRC_SIZE;
rx_data += sizeof(struct rx_desc);
 
-   skb = napi_alloc_skb(>napi, pkt_len);
+   skb = napi_alloc_skb(napi, pkt_len);
if (!skb) {
stats->rx_dropped++;
goto find_next_rx;
@@ -1835,7 +1836,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
skb->protocol = eth_type_trans(skb, netdev);
rtl_rx_vlan_tag(rx_desc, skb);
if (work_done < budget) {
-   napi_gro_receive(>napi, skb);
+   napi_gro_receive(napi, skb);
work_done++;
stats->rx_packets++;
stats->rx_bytes += pkt_len;
@@ -3150,6 +3151,7 @@ static bool rtl8153_in_nway(struct r8152 *tp)
 static void set_carrier(struct r8152 *tp)
 {
struct net_device *netdev = tp->netdev;
+   struct napi_struct *napi = >napi;
u8 speed;
 
speed = rtl8152_get_speed(tp);
@@ -3159,7 +3161,7 @@ static void set_carrier(struct r8152 *tp)
tp->rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, >flags);
netif_stop_queue(netdev);
-   napi_disable(>napi);
+   napi_disable(napi);
netif_carrier_on(netdev);
rtl_start_rx(tp);
napi_enable(>napi);
@@ -3169,9 +3171,9 @@ static void set_carrier(struct r8152 *tp)
} else {
if (netif_carrier_ok(netdev)) {
netif_carrier_off(netdev);
-   napi_disable(>napi);
+   napi_disable(napi);
tp->rtl_ops.disable(tp);
-   napi_enable(>napi);
+   napi_enable(napi);
netif_info(tp, link, netdev, "carrier off\n");
}
}
@@ -3633,11 +3635,13 @@ static int rtl8152_runtime_suspend(struct r8152 *tp)
tp->rtl_ops.autosuspend_en(tp, true);
 
if (netif_carrier_ok(netdev)) {
-   napi_disable(>napi);
+   struct napi_struct *napi = >napi;
+
+   napi_disable(napi);
rtl_stop_rx(tp);
rxdy_gated_en(tp, false);
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
-   napi_enable(>napi);
+   napi_enable(napi);
}
}
 
@@ -3653,12 +3657,14 @@ static int rtl8152_system_suspend(struct r8152 *tp)
netif_device_detach(netdev);
 
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
+   struct napi_struct *napi = >napi;
+
clear_bit(WORK_ENABLE, >flags);
usb_kill_urb(tp->intr_urb);
-   napi_disable(>napi);
+   napi_disable(napi);
cancel_delayed_work_sync(>schedule);
tp->rtl_ops.down(tp);
-   napi_enable(>napi);
+   napi_enable(napi);
}
 
return ret;
@@ -3684,35 +3690,38 @@ static int rtl8152_suspend(struct usb_interface *intf, 
pm_message_t message)
 static int rtl8152_resume(struct usb_interface *intf)
 {
struct r8152 *tp = usb_get_intfdata(intf);
+   struct net_device *netdev = tp->netdev;
 
mutex_lock(>control);
 
if (!test_bit(SELECTIVE_SUSPEND, >flags)) {
 

[PATCH net-next] r8152: simply the arguments

[Hayes Wang]

Replace >napi with napi and tp->netdev with netdev.

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 44 +++-
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 227e1fd..e480e9f 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1761,6 +1761,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
unsigned long flags;
struct list_head *cursor, *next, rx_queue;
int ret = 0, work_done = 0;
+   struct napi_struct *napi = >napi;
 
if (!skb_queue_empty(>rx_queue)) {
while (work_done < budget) {
@@ -1773,7 +1774,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
break;
 
pkt_len = skb->len;
-   napi_gro_receive(>napi, skb);
+   napi_gro_receive(napi, skb);
work_done++;
stats->rx_packets++;
stats->rx_bytes += pkt_len;
@@ -1823,7 +1824,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
pkt_len -= CRC_SIZE;
rx_data += sizeof(struct rx_desc);
 
-   skb = napi_alloc_skb(>napi, pkt_len);
+   skb = napi_alloc_skb(napi, pkt_len);
if (!skb) {
stats->rx_dropped++;
goto find_next_rx;
@@ -1835,7 +1836,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
skb->protocol = eth_type_trans(skb, netdev);
rtl_rx_vlan_tag(rx_desc, skb);
if (work_done < budget) {
-   napi_gro_receive(>napi, skb);
+   napi_gro_receive(napi, skb);
work_done++;
stats->rx_packets++;
stats->rx_bytes += pkt_len;
@@ -3150,6 +3151,8 @@ static bool rtl8153_in_nway(struct r8152 *tp)
 static void set_carrier(struct r8152 *tp)
 {
struct net_device *netdev = tp->netdev;
+   struct napi_struct *napi = >napi;
+
u8 speed;
 
speed = rtl8152_get_speed(tp);
@@ -3159,7 +3162,7 @@ static void set_carrier(struct r8152 *tp)
tp->rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, >flags);
netif_stop_queue(netdev);
-   napi_disable(>napi);
+   napi_disable(napi);
netif_carrier_on(netdev);
rtl_start_rx(tp);
napi_enable(>napi);
@@ -3169,9 +3172,9 @@ static void set_carrier(struct r8152 *tp)
} else {
if (netif_carrier_ok(netdev)) {
netif_carrier_off(netdev);
-   napi_disable(>napi);
+   napi_disable(napi);
tp->rtl_ops.disable(tp);
-   napi_enable(>napi);
+   napi_enable(napi);
netif_info(tp, link, netdev, "carrier off\n");
}
}
@@ -3633,11 +3636,13 @@ static int rtl8152_runtime_suspend(struct r8152 *tp)
tp->rtl_ops.autosuspend_en(tp, true);
 
if (netif_carrier_ok(netdev)) {
-   napi_disable(>napi);
+   struct napi_struct *napi = >napi;
+
+   napi_disable(napi);
rtl_stop_rx(tp);
rxdy_gated_en(tp, false);
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
-   napi_enable(>napi);
+   napi_enable(napi);
}
}
 
@@ -3653,12 +3658,14 @@ static int rtl8152_system_suspend(struct r8152 *tp)
netif_device_detach(netdev);
 
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
+   struct napi_struct *napi = >napi;
+
clear_bit(WORK_ENABLE, >flags);
usb_kill_urb(tp->intr_urb);
-   napi_disable(>napi);
+   napi_disable(napi);
cancel_delayed_work_sync(>schedule);
tp->rtl_ops.down(tp);
-   napi_enable(>napi);
+   napi_enable(napi);
}
 
return ret;
@@ -3684,35 +3691,38 @@ static int rtl8152_suspend(struct usb_interface *intf, 
pm_message_t message)
 static int rtl8152_resume(struct usb_interface *intf)
 {
struct r8152 *tp = usb_get_intfdata(intf);
+   struct net_device *netdev = tp->netdev;
 
mutex_lock(>control);
 
if (!test_bit(SELECTIVE_SUSPEND, >flags)) {
 

RE: [PATCH net] r8152: fix the list rx_done may be used without initialization

[Hayes Wang]

Petr Vorel [mailto:petr.vo...@gmail.com]
> Sent: Tuesday, March 14, 2017 4:54 PM
[...]
> thanks for fixing!

Does it work?

Best Regards,
Hayes

[PATCH net] r8152: fix the list rx_done may be used without initialization

[Hayes Wang]

The list rx_done would be initialized when the linking on occurs.
Therefore, if a napi is scheduled without any linking on before,
the following kernel panic would happen.

BUG: unable to handle kernel NULL pointer dereference at 008
IP: [] r8152_poll+0xe1e/0x1210 [r8152]
PGD 0
Oops: 0002 [#1] SMP

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 986243c..bb3eedd 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1362,6 +1362,7 @@ static int alloc_all_mem(struct r8152 *tp)
spin_lock_init(>rx_lock);
spin_lock_init(>tx_lock);
INIT_LIST_HEAD(>tx_free);
+   INIT_LIST_HEAD(>rx_done);
skb_queue_head_init(>tx_queue);
skb_queue_head_init(>rx_queue);
 
-- 
2.7.4

RE: [PATCH] net: usb: r8152: use new api ethtool_{get|set}_link_ksettings

[Hayes Wang]

Philippe Reynes [mailto:trem...@gmail.com]
> Sent: Monday, March 13, 2017 5:42 AM
> Subject: [PATCH] net: usb: r8152: use new api ethtool_{get|set}_link_ksettings
> 
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> As I don't have the hardware, I'd be very pleased if
> someone may test this patch.
> 
> Signed-off-by: Philippe Reynes <trem...@gmail.com>
> ---
>  drivers/net/usb/r8152.c |   21 -
>  1 files changed, 12 insertions(+), 9 deletions(-)

Acked-by: Hayes Wang <hayesw...@realtek.com>

RE: [PATCH net v3 0/4] r8152: fix scheduling napi

[Hayes Wang]

David Miller [mailto:da...@davemloft.net]
> Sent: Thursday, January 26, 2017 11:48 AM
[...] 
> Series applied.

Thank you very much. I would try to find better way, too.

Best Regards,
Hayes

RE: [PATCH net v2 0/4] r8152: fix scheduling napi

[Hayes Wang]

David Miller [mailto:da...@davemloft.net]
> Sent: Thursday, January 26, 2017 3:31 AM
[...]
> I think the fundamental issue is that since you can't stop URBs from
> queueing up, you cannot properly synchronize NAPI and schedule polling
> properly.
> 
> From my perspective what happened here is you want GRO support, but it
> comes at the expense of this extremely racey NAPI support which does
> not at all achieve one of the main advantages of NAPI which is
> interrupt mitigation.

May you apply these patches first, until I find another way to replace
current one? The driver uses NAPI now and I have no idea to find better
way to replace current one. I think it would take me long time to find
out the solution. And the issue is still there until I finish this work.
If now I give up the NAPI and its advantages except for the interrupt
mitigation, some things would become worse.

Our hw supports packet aggregation. The purpose is interrupt mitigation,
too. I wouldn't say it is better than what the NAPI does. However, I
could say we try to improve it. If the interrupt could be disabled, I
would be happy to do it. However, it is the limitation of USB devices.
That is why I still use NAPI even though the interrupt cannot be disabled
for USB devices. Because one of the advantages of NAPI couldn't be
satisfied, I must not use the NAPI. Doesn't it seem too strict?

Best Regards,
Hayes

RE: NAPI on USB network drivers

[Hayes Wang]

Eric Dumazet [mailto:eric.duma...@gmail.com]
> Sent: Wednesday, January 25, 2017 10:13 PM
[...]
> You also could use napi_complete_done() instead of napi_complete(), as
> it allows users to tune the performance vs latency for GRO.
> 
> Looking at this driver, I do not see any limitation on the number of
> skbs that can be pushed into tp->rx_queue.
> 
> I wonder if this queue can end up consuming all memory of a host under
> stress.
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index
> e1466b4d2b6c727148a884672bbd9593bf04b3ac..221df4a931b5c1073f1922d0fa0b
> bff158c73b7d 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1840,7 +1840,10 @@ static int rx_bottom(struct r8152 *tp, int budget)
>   stats->rx_packets++;
>   stats->rx_bytes += pkt_len;
>   } else {
> - __skb_queue_tail(>rx_queue, skb);
> + if (unlikely(skb_queue_len(>rx_queue) >= 
> 1000))
> + kfree_skb(skb);
> + else
> + __skb_queue_tail(>rx_queue, skb);
>   }
> 
>  find_next_rx:

Thanks for your suggestion. I would submit it later.

Best Regards,
Hayes

[PATCH net v3 4/4] r8152: check rx after napi is enabled

[Hayes Wang]

Schedule the napi after napi_enable() for rx, if it is necessary.

If the rx is completed when napi is disabled, the sheduling of napi
would be lost. Then, no one handles the rx packet until next napi
is scheduled.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 4785d2b..ad42295 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -32,7 +32,7 @@
 #define NETNEXT_VERSION"08"
 
 /* Information for net */
-#define NET_VERSION"7"
+#define NET_VERSION"8"
 
 #define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION
 #define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_s...@realtek.com>"
@@ -3561,6 +3561,9 @@ static int rtl8152_post_reset(struct usb_interface *intf)
netif_wake_queue(netdev);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
+   if (!list_empty(>rx_done))
+   napi_schedule(>napi);
+
return 0;
 }
 
@@ -3700,6 +3703,8 @@ static int rtl8152_resume(struct usb_interface *intf)
napi_enable(>napi);
clear_bit(SELECTIVE_SUSPEND, >flags);
smp_mb__after_atomic();
+   if (!list_empty(>rx_done))
+   napi_schedule(>napi);
} else {
tp->rtl_ops.up(tp);
netif_carrier_off(tp->netdev);
-- 
2.7.4

[PATCH net v3 1/4] r8152: avoid start_xmit to call napi_schedule during autosuspend

[Hayes Wang]

Adjust the setting of the flag of SELECTIVE_SUSPEND to prevent start_xmit()
from calling napi_schedule() directly during runtime suspend.

After calling napi_disable() or clearing the flag of WORK_ENABLE,
scheduling the napi is useless.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e1466b4..23bef8e 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3585,10 +3585,15 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
struct net_device *netdev = tp->netdev;
int ret = 0;
 
+   set_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
+
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
u32 rcr = 0;
 
if (delay_autosuspend(tp)) {
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
ret = -EBUSY;
goto out1;
}
@@ -3605,6 +3610,8 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
if (!(ocp_data & RXFIFO_EMPTY)) {
rxdy_gated_en(tp, false);
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
ret = -EBUSY;
goto out1;
}
@@ -3624,8 +3631,6 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
}
}
 
-   set_bit(SELECTIVE_SUSPEND, >flags);
-
 out1:
return ret;
 }
@@ -3681,12 +3686,13 @@ static int rtl8152_resume(struct usb_interface *intf)
if (netif_running(tp->netdev) && tp->netdev->flags & IFF_UP) {
if (test_bit(SELECTIVE_SUSPEND, >flags)) {
tp->rtl_ops.autosuspend_en(tp, false);
-   clear_bit(SELECTIVE_SUSPEND, >flags);
napi_disable(>napi);
set_bit(WORK_ENABLE, >flags);
if (netif_carrier_ok(tp->netdev))
rtl_start_rx(tp);
napi_enable(>napi);
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
} else {
tp->rtl_ops.up(tp);
netif_carrier_off(tp->netdev);
-- 
2.7.4

[PATCH net v3 0/4] r8152: fix scheduling napi

[Hayes Wang]

v3:
simply the argument for patch #3. Replace >napi with napi.

v2:
Add smp_mb__after_atomic() for patch #1.

v1:
Scheduling the napi during the following periods would let it be ignored.
And the events wouldn't be handled until next napi_schedule() is called.

1. after napi_disable and before napi_enable().
2. after all actions of napi function is completed and before calling
   napi_complete().

If no next napi_schedule() is called, tx or rx would stop working.

In order to avoid these situations, the followings solutions are applied.

1. prevent start_xmit() from calling napi_schedule() during runtime suspend
   or after napi_disable().
2. re-schedule the napi for tx if it is necessary.
3. check if any rx is finished or not after napi_enable().

Hayes Wang (4):
  r8152: avoid start_xmit to call napi_schedule during autosuspend
  r8152: avoid start_xmit to schedule napi when napi is disabled
  r8152: re-schedule napi for tx
  r8152: check rx after napi is enabled

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

-- 
2.7.4

[PATCH net v3 2/4] r8152: avoid start_xmit to schedule napi when napi is disabled

[Hayes Wang]

Stop the tx when the napi is disabled to prevent napi_schedule() is
called.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 23bef8e..ec882be 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3155,10 +3155,13 @@ static void set_carrier(struct r8152 *tp)
if (!netif_carrier_ok(netdev)) {
tp->rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, >flags);
+   netif_stop_queue(netdev);
napi_disable(>napi);
netif_carrier_on(netdev);
rtl_start_rx(tp);
napi_enable(>napi);
+   netif_wake_queue(netdev);
+   netif_info(tp, link, netdev, "carrier on\n");
}
} else {
if (netif_carrier_ok(netdev)) {
@@ -3166,6 +3169,7 @@ static void set_carrier(struct r8152 *tp)
napi_disable(>napi);
tp->rtl_ops.disable(tp);
napi_enable(>napi);
+   netif_info(tp, link, netdev, "carrier off\n");
}
}
 }
@@ -3515,12 +3519,12 @@ static int rtl8152_pre_reset(struct usb_interface *intf)
if (!netif_running(netdev))
return 0;
 
+   netif_stop_queue(netdev);
napi_disable(>napi);
clear_bit(WORK_ENABLE, >flags);
usb_kill_urb(tp->intr_urb);
cancel_delayed_work_sync(>schedule);
if (netif_carrier_ok(netdev)) {
-   netif_stop_queue(netdev);
mutex_lock(>control);
tp->rtl_ops.disable(tp);
mutex_unlock(>control);
@@ -3548,10 +3552,10 @@ static int rtl8152_post_reset(struct usb_interface 
*intf)
rtl_start_rx(tp);
rtl8152_set_rx_mode(netdev);
mutex_unlock(>control);
-   netif_wake_queue(netdev);
}
 
napi_enable(>napi);
+   netif_wake_queue(netdev);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
return 0;
-- 
2.7.4

[PATCH net v3 3/4] r8152: re-schedule napi for tx

[Hayes Wang]

Re-schedule napi after napi_complete() for tx, if it is necessay.

In r8152_poll(), if the tx is completed after tx_bottom() and before
napi_complete(), the scheduling of napi would be lost. Then, no
one handles the next tx until the next napi_schedule() is called.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ec882be..4785d2b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1936,6 +1936,9 @@ static int r8152_poll(struct napi_struct *napi, int 
budget)
napi_complete(napi);
if (!list_empty(>rx_done))
napi_schedule(napi);
+   else if (!skb_queue_empty(>tx_queue) &&
+!list_empty(>tx_free))
+   napi_schedule(napi);
}
 
return work_done;
-- 
2.7.4

RE: [PATCH net v2 3/4] r8152: re-schedule napi for tx

[Hayes Wang]

Eric Dumazet [mailto:eric.duma...@gmail.com]
> Sent: Wednesday, January 25, 2017 9:57 PM
[...]
> > napi_complete(napi);
> > if (!list_empty(>rx_done))
> > napi_schedule(napi);
> > +   else if (!skb_queue_empty(>tx_queue) &&
> > +!list_empty(>tx_free))
> > +   napi_schedule(>napi);
> 
> Why using >napi instead of napi here, as done 3 lines above ?

Oops. I would fix it. Thanks.

Best Regards,
Hayes

RE: NAPI on USB network drivers

[Hayes Wang]

Oliver Neukum [mailto:oneu...@suse.com]
> Sent: Wednesday, January 25, 2017 5:35 PM
[...]
> looking at r8152 I noticed that it uses NAPI. I never considered
> this for the generic USB networking code as you cannot disable
> interrupts for USB. Is it still worth it? What are the benefits?

You could use napi_gro_receive() and it influences the performance.

Best Regards,
Hayes

[PATCH net v2 0/4] r8152: fix scheduling napi

[Hayes Wang]

v2:
Add smp_mb__after_atomic() for patch #1.

v1:
Scheduling the napi during the following periods would let it be ignored.
And the events wouldn't be handled until next napi_schedule() is called.

1. after napi_disable and before napi_enable().
2. after all actions of napi function is completed and before calling
   napi_complete().

If no next napi_schedule() is called, tx or rx would stop working.

In order to avoid these situations, the followings solutions are applied.

1. prevent start_xmit() from calling napi_schedule() during runtime suspend
   or after napi_disable().
2. re-schedule the napi for tx if it is necessary.
3. check if any rx is finished or not after napi_enable().

Hayes Wang (4):
  r8152: avoid start_xmit to call napi_schedule during autosuspend
  r8152: avoid start_xmit to schedule napi when napi is disabled
  r8152: re-schedule napi for tx
  r8152: check rx after napi is enabled

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

-- 
2.7.4

[PATCH net v2 2/4] r8152: avoid start_xmit to schedule napi when napi is disabled

[Hayes Wang]

Stop the tx when the napi is disabled to prevent napi_schedule() is
called.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 23bef8e..ec882be 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3155,10 +3155,13 @@ static void set_carrier(struct r8152 *tp)
if (!netif_carrier_ok(netdev)) {
tp->rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, >flags);
+   netif_stop_queue(netdev);
napi_disable(>napi);
netif_carrier_on(netdev);
rtl_start_rx(tp);
napi_enable(>napi);
+   netif_wake_queue(netdev);
+   netif_info(tp, link, netdev, "carrier on\n");
}
} else {
if (netif_carrier_ok(netdev)) {
@@ -3166,6 +3169,7 @@ static void set_carrier(struct r8152 *tp)
napi_disable(>napi);
tp->rtl_ops.disable(tp);
napi_enable(>napi);
+   netif_info(tp, link, netdev, "carrier off\n");
}
}
 }
@@ -3515,12 +3519,12 @@ static int rtl8152_pre_reset(struct usb_interface *intf)
if (!netif_running(netdev))
return 0;
 
+   netif_stop_queue(netdev);
napi_disable(>napi);
clear_bit(WORK_ENABLE, >flags);
usb_kill_urb(tp->intr_urb);
cancel_delayed_work_sync(>schedule);
if (netif_carrier_ok(netdev)) {
-   netif_stop_queue(netdev);
mutex_lock(>control);
tp->rtl_ops.disable(tp);
mutex_unlock(>control);
@@ -3548,10 +3552,10 @@ static int rtl8152_post_reset(struct usb_interface 
*intf)
rtl_start_rx(tp);
rtl8152_set_rx_mode(netdev);
mutex_unlock(>control);
-   netif_wake_queue(netdev);
}
 
napi_enable(>napi);
+   netif_wake_queue(netdev);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
return 0;
-- 
2.7.4

[PATCH net v2 3/4] r8152: re-schedule napi for tx

[Hayes Wang]

Re-schedule napi after napi_complete() for tx, if it is necessay.

In r8152_poll(), if the tx is completed after tx_bottom() and before
napi_complete(), the scheduling of napi would be lost. Then, no
one handles the next tx until the next napi_schedule() is called.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ec882be..45d168e 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1936,6 +1936,9 @@ static int r8152_poll(struct napi_struct *napi, int 
budget)
napi_complete(napi);
if (!list_empty(>rx_done))
napi_schedule(napi);
+   else if (!skb_queue_empty(>tx_queue) &&
+!list_empty(>tx_free))
+   napi_schedule(>napi);
}
 
return work_done;
-- 
2.7.4

[PATCH net v2 1/4] r8152: avoid start_xmit to call napi_schedule during autosuspend

[Hayes Wang]

Adjust the setting of the flag of SELECTIVE_SUSPEND to prevent start_xmit()
from calling napi_schedule() directly during runtime suspend.

After calling napi_disable() or clearing the flag of WORK_ENABLE,
scheduling the napi is useless.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e1466b4..23bef8e 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3585,10 +3585,15 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
struct net_device *netdev = tp->netdev;
int ret = 0;
 
+   set_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
+
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
u32 rcr = 0;
 
if (delay_autosuspend(tp)) {
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
ret = -EBUSY;
goto out1;
}
@@ -3605,6 +3610,8 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
if (!(ocp_data & RXFIFO_EMPTY)) {
rxdy_gated_en(tp, false);
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
ret = -EBUSY;
goto out1;
}
@@ -3624,8 +3631,6 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
}
}
 
-   set_bit(SELECTIVE_SUSPEND, >flags);
-
 out1:
return ret;
 }
@@ -3681,12 +3686,13 @@ static int rtl8152_resume(struct usb_interface *intf)
if (netif_running(tp->netdev) && tp->netdev->flags & IFF_UP) {
if (test_bit(SELECTIVE_SUSPEND, >flags)) {
tp->rtl_ops.autosuspend_en(tp, false);
-   clear_bit(SELECTIVE_SUSPEND, >flags);
napi_disable(>napi);
set_bit(WORK_ENABLE, >flags);
if (netif_carrier_ok(tp->netdev))
rtl_start_rx(tp);
napi_enable(>napi);
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
} else {
tp->rtl_ops.up(tp);
netif_carrier_off(tp->netdev);
-- 
2.7.4

[PATCH net v2 4/4] r8152: check rx after napi is enabled

[Hayes Wang]

Schedule the napi after napi_enable() for rx, if it is necessary.

If the rx is completed when napi is disabled, the sheduling of napi
would be lost. Then, no one handles the rx packet until next napi
is scheduled.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 45d168e..8924520 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -32,7 +32,7 @@
 #define NETNEXT_VERSION"08"
 
 /* Information for net */
-#define NET_VERSION"7"
+#define NET_VERSION"8"
 
 #define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION
 #define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_s...@realtek.com>"
@@ -3561,6 +3561,9 @@ static int rtl8152_post_reset(struct usb_interface *intf)
netif_wake_queue(netdev);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
+   if (!list_empty(>rx_done))
+   napi_schedule(>napi);
+
return 0;
 }
 
@@ -3700,6 +3703,8 @@ static int rtl8152_resume(struct usb_interface *intf)
napi_enable(>napi);
clear_bit(SELECTIVE_SUSPEND, >flags);
smp_mb__after_atomic();
+   if (!list_empty(>rx_done))
+   napi_schedule(>napi);
} else {
tp->rtl_ops.up(tp);
netif_carrier_off(tp->netdev);
-- 
2.7.4

[PATCH net-next] r8152: fix the wrong spelling

[Hayes Wang]

Replace rumtime with runtime.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f3b48ad..d59d773 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3576,7 +3576,7 @@ static bool delay_autosuspend(struct r8152 *tp)
return false;
 }
 
-static int rtl8152_rumtime_suspend(struct r8152 *tp)
+static int rtl8152_runtime_suspend(struct r8152 *tp)
 {
struct net_device *netdev = tp->netdev;
int ret = 0;
@@ -3653,7 +3653,7 @@ static int rtl8152_suspend(struct usb_interface *intf, 
pm_message_t message)
mutex_lock(>control);
 
if (PMSG_IS_AUTO(message))
-   ret = rtl8152_rumtime_suspend(tp);
+   ret = rtl8152_runtime_suspend(tp);
else
ret = rtl8152_system_suspend(tp);
 
-- 
2.7.4

[PATCH net 4/4] r8152: check rx after napi is enabled

[Hayes Wang]

Schedule the napi after napi_enable() for rx, if it is necessary.

If the rx is completed when napi is disabled, the sheduling of napi
would be lost. Then, no one handles the rx packet until next napi
is scheduled.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f65109b..f0f55b3 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -32,7 +32,7 @@
 #define NETNEXT_VERSION"08"
 
 /* Information for net */
-#define NET_VERSION"7"
+#define NET_VERSION"8"
 
 #define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION
 #define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_s...@realtek.com>"
@@ -3561,6 +3561,9 @@ static int rtl8152_post_reset(struct usb_interface *intf)
netif_wake_queue(netdev);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
+   if (!list_empty(>rx_done))
+   napi_schedule(>napi);
+
return 0;
 }
 
@@ -3696,6 +3699,8 @@ static int rtl8152_resume(struct usb_interface *intf)
rtl_start_rx(tp);
napi_enable(>napi);
clear_bit(SELECTIVE_SUSPEND, >flags);
+   if (!list_empty(>rx_done))
+   napi_schedule(>napi);
} else {
tp->rtl_ops.up(tp);
netif_carrier_off(tp->netdev);
-- 
2.7.4

[PATCH net 3/4] r8152: re-schedule napi for tx

[Hayes Wang]

Re-schedule napi after napi_complete() for tx, if it is necessay.

In r8152_poll(), if the tx is completed after tx_bottom() and before
napi_complete(), the scheduling of napi would be lost. Then, no
one handles the next tx until the next napi_schedule() is called.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 3454238..f65109b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1936,6 +1936,9 @@ static int r8152_poll(struct napi_struct *napi, int 
budget)
napi_complete(napi);
if (!list_empty(>rx_done))
napi_schedule(napi);
+   else if (!skb_queue_empty(>tx_queue) &&
+!list_empty(>tx_free))
+   napi_schedule(>napi);
}
 
return work_done;
-- 
2.7.4

[PATCH net 0/4] r8152: fix scheduling napi

[Hayes Wang]

Scheduling the napi during the following periods would let it be ignored.
And the events wouldn't be handled until next napi_schedule() is called.

1. after napi_disable and before napi_enable().
2. after all actions of napi function is completed and before calling
   napi_complete().

If no next napi_schedule() is called, tx or rx would stop working.

In order to avoid these situations, the followings solutions are applied.

1. prevent start_xmit() from calling napi_schedule() during runtime suspend
   or after napi_disable().
2. re-schedule the napi for tx if it is necessary.
3. check if any rx is finished or not after napi_enable().

Hayes Wang (4):
  r8152: avoid start_xmit to call napi_schedule during autosuspend
  r8152: avoid start_xmit to schedule napi when napi is disabled
  r8152: re-schedule napi for tx
  r8152: check rx after napi is enabled

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

-- 
2.7.4

[PATCH net 2/4] r8152: avoid start_xmit to schedule napi when napi is disabled

[Hayes Wang]

Stop the tx when the napi is disabled to prevent napi_schedule() is
called.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 27b0b44..3454238 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3155,10 +3155,13 @@ static void set_carrier(struct r8152 *tp)
if (!netif_carrier_ok(netdev)) {
tp->rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, >flags);
+   netif_stop_queue(netdev);
napi_disable(>napi);
netif_carrier_on(netdev);
rtl_start_rx(tp);
napi_enable(>napi);
+   netif_wake_queue(netdev);
+   netif_info(tp, link, netdev, "carrier on\n");
}
} else {
if (netif_carrier_ok(netdev)) {
@@ -3166,6 +3169,7 @@ static void set_carrier(struct r8152 *tp)
napi_disable(>napi);
tp->rtl_ops.disable(tp);
napi_enable(>napi);
+   netif_info(tp, link, netdev, "carrier off\n");
}
}
 }
@@ -3515,12 +3519,12 @@ static int rtl8152_pre_reset(struct usb_interface *intf)
if (!netif_running(netdev))
return 0;
 
+   netif_stop_queue(netdev);
napi_disable(>napi);
clear_bit(WORK_ENABLE, >flags);
usb_kill_urb(tp->intr_urb);
cancel_delayed_work_sync(>schedule);
if (netif_carrier_ok(netdev)) {
-   netif_stop_queue(netdev);
mutex_lock(>control);
tp->rtl_ops.disable(tp);
mutex_unlock(>control);
@@ -3548,10 +3552,10 @@ static int rtl8152_post_reset(struct usb_interface 
*intf)
rtl_start_rx(tp);
rtl8152_set_rx_mode(netdev);
mutex_unlock(>control);
-   netif_wake_queue(netdev);
}
 
napi_enable(>napi);
+   netif_wake_queue(netdev);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
return 0;
-- 
2.7.4

[PATCH net 1/4] r8152: avoid start_xmit to call napi_schedule during autosuspend

[Hayes Wang]

Adjust the setting of the flag of SELECTIVE_SUSPEND to prevent start_xmit()
from calling napi_schedule() directly during runtime suspend.

After calling napi_disable() or clearing the flag of WORK_ENABLE,
scheduling the napi is useless.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e1466b4..27b0b44 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3585,10 +3585,13 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
struct net_device *netdev = tp->netdev;
int ret = 0;
 
+   set_bit(SELECTIVE_SUSPEND, >flags);
+
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
u32 rcr = 0;
 
if (delay_autosuspend(tp)) {
+   clear_bit(SELECTIVE_SUSPEND, >flags);
ret = -EBUSY;
goto out1;
}
@@ -3605,6 +3608,7 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
if (!(ocp_data & RXFIFO_EMPTY)) {
rxdy_gated_en(tp, false);
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
+   clear_bit(SELECTIVE_SUSPEND, >flags);
ret = -EBUSY;
goto out1;
}
@@ -3624,8 +3628,6 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
}
}
 
-   set_bit(SELECTIVE_SUSPEND, >flags);
-
 out1:
return ret;
 }
@@ -3681,12 +3683,12 @@ static int rtl8152_resume(struct usb_interface *intf)
if (netif_running(tp->netdev) && tp->netdev->flags & IFF_UP) {
if (test_bit(SELECTIVE_SUSPEND, >flags)) {
tp->rtl_ops.autosuspend_en(tp, false);
-   clear_bit(SELECTIVE_SUSPEND, >flags);
napi_disable(>napi);
set_bit(WORK_ENABLE, >flags);
if (netif_carrier_ok(tp->netdev))
rtl_start_rx(tp);
napi_enable(>napi);
+   clear_bit(SELECTIVE_SUSPEND, >flags);
} else {
tp->rtl_ops.up(tp);
netif_carrier_off(tp->netdev);
-- 
2.7.4

[PATCH net] r8152: don't execute runtime suspend if the tx is not empty

[Hayes Wang]

Runtime suspend shouldn't be executed if the tx queue is not empty,
because the device is not idle.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0e99af0..e1466b4 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -32,7 +32,7 @@
 #define NETNEXT_VERSION"08"
 
 /* Information for net */
-#define NET_VERSION"6"
+#define NET_VERSION"7"
 
 #define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION
 #define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_s...@realtek.com>"
@@ -3574,6 +3574,8 @@ static bool delay_autosuspend(struct r8152 *tp)
 */
if (!sw_linking && tp->rtl_ops.in_nway(tp))
return true;
+   else if (!skb_queue_empty(>tx_queue))
+   return true;
else
return false;
 }
-- 
2.7.4

[PATCH net] r8152: fix rtl8152_post_reset function

[Hayes Wang]

The rtl8152_post_reset() should sumbit rx urb and interrupt transfer,
otherwise the rx wouldn't work and the linking change couldn't be
detected.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f3b48ad..0e99af0 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3545,12 +3545,14 @@ static int rtl8152_post_reset(struct usb_interface 
*intf)
if (netif_carrier_ok(netdev)) {
mutex_lock(>control);
tp->rtl_ops.enable(tp);
+   rtl_start_rx(tp);
rtl8152_set_rx_mode(netdev);
mutex_unlock(>control);
netif_wake_queue(netdev);
}
 
napi_enable(>napi);
+   usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
return 0;
 }
-- 
2.7.4

RE: [PATCH net] r8152: fix the rx doesn't work

[Hayes Wang]

> Subject: [PATCH net] r8152: fix the rx doesn't work
> 
> The rtl8152_post_reset() doesn't submit the rx urb, so the rx wouldn't work.
> 
> Signed-off-by: Hayes Wang <hayesw...@realtek.com>

Excuse me. Please ignore this patch. I would submit another one.

[PATCH net] r8152: fix the rx doesn't work

[Hayes Wang]

The rtl8152_post_reset() doesn't submit the rx urb, so the rx wouldn't work.

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/r8152.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f3b48ad..e8f4f88 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3545,6 +3545,7 @@ static int rtl8152_post_reset(struct usb_interface *intf)
if (netif_carrier_ok(netdev)) {
mutex_lock(>control);
tp->rtl_ops.enable(tp);
+   rtl_start_rx(tp);
rtl8152_set_rx_mode(netdev);
mutex_unlock(>control);
netif_wake_queue(netdev);
-- 
2.7.4

[PATCH net] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Fix the hw rx checksum is always enabled, and the user couldn't switch
it to sw rx checksum.

Note that the RTL_VER_01 only support sw rx checksum only. Besides,
the hw rx checksum for RTL_VER_02 is disabled after
commit b9a321b48af4 ("r8152: Fix broken RX checksums."). Re-enable it.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index be41856..f3b48ad 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1730,7 +1730,7 @@ static u8 r8152_rx_csum(struct r8152 *tp, struct rx_desc 
*rx_desc)
u8 checksum = CHECKSUM_NONE;
u32 opts2, opts3;
 
-   if (tp->version == RTL_VER_01 || tp->version == RTL_VER_02)
+   if (!(tp->netdev->features & NETIF_F_RXCSUM))
goto return_result;
 
opts2 = le32_to_cpu(rx_desc->opts2);
@@ -4356,6 +4356,11 @@ static int rtl8152_probe(struct usb_interface *intf,
NETIF_F_HIGHDMA | NETIF_F_FRAGLIST |
NETIF_F_IPV6_CSUM | NETIF_F_TSO6;
 
+   if (tp->version == RTL_VER_01) {
+   netdev->features &= ~NETIF_F_RXCSUM;
+   netdev->hw_features &= ~NETIF_F_RXCSUM;
+   }
+
netdev->ethtool_ops = 
netif_set_gso_max_size(netdev, RTL_LIMITED_TSO_SIZE);
 
-- 
2.7.4

[PATCH net 0/2] r8152: fix autosuspend issue

[Hayes Wang]

Avoid rx is split into two parts when runtime suspend occurs.

Hayes Wang (2):
  r8152: split rtl8152_suspend function
  r8152: fix rx issue for runtime suspend

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

-- 
2.7.4

[PATCH net 1/2] r8152: split rtl8152_suspend function

[Hayes Wang]

Split rtl8152_suspend() into rtl8152_system_suspend() and
rtl8152_rumtime_suspend().

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 7dc6122..c5e6d88 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3576,39 +3576,62 @@ static bool delay_autosuspend(struct r8152 *tp)
return false;
 }
 
-static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
+static int rtl8152_rumtime_suspend(struct r8152 *tp)
 {
-   struct r8152 *tp = usb_get_intfdata(intf);
struct net_device *netdev = tp->netdev;
int ret = 0;
 
-   mutex_lock(>control);
-
-   if (PMSG_IS_AUTO(message)) {
-   if (netif_running(netdev) && delay_autosuspend(tp)) {
+   if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
+   if (delay_autosuspend(tp)) {
ret = -EBUSY;
goto out1;
}
 
-   set_bit(SELECTIVE_SUSPEND, >flags);
-   } else {
-   netif_device_detach(netdev);
+   clear_bit(WORK_ENABLE, >flags);
+   usb_kill_urb(tp->intr_urb);
+   napi_disable(>napi);
+   rtl_stop_rx(tp);
+   tp->rtl_ops.autosuspend_en(tp, true);
+   napi_enable(>napi);
}
 
+   set_bit(SELECTIVE_SUSPEND, >flags);
+
+out1:
+   return ret;
+}
+
+static int rtl8152_system_suspend(struct r8152 *tp)
+{
+   struct net_device *netdev = tp->netdev;
+   int ret = 0;
+
+   netif_device_detach(netdev);
+
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
clear_bit(WORK_ENABLE, >flags);
usb_kill_urb(tp->intr_urb);
napi_disable(>napi);
-   if (test_bit(SELECTIVE_SUSPEND, >flags)) {
-   rtl_stop_rx(tp);
-   tp->rtl_ops.autosuspend_en(tp, true);
-   } else {
-   cancel_delayed_work_sync(>schedule);
-   tp->rtl_ops.down(tp);
-   }
+   cancel_delayed_work_sync(>schedule);
+   tp->rtl_ops.down(tp);
napi_enable(>napi);
}
-out1:
+
+   return ret;
+}
+
+static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
+{
+   struct r8152 *tp = usb_get_intfdata(intf);
+   int ret;
+
+   mutex_lock(>control);
+
+   if (PMSG_IS_AUTO(message))
+   ret = rtl8152_rumtime_suspend(tp);
+   else
+   ret = rtl8152_system_suspend(tp);
+
mutex_unlock(>control);
 
return ret;
-- 
2.7.4

[PATCH net 2/2] r8152: fix rx issue for runtime suspend

[Hayes Wang]

Pause the rx and make sure the rx fifo is empty when the autosuspend
occurs.

If the rx data comes when the driver is canceling the rx urb, the host
controller would stop getting the data from the device and continue
it after next rx urb is submitted. That is, one continuing data is
split into two different urb buffers. That let the driver take the
data as a rx descriptor, and unexpected behavior happens.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index c5e6d88..be41856 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3582,17 +3582,42 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
int ret = 0;
 
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
+   u32 rcr = 0;
+
if (delay_autosuspend(tp)) {
ret = -EBUSY;
goto out1;
}
 
+   if (netif_carrier_ok(netdev)) {
+   u32 ocp_data;
+
+   rcr = ocp_read_dword(tp, MCU_TYPE_PLA, PLA_RCR);
+   ocp_data = rcr & ~RCR_ACPT_ALL;
+   ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, ocp_data);
+   rxdy_gated_en(tp, true);
+   ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA,
+PLA_OOB_CTRL);
+   if (!(ocp_data & RXFIFO_EMPTY)) {
+   rxdy_gated_en(tp, false);
+   ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
+   ret = -EBUSY;
+   goto out1;
+   }
+   }
+
clear_bit(WORK_ENABLE, >flags);
usb_kill_urb(tp->intr_urb);
-   napi_disable(>napi);
-   rtl_stop_rx(tp);
+
tp->rtl_ops.autosuspend_en(tp, true);
-   napi_enable(>napi);
+
+   if (netif_carrier_ok(netdev)) {
+   napi_disable(>napi);
+   rtl_stop_rx(tp);
+   rxdy_gated_en(tp, false);
+   ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
+   napi_enable(>napi);
+   }
}
 
set_bit(SELECTIVE_SUSPEND, >flags);
-- 
2.7.4

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Ansis Atteka [mailto:aatt...@nicira.com]
> Sent: Tuesday, January 03, 2017 8:41 AM
[...]
> Hayes, in your iperf reproduction environment did you
> 1) connect sender and receiver directly with an Ethernet cable?
> 2) use iperf's TCP mode instead of UDP mode, because I believe that
> with UDP mode packets are more likely to be sparsely distributed?
> Also, this bug is way easier to reproduce when IP fragmentation kicks
> in because IP fragments are typically sent out very close to each
> other.
> 3) were you plugging your USB Ethernet dongle in USB 3.0 port or
> whatever Mark was using? It seems that each USB mode has different
> coalesce parameters and yours might have work "out of box"?

Yes. I connect them directly and use iperf's TCP mode. However,
I test the RTL8152 which only support USB 2.0. Therefore, I don't
think it occurs with different coalesce parameters.

> While I would not call this a proper fix, because it simply reduces
> coalescing timeouts by order of 10X and most likely does not eliminate
> security aspects of the bug, it at least made my system functionally
> stable and I don't see either of those two bugs in my setup anymore:

Do you try commit a59e6d815226 ("r8152: correct the rx early size"),
or you have used it?

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord 

I find an issue about autosuspend, and it may result in the same
problem with you. I don't sure if this is helpful to you, because
it only occurs when enabling the autosuspend.

Best Regards,
Hayes

/*
 *  Copyright (c) 2014 Realtek Semiconductor Corp. All rights reserved.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * version 2 as published by the Free Software Foundation.
 *
 */

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

/* Information for net-next */
#define NETNEXT_VERSION "08"

/* Information for net */
#define NET_VERSION "2"

#define DRIVER_VERSION  "v1." NETNEXT_VERSION "." NET_VERSION
#define DRIVER_AUTHOR "Realtek linux nic maintainers "
#define DRIVER_DESC "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters"
#define MODULENAME "r8152"

#define R8152_PHY_ID32

#define PLA_IDR 0xc000
#define PLA_RCR 0xc010
#define PLA_RMS 0xc016
#define PLA_RXFIFO_CTRL00xc0a0
#define PLA_RXFIFO_CTRL10xc0a4
#define PLA_RXFIFO_CTRL20xc0a8
#define PLA_DMY_REG00xc0b0
#define PLA_FMC 0xc0b4
#define PLA_CFG_WOL 0xc0b6
#define PLA_TEREDO_CFG  0xc0bc
#define PLA_MAR 0xcd00
#define PLA_BACKUP  0xd000
#define PAL_BDC_CR  0xd1a0
#define PLA_TEREDO_TIMER0xd2cc
#define PLA_REALWOW_TIMER   0xd2e8
#define PLA_LEDSEL  0xdd90
#define PLA_LED_FEATURE 0xdd92
#define PLA_PHYAR   0xde00
#define PLA_BOOT_CTRL   0xe004
#define PLA_GPHY_INTR_IMR   0xe022
#define PLA_EEE_CR  0xe040
#define PLA_EEEP_CR 0xe080
#define PLA_MAC_PWR_CTRL0xe0c0
#define PLA_MAC_PWR_CTRL2   0xe0ca
#define PLA_MAC_PWR_CTRL3   0xe0cc
#define PLA_MAC_PWR_CTRL4   0xe0ce
#define PLA_WDT6_CTRL   0xe428
#define PLA_TCR00xe610
#define PLA_TCR10xe612
#define PLA_MTPS0xe615
#define PLA_TXFIFO_CTRL 0xe618
#define PLA_RSTTALLY0xe800
#define PLA_CR  0xe813
#define PLA_CRWECR  0xe81c
#define PLA_CONFIG120xe81e  /* CONFIG1, CONFIG2 */
#define PLA_CONFIG340xe820  /* CONFIG3, CONFIG4 */
#define PLA_CONFIG5 0xe822
#define PLA_PHY_PWR 0xe84c
#define PLA_OOB_CTRL0xe84f
#define PLA_CPCR0xe854
#define PLA_MISC_0  0xe858
#define PLA_MISC_1  0xe85a
#define PLA_OCP_GPHY_BASE   0xe86c
#define PLA_TALLYCNT0xe890
#define PLA_SFF_STS_7   0xe8de
#define PLA_PHYSTATUS   0xe908
#define PLA_BP_BA   0xfc26
#define PLA_BP_00xfc28
#define PLA_BP_10xfc2a
#define PLA_BP_20xfc2c
#define PLA_BP_30xfc2e
#define PLA_BP_40xfc30
#define PLA_BP_50xfc32
#define PLA_BP_60xfc34
#define PLA_BP_70xfc36
#define PLA_BP_EN   0xfc38

#define USB_USB2PHY 0xb41e
#define USB_SSPHYLINK2  0xb428
#define USB_U2P3_CTRL   0xb460
#define USB_CSR_DUMMY1  0xb464
#define USB_CSR_DUMMY2  0xb466
#define USB_DEV_STAT0xb808
#define USB_CONNECT_TIMER   0xcbf8
#define USB_BURST_SIZE  0xcfc0
#define USB_USB_CTRL0xd406
#define USB_PHY_CTRL0xd408
#define USB_TX_AGG  0xd40a
#define USB_RX_BUF_TH   0xd40c
#define USB_USB_TIMER   0xd428
#define USB_RX_EARLY_TIMEOUT0xd42c
#define USB_RX_EARLY_SIZE   0xd42e
#define USB_PM_CTRL_STATUS  0xd432
#define USB_TX_DMA  0xd434
#define USB_TOLERANCE   0xd490
#define USB_LPM_CTRL0xd41a
#define USB_UPS_CTRL0xd800
#define USB_MISC_0  0xd81a
#define USB_POWER_CUT   0xd80a
#define USB_AFE_CTRL2   0xd824
#define USB_WDT11_CTRL  0xe43c
#define USB_BP_BA   0xfc26
#define USB_BP_00xfc28
#define USB_BP_10xfc2a
#define USB_BP_20xfc2c
#define USB_BP_30xfc2e
#define USB_BP_40xfc30
#define USB_BP_50xfc32
#define USB_BP_60xfc34
#define USB_BP_70xfc36
#define USB_BP_EN   0xfc38

/* OCP Registers */
#define OCP_ALDPS_CONFIG0x2010
#define OCP_EEE_CONFIG1 0x2080
#define OCP_EEE_CONFIG2 0x2092
#define OCP_EEE_CONFIG3 0x2094
#define OCP_BASE_MII0xa400
#define OCP_EEE_AR  0xa41a
#define OCP_EEE_DATA0xa41c
#define OCP_PHY_STATUS  0xa420
#define 

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord 
[...]
> > Not sure why, because there really is no other way for the data to
> > appear where it does at the beginning of that URB buffer.
> >
> > This does seem a rather unexpected burden to place upon someone
> > reporting a regression in a USB network driver that corrupts user data.
> 
> If you are the only person who can actively reproduce this, which
> seems to be the case right now, this is unfortunately the only way to
> reach a proper analysis and fix.

I have tested it with iperf more than five days without any error.
I would think if there is any other way to reproduce it.

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Friday, November 25, 2016 3:11 AM
[...]
> On 16-11-24 02:00 PM, Greg KH wrote:
> > On Thu, Nov 24, 2016 at 01:34:08PM -0500, Mark Lord wrote:
> >> One thought:  bulk data streams are byte streams, not packets.
> >> Scheduling on the USB bus can break up larger transfers across
> >> multiple in-kernel buffers.  A "real" URB buffer on USB2 is max 512 bytes.
> >> The driver is providing 16384-byte buffers, and assumes that data will
> >> never spill over from one such buffer to the next.
> >> Yet the observations here consistently show otherwise.
> >
> > Wait, how do you know that data will not spill over?  What is making
> > that guarantee?  Will the USB device send a "zero packet" in order to
> > show that all of the "logical" data is now sent for this specific
> > endpoint?  Is there some sort of "framing" that the device does with the
> > USB data so that the driver "knows" where the end of packet is?
> 
> Exactly my point.
> 
> > Check the zero-packet stuff for this device, that's tripped up many a
> > USB driver writer over the years, myself included.
> 
> I haven't tripped over it myself, but only because we were careful
> to allow for such in the USB drivers I have worked on.
> 
> The r8152 driver just assumes it never happens.

What is the value of /sys/bus/usb/devices/.../power/control ?
Could you make sure it is "on" and try again?
Or you could call usb_disable_autosuspend() in probe().

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

> Mark Lord [mailto:ml...@pobox.com]
> > Sent: Friday, November 25, 2016 12:44 AM
> [...]
> > The bad data in this case is ASCII:
> >
> >  "SRC=m3400:/ TARGET=/m340"
> >
> > This data is what is seen in /run/mount/utab, a file that is read/written 
> > over NFS
> on
> > each boot.
> >
> >  "SRC=m3400:/ TARGET=/m3400 ROOT=/
> > ATTRS=nolock,addr=192.168.8.1\n"
> >
> > But how does this ASCII data end up at offset zero of the rx buffer??
> > Not possible -- this isn't even stale data, because only an rx_desc could
> > be at that offset in that buffer.
> >
> > So even if this were a platform memory coherency issue, one should still
> > never see ASCII data at the beginning of an rx buffer.  The driver NEVER
> > writes anything to the rx buffers.  Only the USB hardware ever does.
> >
> > And only the r8152 dongle/driver exhibits this issue.
> > Other USB dongles do not.  They *might* still have such issues,
> > but because they use software checksums, the bad packets are 
> > caught/rejected.
> 
> Do you test it by rebooting? Maybe you could try a patch
> commit 93fe9b183840 ("r8152: reset the bmu"). However, it should
> only occur for the first urb buffer after rx is reset. I don't
> think you would reset the rx frequently, so the situation seems
> to be different.

Forgive me. I provide wrong information. This is about RTL8153,
not RTL8152.

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Friday, November 25, 2016 12:44 AM
[...]
> The bad data in this case is ASCII:
> 
>  "SRC=m3400:/ TARGET=/m340"
> 
> This data is what is seen in /run/mount/utab, a file that is read/written 
> over NFS on
> each boot.
> 
>  "SRC=m3400:/ TARGET=/m3400 ROOT=/
> ATTRS=nolock,addr=192.168.8.1\n"
> 
> But how does this ASCII data end up at offset zero of the rx buffer??
> Not possible -- this isn't even stale data, because only an rx_desc could
> be at that offset in that buffer.
> 
> So even if this were a platform memory coherency issue, one should still
> never see ASCII data at the beginning of an rx buffer.  The driver NEVER
> writes anything to the rx buffers.  Only the USB hardware ever does.
> 
> And only the r8152 dongle/driver exhibits this issue.
> Other USB dongles do not.  They *might* still have such issues,
> but because they use software checksums, the bad packets are caught/rejected.

Do you test it by rebooting? Maybe you could try a patch
commit 93fe9b183840 ("r8152: reset the bmu"). However, it should
only occur for the first urb buffer after rx is reset. I don't
think you would reset the rx frequently, so the situation seems
to be different.

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Thursday, November 24, 2016 11:25 PM
[...]
> x86 has near fully-coherent memory, so it is the "easy" platform
> to get things working on.  But Linux supports a very diverse number
> of platforms, with varying degrees of cache/memory coherency,
> and it can be tricky for things to work correctly on all of them.

However, I have test iperf on raspberry pi v1 which you suggest
for more than one day. I still couldn't reproduce your issue.

> If you are testing with the driver as currently in 4.4.34,
> then you won't even notice when things are screwing up,
> because the driver just silently drops packets.
> Or it passes them on without noticing that they have bad data.

I only drop the packet silently when the rx descriptor outside
the urb buffer. Then, I check the rx descriptor before checking
the length of the packet.

> Here (attached) is the instrumented driver I am using here now.
> I suggest you use it or something similar when testing,
> and not the stock driver.

I would test it again with your driver.

[...]
> Also, unrelated, but inside r8152_submit_rx() there is this code:
> 
>  /* The rx would be stopped, so skip submitting */
>  if (test_bit(RTL8152_UNPLUG, >flags) ||
>  !test_bit(WORK_ENABLE, >flags)
> || !netif_carrier_ok(tp->netdev))
> return 0;
> 
> If that "return 0" statement is ever executed, doesn't it result
> in the loss/leak of a buffer?

They would be found back by calling rtl_start_rx(), when the rx
is restarted.

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Thursday, November 24, 2016 8:31 PM
[...]
> Nope.  Guard zones did not fix it, so it's probably not a prefetch issue.
> Oddly, adding a couple of memory barriers to specific places in the driver
> does help, A LOT.  Still not 100%, but it did pass 1800 reboot tests over 
> night
> with only three bad rx_desc's reported.
> 
> That's a new record here for the driver using kmalloc'd buffers,
> and put reliability on par with using non-cacheable buffers.
> 
> Any way we look at it though, the chip/driver are simply unreliable,
> and relying upon hardware checksums (which fail due to the driver
> looking at garbage rather than the checksum bits) leads to data corruption.

I don't think the garbage results from our driver or device.
If it is the issue about memory, I think the host driver ought
to deal with it, because it handles the DMA.

Besides, it doesn't seem to occur for all platforms. I have
tested the iperf more than 26 hours, and it still works fine.
I think I would get the same result on x86 or x86_64 platform.

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Wednesday, November 23, 2016 9:41 PM
[...] 
> >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);
> >}
> 
> How is ocp_data used by the hardware?
> Shouldn't the calculation also include sizeof(rx_desc) in there somewhere?

I check your question with our hw engineers, and you are right.
The size of rx descriptor should be calculated, too.

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Thursday, November 24, 2016 3:30 AM
[...]
> Worth repeating: other dongles we have tried, eg. those using the asix driver,
> do not cause us any troubles here.  Only the r8152 dongles do.

I couldn't tell you why you would see the problem. I have tested the
RTL8152 on raspberry pi platform with iperf more than 17 hours. And
I don't see any invalid rx descriptor. I don't think it really is the
issue about our hw.

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [ml...@pobox.com]
[...]
> What does this code do:

> >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);
> >}

This only works for RTL8153. However, what you use is RTL8152.
It is like delay completion. It is used to reduce the loading of CPU
by letting a transfer contain more data to reduce the number of
transfers.

> How is ocp_data used by the hardware?
> Shouldn't the calculation also include sizeof(rx_desc) in there somewhere?

The algorithm is from our hw engineers, and it should be 

   (agg_buf_sz - packet size) / 8

You could refer to commit a59e6d815226 ("r8152: correct the rx early size").

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Friday, November 18, 2016 8:03 PM
[..]
> How does the RTL8152 know that the limit is 16KB,
> rather than some other number?  Is this a hardwired number
> in the hardware, or is it a parameter that the software
> sends to the chip during initialization?

It is the limitation of the hardware.

> I have a USB analyzer, but it is difficult to figure out how
> to program an appropriate trigger point for the capture,
> since the problem (with 16KB URBs) takes minutes to hours
> or even days to trigger.

It is good. Our hw engineers real want it. Maybe you could send
a specific packet, and trigger it. You could allocate a skb and
fill the data which you prefer, and call

skb_queue_tail(>tx_queue, skb);

[...]
> The first issue is that a packet sometimes begins in one URB,
> and completes in the next URB, without an rx_desc at the start
> of the second URB.  This I have already reported earlier.

However, our hw engineer says it wouldn't happen. Our hw always
sends rx_desc + packet + padding. The hw wouldn't split it to
two or more transmission. That is why I wonder who does it.

> But the driver, as written, sometimes accesses bytes outside
> of the 16KB URB buffer, because it trusts the non-existent
> rx_desc in these cases, and also because it accesses bytes
> from the rx_desc without first checking whether there is
> sufficient remaining space in the URB to hold an rx_desc.

I think I check them. According to the followning code,

list_for_each_safe(cursor, next, _queue) {
struct rx_desc *rx_desc;
struct rx_agg *agg;
int len_used = 0;
struct urb *urb;
u8 *rx_data;

...

rx_desc = agg->head;
rx_data = agg->head;
len_used += sizeof(struct rx_desc); //<-- add the size of next 
rx_desc

while (urb->actual_length > len_used) {
struct net_device *netdev = tp->netdev;
struct net_device_stats *stats = >stats;
unsigned int pkt_len;
struct sk_buff *skb;

pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
if (pkt_len < ETH_ZLEN)
break;

len_used += pkt_len;
if (urb->actual_length < len_used)
break;

pkt_len -= CRC_SIZE;
rx_data += sizeof(struct rx_desc);

...

find_next_rx:
rx_data = rx_agg_align(rx_data + pkt_len + CRC_SIZE);
rx_desc = (struct rx_desc *)rx_data;
len_used = (int)(rx_data - (u8 *)agg->head);
len_used += sizeof(struct rx_desc); //<-- add the size 
of next rx_desc
}

submit:
...
}

The while loop would check if the next rx_desc is inside the urb
buffer, because the len_used includes the size of the next rx_desc.
Then, in the while loop, the len_used adds the packet size and check
with urb->actual_length again. These make sure the rx_desc and the
packet are inside the urb buffer. Except the urb->actual_length
is more than agg_buf_sz. However, I don't think it would happen.

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Thursday, November 17, 2016 9:42 PM
[...]
> What the above sample shows, is the URB transfer buffer ran out of space in 
> the
> middle
> of a packet, and the hardware then tried to just continue that same packet in 
> the
> next URB,
> without an rx_desc header inserted.  The r8152.c driver always assumes the URB
> buffer begins
> with an rx_desc, so of course this behaviour produces really weird effects, 
> and
> system crashes, etc..

The USB device wouldn't know the address and size of buffer. Only
the USB host controller knows. Therefore, the device sends the
data to host, and the host fills the memory. According to your
description, it seems the host splits the data from the device
into two different buffers (or URB transfers). I wonder if it would
occur. As far as I know, the host wouldn't allow the buffer size
less than the data length.

Our hw engineers need the log from the USB analyzer to confirm
what the device sends to the host. However, I don't think you
have USB analyzer to do this. I would try to reproduce the issue.
But, I am busy, so I don't think I would response quickly.

Besides, the maximum data length which the RTL8152 would send to
the host is 16KB. That is, if the agg_buf_sz is 16KB, the host
wouldn't split it. However, you still see problems for it.

[...]
> It is not clear to me how the chip decides when to forward an rx URB to the 
> host.
> If you could describe how that part works for us, then it would help in 
> further
> understanding why fast systems (eg. a PC) don't generally notice the issue,
> while much slower embedded systems do see the issue regularly.

The driver expects the rx buffer would be

rx_desc + a packet + padding to 8 alignment + 
rx_desc + a packet + padding to 8 alignment + ...

Therefore, when a urb transfer is completed, the driver parsers
the buffer by this way. After the buffer is handled, it would
be submitted to the host, until the transfer is completed again.
If the submitting fail, the driver would try again later. The
urb->actual_length means how much data the host fills. The drive
uses it to check the end of the data. The urb->status mean if
the transfer is successful. The driver submits the urb to the
host directly if the status is not successful.

Best Regards,
Hayes

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

[...]
> Fix the hw rx checksum is always enabled, and the user couldn't switch
> it to sw rx checksum.
> 
> Note that the RTL_VER_01 only supports sw rx checksum only. Besides,
> the hw rx checksum for RTL_VER_02 is disabled after
> commit b9a321b48af4 ("r8152: Fix broken RX checksums."). Re-enable it.

Excuse me. If I want to re-send this one patch, should I let
RTL_VER_02 use rx hw checksum?

Best Regards,
Hayes

RE: [PATCH net 2/2] r8152: rx descriptor check

[Hayes Wang]

Francois Romieu [mailto:rom...@fr.zoreil.com]
> Sent: Tuesday, November 15, 2016 9:11 AM
[...]
> If it was possible to get it wrong once, it should be possible to
> get it wrong twice, especially if some part of the hardware design
> is recycled. I don't mean anything else.

I agree with you. However, I have to let it could be reproduced
for confirming it.

Besides, the behavior is different for PCIe and USB device. There
is no action of DMA for USB device. It is done by the USB host
controller. And, the USB host controller wouldn't allow the device
sends a data which is more than the size of the buffer.

Best Regards,
Hayes

RE: [PATCH net 2/2] r8152: rx descriptor check

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Monday, November 14, 2016 4:34 AM
[...]
> Perhaps the driver
> is somehow accessing the buffer space again after doing usb_submit_urb()?
> That would certainly produce this kind of behaviour.

I don't think so. First, the driver only read the received buffer.
That is, the driver would not change (or write) the data. Second,
The driver would lose the point address of the received buffer
after submitting the urb to the USB host controller, until the
transfer is completed by the USB host controller. That is, the
driver doesn't how to access the buffer after calling usb_submit_urb().

Best Regards,
Hayes

RE: [PATCH net 2/2] r8152: rx descriptor check

[Hayes Wang]

David Miller [mailto:da...@davemloft.net]
> Sent: Monday, November 14, 2016 1:40 AM
[...]
> If you add this patch now, there is a much smaller likelyhood that you
> will work with a high priority to figure out _why_ this is happening.
> 
> For all we know this could be a platform bug in the DMA API for the
> systems in question.
> 
> It could also be a bug elsewhere in the driver, either in setting up
> the descriptor DMA mappings or how the chip is programmed.
> 
> Either way the true cause must be found before we start throwing
> changes like this into the driver.

Our hw engineer could check our device, and I could check the
driver. However, for the other parts, such as the USB host
controller or memory, it is difficult for me to make sure whether
they are correct or not. I could only promise our devices and
driver work fine.

Best Regards,
Hayes

RE: [PATCH net 2/2] r8152: rx descriptor check

[Hayes Wang]

Francois Romieu [mailto:rom...@fr.zoreil.com]
> Sent: Friday, November 11, 2016 8:13 PM
[...]
> Invalid packet size corrupted receive descriptors in Realtek's device
> reminds of CVE-2009-4537.

Do you mean that the driver would get a packet exceed the size
which is set to RxMaxSize? I check it with our hw engineers.
They don't get any issue about RxMaxSize. And their test for
RxMaxSize register is fine.

> Is the silicium of both devices different enough to prevent the same
> exploit to happen ?

For this case, I don't think the device provide a invalid value
for the receive descriptors. However, the driver sees a different
value. That is why I say the memory is unbelievable.

Best Regards,
Hayes

[PATCH net 0/2] r8152: rx patches

[Hayes Wang]

Let the rx sw checksum available and add some checks for rx desc.

Hayes Wang (2):
  r8152: fix the sw rx checksum is unavailable
  r8152: rx descriptor check

 drivers/net/usb/r8152.c | 46 +-
 1 file changed, 45 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH net 2/2] r8152: rx descriptor check

[Hayes Wang]

For some platforms, the data in memory is not the same with the one
from the device. That is, the data of memory is unbelievable. The
check is used to find out this situation.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0e42a78..e766121 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1756,6 +1756,43 @@ static u8 r8152_rx_csum(struct r8152 *tp, struct rx_desc 
*rx_desc)
return checksum;
 }
 
+static int invalid_rx_desc(struct r8152 *tp, struct rx_desc *rx_desc)
+{
+   u32 opts1 = le32_to_cpu(rx_desc->opts1);
+   u32 opts2 = le32_to_cpu(rx_desc->opts2);
+   unsigned int pkt_len = opts1 & RX_LEN_MASK;
+
+   switch (tp->version) {
+   case RTL_VER_01:
+   case RTL_VER_02:
+   if (pkt_len > RTL8152_RMS)
+   return -EIO;
+   break;
+   default:
+   if (pkt_len > RTL8153_RMS)
+   return -EIO;
+   break;
+   }
+
+   switch (opts2 & (RD_IPV4_CS | RD_IPV6_CS)) {
+   case (RD_IPV4_CS | RD_IPV6_CS):
+   return -EIO;
+   case RD_IPV4_CS:
+   case RD_IPV6_CS:
+   switch (opts2 & (RD_UDP_CS | RD_TCP_CS)) {
+   case (RD_UDP_CS | RD_TCP_CS):
+   return -EIO;
+   default:
+   break;
+   }
+   break;
+   default:
+   break;
+   }
+
+   return 0;
+}
+
 static int rx_bottom(struct r8152 *tp, int budget)
 {
unsigned long flags;
@@ -1812,6 +1849,18 @@ static int rx_bottom(struct r8152 *tp, int budget)
unsigned int pkt_len;
struct sk_buff *skb;
 
+   if (unlikely(invalid_rx_desc(tp, rx_desc))) {
+   if (net_ratelimit())
+   netif_err(tp, rx_err, netdev,
+ "Memory unbelievable\n");
+   if (tp->netdev->features & NETIF_F_RXCSUM) {
+   tp->netdev->features &= ~NETIF_F_RXCSUM;
+   netif_err(tp, rx_err, netdev,
+ "rx checksum off\n");
+   }
+   break;
+   }
+
pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
if (pkt_len < ETH_ZLEN)
break;
-- 
2.7.4

[PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Fix the hw rx checksum is always enabled, and the user couldn't switch
it to sw rx checksum.

Note that the RTL_VER_01 only supports sw rx checksum only. Besides,
the hw rx checksum for RTL_VER_02 is disabled after
commit b9a321b48af4 ("r8152: Fix broken RX checksums."). Re-enable it.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 75c5168..0e42a78 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1730,7 +1730,7 @@ static u8 r8152_rx_csum(struct r8152 *tp, struct rx_desc 
*rx_desc)
u8 checksum = CHECKSUM_NONE;
u32 opts2, opts3;
 
-   if (tp->version == RTL_VER_01 || tp->version == RTL_VER_02)
+   if (!(tp->netdev->features & NETIF_F_RXCSUM))
goto return_result;
 
opts2 = le32_to_cpu(rx_desc->opts2);
@@ -4307,6 +4307,11 @@ static int rtl8152_probe(struct usb_interface *intf,
NETIF_F_HIGHDMA | NETIF_F_FRAGLIST |
NETIF_F_IPV6_CSUM | NETIF_F_TSO6;
 
+   if (tp->version == RTL_VER_01) {
+   netdev->features &= ~NETIF_F_RXCSUM;
+   netdev->hw_features &= ~NETIF_F_RXCSUM;
+   }
+
netdev->ethtool_ops = 
netif_set_gso_max_size(netdev, RTL_LIMITED_TSO_SIZE);
 
-- 
2.7.4

RE: [PATCH net] r8152: Fix broken RX checksums.

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Friday, November 04, 2016 9:50 PM
[...]
> Yeah, the device or driver is definitely getting confused with rx_desc 
> structures.
> I added code to check for unlikely rx_desc values, and it found this for 
> starters:
> 
> rx_desc: 00480801 00480401 00480001 0048fc00 0048f800 0048f400
> pkt_len=2045
> rx_data: 00 f0 48 00 00 ec 48 00 00 e8 48 00 00 e4 48 00 00 e0 48 00 00 dc 48 
> 00
> 00 d8 48 00 00 d4 48 00
> rx_data: 00 d0 48 00 00 cc 48 00 00 c8 48 00 00 c4 48 00 00 c0 48 00 00 bc 48 
> 00
> 00 b8 48 00 00 b4 48 00
> rx_data: 00 b0 48 00 00 ac 48 00 00 01 00 00 81 ed 00 00 00 01 00 00 00 00 00 
> 00
> 00 00 00 02 4d ac 00 00
> rx_data: 10 00 ff ff ff ff 00 00 01 28 83 d6 ff 6d 00 20 25 b1 58 1b 68 ff 00 
> 05 20 01
> 56 41 17 35 00 00
> ...
> 
> The MTU/MRU on this link is the standard 1500 bytes, so a pkt_len of 2045 
> isn't
> valid here.
> And the rx_desc values look an awful lot like the rx_data values that follow 
> it.
> 
> There's definitely more broken here than just TCP RX checksums.

I don't think it is the issue of our hw. If it happens, windows or
other OS may have problems, too. It is like the memory issue described
in commit 990c9b347245("Merge branch 'r8152-fixes'"). It seems that
the data in memory is not same with the one from the device.

Besides, I test the raspberry pi with RTL8152. However, I don't find
any checksum issue for TCP. I try to copy a large file and md5sum it
through NFS. It works fine.

Best Regards,
Hayes

RE: [PATCH net] r8152: Fix broken RX checksums.

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Thursday, November 03, 2016 2:30 AM
> To: Hayes Wang; David Miller
[...]
> I have poked at it some more, and thus far it appears that it is
> only necessary to disable TCP rx checksums.  The system doesn't crash
> when only IP/UDP checksums are enabled, but does when TCP checksums are on.
> 
> This happens regardless of whether RX_AGG is disabled or enabled,
> and increasing/decreasing the number of RX URBs (RTL8152_MAX_RX)
> doesn't seem to affect it.

I test Raspberry Pi v1, but I couldn't boot with NFSROOT through
both onboard nic and RTL8152. I get following error.

   VFS: Unable to mount root fs via NFS, trying floppy.
   Kernel panic - not syncing: VFS: Unable to mount root fs on 
unknown-block(2,0)

However, if I start the system without NFSROOT, I could mount the nfs fs.
Any idea?

NFS server: Fedora 24
Raspberry Pi OS: 2016-09-23-raspbian-jessie

Content of /etc/exports:

   /nfsexport *(rw,sync,no_root_squash)

I change the cmdline.txt from

   dwc_otg.lpm_enable=0 console=serial0,115200 console=tty1
   root=/dev/mmcblk0p2 rootfstype=ext4 elevator=deadline
   fsck.repair=yes rootwait quiet splash plymouth.ignore-serial-consoles

to

   dwc_otg.lpm_enable=0 console=serial0,115200 console=tty1
   root=/dev/nfs nfsroot=192.168.94.2:/nfsexport ip=192.168.94.22
   rw rootwait quiet splash plymouth.ignore-serial-consoles

Best Regards,
Hayes

RE: [PATCH net] r8152: Fix broken RX checksums.

[Hayes Wang]

> >>> The r8152 driver has been broken since (approx) 3.16.xx
> >>> when support was added for hardware RX checksums
> >>> on newer chip versions.  Symptoms include random
> >>> segfaults and silent data corruption over NFS.
> >>>
> >>> The hardware checksum logig does not work on the VER_02
> >>> dongles I have here when used with a slow embedded system CPU.
> >>> Google reveals others reporting similar issues on Raspberry Pi.
> >>>
> >>> So, disable hardware RX checksum support for VER_02, and fix
> >>> an obvious coding error for IPV6 checksums in the same function.
> >>>
> >>> Because this bug results in silent data corruption,
> >>> it is a good candidate for back-porting to -stable >= 3.16.xx.
> >>>
> >>> Signed-off-by: Mark Lord <ml...@pobox.com>
> >>
> >> Applied and queued up for -stable, thanks.
> >
> > Thanks.  Now that this is taken care of, I do wonder if perhaps
> > RX checksums ought to be enabled at all for ANY versions of this chip?
> 
> You should really start a dialogue with the developer who has been
> making the most, if not all, of the major changes to this driver over
> the past few years, Hayes Wang.

Our hw engineer says only VER_01 has the issue about rx checksum.
I need more information for checking it.

Best Regards,
Hayes

[PATCH net-next] r8152: add new products of Lenovo

[Hayes Wang]

Add the following four products of Lenovo and sort the order of the list.

VID PID
0x17ef  0x3062
0x17ef  0x3069
0x17ef  0x720c
0x17ef  0x7214

Signed-off-by: Hayes Wang <hayesw...@realtek.com>
---
 drivers/net/usb/cdc_ether.c | 28 
 drivers/net/usb/r8152.c |  6 +-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index c47ec0a..45e5e43 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -687,6 +687,20 @@ static const struct usb_device_id  products[] = {
.driver_info = 0,
 },
 
+/* ThinkPad USB-C Dock (based on Realtek RTL8153) */
+{
+   USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x3062, USB_CLASS_COMM,
+   USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+   .driver_info = 0,
+},
+
+/* ThinkPad Thunderbolt 3 Dock (based on Realtek RTL8153) */
+{
+   USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x3069, USB_CLASS_COMM,
+   USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+   .driver_info = 0,
+},
+
 /* Lenovo Thinkpad USB 3.0 Ethernet Adapters (based on Realtek RTL8153) */
 {
USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x7205, USB_CLASS_COMM,
@@ -694,6 +708,20 @@ static const struct usb_device_id  products[] = {
.driver_info = 0,
 },
 
+/* Lenovo USB C to Ethernet Adapter (based on Realtek RTL8153) */
+{
+   USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x720c, USB_CLASS_COMM,
+   USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+   .driver_info = 0,
+},
+
+/* Lenovo USB-C Travel Hub (based on Realtek RTL8153) */
+{
+   USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x7214, USB_CLASS_COMM,
+   USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+   .driver_info = 0,
+},
+
 /* NVIDIA Tegra USB 3.0 Ethernet Adapters (based on Realtek RTL8153) */
 {
USB_DEVICE_AND_INTERFACE_INFO(NVIDIA_VENDOR_ID, 0x09ff, USB_CLASS_COMM,
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2886946..8d6e13c 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -4411,8 +4411,12 @@ static struct usb_device_id rtl8152_table[] = {
{REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8152)},
{REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8153)},
{REALTEK_USB_DEVICE(VENDOR_ID_SAMSUNG, 0xa101)},
-   {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7205)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x304f)},
+   {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x3062)},
+   {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x3069)},
+   {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7205)},
+   {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x720c)},
+   {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7214)},
{REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA,  0x09ff)},
{}
 };
-- 
2.7.4