[U-Boot] [PATCH V3 2/3] usb: eth: add ASIX AX88179 DRIVER

[Marek Vasut <marex@denx.de>]

On Friday, September 26, 2014 at 10:08:48 AM, Rene Griessl wrote:
> changes in v3:
> 	-added all compatible devices from linux driver
> 	-fixed issues from review
> 
> changes in v2:
>         -added usb_ether.h to change list
>         -added 2nd patch to enable driver for arndale board

The changelog goes to the [*] marker below. And you're missing a meaningful 
commit message too. Also, if the driver is pulled from Linux, please specify
from which commit in there, so future developers might re-sync the driver if 
needed be and they'd know from which point the driver was derived.

> Signed-off-by: Rene Griessl <rgriessl@cit-ec.uni-bielefeld.de>
> ---
>  drivers/usb/eth/Makefile    |   1 +
>  drivers/usb/eth/asix88179.c | 659
> ++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/eth/usb_ether.c |
>   7 +
>  include/usb_ether.h         |   6 +
>  4 files changed, 673 insertions(+)
>  create mode 100644 drivers/usb/eth/asix88179.c
> 
> diff --git a/drivers/usb/eth/Makefile b/drivers/usb/eth/Makefile
> index e6ae9f1..c92d2b0 100644
> --- a/drivers/usb/eth/Makefile
> +++ b/drivers/usb/eth/Makefile
> @@ -6,5 +6,6 @@
>  # new USB host ethernet layer dependencies
>  obj-$(CONFIG_USB_HOST_ETHER) += usb_ether.o
>  obj-$(CONFIG_USB_ETHER_ASIX) += asix.o
> +obj-$(CONFIG_USB_ETHER_ASIX88179) += asix88179.o
>  obj-$(CONFIG_USB_ETHER_MCS7830) += mcs7830.o
>  obj-$(CONFIG_USB_ETHER_SMSC95XX) += smsc95xx.o
> diff --git a/drivers/usb/eth/asix88179.c b/drivers/usb/eth/asix88179.c
> new file mode 100644
> index 0000000..2079056
> --- /dev/null
> +++ b/drivers/usb/eth/asix88179.c
> @@ -0,0 +1,659 @@
> +/*
> + * Copyright (c) 2014 Rene Griessl <rgriessl@cit-ec.uni-bielefeld.de>
> + * based on the U-Boot Asix driver as well as information
> + * from the Linux AX88179_178a driver
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +

One newline too many here.

> +#include <common.h>
> +#include <usb.h>
> +#include <net.h>
> +#include <linux/mii.h>
> +#include "usb_ether.h"
> +#include <malloc.h>
> +
> +

DTTO

> +/* ASIX AX88179 based USB 3.0 Ethernet Devices */
> +#define AX88179_PHY_ID				0x03
> +#define AX_EEPROM_LEN				0x100
> +#define AX88179_EEPROM_MAGIC			0x17900b95
> +#define AX_MCAST_FLTSIZE			8
> +#define AX_MAX_MCAST				64
> +#define AX_INT_PPLS_LINK			(1 << 16)
> +#define AX_RXHDR_L4_TYPE_MASK			0x1c
> +#define AX_RXHDR_L4_TYPE_UDP			4
> +#define AX_RXHDR_L4_TYPE_TCP			16
> +#define AX_RXHDR_L3CSUM_ERR			2
> +#define AX_RXHDR_L4CSUM_ERR			1
> +#define AX_RXHDR_CRC_ERR			(1 << 29)
> +#define AX_RXHDR_DROP_ERR			(1 << 31)
> +#define AX_ACCESS_MAC				0x01
> +#define AX_ACCESS_PHY				0x02
> +#define AX_ACCESS_EEPROM			0x04
> +#define AX_ACCESS_EFUS				0x05
> +#define AX_PAUSE_WATERLVL_HIGH			0x54
> +#define AX_PAUSE_WATERLVL_LOW			0x55
> +
> +#define PHYSICAL_LINK_STATUS			0x02
> +	#define	AX_USB_SS		0x04
> +	#define	AX_USB_HS		0x02
> +
> +#define GENERAL_STATUS				0x03
> +/* Check AX88179 version. UA1:Bit2 = 0,  UA2:Bit2 = 1 */
> +	#define	AX_SECLD		0x04
> +
> +#define AX_SROM_ADDR				0x07
> +#define AX_SROM_CMD				0x0a
> +	#define EEP_RD			0x04
> +	#define EEP_BUSY		0x10

If those are bits, then just use (1 << n) notation.
[...]

> +static int curr_eth_dev; /* index for name of next device detected */
> +
> +/* driver private */
> +struct asix_private {
> +	int flags;
> +};

This thing is a little iffy ... do you really need a full-blown struct here or 
can you just use array ?

> +/*
> + * Asix infrastructure commands
> + */
> +static int asix_write_cmd(struct ueth_data *dev, u8 cmd, u16 value, u16
> index, +			     u16 size, void *data)
> +{
> +	int len;
> +
> +	debug("asix_write_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
> +	      cmd, value, index, size);
> +
> +	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, size);

