[PATCH 0/4] Add a driver for the ASIX AX88172A

[PATCH 0/4] Add a driver for the ASIX AX88172A

[Christian Riesch]

Hi,

this patch adds a driver for the ASIX AX88172A USB 2.0 to 10/100M
Fast Ethernet Controller.

Although this chip is already supported by the AX88772 code in
drivers/net/usb/asix.c, I submit a new driver since the existing
driver lacks an important feature: It only supports an
Ethernet connection using the internal PHY embedded in the AX88172A.

The driver for the AX88172A is based on drivers/net/usb/asix.c
and the work of Michael Riesch <michael@riesch.at>.

The first patch in the patchset fixes checkpatch warnings in asix.c.

The second and the third patch factor out common code which is shared
between the existing drivers and the new driver for the AX88172A.

The fourth patch finally adds support for the AX88172A.

The patchset applies on top of net-next.

I have a few questions:

1) Is it ok to factor out the common code like I did? Or should
   it go into a separate kernel module?

2) phylib/usbnet: Currently I have an empty .status function
   in my const struct driver_info ax88172a_info. I think this
   notifies me of a link change, right? I don't know
   what I should do in this function. Trigger the phy state machine
   like a phy interrupt would do, since link changes are handled
   by the phy state machine?

I have tested the patch with the ASIX AX88172A demo board (using the
internal PHY) and a custom board (AX88172A and National DP83640 PHY).

I am looking forward to your comments. Since this is my first submission
of a larger patchset to a kernel mailing list, I would like to thank you
in advance for your patience :-) The patch is in an early state and
certainly needs improvement!

Regards, Christian

Christian Riesch (4):
  asix: Fix checkpatch warnings
  asix: Rename asix.c to asix_devices.c
  asix: Factor out common code
  asix: Add a new driver for the AX88172A

 drivers/net/usb/Makefile       |    1 +
 drivers/net/usb/asix.c         | 1660 ----------------------------------------
 drivers/net/usb/asix.h         |  211 +++++
 drivers/net/usb/asix_common.c  |  525 +++++++++++++
 drivers/net/usb/asix_devices.c | 1033 +++++++++++++++++++++++++
 drivers/net/usb/ax88172a.c     |  407 ++++++++++
 6 files changed, 2177 insertions(+), 1660 deletions(-)
 delete mode 100644 drivers/net/usb/asix.c
 create mode 100644 drivers/net/usb/asix.h
 create mode 100644 drivers/net/usb/asix_common.c
 create mode 100644 drivers/net/usb/asix_devices.c
 create mode 100644 drivers/net/usb/ax88172a.c

[PATCH 1/4] asix: Fix checkpatch warnings

[Christian Riesch]

Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
---
 drivers/net/usb/asix.c |  242 +++++++++++++++++++++++++-----------------------
 1 files changed, 126 insertions(+), 116 deletions(-)

diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index 3ae80ec..9210f40 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -20,8 +20,8 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-// #define	DEBUG			// error path messages, extra info
-// #define	VERBOSE			// more; success messages
+/* #define	DEBUG	*/		/* error path messages, extra info */
+/* #define	VERBOSE	*/		/* more; success messages */
 
 #include <linux/module.h>
 #include <linux/kmod.h>
@@ -81,7 +81,7 @@
 #define AX88172_MEDIUM_TX		0x04
 #define AX88172_MEDIUM_FC		0x10
 #define AX88172_MEDIUM_DEFAULT \
-		( AX88172_MEDIUM_FD | AX88172_MEDIUM_TX | AX88172_MEDIUM_FC )
+		(AX88172_MEDIUM_FD | AX88172_MEDIUM_TX | AX88172_MEDIUM_FC)
 
 #define AX_MCAST_FILTER_SIZE		8
 #define AX_MAX_MCAST			64
@@ -253,8 +253,8 @@ static void asix_async_cmd_callback(struct urb *urb)
 	int status = urb->status;
 
 	if (status < 0)
-		printk(KERN_DEBUG "asix_async_cmd_callback() failed with %d",
-			status);
+		pr_debug("asix_async_cmd_callback() failed with %d",
+			 status);
 
 	kfree(req);
 	usb_free_urb(urb);
@@ -262,7 +262,7 @@ static void asix_async_cmd_callback(struct urb *urb)
 
 static void
 asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
-				    u16 size, void *data)
+		     u16 size, void *data)
 {
 	struct usb_ctrlrequest *req;
 	int status;
@@ -399,9 +399,10 @@ static void asix_status(struct usbnet *dev, struct urb *urb)
 	if (netif_carrier_ok(dev->net) != link) {
 		if (link) {
 			netif_carrier_on(dev->net);
-			usbnet_defer_kevent (dev, EVENT_LINK_RESET );
-		} else
+			usbnet_defer_kevent(dev, EVENT_LINK_RESET);
+		} else {
 			netif_carrier_off(dev->net);
+		}
 		netdev_dbg(dev->net, "Link Status is: %d\n", link);
 	}
 }
@@ -432,7 +433,8 @@ static inline int asix_get_phy_addr(struct usbnet *dev)
 	netdev_dbg(dev->net, "asix_get_phy_addr()\n");
 
 	if (ret < 0) {
-		netdev_err(dev->net, "Error reading PHYID register: %02x\n", ret);
+		netdev_err(dev->net, "Error reading PHYID register: %02x\n",
+			   ret);
 		goto out;
 	}
 	netdev_dbg(dev->net, "asix_get_phy_addr() returning 0x%04x\n",
@@ -447,9 +449,10 @@ static int asix_sw_reset(struct usbnet *dev, u8 flags)
 {
 	int ret;
 
-        ret = asix_write_cmd(dev, AX_CMD_SW_RESET, flags, 0, 0, NULL);
+	ret = asix_write_cmd(dev, AX_CMD_SW_RESET, flags, 0, 0, NULL);
 	if (ret < 0)
-		netdev_err(dev->net, "Failed to send software reset: %02x\n", ret);
+		netdev_err(dev->net, "Failed to send software reset: %02x\n",
+			   ret);
 
 	return ret;
 }
@@ -460,7 +463,8 @@ static u16 asix_read_rx_ctl(struct usbnet *dev)
 	int ret = asix_read_cmd(dev, AX_CMD_READ_RX_CTL, 0, 0, 2, &v);
 
 	if (ret < 0) {
-		netdev_err(dev->net, "Error reading RX_CTL register: %02x\n", ret);
+		netdev_err(dev->net, "Error reading RX_CTL register: %02x\n",
+			   ret);
 		goto out;
 	}
 	ret = le16_to_cpu(v);
@@ -500,7 +504,8 @@ static int asix_write_medium_mode(struct usbnet *dev, u16 mode)
 {
 	int ret;
 
-	netdev_dbg(dev->net, "asix_write_medium_mode() - mode = 0x%04x\n", mode);
+	netdev_dbg(dev->net, "asix_write_medium_mode() - mode = 0x%04x\n",
+		   mode);
 	ret = asix_write_cmd(dev, AX_CMD_WRITE_MEDIUM_MODE, mode, 0, 0, NULL);
 	if (ret < 0)
 		netdev_err(dev->net, "Failed to write Medium Mode mode to 0x%04x: %02x\n",
@@ -559,7 +564,7 @@ static void asix_set_multicast(struct net_device *net)
 		}
 
 		asix_write_cmd_async(dev, AX_CMD_WRITE_MULTI_FILTER, 0, 0,
-				   AX_MCAST_FILTER_SIZE, data->multi_filter);
+				     AX_MCAST_FILTER_SIZE, data->multi_filter);
 
 		rx_ctl |= AX_RX_CTL_AM;
 	}
@@ -575,7 +580,7 @@ static int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)
 	mutex_lock(&dev->phy_mutex);
 	asix_set_sw_mii(dev);
 	asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
-				(__u16)loc, 2, &res);
+		      (__u16)loc, 2, &res);
 	asix_set_hw_mii(dev);
 	mutex_unlock(&dev->phy_mutex);
 
@@ -609,7 +614,8 @@ static u32 asix_get_phyid(struct usbnet *dev)
 
 	/* Poll for the rare case the FW or phy isn't ready yet.  */
 	for (i = 0; i < 100; i++) {
-		phy_reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_PHYSID1);
+		phy_reg = asix_mdio_read(dev->net, dev->mii.phy_id,
+					 MII_PHYSID1);
 		if (phy_reg != 0 && phy_reg != 0xFFFF)
 			break;
 		mdelay(1);
@@ -660,7 +666,7 @@ asix_set_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
 		opt |= AX_MONITOR_MAGIC;
 
 	if (asix_write_cmd(dev, AX_CMD_WRITE_MONITOR_MODE,
-			      opt, 0, 0, NULL) < 0)
+			   opt, 0, 0, NULL) < 0)
 		return -EINVAL;
 
 	return 0;
@@ -690,24 +696,24 @@ static int asix_get_eeprom(struct net_device *net,
 	eeprom->magic = AX_EEPROM_MAGIC;
 
 	/* ax8817x returns 2 bytes from eeprom on read */
-	for (i=0; i < eeprom->len / 2; i++) {
+	for (i = 0; i < eeprom->len / 2; i++) {
 		if (asix_read_cmd(dev, AX_CMD_READ_EEPROM,
-			eeprom->offset + i, 0, 2, &ebuf[i]) < 0)
+				  eeprom->offset + i, 0, 2, &ebuf[i]) < 0)
 			return -EINVAL;
 	}
 	return 0;
 }
 
-static void asix_get_drvinfo (struct net_device *net,
-				 struct ethtool_drvinfo *info)
+static void asix_get_drvinfo(struct net_device *net,
+			     struct ethtool_drvinfo *info)
 {
 	struct usbnet *dev = netdev_priv(net);
 	struct asix_data *data = (struct asix_data *)&dev->data;
 
 	/* Inherit standard device info */
 	usbnet_get_drvinfo(net, info);
-	strncpy (info->driver, DRIVER_NAME, sizeof info->driver);
-	strncpy (info->version, DRIVER_VERSION, sizeof info->version);
+	strncpy(info->driver, DRIVER_NAME, sizeof info->driver);
+	strncpy(info->version, DRIVER_VERSION, sizeof info->version);
 	info->eedump_len = data->eeprom_len;
 }
 
@@ -718,7 +724,7 @@ static u32 asix_get_link(struct net_device *net)
 	return mii_link_ok(&dev->mii);
 }
 
-static int asix_ioctl (struct net_device *net, struct ifreq *rq, int cmd)
+static int asix_ioctl(struct net_device *net, struct ifreq *rq, int cmd)
 {
 	struct usbnet *dev = netdev_priv(net);
 
@@ -744,7 +750,7 @@ static int asix_set_mac_address(struct net_device *net, void *p)
 	 * is tricky to free later */
 	memcpy(data->mac_addr, addr->sa_data, ETH_ALEN);
 	asix_write_cmd_async(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN,
-							data->mac_addr);
+			     data->mac_addr);
 
 	return 0;
 }
@@ -797,7 +803,7 @@ static void ax88172_set_multicast(struct net_device *net)
 		}
 
 		asix_write_cmd_async(dev, AX_CMD_WRITE_MULTI_FILTER, 0, 0,
-				   AX_MCAST_FILTER_SIZE, data->multi_filter);
+				     AX_MCAST_FILTER_SIZE, data->multi_filter);
 
 		rx_ctl |= 0x10;
 	}
@@ -831,7 +837,7 @@ static const struct net_device_ops ax88172_netdev_ops = {
 	.ndo_start_xmit		= usbnet_start_xmit,
 	.ndo_tx_timeout		= usbnet_tx_timeout,
 	.ndo_change_mtu		= usbnet_change_mtu,
-	.ndo_set_mac_address 	= eth_mac_addr,
+	.ndo_set_mac_address	= eth_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl		= asix_ioctl,
 	.ndo_set_rx_mode	= ax88172_set_multicast,
@@ -847,7 +853,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	data->eeprom_len = AX88172_EEPROM_LEN;
 
-	usbnet_get_endpoints(dev,intf);
+	usbnet_get_endpoints(dev, intf);
 
 	/* Toggle the GPIOs in a manufacturer/model specific way */
 	for (i = 2; i >= 0; i--) {
@@ -883,7 +889,7 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
 	asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
-		ADVERTISE_ALL | ADVERTISE_CSMA | ADVERTISE_PAUSE_CAP);
+			ADVERTISE_ALL | ADVERTISE_CSMA | ADVERTISE_PAUSE_CAP);
 	mii_nway_restart(&dev->mii);
 
 	return 0;
@@ -1040,7 +1046,7 @@ static const struct net_device_ops ax88772_netdev_ops = {
 	.ndo_start_xmit		= usbnet_start_xmit,
 	.ndo_tx_timeout		= usbnet_tx_timeout,
 	.ndo_change_mtu		= usbnet_change_mtu,
-	.ndo_set_mac_address 	= asix_set_mac_address,
+	.ndo_set_mac_address	= asix_set_mac_address,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl		= asix_ioctl,
 	.ndo_set_rx_mode        = asix_set_multicast,
@@ -1055,7 +1061,7 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	data->eeprom_len = AX88772_EEPROM_LEN;
 
-	usbnet_get_endpoints(dev,intf);
+	usbnet_get_endpoints(dev, intf);
 
 	/* Get the MAC address */
 	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
@@ -1143,16 +1149,18 @@ static int marvell_phy_init(struct usbnet *dev)
 	if (data->ledmode) {
 		reg = asix_mdio_read(dev->net, dev->mii.phy_id,
 			MII_MARVELL_LED_CTRL);
-		netdev_dbg(dev->net, "MII_MARVELL_LED_CTRL (1) = 0x%04x\n", reg);
+		netdev_dbg(dev->net, "MII_MARVELL_LED_CTRL (1) = 0x%04x\n",
+			   reg);
 
 		reg &= 0xf8ff;
 		reg |= (1 + 0x0100);
 		asix_mdio_write(dev->net, dev->mii.phy_id,
-			MII_MARVELL_LED_CTRL, reg);
+				MII_MARVELL_LED_CTRL, reg);
 
 		reg = asix_mdio_read(dev->net, dev->mii.phy_id,
-			MII_MARVELL_LED_CTRL);
-		netdev_dbg(dev->net, "MII_MARVELL_LED_CTRL (2) = 0x%04x\n", reg);
+				     MII_MARVELL_LED_CTRL);
+		netdev_dbg(dev->net, "MII_MARVELL_LED_CTRL (2) = 0x%04x\n",
+			   reg);
 		reg &= 0xfc0f;
 	}
 
@@ -1165,16 +1173,16 @@ static int rtl8211cl_phy_init(struct usbnet *dev)
 
 	netdev_dbg(dev->net, "rtl8211cl_phy_init()\n");
 
-	asix_mdio_write (dev->net, dev->mii.phy_id, 0x1f, 0x0005);
-	asix_mdio_write (dev->net, dev->mii.phy_id, 0x0c, 0);
-	asix_mdio_write (dev->net, dev->mii.phy_id, 0x01,
-		asix_mdio_read (dev->net, dev->mii.phy_id, 0x01) | 0x0080);
-	asix_mdio_write (dev->net, dev->mii.phy_id, 0x1f, 0);
+	asix_mdio_write(dev->net, dev->mii.phy_id, 0x1f, 0x0005);
+	asix_mdio_write(dev->net, dev->mii.phy_id, 0x0c, 0);
+	asix_mdio_write(dev->net, dev->mii.phy_id, 0x01,
+		asix_mdio_read(dev->net, dev->mii.phy_id, 0x01) | 0x0080);
+	asix_mdio_write(dev->net, dev->mii.phy_id, 0x1f, 0);
 
 	if (data->ledmode == 12) {
-		asix_mdio_write (dev->net, dev->mii.phy_id, 0x1f, 0x0002);
-		asix_mdio_write (dev->net, dev->mii.phy_id, 0x1a, 0x00cb);
-		asix_mdio_write (dev->net, dev->mii.phy_id, 0x1f, 0);
+		asix_mdio_write(dev->net, dev->mii.phy_id, 0x1f, 0x0002);
+		asix_mdio_write(dev->net, dev->mii.phy_id, 0x1a, 0x00cb);
+		asix_mdio_write(dev->net, dev->mii.phy_id, 0x1f, 0);
 	}
 
 	return 0;
@@ -1190,14 +1198,14 @@ static int marvell_led_status(struct usbnet *dev, u16 speed)
 	reg &= 0xfc0f;
 
 	switch (speed) {
-		case SPEED_1000:
-			reg |= 0x03e0;
-			break;
-		case SPEED_100:
-			reg |= 0x03b0;
-			break;
-		default:
-			reg |= 0x02f0;
+	case SPEED_1000:
+		reg |= 0x03e0;
+		break;
+	case SPEED_100:
+		reg |= 0x03b0;
+		break;
+	default:
+		reg |= 0x02f0;
 	}
 
 	netdev_dbg(dev->net, "marvell_led_status() writing 0x%04x\n", reg);
@@ -1265,8 +1273,9 @@ static int ax88178_reset(struct usbnet *dev)
 	if (data->phymode == PHY_MODE_MARVELL) {
 		marvell_phy_init(dev);
 		msleep(60);
-	} else if (data->phymode == PHY_MODE_RTL8211CL)
+	} else if (data->phymode == PHY_MODE_RTL8211CL) {
 		rtl8211cl_phy_init(dev);
+	}
 
 	asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
 			BMCR_RESET | BMCR_ANENABLE);
@@ -1394,11 +1403,11 @@ static const struct net_device_ops ax88178_netdev_ops = {
 	.ndo_stop		= usbnet_stop,
 	.ndo_start_xmit		= usbnet_start_xmit,
 	.ndo_tx_timeout		= usbnet_tx_timeout,
-	.ndo_set_mac_address 	= asix_set_mac_address,
+	.ndo_set_mac_address	= asix_set_mac_address,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_rx_mode	= asix_set_multicast,
-	.ndo_do_ioctl 		= asix_ioctl,
-	.ndo_change_mtu 	= ax88178_change_mtu,
+	.ndo_do_ioctl		= asix_ioctl,
+	.ndo_change_mtu		= ax88178_change_mtu,
 };
 
 static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
@@ -1409,7 +1418,7 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	data->eeprom_len = AX88772_EEPROM_LEN;
 
-	usbnet_get_endpoints(dev,intf);
+	usbnet_get_endpoints(dev, intf);
 
 	/* Get the MAC address */
 	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
@@ -1494,7 +1503,8 @@ static const struct driver_info ax88772_info = {
 	.status = asix_status,
 	.link_reset = ax88772_link_reset,
 	.reset = ax88772_reset,
-	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
+	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
+		 FLAG_MULTI_PACKET,
 	.rx_fixup = asix_rx_fixup,
 	.tx_fixup = asix_tx_fixup,
 };
