2013-08-16 08:11:04

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 1/7] r8152: remove clearing the memory to zero for netdev priv

Remove memset(tp, 0, sizeof(*tp));

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ef2498c..c13662b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2105,7 +2105,6 @@ static int rtl8152_probe(struct usb_interface *intf,

SET_NETDEV_DEV(netdev, &intf->dev);
tp = netdev_priv(netdev);
- memset(tp, 0, sizeof(*tp));
tp->msg_enable = 0x7FFF;

tasklet_init(&tp->tl, bottom_half, (unsigned long)tp);
--
1.8.3.1


2013-08-16 08:10:57

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 2/7] r8152: replace void * with struct r8152 *

Change the type of contex of tx_agg and rx_agg from void * to
staruc r8152 *.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index c13662b..a18f02d 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -329,10 +329,12 @@ struct tx_desc {
#define IPV6_CS (1 << 28) /* Calculate IPv6 checksum */
};

+struct r8152;
+
struct rx_agg {
struct list_head list;
struct urb *urb;
- void *context;
+ struct r8152 *context;
void *buffer;
void *head;
};
@@ -340,7 +342,7 @@ struct rx_agg {
struct tx_agg {
struct list_head list;
struct urb *urb;
- void *context;
+ struct r8152 *context;
void *buffer;
void *head;
u32 skb_num;
--
1.8.3.1

2013-08-16 08:10:52

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 3/7] r8152: replace lockflags with flags

Replace lockflags with flags.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index a18f02d..41b99ce 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -769,7 +769,7 @@ static struct net_device_stats *rtl8152_get_stats(struct net_device *dev)
static void read_bulk_callback(struct urb *urb)
{
struct net_device *netdev;
- unsigned long lockflags;
+ unsigned long flags;
int status = urb->status;
struct rx_agg *agg;
struct r8152 *tp;
@@ -798,9 +798,9 @@ static void read_bulk_callback(struct urb *urb)
if (urb->actual_length < ETH_ZLEN)
break;

- spin_lock_irqsave(&tp->rx_lock, lockflags);
+ spin_lock_irqsave(&tp->rx_lock, flags);
list_add_tail(&agg->list, &tp->rx_done);
- spin_unlock_irqrestore(&tp->rx_lock, lockflags);
+ spin_unlock_irqrestore(&tp->rx_lock, flags);
tasklet_schedule(&tp->tl);
return;
case -ESHUTDOWN:
@@ -821,9 +821,9 @@ static void read_bulk_callback(struct urb *urb)
if (result == -ENODEV) {
netif_device_detach(tp->netdev);
} else if (result) {
- spin_lock_irqsave(&tp->rx_lock, lockflags);
+ spin_lock_irqsave(&tp->rx_lock, flags);
list_add_tail(&agg->list, &tp->rx_done);
- spin_unlock_irqrestore(&tp->rx_lock, lockflags);
+ spin_unlock_irqrestore(&tp->rx_lock, flags);
tasklet_schedule(&tp->tl);
}
}
@@ -831,7 +831,7 @@ static void read_bulk_callback(struct urb *urb)
static void write_bulk_callback(struct urb *urb)
{
struct net_device_stats *stats;
- unsigned long lockflags;
+ unsigned long flags;
struct tx_agg *agg;
struct r8152 *tp;
int status = urb->status;
@@ -853,9 +853,9 @@ static void write_bulk_callback(struct urb *urb)
stats->tx_bytes += agg->skb_len;
}

- spin_lock_irqsave(&tp->tx_lock, lockflags);
+ spin_lock_irqsave(&tp->tx_lock, flags);
list_add_tail(&agg->list, &tp->tx_free);
- spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+ spin_unlock_irqrestore(&tp->tx_lock, flags);

if (!netif_carrier_ok(tp->netdev))
return;
@@ -1119,7 +1119,7 @@ static void rx_bottom(struct r8152 *tp)
struct net_device *netdev;
struct rx_agg *agg;
struct rx_desc *rx_desc;
- unsigned long lockflags;
+ unsigned long flags;
struct list_head *cursor, *next;
struct sk_buff *skb;
struct urb *urb;
@@ -1132,16 +1132,16 @@ static void rx_bottom(struct r8152 *tp)

stats = rtl8152_get_stats(netdev);

- spin_lock_irqsave(&tp->rx_lock, lockflags);
+ spin_lock_irqsave(&tp->rx_lock, flags);
list_for_each_safe(cursor, next, &tp->rx_done) {
list_del_init(cursor);
- spin_unlock_irqrestore(&tp->rx_lock, lockflags);
+ spin_unlock_irqrestore(&tp->rx_lock, flags);

agg = list_entry(cursor, struct rx_agg, list);
urb = agg->urb;
if (urb->actual_length < ETH_ZLEN) {
ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
- spin_lock_irqsave(&tp->rx_lock, lockflags);
+ spin_lock_irqsave(&tp->rx_lock, flags);
if (ret && ret != -ENODEV) {
list_add_tail(&agg->list, next);
tasklet_schedule(&tp->tl);
@@ -1182,13 +1182,13 @@ static void rx_bottom(struct r8152 *tp)
}

ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
- spin_lock_irqsave(&tp->rx_lock, lockflags);
+ spin_lock_irqsave(&tp->rx_lock, flags);
if (ret && ret != -ENODEV) {
list_add_tail(&agg->list, next);
tasklet_schedule(&tp->tl);
}
}
- spin_unlock_irqrestore(&tp->rx_lock, lockflags);
+ spin_unlock_irqrestore(&tp->rx_lock, flags);
}

static void tx_bottom(struct r8152 *tp)
@@ -1196,7 +1196,7 @@ static void tx_bottom(struct r8152 *tp)
struct net_device_stats *stats;
struct net_device *netdev;
struct tx_agg *agg;
- unsigned long lockflags;
+ unsigned long flags;
u32 remain, total;
u8 *tx_data;
int res;
@@ -1205,7 +1205,7 @@ static void tx_bottom(struct r8152 *tp)

next_agg:
agg = NULL;
- spin_lock_irqsave(&tp->tx_lock, lockflags);
+ spin_lock_irqsave(&tp->tx_lock, flags);
if (!skb_queue_empty(&tp->tx_queue) && !list_empty(&tp->tx_free)) {
struct list_head *cursor;

@@ -1213,7 +1213,7 @@ next_agg:
list_del_init(cursor);
agg = list_entry(cursor, struct tx_agg, list);
}
- spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+ spin_unlock_irqrestore(&tp->tx_lock, flags);

if (!agg)
return;
@@ -1268,9 +1268,9 @@ next_agg:
netif_warn(tp, tx_err, netdev,
"failed tx_urb %d\n", res);
stats->tx_dropped += agg->skb_num;
- spin_lock_irqsave(&tp->tx_lock, lockflags);
+ spin_lock_irqsave(&tp->tx_lock, flags);
list_add_tail(&agg->list, &tp->tx_free);
- spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+ spin_unlock_irqrestore(&tp->tx_lock, flags);
}
return;
}
@@ -1373,7 +1373,7 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
{
struct r8152 *tp = netdev_priv(netdev);
struct net_device_stats *stats = rtl8152_get_stats(netdev);
- unsigned long lockflags;
+ unsigned long flags;
struct tx_agg *agg = NULL;
struct tx_desc *tx_desc;
unsigned int len;
@@ -1382,7 +1382,7 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,

skb_tx_timestamp(skb);

- spin_lock_irqsave(&tp->tx_lock, lockflags);
+ spin_lock_irqsave(&tp->tx_lock, flags);
if (!list_empty(&tp->tx_free) && skb_queue_empty(&tp->tx_queue)) {
struct list_head *cursor;

@@ -1390,7 +1390,7 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
list_del_init(cursor);
agg = list_entry(cursor, struct tx_agg, list);
}
- spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+ spin_unlock_irqrestore(&tp->tx_lock, flags);

if (!agg) {
skb_queue_tail(&tp->tx_queue, skb);
@@ -1419,9 +1419,9 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
netif_warn(tp, tx_err, netdev,
"failed tx_urb %d\n", res);
stats->tx_dropped++;
- spin_lock_irqsave(&tp->tx_lock, lockflags);
+ spin_lock_irqsave(&tp->tx_lock, flags);
list_add_tail(&agg->list, &tp->tx_free);
- spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+ spin_unlock_irqrestore(&tp->tx_lock, flags);
}
}

