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

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. [...]