[U-Boot] [PATCH v2 1/2] usb: eth: add ASIX AX88179 DRIVER

[Marek Vasut]

On Wednesday, September 03, 2014 at 04:31:20 PM, Rene Griessl wrote:
> changes in v2:
> 	-added usb_ether.h to change list
> 	-added 2nd patch to enable driver for arndale board
> 
> Signed-off-by: Rene Griessl <rgriessl at cit-ec.uni-bielefeld.de>

I see that in Linux, there is asix_common.c stuff. Can this driver share any 
parts with drivers/net/ax88* ?

> ---
>  drivers/usb/eth/Makefile    |   3 +
>  drivers/usb/eth/asix88179.c | 641
> ++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/eth/usb_ether.c |
>   7 +
>  include/usb_ether.h         |   6 +
>  4 files changed, 657 insertions(+)
>  create mode 100644 drivers/usb/eth/asix88179.c
> 
> diff --git a/drivers/usb/eth/Makefile b/drivers/usb/eth/Makefile
> index 94551c4..fad4acd 100644
> --- a/drivers/usb/eth/Makefile
> +++ b/drivers/usb/eth/Makefile
> @@ -8,5 +8,8 @@ obj-$(CONFIG_USB_HOST_ETHER) += usb_ether.o
>  ifdef CONFIG_USB_ETHER_ASIX
>  obj-y += asix.o
>  endif
> +ifdef CONFIG_USB_ETHER_ASIX_88179
> +obj-y += asix88179.o
> +endif

This should be obj-$(CONFIG....) as seen below. Fix the asix one in a separate 
patch please.

[...]

> +/* 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			((u32)BIT(16))

The u32 cast is not needed. Also, please drop the BIT() macro, it's just 
obfuscating the code, just use (1 << 16) instead. Please fix globally.

> +#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			((u32)BIT(29))
> +#define AX_RXHDR_DROP_ERR			((u32)BIT(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

[...]

> +static inline int asix_get_phy_addr(struct ueth_data *dev)
> +{
> +	ALLOC_CACHE_ALIGN_BUFFER(u8, buf, 2);
> +	int ret = -1;
> +	if (dev->pusb_dev->descriptor.idProduct == 0x1790) {
> +		ret = asix_read_cmd(dev, AX_ACCESS_MAC, 0x10, 6, 6, buf);
> +		debug("asix_get_phy_addr() returning 
0x%02x%02x%02x%02x%02x%02x\n",
> +		      buf[0], buf[1], buf[2], buf[3], buf[4], buf[5]);
> +		}
> +	else {
> +		}

Uh, this check needs some rework, right ? Also, you want to lint your patches 
with ./scripts/checkpatch.pl tool before resubmitting.

> +	return ret;
> +}
> +
> +
> +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, 0x10, 6, 6, buf);
> +		debug("asix_read_mac() returning 0x%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);

Why does the buffer need to be aligned here ? It's just a buffer that is not 
used for DMA, no ?

> +	u16 *tmp16;

Is it because you were getting unaligned accesses , since when the buffer itself 
was not aligned and you did cast it to u16, the CPU triggered unaligned access ?

> +	u8 *tmp;
[...]