--
1.8.3.1

2013-08-16 08:11:39

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 7/7] r8152: add comments

Add comments.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 825edfe..f3fce41 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -790,6 +790,9 @@ static void read_bulk_callback(struct urb *urb)
return;

netdev = tp->netdev;
+
+ /* When link down, the driver would cancel all bulks. */
+ /* This avoid the re-submitting bulk */
if (!netif_carrier_ok(netdev))
return;

@@ -1296,6 +1299,8 @@ static void bottom_half(unsigned long data)
if (!test_bit(WORK_ENABLE, &tp->flags))
return;

+ /* When link down, the driver would cancel all bulks. */
+ /* This avoid the re-submitting bulk */
if (!netif_carrier_ok(tp->netdev))
return;

--
1.8.3.1

2013-08-16 08:11:47

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 5/7] r8152: move some declearation of variables

Move some declearation of variables in rx_bottom().

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 4dee76b..0a88f64 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1133,25 +1133,19 @@ r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc, struct sk_buff *skb)

static void rx_bottom(struct r8152 *tp)
{
- struct net_device_stats *stats;
- struct net_device *netdev;
- struct rx_agg *agg;
- struct rx_desc *rx_desc;
unsigned long flags;
struct list_head *cursor, *next;
- struct sk_buff *skb;
- struct urb *urb;
- unsigned pkt_len;
- int len_used;
- u8 *rx_data;
- int ret;
-
- netdev = tp->netdev;
-
- stats = rtl8152_get_stats(netdev);

spin_lock_irqsave(&tp->rx_lock, flags);
list_for_each_safe(cursor, next, &tp->rx_done) {
+ struct rx_desc *rx_desc;
+ struct rx_agg *agg;
+ unsigned pkt_len;
+ int len_used = 0;
+ struct urb *urb;
+ u8 *rx_data;
+ int ret;
+
list_del_init(cursor);
spin_unlock_irqrestore(&tp->rx_lock, flags);

@@ -1160,16 +1154,21 @@ static void rx_bottom(struct r8152 *tp)
if (urb->actual_length < ETH_ZLEN)
goto submit;

- len_used = 0;
rx_desc = agg->head;
rx_data = agg->head;
pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
len_used += sizeof(struct rx_desc) + pkt_len;

while (urb->actual_length >= len_used) {
+ struct net_device *netdev = tp->netdev;
+ struct net_device_stats *stats;
+ struct sk_buff *skb;
+
if (pkt_len < ETH_ZLEN)
break;

+ stats = rtl8152_get_stats(netdev);
+
pkt_len -= 4; /* CRC */
rx_data += sizeof(struct rx_desc);

--
1.8.3.1

2013-08-16 08:11:54

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 6/7] r8152: adjust tx_bottom function

Split some parts of code into another function to simplify
tx_bottom(). Use while loop to replace the goto loop.

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0a88f64..825edfe 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1131,6 +1131,51 @@ r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc, struct sk_buff *skb)
}
}

