[PATCH 0/5] drivers: net: usb: rtl8150: bug fixing and cleanup

[Petko Manolov]

From: Petko Manolov <pet...@nucleusys.com>

This patch series adds rtl8150.h, which contains structure and constant
definitons formerly found in rtl8150.c, removes socket buffer
pre-allocated pool and uses dynamically allocated memory for the
asynchronous URB requests, thus avoids clobbering the previously scheduled
for update value.

This patch series is against 'net' tree.

Signed-off-by: Petko Manolov <pet...@nucleusys.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Petko Manolov]

From: Petko Manolov <pet...@nucleusys.com>

adding rtl8150.h;

Signed-off-by: Petko Manolov <pet...@nucleusys.com>
---
drivers/net/usb/rtl8150.h | 131 +++++++++++++++++++++++++++++++++++
1 file changed, 131 insertions(+)

diff --git a/drivers/net/usb/rtl8150.h b/drivers/net/usb/rtl8150.h
new file mode 100644
index 0000000..cfb1e74
--- /dev/null
+++ b/drivers/net/usb/rtl8150.h
@@ -0,0 +1,131 @@
+/*
+ * Copyright (c) 2002-2013 Petko Manolov (pet...@nucleusys.com)
+ *
+ * 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.
+ */
+
+#ifndef __rtl8150_h__
+#define __rtl8150_h__
+
+#define IDR 0x0120
+#define MAR 0x0126
+#define CR 0x012e
+#define TCR 0x012f
+#define RCR 0x0130
+#define TSR 0x0132
+#define RSR 0x0133
+#define CON0 0x0135
+#define CON1 0x0136
+#define MSR 0x0137
+#define PHYADD 0x0138
+#define PHYDAT 0x0139
+#define PHYCNT 0x013b
+#define GPPC 0x013d
+#define BMCR 0x0140
+#define BMSR 0x0142
+#define ANAR 0x0144
+#define ANLP 0x0146
+#define AER 0x0148
+#define CSCR 0x014C /* This one has the link status */
+#define CSCR_LINK_STATUS (1 << 3)
+
+#define IDR_EEPROM 0x1202
+
+#define PHY_READ 0
+#define PHY_WRITE 0x20
+#define PHY_GO 0x40
+
+#define MII_TIMEOUT 10
+#define INTBUFSIZE 8
+
+#define RTL8150_REQT_READ 0xc0
+#define RTL8150_REQT_WRITE 0x40
+#define RTL8150_REQ_GET_REGS 0x05
+#define RTL8150_REQ_SET_REGS 0x05
+
+
+/* Transmit status register errors */
+#define TSR_ECOL (1<<5)
+#define TSR_LCOL (1<<4)
+#define TSR_LOSS_CRS (1<<3)
+#define TSR_JBR (1<<2)
+#define TSR_ERRORS (TSR_ECOL | TSR_LCOL | TSR_LOSS_CRS | TSR_JBR)
+/* Receive status register errors */
+#define RSR_CRC (1<<2)
+#define RSR_FAE (1<<1)
+#define RSR_ERRORS (RSR_CRC | RSR_FAE)
+
+/* Media status register definitions */
+#define MSR_DUPLEX (1<<4)
+#define MSR_SPEED (1<<3)
+#define MSR_LINK (1<<2)
+
+/* Interrupt pipe data */
+#define INT_TSR 0x00
+#define INT_RSR 0x01
+#define INT_MSR 0x02
+#define INT_WAKSR 0x03
+#define INT_TXOK_CNT 0x04
+#define INT_RXLOST_CNT 0x05
+#define INT_CRERR_CNT 0x06
+#define INT_COL_CNT 0x07
+
+
+#define RTL8150_MTU 1540
+#define RTL8150_TX_TIMEOUT (HZ)
+#define RX_SKB_POOL_SIZE 4
+
+/* rtl8150 flags */
+#define RTL8150_HW_CRC 0
+#define RX_REG_SET 1
+#define RTL8150_UNPLUG 2
+#define RX_URB_FAIL 3
+
+/* Define these values to match your device */
+#define VENDOR_ID_REALTEK 0x0bda
+#define VENDOR_ID_MELCO 0x0411
+#define VENDOR_ID_MICRONET 0x3980
+#define VENDOR_ID_LONGSHINE 0x07b8
+#define VENDOR_ID_OQO 0x1557
+#define VENDOR_ID_ZYXEL 0x0586
+
+#define PRODUCT_ID_RTL8150 0x8150
+#define PRODUCT_ID_LUAKTX 0x0012
+#define PRODUCT_ID_LCS8138TX 0x401a
+#define PRODUCT_ID_SP128AR 0x0003
+#define PRODUCT_ID_PRESTIGE 0x401a
+
+#undef EEPROM_WRITE
+
+/* table of devices that work with this driver */
+static struct usb_device_id rtl8150_table[] = {
+ {USB_DEVICE(VENDOR_ID_REALTEK, PRODUCT_ID_RTL8150)},
+ {USB_DEVICE(VENDOR_ID_MELCO, PRODUCT_ID_LUAKTX)},
+ {USB_DEVICE(VENDOR_ID_MICRONET, PRODUCT_ID_SP128AR)},
+ {USB_DEVICE(VENDOR_ID_LONGSHINE, PRODUCT_ID_LCS8138TX)},
+ {USB_DEVICE(VENDOR_ID_OQO, PRODUCT_ID_RTL8150)},
+ {USB_DEVICE(VENDOR_ID_ZYXEL, PRODUCT_ID_PRESTIGE)},
+ {}
+};
+
+struct rtl8150 {
+ unsigned long flags;
+ struct usb_device *udev;
+ struct tasklet_struct tl;
+ struct net_device *netdev;
+ struct urb *rx_urb, *tx_urb, *intr_urb, *ctrl_urb;
+ struct sk_buff *tx_skb, *rx_skb;
+ struct sk_buff *rx_skb_pool[RX_SKB_POOL_SIZE];
+ spinlock_t rx_pool_lock;
+ struct usb_ctrlrequest dr;
+ int intr_interval;
+ __le16 rx_creg;
+ u8 *intr_buff;
+ u8 phy;
+};
+
+typedef struct rtl8150 rtl8150_t;
+
+#endif /* __rtl8150_h__ */

