[PATCH 1/6] smsc95xx: change hard_header_len to include csum offload bytes

Previous thread: [PATCH 1/4] smsc75xx: SMSC LAN75xx USB gigabit ethernet adapter driver by Steve Glendinning on Wednesday, March 10, 2010 - 11:26 am. (1 message)

Next thread: [PATCH 0/6] smsc95xx: minor fixes and tidy-ups by Steve Glendinning on Wednesday, March 10, 2010 - 11:32 am. (1 message)
From: Steve Glendinning
Date: Wednesday, March 10, 2010 - 11:32 am

If tx checksum offload is enabled on this device it requires 12
bytes of headroom instead of 8.  This patch sets hard_header_len
to the larger of the two to help avoid skb_copy_expand.

Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
---
 drivers/net/usb/smsc95xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index df9179a..4db0840 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1037,7 +1037,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->net->netdev_ops = &smsc95xx_netdev_ops;
 	dev->net->ethtool_ops = &smsc95xx_ethtool_ops;
 	dev->net->flags |= IFF_MULTICAST;
-	dev->net->hard_header_len += SMSC95XX_TX_OVERHEAD;
+	dev->net->hard_header_len += SMSC95XX_TX_OVERHEAD_CSUM;
 	return 0;
 }
 
-- 
1.6.6.1

--

From: Steve Glendinning
Date: Wednesday, March 10, 2010 - 11:32 am

Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
---
 drivers/net/usb/smsc95xx.c |   31 ++++++++++++++++---------------
 1 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index a2cb77d..a55dc27 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -45,6 +45,7 @@
 #define SMSC95XX_INTERNAL_PHY_ID	(1)
 #define SMSC95XX_TX_OVERHEAD		(8)
 #define SMSC95XX_TX_OVERHEAD_CSUM	(12)
+#define USB_VENDOR_ID_SMSC		(0x0424)
 
 struct smsc95xx_priv {
 	u32 mac_cr;
@@ -1225,77 +1226,77 @@ static const struct driver_info smsc95xx_info = {
 static const struct usb_device_id products[] = {
 	{
 		/* SMSC9500 USB Ethernet Device */
-		USB_DEVICE(0x0424, 0x9500),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9500),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9505 USB Ethernet Device */
-		USB_DEVICE(0x0424, 0x9505),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9505),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9500A USB Ethernet Device */
-		USB_DEVICE(0x0424, 0x9E00),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9E00),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9505A USB Ethernet Device */
-		USB_DEVICE(0x0424, 0x9E01),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9E01),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9512/9514 USB Hub & Ethernet Device */
-		USB_DEVICE(0x0424, 0xec00),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0xec00),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9500 USB Ethernet Device (SAL10) */
-		USB_DEVICE(0x0424, 0x9900),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9900),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9505 USB Ethernet Device (SAL10) */
-		USB_DEVICE(0x0424, 0x9901),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9901),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9500A USB Ethernet Device (SAL10) ...
From: Steve Glendinning
Date: Wednesday, March 10, 2010 - 11:32 am

This patch adds additional checks of the values returned by
smsc95xx_(read|write)_reg, and wraps their common patterns
in macros.

Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
---
 drivers/net/usb/smsc95xx.c |  302 +++++++++++++++++++-------------------------
 1 files changed, 128 insertions(+), 174 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 56fe73d..aeaa793 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -47,6 +47,15 @@
 #define SMSC95XX_TX_OVERHEAD_CSUM	(12)
 #define USB_VENDOR_ID_SMSC		(0x0424)
 