+static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
+{
+ u32 remain;
+ u8 *tx_data;
+
+ tx_data = agg->head;
+ agg->skb_num = agg->skb_len = 0;
+ remain = rx_buf_sz - sizeof(struct tx_desc);
+
+ while (remain >= ETH_ZLEN) {
+ struct tx_desc *tx_desc;
+ struct sk_buff *skb;
+ unsigned int len;
+
+ skb = skb_dequeue(&tp->tx_queue);
+ if (!skb)
+ break;
+
+ len = skb->len;
+ if (remain < len) {
+ skb_queue_head(&tp->tx_queue, skb);
+ break;
+ }
+
+ tx_desc = (struct tx_desc *)tx_data;
+ tx_data += sizeof(*tx_desc);
+
+ r8152_tx_csum(tp, tx_desc, skb);
+ memcpy(tx_data, skb->data, len);
+ agg->skb_num++;
+ agg->skb_len += len;
+ dev_kfree_skb_any(skb);
+
+ tx_data = tx_agg_align(tx_data + len);
+ remain = rx_buf_sz - sizeof(*tx_desc) -
+ (u32)((void *)tx_data - agg->head);
+ }
+
+ usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2),
+ agg->head, (int)(tx_data - (u8 *)agg->head),
+ (usb_complete_t)write_bulk_callback, agg);
+
+ return usb_submit_urb(agg->urb, GFP_ATOMIC);
+}
+
static void rx_bottom(struct r8152 *tp)
{
unsigned long flags;
@@ -1204,82 +1249,39 @@ submit:

static void tx_bottom(struct r8152 *tp)
{
- struct net_device_stats *stats;
- struct net_device *netdev;
- struct tx_agg *agg;
- unsigned long flags;
- u32 remain, total;
- u8 *tx_data;
int res;

- netdev = tp->netdev;
-
-next_agg:
- agg = NULL;
- if (skb_queue_empty(&tp->tx_queue))
- return;
+ do {
+ struct tx_agg *agg;

- agg = r8152_get_tx_agg(tp);
- if (!agg)
- return;
-
- tx_data = agg->head;
- agg->skb_num = agg->skb_len = 0;
- remain = rx_buf_sz - sizeof(struct tx_desc);
- total = 0;
-
- while (remain >= ETH_ZLEN) {
- struct tx_desc *tx_desc;
- struct sk_buff *skb;
- unsigned int len;
-
- skb = skb_dequeue(&tp->tx_queue);
- if (!skb)
+ if (skb_queue_empty(&tp->tx_queue))
break;

- len = skb->len;
- if (remain < len) {
- skb_queue_head(&tp->tx_queue, skb);
+ agg = r8152_get_tx_agg(tp);
+ if (!agg)
break;
- }
-
- tx_data = tx_agg_align(tx_data);
- tx_desc = (struct tx_desc *)tx_data;
- tx_data += sizeof(*tx_desc);

- r8152_tx_csum(tp, tx_desc, skb);
- memcpy(tx_data, skb->data, len);
- agg->skb_num++;
- agg->skb_len += len;
- dev_kfree_skb_any(skb);
-
- tx_data += len;
- remain = rx_buf_sz - sizeof(*tx_desc) -
- (u32)(tx_agg_align(tx_data) - agg->head);
- }
-
- usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2),
- agg->head, (int)(tx_data - (u8 *)agg->head),
- (usb_complete_t)write_bulk_callback, agg);
- res = usb_submit_urb(agg->urb, GFP_ATOMIC);
+ res = r8152_tx_agg_fill(tp, agg);
+ if (res) {
+ struct net_device_stats *stats;
+ struct net_device *netdev;
+ unsigned long flags;

- stats = rtl8152_get_stats(netdev);
+ netdev = tp->netdev;
+ stats = rtl8152_get_stats(netdev);

- if (res) {
- /* Can we get/handle EPIPE here? */
- if (res == -ENODEV) {
- netif_device_detach(netdev);
- } else {
- netif_warn(tp, tx_err, netdev,
- "failed tx_urb %d\n", res);
- stats->tx_dropped += agg->skb_num;
- spin_lock_irqsave(&tp->tx_lock, flags);
- list_add_tail(&agg->list, &tp->tx_free);
- spin_unlock_irqrestore(&tp->tx_lock, flags);
+ if (res == -ENODEV) {
+ netif_device_detach(netdev);
+ } else {
+ netif_warn(tp, tx_err, netdev,
+ "failed tx_urb %d\n", res);
+ stats->tx_dropped += agg->skb_num;
+ spin_lock_irqsave(&tp->tx_lock, flags);
+ list_add_tail(&agg->list, &tp->tx_free);
+ spin_unlock_irqrestore(&tp->tx_lock, flags);
+ }
}
- return;
- }
- goto next_agg;
+ } while (res == 0);
}