[Petko Manolov]

From: Petko Manolov <pet...@nucleusys.com>

removing socket buffer pre-allocation pool;

Signed-off-by: Petko Manolov <pet...@nucleusys.com>
---

drivers/net/usb/rtl8150.c | 60 +++--------------------------------
drivers/net/usb/rtl8150.h | 3 --
2 files changed, 5 insertions(+), 58 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 7d1897b..fd4bc2a 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -240,21 +240,6 @@ static void unlink_all_urbs(rtl8150_t * dev)
usb_kill_urb(dev->ctrl_urb);
}

-static inline struct sk_buff *pull_skb(rtl8150_t *dev)
-{
- struct sk_buff *skb;
- int i;
-
- for (i = 0; i < RX_SKB_POOL_SIZE; i++) {
- if (dev->rx_skb_pool[i]) {
- skb = dev->rx_skb_pool[i];
- dev->rx_skb_pool[i] = NULL;
- return skb;
- }
- }
- return NULL;
-}
-
static void read_bulk_callback(struct urb *urb)
{
rtl8150_t *dev;
@@ -305,9 +290,7 @@ static void read_bulk_callback(struct urb *urb)
netdev->stats.rx_packets++;
netdev->stats.rx_bytes += pkt_len;

- spin_lock(&dev->rx_pool_lock);
- skb = pull_skb(dev);
- spin_unlock(&dev->rx_pool_lock);
+ skb = __netdev_alloc_skb_ip_align(dev->netdev, RTL8150_MTU, GFP_ATOMIC);
if (!skb)
goto resched;

@@ -441,47 +424,16 @@ static int rtl8150_resume(struct usb_interface *intf)
**
*/

-static void fill_skb_pool(rtl8150_t *dev)
-{
- struct sk_buff *skb;
- int i;
-
- for (i = 0; i < RX_SKB_POOL_SIZE; i++) {
- if (dev->rx_skb_pool[i])
- continue;
- skb = dev_alloc_skb(RTL8150_MTU + 2);
- if (!skb) {
- return;
- }
- skb_reserve(skb, 2);
- dev->rx_skb_pool[i] = skb;
- }
-}
-
-static void free_skb_pool(rtl8150_t *dev)
-{
- int i;
-
- for (i = 0; i < RX_SKB_POOL_SIZE; i++)
- if (dev->rx_skb_pool[i])
- dev_kfree_skb(dev->rx_skb_pool[i]);
-}
-
static void rx_fixup(unsigned long data)
{
struct rtl8150 *dev = (struct rtl8150 *)data;
struct sk_buff *skb;
int status;

- spin_lock_irq(&dev->rx_pool_lock);
- fill_skb_pool(dev);
- spin_unlock_irq(&dev->rx_pool_lock);
if (test_bit(RX_URB_FAIL, &dev->flags))
if (dev->rx_skb)
goto try_again;
- spin_lock_irq(&dev->rx_pool_lock);
- skb = pull_skb(dev);
- spin_unlock_irq(&dev->rx_pool_lock);
+ skb = __netdev_alloc_skb_ip_align(dev->netdev, RTL8150_MTU, GFP_ATOMIC);
if (skb == NULL)
goto tlsched;
dev->rx_skb = skb;
@@ -611,7 +563,9 @@ static int rtl8150_open(struct net_device *netdev)
int res;

if (dev->rx_skb == NULL)
- dev->rx_skb = pull_skb(dev);
+ dev->rx_skb = __netdev_alloc_skb_ip_align(dev->netdev,
+ RTL8150_MTU,
+ GFP_ATOMIC);
if (!dev->rx_skb)
return -ENOMEM;

@@ -764,7 +718,6 @@ static int rtl8150_probe(struct usb_interface *intf,
}

tasklet_init(&dev->tl, rx_fixup, (unsigned long)dev);
- spin_lock_init(&dev->rx_pool_lock);

dev->udev = udev;
dev->netdev = netdev;
@@ -781,7 +734,6 @@ static int rtl8150_probe(struct usb_interface *intf,
dev_err(&intf->dev, "couldn't reset the device\n");
goto out1;
}
- fill_skb_pool(dev);
set_ethernet_addr(dev);

usb_set_intfdata(intf, dev);
@@ -797,7 +749,6 @@ static int rtl8150_probe(struct usb_interface *intf,

out2:
usb_set_intfdata(intf, NULL);
- free_skb_pool(dev);
out1:
free_all_urbs(dev);
out:
@@ -817,7 +768,6 @@ static void rtl8150_disconnect(struct usb_interface *intf)
unregister_netdev(dev->netdev);
unlink_all_urbs(dev);
free_all_urbs(dev);
- free_skb_pool(dev);
if (dev->rx_skb)
dev_kfree_skb(dev->rx_skb);
kfree(dev->intr_buff);
diff --git a/drivers/net/usb/rtl8150.h b/drivers/net/usb/rtl8150.h
index cfb1e74..a29410c 100644
--- a/drivers/net/usb/rtl8150.h
+++ b/drivers/net/usb/rtl8150.h
@@ -75,7 +75,6 @@

#define RTL8150_MTU 1540
#define RTL8150_TX_TIMEOUT (HZ)
-#define RX_SKB_POOL_SIZE 4

/* rtl8150 flags */
#define RTL8150_HW_CRC 0
@@ -117,8 +116,6 @@ struct rtl8150 {
struct net_device *netdev;

struct urb *rx_urb, *tx_urb, *intr_urb, *ctrl_urb;

struct sk_buff *tx_skb, *rx_skb;
- struct sk_buff *rx_skb_pool[RX_SKB_POOL_SIZE];
- spinlock_t rx_pool_lock;
struct usb_ctrlrequest dr;
int intr_interval;
__le16 rx_creg;

[Petko Manolov]

From: Petko Manolov <pet...@nucleusys.com>

[get|set]_registers() will now display a message in case of error condition
and if DEBUG is enabled. All resources required for asynchronous control URB
submission are being dynamically (de)allocated.

Signed-off-by: Petko Manolov <pet...@nucleusys.com>
---

drivers/net/usb/rtl8150.c | 129 +++++++++++++++++------------------
drivers/net/usb/rtl8150.h | 9 ++-
2 files changed, 68 insertions(+), 70 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index fd4bc2a..0e226d8 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -15,7 +15,6 @@
#include <linux/mii.h>
#include <linux/ethtool.h>
#include <linux/usb.h>
-#include <asm/uaccess.h>

#include "rtl8150.h"

@@ -29,69 +28,75 @@ MODULE_DEVICE_TABLE(usb, rtl8150_table);
static const char driver_name [] = "rtl8150";

/*
-**
-** device related part of the code
-**
-*/
+ *
+ * device related part of the code
+ *
+ */
static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
{
- return usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
- RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
- indx, 0, data, size, 500);
+ int res;
+
+ res = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
+ RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
+ indx, 0, data, size, 500);
+ if (res < 0)
+ dev_dbg(&dev->udev->dev, "%s returned %d\n", __func__, res);
+ return res;
}

static int set_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
{
- return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
- indx, 0, data, size, 500);
+ int res;
+
+ res = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
+ RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
+ indx, 0, data, size, 500);
+ if (res < 0)
+ dev_dbg(&dev->udev->dev, "%s returned %d\n", __func__, res);
+ return res;
}

