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

René Griessl rgriessl at cit-ec.uni-bielefeld.de
Fri Sep 26 11:35:02 CEST 2014


>> 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.
Were exactly can I find the marker?
>> Signed-off-by: Rene Griessl <rgriessl at 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 at 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 ?
This struct is used to cast the flags to the U-Boot ueth driver. (see 
line 589 of c-file)
>> +/*
>> + * 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.
Really? I took all of the variables from the function call. So with one 
has not 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.
I will do some rework here...
> [...]
>
>> +/*
>> + * 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 ?
it quits the init function, because a link is detected. Is it more 
likely to put a goto here?
>> +	} 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 ...
I will put the link detect routine in a separate function.
>> +	}
>> +
>> +	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.
> [...]

-- 

--------------------------------------------------------

Dipl.-Ing. René Griessl
Universität Bielefeld
AG Kognitronik & Sensorik
Exzellenzcluster Cognitive Interaction Technology (CITEC)
Inspiration 1 (Zehlendorfer Damm 199)
33615 Bielefeld

Telefon : +49 521-106-67362
Fax     : +49 521-106-12348
eMail   : rgriessl at cit-ec.uni-bielefeld.de
Internet: http://www.ks.cit-ec.uni-bielefeld.de/



More information about the U-Boot mailing list