static void bottom_half(unsigned long data)
--
1.8.3.1

2013-08-16 08:13:27

by Hayes Wang

[permalink] [raw]
Subject: [PATCH net-next 4/7] r8152: adjust some duplicated code

- Use r8152_get_tx_agg for getting tx agg list
- Replace submit rx with goto submit

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

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 41b99ce..4dee76b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1067,6 +1067,24 @@ err1:
return -ENOMEM;
}

+static struct tx_agg *r8152_get_tx_agg(struct r8152 *tp)
+{
+ struct tx_agg *agg = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&tp->tx_lock, flags);
+ if (!list_empty(&tp->tx_free)) {
+ struct list_head *cursor;
+
+ cursor = tp->tx_free.next;
+ list_del_init(cursor);
+ agg = list_entry(cursor, struct tx_agg, list);
+ }
+ spin_unlock_irqrestore(&tp->tx_lock, flags);
+
+ return agg;
+}
+
static void
r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc, struct sk_buff *skb)
{
@@ -1139,15 +1157,8 @@ static void rx_bottom(struct r8152 *tp)

agg = list_entry(cursor, struct rx_agg, list);
urb = agg->urb;
- if (urb->actual_length < ETH_ZLEN) {
- ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
- spin_lock_irqsave(&tp->rx_lock, flags);
- if (ret && ret != -ENODEV) {
- list_add_tail(&agg->list, next);
- tasklet_schedule(&tp->tl);
- }
- continue;
- }
+ if (urb->actual_length < ETH_ZLEN)
+ goto submit;

len_used = 0;
rx_desc = agg->head;
@@ -1181,6 +1192,7 @@ static void rx_bottom(struct r8152 *tp)
len_used += sizeof(struct rx_desc) + pkt_len;
}

