Re: [U-Boot] [PATCH v2 1/2] usb: eth: add ASIX AX88179 DRIVER
Am 10.09.2014 17:00, schrieb Marek Vasut: On Wednesday, September 10, 2014 at 12:00:29 PM, René Griessl wrote: 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 rgrie...@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 I didn't look deep enough. The 88179 driver is a completely separate driver, not sharing any code what-so-ever with the other ASIX driver, yes ? The USB read and write cmd functions are similar and the probe function is the same. So they are sharing 6% of code. What do you have in mind? Do you want to build one driver supporting all devices? [...] + 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 This is rather surprising, but you're right. Please fix this up, the else can be dropped and the trailing } can be indented just under the if () clause. OK + 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); + } Same here. + 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? Oh I see. The smart solution would be to add bounce buffer into the USB stack, but unless you want to dive into this, this solution would be OK here. OK, then I will leave it this way. -- 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 : rgrie...@cit-ec.uni-bielefeld.de Internet: http://www.ks.cit-ec.uni-bielefeld.de/ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] usb: eth: add ASIX AX88179 DRIVER
On Thursday, September 11, 2014 at 10:33:57 AM, René Griessl wrote: [...] I didn't look deep enough. The 88179 driver is a completely separate driver, not sharing any code what-so-ever with the other ASIX driver, yes ? The USB read and write cmd functions are similar and the probe function is the same. So they are sharing 6% of code. What do you have in mind? Do you want to build one driver supporting all devices? Yes, but if that's not easily doable, just like you point out, then I am fine with two drivers. Thank you! [..] Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] usb: eth: add ASIX AX88179 DRIVER
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 rgrie...@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_ERR2 +#define AX_RXHDR_L4CSUM_ERR1 +#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; [...] ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] usb: eth: add ASIX AX88179 DRIVER
On Wednesday, September 10, 2014 at 12:00:29 PM, René Griessl wrote: 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 rgrie...@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 I didn't look deep enough. The 88179 driver is a completely separate driver, not sharing any code what-so-ever with the other ASIX driver, yes ? [...] +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 This is rather surprising, but you're right. Please fix this up, the else can be dropped and the trailing } can be indented just under the if () clause. + 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); + } Same here. + 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? Oh I see. The smart solution would be to add bounce buffer into the USB stack, but unless you want to dive into this, this solution would be OK here. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] usb: eth: add ASIX AX88179 DRIVER
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 rgrie...@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_LEN0x100 +#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_MASK0x1c +#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_MAC0x01 +#define AX_ACCESS_PHY0x02 +#define AX_ACCESS_EEPROM 0x04 +#define AX_ACCESS_EFUS 0x05 +#define AX_PAUSE_WATERLVL_HIGH 0x54 +#define AX_PAUSE_WATERLVL_LOW0x55 [...] +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; [...] ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 1/2] usb: eth: add ASIX AX88179 DRIVER
changes in v2: -added usb_ether.h to change list -added 2nd patch to enable driver for arndale board Signed-off-by: Rene Griessl rgrie...@cit-ec.uni-bielefeld.de --- 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 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 000..55cc48b --- /dev/null +++ b/drivers/usb/eth/asix88179.c @@ -0,0 +1,641 @@ +/* + * Copyright (c) 2014 Rene Griessl rgrie...@cit-ec.uni-bielefeld.de + * based on the U-Boot Asix driver as well as information + * from the Linux AX88179 driver + * + * SPDX-License-Identifier:GPL-2.0+ + */ + + +#include common.h +#include usb.h +#include net.h +#include linux/mii.h +#include usb_ether.h +#include malloc.h + + +/* 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)) +#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_ERR2 +#define AX_RXHDR_L4CSUM_ERR1 +#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 + +#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_SECLD0x04 + +#define AX_SROM_ADDR 0x07 +#define AX_SROM_CMD0x0a + #define EEP_RD 0x04 + #define EEP_BUSY0x10 + +#define AX_SROM_DATA_LOW 0x08 +#define AX_SROM_DATA_HIGH 0x09 + +#define AX_RX_CTL 0x0b + #define AX_RX_CTL_DROPCRCERR0x0100 + #define AX_RX_CTL_IPE 0x0200 + #define AX_RX_CTL_START 0x0080 + #define AX_RX_CTL_AP0x0020 + #define AX_RX_CTL_AM0x0010 + #define AX_RX_CTL_AB0x0008 + #define AX_RX_CTL_AMALL 0x0002 + #define AX_RX_CTL_PRO 0x0001 + #define AX_RX_CTL_STOP 0x + +#define AX_NODE_ID 0x10 +#define AX_MULFLTARY 0x16 + +#define AX_MEDIUM_STATUS_MODE 0x22 + #define AX_MEDIUM_GIGAMODE 0x01 + #define AX_MEDIUM_FULL_DUPLEX 0x02 + #define AX_MEDIUM_EN_125MHZ 0x08 + #define AX_MEDIUM_RXFLOW_CTRLEN 0x10 + #define AX_MEDIUM_TXFLOW_CTRLEN 0x20 + #define AX_MEDIUM_RECEIVE_EN0x100 + #define AX_MEDIUM_PS0x200 + #define AX_MEDIUM_JUMBO_EN 0x8040 + +#define AX_MONITOR_MOD 0x24 + #define AX_MONITOR_MODE_RWLC0x02 + #define AX_MONITOR_MODE_RWMP0x04 + #define AX_MONITOR_MODE_PMEPOL 0x20 + #define AX_MONITOR_MODE_PMETYPE 0x40 + +#define AX_GPIO_CTRL 0x25 + #define AX_GPIO_CTRL_GPIO3EN0x80 + #define AX_GPIO_CTRL_GPIO2EN0x40 + #define AX_GPIO_CTRL_GPIO1EN0x20 + +#define AX_PHYPWR_RSTCTL 0x26 + #define AX_PHYPWR_RSTCTL_BZ 0x0010 + #define AX_PHYPWR_RSTCTL_IPRL 0x0020 + #define AX_PHYPWR_RSTCTL_AT 0x1000 + +#define AX_RX_BULKIN_QCTRL 0x2e +#define AX_CLK_SELECT 0x33 + #define AX_CLK_SELECT_BCS 0x01 + #define AX_CLK_SELECT_ACS 0x02 + #define AX_CLK_SELECT_ULR 0x08 + +#define AX_RXCOE_CTL