@@ -1510,133 +1520,133 @@ static const struct driver_info ax88178_info = {
 	.tx_fixup = asix_tx_fixup,
 };
 
-static const struct usb_device_id	products [] = {
+static const struct usb_device_id	products[] = {
 {
-	// Linksys USB200M
-	USB_DEVICE (0x077b, 0x2226),
+	/* Linksys USB200M */
+	USB_DEVICE(0x077b, 0x2226),
 	.driver_info =	(unsigned long) &ax8817x_info,
 }, {
-	// Netgear FA120
-	USB_DEVICE (0x0846, 0x1040),
+	/* Netgear FA120 */
+	USB_DEVICE(0x0846, 0x1040),
 	.driver_info =  (unsigned long) &netgear_fa120_info,
 }, {
-	// DLink DUB-E100
-	USB_DEVICE (0x2001, 0x1a00),
+	/* DLink DUB-E100 */
+	USB_DEVICE(0x2001, 0x1a00),
 	.driver_info =  (unsigned long) &dlink_dub_e100_info,
 }, {
-	// Intellinet, ST Lab USB Ethernet
-	USB_DEVICE (0x0b95, 0x1720),
+	/* Intellinet, ST Lab USB Ethernet */
+	USB_DEVICE(0x0b95, 0x1720),
 	.driver_info =  (unsigned long) &ax8817x_info,
 }, {
-	// Hawking UF200, TrendNet TU2-ET100
-	USB_DEVICE (0x07b8, 0x420a),
+	/* Hawking UF200, TrendNet TU2-ET100 */
+	USB_DEVICE(0x07b8, 0x420a),
 	.driver_info =  (unsigned long) &hawking_uf200_info,
 }, {
-	// Billionton Systems, USB2AR
-	USB_DEVICE (0x08dd, 0x90ff),
+	/* Billionton Systems, USB2AR */
+	USB_DEVICE(0x08dd, 0x90ff),
 	.driver_info =  (unsigned long) &ax8817x_info,
 }, {
-	// ATEN UC210T
-	USB_DEVICE (0x0557, 0x2009),
+	/* ATEN UC210T */
+	USB_DEVICE(0x0557, 0x2009),
 	.driver_info =  (unsigned long) &ax8817x_info,
 }, {
-	// Buffalo LUA-U2-KTX
-	USB_DEVICE (0x0411, 0x003d),
+	/* Buffalo LUA-U2-KTX */
+	USB_DEVICE(0x0411, 0x003d),
 	.driver_info =  (unsigned long) &ax8817x_info,
 }, {
-	// Buffalo LUA-U2-GT 10/100/1000
-	USB_DEVICE (0x0411, 0x006e),
+	/* Buffalo LUA-U2-GT 10/100/1000 */
+	USB_DEVICE(0x0411, 0x006e),
 	.driver_info =  (unsigned long) &ax88178_info,
 }, {
-	// Sitecom LN-029 "USB 2.0 10/100 Ethernet adapter"
-	USB_DEVICE (0x6189, 0x182d),
+	/* Sitecom LN-029 "USB 2.0 10/100 Ethernet adapter" */
+	USB_DEVICE(0x6189, 0x182d),
 	.driver_info =  (unsigned long) &ax8817x_info,
 }, {
-	// Sitecom LN-031 "USB 2.0 10/100/1000 Ethernet adapter"
-	USB_DEVICE (0x0df6, 0x0056),
+	/* Sitecom LN-031 "USB 2.0 10/100/1000 Ethernet adapter" */
+	USB_DEVICE(0x0df6, 0x0056),
 	.driver_info =  (unsigned long) &ax88178_info,
 }, {
-	// corega FEther USB2-TX
-	USB_DEVICE (0x07aa, 0x0017),
+	/* corega FEther USB2-TX */
+	USB_DEVICE(0x07aa, 0x0017),
 	.driver_info =  (unsigned long) &ax8817x_info,
 }, {
-	// Surecom EP-1427X-2
-	USB_DEVICE (0x1189, 0x0893),
+	/* Surecom EP-1427X-2 */
+	USB_DEVICE(0x1189, 0x0893),
 	.driver_info = (unsigned long) &ax8817x_info,
 }, {
-	// goodway corp usb gwusb2e
-	USB_DEVICE (0x1631, 0x6200),
+	/* goodway corp usb gwusb2e */
+	USB_DEVICE(0x1631, 0x6200),
 	.driver_info = (unsigned long) &ax8817x_info,
 }, {
-	// JVC MP-PRX1 Port Replicator
-	USB_DEVICE (0x04f1, 0x3008),
+	/* JVC MP-PRX1 Port Replicator */
+	USB_DEVICE(0x04f1, 0x3008),
 	.driver_info = (unsigned long) &ax8817x_info,
 }, {
-	// ASIX AX88772B 10/100
-	USB_DEVICE (0x0b95, 0x772b),
+	/* ASIX AX88772B 10/100 */
+	USB_DEVICE(0x0b95, 0x772b),
 	.driver_info = (unsigned long) &ax88772_info,
 }, {
-	// ASIX AX88772 10/100
-	USB_DEVICE (0x0b95, 0x7720),
+	/* ASIX AX88772 10/100 */
+	USB_DEVICE(0x0b95, 0x7720),
 	.driver_info = (unsigned long) &ax88772_info,
 }, {
-	// ASIX AX88178 10/100/1000
-	USB_DEVICE (0x0b95, 0x1780),
+	/* ASIX AX88178 10/100/1000 */
+	USB_DEVICE(0x0b95, 0x1780),
 	.driver_info = (unsigned long) &ax88178_info,
 }, {
-	// Logitec LAN-GTJ/U2A
-	USB_DEVICE (0x0789, 0x0160),
+	/* Logitec LAN-GTJ/U2A */
+	USB_DEVICE(0x0789, 0x0160),
 	.driver_info = (unsigned long) &ax88178_info,
 }, {
-	// Linksys USB200M Rev 2
-	USB_DEVICE (0x13b1, 0x0018),
+	/* Linksys USB200M Rev 2 */
+	USB_DEVICE(0x13b1, 0x0018),
 	.driver_info = (unsigned long) &ax88772_info,
 }, {
-	// 0Q0 cable ethernet
-	USB_DEVICE (0x1557, 0x7720),
+	/* 0Q0 cable ethernet */
+	USB_DEVICE(0x1557, 0x7720),
 	.driver_info = (unsigned long) &ax88772_info,
 }, {
-	// DLink DUB-E100 H/W Ver B1
-	USB_DEVICE (0x07d1, 0x3c05),
+	/* DLink DUB-E100 H/W Ver B1 */
+	USB_DEVICE(0x07d1, 0x3c05),
 	.driver_info = (unsigned long) &ax88772_info,
 }, {
-	// DLink DUB-E100 H/W Ver B1 Alternate
-	USB_DEVICE (0x2001, 0x3c05),
+	/* DLink DUB-E100 H/W Ver B1 Alternate */
+	USB_DEVICE(0x2001, 0x3c05),
 	.driver_info = (unsigned long) &ax88772_info,
 }, {
-	// Linksys USB1000
-	USB_DEVICE (0x1737, 0x0039),
+	/* Linksys USB1000 */
+	USB_DEVICE(0x1737, 0x0039),
 	.driver_info = (unsigned long) &ax88178_info,
 }, {
-	// IO-DATA ETG-US2
-	USB_DEVICE (0x04bb, 0x0930),
+	/* IO-DATA ETG-US2 */
+	USB_DEVICE(0x04bb, 0x0930),
 	.driver_info = (unsigned long) &ax88178_info,
 }, {
-	// Belkin F5D5055
+	/* Belkin F5D5055 */
 	USB_DEVICE(0x050d, 0x5055),
 	.driver_info = (unsigned long) &ax88178_info,
 }, {
-	// Apple USB Ethernet Adapter
+	/* Apple USB Ethernet Adapter */
 	USB_DEVICE(0x05ac, 0x1402),
 	.driver_info = (unsigned long) &ax88772_info,
 }, {
-	// Cables-to-Go USB Ethernet Adapter
+	/* Cables-to-Go USB Ethernet Adapter */
 	USB_DEVICE(0x0b95, 0x772a),
 	.driver_info = (unsigned long) &ax88772_info,
 }, {
-	// ABOCOM for pci
+	/* ABOCOM for pci */
 	USB_DEVICE(0x14ea, 0xab11),
 	.driver_info = (unsigned long) &ax88178_info,
 }, {
-	// ASIX 88772a
+	/* ASIX 88772a */
 	USB_DEVICE(0x0db0, 0xa877),
 	.driver_info = (unsigned long) &ax88772_info,
 }, {
-	// Asus USB Ethernet Adapter
-	USB_DEVICE (0x0b95, 0x7e2b),
+	/* Asus USB Ethernet Adapter */
+	USB_DEVICE(0x0b95, 0x7e2b),
 	.driver_info = (unsigned long) &ax88772_info,
 },
-	{ },		// END
+	{ },		/* END */
 };
 MODULE_DEVICE_TABLE(usb, products);
 
-- 
1.7.0.4

Re: [PATCH 1/4] asix: Fix checkpatch warnings

[Eric Dumazet]

On Fri, 2012-07-06 at 13:33 +0200, Christian Riesch wrote:
> Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
> ---

> -		netdev_err(dev->net, "Error reading PHYID register: %02x\n", ret);
> +		netdev_err(dev->net, "Error reading PHYID register: %02x\n",
> +			   ret);


Thats ridiculous

Not all checkpatch warnings are meaningful.

I mean, they probably are for new code, but for existing one this is a
waste of time.

Re: [PATCH 1/4] asix: Fix checkpatch warnings

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 06 Jul 2012 13:58:39 +0200

> On Fri, 2012-07-06 at 13:33 +0200, Christian Riesch wrote:
>> Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
>> ---
> 
>> -		netdev_err(dev->net, "Error reading PHYID register: %02x\n", ret);
>> +		netdev_err(dev->net, "Error reading PHYID register: %02x\n",
>> +			   ret);
> 
> 
> Thats ridiculous
> 
> Not all checkpatch warnings are meaningful.
> 
> I mean, they probably are for new code, but for existing one this is a
> waste of time.

Agreed.

Re: [PATCH 1/4] asix: Fix checkpatch warnings

[Grant Grundler]

On Fri, Jul 6, 2012 at 4:58 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2012-07-06 at 13:33 +0200, Christian Riesch wrote:
>> Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
>> ---
>
>> -             netdev_err(dev->net, "Error reading PHYID register: %02x\n", ret);
>> +             netdev_err(dev->net, "Error reading PHYID register: %02x\n",
>> +                        ret);
>
>
> Thats ridiculous

Agreed. But it's not Christian's fault and I'm pretty sure you aren't
blaming him.

> Not all checkpatch warnings are meaningful.

Agreed.

> I mean, they probably are for new code, but for existing one this is a
> waste of time.

Christian is clearly running checkpatch.pl as suggested in
Documentation/SubmittingPatches. He missed the part about "You should
be able to justify all violations that remain in your patch." and/or
didn't know about "fixing existing code is a waste of time".

Given the extent of the changes Christian is making (factoring out
asix common code from model specific code), it's helpful to have clean
checkpatch runs. I don't believe it's a waste of time to apply this
patch. Is it conflicting with any other code changes that are in
flight now?

thanks,
grant

Re: [PATCH 1/4] asix: Fix checkpatch warnings

[Eric Dumazet]

On Fri, 2012-07-06 at 14:43 -0700, Grant Grundler wrote:

> Christian is clearly running checkpatch.pl as suggested in
> Documentation/SubmittingPatches. He missed the part about "You should
> be able to justify all violations that remain in your patch." and/or
> didn't know about "fixing existing code is a waste of time".
> 
> Given the extent of the changes Christian is making (factoring out
> asix common code from model specific code), it's helpful to have clean
> checkpatch runs. I don't believe it's a waste of time to apply this
> patch. Is it conflicting with any other code changes that are in
> flight now?

It was a waste of time for me, at least (Since I was CCed for the
patch), and just sent my personal opinion on checkpatch generated
patches.

Splitting a perfectly good line :

netdev_err(dev->net, "Error reading PHYID register: %02x\n", ret);

into

netdev_err(dev->net, "Error reading PHYID register: %02x\n",
	   ret);

is a clear sign of how stupid checkpatch is.

And fact we can spend time on discussions about checkpatch is
astonishing.

Automatic tools should be smart and ease people tasks, not
slowing them.

Note that Christian patch serie in itself is good, I don't want to block
it at all.

Re: [PATCH 1/4] asix: Fix checkpatch warnings

[Christian Riesch]

Hi all,

On Sat, Jul 7, 2012 at 10:36 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2012-07-06 at 14:43 -0700, Grant Grundler wrote:
>
>> Christian is clearly running checkpatch.pl as suggested in
>> Documentation/SubmittingPatches. He missed the part about "You should
>> be able to justify all violations that remain in your patch." and/or
>> didn't know about "fixing existing code is a waste of time".
>>
>> Given the extent of the changes Christian is making (factoring out
>> asix common code from model specific code), it's helpful to have clean
>> checkpatch runs. I don't believe it's a waste of time to apply this
>> patch. Is it conflicting with any other code changes that are in
>> flight now?
>
> It was a waste of time for me, at least (Since I was CCed for the
> patch), and just sent my personal opinion on checkpatch generated
> patches.
>
> Splitting a perfectly good line :
>
> netdev_err(dev->net, "Error reading PHYID register: %02x\n", ret);
>
> into
>
> netdev_err(dev->net, "Error reading PHYID register: %02x\n",
>            ret);
>
> is a clear sign of how stupid checkpatch is.
>
> And fact we can spend time on discussions about checkpatch is
> astonishing.
>
> Automatic tools should be smart and ease people tasks, not
> slowing them.

Thanks for your comments! I admit that I was just annoyed by the lots
of checkpatch warnings and therefore just brainlessly did everything
to make it stop complaining, there was not much thinking involved. I
will try to be more sensible when I put together the next version.

But first I'd like to draw your attention to the other patches in the
patchset :-) I asked a few questions in the cover letter [1] and I
would like to hear your opinion about it. Thanks a lot!

>
> Note that Christian patch serie in itself is good, I don't want to block
> it at all.

Thanks :-)

Regards, Christian


[1] http://marc.info/?l=linux-netdev&m=134157544400926&w=2

Re: [PATCH 1/4] asix: Fix checkpatch warnings

[Joe Perches]

On Fri, 2012-07-06 at 13:33 +0200, Christian Riesch wrote:
>

Hi Christian.  Just some trivial comments for a
trivial cleanup patch.

> diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
> index 3ae80ec..9210f40 100644
> --- a/drivers/net/usb/asix.c
> +++ b/drivers/net/usb/asix.c
> @@ -20,8 +20,8 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> -// #define	DEBUG			// error path messages, extra info
> -// #define	VERBOSE			// more; success messages
> +/* #define	DEBUG	*/		/* error path messages, extra info */
> +/* #define	VERBOSE	*/		/* more; success messages */

Might as well delete as change the comment style.
It isn't applicable after the patch.

> @@ -253,8 +253,8 @@ static void asix_async_cmd_callback(struct urb *urb)
>  	int status = urb->status;
>  
>  	if (status < 0)
> -		printk(KERN_DEBUG "asix_async_cmd_callback() failed with %d",
> -			status);
> +		pr_debug("asix_async_cmd_callback() failed with %d",
> +			 status);

Probably better with "%s: "..., __func__, ...
Missing a newline too.
There are several other uses of embedded function names
that could be modified.

> @@ -432,7 +433,8 @@ static inline int asix_get_phy_addr(struct usbnet *dev)
>  	netdev_dbg(dev->net, "asix_get_phy_addr()\n");
>  
>  	if (ret < 0) {
> -		netdev_err(dev->net, "Error reading PHYID register: %02x\n", ret);
> +		netdev_err(dev->net, "Error reading PHYID register: %02x\n",
> +			   ret);

80 column zealotry?  If you want, but it's probably past
the time that's really desirable or necessary.

> @@ -575,7 +580,7 @@ static int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)
>  	mutex_lock(&dev->phy_mutex);
>  	asix_set_sw_mii(dev);
>  	asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
> -				(__u16)loc, 2, &res);
> +		      (__u16)loc, 2, &res);

Fits on 1 line.