+#define check_warn(ret, fmt, args...) \
+	({ if (ret < 0) netdev_warn(dev->net, fmt, ##args); })
+
+#define check_warn_return(ret, format, args...) \
+	({ if (ret < 0) { netdev_warn(dev->net, fmt, ##args); return ret; } })
+
+#define check_warn_goto_done(ret, format, args...) \
+	({ if (ret < 0) { netdev_warn(dev->net, fmt, ##args); goto done; } })
+
 struct smsc95xx_priv {
 	u32 mac_cr;
 	spinlock_t mac_cr_lock;
@@ -63,7 +72,8 @@ static int turbo_mode = true;
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 
-static int smsc95xx_read_reg(struct usbnet *dev, u32 index, u32 *data)
+static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
+					  u32 *data)
 {
 	u32 *buf = kmalloc(4, GFP_KERNEL);
 	int ret;
@@ -88,7 +98,8 @@ static int smsc95xx_read_reg(struct usbnet *dev, u32 index, u32 *data)
 	return ret;
 }
 
-static int smsc95xx_write_reg(struct usbnet *dev, u32 index, u32 data)
+static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
+					   u32 data)
 {
 	u32 *buf = kmalloc(4, GFP_KERNEL);
 	int ret;
@@ -120,9 +131,12 @@ static int smsc95xx_phy_wait_not_busy(struct usbnet *dev)
 {
 	unsigned long start_time = jiffies;
 	u32 val;
+	int ret;
 
 	do {
-		smsc95xx_read_reg(dev, MII_ADDR, &val);
+		ret = smsc95xx_read_reg(dev, MII_ADDR, ...
From: Steve Glendinning
Date: Wednesday, March 10, 2010 - 11:32 am

Removes unnecessary variables as smsc95xx_write_reg takes its
value by parameter.  Early versions passed this parameter by
reference.

Also replace hardcoded interrupt status value with a #define

Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
---
 drivers/net/usb/smsc95xx.c |   29 +++++++++--------------------
 drivers/net/usb/smsc95xx.h |    1 +
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index aeaa793..18adc39 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -485,15 +485,13 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 	struct ethtool_cmd ecmd;
 	unsigned long flags;
 	u16 lcladv, rmtadv;
-	u32 intdata;
 	int ret;
 
 	/* clear interrupt status */
 	ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
 	check_warn_return(ret, "Error reading PHY_INT_SRC");
 
-	intdata = 0xFFFFFFFF;
-	ret = smsc95xx_write_reg(dev, INT_STS, intdata);
+	ret = smsc95xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL_);
 	check_warn_return(ret, "Error writing INT_STS");
 
 	mii_check_media(mii, 1, 1);
@@ -700,7 +698,6 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
 {
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
 	unsigned long flags;
-	u32 reg_val;
 	int ret;
 
 	/* Enable Tx at MAC */
@@ -712,8 +709,7 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
 	check_warn_return(ret, "Failed to write MAC_CR: %d\n", ret);
 
 	/* Enable Tx at SCSRs */
-	reg_val = TX_CFG_ON_;
-	ret = smsc95xx_write_reg(dev, TX_CFG, reg_val);
+	ret = smsc95xx_write_reg(dev, TX_CFG, TX_CFG_ON_);
 	check_warn_return(ret, "Failed to write TX_CFG: %d\n", ret);
 
 	return 0;
@@ -786,8 +782,7 @@ static int smsc95xx_reset(struct usbnet *dev)
 
 	netif_dbg(dev, ifup, dev->net, "entering smsc95xx_reset\n");
 
-	write_buf = HW_CFG_LRST_;
-	ret = smsc95xx_write_reg(dev, HW_CFG, write_buf);
+	ret = smsc95xx_write_reg(dev, HW_CFG, ...
From: Steve Glendinning
Date: Wednesday, March 10, 2010 - 11:32 am

During init, the device reset is unexpected to complete immediately,
so sleep before testing the condition rather than after it.

Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
---
 drivers/net/usb/smsc95xx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index a55dc27..56fe73d 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -768,12 +768,12 @@ static int smsc95xx_reset(struct usbnet *dev)
 
 	timeout = 0;
 	do {
+		msleep(10);
 		ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
 		if (ret < 0) {
 			netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
 			return ret;
 		}
-		msleep(10);
 		timeout++;
 	} while ((read_buf & HW_CFG_LRST_) && (timeout < 100));
 
@@ -791,12 +791,12 @@ static int smsc95xx_reset(struct usbnet *dev)
 
 	timeout = 0;
 	do {
+		msleep(10);
 		ret = smsc95xx_read_reg(dev, PM_CTRL, &read_buf);
 		if (ret < 0) {
 			netdev_warn(dev->net, "Failed to read PM_CTRL: %d\n", ret);
 			return ret;
 		}
-		msleep(10);
 		timeout++;
 	} while ((read_buf & PM_CTL_PHY_RST_) && (timeout < 100));
 
-- 
1.6.6.1

--

From: David Miller
Date: Monday, March 15, 2010 - 3:39 pm

From: Steve Glendinning <steve.glendinning@smsc.com>

I don't see any real need for this, really.  It's a constant,
device ID tables are constants and people know how to go lookup
a vendor ID if they really want to know who it is.  This doesn't
add any readability to the driver at all.

Also you're mixing cleanups and bug fixes into your patch
set.  Please seperate out the pure bug fixes so I can apply
them to net-2.6  The rest can go in later when net-next-2.6
opens up.
--

From: Steve Glendinning
Date: Wednesday, March 10, 2010 - 11:32 am

This patch ensures the PHY correctly completes its reset before
setting register values.

Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
---
 drivers/net/usb/smsc95xx.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 4db0840..a2cb77d 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -709,6 +709,8 @@ static void smsc95xx_start_rx_path(struct usbnet *dev)
 
 static int smsc95xx_phy_initialize(struct usbnet *dev)
 {
+	int bmcr, timeout = 0;
+
 	/* Initialize MII structure */
 	dev->mii.dev = dev->net;
 	dev->mii.mdio_read = smsc95xx_mdio_read;
@@ -717,7 +719,20 @@ static int smsc95xx_phy_initialize(struct usbnet *dev)
 	dev->mii.reg_num_mask = 0x1f;
 	dev->mii.phy_id = SMSC95XX_INTERNAL_PHY_ID;
 
+	/* reset phy and wait for reset to complete */
 	smsc95xx_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
+
+	do {
+		msleep(10);
+		bmcr = smsc95xx_mdio_read(dev->net, dev->mii.phy_id, MII_BMCR);
+		timeout++;
+	} while ((bmcr & MII_BMCR) && (timeout < 100));
+
+	if (timeout >= 100) {
+		netdev_warn(dev->net, "timeout on PHY Reset");
+		return -EIO;
+	}
+
 	smsc95xx_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
 		ADVERTISE_ALL | ADVERTISE_CSMA | ADVERTISE_PAUSE_CAP |
 		ADVERTISE_PAUSE_ASYM);
-- 
1.6.6.1

--

Previous thread: [PATCH 1/4] smsc75xx: SMSC LAN75xx USB gigabit ethernet adapter driver by Steve Glendinning on Wednesday, March 10, 2010 - 11:26 am. (1 message)

Next thread: [PATCH 0/6] smsc95xx: minor fixes and tidy-ups by Steve Glendinning on Wednesday, March 10, 2010 - 11:32 am. (1 message)