-static void ctrl_callback(struct urb *urb)
+static void async_set_reg_cb(struct urb *urb)
{
- rtl8150_t *dev;
+ struct async_req *req = (struct async_req *)urb->context;
int status = urb->status;

- switch (status) {
- case 0:
- break;
- case -EINPROGRESS:
- break;
- case -ENOENT:
- break;
- default:
- if (printk_ratelimit())
- dev_warn(&urb->dev->dev, "ctrl urb status %d\n", status);
- }
- dev = urb->context;
- clear_bit(RX_REG_SET, &dev->flags);
+ if (status < 0)
+ dev_dbg(&urb->dev->dev, "%s failed with %d", __func__, status);
+ kfree(req);
+ usb_free_urb(urb);
}

-static int async_set_registers(rtl8150_t * dev, u16 indx, u16 size)
+static int async_set_registers(rtl8150_t *dev, u16 indx, u16 size, u16 reg)
{
- int ret;
-
- if (test_bit(RX_REG_SET, &dev->flags))
- return -EAGAIN;
-
- dev->dr.bRequestType = RTL8150_REQT_WRITE;
- dev->dr.bRequest = RTL8150_REQ_SET_REGS;
- dev->dr.wValue = cpu_to_le16(indx);
- dev->dr.wIndex = 0;
- dev->dr.wLength = cpu_to_le16(size);
- dev->ctrl_urb->transfer_buffer_length = size;
- usb_fill_control_urb(dev->ctrl_urb, dev->udev,
- usb_sndctrlpipe(dev->udev, 0), (char *) &dev->dr,
- &dev->rx_creg, size, ctrl_callback, dev);
- if ((ret = usb_submit_urb(dev->ctrl_urb, GFP_ATOMIC))) {
- if (ret == -ENODEV)
- netif_device_detach(dev->netdev);
- dev_err(&dev->udev->dev,
- "control request submission failed: %d\n", ret);
- } else
- set_bit(RX_REG_SET, &dev->flags);
+ int res = -ENOMEM;
+ struct urb *async_urb;
+ struct async_req *req;

- return ret;
+ req = kmalloc(sizeof(struct async_req), GFP_ATOMIC);
+ if (req == NULL)
+ return res;
+ async_urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (async_urb == NULL) {
+ kfree(req);
+ return res;
+ }
+ req->rx_creg = cpu_to_le16(reg);
+ req->dr.bRequestType = RTL8150_REQT_WRITE;
+ req->dr.bRequest = RTL8150_REQ_SET_REGS;
+ req->dr.wIndex = 0;
+ req->dr.wValue = cpu_to_le16(indx);
+ req->dr.wLength = cpu_to_le16(size);
+ usb_fill_control_urb(async_urb, dev->udev,
+ usb_sndctrlpipe(dev->udev, 0), (void *) &req->dr,
+ &req->rx_creg, size, async_set_reg_cb, req);
+ res = usb_submit_urb(async_urb, GFP_ATOMIC);
+ if (res) {
+ if (res == -ENODEV)
+ netif_device_detach(dev->netdev);
+ dev_err(&dev->udev->dev, "%s failed with %d\n", __func__, res);
+ }
+ return res;
}

static int read_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 * reg)
@@ -213,14 +218,6 @@ static int alloc_all_urbs(rtl8150_t * dev)
usb_free_urb(dev->tx_urb);
return 0;
}
- dev->ctrl_urb = usb_alloc_urb(0, GFP_KERNEL);
- if (!dev->ctrl_urb) {
- usb_free_urb(dev->rx_urb);
- usb_free_urb(dev->tx_urb);
- usb_free_urb(dev->intr_urb);
- return 0;
- }
-
return 1;
}

@@ -229,7 +226,6 @@ static void free_all_urbs(rtl8150_t * dev)
usb_free_urb(dev->rx_urb);
usb_free_urb(dev->tx_urb);
usb_free_urb(dev->intr_urb);
- usb_free_urb(dev->ctrl_urb);

}

static void unlink_all_urbs(rtl8150_t * dev)

@@ -237,7 +233,6 @@ static void unlink_all_urbs(rtl8150_t * dev)
usb_kill_urb(dev->rx_urb);
usb_kill_urb(dev->tx_urb);
usb_kill_urb(dev->intr_urb);
- usb_kill_urb(dev->ctrl_urb);

}

static void read_bulk_callback(struct urb *urb)