> +static void asix_get_drvinfo(struct net_device *net,
> +			     struct ethtool_drvinfo *info)
>  {
>  	struct usbnet *dev = netdev_priv(net);
>  	struct asix_data *data = (struct asix_data *)&dev->data;
>  
>  	/* Inherit standard device info */
>  	usbnet_get_drvinfo(net, info);
> -	strncpy (info->driver, DRIVER_NAME, sizeof info->driver);
> -	strncpy (info->version, DRIVER_VERSION, sizeof info->version);
> +	strncpy(info->driver, DRIVER_NAME, sizeof info->driver);
> +	strncpy(info->version, DRIVER_VERSION, sizeof info->version);

Most every kernel use of sizeof uses parens like:

	strncpy(info->driver, DRIVER_NAME, sizeof(info->driver));
	strncpy(info->version, DRIVER_VERSION, sizeof(info->version));
 
@@ -1510,133 +1520,133 @@ static const struct driver_info ax88178_info = {
>  	.tx_fixup = asix_tx_fixup,
>  };
>  
> -static const struct usb_device_id	products [] = {
> +static const struct usb_device_id	products[] = {

Maybe use a space not a tab after usb_device_id.

>  {
> -	// Linksys USB200M
> -	USB_DEVICE (0x077b, 0x2226),
> +	/* Linksys USB200M */
> +	USB_DEVICE(0x077b, 0x2226),
>  	.driver_info =	(unsigned long) &ax8817x_info,
>  }, {

I think all of these would look more reasonable on single
lines like

	{ USB_DEVICE(0xxxxx, 0xxxxx), .driver_info = (unsigned long)&func },

or maybe add another macro like:

#define ASIX_USB_DEVICE(vendor, product, driver)	\
	USB_DEVICE(vendor, product), .driver_info = (unsigned long)driver)

and make these

	{ ASIX_USB_DEVICE(0xxxxx, 0xxxxx, &func) },	/* description */

Come to think of it, the & for the function address
isn't necessary either.

cheers, Joe

[PATCH 2/4] asix: Rename asix.c to asix_devices.c

[Christian Riesch]

Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
---
 drivers/net/usb/Makefile                   |    1 +
 drivers/net/usb/{asix.c => asix_devices.c} |    0
 2 files changed, 1 insertions(+), 0 deletions(-)
 rename drivers/net/usb/{asix.c => asix_devices.c} (100%)

diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index a2e2d72..2c8f7b4 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_USB_PEGASUS)	+= pegasus.o
 obj-$(CONFIG_USB_RTL8150)	+= rtl8150.o
 obj-$(CONFIG_USB_HSO)		+= hso.o
 obj-$(CONFIG_USB_NET_AX8817X)	+= asix.o
+asix-y := asix_devices.o
 obj-$(CONFIG_USB_NET_CDCETHER)	+= cdc_ether.o
 obj-$(CONFIG_USB_NET_CDC_EEM)	+= cdc_eem.o
 obj-$(CONFIG_USB_NET_DM9601)	+= dm9601.o
diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix_devices.c
similarity index 100%
rename from drivers/net/usb/asix.c
rename to drivers/net/usb/asix_devices.c
-- 
1.7.0.4

[PATCH 3/4] asix: Factor out common code

[Christian Riesch]

Allow the new driver for the AX88172A to share code with the
existing drivers for ASIX devices.

Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
---
 drivers/net/usb/Makefile       |    2 +-
 drivers/net/usb/asix.h         |  211 +++++++++++++
 drivers/net/usb/asix_common.c  |  525 ++++++++++++++++++++++++++++++++
 drivers/net/usb/asix_devices.c |  645 +---------------------------------------
 4 files changed, 738 insertions(+), 645 deletions(-)
 create mode 100644 drivers/net/usb/asix.h
 create mode 100644 drivers/net/usb/asix_common.c

diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index 2c8f7b4..a9490d9 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_USB_PEGASUS)	+= pegasus.o
 obj-$(CONFIG_USB_RTL8150)	+= rtl8150.o
 obj-$(CONFIG_USB_HSO)		+= hso.o
 obj-$(CONFIG_USB_NET_AX8817X)	+= asix.o
-asix-y := asix_devices.o
+asix-y := asix_devices.o asix_common.o
 obj-$(CONFIG_USB_NET_CDCETHER)	+= cdc_ether.o
 obj-$(CONFIG_USB_NET_CDC_EEM)	+= cdc_eem.o
 obj-$(CONFIG_USB_NET_DM9601)	+= dm9601.o
diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
new file mode 100644
index 0000000..5339578
--- /dev/null
+++ b/drivers/net/usb/asix.h
@@ -0,0 +1,211 @@
+/*
+ * ASIX AX8817X based USB 2.0 Ethernet Devices
+ * Copyright (C) 2003-2006 David Hollis <dhollis@davehollis.com>
+ * Copyright (C) 2005 Phil Chang <pchang23@sbcglobal.net>
+ * Copyright (C) 2006 James Painter <jamie.painter@iname.com>
+ * Copyright (c) 2002-2003 TiVo Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef _ASIX_H
+#define _ASIX_H
+
+#include <linux/module.h>
+#include <linux/kmod.h>
+#include <linux/init.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/ethtool.h>
+#include <linux/workqueue.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/crc32.h>
+#include <linux/usb/usbnet.h>
+#include <linux/slab.h>
+#include <linux/if_vlan.h>
+
+/* #define	DEBUG	*/		/* error path messages, extra info */
+/* #define	VERBOSE	*/		/* more; success messages */
+
+#define DRIVER_VERSION "22-Dec-2011"
+#define DRIVER_NAME "asix"
+
+/* ASIX AX8817X based USB 2.0 Ethernet Devices */
+#define AX_CMD_SET_SW_MII		0x06
+#define AX_CMD_READ_MII_REG		0x07
+#define AX_CMD_WRITE_MII_REG		0x08
+#define AX_CMD_SET_HW_MII		0x0a
+#define AX_CMD_READ_EEPROM		0x0b
+#define AX_CMD_WRITE_EEPROM		0x0c
+#define AX_CMD_WRITE_ENABLE		0x0d
+#define AX_CMD_WRITE_DISABLE		0x0e
+#define AX_CMD_READ_RX_CTL		0x0f
+#define AX_CMD_WRITE_RX_CTL		0x10
+#define AX_CMD_READ_IPG012		0x11
+#define AX_CMD_WRITE_IPG0		0x12
+#define AX_CMD_WRITE_IPG1		0x13
+#define AX_CMD_READ_NODE_ID		0x13
+#define AX_CMD_WRITE_NODE_ID		0x14
+#define AX_CMD_WRITE_IPG2		0x14
+#define AX_CMD_WRITE_MULTI_FILTER	0x16
+#define AX88172_CMD_READ_NODE_ID	0x17
+#define AX_CMD_READ_PHY_ID		0x19
+#define AX_CMD_READ_MEDIUM_STATUS	0x1a
+#define AX_CMD_WRITE_MEDIUM_MODE	0x1b
+#define AX_CMD_READ_MONITOR_MODE	0x1c
+#define AX_CMD_WRITE_MONITOR_MODE	0x1d
+#define AX_CMD_READ_GPIOS		0x1e
+#define AX_CMD_WRITE_GPIOS		0x1f
+#define AX_CMD_SW_RESET			0x20
+#define AX_CMD_SW_PHY_STATUS		0x21
+#define AX_CMD_SW_PHY_SELECT		0x22
+
+#define AX_MONITOR_MODE			0x01
+#define AX_MONITOR_LINK			0x02
+#define AX_MONITOR_MAGIC		0x04
+#define AX_MONITOR_HSFS			0x10
+
+/* AX88172 Medium Status Register values */
+#define AX88172_MEDIUM_FD		0x02
+#define AX88172_MEDIUM_TX		0x04
+#define AX88172_MEDIUM_FC		0x10
+#define AX88172_MEDIUM_DEFAULT \
+		(AX88172_MEDIUM_FD | AX88172_MEDIUM_TX | AX88172_MEDIUM_FC)
+
+#define AX_MCAST_FILTER_SIZE		8
+#define AX_MAX_MCAST			64
+
+#define AX_SWRESET_CLEAR		0x00
+#define AX_SWRESET_RR			0x01
+#define AX_SWRESET_RT			0x02
+#define AX_SWRESET_PRTE			0x04
+#define AX_SWRESET_PRL			0x08
+#define AX_SWRESET_BZ			0x10
+#define AX_SWRESET_IPRL			0x20
+#define AX_SWRESET_IPPD			0x40
+
+#define AX88772_IPG0_DEFAULT		0x15
+#define AX88772_IPG1_DEFAULT		0x0c
+#define AX88772_IPG2_DEFAULT		0x12
+
+/* AX88772 & AX88178 Medium Mode Register */
+#define AX_MEDIUM_PF		0x0080
+#define AX_MEDIUM_JFE		0x0040
+#define AX_MEDIUM_TFC		0x0020
+#define AX_MEDIUM_RFC		0x0010
+#define AX_MEDIUM_ENCK		0x0008
+#define AX_MEDIUM_AC		0x0004
+#define AX_MEDIUM_FD		0x0002
+#define AX_MEDIUM_GM		0x0001
+#define AX_MEDIUM_SM		0x1000
+#define AX_MEDIUM_SBP		0x0800
+#define AX_MEDIUM_PS		0x0200
+#define AX_MEDIUM_RE		0x0100
+
+#define AX88178_MEDIUM_DEFAULT	\
+	(AX_MEDIUM_PS | AX_MEDIUM_FD | AX_MEDIUM_AC | \
+	 AX_MEDIUM_RFC | AX_MEDIUM_TFC | AX_MEDIUM_JFE | \
+	 AX_MEDIUM_RE)
+
+#define AX88772_MEDIUM_DEFAULT	\
+	(AX_MEDIUM_FD | AX_MEDIUM_RFC | \
+	 AX_MEDIUM_TFC | AX_MEDIUM_PS | \
+	 AX_MEDIUM_AC | AX_MEDIUM_RE)
+
+/* AX88772 & AX88178 RX_CTL values */
+#define AX_RX_CTL_SO		0x0080
+#define AX_RX_CTL_AP		0x0020
+#define AX_RX_CTL_AM		0x0010
+#define AX_RX_CTL_AB		0x0008
+#define AX_RX_CTL_SEP		0x0004
+#define AX_RX_CTL_AMALL		0x0002
+#define AX_RX_CTL_PRO		0x0001
+#define AX_RX_CTL_MFB_2048	0x0000
+#define AX_RX_CTL_MFB_4096	0x0100
+#define AX_RX_CTL_MFB_8192	0x0200
+#define AX_RX_CTL_MFB_16384	0x0300
+
+#define AX_DEFAULT_RX_CTL	(AX_RX_CTL_SO | AX_RX_CTL_AB)
+
+/* GPIO 0 .. 2 toggles */
+#define AX_GPIO_GPO0EN		0x01	/* GPIO0 Output enable */
+#define AX_GPIO_GPO_0		0x02	/* GPIO0 Output value */
+#define AX_GPIO_GPO1EN		0x04	/* GPIO1 Output enable */
+#define AX_GPIO_GPO_1		0x08	/* GPIO1 Output value */
+#define AX_GPIO_GPO2EN		0x10	/* GPIO2 Output enable */
+#define AX_GPIO_GPO_2		0x20	/* GPIO2 Output value */
+#define AX_GPIO_RESERVED	0x40	/* Reserved */
+#define AX_GPIO_RSE		0x80	/* Reload serial EEPROM */
+
+#define AX_EEPROM_MAGIC		0xdeadbeef
+#define AX88172_EEPROM_LEN	0x40
+#define AX88772_EEPROM_LEN	0xff
+
+/* This structure cannot exceed sizeof(unsigned long [5]) AKA 20 bytes */
+struct asix_data {
+	u8 multi_filter[AX_MCAST_FILTER_SIZE];
+	u8 mac_addr[ETH_ALEN];
+	u8 phymode;
+	u8 ledmode;
+	u8 eeprom_len;
+};
+
+int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
+		  u16 size, void *data);
+
+int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
+		   u16 size, void *data);
+
+void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value,
+			  u16 index, u16 size, void *data);
+
+int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb);
+
+struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
+			      gfp_t flags);
+
+int asix_set_sw_mii(struct usbnet *dev);
+int asix_set_hw_mii(struct usbnet *dev);
+
+int asix_get_phy_addr(struct usbnet *dev);
+
+int asix_sw_reset(struct usbnet *dev, u8 flags);
+
+u16 asix_read_rx_ctl(struct usbnet *dev);
+int asix_write_rx_ctl(struct usbnet *dev, u16 mode);
+
+u16 asix_read_medium_status(struct usbnet *dev);
+int asix_write_medium_mode(struct usbnet *dev, u16 mode);
+
+int asix_write_gpio(struct usbnet *dev, u16 value, int sleep);
+
+void asix_set_multicast(struct net_device *net);
+
+int asix_mdio_read(struct net_device *netdev, int phy_id, int loc);
+void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val);
+
+void asix_get_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo);
+int asix_set_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo);
+
+int asix_get_eeprom_len(struct net_device *net);
+int asix_get_eeprom(struct net_device *net, struct ethtool_eeprom *eeprom,
+		    u8 *data);
+
+void asix_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info);
+
+int asix_set_mac_address(struct net_device *net, void *p);
+
+#endif /* _ASIX_H */
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
new file mode 100644
index 0000000..8231948
--- /dev/null
+++ b/drivers/net/usb/asix_common.c
@@ -0,0 +1,525 @@
+/*
+ * ASIX AX8817X based USB 2.0 Ethernet Devices
+ * Copyright (C) 2003-2006 David Hollis <dhollis@davehollis.com>
+ * Copyright (C) 2005 Phil Chang <pchang23@sbcglobal.net>
+ * Copyright (C) 2006 James Painter <jamie.painter@iname.com>
+ * Copyright (c) 2002-2003 TiVo Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include "asix.h"
+
+int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
+		  u16 size, void *data)
+{
+	void *buf;
+	int err = -ENOMEM;
+
+	netdev_dbg(dev->net, "asix_read_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
+		   cmd, value, index, size);
+
+	buf = kmalloc(size, GFP_KERNEL);
+	if (!buf)
+		goto out;
+
+	err = usb_control_msg(
+		dev->udev,
+		usb_rcvctrlpipe(dev->udev, 0),
+		cmd,
+		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+		value,
+		index,
+		buf,
+		size,
+		USB_CTRL_GET_TIMEOUT);
+	if (err == size)
+		memcpy(data, buf, size);
+	else if (err >= 0)
+		err = -EINVAL;
+	kfree(buf);
+
+out:
+	return err;
+}
+
+int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
+			     u16 size, void *data)
+{
+	void *buf = NULL;
+	int err = -ENOMEM;
+
+	netdev_dbg(dev->net, "asix_write_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
+		   cmd, value, index, size);
+
+	if (data) {
+		buf = kmemdup(data, size, GFP_KERNEL);
+		if (!buf)
+			goto out;
+	}
+
+	err = usb_control_msg(
+		dev->udev,
+		usb_sndctrlpipe(dev->udev, 0),
+		cmd,
+		USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+		value,
+		index,
+		buf,
+		size,
+		USB_CTRL_SET_TIMEOUT);
+	kfree(buf);
+
+out:
+	return err;
+}
+
+static void asix_async_cmd_callback(struct urb *urb)
+{
+	struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
+	int status = urb->status;
+
+	if (status < 0)
+		pr_debug("asix_async_cmd_callback() failed with %d",
+			 status);
+
+	kfree(req);
+	usb_free_urb(urb);
+}
+
+void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
+			  u16 size, void *data)
+{
+	struct usb_ctrlrequest *req;
+	int status;
+	struct urb *urb;
+
+	netdev_dbg(dev->net, "asix_write_cmd_async() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
+		   cmd, value, index, size);
+
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb) {
+		netdev_err(dev->net, "Error allocating URB in write_cmd_async!\n");
+		return;
+	}
+
+	req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
+	if (!req) {
+		netdev_err(dev->net, "Failed to allocate memory for control request\n");
+		usb_free_urb(urb);
+		return;
+	}
+
+	req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
+	req->bRequest = cmd;
+	req->wValue = cpu_to_le16(value);
+	req->wIndex = cpu_to_le16(index);
+	req->wLength = cpu_to_le16(size);
+
+	usb_fill_control_urb(urb, dev->udev,
+			     usb_sndctrlpipe(dev->udev, 0),
+			     (void *)req, data, size,
+			     asix_async_cmd_callback, req);
+
+	status = usb_submit_urb(urb, GFP_ATOMIC);
+	if (status < 0) {
+		netdev_err(dev->net, "Error submitting the control message: status=%d\n",
+			   status);
+		kfree(req);
+		usb_free_urb(urb);
+	}
+}
+
+int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+{
+	int offset = 0;
+
+	while (offset + sizeof(u32) < skb->len) {
+		struct sk_buff *ax_skb;
+		u16 size;
+		u32 header = get_unaligned_le32(skb->data + offset);
+
+		offset += sizeof(u32);
+
+		/* get the packet length */
+		size = (u16) (header & 0x7ff);
+		if (size != ((~header >> 16) & 0x07ff)) {
+			netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
+			return 0;
+		}
+
+		if ((size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) ||
+		    (size + offset > skb->len)) {
+			netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
+				   size);
+			return 0;
+		}
+		ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
+		if (!ax_skb)
+			return 0;
+
+		skb_put(ax_skb, size);
+		memcpy(ax_skb->data, skb->data + offset, size);
+		usbnet_skb_return(dev, ax_skb);
+
+		offset += (size + 1) & 0xfffe;
+	}
+
+	if (skb->len != offset) {
+		netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
+			   skb->len);
+		return 0;
+	}
+	return 1;
+}
+
+struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
+			      gfp_t flags)
+{
+	int padlen;
+	int headroom = skb_headroom(skb);
+	int tailroom = skb_tailroom(skb);
+	u32 packet_len;
+	u32 padbytes = 0xffff0000;
+
+	padlen = ((skb->len + 4) & (dev->maxpacket - 1)) ? 0 : 4;
+
+	if ((!skb_cloned(skb)) &&
+	    ((headroom + tailroom) >= (4 + padlen))) {
+		if ((headroom < 4) || (tailroom < padlen)) {
+			skb->data = memmove(skb->head + 4, skb->data, skb->len);
+			skb_set_tail_pointer(skb, skb->len);
+		}
+	} else {
+		struct sk_buff *skb2;
+		skb2 = skb_copy_expand(skb, 4, padlen, flags);
+		dev_kfree_skb_any(skb);
+		skb = skb2;
+		if (!skb)
+			return NULL;
+	}
+
+	skb_push(skb, 4);
+	packet_len = (((skb->len - 4) ^ 0x0000ffff) << 16) + (skb->len - 4);
+	cpu_to_le32s(&packet_len);
+	skb_copy_to_linear_data(skb, &packet_len, sizeof(packet_len));
+
+	if (padlen) {
+		cpu_to_le32s(&padbytes);
+		memcpy(skb_tail_pointer(skb), &padbytes, sizeof(padbytes));
+		skb_put(skb, sizeof(padbytes));
+	}
+	return skb;
+}
+
+int asix_set_sw_mii(struct usbnet *dev)
+{
+	int ret;
+	ret = asix_write_cmd(dev, AX_CMD_SET_SW_MII, 0x0000, 0, 0, NULL);
+	if (ret < 0)
+		netdev_err(dev->net, "Failed to enable software MII access\n");
+	return ret;
+}
+
+int asix_set_hw_mii(struct usbnet *dev)
+{
+	int ret;
+	ret = asix_write_cmd(dev, AX_CMD_SET_HW_MII, 0x0000, 0, 0, NULL);
+	if (ret < 0)
+		netdev_err(dev->net, "Failed to enable hardware MII access\n");
+	return ret;
+}
+
+int asix_get_phy_addr(struct usbnet *dev)
+{
+	u8 buf[2];
+	int ret = asix_read_cmd(dev, AX_CMD_READ_PHY_ID, 0, 0, 2, buf);
+
+	netdev_dbg(dev->net, "asix_get_phy_addr()\n");
+
+	if (ret < 0) {
+		netdev_err(dev->net, "Error reading PHYID register: %02x\n",
+			   ret);
+		goto out;
+	}
+	netdev_dbg(dev->net, "asix_get_phy_addr() returning 0x%04x\n",
+		   *((__le16 *)buf));
+	ret = buf[1];
+
+out:
+	return ret;
+}
+
+int asix_sw_reset(struct usbnet *dev, u8 flags)
+{
+	int ret;
+
+	ret = asix_write_cmd(dev, AX_CMD_SW_RESET, flags, 0, 0, NULL);
+	if (ret < 0)
+		netdev_err(dev->net, "Failed to send software reset: %02x\n",
+			   ret);
+
+	return ret;
+}
+
+u16 asix_read_rx_ctl(struct usbnet *dev)
+{
+	__le16 v;
+	int ret = asix_read_cmd(dev, AX_CMD_READ_RX_CTL, 0, 0, 2, &v);
+
+	if (ret < 0) {
+		netdev_err(dev->net, "Error reading RX_CTL register: %02x\n",
+			   ret);
+		goto out;
+	}
+	ret = le16_to_cpu(v);
+out:
+	return ret;
+}
+
+int asix_write_rx_ctl(struct usbnet *dev, u16 mode)
+{
+	int ret;
+
+	netdev_dbg(dev->net, "asix_write_rx_ctl() - mode = 0x%04x\n", mode);
+	ret = asix_write_cmd(dev, AX_CMD_WRITE_RX_CTL, mode, 0, 0, NULL);
+	if (ret < 0)
+		netdev_err(dev->net, "Failed to write RX_CTL mode to 0x%04x: %02x\n",
+			   mode, ret);
+
+	return ret;
+}
+
+u16 asix_read_medium_status(struct usbnet *dev)
+{
+	__le16 v;
+	int ret = asix_read_cmd(dev, AX_CMD_READ_MEDIUM_STATUS, 0, 0, 2, &v);
+
+	if (ret < 0) {
+		netdev_err(dev->net, "Error reading Medium Status register: %02x\n",
+			   ret);
+		return ret;	/* TODO: callers not checking for error ret */
+	}
+
+	return le16_to_cpu(v);
+
+}
+
+int asix_write_medium_mode(struct usbnet *dev, u16 mode)
+{
+	int ret;
+
+	netdev_dbg(dev->net, "asix_write_medium_mode() - mode = 0x%04x\n",
+		   mode);
+	ret = asix_write_cmd(dev, AX_CMD_WRITE_MEDIUM_MODE, mode, 0, 0, NULL);
+	if (ret < 0)
+		netdev_err(dev->net, "Failed to write Medium Mode mode to 0x%04x: %02x\n",
+			   mode, ret);
+
+	return ret;
+}
+
+int asix_write_gpio(struct usbnet *dev, u16 value, int sleep)
+{
+	int ret;
+
+	netdev_dbg(dev->net, "asix_write_gpio() - value = 0x%04x\n", value);
+	ret = asix_write_cmd(dev, AX_CMD_WRITE_GPIOS, value, 0, 0, NULL);
+	if (ret < 0)
+		netdev_err(dev->net, "Failed to write GPIO value 0x%04x: %02x\n",
+			   value, ret);
+
+	if (sleep)
+		msleep(sleep);
+
+	return ret;
+}
+
+/*
+ * AX88772 & AX88178 have a 16-bit RX_CTL value
+ */
+void asix_set_multicast(struct net_device *net)
+{
+	struct usbnet *dev = netdev_priv(net);
+	struct asix_data *data = (struct asix_data *)&dev->data;
+	u16 rx_ctl = AX_DEFAULT_RX_CTL;
+
+	if (net->flags & IFF_PROMISC) {
+		rx_ctl |= AX_RX_CTL_PRO;
+	} else if (net->flags & IFF_ALLMULTI ||
+		   netdev_mc_count(net) > AX_MAX_MCAST) {
+		rx_ctl |= AX_RX_CTL_AMALL;
+	} else if (netdev_mc_empty(net)) {
+		/* just broadcast and directed */
+	} else {
+		/* We use the 20 byte dev->data
+		 * for our 8 byte filter buffer
+		 * to avoid allocating memory that
+		 * is tricky to free later */
+		struct netdev_hw_addr *ha;
+		u32 crc_bits;
+
+		memset(data->multi_filter, 0, AX_MCAST_FILTER_SIZE);
+
+		/* Build the multicast hash filter. */
+		netdev_for_each_mc_addr(ha, net) {
+			crc_bits = ether_crc(ETH_ALEN, ha->addr) >> 26;
+			data->multi_filter[crc_bits >> 3] |=
+			    1 << (crc_bits & 7);
+		}
+
+		asix_write_cmd_async(dev, AX_CMD_WRITE_MULTI_FILTER, 0, 0,
+				     AX_MCAST_FILTER_SIZE, data->multi_filter);
+
+		rx_ctl |= AX_RX_CTL_AM;
+	}
+
+	asix_write_cmd_async(dev, AX_CMD_WRITE_RX_CTL, rx_ctl, 0, 0, NULL);
+}
+
+int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)
+{
+	struct usbnet *dev = netdev_priv(netdev);
+	__le16 res;
+
+	mutex_lock(&dev->phy_mutex);
+	asix_set_sw_mii(dev);
+	asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
+		      (__u16)loc, 2, &res);
+	asix_set_hw_mii(dev);
+	mutex_unlock(&dev->phy_mutex);
+
+	netdev_dbg(dev->net, "asix_mdio_read() phy_id=0x%02x, loc=0x%02x, returns=0x%04x\n",
+		   phy_id, loc, le16_to_cpu(res));
+
+	return le16_to_cpu(res);
+}
+
+void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
+{
+	struct usbnet *dev = netdev_priv(netdev);
+	__le16 res = cpu_to_le16(val);
+
+	netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",
+		   phy_id, loc, val);
+	mutex_lock(&dev->phy_mutex);
+	asix_set_sw_mii(dev);
+	asix_write_cmd(dev, AX_CMD_WRITE_MII_REG, phy_id, (__u16)loc, 2, &res);
+	asix_set_hw_mii(dev);
+	mutex_unlock(&dev->phy_mutex);
+}
+
+void asix_get_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
+{
+	struct usbnet *dev = netdev_priv(net);
+	u8 opt;
+
+	if (asix_read_cmd(dev, AX_CMD_READ_MONITOR_MODE, 0, 0, 1, &opt) < 0) {
+		wolinfo->supported = 0;
+		wolinfo->wolopts = 0;
+		return;
+	}
+	wolinfo->supported = WAKE_PHY | WAKE_MAGIC;
+	wolinfo->wolopts = 0;
+	if (opt & AX_MONITOR_LINK)
+		wolinfo->wolopts |= WAKE_PHY;
+	if (opt & AX_MONITOR_MAGIC)
+		wolinfo->wolopts |= WAKE_MAGIC;
+}
+
+int asix_set_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
+{
+	struct usbnet *dev = netdev_priv(net);
+	u8 opt = 0;
+
+	if (wolinfo->wolopts & WAKE_PHY)
+		opt |= AX_MONITOR_LINK;
+	if (wolinfo->wolopts & WAKE_MAGIC)
+		opt |= AX_MONITOR_MAGIC;
+
+	if (asix_write_cmd(dev, AX_CMD_WRITE_MONITOR_MODE,
+			   opt, 0, 0, NULL) < 0)
+		return -EINVAL;
+
+	return 0;
+}
+
+int asix_get_eeprom_len(struct net_device *net)
+{
+	struct usbnet *dev = netdev_priv(net);
+	struct asix_data *data = (struct asix_data *)&dev->data;
+
+	return data->eeprom_len;
+}
+
+int asix_get_eeprom(struct net_device *net, struct ethtool_eeprom *eeprom,
+		    u8 *data)
+{
+	struct usbnet *dev = netdev_priv(net);
+	__le16 *ebuf = (__le16 *)data;
+	int i;
+
+	/* Crude hack to ensure that we don't overwrite memory
+	 * if an odd length is supplied
+	 */
+	if (eeprom->len % 2)
+		return -EINVAL;
+
+	eeprom->magic = AX_EEPROM_MAGIC;
+
+	/* ax8817x returns 2 bytes from eeprom on read */
+	for (i = 0; i < eeprom->len / 2; i++) {
+		if (asix_read_cmd(dev, AX_CMD_READ_EEPROM,
+				  eeprom->offset + i, 0, 2, &ebuf[i]) < 0)
+			return -EINVAL;
+	}
+	return 0;
+}
+
+void asix_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
+{
+	struct usbnet *dev = netdev_priv(net);
+	struct asix_data *data = (struct asix_data *)&dev->data;
+
+	/* Inherit standard device info */
+	usbnet_get_drvinfo(net, info);
+	strncpy(info->driver, DRIVER_NAME, sizeof info->driver);
+	strncpy(info->version, DRIVER_VERSION, sizeof info->version);
+	info->eedump_len = data->eeprom_len;
+}
+
+int asix_set_mac_address(struct net_device *net, void *p)
+{
+	struct usbnet *dev = netdev_priv(net);
+	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct sockaddr *addr = p;
+
+	if (netif_running(net))
+		return -EBUSY;
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+
+	memcpy(net->dev_addr, addr->sa_data, ETH_ALEN);
+
+	/* We use the 20 byte dev->data
+	 * for our 6 byte mac buffer
+	 * to avoid allocating memory that
+	 * is tricky to free later */
+	memcpy(data->mac_addr, addr->sa_data, ETH_ALEN);
+	asix_write_cmd_async(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN,
+			     data->mac_addr);
+
+	return 0;
+}
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 9210f40..c8682a5 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -23,134 +23,7 @@
 /* #define	DEBUG	*/		/* error path messages, extra info */
 /* #define	VERBOSE	*/		/* more; success messages */
 
