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

[René Griessl]

Am 09.09.2014 16:34, schrieb 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* ?

The asix_common.c includes asix.h. There you see that the common driver 
supports following devices:
AX88172
AX88772
AX88178
but it is not supporting AX88179 and AX88178a, they have a separate 
driver called ax88179_178a.c
These two have a different style in accessing MAC registers and PHY

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

OK

>> +/* 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.

OK (was just copy'n'paste from the linux driver)

>> +#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.

was OK for ./scripts/checkpatch.pl
but I can change that

>> +	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 ?

Thats right, if I do not align I get unaligned accesses during USB 
communication.
Is there a smarter solution for that?

>> +	u8 *tmp;
> [...]