@@ -464,7 +459,6 @@ static int enable_net_traffic(rtl8150_t * dev)
}
/* RCR bit7=1 attach Rx info at the end; =0 HW CRC (which is broken) */
rcr = 0x9e;
- dev->rx_creg = cpu_to_le16(rcr);
tcr = 0xd8;
cr = 0x0c;
if (!(rcr & 0x80))
@@ -497,20 +491,21 @@ static void rtl8150_tx_timeout(struct net_device *netdev)
static void rtl8150_set_multicast(struct net_device *netdev)
{
rtl8150_t *dev = netdev_priv(netdev);
+ u16 rx_creg = 0x9e;
+
netif_stop_queue(netdev);
if (netdev->flags & IFF_PROMISC) {
- dev->rx_creg |= cpu_to_le16(0x0001);
+ rx_creg |= 0x0001;
dev_info(&netdev->dev, "%s: promiscuous mode\n", netdev->name);
- } else if (!netdev_mc_empty(netdev) ||
- (netdev->flags & IFF_ALLMULTI)) {
- dev->rx_creg &= cpu_to_le16(0xfffe);
- dev->rx_creg |= cpu_to_le16(0x0002);
+ } else if (!netdev_mc_empty(netdev) || (netdev->flags & IFF_ALLMULTI)) {
+ rx_creg &= 0xfffe;
+ rx_creg |= 0x0002;
dev_info(&netdev->dev, "%s: allmulti set\n", netdev->name);
} else {
/* ~RX_MULTICAST, ~RX_PROMISCUOUS */
- dev->rx_creg &= cpu_to_le16(0x00fc);
+ rx_creg &= 0x00fc;
}
- async_set_registers(dev, RCR, 2);
+ async_set_registers(dev, RCR, sizeof(rx_creg), rx_creg);
netif_wake_queue(netdev);
}

diff --git a/drivers/net/usb/rtl8150.h b/drivers/net/usb/rtl8150.h
index a29410c..2dff8f4 100644
--- a/drivers/net/usb/rtl8150.h
+++ b/drivers/net/usb/rtl8150.h
@@ -114,15 +114,18 @@ struct rtl8150 {
struct usb_device *udev;
struct tasklet_struct tl;
struct net_device *netdev;
- struct urb *rx_urb, *tx_urb, *intr_urb, *ctrl_urb;
+ struct urb *rx_urb, *tx_urb, *intr_urb;
struct sk_buff *tx_skb, *rx_skb;
- struct usb_ctrlrequest dr;
int intr_interval;
- __le16 rx_creg;
u8 *intr_buff;
u8 phy;
};

typedef struct rtl8150 rtl8150_t;

+struct async_req {
+ struct usb_ctrlrequest dr;
+ u16 rx_creg;
+};
+
#endif /* __rtl8150_h__ */

[Petko Manolov]

From: Petko Manolov <pet...@nucleusys.com>

copyright, email address and styling update;

Signed-off-by: Petko Manolov <pet...@nucleusys.com>
---

drivers/net/usb/rtl8150.c | 69 +++++++++++++++++------------------
1 file changed, 34 insertions(+), 35 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 0e226d8..928546e 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2002 Petko Manolov (pet...@users.sourceforge.net)

+ * Copyright (c) 2002-2013 Petko Manolov (pet...@nucleusys.com)

*

* This program is free software; you can redistribute it and/or

* modify it under the terms of the GNU General Public License

@@ -19,8 +19,8 @@
#include "rtl8150.h"

/* Version Information */
-#define DRIVER_VERSION "v0.6.2 (2004/08/27)"
-#define DRIVER_AUTHOR "Petko Manolov <pet...@users.sourceforge.net>"
+#define DRIVER_VERSION "v0.9.3 (2013/05/18)"
+#define DRIVER_AUTHOR "Petko Manolov <pet...@nucleusys.com>"
#define DRIVER_DESC "rtl8150 based usb-ethernet driver"

MODULE_DEVICE_TABLE(usb, rtl8150_table);
@@ -32,7 +32,7 @@ static const char driver_name [] = "rtl8150";

* device related part of the code

*
*/
-static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
+static int get_registers(rtl8150_t *dev, u16 indx, u16 size, void *data)
{
int res;

@@ -44,7 +44,7 @@ static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
return res;
}

-static int set_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
+static int set_registers(rtl8150_t *dev, u16 indx, u16 size, void *data)
{
int res;

@@ -99,7 +99,7 @@ static int async_set_registers(rtl8150_t *dev, u16 indx, u16 size, u16 reg)
return res;
}

-static int read_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 * reg)
+static int read_mii_word(rtl8150_t *dev, u8 phy, __u8 indx, u16 *reg)
{
int i;
u8 data[3], tmp;
@@ -123,7 +123,7 @@ static int read_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 * reg)
return 1;
}

-static int write_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 reg)
+static int write_mii_word(rtl8150_t *dev, u8 phy, __u8 indx, u16 reg)
{
int i;
u8 data[3], tmp;
@@ -146,7 +146,7 @@ static int write_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 reg)
return 1;
}

-static inline void set_ethernet_addr(rtl8150_t * dev)
+static inline void set_ethernet_addr(rtl8150_t *dev)
{
u8 node_id[6];

@@ -189,7 +189,7 @@ static int rtl8150_set_mac_address(struct net_device *netdev, void *p)
return 0;
}

-static int rtl8150_reset(rtl8150_t * dev)
+static int rtl8150_reset(rtl8150_t *dev)
{
u8 data = 0x10;
int i = HZ;
@@ -202,7 +202,7 @@ static int rtl8150_reset(rtl8150_t * dev)
return (i > 0) ? 1 : 0;
}

-static int alloc_all_urbs(rtl8150_t * dev)
+static int alloc_all_urbs(rtl8150_t *dev)
{
dev->rx_urb = usb_alloc_urb(0, GFP_KERNEL);
if (!dev->rx_urb)
@@ -221,14 +221,14 @@ static int alloc_all_urbs(rtl8150_t * dev)
return 1;
}

-static void free_all_urbs(rtl8150_t * dev)
+static void free_all_urbs(rtl8150_t *dev)
{

usb_free_urb(dev->rx_urb);
usb_free_urb(dev->tx_urb);
usb_free_urb(dev->intr_urb);
}