-#include <linux/module.h>
-#include <linux/kmod.h>
-#include <linux/init.h>
-#include <linux/netdevice.h>
-#include <linux/etherdevice.h>
-#include <linux/ethtool.h>
-#include <linux/workqueue.h>
-#include <linux/mii.h>
-#include <linux/usb.h>
-#include <linux/crc32.h>
-#include <linux/usb/usbnet.h>
-#include <linux/slab.h>
-#include <linux/if_vlan.h>
-
-#define DRIVER_VERSION "22-Dec-2011"
-#define DRIVER_NAME "asix"
-
-/* ASIX AX8817X based USB 2.0 Ethernet Devices */
-
-#define AX_CMD_SET_SW_MII		0x06
-#define AX_CMD_READ_MII_REG		0x07
-#define AX_CMD_WRITE_MII_REG		0x08
-#define AX_CMD_SET_HW_MII		0x0a
-#define AX_CMD_READ_EEPROM		0x0b
-#define AX_CMD_WRITE_EEPROM		0x0c
-#define AX_CMD_WRITE_ENABLE		0x0d
-#define AX_CMD_WRITE_DISABLE		0x0e
-#define AX_CMD_READ_RX_CTL		0x0f
-#define AX_CMD_WRITE_RX_CTL		0x10
-#define AX_CMD_READ_IPG012		0x11
-#define AX_CMD_WRITE_IPG0		0x12
-#define AX_CMD_WRITE_IPG1		0x13
-#define AX_CMD_READ_NODE_ID		0x13
-#define AX_CMD_WRITE_NODE_ID		0x14
-#define AX_CMD_WRITE_IPG2		0x14
-#define AX_CMD_WRITE_MULTI_FILTER	0x16
-#define AX88172_CMD_READ_NODE_ID	0x17
-#define AX_CMD_READ_PHY_ID		0x19
-#define AX_CMD_READ_MEDIUM_STATUS	0x1a
-#define AX_CMD_WRITE_MEDIUM_MODE	0x1b
-#define AX_CMD_READ_MONITOR_MODE	0x1c
-#define AX_CMD_WRITE_MONITOR_MODE	0x1d
-#define AX_CMD_READ_GPIOS		0x1e
-#define AX_CMD_WRITE_GPIOS		0x1f
-#define AX_CMD_SW_RESET			0x20
-#define AX_CMD_SW_PHY_STATUS		0x21
-#define AX_CMD_SW_PHY_SELECT		0x22
-
-#define AX_MONITOR_MODE			0x01
-#define AX_MONITOR_LINK			0x02
-#define AX_MONITOR_MAGIC		0x04
-#define AX_MONITOR_HSFS			0x10
-
-/* AX88172 Medium Status Register values */
-#define AX88172_MEDIUM_FD		0x02
-#define AX88172_MEDIUM_TX		0x04
-#define AX88172_MEDIUM_FC		0x10
-#define AX88172_MEDIUM_DEFAULT \
-		(AX88172_MEDIUM_FD | AX88172_MEDIUM_TX | AX88172_MEDIUM_FC)
-
-#define AX_MCAST_FILTER_SIZE		8
-#define AX_MAX_MCAST			64
-
-#define AX_SWRESET_CLEAR		0x00
-#define AX_SWRESET_RR			0x01
-#define AX_SWRESET_RT			0x02
-#define AX_SWRESET_PRTE			0x04
-#define AX_SWRESET_PRL			0x08
-#define AX_SWRESET_BZ			0x10
-#define AX_SWRESET_IPRL			0x20
-#define AX_SWRESET_IPPD			0x40
-
-#define AX88772_IPG0_DEFAULT		0x15
-#define AX88772_IPG1_DEFAULT		0x0c
-#define AX88772_IPG2_DEFAULT		0x12
-
-/* AX88772 & AX88178 Medium Mode Register */
-#define AX_MEDIUM_PF		0x0080
-#define AX_MEDIUM_JFE		0x0040
-#define AX_MEDIUM_TFC		0x0020
-#define AX_MEDIUM_RFC		0x0010
-#define AX_MEDIUM_ENCK		0x0008
-#define AX_MEDIUM_AC		0x0004
-#define AX_MEDIUM_FD		0x0002
-#define AX_MEDIUM_GM		0x0001
-#define AX_MEDIUM_SM		0x1000
-#define AX_MEDIUM_SBP		0x0800
-#define AX_MEDIUM_PS		0x0200
-#define AX_MEDIUM_RE		0x0100
-
-#define AX88178_MEDIUM_DEFAULT	\
-	(AX_MEDIUM_PS | AX_MEDIUM_FD | AX_MEDIUM_AC | \
-	 AX_MEDIUM_RFC | AX_MEDIUM_TFC | AX_MEDIUM_JFE | \
-	 AX_MEDIUM_RE)
-
-#define AX88772_MEDIUM_DEFAULT	\
-	(AX_MEDIUM_FD | AX_MEDIUM_RFC | \
-	 AX_MEDIUM_TFC | AX_MEDIUM_PS | \
-	 AX_MEDIUM_AC | AX_MEDIUM_RE)
-
-/* AX88772 & AX88178 RX_CTL values */
-#define AX_RX_CTL_SO		0x0080
-#define AX_RX_CTL_AP		0x0020
-#define AX_RX_CTL_AM		0x0010
-#define AX_RX_CTL_AB		0x0008
-#define AX_RX_CTL_SEP		0x0004
-#define AX_RX_CTL_AMALL		0x0002
-#define AX_RX_CTL_PRO		0x0001
-#define AX_RX_CTL_MFB_2048	0x0000
-#define AX_RX_CTL_MFB_4096	0x0100
-#define AX_RX_CTL_MFB_8192	0x0200
-#define AX_RX_CTL_MFB_16384	0x0300
-
-#define AX_DEFAULT_RX_CTL	(AX_RX_CTL_SO | AX_RX_CTL_AB)
-
-/* GPIO 0 .. 2 toggles */
-#define AX_GPIO_GPO0EN		0x01	/* GPIO0 Output enable */
-#define AX_GPIO_GPO_0		0x02	/* GPIO0 Output value */
-#define AX_GPIO_GPO1EN		0x04	/* GPIO1 Output enable */
-#define AX_GPIO_GPO_1		0x08	/* GPIO1 Output value */
-#define AX_GPIO_GPO2EN		0x10	/* GPIO2 Output enable */
-#define AX_GPIO_GPO_2		0x20	/* GPIO2 Output value */
-#define AX_GPIO_RESERVED	0x40	/* Reserved */
-#define AX_GPIO_RSE		0x80	/* Reload serial EEPROM */
-
-#define AX_EEPROM_MAGIC		0xdeadbeef
-#define AX88172_EEPROM_LEN	0x40
-#define AX88772_EEPROM_LEN	0xff
+#include "asix.h"
 
 #define PHY_MODE_MARVELL	0x0000
 #define MII_MARVELL_LED_CTRL	0x0018
@@ -166,15 +39,6 @@
 
 #define	PHY_MODE_RTL8211CL	0x000C
 
-/* This structure cannot exceed sizeof(unsigned long [5]) AKA 20 bytes */
-struct asix_data {
-	u8 multi_filter[AX_MCAST_FILTER_SIZE];
-	u8 mac_addr[ETH_ALEN];
-	u8 phymode;
-	u8 ledmode;
-	u8 eeprom_len;
-};
-
 struct ax88172_int_data {
 	__le16 res1;
 	u8 link;
@@ -183,209 +47,6 @@ struct ax88172_int_data {
 	__le16 res3;
 } __packed;
 
-static int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
-			    u16 size, void *data)
-{
-	void *buf;
-	int err = -ENOMEM;
-
-	netdev_dbg(dev->net, "asix_read_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
-		   cmd, value, index, size);
-
-	buf = kmalloc(size, GFP_KERNEL);
-	if (!buf)
-		goto out;
-
-	err = usb_control_msg(
-		dev->udev,
-		usb_rcvctrlpipe(dev->udev, 0),
-		cmd,
-		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		value,
-		index,
-		buf,
-		size,
-		USB_CTRL_GET_TIMEOUT);
-	if (err == size)
-		memcpy(data, buf, size);
-	else if (err >= 0)
-		err = -EINVAL;
-	kfree(buf);
-
-out:
-	return err;
-}
-
-static int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
-			     u16 size, void *data)
-{
-	void *buf = NULL;
-	int err = -ENOMEM;
-
-	netdev_dbg(dev->net, "asix_write_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
-		   cmd, value, index, size);
-
-	if (data) {
-		buf = kmemdup(data, size, GFP_KERNEL);
-		if (!buf)
-			goto out;
-	}
-
-	err = usb_control_msg(
-		dev->udev,
-		usb_sndctrlpipe(dev->udev, 0),
-		cmd,
-		USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		value,
-		index,
-		buf,
-		size,
-		USB_CTRL_SET_TIMEOUT);
-	kfree(buf);
-
-out:
-	return err;
-}
-
-static void asix_async_cmd_callback(struct urb *urb)
-{
-	struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-	int status = urb->status;
-
-	if (status < 0)
-		pr_debug("asix_async_cmd_callback() failed with %d",
-			 status);
-
-	kfree(req);
-	usb_free_urb(urb);
-}
-
-static void
-asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
-		     u16 size, void *data)
-{
-	struct usb_ctrlrequest *req;
-	int status;
-	struct urb *urb;
-
-	netdev_dbg(dev->net, "asix_write_cmd_async() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
-		   cmd, value, index, size);
-
-	urb = usb_alloc_urb(0, GFP_ATOMIC);
-	if (!urb) {
-		netdev_err(dev->net, "Error allocating URB in write_cmd_async!\n");
-		return;
-	}
-
-	req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
-	if (!req) {
-		netdev_err(dev->net, "Failed to allocate memory for control request\n");
-		usb_free_urb(urb);
-		return;
-	}
-
-	req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-	req->bRequest = cmd;
-	req->wValue = cpu_to_le16(value);
-	req->wIndex = cpu_to_le16(index);
-	req->wLength = cpu_to_le16(size);
-
-	usb_fill_control_urb(urb, dev->udev,
-			     usb_sndctrlpipe(dev->udev, 0),
-			     (void *)req, data, size,
-			     asix_async_cmd_callback, req);
-
-	status = usb_submit_urb(urb, GFP_ATOMIC);
-	if (status < 0) {
-		netdev_err(dev->net, "Error submitting the control message: status=%d\n",
-			   status);
-		kfree(req);
-		usb_free_urb(urb);
-	}
-}
-
-static int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
-{
-	int offset = 0;
-
-	while (offset + sizeof(u32) < skb->len) {
-		struct sk_buff *ax_skb;
-		u16 size;
-		u32 header = get_unaligned_le32(skb->data + offset);
-
-		offset += sizeof(u32);
-
-		/* get the packet length */
-		size = (u16) (header & 0x7ff);
-		if (size != ((~header >> 16) & 0x07ff)) {
-			netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
-			return 0;
-		}
-
-		if ((size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) ||
-		    (size + offset > skb->len)) {
-			netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
-				   size);
-			return 0;
-		}
-		ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
-		if (!ax_skb)
-			return 0;
-
-		skb_put(ax_skb, size);
-		memcpy(ax_skb->data, skb->data + offset, size);
-		usbnet_skb_return(dev, ax_skb);
-
-		offset += (size + 1) & 0xfffe;
-	}
-
-	if (skb->len != offset) {
-		netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
-			   skb->len);
-		return 0;
-	}
-	return 1;
-}
-
-static struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
-					gfp_t flags)
-{
-	int padlen;
-	int headroom = skb_headroom(skb);
-	int tailroom = skb_tailroom(skb);
-	u32 packet_len;
-	u32 padbytes = 0xffff0000;
-
-	padlen = ((skb->len + 4) & (dev->maxpacket - 1)) ? 0 : 4;
-
-	if ((!skb_cloned(skb)) &&
-	    ((headroom + tailroom) >= (4 + padlen))) {
-		if ((headroom < 4) || (tailroom < padlen)) {
-			skb->data = memmove(skb->head + 4, skb->data, skb->len);
-			skb_set_tail_pointer(skb, skb->len);
-		}
-	} else {
-		struct sk_buff *skb2;
-		skb2 = skb_copy_expand(skb, 4, padlen, flags);
-		dev_kfree_skb_any(skb);
-		skb = skb2;
-		if (!skb)
-			return NULL;
-	}
-
-	skb_push(skb, 4);
-	packet_len = (((skb->len - 4) ^ 0x0000ffff) << 16) + (skb->len - 4);
-	cpu_to_le32s(&packet_len);
-	skb_copy_to_linear_data(skb, &packet_len, sizeof(packet_len));
-
-	if (padlen) {
-		cpu_to_le32s(&padbytes);
-		memcpy(skb_tail_pointer(skb), &padbytes, sizeof(padbytes));
-		skb_put(skb, sizeof(padbytes));
-	}
-	return skb;
-}
-
 static void asix_status(struct usbnet *dev, struct urb *urb)
 {
 	struct ax88172_int_data *event;
@@ -407,204 +68,6 @@ static void asix_status(struct usbnet *dev, struct urb *urb)
 	}
 }
 
