[PATCH 0/6] smsc95xx: minor fixes and tidy-ups

[PATCH 0/6] smsc95xx: minor fixes and tidy-ups

[Steve Glendinning]

This patchset contains minor bugfixes and tidy-ups to the smsc95xx
driver.  Please consider for net-next.

Steve Glendinning (6):
  smsc95xx: change hard_header_len to include csum offload bytes
  smsc95xx: wait for PHY to complete reset during init
  smsc95xx: add constant definition for SMSC USB vendor id
  smsc95xx: sleep before read for lengthy operations
  smsc95xx: check return code from control messages
  smsc95xx: remove unnecessary variables

 drivers/net/usb/smsc95xx.c |  379 ++++++++++++++++++++------------------------
 drivers/net/usb/smsc95xx.h |    1 +
 2 files changed, 170 insertions(+), 210 deletions(-)

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

[Steve Glendinning]

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

[PATCH 2/6] smsc95xx: wait for PHY to complete reset during init

[Steve Glendinning]

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

[PATCH 3/6] smsc95xx: add constant definition for SMSC USB vendor id

[Steve Glendinning]

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) */
-		USB_DEVICE(0x0424, 0x9902),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9902),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9505A USB Ethernet Device (SAL10) */
-		USB_DEVICE(0x0424, 0x9903),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9903),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9512/9514 USB Hub & Ethernet Device (SAL10) */
-		USB_DEVICE(0x0424, 0x9904),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9904),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9500A USB Ethernet Device (HAL) */
-		USB_DEVICE(0x0424, 0x9905),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9905),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9505A USB Ethernet Device (HAL) */
-		USB_DEVICE(0x0424, 0x9906),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9906),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9500 USB Ethernet Device (Alternate ID) */
-		USB_DEVICE(0x0424, 0x9907),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9907),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9500A USB Ethernet Device (Alternate ID) */
-		USB_DEVICE(0x0424, 0x9908),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9908),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{
 		/* SMSC9512/9514 USB Hub & Ethernet Device (Alternate ID) */
-		USB_DEVICE(0x0424, 0x9909),
+		USB_DEVICE(USB_VENDOR_ID_SMSC, 0x9909),
 		.driver_info = (unsigned long) &smsc95xx_info,
 	},
 	{ },		/* END */
-- 
1.6.6.1

[PATCH 4/6] smsc95xx: sleep before read for lengthy operations

[Steve Glendinning]

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

[PATCH 5/6] smsc95xx: check return code from control messages

[Steve Glendinning]

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, &val);
+		check_warn_return(ret, "Error reading MII_ACCESS");
+
 		if (!(val & MII_BUSY_))
 			return 0;
 	} while (!time_after(jiffies, start_time + HZ));