-static void unlink_all_urbs(rtl8150_t * dev)
+static void unlink_all_urbs(rtl8150_t *dev)
{
usb_kill_urb(dev->rx_urb);
usb_kill_urb(dev->tx_urb);
@@ -364,12 +364,12 @@ static void intr_callback(struct urb *urb)
if ((d[INT_MSR] & MSR_LINK) == 0) {
if (netif_carrier_ok(dev->netdev)) {
netif_carrier_off(dev->netdev);
- netdev_dbg(dev->netdev, "%s: LINK LOST\n", __func__);
+ netdev_dbg(dev->netdev, "%s: link lost\n", __func__);
}
} else {
if (!netif_carrier_ok(dev->netdev)) {
netif_carrier_on(dev->netdev);
- netdev_dbg(dev->netdev, "%s: LINK CAME BACK\n", __func__);
+ netdev_dbg(dev->netdev, "%s: link is back\n", __func__);
}
}

@@ -450,7 +450,7 @@ tlsched:
tasklet_schedule(&dev->tl);
}

-static int enable_net_traffic(rtl8150_t * dev)
+static int enable_net_traffic(rtl8150_t *dev)
{
u8 cr, tcr, rcr, msr;

@@ -471,7 +471,7 @@ static int enable_net_traffic(rtl8150_t * dev)
return 0;
}

-static void disable_net_traffic(rtl8150_t * dev)
+static void disable_net_traffic(rtl8150_t *dev)
{
u8 cr;

@@ -510,7 +510,7 @@ static void rtl8150_set_multicast(struct net_device *netdev)
}

static netdev_tx_t rtl8150_start_xmit(struct sk_buff *skb,
- struct net_device *netdev)
+ struct net_device *netdev)
{
rtl8150_t *dev = netdev_priv(netdev);
int count, res;
@@ -681,16 +681,15 @@ static int rtl8150_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
}

static const struct net_device_ops rtl8150_netdev_ops = {
- .ndo_open = rtl8150_open,
- .ndo_stop = rtl8150_close,
- .ndo_do_ioctl = rtl8150_ioctl,
- .ndo_start_xmit = rtl8150_start_xmit,
- .ndo_tx_timeout = rtl8150_tx_timeout,
- .ndo_set_rx_mode = rtl8150_set_multicast,
- .ndo_set_mac_address = rtl8150_set_mac_address,
-
- .ndo_change_mtu = eth_change_mtu,
- .ndo_validate_addr = eth_validate_addr,
+ .ndo_open = rtl8150_open,
+ .ndo_stop = rtl8150_close,
+ .ndo_do_ioctl = rtl8150_ioctl,
+ .ndo_start_xmit = rtl8150_start_xmit,
+ .ndo_tx_timeout = rtl8150_tx_timeout,
+ .ndo_set_rx_mode = rtl8150_set_multicast,
+ .ndo_set_mac_address = rtl8150_set_mac_address,
+ .ndo_change_mtu = eth_change_mtu,
+ .ndo_validate_addr = eth_validate_addr,
};

static int rtl8150_probe(struct usb_interface *intf,

@@ -738,8 +737,8 @@ static int rtl8150_probe(struct usb_interface *intf,
goto out2;
}

- dev_info(&intf->dev, "%s: rtl8150 is detected\n", netdev->name);
-
+ dev_info(&intf->dev, "%s: %s %s\n", netdev->name,
+ DRIVER_DESC, DRIVER_VERSION);
return 0;

out2:
@@ -771,12 +770,12 @@ static void rtl8150_disconnect(struct usb_interface *intf)
}

static struct usb_driver rtl8150_driver = {
- .name = driver_name,
- .probe = rtl8150_probe,
- .disconnect = rtl8150_disconnect,
- .id_table = rtl8150_table,
- .suspend = rtl8150_suspend,
- .resume = rtl8150_resume,
+ .name = driver_name,
+ .probe = rtl8150_probe,
+ .disconnect = rtl8150_disconnect,
+ .id_table = rtl8150_table,
+ .suspend = rtl8150_suspend,
+ .resume = rtl8150_resume,
.disable_hub_initiated_lpm = 1,
};

[Petko Manolov]

From: Petko Manolov <pet...@nucleusys.com>

Moving constant and structure definitions out of rtl8150.c;

Signed-off-by: Petko Manolov <pet...@nucleusys.com>
---

drivers/net/usb/rtl8150.c | 121 +----------------------------------
1 file changed, 2 insertions(+), 119 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index a491d3a..7d1897b 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -17,132 +17,15 @@
#include <linux/usb.h>
#include <asm/uaccess.h>

+#include "rtl8150.h"
+
/* Version Information */
#define DRIVER_VERSION "v0.6.2 (2004/08/27)"
#define DRIVER_AUTHOR "Petko Manolov <pet...@users.sourceforge.net>"

#define DRIVER_DESC "rtl8150 based usb-ethernet driver"