-static inline int asix_set_sw_mii(struct usbnet *dev)
-{
-	int ret;
-	ret = asix_write_cmd(dev, AX_CMD_SET_SW_MII, 0x0000, 0, 0, NULL);
-	if (ret < 0)
-		netdev_err(dev->net, "Failed to enable software MII access\n");
-	return ret;
-}
-
-static inline int asix_set_hw_mii(struct usbnet *dev)
-{
-	int ret;
-	ret = asix_write_cmd(dev, AX_CMD_SET_HW_MII, 0x0000, 0, 0, NULL);
-	if (ret < 0)
-		netdev_err(dev->net, "Failed to enable hardware MII access\n");
-	return ret;
-}
-
-static inline int asix_get_phy_addr(struct usbnet *dev)
-{
-	u8 buf[2];
-	int ret = asix_read_cmd(dev, AX_CMD_READ_PHY_ID, 0, 0, 2, buf);
-
-	netdev_dbg(dev->net, "asix_get_phy_addr()\n");
-
-	if (ret < 0) {
-		netdev_err(dev->net, "Error reading PHYID register: %02x\n",
-			   ret);
-		goto out;
-	}
-	netdev_dbg(dev->net, "asix_get_phy_addr() returning 0x%04x\n",
-		   *((__le16 *)buf));
-	ret = buf[1];
-
-out:
-	return ret;
-}
-
-static int asix_sw_reset(struct usbnet *dev, u8 flags)
-{
-	int ret;
-
-	ret = asix_write_cmd(dev, AX_CMD_SW_RESET, flags, 0, 0, NULL);
-	if (ret < 0)
-		netdev_err(dev->net, "Failed to send software reset: %02x\n",
-			   ret);
-
-	return ret;
-}
-
-static u16 asix_read_rx_ctl(struct usbnet *dev)
-{
-	__le16 v;
-	int ret = asix_read_cmd(dev, AX_CMD_READ_RX_CTL, 0, 0, 2, &v);
-
-	if (ret < 0) {
-		netdev_err(dev->net, "Error reading RX_CTL register: %02x\n",
-			   ret);
-		goto out;
-	}
-	ret = le16_to_cpu(v);
-out:
-	return ret;
-}
-
-static int asix_write_rx_ctl(struct usbnet *dev, u16 mode)
-{
-	int ret;
-
-	netdev_dbg(dev->net, "asix_write_rx_ctl() - mode = 0x%04x\n", mode);
-	ret = asix_write_cmd(dev, AX_CMD_WRITE_RX_CTL, mode, 0, 0, NULL);
-	if (ret < 0)
-		netdev_err(dev->net, "Failed to write RX_CTL mode to 0x%04x: %02x\n",
-			   mode, ret);
-
-	return ret;
-}
-
-static u16 asix_read_medium_status(struct usbnet *dev)
-{
-	__le16 v;
-	int ret = asix_read_cmd(dev, AX_CMD_READ_MEDIUM_STATUS, 0, 0, 2, &v);
-
-	if (ret < 0) {
-		netdev_err(dev->net, "Error reading Medium Status register: %02x\n",
-			   ret);
-		return ret;	/* TODO: callers not checking for error ret */
-	}
-
-	return le16_to_cpu(v);
-
-}
-
-static int asix_write_medium_mode(struct usbnet *dev, u16 mode)
-{
-	int ret;
-
-	netdev_dbg(dev->net, "asix_write_medium_mode() - mode = 0x%04x\n",
-		   mode);
-	ret = asix_write_cmd(dev, AX_CMD_WRITE_MEDIUM_MODE, mode, 0, 0, NULL);
-	if (ret < 0)
-		netdev_err(dev->net, "Failed to write Medium Mode mode to 0x%04x: %02x\n",
-			   mode, ret);
-
-	return ret;
-}
-
-static int asix_write_gpio(struct usbnet *dev, u16 value, int sleep)
-{
-	int ret;
-
-	netdev_dbg(dev->net, "asix_write_gpio() - value = 0x%04x\n", value);
-	ret = asix_write_cmd(dev, AX_CMD_WRITE_GPIOS, value, 0, 0, NULL);
-	if (ret < 0)
-		netdev_err(dev->net, "Failed to write GPIO value 0x%04x: %02x\n",
-			   value, ret);
-
-	if (sleep)
-		msleep(sleep);
-
-	return ret;
-}
-
-/*
- * AX88772 & AX88178 have a 16-bit RX_CTL value
- */
-static void asix_set_multicast(struct net_device *net)
-{
-	struct usbnet *dev = netdev_priv(net);
-	struct asix_data *data = (struct asix_data *)&dev->data;
-	u16 rx_ctl = AX_DEFAULT_RX_CTL;
-
-	if (net->flags & IFF_PROMISC) {
-		rx_ctl |= AX_RX_CTL_PRO;
-	} else if (net->flags & IFF_ALLMULTI ||
-		   netdev_mc_count(net) > AX_MAX_MCAST) {
-		rx_ctl |= AX_RX_CTL_AMALL;
-	} else if (netdev_mc_empty(net)) {
-		/* just broadcast and directed */
-	} else {
-		/* We use the 20 byte dev->data
-		 * for our 8 byte filter buffer
-		 * to avoid allocating memory that
-		 * is tricky to free later */
-		struct netdev_hw_addr *ha;
-		u32 crc_bits;
-
-		memset(data->multi_filter, 0, AX_MCAST_FILTER_SIZE);
-
-		/* Build the multicast hash filter. */
-		netdev_for_each_mc_addr(ha, net) {
-			crc_bits = ether_crc(ETH_ALEN, ha->addr) >> 26;
-			data->multi_filter[crc_bits >> 3] |=
-			    1 << (crc_bits & 7);
-		}
-
-		asix_write_cmd_async(dev, AX_CMD_WRITE_MULTI_FILTER, 0, 0,
-				     AX_MCAST_FILTER_SIZE, data->multi_filter);
-
-		rx_ctl |= AX_RX_CTL_AM;
-	}
-
-	asix_write_cmd_async(dev, AX_CMD_WRITE_RX_CTL, rx_ctl, 0, 0, NULL);
-}
-
-static int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	__le16 res;
-
-	mutex_lock(&dev->phy_mutex);
-	asix_set_sw_mii(dev);
-	asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
-		      (__u16)loc, 2, &res);
-	asix_set_hw_mii(dev);
-	mutex_unlock(&dev->phy_mutex);
-
-	netdev_dbg(dev->net, "asix_mdio_read() phy_id=0x%02x, loc=0x%02x, returns=0x%04x\n",
-		   phy_id, loc, le16_to_cpu(res));
-
-	return le16_to_cpu(res);
-}
-
-static void
-asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	__le16 res = cpu_to_le16(val);
-
-	netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",
-		   phy_id, loc, val);
-	mutex_lock(&dev->phy_mutex);
-	asix_set_sw_mii(dev);
-	asix_write_cmd(dev, AX_CMD_WRITE_MII_REG, phy_id, (__u16)loc, 2, &res);
-	asix_set_hw_mii(dev);
-	mutex_unlock(&dev->phy_mutex);
-}
-
 /* Get the PHY Identifier from the PHYSID1 & PHYSID2 MII registers */
 static u32 asix_get_phyid(struct usbnet *dev)
 {
@@ -635,88 +98,6 @@ static u32 asix_get_phyid(struct usbnet *dev)
 	return phy_id;
 }
 
-static void
-asix_get_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
-{
-	struct usbnet *dev = netdev_priv(net);
-	u8 opt;
-
-	if (asix_read_cmd(dev, AX_CMD_READ_MONITOR_MODE, 0, 0, 1, &opt) < 0) {
-		wolinfo->supported = 0;
-		wolinfo->wolopts = 0;
-		return;
-	}
-	wolinfo->supported = WAKE_PHY | WAKE_MAGIC;
-	wolinfo->wolopts = 0;
-	if (opt & AX_MONITOR_LINK)
-		wolinfo->wolopts |= WAKE_PHY;
-	if (opt & AX_MONITOR_MAGIC)
-		wolinfo->wolopts |= WAKE_MAGIC;
-}
-
-static int
-asix_set_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
-{
-	struct usbnet *dev = netdev_priv(net);
-	u8 opt = 0;
-
-	if (wolinfo->wolopts & WAKE_PHY)
-		opt |= AX_MONITOR_LINK;
-	if (wolinfo->wolopts & WAKE_MAGIC)
-		opt |= AX_MONITOR_MAGIC;
-
-	if (asix_write_cmd(dev, AX_CMD_WRITE_MONITOR_MODE,
-			   opt, 0, 0, NULL) < 0)
-		return -EINVAL;
-
-	return 0;
-}
-
-static int asix_get_eeprom_len(struct net_device *net)
-{
-	struct usbnet *dev = netdev_priv(net);
-	struct asix_data *data = (struct asix_data *)&dev->data;
-
-	return data->eeprom_len;
-}
-
-static int asix_get_eeprom(struct net_device *net,
-			      struct ethtool_eeprom *eeprom, u8 *data)
-{
-	struct usbnet *dev = netdev_priv(net);
-	__le16 *ebuf = (__le16 *)data;
-	int i;
-
-	/* Crude hack to ensure that we don't overwrite memory
-	 * if an odd length is supplied
-	 */
-	if (eeprom->len % 2)
-		return -EINVAL;
-
-	eeprom->magic = AX_EEPROM_MAGIC;
-
-	/* ax8817x returns 2 bytes from eeprom on read */
-	for (i = 0; i < eeprom->len / 2; i++) {
-		if (asix_read_cmd(dev, AX_CMD_READ_EEPROM,
-				  eeprom->offset + i, 0, 2, &ebuf[i]) < 0)
-			return -EINVAL;
-	}
-	return 0;
-}
-
-static void asix_get_drvinfo(struct net_device *net,
-			     struct ethtool_drvinfo *info)
-{
-	struct usbnet *dev = netdev_priv(net);
-	struct asix_data *data = (struct asix_data *)&dev->data;
-
-	/* Inherit standard device info */
-	usbnet_get_drvinfo(net, info);
-	strncpy(info->driver, DRIVER_NAME, sizeof info->driver);
-	strncpy(info->version, DRIVER_VERSION, sizeof info->version);
-	info->eedump_len = data->eeprom_len;
-}
-
 static u32 asix_get_link(struct net_device *net)
 {
 	struct usbnet *dev = netdev_priv(net);
@@ -731,30 +112,6 @@ static int asix_ioctl(struct net_device *net, struct ifreq *rq, int cmd)
 	return generic_mii_ioctl(&dev->mii, if_mii(rq), cmd, NULL);
 }
 
-static int asix_set_mac_address(struct net_device *net, void *p)
-{
-	struct usbnet *dev = netdev_priv(net);
-	struct asix_data *data = (struct asix_data *)&dev->data;
-	struct sockaddr *addr = p;
-
-	if (netif_running(net))
-		return -EBUSY;
-	if (!is_valid_ether_addr(addr->sa_data))
-		return -EADDRNOTAVAIL;
-
-	memcpy(net->dev_addr, addr->sa_data, ETH_ALEN);
-
-	/* We use the 20 byte dev->data
-	 * for our 6 byte mac buffer
-	 * to avoid allocating memory that
-	 * is tricky to free later */
-	memcpy(data->mac_addr, addr->sa_data, ETH_ALEN);
-	asix_write_cmd_async(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN,
-			     data->mac_addr);
-
-	return 0;
-}
-
 /* We need to override some ethtool_ops so we require our
    own structure so we don't interfere with other usbnet
    devices that may be connected at the same time. */
-- 
1.7.0.4

[PATCH 4/4] asix: Add a new driver for the AX88172A

[Christian Riesch]

The Asix AX88172A is a USB 2.0 Ethernet interface that supports both an
internal PHY as well as an external PHY (connected via MII).

This patch adds a driver for the AX88172A and provides support for
both modes and supports phylib.

Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
---
 drivers/net/usb/Makefile       |    2 +-
 drivers/net/usb/asix_devices.c |    6 +
 drivers/net/usb/ax88172a.c     |  407 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 414 insertions(+), 1 deletions(-)
 create mode 100644 drivers/net/usb/ax88172a.c

diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index a9490d9..bf06300 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_USB_PEGASUS)	+= pegasus.o
 obj-$(CONFIG_USB_RTL8150)	+= rtl8150.o
 obj-$(CONFIG_USB_HSO)		+= hso.o
 obj-$(CONFIG_USB_NET_AX8817X)	+= asix.o
-asix-y := asix_devices.o asix_common.o
+asix-y := asix_devices.o asix_common.o ax88172a.o
 obj-$(CONFIG_USB_NET_CDCETHER)	+= cdc_ether.o
 obj-$(CONFIG_USB_NET_CDC_EEM)	+= cdc_eem.o
 obj-$(CONFIG_USB_NET_DM9601)	+= dm9601.o
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index c8682a5..02b8c21 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -877,6 +877,8 @@ static const struct driver_info ax88178_info = {
 	.tx_fixup = asix_tx_fixup,
 };
 