@@ -134,33 +148,32 @@ static int smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx)
 {
 	struct usbnet *dev = netdev_priv(netdev);
 	u32 val, addr;
+	int ret;
 
 	mutex_lock(&dev->phy_mutex);
 
 	/* confirm MII not busy */
-	if (smsc95xx_phy_wait_not_busy(dev)) {
-		netdev_warn(dev->net, "MII is busy in smsc95xx_mdio_read\n");
-		mutex_unlock(&dev->phy_mutex);
-		return -EIO;
-	}
+	ret = smsc95xx_phy_wait_not_busy(dev);
+	check_warn_goto_done(ret, "MII is busy in smsc95xx_mdio_read");
 
 	/* set the address, index & direction (read from PHY) */
 	phy_id &= dev->mii.phy_id_mask;
 	idx &= dev->mii.reg_num_mask;
 	addr = (phy_id << 11) | (idx << 6) | MII_READ_;
-	smsc95xx_write_reg(dev, MII_ADDR, addr);
+	ret = smsc95xx_write_reg(dev, MII_ADDR, addr);
+	check_warn_goto_done(ret, "Error writing MII_ACCESS");
 
-	if (smsc95xx_phy_wait_not_busy(dev)) {
-		netdev_warn(dev->net, "Timed out reading MII reg %02X\n", idx);
-		mutex_unlock(&dev->phy_mutex);
-		return -EIO;
-	}
+	ret = smsc95xx_phy_wait_not_busy(dev);
+	check_warn_goto_done(ret, "Timed out reading MII reg %02X", idx);
 
-	smsc95xx_read_reg(dev, MII_DATA, &val);
+	ret = smsc95xx_read_reg(dev, MII_DATA, &val);
+	check_warn_goto_done(ret, "Error reading MII_DATA");
 
-	mutex_unlock(&dev->phy_mutex);
+	ret = (u16)(val & 0xFFFF);
 
-	return (u16)(val & 0xFFFF);
+done:
+	mutex_unlock(&dev->phy_mutex);
+	return ret;
 }
 
 static void smsc95xx_mdio_write(struct net_device *netdev, int phy_id, int idx,
@@ -168,28 +181,29 @@ static void smsc95xx_mdio_write(struct net_device *netdev, int phy_id, int idx,
 {
 	struct usbnet *dev = netdev_priv(netdev);
 	u32 val, addr;
+	int ret;
 
 	mutex_lock(&dev->phy_mutex);
 
 	/* confirm MII not busy */
-	if (smsc95xx_phy_wait_not_busy(dev)) {
-		netdev_warn(dev->net, "MII is busy in smsc95xx_mdio_write\n");
-		mutex_unlock(&dev->phy_mutex);
-		return;
-	}
+	ret = smsc95xx_phy_wait_not_busy(dev);
+	check_warn_goto_done(ret, "MII is busy in smsc95xx_mdio_write");
 
 	val = regval;
-	smsc95xx_write_reg(dev, MII_DATA, val);
+	ret = smsc95xx_write_reg(dev, MII_DATA, val);
+	check_warn_goto_done(ret, "Error writing MII_DATA");
 
 	/* set the address, index & direction (write to PHY) */
 	phy_id &= dev->mii.phy_id_mask;
 	idx &= dev->mii.reg_num_mask;
 	addr = (phy_id << 11) | (idx << 6) | MII_WRITE_;
-	smsc95xx_write_reg(dev, MII_ADDR, addr);
+	ret = smsc95xx_write_reg(dev, MII_ADDR, addr);
+	check_warn_goto_done(ret, "Error writing MII_ACCESS");
 
-	if (smsc95xx_phy_wait_not_busy(dev))
-		netdev_warn(dev->net, "Timed out writing MII reg %02X\n", idx);
+	ret = smsc95xx_phy_wait_not_busy(dev);
+	check_warn_goto_done(ret, "Timed out writing MII reg %02X", idx);
 
+done:
 	mutex_unlock(&dev->phy_mutex);
 }
 
@@ -197,9 +211,12 @@ static int smsc95xx_wait_eeprom(struct usbnet *dev)
 {
 	unsigned long start_time = jiffies;
 	u32 val;
+	int ret;
 
 	do {
-		smsc95xx_read_reg(dev, E2P_CMD, &val);
+		ret = smsc95xx_read_reg(dev, E2P_CMD, &val);
+		check_warn_return(ret, "Error reading E2P_CMD");
+
 		if (!(val & E2P_CMD_BUSY_) || (val & E2P_CMD_TIMEOUT_))
 			break;
 		udelay(40);
@@ -217,9 +234,11 @@ static int smsc95xx_eeprom_confirm_not_busy(struct usbnet *dev)
 {
 	unsigned long start_time = jiffies;
 	u32 val;
+	int ret;
 
 	do {
-		smsc95xx_read_reg(dev, E2P_CMD, &val);
+		ret = smsc95xx_read_reg(dev, E2P_CMD, &val);
+		check_warn_return(ret, "Error reading E2P_CMD");
 
 		if (!(val & E2P_CMD_BUSY_))
 			return 0;
@@ -246,13 +265,15 @@ static int smsc95xx_read_eeprom(struct usbnet *dev, u32 offset, u32 length,
 
 	for (i = 0; i < length; i++) {
 		val = E2P_CMD_BUSY_ | E2P_CMD_READ_ | (offset & E2P_CMD_ADDR_);
-		smsc95xx_write_reg(dev, E2P_CMD, val);
+		ret = smsc95xx_write_reg(dev, E2P_CMD, val);
+		check_warn_return(ret, "Error writing E2P_CMD");
 
 		ret = smsc95xx_wait_eeprom(dev);
 		if (ret < 0)
 			return ret;
 
-		smsc95xx_read_reg(dev, E2P_DATA, &val);
+		ret = smsc95xx_read_reg(dev, E2P_DATA, &val);
+		check_warn_return(ret, "Error reading E2P_DATA");
 
 		data[i] = val & 0xFF;
 		offset++;
@@ -276,7 +297,8 @@ static int smsc95xx_write_eeprom(struct usbnet *dev, u32 offset, u32 length,
 
 	/* Issue write/erase enable command */
 	val = E2P_CMD_BUSY_ | E2P_CMD_EWEN_;
-	smsc95xx_write_reg(dev, E2P_CMD, val);
+	ret = smsc95xx_write_reg(dev, E2P_CMD, val);
+	check_warn_return(ret, "Error writing E2P_CMD");
 
 	ret = smsc95xx_wait_eeprom(dev);
 	if (ret < 0)
@@ -286,11 +308,13 @@ static int smsc95xx_write_eeprom(struct usbnet *dev, u32 offset, u32 length,
 
 		/* Fill data register */
 		val = data[i];
-		smsc95xx_write_reg(dev, E2P_DATA, val);
+		ret = smsc95xx_write_reg(dev, E2P_DATA, val);
+		check_warn_return(ret, "Error writing E2P_DATA");
 
 		/* Send "write" command */
 		val = E2P_CMD_BUSY_ | E2P_CMD_WRITE_ | (offset & E2P_CMD_ADDR_);
-		smsc95xx_write_reg(dev, E2P_CMD, val);
+		ret = smsc95xx_write_reg(dev, E2P_CMD, val);
+		check_warn_return(ret, "Error writing E2P_CMD");
 
 		ret = smsc95xx_wait_eeprom(dev);
 		if (ret < 0)
@@ -308,8 +332,7 @@ static void smsc95xx_async_cmd_callback(struct urb *urb)
 	struct usbnet *dev = usb_context->dev;
 	int status = urb->status;
 
-	if (status < 0)
-		netdev_warn(dev->net, "async callback failed with %d\n", status);
+	check_warn(status, "async callback failed with %d\n", status);
 
 	kfree(usb_context);
 	usb_free_urb(urb);
@@ -415,16 +438,14 @@ static void smsc95xx_set_multicast(struct net_device *netdev)
 	smsc95xx_write_reg_async(dev, MAC_CR, &pdata->mac_cr);
 }
 
-static void smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
-					    u16 lcladv, u16 rmtadv)
+static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
+					   u16 lcladv, u16 rmtadv)
 {
 	u32 flow, afc_cfg = 0;
 
 	int ret = smsc95xx_read_reg(dev, AFC_CFG, &afc_cfg);
-	if (ret < 0) {
-		netdev_warn(dev->net, "error reading AFC_CFG\n");
-		return;
-	}
+	check_warn_return(ret, "Error reading AFC_CFG");
+
 
 	if (duplex == DUPLEX_FULL) {
 		u8 cap = mii_resolve_flowctrl_fdx(lcladv, rmtadv);
@@ -448,8 +469,13 @@ static void smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
 		afc_cfg |= 0xF;
 	}
 
-	smsc95xx_write_reg(dev, FLOW, flow);
-	smsc95xx_write_reg(dev,	AFC_CFG, afc_cfg);
+	ret = smsc95xx_write_reg(dev, FLOW, flow);
+	check_warn_return(ret, "Error writing FLOW");
+
+	ret = smsc95xx_write_reg(dev,	AFC_CFG, afc_cfg);
+	check_warn_return(ret, "Error writing AFC_CFG");
+
+	return 0;
 }
 
 static int smsc95xx_link_reset(struct usbnet *dev)
@@ -460,11 +486,15 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 	unsigned long flags;
 	u16 lcladv, rmtadv;
 	u32 intdata;
+	int ret;
 
 	/* clear interrupt status */
-	smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
+	ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
+	check_warn_return(ret, "Error reading PHY_INT_SRC");
+
 	intdata = 0xFFFFFFFF;
-	smsc95xx_write_reg(dev, INT_STS, intdata);
+	ret = smsc95xx_write_reg(dev, INT_STS, intdata);
+	check_warn_return(ret, "Error writing INT_STS");
 
 	mii_check_media(mii, 1, 1);
 	mii_ethtool_gset(&dev->mii, &ecmd);
@@ -484,7 +514,8 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 	}
 	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
 
-	smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
+	ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
+	check_warn_return(ret, "Error writing MAC_CR");
 
 	smsc95xx_phy_update_flowcontrol(dev, ecmd.duplex, lcladv, rmtadv);
 
@@ -519,10 +550,7 @@ static int smsc95xx_set_csums(struct usbnet *dev)
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
 	u32 read_buf;
 	int ret = smsc95xx_read_reg(dev, COE_CR, &read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to read COE_CR: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to read COE_CR: %d\n", ret);
 
 	if (pdata->use_tx_csum)
 		read_buf |= Tx_COE_EN_;
@@ -535,10 +563,7 @@ static int smsc95xx_set_csums(struct usbnet *dev)
 		read_buf &= ~Rx_COE_EN_;
 
 	ret = smsc95xx_write_reg(dev, COE_CR, read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write COE_CR: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write COE_CR: %d\n", ret);
 
 	netif_dbg(dev, hw, dev->net, "COE_CR = 0x%08x\n", read_buf);
 	return 0;
@@ -662,50 +687,53 @@ static int smsc95xx_set_mac_address(struct usbnet *dev)
 	int ret;
 
 	ret = smsc95xx_write_reg(dev, ADDRL, addr_lo);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write ADDRL: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write ADDRL: %d\n", ret);
 
 	ret = smsc95xx_write_reg(dev, ADDRH, addr_hi);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write ADDRH: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write ADDRH: %d\n", ret);
 
 	return 0;
 }
 
 /* starts the TX path */
-static void smsc95xx_start_tx_path(struct usbnet *dev)
+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 */
 	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
 	pdata->mac_cr |= MAC_CR_TXEN_;
 	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
 
-	smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
+	ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
+	check_warn_return(ret, "Failed to write MAC_CR: %d\n", ret);
 
 	/* Enable Tx at SCSRs */
 	reg_val = TX_CFG_ON_;
-	smsc95xx_write_reg(dev, TX_CFG, reg_val);
+	ret = smsc95xx_write_reg(dev, TX_CFG, reg_val);
+	check_warn_return(ret, "Failed to write TX_CFG: %d\n", ret);
+
+	return 0;
 }
 
 /* Starts the Receive path */
-static void smsc95xx_start_rx_path(struct usbnet *dev)
+static int smsc95xx_start_rx_path(struct usbnet *dev)
 {
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
 	unsigned long flags;
+	int ret;
 
 	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
 	pdata->mac_cr |= MAC_CR_RXEN_;
 	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
 
-	smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
+	ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
+	check_warn_return(ret, "Failed to write MAC_CR: %d\n", ret);
+
+	return 0;
 }
 
 static int smsc95xx_phy_initialize(struct usbnet *dev)
@@ -760,20 +788,13 @@ static int smsc95xx_reset(struct usbnet *dev)
 
 	write_buf = HW_CFG_LRST_;
 	ret = smsc95xx_write_reg(dev, HW_CFG, write_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write HW_CFG_LRST_ bit in HW_CFG register, ret = %d\n",
-			    ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write HW_CFG: %d\n", ret);
 
 	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;
-		}
+		check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
 		timeout++;
 	} while ((read_buf & HW_CFG_LRST_) && (timeout < 100));
 
@@ -784,19 +805,13 @@ static int smsc95xx_reset(struct usbnet *dev)
 
 	write_buf = PM_CTL_PHY_RST_;
 	ret = smsc95xx_write_reg(dev, PM_CTRL, write_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write PM_CTRL: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write PM_CTRL: %d\n", ret);
 
 	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;
-		}
+		check_warn_return(ret, "Failed to read PM_CTRL: %d\n", ret);
 		timeout++;
 	} while ((read_buf & PM_CTL_PHY_RST_) && (timeout < 100));
 
@@ -815,10 +830,7 @@ static int smsc95xx_reset(struct usbnet *dev)
 		  "MAC Address: %pM\n", dev->net->dev_addr);
 
 	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;
-	}
+	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
 
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from HW_CFG : 0x%08x\n", read_buf);
@@ -826,17 +838,11 @@ static int smsc95xx_reset(struct usbnet *dev)
 	read_buf |= HW_CFG_BIR_;
 
 	ret = smsc95xx_write_reg(dev, HW_CFG, read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write HW_CFG_BIR_ bit in HW_CFG register, ret = %d\n",
-			    ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write HW_CFG: %d\n", ret);
 
 	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;
-	}
+	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from HW_CFG after writing HW_CFG_BIR_: 0x%08x\n",
 		  read_buf);
@@ -856,41 +862,29 @@ static int smsc95xx_reset(struct usbnet *dev)
 		  "rx_urb_size=%ld\n", (ulong)dev->rx_urb_size);
 
 	ret = smsc95xx_write_reg(dev, BURST_CAP, burst_cap);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write BURST_CAP: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write BURST_CAP: %d\n", ret);
 
 	ret = smsc95xx_read_reg(dev, BURST_CAP, &read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to read BURST_CAP: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to read BURST_CAP: %d\n", ret);
+
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from BURST_CAP after writing: 0x%08x\n",
 		  read_buf);
 
 	read_buf = DEFAULT_BULK_IN_DELAY;
 	ret = smsc95xx_write_reg(dev, BULK_IN_DLY, read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "ret = %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write BULK_IN_DLY: %d\n", ret);
 
 	ret = smsc95xx_read_reg(dev, BULK_IN_DLY, &read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to read BULK_IN_DLY: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to read BULK_IN_DLY: %d\n", ret);
+
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from BULK_IN_DLY after writing: 0x%08x\n",
 		  read_buf);
 
 	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;