-#define IDR 0x0120
-#define MAR 0x0126
-#define CR 0x012e
-#define TCR 0x012f
-#define RCR 0x0130
-#define TSR 0x0132
-#define RSR 0x0133
-#define CON0 0x0135
-#define CON1 0x0136
-#define MSR 0x0137
-#define PHYADD 0x0138
-#define PHYDAT 0x0139
-#define PHYCNT 0x013b
-#define GPPC 0x013d
-#define BMCR 0x0140
-#define BMSR 0x0142
-#define ANAR 0x0144
-#define ANLP 0x0146
-#define AER 0x0148
-#define CSCR 0x014C /* This one has the link status */
-#define CSCR_LINK_STATUS (1 << 3)
-
-#define IDR_EEPROM 0x1202
-
-#define PHY_READ 0
-#define PHY_WRITE 0x20
-#define PHY_GO 0x40
-
-#define MII_TIMEOUT 10
-#define INTBUFSIZE 8
-
-#define RTL8150_REQT_READ 0xc0
-#define RTL8150_REQT_WRITE 0x40
-#define RTL8150_REQ_GET_REGS 0x05
-#define RTL8150_REQ_SET_REGS 0x05
-
-
-/* Transmit status register errors */
-#define TSR_ECOL (1<<5)
-#define TSR_LCOL (1<<4)
-#define TSR_LOSS_CRS (1<<3)
-#define TSR_JBR (1<<2)
-#define TSR_ERRORS (TSR_ECOL | TSR_LCOL | TSR_LOSS_CRS | TSR_JBR)
-/* Receive status register errors */
-#define RSR_CRC (1<<2)
-#define RSR_FAE (1<<1)
-#define RSR_ERRORS (RSR_CRC | RSR_FAE)
-
-/* Media status register definitions */
-#define MSR_DUPLEX (1<<4)
-#define MSR_SPEED (1<<3)
-#define MSR_LINK (1<<2)
-
-/* Interrupt pipe data */
-#define INT_TSR 0x00
-#define INT_RSR 0x01
-#define INT_MSR 0x02
-#define INT_WAKSR 0x03
-#define INT_TXOK_CNT 0x04
-#define INT_RXLOST_CNT 0x05
-#define INT_CRERR_CNT 0x06
-#define INT_COL_CNT 0x07
-
-
-#define RTL8150_MTU 1540
-#define RTL8150_TX_TIMEOUT (HZ)
-#define RX_SKB_POOL_SIZE 4
-
-/* rtl8150 flags */
-#define RTL8150_HW_CRC 0
-#define RX_REG_SET 1
-#define RTL8150_UNPLUG 2
-#define RX_URB_FAIL 3
-
-/* Define these values to match your device */
-#define VENDOR_ID_REALTEK 0x0bda
-#define VENDOR_ID_MELCO 0x0411
-#define VENDOR_ID_MICRONET 0x3980
-#define VENDOR_ID_LONGSHINE 0x07b8
-#define VENDOR_ID_OQO 0x1557
-#define VENDOR_ID_ZYXEL 0x0586
-
-#define PRODUCT_ID_RTL8150 0x8150
-#define PRODUCT_ID_LUAKTX 0x0012
-#define PRODUCT_ID_LCS8138TX 0x401a
-#define PRODUCT_ID_SP128AR 0x0003
-#define PRODUCT_ID_PRESTIGE 0x401a
-
-#undef EEPROM_WRITE
-
-/* table of devices that work with this driver */
-static struct usb_device_id rtl8150_table[] = {
- {USB_DEVICE(VENDOR_ID_REALTEK, PRODUCT_ID_RTL8150)},
- {USB_DEVICE(VENDOR_ID_MELCO, PRODUCT_ID_LUAKTX)},
- {USB_DEVICE(VENDOR_ID_MICRONET, PRODUCT_ID_SP128AR)},
- {USB_DEVICE(VENDOR_ID_LONGSHINE, PRODUCT_ID_LCS8138TX)},
- {USB_DEVICE(VENDOR_ID_OQO, PRODUCT_ID_RTL8150)},
- {USB_DEVICE(VENDOR_ID_ZYXEL, PRODUCT_ID_PRESTIGE)},
- {}
-};
-
MODULE_DEVICE_TABLE(usb, rtl8150_table);

-struct rtl8150 {
- unsigned long flags;
- struct usb_device *udev;
- struct tasklet_struct tl;
- struct net_device *netdev;

- struct urb *rx_urb, *tx_urb, *intr_urb, *ctrl_urb;

- struct sk_buff *tx_skb, *rx_skb;

- struct sk_buff *rx_skb_pool[RX_SKB_POOL_SIZE];
- spinlock_t rx_pool_lock;

- struct usb_ctrlrequest dr;
- int intr_interval;
- __le16 rx_creg;
- u8 *intr_buff;
- u8 phy;
-};
-
-typedef struct rtl8150 rtl8150_t;
-

static const char driver_name [] = "rtl8150";