+extern const struct driver_info ax88172a_info;
+
 static const struct usb_device_id	products[] = {
 {
 	/* Linksys USB200M */
@@ -1002,6 +1004,10 @@ static const struct usb_device_id	products[] = {
 	/* Asus USB Ethernet Adapter */
 	USB_DEVICE(0x0b95, 0x7e2b),
 	.driver_info = (unsigned long) &ax88772_info,
+}, {
+	/* ASIX 88172a demo board */
+	USB_DEVICE(0x0b95, 0x172a),
+	.driver_info = (unsigned long) &ax88172a_info,
 },
 	{ },		/* END */
 };
diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
new file mode 100644
index 0000000..9f2d1fd
--- /dev/null
+++ b/drivers/net/usb/ax88172a.c
@@ -0,0 +1,407 @@
+/*
+ * ASIX AX88172A based USB 2.0 Ethernet Devices
+ * Copyright (C) 2012 OMICRON electronics GmbH
+ *
+ * Supports external PHYs via phylib. Based on the driver for the
+ * AX88772. Original copyrights follow:
+ *
+ * Copyright (C) 2003-2006 David Hollis <dhollis@davehollis.com>
+ * Copyright (C) 2005 Phil Chang <pchang23@sbcglobal.net>
+ * Copyright (C) 2006 James Painter <jamie.painter@iname.com>
+ * Copyright (c) 2002-2003 TiVo Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include "asix.h"
+#include <linux/phy.h>
+
+struct ax88172a_private {
+	int use_embdphy;
+	struct mii_bus *mdio;
+	struct phy_device *phydev;
+	char phy_name[20];
+	u16 phy_addr;
+	u16 oldmode;
+};
+
+static inline int asix_read_phy_addr(struct usbnet *dev, int internal)
+{
+	int offset = (internal ? 1 : 0);
+	u8 buf[2];
+	int ret = asix_read_cmd(dev, AX_CMD_READ_PHY_ID, 0, 0, 2, buf);
+
+	netdev_dbg(dev->net, "asix_get_phy_addr()\n");
+
+	if (ret < 0) {
+		netdev_err(dev->net, "Error reading PHYID register: %02x\n",
+			   ret);
+		goto out;
+	}
+	netdev_dbg(dev->net, "asix_get_phy_addr() returning 0x%04x\n",
+		   *((__le16 *)buf));
+	ret = buf[offset];
+
+out:
+	return ret;
+}
+
+static int ax88172a_ioctl(struct net_device *net, struct ifreq *rq, int cmd)
+{
+	return phy_mii_ioctl(net->phydev, rq, cmd);
+}
+
+/* MDIO read and write wrappers for phylib */
+static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+	return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id,
+			      regnum);
+}
+
+static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum,
+			       u16 val)
+{
+	asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum,
+			val);
+	return 0;
+}
+
+/* set MAC link settings according to information from phylib */
+static void asix_adjust_link(struct net_device *netdev)
+{
+	struct phy_device *phydev = netdev->phydev;
+	struct usbnet *dev = netdev_priv(netdev);
+	struct ax88172a_private *priv =
+		(struct ax88172a_private *)dev->driver_priv;
+	u16 mode = 0;
+
+	dbg("asix_adjust_link called\n");
+
+	if (phydev->link) {
+		mode = AX88772_MEDIUM_DEFAULT;
+
+		if (phydev->duplex == DUPLEX_HALF)
+			mode &= ~AX_MEDIUM_FD;
+
+		if (phydev->speed != SPEED_100)
+			mode &= ~AX_MEDIUM_PS;
+	}
+
+	if (mode != priv->oldmode) {
+		asix_write_medium_mode(dev, mode);
+		priv->oldmode = mode;
+		dbg("asix_adjust_link  speed: %u duplex: %d setting mode to 0x%04x\n",
+		    phydev->speed, phydev->duplex, mode);
+		phy_print_status(phydev);
+	}
+}
+
+static void ax88172a_status(struct usbnet *dev, struct urb *urb)
+{
+}
+
+/* use phylib infrastructure */
+static int ax88172a_init_mdio(struct usbnet *dev)
+{
+	struct ax88172a_private *priv =
+		(struct ax88172a_private *)dev->driver_priv;
+	int ret, i;
+
+	priv->mdio = mdiobus_alloc();
+	if (!priv->mdio) {
+		dbg("Could not allocate MDIO bus");
+		return -1;
+	}
+
+	priv->mdio->priv = (void *)dev;
+	priv->mdio->read = &asix_mdio_bus_read;
+	priv->mdio->write = &asix_mdio_bus_write;
+	priv->mdio->name = "Asix MDIO Bus";
+	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%s",
+		 dev_name(dev->net->dev.parent));
+
+	priv->mdio->irq = kzalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
+	if (!priv->mdio->irq) {
+		dbg("Could not allocate MDIO->IRQ");
+		ret = -ENOMEM;
+		goto mfree;
+	}
+	for (i = 0; i < PHY_MAX_ADDR; i++)
+		priv->mdio->irq[i] = PHY_POLL;
+
+	ret = mdiobus_register(priv->mdio);
+	if (ret) {
+		dbg("Could not register MDIO bus");
+		goto ifree;
+	}
+	snprintf(priv->phy_name, 20, PHY_ID_FMT,
+		 priv->mdio->id, priv->phy_addr);
+
+	priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link,
+				   0, PHY_INTERFACE_MODE_MII);
+	if (IS_ERR(priv->phydev)) {
+		dbg("Could not connect to PHY device");
+		ret = PTR_ERR(priv->phydev);
+		goto munreg;
+	}
+	dbg("dev->net->phydev (%s) is now 0x%p", priv->phy_name,
+	    dev->net->phydev);
+
+	/* During power-up, the AX88172A set the power down (BMCR_PDOWN)
+	 *   bit of the PHY. Bring the PHY up again.
+	 */
+	genphy_resume(priv->phydev);
+
+	phy_start(priv->phydev);
+
+	return 0;
+munreg:
+	mdiobus_unregister(priv->mdio);
+ifree:
+	kfree(priv->mdio->irq);
+mfree:
+	mdiobus_free(priv->mdio);
+	return ret;
+}
+
+static void ax88172a_remove_mdio(struct usbnet *dev)
+{
+	struct ax88172a_private *priv =
+		(struct ax88172a_private *)dev->driver_priv;
+
+	phy_stop(priv->phydev);
+	phy_disconnect(priv->phydev);
+	mdiobus_unregister(priv->mdio);
+	kfree(priv->mdio->irq);
+	mdiobus_free(priv->mdio);
+}
+
+static const struct net_device_ops ax88172a_netdev_ops = {
+	.ndo_open		= usbnet_open,
+	.ndo_stop		= usbnet_stop,
+	.ndo_start_xmit		= usbnet_start_xmit,
+	.ndo_tx_timeout		= usbnet_tx_timeout,
+	.ndo_change_mtu		= usbnet_change_mtu,
+	.ndo_set_mac_address	= asix_set_mac_address,
+	.ndo_validate_addr	= eth_validate_addr,
+	.ndo_do_ioctl		= ax88172a_ioctl,
+	.ndo_set_rx_mode        = asix_set_multicast,
+};
+
+int ax88172a_get_settings(struct net_device *net, struct ethtool_cmd *cmd)
+{
+	return phy_ethtool_gset(net->phydev, cmd);
+}
+
+int ax88172a_set_settings(struct net_device *net, struct ethtool_cmd *cmd)
+{
+	return phy_ethtool_sset(net->phydev, cmd);
+}
+
+int ax88172a_nway_reset(struct net_device *net)
+{
+	return phy_start_aneg(net->phydev);
+}
+
+static const struct ethtool_ops ax88172a_ethtool_ops = {
+	.get_drvinfo		= asix_get_drvinfo,
+	.get_link		= usbnet_get_link,
+	.get_msglevel		= usbnet_get_msglevel,
+	.set_msglevel		= usbnet_set_msglevel,
+	.get_wol		= asix_get_wol,
+	.set_wol		= asix_set_wol,
+	.get_eeprom_len		= asix_get_eeprom_len,
+	.get_eeprom		= asix_get_eeprom,
+	.get_settings		= ax88172a_get_settings,
+	.set_settings		= ax88172a_set_settings,
+	.nway_reset		= ax88172a_nway_reset,
+};
+
+static int ax88172a_reset_phy(struct usbnet *dev, int embd_phy)
+{
+	int ret;
+
+	ret = asix_sw_reset(dev, AX_SWRESET_IPPD);
+	if (ret < 0)
+		goto err;
+
+	msleep(150);
+	ret = asix_sw_reset(dev, AX_SWRESET_CLEAR);
+	if (ret < 0)
+		goto err;
+
+	msleep(150);
+
+	ret = asix_sw_reset(dev, embd_phy ? AX_SWRESET_IPRL : AX_SWRESET_IPPD);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+
+err:
+	return ret;
+}
+
+
+static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+	int ret;
+	struct asix_data *data = (struct asix_data *)&dev->data;
+	u8 buf[ETH_ALEN];
+	struct ax88172a_private *priv;
+
+	data->eeprom_len = AX88772_EEPROM_LEN;
+
+	usbnet_get_endpoints(dev, intf);
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dbg("Could not allocate memory for private data");
+		return -ENOMEM;
+	}
+	dev->driver_priv = priv;
+
+	/* Get the MAC address */
+	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
+	if (ret < 0) {
+		dbg("Failed to read MAC address: %d", ret);
+		goto free;
+	}
+	memcpy(dev->net->dev_addr, buf, ETH_ALEN);
+
+	dev->net->netdev_ops = &ax88172a_netdev_ops;
+	dev->net->ethtool_ops = &ax88172a_ethtool_ops;
+
+	/* are we using the internal or the external phy? */
+	ret = asix_read_cmd(dev, AX_CMD_SW_PHY_STATUS, 0, 0, 1, buf);
+	if (ret < 0) {
+		dbg("Failed to read software interface selection register: %d",
+		    ret);
+		goto free;
+	}
+	dbg("AX_CMD_SW_PHY_STATUS = 0x%02x\n", buf[0]);
+	switch ((buf[0] & 0x0c) >> 2) {
+	case 0:
+		dbg("use internal phy\n");
+		priv->use_embdphy = 1;
+		break;
+	case 1:
+		dbg("use external phy\n");
+		priv->use_embdphy = 0;
+		break;
+	default:
+		dbg("Interface mode not supported by driver\n");
+		goto free;
+	}
+
+	priv->phy_addr = asix_read_phy_addr(dev, priv->use_embdphy);
+	ax88172a_reset_phy(dev, priv->use_embdphy);
+
+	/* Asix framing packs multiple eth frames into a 2K usb bulk transfer */
+	if (dev->driver_info->flags & FLAG_FRAMING_AX) {
+		/* hard_mtu  is still the default - the device does not support
+		   jumbo eth frames */
+		dev->rx_urb_size = 2048;
+	}
+
+	/* init MDIO bus */
+	ret = ax88172a_init_mdio(dev);
+	if (ret)
+		goto free;
+
+	return 0;
+
+free:
+	kfree(priv);
+	return ret;
+}
+
+static void ax88172a_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+	struct ax88172a_private *priv =
+		(struct ax88172a_private *)dev->driver_priv;
+
+	ax88172a_remove_mdio(dev);
+	kfree(priv);
+}
+
+static int ax88172a_reset(struct usbnet *dev)
+{
+	struct asix_data *data = (struct asix_data *)&dev->data;
+	struct ax88172a_private *priv =
+		(struct ax88172a_private *)dev->driver_priv;
+	int ret;
+	u16 rx_ctl;
+
+	ax88172a_reset_phy(dev, priv->use_embdphy);
+
+	msleep(150);
+	rx_ctl = asix_read_rx_ctl(dev);
+	dbg("RX_CTL is 0x%04x after software reset", rx_ctl);
+	ret = asix_write_rx_ctl(dev, 0x0000);
+	if (ret < 0)
+		goto out;
+
+	rx_ctl = asix_read_rx_ctl(dev);
+	dbg("RX_CTL is 0x%04x setting to 0x0000", rx_ctl);
+
+	msleep(150);
+
+	ax88172a_nway_reset(dev->net);
+
+	ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0,
+				AX88772_IPG0_DEFAULT | AX88772_IPG1_DEFAULT,
+				AX88772_IPG2_DEFAULT, 0, NULL);
+	if (ret < 0) {
+		dbg("Write IPG,IPG1,IPG2 failed: %d", ret);
+		goto out;
+	}
+
+	/* Rewrite MAC address */
+	memcpy(data->mac_addr, dev->net->dev_addr, ETH_ALEN);
+	ret = asix_write_cmd(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN,
+							data->mac_addr);
+	if (ret < 0)
+		goto out;
+
+	/* Set RX_CTL to default values with 2k buffer, and enable cactus */
+	ret = asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL);
+	if (ret < 0)
+		goto out;
+
+	rx_ctl = asix_read_rx_ctl(dev);
+	dbg("RX_CTL is 0x%04x after all initializations", rx_ctl);
+
+	rx_ctl = asix_read_medium_status(dev);
+	dbg("Medium Status is 0x%04x after all initializations", rx_ctl);
+
+	return 0;
+
+out:
+	return ret;
+
+}
+
+const struct driver_info ax88172a_info = {
+	.description = "ASIX AX88172A USB 2.0 Ethernet",
+	.bind = ax88172a_bind,
+	.unbind = ax88172a_unbind,
+	.status = ax88172a_status,
+	.reset = ax88172a_reset,
+	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
+		 FLAG_MULTI_PACKET,
+	.rx_fixup = asix_rx_fixup,
+	.tx_fixup = asix_tx_fixup,
+};
-- 
1.7.0.4

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Ben Hutchings]

On Fri, 2012-07-06 at 13:33 +0200, Christian Riesch wrote:
> The Asix AX88172A is a USB 2.0 Ethernet interface that supports both an
> internal PHY as well as an external PHY (connected via MII).
> 
> This patch adds a driver for the AX88172A and provides support for
> both modes and supports phylib.
[...]
> +static int ax88172a_init_mdio(struct usbnet *dev)
> +{
> +	struct ax88172a_private *priv =
> +		(struct ax88172a_private *)dev->driver_priv;
> +	int ret, i;
> +
> +	priv->mdio = mdiobus_alloc();
> +	if (!priv->mdio) {
> +		dbg("Could not allocate MDIO bus");
> +		return -1;
> +	}
> +
> +	priv->mdio->priv = (void *)dev;
> +	priv->mdio->read = &asix_mdio_bus_read;
> +	priv->mdio->write = &asix_mdio_bus_write;
> +	priv->mdio->name = "Asix MDIO Bus";
> +	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%s",
> +		 dev_name(dev->net->dev.parent));
[...]

I think you need to ensure that the bus identifier is unique throughout
its lifetime, but net devices can be renamed and that could lead to a
collision.  Perhaps you could use the ifindex or the USB device path
(though that might be too long).

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Michael Riesch]

On Fri, 2012-07-06 at 18:37 +0100, Ben Hutchings wrote:
> > +	priv->mdio->priv = (void *)dev;
> > +	priv->mdio->read = &asix_mdio_bus_read;
> > +	priv->mdio->write = &asix_mdio_bus_write;
> > +	priv->mdio->name = "Asix MDIO Bus";
> > +	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%s",
> > +		 dev_name(dev->net->dev.parent));
> [...]
> 
> I think you need to ensure that the bus identifier is unique throughout
> its lifetime, but net devices can be renamed and that could lead to a
> collision.  Perhaps you could use the ifindex or the USB device path

Ben,

the dev_name function in the code above returns the sysfs filename of
the USB device (e.g. 1-0:1.0).

> (though that might be too long).

This may be a problem. The bus identifier may be 17 characters long, so
if we leave the endpoint/configuration part (:1.0) and the prefix away
it should be fine in any "normal" system. However, on a system with a
more-than-9-root-hubs 5-tier 127-devices-each USB infrastructure it
results in collisions. So is this approach acceptable?

Using the ifindex sounds good to me,

snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%d",
	dev->net->ifindex);

works on any system with less than 10^12 network interfaces.

Regards,
Michael

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Ben Hutchings]

On Sun, 2012-07-08 at 17:39 +0200, Michael Riesch wrote:
> On Fri, 2012-07-06 at 18:37 +0100, Ben Hutchings wrote:
> > > +	priv->mdio->priv = (void *)dev;
> > > +	priv->mdio->read = &asix_mdio_bus_read;
> > > +	priv->mdio->write = &asix_mdio_bus_write;
> > > +	priv->mdio->name = "Asix MDIO Bus";
> > > +	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%s",
> > > +		 dev_name(dev->net->dev.parent));
> > [...]
> > 
> > I think you need to ensure that the bus identifier is unique throughout
> > its lifetime, but net devices can be renamed and that could lead to a
> > collision.  Perhaps you could use the ifindex or the USB device path
> 
> Ben,
> 
> the dev_name function in the code above returns the sysfs filename of
> the USB device (e.g. 1-0:1.0).
[...]

Sorry, I didn't read that carefully enough.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Christian Riesch]

Hi Ben and Michael,

On Sun, Jul 8, 2012 at 5:39 PM, Michael Riesch <michael@riesch.at> wrote:
> On Fri, 2012-07-06 at 18:37 +0100, Ben Hutchings wrote:
>> > +   priv->mdio->priv = (void *)dev;
>> > +   priv->mdio->read = &asix_mdio_bus_read;
>> > +   priv->mdio->write = &asix_mdio_bus_write;
>> > +   priv->mdio->name = "Asix MDIO Bus";
>> > +   snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%s",
>> > +            dev_name(dev->net->dev.parent));
>> [...]
>>
>> I think you need to ensure that the bus identifier is unique throughout
>> its lifetime, but net devices can be renamed and that could lead to a
>> collision.  Perhaps you could use the ifindex or the USB device path
>
> Ben,
>
> the dev_name function in the code above returns the sysfs filename of
> the USB device (e.g. 1-0:1.0).
>
>> (though that might be too long).
>
> This may be a problem. The bus identifier may be 17 characters long, so
> if we leave the endpoint/configuration part (:1.0) and the prefix away
> it should be fine in any "normal" system. However, on a system with a
> more-than-9-root-hubs 5-tier 127-devices-each USB infrastructure it
> results in collisions. So is this approach acceptable?
>
> Using the ifindex sounds good to me,
>
> snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%d",
>         dev->net->ifindex);
>
> works on any system with less than 10^12 network interfaces.

Ok, I'll change that to use ifindex. I didn't like the mdio bus name
with lots of colons, dashes, and periods anyway...
Thank you!

Regards, Christian

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Christian Riesch]

Hi Ben and Michael,

On Mon, Jul 9, 2012 at 12:30 PM, Christian Riesch
<christian.riesch@omicron.at> wrote:
> Hi Ben and Michael,
>
> On Sun, Jul 8, 2012 at 5:39 PM, Michael Riesch <michael@riesch.at> wrote:
>> On Fri, 2012-07-06 at 18:37 +0100, Ben Hutchings wrote:
>>> > +   priv->mdio->priv = (void *)dev;
>>> > +   priv->mdio->read = &asix_mdio_bus_read;
>>> > +   priv->mdio->write = &asix_mdio_bus_write;
>>> > +   priv->mdio->name = "Asix MDIO Bus";
>>> > +   snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%s",
>>> > +            dev_name(dev->net->dev.parent));
>>> [...]
>>>
>>> I think you need to ensure that the bus identifier is unique throughout
>>> its lifetime, but net devices can be renamed and that could lead to a
>>> collision.  Perhaps you could use the ifindex or the USB device path
>>
>> Ben,
>>
>> the dev_name function in the code above returns the sysfs filename of
>> the USB device (e.g. 1-0:1.0).
>>
>>> (though that might be too long).
>>
>> This may be a problem. The bus identifier may be 17 characters long, so
>> if we leave the endpoint/configuration part (:1.0) and the prefix away
>> it should be fine in any "normal" system. However, on a system with a
>> more-than-9-root-hubs 5-tier 127-devices-each USB infrastructure it
>> results in collisions. So is this approach acceptable?
>>
>> Using the ifindex sounds good to me,
>>
>> snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%d",
>>         dev->net->ifindex);
>>
>> works on any system with less than 10^12 network interfaces.
>
> Ok, I'll change that to use ifindex.

No, I won't.
At the time the mdio bus is registered, ifindex is not yet set, so the
snprintf would always result in "asix-0".

Any ideas?
Should I keep the dev_name(dev->net->dev.parent) and hope that nobody
uses that many USB devices/hubs so that it is short enough? Or use
some other solution? A global counter in ax88172a.c that numbers the
mdio busses (asix-0, asix-1...)?

Regards, Christian

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Christian Riesch]

Hi again,

On Wed, Jul 11, 2012 at 10:27 AM, Christian Riesch
<christian.riesch@omicron.at> wrote:
> Hi Ben and Michael,
>
> On Mon, Jul 9, 2012 at 12:30 PM, Christian Riesch
> <christian.riesch@omicron.at> wrote:
>> Hi Ben and Michael,
>>
>> On Sun, Jul 8, 2012 at 5:39 PM, Michael Riesch <michael@riesch.at> wrote:
>>> On Fri, 2012-07-06 at 18:37 +0100, Ben Hutchings wrote:
>>>> > +   priv->mdio->priv = (void *)dev;
>>>> > +   priv->mdio->read = &asix_mdio_bus_read;
>>>> > +   priv->mdio->write = &asix_mdio_bus_write;
>>>> > +   priv->mdio->name = "Asix MDIO Bus";
>>>> > +   snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%s",
>>>> > +            dev_name(dev->net->dev.parent));
>>>> [...]
>>>>
>>>> I think you need to ensure that the bus identifier is unique throughout
>>>> its lifetime, but net devices can be renamed and that could lead to a
>>>> collision.  Perhaps you could use the ifindex or the USB device path
>>>
>>> Ben,
>>>
>>> the dev_name function in the code above returns the sysfs filename of
>>> the USB device (e.g. 1-0:1.0).
>>>
>>>> (though that might be too long).
>>>
>>> This may be a problem. The bus identifier may be 17 characters long, so
>>> if we leave the endpoint/configuration part (:1.0) and the prefix away
>>> it should be fine in any "normal" system. However, on a system with a
>>> more-than-9-root-hubs 5-tier 127-devices-each USB infrastructure it
>>> results in collisions. So is this approach acceptable?
>>>
>>> Using the ifindex sounds good to me,
>>>
>>> snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%d",
>>>         dev->net->ifindex);
>>>
>>> works on any system with less than 10^12 network interfaces.
>>
>> Ok, I'll change that to use ifindex.
>
> No, I won't.
> At the time the mdio bus is registered, ifindex is not yet set, so the
> snprintf would always result in "asix-0".

