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

René Griessl
2014-09-26T09:35:02Z

>> 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@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 ?
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/