/*

[David Miller]

If you use the same exact subject in every single patch, someone
reading the commit shortlog can't tell at all what different is
happening in each of the changes.

Choose better, unique, subject lines for your patches and resubmit
this series.

Thanks.

[Francois Romieu]

Petko Manolov <pet...@nucleusys.com> :

> From: Petko Manolov <pet...@nucleusys.com>
>
> Moving constant and structure definitions out of rtl8150.c;

What's the point ?

[...]

> ---
> drivers/net/usb/rtl8150.c | 121 +----------------------------------
> 1 file changed, 2 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index a491d3a..7d1897b 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -17,132 +17,15 @@
> #include <linux/usb.h>
> #include <asm/uaccess.h>
>
> +#include "rtl8150.h"

It won't compile. You shouldn't do that.

--
Ueimor

[Francois Romieu]

Petko Manolov <pet...@nucleusys.com> :
[...]

> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 7d1897b..fd4bc2a 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c

[...]

> static void rx_fixup(unsigned long data)
> {
> struct rtl8150 *dev = (struct rtl8150 *)data;
> struct sk_buff *skb;
> int status;
>
> - spin_lock_irq(&dev->rx_pool_lock);
> - fill_skb_pool(dev);
> - spin_unlock_irq(&dev->rx_pool_lock);
> if (test_bit(RX_URB_FAIL, &dev->flags))
> if (dev->rx_skb)
> goto try_again;
> - spin_lock_irq(&dev->rx_pool_lock);
> - skb = pull_skb(dev);
> - spin_unlock_irq(&dev->rx_pool_lock);
> + skb = __netdev_alloc_skb_ip_align(dev->netdev, RTL8150_MTU, GFP_ATOMIC);

You can use plain netdev_alloc_skb_ip_align.

--
Ueimor

[Francois Romieu]

Petko Manolov <pet...@nucleusys.com> :
[...]

> static int set_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
> {
> - return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
> - RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
> - indx, 0, data, size, 500);
> + int res;
> +
> + res = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
> + RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
> + indx, 0, data, size, 500);
> + if (res < 0)
> + dev_dbg(&dev->udev->dev, "%s returned %d\n", __func__, res);
> + return res;

You may move it into a separate patch. It is completely unrelated to the
ctrl_urb changes.

[...]

> +static int async_set_registers(rtl8150_t *dev, u16 indx, u16 size, u16 reg)
> {

[...]

> + usb_fill_control_urb(async_urb, dev->udev,
> + usb_sndctrlpipe(dev->udev, 0), (void *) &req->dr,

Useless void * cast.

--
Ueimor

[Francois Romieu]

Petko Manolov <pet...@nucleusys.com> :
[...]

> @@ -681,16 +681,15 @@ static int rtl8150_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
> }
>
> static const struct net_device_ops rtl8150_netdev_ops = {
> - .ndo_open = rtl8150_open,
> - .ndo_stop = rtl8150_close,
> - .ndo_do_ioctl = rtl8150_ioctl,
> - .ndo_start_xmit = rtl8150_start_xmit,
> - .ndo_tx_timeout = rtl8150_tx_timeout,
> - .ndo_set_rx_mode = rtl8150_set_multicast,
> - .ndo_set_mac_address = rtl8150_set_mac_address,
> -
> - .ndo_change_mtu = eth_change_mtu,
> - .ndo_validate_addr = eth_validate_addr,
> + .ndo_open = rtl8150_open,
> + .ndo_stop = rtl8150_close,
> + .ndo_do_ioctl = rtl8150_ioctl,
> + .ndo_start_xmit = rtl8150_start_xmit,
> + .ndo_tx_timeout = rtl8150_tx_timeout,
> + .ndo_set_rx_mode = rtl8150_set_multicast,
> + .ndo_set_mac_address = rtl8150_set_mac_address,
> + .ndo_change_mtu = eth_change_mtu,
> + .ndo_validate_addr = eth_validate_addr,

This is wrong.

> @@ -771,12 +770,12 @@ static void rtl8150_disconnect(struct usb_interface *intf)
> }
>
> static struct usb_driver rtl8150_driver = {
> - .name = driver_name,
> - .probe = rtl8150_probe,
> - .disconnect = rtl8150_disconnect,
> - .id_table = rtl8150_table,
> - .suspend = rtl8150_suspend,
> - .resume = rtl8150_resume,
> + .name = driver_name,
> + .probe = rtl8150_probe,
> + .disconnect = rtl8150_disconnect,
> + .id_table = rtl8150_table,
> + .suspend = rtl8150_suspend,
> + .resume = rtl8150_resume,
> .disable_hub_initiated_lpm = 1,

Sic.

--
Ueimor

[Petko Manolov]

I am inclined to agree with you.

[Petko Manolov]

On Sat, 18 May 2013, Francois Romieu wrote:

> Petko Manolov <pet...@nucleusys.com> :
> > From: Petko Manolov <pet...@nucleusys.com>
> >
> > Moving constant and structure definitions out of rtl8150.c;
>
> What's the point ?

The general logic of having .h files applies.

> [...]
> > ---
> > drivers/net/usb/rtl8150.c | 121 +----------------------------------
> > 1 file changed, 2 insertions(+), 119 deletions(-)
> >
> > diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> > index a491d3a..7d1897b 100644
> > --- a/drivers/net/usb/rtl8150.c
> > +++ b/drivers/net/usb/rtl8150.c
> > @@ -17,132 +17,15 @@
> > #include <linux/usb.h>
> > #include <asm/uaccess.h>
> >
> > +#include "rtl8150.h"
>
> It won't compile. You shouldn't do that.

It does compile. Both inside and outside of the tree.

If the proper place for rtl8150.h is somewhere in include/linux/... then
it is different matter.

[Petko Manolov]

On Sat, 18 May 2013, Francois Romieu wrote:

> Petko Manolov <pet...@nucleusys.com> :
> [...]
> > diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> > index 7d1897b..fd4bc2a 100644
> > --- a/drivers/net/usb/rtl8150.c
> > +++ b/drivers/net/usb/rtl8150.c
> [...]
> > static void rx_fixup(unsigned long data)
> > {
> > struct rtl8150 *dev = (struct rtl8150 *)data;
> > struct sk_buff *skb;
> > int status;
> >
> > - spin_lock_irq(&dev->rx_pool_lock);
> > - fill_skb_pool(dev);
> > - spin_unlock_irq(&dev->rx_pool_lock);
> > if (test_bit(RX_URB_FAIL, &dev->flags))
> > if (dev->rx_skb)
> > goto try_again;
> > - spin_lock_irq(&dev->rx_pool_lock);
> > - skb = pull_skb(dev);
> > - spin_unlock_irq(&dev->rx_pool_lock);
> > + skb = __netdev_alloc_skb_ip_align(dev->netdev, RTL8150_MTU, GFP_ATOMIC);
>
> You can use plain netdev_alloc_skb_ip_align.

Yep, except for the call in rtl8150_open() (where GFP_KERNEL is more
appropriate) i should have used the plain version.

[Petko Manolov]

On Sat, 18 May 2013, Francois Romieu wrote:

> Petko Manolov <pet...@nucleusys.com> :
> [...]
> > static int set_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
> > {
> > - return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
> > - RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
> > - indx, 0, data, size, 500);
> > + int res;
> > +
> > + res = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
> > + RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
> > + indx, 0, data, size, 500);
> > + if (res < 0)
> > + dev_dbg(&dev->udev->dev, "%s returned %d\n", __func__, res);
> > + return res;
>
> You may move it into a separate patch. It is completely unrelated to the
> ctrl_urb changes.

The change is so trivial i thought i can smuggle it unnoticed. :-)

> [...]
> > +static int async_set_registers(rtl8150_t *dev, u16 indx, u16 size, u16 reg)
> > {
>
> [...]
> > + usb_fill_control_urb(async_urb, dev->udev,
> > + usb_sndctrlpipe(dev->udev, 0), (void *) &req->dr,
>
> Useless void * cast.

Wrong. The compiler actually moans quite a lot:

/usr/src/git/rtl8150/rtl8150.c: In function οΏ½async_set_registersοΏ½:
/usr/src/git/rtl8150/rtl8150.c:92:9: warning: passing argument 4 of οΏ½usb_fill_control_urbοΏ½ from incompatible pointer type [enabled by default]
In file included from /usr/src/git/rtl8150/rtl8150.c:17:0:
include/linux/usb.h:1440:20: note: expected οΏ½unsigned char *οΏ½ but argument is of type οΏ½struct usb_ctrlrequest *οΏ½

[Francois Romieu]

Petko Manolov <pet...@nucleusys.com> :

> On Sat, 18 May 2013, Francois Romieu wrote:

> > > From: Petko Manolov <pet...@nucleusys.com>
[...]

> > > Moving constant and structure definitions out of rtl8150.c;
> >
> > What's the point ?
>
> The general logic of having .h files applies.

Which one ?
- share it through the kernel or with userspace
- personal choice .c split

I don't mind the later even if it does not exactly apply to a
WIP driver. I'd still avoid future copycat followers though.

[...]

> > > drivers/net/usb/rtl8150.c | 121 +----------------------------------
> > > 1 file changed, 2 insertions(+), 119 deletions(-)
> > >
> > > diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> > > index a491d3a..7d1897b 100644
> > > --- a/drivers/net/usb/rtl8150.c
> > > +++ b/drivers/net/usb/rtl8150.c
> > > @@ -17,132 +17,15 @@
> > > #include <linux/usb.h>
> > > #include <asm/uaccess.h>
> > >
> > > +#include "rtl8150.h"
> >
> > It won't compile. You shouldn't do that.
>
> It does compile. Both inside and outside of the tree.

The rtl8150.h file is created in patch #2. This is patch #1.

So...

--
Ueimor

[Francois Romieu]

Petko Manolov <pet...@nucleusys.com> :

[...]
> > > + usb_fill_control_urb(async_urb, dev->udev,
> > > + usb_sndctrlpipe(dev->udev, 0), (void *) &req->dr,
> >
> > Useless void * cast.
>
> Wrong. The compiler actually moans quite a lot:
>

> /usr/src/git/rtl8150/rtl8150.c: In function ?async_set_registers?:
> /usr/src/git/rtl8150/rtl8150.c:92:9: warning: passing argument 4 of ?usb_fill_control_urb? from incompatible pointer type [enabled by default]

> In file included from /usr/src/git/rtl8150/rtl8150.c:17:0:

> include/linux/usb.h:1440:20: note: expected ?unsigned char *? but argument is of type ?struct usb_ctrlrequest *?

Sorry, I confused it with transfer_buffer. Some drivers go through unsigned
char *, some widen it to void *.

[Petko Manolov]

On Sun, 19 May 2013, Francois Romieu wrote:

> Which one ?
> - share it through the kernel or with userspace
> - personal choice .c split

It is obviously not the former. I think that in general constant and
other definitions (in their majority) should be in a header file. I
definitely like this way better.

Perhaps in this particular case my patch is a bit of an overkill as the
code lines aren't that many. If the consensus is in this direction i'll
revert this part of the series.

> I don't mind the later even if it does not exactly apply to a
> WIP driver. I'd still avoid future copycat followers though.

This isn't WIP anymore as the W(ork) part is missing. After so many quiet
years i assume the experimental tag should be removed.

> The rtl8150.h file is created in patch #2. This is patch #1.
>
> So...

So first apply patch #1 and then patch #2.

However, if it is required for the driver to be in build-able form after
applying any single patch i'll coalesce #1 and #2 before next submission.


David?


Petko

[David Miller]

From: Petko Manolov <pet...@nucleusys.com>
Date: Mon, 20 May 2013 09:58:39 +0300 (EEST)

> So first apply patch #1 and then patch #2.

Then nobody can properly GIT bisect through your patch series.

The tree must work and build properly at each and every step
of your patch series.

[Petko Manolov]

On Mon, 20 May 2013, David Miller wrote:

> From: Petko Manolov <pet...@nucleusys.com>
> Date: Mon, 20 May 2013 09:58:39 +0300 (EEST)
>
> > So first apply patch #1 and then patch #2.
>
> Then nobody can properly GIT bisect through your patch series.
>
> The tree must work and build properly at each and every step
> of your patch series.

Got it. What about the .c/.h split?

[David Miller]

From: Petko Manolov <pet...@nucleusys.com>
Date: Mon, 20 May 2013 10:09:00 +0300 (EEST)

> What about the .c/.h split?

I have no strong opinion either way.

[Petko Manolov]

On Mon, 20 May 2013, David Miller wrote:

> From: Petko Manolov <pet...@nucleusys.com>
> Date: Mon, 20 May 2013 10:09:00 +0300 (EEST)
>
> > What about the .c/.h split?
>
> I have no strong opinion either way.

OK, so i'll prepare new patch series that coalesce my original patch #1
and #2, and apply the Francois suggestion about using the generic
netdev_alloc_skb_ip_align() in the interrupt path.

Which tree would you want me to diff against?

[David Miller]

From: Petko Manolov <pet...@nucleusys.com>
Date: Mon, 20 May 2013 10:18:17 +0300 (EEST)

> On Mon, 20 May 2013, David Miller wrote:
>
>> From: Petko Manolov <pet...@nucleusys.com>
>> Date: Mon, 20 May 2013 10:09:00 +0300 (EEST)
>>
>> > What about the .c/.h split?
>>
>> I have no strong opinion either way.
>
> OK, so i'll prepare new patch series that coalesce my original patch #1
> and #2, and apply the Francois suggestion about using the generic
> netdev_alloc_skb_ip_align() in the interrupt path.
>
> Which tree would you want me to diff against?

As has been explained to you already, cleanups belong in 'net-next',
bug fixes belong in 'net'.

If you series has both, you have to submit them separately. Submit
the bug fixes to 'net', then the next time I merge 'net' into 'net-next'
you can submit the cleanups on top against 'net-next'.

[Petko Manolov]

On Mon, 20 May 2013, David Miller wrote:

> From: Petko Manolov <pet...@nucleusys.com>
> Date: Mon, 20 May 2013 10:18:17 +0300 (EEST)
>
> > On Mon, 20 May 2013, David Miller wrote:
> >
> >> From: Petko Manolov <pet...@nucleusys.com>
> >> Date: Mon, 20 May 2013 10:09:00 +0300 (EEST)
> >>
> >> > What about the .c/.h split?
> >>
> >> I have no strong opinion either way.
> >
> > OK, so i'll prepare new patch series that coalesce my original patch #1
> > and #2, and apply the Francois suggestion about using the generic
> > netdev_alloc_skb_ip_align() in the interrupt path.
> >
> > Which tree would you want me to diff against?
>
> As has been explained to you already, cleanups belong in 'net-next',
> bug fixes belong in 'net'.
>
> If you series has both, you have to submit them separately. Submit
> the bug fixes to 'net', then the next time I merge 'net' into 'net-next'
> you can submit the cleanups on top against 'net-next'.

Got it. Thanks.