What do you think about
snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
dev->udev->bus->busnum, dev->udev->devnum);
??

This would use the busnum/devnum identifier as reported by lsusb and
would be short enough for an mdio bus name.

Thanks, Christian

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Michael Riesch]

On Wed, 2012-07-11 at 17:10 +0200, Christian Riesch wrote:
> Hi again,
[...]
> >>> Using the ifindex sounds good to me,
> >>>
> >>> snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%d",
> >>>         dev->net->ifindex);
> >>>
> >>> works on any system with less than 10^12 network interfaces.
> >>
> >> Ok, I'll change that to use ifindex.
> >
> > No, I won't.
> > At the time the mdio bus is registered, ifindex is not yet set, so the
> > snprintf would always result in "asix-0".
> 
> What do you think about
> snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
> dev->udev->bus->busnum, dev->udev->devnum);
> ??
> 
> This would use the busnum/devnum identifier as reported by lsusb and
> would be short enough for an mdio bus name.

IMHO this would be a good solution - it is unique and there you can tell
which MDIO bus belongs to which USB device by the name (not sure if
someone will ever need to do that, but it is neat).

Regards, Michael

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Grant Grundler]

On Fri, Jul 6, 2012 at 4:33 AM, Christian Riesch
<christian.riesch@omicron.at> wrote:
> The Asix AX88172A is a USB 2.0 Ethernet interface that supports both an
> internal PHY as well as an external PHY (connected via MII).
>
> This patch adds a driver for the AX88172A and provides support for
> both modes and supports phylib.

Christian,
In general this looks fine to me...but I wouldn't know about "bus
identifier life times" (Ben Hutchings comment).

My nit pick is the declaration and of use_embdphy. An alternative
coding _suggestion_ below.  I'm not substantially altering the
functionality.

thanks,
grant

>
> Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
> ---
>  drivers/net/usb/Makefile       |    2 +-
>  drivers/net/usb/asix_devices.c |    6 +
>  drivers/net/usb/ax88172a.c     |  407 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 414 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/net/usb/ax88172a.c
>
> diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
> index a9490d9..bf06300 100644
> --- a/drivers/net/usb/Makefile
> +++ b/drivers/net/usb/Makefile
> @@ -8,7 +8,7 @@ obj-$(CONFIG_USB_PEGASUS)       += pegasus.o
>  obj-$(CONFIG_USB_RTL8150)      += rtl8150.o
>  obj-$(CONFIG_USB_HSO)          += hso.o
>  obj-$(CONFIG_USB_NET_AX8817X)  += asix.o
> -asix-y := asix_devices.o asix_common.o
> +asix-y := asix_devices.o asix_common.o ax88172a.o
>  obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o
>  obj-$(CONFIG_USB_NET_CDC_EEM)  += cdc_eem.o
>  obj-$(CONFIG_USB_NET_DM9601)   += dm9601.o
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index c8682a5..02b8c21 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -877,6 +877,8 @@ static const struct driver_info ax88178_info = {
>         .tx_fixup = asix_tx_fixup,
>  };
>
> +extern const struct driver_info ax88172a_info;
> +
>  static const struct usb_device_id      products[] = {
>  {
>         /* Linksys USB200M */
> @@ -1002,6 +1004,10 @@ static const struct usb_device_id        products[] = {
>         /* Asus USB Ethernet Adapter */
>         USB_DEVICE(0x0b95, 0x7e2b),
>         .driver_info = (unsigned long) &ax88772_info,
> +}, {
> +       /* ASIX 88172a demo board */
> +       USB_DEVICE(0x0b95, 0x172a),
> +       .driver_info = (unsigned long) &ax88172a_info,
>  },
>         { },            /* END */
>  };
> diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
> new file mode 100644
> index 0000000..9f2d1fd
> --- /dev/null
> +++ b/drivers/net/usb/ax88172a.c
> @@ -0,0 +1,407 @@
> +/*
> + * ASIX AX88172A based USB 2.0 Ethernet Devices
> + * Copyright (C) 2012 OMICRON electronics GmbH
> + *
> + * Supports external PHYs via phylib. Based on the driver for the
> + * AX88772. Original copyrights follow:
> + *
> + * Copyright (C) 2003-2006 David Hollis <dhollis@davehollis.com>
> + * Copyright (C) 2005 Phil Chang <pchang23@sbcglobal.net>
> + * Copyright (C) 2006 James Painter <jamie.painter@iname.com>
> + * Copyright (c) 2002-2003 TiVo Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#include "asix.h"
> +#include <linux/phy.h>
> +
> +struct ax88172a_private {
> +       int use_embdphy;

Can you move the "int" to the end of the struct?
It's cleaner to have fields "natively align". ie pointers should start
at 8 byte alignments when compiled for 64-bit.

> +       struct mii_bus *mdio;
> +       struct phy_device *phydev;
> +       char phy_name[20];
> +       u16 phy_addr;
> +       u16 oldmode;
> +};
> +
> +static inline int asix_read_phy_addr(struct usbnet *dev, int internal)
> +{
> +       int offset = (internal ? 1 : 0);

One could use "internal" parameter directly for indexing if
use_embdphy were renamed to use_extphy and the logic inverted..

> +       u8 buf[2];
> +       int ret = asix_read_cmd(dev, AX_CMD_READ_PHY_ID, 0, 0, 2, buf);
> +
> +       netdev_dbg(dev->net, "asix_get_phy_addr()\n");
> +
> +       if (ret < 0) {
> +               netdev_err(dev->net, "Error reading PHYID register: %02x\n",
> +                          ret);
> +               goto out;
> +       }
> +       netdev_dbg(dev->net, "asix_get_phy_addr() returning 0x%04x\n",
> +                  *((__le16 *)buf));
> +       ret = buf[offset];
> +
> +out:
> +       return ret;
> +}
> +
> +static int ax88172a_ioctl(struct net_device *net, struct ifreq *rq, int cmd)
> +{
> +       return phy_mii_ioctl(net->phydev, rq, cmd);
> +}
> +
> +/* MDIO read and write wrappers for phylib */
> +static int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
> +{
> +       return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id,
> +                             regnum);
> +}
> +
> +static int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum,
> +                              u16 val)
> +{
> +       asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum,
> +                       val);
> +       return 0;
> +}
> +
> +/* set MAC link settings according to information from phylib */
> +static void asix_adjust_link(struct net_device *netdev)
> +{
> +       struct phy_device *phydev = netdev->phydev;
> +       struct usbnet *dev = netdev_priv(netdev);
> +       struct ax88172a_private *priv =
> +               (struct ax88172a_private *)dev->driver_priv;
> +       u16 mode = 0;
> +
> +       dbg("asix_adjust_link called\n");
> +
> +       if (phydev->link) {
> +               mode = AX88772_MEDIUM_DEFAULT;
> +
> +               if (phydev->duplex == DUPLEX_HALF)
> +                       mode &= ~AX_MEDIUM_FD;
> +
> +               if (phydev->speed != SPEED_100)
> +                       mode &= ~AX_MEDIUM_PS;
> +       }
> +
> +       if (mode != priv->oldmode) {
> +               asix_write_medium_mode(dev, mode);
> +               priv->oldmode = mode;
> +               dbg("asix_adjust_link  speed: %u duplex: %d setting mode to 0x%04x\n",
> +                   phydev->speed, phydev->duplex, mode);
> +               phy_print_status(phydev);
> +       }
> +}
> +
> +static void ax88172a_status(struct usbnet *dev, struct urb *urb)
> +{
> +}
> +
> +/* use phylib infrastructure */
> +static int ax88172a_init_mdio(struct usbnet *dev)
> +{
> +       struct ax88172a_private *priv =
> +               (struct ax88172a_private *)dev->driver_priv;
> +       int ret, i;
> +
> +       priv->mdio = mdiobus_alloc();
> +       if (!priv->mdio) {
> +               dbg("Could not allocate MDIO bus");
> +               return -1;
> +       }
> +
> +       priv->mdio->priv = (void *)dev;
> +       priv->mdio->read = &asix_mdio_bus_read;
> +       priv->mdio->write = &asix_mdio_bus_write;
> +       priv->mdio->name = "Asix MDIO Bus";
> +       snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%s",
> +                dev_name(dev->net->dev.parent));
> +
> +       priv->mdio->irq = kzalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
> +       if (!priv->mdio->irq) {
> +               dbg("Could not allocate MDIO->IRQ");
> +               ret = -ENOMEM;
> +               goto mfree;
> +       }
> +       for (i = 0; i < PHY_MAX_ADDR; i++)
> +               priv->mdio->irq[i] = PHY_POLL;
> +
> +       ret = mdiobus_register(priv->mdio);
> +       if (ret) {
> +               dbg("Could not register MDIO bus");
> +               goto ifree;
> +       }
> +       snprintf(priv->phy_name, 20, PHY_ID_FMT,
> +                priv->mdio->id, priv->phy_addr);
> +
> +       priv->phydev = phy_connect(dev->net, priv->phy_name, &asix_adjust_link,
> +                                  0, PHY_INTERFACE_MODE_MII);
> +       if (IS_ERR(priv->phydev)) {
> +               dbg("Could not connect to PHY device");
> +               ret = PTR_ERR(priv->phydev);
> +               goto munreg;
> +       }
> +       dbg("dev->net->phydev (%s) is now 0x%p", priv->phy_name,
> +           dev->net->phydev);
> +
> +       /* During power-up, the AX88172A set the power down (BMCR_PDOWN)
> +        *   bit of the PHY. Bring the PHY up again.
> +        */
> +       genphy_resume(priv->phydev);
> +
> +       phy_start(priv->phydev);
> +
> +       return 0;
> +munreg:
> +       mdiobus_unregister(priv->mdio);
> +ifree:
> +       kfree(priv->mdio->irq);
> +mfree:
> +       mdiobus_free(priv->mdio);
> +       return ret;
> +}
> +
> +static void ax88172a_remove_mdio(struct usbnet *dev)
> +{
> +       struct ax88172a_private *priv =
> +               (struct ax88172a_private *)dev->driver_priv;
> +
> +       phy_stop(priv->phydev);
> +       phy_disconnect(priv->phydev);
> +       mdiobus_unregister(priv->mdio);
> +       kfree(priv->mdio->irq);
> +       mdiobus_free(priv->mdio);
> +}
> +
> +static const struct net_device_ops ax88172a_netdev_ops = {
> +       .ndo_open               = usbnet_open,
> +       .ndo_stop               = usbnet_stop,
> +       .ndo_start_xmit         = usbnet_start_xmit,
> +       .ndo_tx_timeout         = usbnet_tx_timeout,
> +       .ndo_change_mtu         = usbnet_change_mtu,
> +       .ndo_set_mac_address    = asix_set_mac_address,
> +       .ndo_validate_addr      = eth_validate_addr,
> +       .ndo_do_ioctl           = ax88172a_ioctl,
> +       .ndo_set_rx_mode        = asix_set_multicast,
> +};
> +
> +int ax88172a_get_settings(struct net_device *net, struct ethtool_cmd *cmd)
> +{
> +       return phy_ethtool_gset(net->phydev, cmd);
> +}
> +
> +int ax88172a_set_settings(struct net_device *net, struct ethtool_cmd *cmd)
> +{
> +       return phy_ethtool_sset(net->phydev, cmd);
> +}
> +
> +int ax88172a_nway_reset(struct net_device *net)
> +{
> +       return phy_start_aneg(net->phydev);
> +}
> +
> +static const struct ethtool_ops ax88172a_ethtool_ops = {
> +       .get_drvinfo            = asix_get_drvinfo,
> +       .get_link               = usbnet_get_link,
> +       .get_msglevel           = usbnet_get_msglevel,
> +       .set_msglevel           = usbnet_set_msglevel,
> +       .get_wol                = asix_get_wol,
> +       .set_wol                = asix_set_wol,
> +       .get_eeprom_len         = asix_get_eeprom_len,
> +       .get_eeprom             = asix_get_eeprom,
> +       .get_settings           = ax88172a_get_settings,
> +       .set_settings           = ax88172a_set_settings,
> +       .nway_reset             = ax88172a_nway_reset,
> +};
> +
> +static int ax88172a_reset_phy(struct usbnet *dev, int embd_phy)
> +{
> +       int ret;
> +
> +       ret = asix_sw_reset(dev, AX_SWRESET_IPPD);
> +       if (ret < 0)
> +               goto err;
> +
> +       msleep(150);
> +       ret = asix_sw_reset(dev, AX_SWRESET_CLEAR);
> +       if (ret < 0)
> +               goto err;
> +
> +       msleep(150);
> +
> +       ret = asix_sw_reset(dev, embd_phy ? AX_SWRESET_IPRL : AX_SWRESET_IPPD);

(would have to swap things here if adopting my suggestions.)

> +       if (ret < 0)
> +               goto err;
> +
> +       return 0;
> +
> +err:
> +       return ret;
> +}
> +
> +
> +static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
> +{
> +       int ret;
> +       struct asix_data *data = (struct asix_data *)&dev->data;
> +       u8 buf[ETH_ALEN];
> +       struct ax88172a_private *priv;
> +
> +       data->eeprom_len = AX88772_EEPROM_LEN;
> +
> +       usbnet_get_endpoints(dev, intf);
> +
> +       priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +       if (!priv) {
> +               dbg("Could not allocate memory for private data");
> +               return -ENOMEM;
> +       }
> +       dev->driver_priv = priv;
> +
> +       /* Get the MAC address */
> +       ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
> +       if (ret < 0) {
> +               dbg("Failed to read MAC address: %d", ret);
> +               goto free;
> +       }
> +       memcpy(dev->net->dev_addr, buf, ETH_ALEN);
> +
> +       dev->net->netdev_ops = &ax88172a_netdev_ops;
> +       dev->net->ethtool_ops = &ax88172a_ethtool_ops;
> +
> +       /* are we using the internal or the external phy? */
> +       ret = asix_read_cmd(dev, AX_CMD_SW_PHY_STATUS, 0, 0, 1, buf);
> +       if (ret < 0) {
> +               dbg("Failed to read software interface selection register: %d",
> +                   ret);
> +               goto free;
> +       }
> +       dbg("AX_CMD_SW_PHY_STATUS = 0x%02x\n", buf[0]);
> +       switch ((buf[0] & 0x0c) >> 2) {
> +       case 0:
> +               dbg("use internal phy\n");
> +               priv->use_embdphy = 1;
> +               break;
> +       case 1:
> +               dbg("use external phy\n");
> +               priv->use_embdphy = 0;
> +               break;
> +       default:
> +               dbg("Interface mode not supported by driver\n");
> +               goto free;
> +       }

This switch statement inverts the existing logic. Much simpler code would be:
    /* buf[0] & 0xc describes phy interface mode */
    if (buf[0] &  8) {
         dbg("Interface mode not supported by driver\n");
         goto free;
    }
    priv->use_extphy = (buf[0] & 4) >> 2;

> +
> +       priv->phy_addr = asix_read_phy_addr(dev, priv->use_embdphy);
> +       ax88172a_reset_phy(dev, priv->use_embdphy);
> +
> +       /* Asix framing packs multiple eth frames into a 2K usb bulk transfer */
> +       if (dev->driver_info->flags & FLAG_FRAMING_AX) {
> +               /* hard_mtu  is still the default - the device does not support
> +                  jumbo eth frames */
> +               dev->rx_urb_size = 2048;
> +       }
> +
> +       /* init MDIO bus */
> +       ret = ax88172a_init_mdio(dev);
> +       if (ret)
> +               goto free;
> +
> +       return 0;
> +
> +free:
> +       kfree(priv);
> +       return ret;
> +}
> +
> +static void ax88172a_unbind(struct usbnet *dev, struct usb_interface *intf)
> +{
> +       struct ax88172a_private *priv =
> +               (struct ax88172a_private *)dev->driver_priv;
> +
> +       ax88172a_remove_mdio(dev);
> +       kfree(priv);
> +}
> +
> +static int ax88172a_reset(struct usbnet *dev)
> +{
> +       struct asix_data *data = (struct asix_data *)&dev->data;
> +       struct ax88172a_private *priv =
> +               (struct ax88172a_private *)dev->driver_priv;
> +       int ret;
> +       u16 rx_ctl;
> +
> +       ax88172a_reset_phy(dev, priv->use_embdphy);
> +
> +       msleep(150);
> +       rx_ctl = asix_read_rx_ctl(dev);
> +       dbg("RX_CTL is 0x%04x after software reset", rx_ctl);
> +       ret = asix_write_rx_ctl(dev, 0x0000);
> +       if (ret < 0)
> +               goto out;
> +
> +       rx_ctl = asix_read_rx_ctl(dev);
> +       dbg("RX_CTL is 0x%04x setting to 0x0000", rx_ctl);
> +
> +       msleep(150);
> +
> +       ax88172a_nway_reset(dev->net);
> +
> +       ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0,
> +                               AX88772_IPG0_DEFAULT | AX88772_IPG1_DEFAULT,
> +                               AX88772_IPG2_DEFAULT, 0, NULL);
> +       if (ret < 0) {
> +               dbg("Write IPG,IPG1,IPG2 failed: %d", ret);
> +               goto out;
> +       }
> +
> +       /* Rewrite MAC address */
> +       memcpy(data->mac_addr, dev->net->dev_addr, ETH_ALEN);
> +       ret = asix_write_cmd(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN,
> +                                                       data->mac_addr);
> +       if (ret < 0)
> +               goto out;
> +
> +       /* Set RX_CTL to default values with 2k buffer, and enable cactus */
> +       ret = asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL);
> +       if (ret < 0)
> +               goto out;
> +
> +       rx_ctl = asix_read_rx_ctl(dev);
> +       dbg("RX_CTL is 0x%04x after all initializations", rx_ctl);
> +
> +       rx_ctl = asix_read_medium_status(dev);
> +       dbg("Medium Status is 0x%04x after all initializations", rx_ctl);
> +
> +       return 0;
> +
> +out:
> +       return ret;
> +
> +}
> +
> +const struct driver_info ax88172a_info = {
> +       .description = "ASIX AX88172A USB 2.0 Ethernet",
> +       .bind = ax88172a_bind,
> +       .unbind = ax88172a_unbind,
> +       .status = ax88172a_status,
> +       .reset = ax88172a_reset,
> +       .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
> +                FLAG_MULTI_PACKET,
> +       .rx_fixup = asix_rx_fixup,
> +       .tx_fixup = asix_tx_fixup,
> +};
> --
> 1.7.0.4
>

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Christian Riesch]