-	}
+	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from HW_CFG: 0x%08x\n", read_buf);
 
@@ -903,103 +897,66 @@ static int smsc95xx_reset(struct usbnet *dev)
 	read_buf |= NET_IP_ALIGN << 9;
 
 	ret = smsc95xx_write_reg(dev, HW_CFG, read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write HW_CFG register, ret=%d\n",
-			    ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write HW_CFG: %d\n", ret);
 
 	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;
-	}
+	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from HW_CFG after writing: 0x%08x\n", read_buf);
 
 	write_buf = 0xFFFFFFFF;
 	ret = smsc95xx_write_reg(dev, INT_STS, write_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write INT_STS register, ret=%d\n",
-			    ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write INT_STS: %d\n", ret);
 
 	ret = smsc95xx_read_reg(dev, ID_REV, &read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to read ID_REV: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to read ID_REG: %d\n", ret);
+
 	netif_dbg(dev, ifup, dev->net, "ID_REV = 0x%08x\n", read_buf);
 
 	/* Configure GPIO pins as LED outputs */
 	write_buf = LED_GPIO_CFG_SPD_LED | LED_GPIO_CFG_LNK_LED |
 		LED_GPIO_CFG_FDX_LED;
 	ret = smsc95xx_write_reg(dev, LED_GPIO_CFG, write_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write LED_GPIO_CFG register, ret=%d\n",
-			    ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write LED_GPIO_CFG: %d\n", ret);
 
 	/* Init Tx */
 	write_buf = 0;
 	ret = smsc95xx_write_reg(dev, FLOW, write_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write FLOW: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write FLOW: %d\n", ret);
 
 	read_buf = AFC_CFG_DEFAULT;
 	ret = smsc95xx_write_reg(dev, AFC_CFG, read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write AFC_CFG: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write AFC_CFG: %d\n", ret);
 
 	/* Don't need mac_cr_lock during initialisation */
 	ret = smsc95xx_read_reg(dev, MAC_CR, &pdata->mac_cr);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to read MAC_CR: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to read MAC_CR: %d\n", ret);
 
 	/* Init Rx */
 	/* Set Vlan */
 	write_buf = (u32)ETH_P_8021Q;
 	ret = smsc95xx_write_reg(dev, VLAN1, write_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write VAN1: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write VLAN1: %d\n", ret);
 
 	/* Enable or disable checksum offload engines */
 	ethtool_op_set_tx_hw_csum(netdev, pdata->use_tx_csum);
 	ret = smsc95xx_set_csums(dev);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to set csum offload: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to set csum offload: %d\n", ret);
 
 	smsc95xx_set_multicast(dev->net);
 
-	if (smsc95xx_phy_initialize(dev) < 0)
-		return -EIO;
+	ret = smsc95xx_phy_initialize(dev);
+	check_warn_return(ret, "Failed to init PHY: %d\n", ret);
 
 	ret = smsc95xx_read_reg(dev, INT_EP_CTL, &read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to read INT_EP_CTL: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to read INT_EP_CTL: %d\n", ret);
 
 	/* enable PHY interrupts */
 	read_buf |= INT_EP_CTL_PHY_INT_;
 
 	ret = smsc95xx_write_reg(dev, INT_EP_CTL, read_buf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to write INT_EP_CTL: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "Failed to write INT_EP_CTL: %d\n", ret);
 
 	smsc95xx_start_tx_path(dev);
 	smsc95xx_start_rx_path(dev);
@@ -1028,10 +985,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	printk(KERN_INFO SMSC_CHIPNAME " v" SMSC_DRIVER_VERSION "\n");
 
 	ret = usbnet_get_endpoints(dev, intf);
-	if (ret < 0) {
-		netdev_warn(dev->net, "usbnet_get_endpoints failed: %d\n", ret);
-		return ret;
-	}
+	check_warn_return(ret, "usbnet_get_endpoints failed: %d\n", ret);
 
 	dev->data[0] = (unsigned long)kzalloc(sizeof(struct smsc95xx_priv),
 		GFP_KERNEL);
-- 
1.6.6.1

[PATCH 6/6] smsc95xx: remove unnecessary variables

[Steve Glendinning]

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, HW_CFG_LRST_);
 	check_warn_return(ret, "Failed to write HW_CFG: %d\n", ret);
 
 	timeout = 0;
@@ -803,8 +798,7 @@ static int smsc95xx_reset(struct usbnet *dev)
 		return ret;
 	}
 
-	write_buf = PM_CTL_PHY_RST_;
-	ret = smsc95xx_write_reg(dev, PM_CTRL, write_buf);
+	ret = smsc95xx_write_reg(dev, PM_CTRL, PM_CTL_PHY_RST_);
 	check_warn_return(ret, "Failed to write PM_CTRL: %d\n", ret);
 
 	timeout = 0;
@@ -871,8 +865,7 @@ static int smsc95xx_reset(struct usbnet *dev)
 		  "Read Value from BURST_CAP after writing: 0x%08x\n",
 		  read_buf);
 
-	read_buf = DEFAULT_BULK_IN_DELAY;
-	ret = smsc95xx_write_reg(dev, BULK_IN_DLY, read_buf);
+	ret = smsc95xx_write_reg(dev, BULK_IN_DLY, DEFAULT_BULK_IN_DELAY);
 	check_warn_return(ret, "Failed to write BULK_IN_DLY: %d\n", ret);
 
 	ret = smsc95xx_read_reg(dev, BULK_IN_DLY, &read_buf);
@@ -905,8 +898,7 @@ static int smsc95xx_reset(struct usbnet *dev)
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from HW_CFG after writing: 0x%08x\n", read_buf);
 
-	write_buf = 0xFFFFFFFF;
-	ret = smsc95xx_write_reg(dev, INT_STS, write_buf);
+	ret = smsc95xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL_);
 	check_warn_return(ret, "Failed to write INT_STS: %d\n", ret);
 
 	ret = smsc95xx_read_reg(dev, ID_REV, &read_buf);
@@ -921,12 +913,10 @@ static int smsc95xx_reset(struct usbnet *dev)
 	check_warn_return(ret, "Failed to write LED_GPIO_CFG: %d\n", ret);
 
 	/* Init Tx */
-	write_buf = 0;
-	ret = smsc95xx_write_reg(dev, FLOW, write_buf);
+	ret = smsc95xx_write_reg(dev, FLOW, 0);
 	check_warn_return(ret, "Failed to write FLOW: %d\n", ret);
 
-	read_buf = AFC_CFG_DEFAULT;
-	ret = smsc95xx_write_reg(dev, AFC_CFG, read_buf);
+	ret = smsc95xx_write_reg(dev, AFC_CFG, AFC_CFG_DEFAULT);
 	check_warn_return(ret, "Failed to write AFC_CFG: %d\n", ret);
 
 	/* Don't need mac_cr_lock during initialisation */
@@ -935,8 +925,7 @@ static int smsc95xx_reset(struct usbnet *dev)
 
 	/* Init Rx */
 	/* Set Vlan */
-	write_buf = (u32)ETH_P_8021Q;
-	ret = smsc95xx_write_reg(dev, VLAN1, write_buf);
+	ret = smsc95xx_write_reg(dev, VLAN1, (u32)ETH_P_8021Q);
 	check_warn_return(ret, "Failed to write VLAN1: %d\n", ret);
 
 	/* Enable or disable checksum offload engines */
diff --git a/drivers/net/usb/smsc95xx.h b/drivers/net/usb/smsc95xx.h
index 86bc449..a275b62 100644
--- a/drivers/net/usb/smsc95xx.h
+++ b/drivers/net/usb/smsc95xx.h
@@ -63,6 +63,7 @@
 #define INT_STS_TDFO_			(0x00001000)
 #define INT_STS_RXDF_			(0x00000800)
 #define INT_STS_GPIOS_			(0x000007FF)
+#define INT_STS_CLEAR_ALL_		(0xFFFFFFFF)
 
 #define RX_CFG				(0x0C)
 #define RX_FIFO_FLUSH_			(0x00000001)
-- 
1.6.6.1

Re: [PATCH 5/6] smsc95xx: check return code from control messages

[Steve.Glendinning]

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

Apologies, this patch is broken.

Please do not apply this anywhere, I'll send a replacement patch shortly.

Steve

Re: [PATCH 3/6] smsc95xx: add constant definition for SMSC USB vendor id

[David Miller]

From: Steve Glendinning <steve.glendinning@smsc.com>
Date: Wed, 10 Mar 2010 18:32:36 +0000

> Signed-off-by: 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.