+submit:
ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
spin_lock_irqsave(&tp->rx_lock, flags);
if (ret && ret != -ENODEV) {
@@ -1205,16 +1217,10 @@ static void tx_bottom(struct r8152 *tp)

next_agg:
agg = NULL;
- spin_lock_irqsave(&tp->tx_lock, flags);
- if (!skb_queue_empty(&tp->tx_queue) && !list_empty(&tp->tx_free)) {
- struct list_head *cursor;
-
- cursor = tp->tx_free.next;
- list_del_init(cursor);
- agg = list_entry(cursor, struct tx_agg, list);
- }
- spin_unlock_irqrestore(&tp->tx_lock, flags);
+ if (skb_queue_empty(&tp->tx_queue))
+ return;

+ agg = r8152_get_tx_agg(tp);
if (!agg)
return;

@@ -1382,15 +1388,10 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,

skb_tx_timestamp(skb);

- spin_lock_irqsave(&tp->tx_lock, flags);
- if (!list_empty(&tp->tx_free) && skb_queue_empty(&tp->tx_queue)) {
- struct list_head *cursor;
-
- cursor = tp->tx_free.next;
- list_del_init(cursor);
- agg = list_entry(cursor, struct tx_agg, list);
- }
- spin_unlock_irqrestore(&tp->tx_lock, flags);
+ /* If tx_queue is not empty, it means at least one previous packt */
+ /* is waiting for sending. Don't send current one before it. */
+ if (skb_queue_empty(&tp->tx_queue))
+ agg = r8152_get_tx_agg(tp);

if (!agg) {
skb_queue_tail(&tp->tx_queue, skb);
--
1.8.3.1

2013-08-20 07:09:16

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next 1/7] r8152: remove clearing the memory to zero for netdev priv


Series applied, thanks.

Please, in the future, provide an initial "[PATCH 0/N] " posting which
gives a general overview to the series, and to which I can apply when
I have something to say about the series as a whole.

Thanks.