Grant,

On Fri, Jul 6, 2012 at 11:20 PM, Grant Grundler <grundler@chromium.org> wrote:
> On Fri, Jul 6, 2012 at 4:33 AM, Christian Riesch
> <christian.riesch@omicron.at> wrote:
>> The Asix AX88172A is a USB 2.0 Ethernet interface that supports both an
>> internal PHY as well as an external PHY (connected via MII).
>>
>> This patch adds a driver for the AX88172A and provides support for
>> both modes and supports phylib.
>
> Christian,
> In general this looks fine to me...but I wouldn't know about "bus
> identifier life times" (Ben Hutchings comment).
>
> My nit pick is the declaration and of use_embdphy. An alternative
> coding _suggestion_ below.  I'm not substantially altering the
> functionality.
>
> thanks,
> grant

[...]

>> +
>> +struct ax88172a_private {
>> +       int use_embdphy;
>
> Can you move the "int" to the end of the struct?
> It's cleaner to have fields "natively align". ie pointers should start
> at 8 byte alignments when compiled for 64-bit.
>
>> +       struct mii_bus *mdio;
>> +       struct phy_device *phydev;
>> +       char phy_name[20];
>> +       u16 phy_addr;
>> +       u16 oldmode;
>> +};
>> +

[...]

>> +       /* are we using the internal or the external phy? */
>> +       ret = asix_read_cmd(dev, AX_CMD_SW_PHY_STATUS, 0, 0, 1, buf);
>> +       if (ret < 0) {
>> +               dbg("Failed to read software interface selection register: %d",
>> +                   ret);
>> +               goto free;
>> +       }
>> +       dbg("AX_CMD_SW_PHY_STATUS = 0x%02x\n", buf[0]);
>> +       switch ((buf[0] & 0x0c) >> 2) {
>> +       case 0:
>> +               dbg("use internal phy\n");
>> +               priv->use_embdphy = 1;
>> +               break;
>> +       case 1:
>> +               dbg("use external phy\n");
>> +               priv->use_embdphy = 0;
>> +               break;
>> +       default:
>> +               dbg("Interface mode not supported by driver\n");
>> +               goto free;
>> +       }
>
> This switch statement inverts the existing logic. Much simpler code would be:
>     /* buf[0] & 0xc describes phy interface mode */
>     if (buf[0] &  8) {
>          dbg("Interface mode not supported by driver\n");
>          goto free;
>     }
>     priv->use_extphy = (buf[0] & 4) >> 2;
>

Thank your for your comments! I'll change that in the next version!
Regards, Christian

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Christian Riesch]

Hi Grant,

On Mon, Jul 9, 2012 at 12:22 PM, Christian Riesch
<christian.riesch@omicron.at> wrote:
> Grant,
>
> On Fri, Jul 6, 2012 at 11:20 PM, Grant Grundler <grundler@chromium.org> wrote:
>> On Fri, Jul 6, 2012 at 4:33 AM, Christian Riesch
>> <christian.riesch@omicron.at> wrote:
>>> The Asix AX88172A is a USB 2.0 Ethernet interface that supports both an
>>> internal PHY as well as an external PHY (connected via MII).
>>>
>>> This patch adds a driver for the AX88172A and provides support for
>>> both modes and supports phylib.
>>
>> Christian,
>> In general this looks fine to me...but I wouldn't know about "bus
>> identifier life times" (Ben Hutchings comment).
>>
>> My nit pick is the declaration and of use_embdphy. An alternative
>> coding _suggestion_ below.  I'm not substantially altering the
>> functionality.
>>
>> thanks,
>> grant
>
> [...]
>
>>> +
>>> +struct ax88172a_private {
>>> +       int use_embdphy;
>>
>> Can you move the "int" to the end of the struct?
>> It's cleaner to have fields "natively align". ie pointers should start
>> at 8 byte alignments when compiled for 64-bit.
>>
>>> +       struct mii_bus *mdio;
>>> +       struct phy_device *phydev;
>>> +       char phy_name[20];
>>> +       u16 phy_addr;
>>> +       u16 oldmode;
>>> +};
>>> +
>
> [...]
>
>>> +       /* are we using the internal or the external phy? */
>>> +       ret = asix_read_cmd(dev, AX_CMD_SW_PHY_STATUS, 0, 0, 1, buf);
>>> +       if (ret < 0) {
>>> +               dbg("Failed to read software interface selection register: %d",
>>> +                   ret);
>>> +               goto free;
>>> +       }
>>> +       dbg("AX_CMD_SW_PHY_STATUS = 0x%02x\n", buf[0]);
>>> +       switch ((buf[0] & 0x0c) >> 2) {
>>> +       case 0:
>>> +               dbg("use internal phy\n");
>>> +               priv->use_embdphy = 1;
>>> +               break;
>>> +       case 1:
>>> +               dbg("use external phy\n");
>>> +               priv->use_embdphy = 0;
>>> +               break;
>>> +       default:
>>> +               dbg("Interface mode not supported by driver\n");
>>> +               goto free;
>>> +       }
>>
>> This switch statement inverts the existing logic. Much simpler code would be:
>>     /* buf[0] & 0xc describes phy interface mode */
>>     if (buf[0] &  8) {
>>          dbg("Interface mode not supported by driver\n");
>>          goto free;
>>     }
>>     priv->use_extphy = (buf[0] & 4) >> 2;
>>
>
> Thank your for your comments! I'll change that in the next version!
> Regards, Christian

After rethinking it I decided to keep the switch structure, but use
defines for the different modes and the bitmask. I think this will be
easier to understand. I will submit a new version of the patchset
later today, please have a look at it.
Thanks!
Christian

Re: [PATCH 4/4] asix: Add a new driver for the AX88172A

[Grant Grundler]

On Thu, Jul 12, 2012 at 12:22 AM, Christian Riesch
<christian.riesch@omicron.at> wrote:
> Hi Grant,
...
> After rethinking it I decided to keep the switch structure, but use
> defines for the different modes and the bitmask. I think this will be
> easier to understand. I will submit a new version of the patchset
> later today, please have a look at it.

That's fine too. I agree the new version is easier to read.

thanks!
grant

> Thanks!
> Christian

Re: [PATCH 0/4] Add a driver for the ASIX AX88172A

[Christian Riesch]

On Fri, Jul 6, 2012 at 1:33 PM, Christian Riesch
<christian.riesch@omicron.at> wrote:
> Hi,
>
> this patch adds a driver for the ASIX AX88172A USB 2.0 to 10/100M
> Fast Ethernet Controller.
>
> Although this chip is already supported by the AX88772 code in
> drivers/net/usb/asix.c, I submit a new driver since the existing
> driver lacks an important feature: It only supports an
> Ethernet connection using the internal PHY embedded in the AX88172A.

Sorry, I forgot to mention the features added by the new driver:
- support for an external PHY connected to the AX88172A
- uses phylib

Regards, Christian

RE: [PATCH 0/4] Add a driver for the ASIX AX88172A

[ASIX Allan Email [office]]

Dear All,

Thanks a lot for your great efforts to support AX88172A on Linux kernel mainline source.  

The following information might be useful for you to implement AX88172A Linux driver in Linux kernel mainline source. Most of AX88172A customers' applications will implement their target applications through the MII interface in MAC mode or through the Rev-MII/Rev-RMII in PHY mode, such as AX88172A (MAC mode) + external fiber PHY, AX88172A (PHY mode or Dual-PHY mode) + external MAC controller, etc. ASIX uses AX88172A EEPROM offset 17h (as Software Field setting, the AX88172A drivers will check this field value to identify the exact applications for different driver initialization procedures) to identify different customers' applications. 

For the AX88172A (PHY mode or Dual-PHY mode) + external MAC controller customers' applications that are embedded applications with AX88172A on-board design, customers should have implemented ASIX's AX88172A Linux driver source on their own embedded Linux kernel platform directly. 

For the AX88172A (MAC mode) + external fiber PHY applications that might be USB to Fiber dongle applications, users might plug the USB to Fiber dongle on any Linux kernel platforms. You can consider referring to ASIX's AX88172A Linux driver source that can be downloaded from AX88172A Driver Download web page (http://www.asix.com.tw/download.php?sub=driverdetail&PItemID=96) to support these kinds of known dongle solutions on Linux kernel mainline source if necessary. Please feel free to let us know if you need more information. Thanks a lot.
 

---
Best regards,
Allan Chou
Technical Support Division
ASIX Electronics Corporation
TEL: 886-3-5799500 ext.228
FAX: 886-3-5799558
E-mail: allan@asix.com.tw 
http://www.asix.com.tw/ 

-----Original Message-----
From: Christian Riesch [mailto:christian.riesch@omicron.at] 
Sent: Friday, July 06, 2012 7:33 PM
To: netdev@vger.kernel.org
Cc: Oliver Neukum; Eric Dumazet; Allan Chou; Mark Lord; Grant Grundler; Ming Lei; Michael Riesch; Christian Riesch
Subject: [PATCH 0/4] Add a driver for the ASIX AX88172A

Hi,

this patch adds a driver for the ASIX AX88172A USB 2.0 to 10/100M
Fast Ethernet Controller.

Although this chip is already supported by the AX88772 code in
drivers/net/usb/asix.c, I submit a new driver since the existing
driver lacks an important feature: It only supports an
Ethernet connection using the internal PHY embedded in the AX88172A.

The driver for the AX88172A is based on drivers/net/usb/asix.c
and the work of Michael Riesch <michael@riesch.at>.

The first patch in the patchset fixes checkpatch warnings in asix.c.

The second and the third patch factor out common code which is shared
between the existing drivers and the new driver for the AX88172A.

The fourth patch finally adds support for the AX88172A.

The patchset applies on top of net-next.

I have a few questions:

1) Is it ok to factor out the common code like I did? Or should
   it go into a separate kernel module?

2) phylib/usbnet: Currently I have an empty .status function
   in my const struct driver_info ax88172a_info. I think this
   notifies me of a link change, right? I don't know
   what I should do in this function. Trigger the phy state machine
   like a phy interrupt would do, since link changes are handled
   by the phy state machine?

I have tested the patch with the ASIX AX88172A demo board (using the
internal PHY) and a custom board (AX88172A and National DP83640 PHY).

I am looking forward to your comments. Since this is my first submission
of a larger patchset to a kernel mailing list, I would like to thank you
in advance for your patience :-) The patch is in an early state and
certainly needs improvement!

Regards, Christian

Christian Riesch (4):
  asix: Fix checkpatch warnings
  asix: Rename asix.c to asix_devices.c
  asix: Factor out common code
  asix: Add a new driver for the AX88172A

 drivers/net/usb/Makefile       |    1 +
 drivers/net/usb/asix.c         | 1660 ----------------------------------------
 drivers/net/usb/asix.h         |  211 +++++
 drivers/net/usb/asix_common.c  |  525 +++++++++++++
 drivers/net/usb/asix_devices.c | 1033 +++++++++++++++++++++++++
 drivers/net/usb/ax88172a.c     |  407 ++++++++++
 6 files changed, 2177 insertions(+), 1660 deletions(-)
 delete mode 100644 drivers/net/usb/asix.c
 create mode 100644 drivers/net/usb/asix.h
 create mode 100644 drivers/net/usb/asix_common.c
 create mode 100644 drivers/net/usb/asix_devices.c
 create mode 100644 drivers/net/usb/ax88172a.c

Re: [PATCH 0/4] Add a driver for the ASIX AX88172A

[Christian Riesch]

Allan,
Thank you for your email and your support! I would be happy if you
would have a look
at the patches, comment on them and test them :-)

On Mon, Jul 9, 2012 at 4:38 AM, ASIX Allan Email [office]
<allan@asix.com.tw> wrote:
> Dear All,
>
> Thanks a lot for your great efforts to support AX88172A on Linux kernel mainline source.
>
> The following information might be useful for you to implement AX88172A Linux driver in
> Linux kernel mainline source. Most of AX88172A customers' applications will
> implement their target applications through the MII interface in MAC mode or through
> the Rev-MII/Rev-RMII in PHY mode, such as AX88172A (MAC mode) + external fiber
> PHY, AX88172A (PHY mode or Dual-PHY mode) + external MAC controller, etc. ASIX
> uses AX88172A EEPROM offset 17h (as Software Field setting, the AX88172A drivers
> will check this field value to identify the exact applications for different driver initialization
> procedures) to identify different customers' applications.
>
> For the AX88172A (PHY mode or Dual-PHY mode) + external MAC controller
> customers' applications that are embedded applications with AX88172A on-board design,
> customers should have implemented ASIX's AX88172A Linux driver source on their own
> embedded Linux kernel platform directly.
>
> For the AX88172A (MAC mode) + external fiber PHY applications that might be USB
> to Fiber dongle applications, users might plug the USB to Fiber dongle on any Linux
> kernel platforms. You can consider referring to ASIX's AX88172A Linux driver source
> that can be downloaded from AX88172A Driver Download web page
> (http://www.asix.com.tw/download.php?sub=driverdetail&PItemID=96) to support
> these kinds of known dongle solutions on Linux kernel mainline source if necessary.
> Please feel free to let us know if you need more information. Thanks a lot.

Currently I check the state of bits 3:2 (SS[1:0]) of the Software
Interface Selection Status Register (21h) of the AX88172A (see
ax88172a_bind in [1]). If these bits are set to 00 my driver will use
the internal PHY, if they are 01 my driver uses an external PHY. So my
board uses the pull-up/down resistor configuration at pin #59 and pin
#60 to select the operation mode.

If I understand correctly you suggest to use the settings in the
EEPROM at offset 17h instead. Could you please provide more
information about this EEPROM settings? The EEPROM memory map in the
AX88172A documentation ends at offset 14h. Is there a reason why I
should not use the hardware configuration with pins #59 and #60?

Best regards, Christian

[1] http://marc.info/?l=linux-netdev&m=134157545200930&w=2

>
>
> ---
> Best regards,
> Allan Chou
> Technical Support Division
> ASIX Electronics Corporation
> TEL: 886-3-5799500 ext.228
> FAX: 886-3-5799558
> E-mail: allan@asix.com.tw
> http://www.asix.com.tw/

Re: [PATCH 0/4] Add a driver for the ASIX AX88172A

[Grant Grundler]

Christian,
Here's my $0.02 response to your questions.

On Fri, Jul 6, 2012 at 4:33 AM, Christian Riesch
<christian.riesch@omicron.at> wrote:
...
> I have a few questions:
>
> 1) Is it ok to factor out the common code like I did? Or should
>    it go into a separate kernel module?

I think it's ok. I'd rather not see additional kernel modules unless
the driver is substantially different.


> 2) phylib/usbnet: Currently I have an empty .status function
>    in my const struct driver_info ax88172a_info. I think this
>    notifies me of a link change, right? I don't know
>    what I should do in this function. Trigger the phy state machine
>    like a phy interrupt would do, since link changes are handled
>    by the phy state machine?

I don't see any Documentation for this entry point. My reading of the
code is driver_info->status (include/linux/usb/usbnet.h) is to poll
current link status from URBs. See  usbnet_probe()  (calls
init_status()) and intr_complete() (calls your stub).  I also looked
at usbnet_cdc_status() in cdc_ethernet.c. Maybe other drivers are
better examples.

hth,
grant

Re: [PATCH 0/4] Add a driver for the ASIX AX88172A

[Mark Lord]

On 12-07-09 01:45 PM, Grant Grundler wrote:
> Christian,
> Here's my $0.02 response to your questions.
> 
> On Fri, Jul 6, 2012 at 4:33 AM, Christian Riesch
> <christian.riesch@omicron.at> wrote:
> ...
>> I have a few questions:
>>
>> 1) Is it ok to factor out the common code like I did? Or should
>>    it go into a separate kernel module?
> 
> I think it's ok. I'd rather not see additional kernel modules unless
> the driver is substantially different.

I'll second that.  Ideally, somebody should pick up the pieces
from my aborted efforts last fall, and just get the real ASIX driver
itself tidied and into the kernel.  Then *everything* would work.

But I doubt that would be feasible at this point.

Cheers

RE: [PATCH 0/4] Add a driver for the ASIX AX88172A

[ASIX Allan Email [office]]

Dear All,

>From ASIX support viewpoint, it might be hard to support all AX88172A target applications on Linux kernel native ax88172a.c driver because some of AX88172A applications are embedded on customers' special target applications such as the AX88172A (PHY mode or Dual-PHY mode) + external MAC controller on-board design applications. For these kinds of AX88172A applications, the AX88172A Linux driver was qualified in our customers' site directly. It means ASIX doesn't have those customers' AX88172A devices in our site for testing. 

But for some AX88172A target applications such as AX88172A + external Fiber PHY and AX88172A + external National DP83640 PHY (Christian is testing under), etc. USB dongle applications, it is possible to support them on Linux kernel native ax88172a.c driver but *** the readme of this driver source might need to indicate clearly what kinds of AX88172A devices had been verified on this ax88172a.c native driver source ***. It will avoid users were confusing why their AX88172A devices couldn't work fine on Linux kernel native driver in future. 

Please let us know if you need more information. Thanks a lot. 


---
Best regards,
Allan Chou
Technical Support Division
ASIX Electronics Corporation
TEL: 886-3-5799500 ext.228
FAX: 886-3-5799558
E-mail: allan@asix.com.tw 
http://www.asix.com.tw/ 

-----Original Message-----
From: Mark Lord [mailto:kernel@teksavvy.com] 
Sent: Tuesday, July 10, 2012 6:27 AM
To: Grant Grundler
Cc: Christian Riesch; netdev@vger.kernel.org; Oliver Neukum; Eric Dumazet; Allan Chou; Ming Lei; Michael Riesch
Subject: Re: [PATCH 0/4] Add a driver for the ASIX AX88172A

On 12-07-09 01:45 PM, Grant Grundler wrote:
> Christian,
> Here's my $0.02 response to your questions.
> 
> On Fri, Jul 6, 2012 at 4:33 AM, Christian Riesch
> <christian.riesch@omicron.at> wrote:
> ...
>> I have a few questions:
>>
>> 1) Is it ok to factor out the common code like I did? Or should
>>    it go into a separate kernel module?
> 
> I think it's ok. I'd rather not see additional kernel modules unless
> the driver is substantially different.

I'll second that.  Ideally, somebody should pick up the pieces
from my aborted efforts last fall, and just get the real ASIX driver
itself tidied and into the kernel.  Then *everything* would work.

But I doubt that would be feasible at this point.

Cheers