I think if you really enable the debug to generate a printf() , the compiler 
will spew that you wrote code before variable declaration.

> +	memcpy(buf, data, size);
> +
> +	len = usb_control_msg(
> +		dev->pusb_dev,
> +		usb_sndctrlpipe(dev->pusb_dev, 0),
> +		cmd,
> +		USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> +		value,
> +		index,
> +		buf,
> +		size,
> +		USB_CTRL_SET_TIMEOUT);
> +
> +	return len == size ? 0 : -1;

Use values from errno.h here ?

> +}
> +
> +static int asix_read_cmd(struct ueth_data *dev, u8 cmd, u16 value, u16
> index, +			    u16 size, void *data)
> +{
> +	int len;
> +
> +	debug("asix_read_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
> +	      cmd, value, index, size);
> +	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, size);
> +
> +
> +	len = usb_control_msg(
> +		dev->pusb_dev,
> +		usb_rcvctrlpipe(dev->pusb_dev, 0),
> +		cmd,
> +		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> +		value,
> +		index,
> +		buf,
> +		size,
> +		USB_CTRL_GET_TIMEOUT);
> +
> +	memcpy(data, buf, size);
> +
> +	return len == size ? 0 : -1;
> +}
> +
> +
> +static int asix_read_mac(struct eth_device *eth)
> +{
> +	struct ueth_data *dev = (struct ueth_data *)eth->priv;
> +	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, ETH_ALEN);
> +
> +	if (dev->pusb_dev->descriptor.idProduct == 0x1790) {
> +		asix_read_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, 6, 6, buf);
> +		debug("asix_read_mac() returning %02x:%02x:%02x:%02x:%02x:
%02x\n",
> +		      buf[0], buf[1], buf[2], buf[3], buf[4], buf[5]);
> +		memcpy(eth->enetaddr, buf, ETH_ALEN);
> +	}
> +	return 0;
> +}
> +
> +
> +
> +static int asix_basic_reset(struct ueth_data *dev)
> +{
> +	ALLOC_CACHE_ALIGN_BUFFER(u8, buf, 6);
> +	u16 *tmp16;
> +	u8 *tmp;
> +
> +	tmp16 = (u16 *)buf;
> +	tmp = (u8 *)buf;
> +
> +	/* Power up ethernet PHY */
> +	*tmp16 = 0;
> +	asix_write_cmd(dev, AX_ACCESS_MAC, AX_PHYPWR_RSTCTL, 2, 2, tmp16);

The asix_write_cmd() has some bounce-buffer logic in it already, does the tmp16 
need to be aligned here too? Also, you might want to use include/bouncebuf.h and 
friends instead of implementing your own bounce buffering.

[...]

> +/*
> + * Asix callbacks
> + */
> +static int asix_init(struct eth_device *eth, bd_t *bd)
> +{
> +	struct ueth_data	*dev = (struct ueth_data *)eth->priv;
> +	int timeout = 0;
> +	int link_detected;
> +
> +	ALLOC_CACHE_ALIGN_BUFFER(u8, buf, 6);
> +	u16 *tmp16;
> +
> +	tmp16 = (u16 *)buf;
> +
> +	debug("** %s()\n", __func__);
> +
> +
> +	/* Configure RX control register => start operation */
> +	*tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_IPE | AX_RX_CTL_START |
> +		 AX_RX_CTL_AP | AX_RX_CTL_AMALL | AX_RX_CTL_AB;
> +	if (asix_write_cmd(dev, AX_ACCESS_MAC, AX_RX_CTL, 2, 2, tmp16) < 0)
> +		goto out_err;
> +
> +	do {
> +		asix_read_cmd(dev, AX_ACCESS_PHY, 0x03, MII_BMSR, 2, buf);
> +		link_detected = *tmp16 & BMSR_LSTATUS;
> +		if (!link_detected) {
> +			if (timeout == 0)
> +				printf("Waiting for Ethernet connection... ");
> +			udelay(TIMEOUT_RESOLUTION * 1000);

mdelay()

> +			timeout += TIMEOUT_RESOLUTION;
> +		}
> +	} while (!link_detected && timeout < PHY_CONNECT_TIMEOUT);

Newline

> +	if (link_detected) {
> +		if (timeout != 0)
> +			printf("done.\n");
> +			return 0;

Where does this return 0; belong to ?

> +	} else {/*reset device and try again*/
> +		printf("unable to connect.\n");
> +		printf("Reset Ethernet Device\n");
> +		asix_basic_reset(dev);
> +		timeout = 0;
> +		do {
> +			asix_read_cmd(dev, AX_ACCESS_PHY, 0x03,
> +				      MII_BMSR, 2, buf);
> +			link_detected = *tmp16 & BMSR_LSTATUS;
> +			if (!link_detected) {
> +				if (timeout == 0)
> +					printf("Waiting for Ethernet 
connection... ");
> +				udelay(TIMEOUT_RESOLUTION * 1000);

mdelay()

> +				timeout += TIMEOUT_RESOLUTION;
> +			}
> +		} while (!link_detected && timeout < PHY_CONNECT_TIMEOUT);
> +		if (link_detected) {
> +			if (timeout != 0)
> +				printf("done.\n");
> +				return 0;
> +			} else {
> +				printf("unable to connect.\n");
> +				goto out_err;
> +				}

The indent is crazy in here ...

> +	}
> +
> +	return 0;
> +out_err:
> +	return -1;
> +}
> +
> +static int asix_send(struct eth_device *eth, void *packet, int length)
> +{
> +	struct ueth_data *dev = (struct ueth_data *)eth->priv;
> +	int err;
> +	u32 packet_len, tx_hdr2;
> +	int actual_len;
> +	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, msg,
> +				 PKTSIZE + (2 * sizeof(packet_len)));
> +
> +	debug("** %s(), len %d\n", __func__, length);
> +
> +	packet_len = length;
> +	cpu_to_le32s(&packet_len);
> +
> +	memcpy(msg, &packet_len, sizeof(packet_len));
> +
> +	tx_hdr2 = 0;
> +	if (((length + 8) % 0x200 /*frame_size*/) == 0)

Define the frame size as a named constant, then use it here.

> +		tx_hdr2 |= 0x80008000;	/* Enable padding */
> +
> +	cpu_to_le32s(&tx_hdr2);
> +
> +	memcpy(msg + sizeof(packet_len), &tx_hdr2, sizeof(tx_hdr2));
> +
> +	memcpy(msg + sizeof(packet_len) + sizeof(tx_hdr2),
> +	       (void *)packet, length);
> +
> +	err = usb_bulk_msg(dev->pusb_dev,
> +				usb_sndbulkpipe(dev->pusb_dev, dev->ep_out),
> +				(void *)msg,
> +				length + sizeof(packet_len) + sizeof(tx_hdr2),
> +				&actual_len,
> +				USB_BULK_SEND_TIMEOUT);
> +	debug("Tx: len = %u, actual = %u, err = %d\n",
> +	      length + sizeof(packet_len), actual_len, err);
> +
> +	return err;
> +}

[...]

> +/* Probe to see if a new device is actually an asix device */
> +int ax88179_eth_probe(struct usb_device *dev, unsigned int ifnum,
> +		      struct ueth_data *ss)
> +{
> +	struct usb_interface *iface;
> +	struct usb_interface_descriptor *iface_desc;
> +	int ep_in_found = 0, ep_out_found = 0;
> +	int i;
> +
> +	/* let's examine the device now */
> +	iface = &dev->config.if_desc[ifnum];
> +	iface_desc = &dev->config.if_desc[ifnum].desc;
> +
> +	for (i = 0; asix_dongles[i].vendor != 0; i++) {
> +		if (dev->descriptor.idVendor == asix_dongles[i].vendor &&
> +		    dev->descriptor.idProduct == asix_dongles[i].product)
> +			/* Found a supported dongle */
> +			break;
> +	}
> +
> +	if (asix_dongles[i].vendor == 0)
> +		return 0;
> +
> +	memset(ss, 0, sizeof(struct ueth_data));
> +
> +	/* At this point, we know we've got a live one */
> +	debug("\n\nUSB Ethernet device detected: %#04x:%#04x\n",
> +	      dev->descriptor.idVendor, dev->descriptor.idProduct);
> +
> +	/* Initialize the ueth_data structure with some useful info */
> +	ss->ifnum = ifnum;
> +	ss->pusb_dev = dev;
> +	ss->subclass = iface_desc->bInterfaceSubClass;
> +	ss->protocol = iface_desc->bInterfaceProtocol;
> +
> +	/* alloc driver private */
> +	ss->dev_priv = calloc(1, sizeof(struct asix_private));
> +	if (!ss->dev_priv)
> +		return 0;
> +
> +	((struct asix_private *)ss->dev_priv)->flags = asix_dongles[i].flags;
> +
> +	/*
> +	 * We are expecting a minimum of 3 endpoints - in, out (bulk), and
> +	 * int. We will ignore any others.
> +	 */
> +	for (i = 0; i < iface_desc->bNumEndpoints; i++) {
> +		/* is it an BULK endpoint? */
> +		if ((iface->ep_desc[i].bmAttributes &
> +		     USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) {

if (! ...)
 continue;

if ((ep_addr & USB_DIR_IN) && !ep_in_found) {
 something
}

if (!(ep_addr & USB_DIR_IN) && !ep_out_found) {
 something
}
> +			u8 ep_addr = iface->ep_desc[i].bEndpointAddress;
> +			if (ep_addr & USB_DIR_IN) {
> +				if (!ep_in_found) {
> +					ss->ep_in = ep_addr &
> +						USB_ENDPOINT_NUMBER_MASK;
> +					ep_in_found = 1;
> +				}
> +			} else {
> +				if (!ep_out_found) {
> +					ss->ep_out = ep_addr &
> +						USB_ENDPOINT_NUMBER_MASK;
> +					ep_out_found = 1;
> +				}
> +			}

See above how to trim down the indent here.
[...]