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

2014-09-11 Thread René Griessl


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

2014-09-11 Thread Marek Vasut
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

2014-09-10 Thread 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 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

2014-09-10 Thread 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 ?

[...]

  +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

2014-09-09 Thread 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* ?

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

2014-09-03 Thread Rene Griessl
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