[U-Boot] [PATCH 0/6] net: ASIX AX88772B enablement

[U-Boot] [PATCH 0/6] net: ASIX AX88772B enablement

[Lucas Stach]

Hi all,

This series supersedes the earlier posted patches to support the
AX88772B chip. I've split this up into a series as what started
as a small patch to get the chip working turned into a journey
through the usbeth stack and now not only reworks a lot of the
ASIX code, but also touches other parts of the stack.

I want to thank Marek, Joe and Mike for their review, valueable
feedback and guidance.

I've verified that we are now actually able to override the MAC
address from the environment. All review feedback so far has been
incorporated.

Lucas Stach (6):
  net: introduce transparent driver private in ueth_data
  net: usbeth: remove misleading warning
  net: asix: split out basic reset function
  net: asix: add write_hwaddr function
  net: asix: add read_mac function
  net: asix: add AX88772B support

 drivers/usb/eth/asix.c     | 180 ++++++++++++++++++++++++++++++++-------------
 drivers/usb/eth/smsc95xx.c |  48 ++++++++----
 include/usb_ether.h        |   8 +-
 net/eth.c                  |   2 -
 4 Dateien ge?ndert, 165 Zeilen hinzugef?gt(+), 73 Zeilen entfernt(-)

-- 
1.7.11.4

[U-Boot] [PATCH 1/6] net: introduce transparent driver private in ueth_data

[Lucas Stach]

Avoid clutter in ueth_data. Individual drivers should not mess
with structures belonging to the core like this.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/usb/eth/smsc95xx.c | 48 ++++++++++++++++++++++++++++++++--------------
 include/usb_ether.h        |  8 ++------
 2 Dateien ge?ndert, 36 Zeilen hinzugef?gt(+), 20 Zeilen entfernt(-)

diff --git a/drivers/usb/eth/smsc95xx.c b/drivers/usb/eth/smsc95xx.c
index c62a8c1..4bf2a16 100644
--- a/drivers/usb/eth/smsc95xx.c
+++ b/drivers/usb/eth/smsc95xx.c
@@ -25,6 +25,7 @@
 #include <usb.h>
 #include <linux/mii.h>
 #include "usb_ether.h"
+#include "malloc.h"
 
 /* SMSC LAN95xx based USB 2.0 Ethernet Devices */
 
@@ -146,6 +147,12 @@
 /* local vars */
 static int curr_eth_dev; /* index for name of next device detected */
 
+/* driver private */
+struct smsc95xx_private {
+	size_t rx_urb_size;  /* maximum USB URB size */
+	u32 mac_cr;  /* MAC control register value */
+	int have_hwaddr;  /* 1 if we have a hardware MAC address */
+};
 
 /*
  * Smsc95xx infrastructure commands
@@ -377,6 +384,7 @@ static int smsc95xx_init_mac_address(struct eth_device *eth,
 static int smsc95xx_write_hwaddr(struct eth_device *eth)
 {
 	struct ueth_data *dev = (struct ueth_data *)eth->priv;
+	struct smsc95xx_private *priv = dev->dev_priv;
 	u32 addr_lo = __get_unaligned_le32(&eth->enetaddr[0]);
 	u32 addr_hi = __get_unaligned_le16(&eth->enetaddr[4]);
 	int ret;
@@ -392,7 +400,7 @@ static int smsc95xx_write_hwaddr(struct eth_device *eth)
 		return ret;
 
 	debug("MAC %pM\n", eth->enetaddr);
-	dev->have_hwaddr = 1;
+	priv->have_hwaddr = 1;
 	return 0;
 }
 
@@ -425,19 +433,22 @@ static int smsc95xx_set_csums(struct ueth_data *dev,
 
 static void smsc95xx_set_multicast(struct ueth_data *dev)
 {
+	struct smsc95xx_private *priv = dev->dev_priv;
+
 	/* No multicast in u-boot */
-	dev->mac_cr &= ~(MAC_CR_PRMS_ | MAC_CR_MCPAS_ | MAC_CR_HPFILT_);
+	priv->mac_cr &= ~(MAC_CR_PRMS_ | MAC_CR_MCPAS_ | MAC_CR_HPFILT_);
 }
 
 /* starts the TX path */
 static void smsc95xx_start_tx_path(struct ueth_data *dev)
 {
+	struct smsc95xx_private *priv = dev->dev_priv;
 	u32 reg_val;
 
 	/* Enable Tx at MAC */
-	dev->mac_cr |= MAC_CR_TXEN_;
+	priv->mac_cr |= MAC_CR_TXEN_;
 
-	smsc95xx_write_reg(dev, MAC_CR, dev->mac_cr);
+	smsc95xx_write_reg(dev, MAC_CR, priv->mac_cr);
 
 	/* Enable Tx at SCSRs */
 	reg_val = TX_CFG_ON_;
@@ -447,8 +458,10 @@ static void smsc95xx_start_tx_path(struct ueth_data *dev)
 /* Starts the Receive path */
 static void smsc95xx_start_rx_path(struct ueth_data *dev)
 {
-	dev->mac_cr |= MAC_CR_RXEN_;
-	smsc95xx_write_reg(dev, MAC_CR, dev->mac_cr);
+	struct smsc95xx_private *priv = dev->dev_priv;
+
+	priv->mac_cr |= MAC_CR_RXEN_;
+	smsc95xx_write_reg(dev, MAC_CR, priv->mac_cr);
 }
 
 /*
@@ -462,6 +475,7 @@ static int smsc95xx_init(struct eth_device *eth, bd_t *bd)
 	u32 burst_cap;
 	int timeout;
 	struct ueth_data *dev = (struct ueth_data *)eth->priv;
+	struct smsc95xx_private *priv = (struct smsc95xx_private *)dev->dev_priv;
 #define TIMEOUT_RESOLUTION 50	/* ms */
 	int link_detected;
 
@@ -504,9 +518,9 @@ static int smsc95xx_init(struct eth_device *eth, bd_t *bd)
 		debug("timeout waiting for PHY Reset\n");
 		return -1;
 	}
-	if (!dev->have_hwaddr && smsc95xx_init_mac_address(eth, dev) == 0)
-		dev->have_hwaddr = 1;
-	if (!dev->have_hwaddr) {
+	if (!priv->have_hwaddr && smsc95xx_init_mac_address(eth, dev) == 0)
+		priv->have_hwaddr = 1;
+	if (!priv->have_hwaddr) {
 		puts("Error: SMSC95xx: No MAC address set - set usbethaddr\n");
 		return -1;
 	}
@@ -532,16 +546,16 @@ static int smsc95xx_init(struct eth_device *eth, bd_t *bd)
 #ifdef TURBO_MODE
 	if (dev->pusb_dev->speed == USB_SPEED_HIGH) {
 		burst_cap = DEFAULT_HS_BURST_CAP_SIZE / HS_USB_PKT_SIZE;
-		dev->rx_urb_size = DEFAULT_HS_BURST_CAP_SIZE;
+		priv->rx_urb_size = DEFAULT_HS_BURST_CAP_SIZE;
 	} else {
 		burst_cap = DEFAULT_FS_BURST_CAP_SIZE / FS_USB_PKT_SIZE;
-		dev->rx_urb_size = DEFAULT_FS_BURST_CAP_SIZE;
+		priv->rx_urb_size = DEFAULT_FS_BURST_CAP_SIZE;
 	}
 #else
 	burst_cap = 0;
-	dev->rx_urb_size = MAX_SINGLE_PACKET_SIZE;
+	priv->rx_urb_size = MAX_SINGLE_PACKET_SIZE;
 #endif
-	debug("rx_urb_size=%ld\n", (ulong)dev->rx_urb_size);
+	debug("rx_urb_size=%ld\n", (ulong)priv->rx_urb_size);
 
 	ret = smsc95xx_write_reg(dev, BURST_CAP, burst_cap);
 	if (ret < 0)
@@ -606,7 +620,7 @@ static int smsc95xx_init(struct eth_device *eth, bd_t *bd)
 	if (ret < 0)
 		return ret;
 
-	ret = smsc95xx_read_reg(dev, MAC_CR, &dev->mac_cr);
+	ret = smsc95xx_read_reg(dev, MAC_CR, &priv->mac_cr);
 	if (ret < 0)
 		return ret;
 
@@ -857,6 +871,12 @@ int smsc95xx_eth_probe(struct usb_device *dev, unsigned int ifnum,
 		return 0;
 	}
 	dev->privptr = (void *)ss;
+
+	/* alloc driver private */
+	ss->dev_priv = calloc(1, sizeof(struct smsc95xx_private));
+	if (!ss->dev_priv)
+		return 0;
+
 	return 1;
 }
 
diff --git a/include/usb_ether.h b/include/usb_ether.h
index a7fb26b..7c7aecb 100644
--- a/include/usb_ether.h
+++ b/include/usb_ether.h
@@ -50,12 +50,8 @@ struct ueth_data {
 	unsigned char	protocol;		/* .............. */
 	unsigned char	irqinterval;	/* Intervall for IRQ Pipe */
 
-	/* private fields for each driver can go here if needed */
-#ifdef CONFIG_USB_ETHER_SMSC95XX
-	size_t rx_urb_size;  /* maximum USB URB size */
-	u32 mac_cr;  /* MAC control register value */
-	int have_hwaddr;  /* 1 if we have a hardware MAC address */
-#endif
+	/* driver private */
+	void *dev_priv;
 };
 
 /*
-- 
1.7.11.4

[U-Boot] [PATCH 2/6] net: usbeth: remove misleading warning

[Lucas Stach]

If the environment doesn't provide a MAC address it's not an
error to use the one provided by the eth adapter. Actually it
should be the default case if the adapter exports it's own
address during get_info().

So just remove the warning that gets printed in this case.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 net/eth.c | 2 --
 1 Datei ge?ndert, 2 Zeilen entfernt(-)

diff --git a/net/eth.c b/net/eth.c
index 1a11ce1..39cd5da 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -217,8 +217,6 @@ int eth_write_hwaddr(struct eth_device *dev, const char *base_name,
 	} else if (is_valid_ether_addr(dev->enetaddr)) {
 		eth_setenv_enetaddr_by_index(base_name, eth_number,
 					     dev->enetaddr);
-		printf("\nWarning: %s using MAC address from net device\n",
-			dev->name);
 	}
 
 	if (dev->write_hwaddr &&
-- 
1.7.11.4

[U-Boot] [PATCH 3/6] net: asix: split out basic reset function

[Lucas Stach]

The basic device reset ensures that the device is ready to
service commands and does not need to get redone before each
network operation.

Split out the basic reset from asix_init() and instead call it
from asix_eth_get_info(), so that it only gets called once.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/usb/eth/asix.c | 44 ++++++++++++++++++++++++++------------------
 1 Datei ge?ndert, 26 Zeilen hinzugef?gt(+), 18 Zeilen entfernt(-)

diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
index 8fb7fc8..50cbbbd 100644
--- a/drivers/usb/eth/asix.c
+++ b/drivers/usb/eth/asix.c
@@ -310,55 +310,60 @@ static int mii_nway_restart(struct ueth_data *dev)
 	return r;
 }
 
-/*
- * Asix callbacks
- */
-static int asix_init(struct eth_device *eth, bd_t *bd)
+static int asix_basic_reset(struct ueth_data *dev)
 {
 	int embd_phy;
-	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, ETH_ALEN);
 	u16 rx_ctl;
-	struct ueth_data	*dev = (struct ueth_data *)eth->priv;
-	int timeout = 0;
-#define TIMEOUT_RESOLUTION 50	/* ms */
-	int link_detected;
-
-	debug("** %s()\n", __func__);
 
 	if (asix_write_gpio(dev,
 			AX_GPIO_RSE | AX_GPIO_GPO_2 | AX_GPIO_GPO2EN, 5) < 0)
-		goto out_err;
+		return -1;
 
 	/* 0x10 is the phy id of the embedded 10/100 ethernet phy */
 	embd_phy = ((asix_get_phy_addr(dev) & 0x1f) == 0x10 ? 1 : 0);
 	if (asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT,
 				embd_phy, 0, 0, NULL) < 0) {
 		debug("Select PHY #1 failed\n");
-		goto out_err;
+		return -1;
 	}
 
 	if (asix_sw_reset(dev, AX_SWRESET_IPPD | AX_SWRESET_PRL) < 0)
-		goto out_err;
+		return -1;
 
 	if (asix_sw_reset(dev, AX_SWRESET_CLEAR) < 0)
-		goto out_err;
+		return -1;
 
 	if (embd_phy) {
 		if (asix_sw_reset(dev, AX_SWRESET_IPRL) < 0)
-			goto out_err;
+			return -1;
 	} else {
 		if (asix_sw_reset(dev, AX_SWRESET_PRTE) < 0)
-			goto out_err;
+			return -1;
 	}
 
 	rx_ctl = asix_read_rx_ctl(dev);
 	debug("RX_CTL is 0x%04x after software reset\n", rx_ctl);
 	if (asix_write_rx_ctl(dev, 0x0000) < 0)
-		goto out_err;
+		return -1;
 
 	rx_ctl = asix_read_rx_ctl(dev);
 	debug("RX_CTL is 0x%04x setting to 0x0000\n", rx_ctl);
 
+	return 0;
+}
+
+/*
+ * 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;
+#define TIMEOUT_RESOLUTION 50	/* ms */
+	int link_detected;
+
+	debug("** %s()\n", __func__);
+
 	/* Get the MAC address */
 	if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
 				0, 0, ETH_ALEN, buf) < 0) {
@@ -635,5 +640,8 @@ int asix_eth_get_info(struct usb_device *dev, struct ueth_data *ss,
 	eth->halt = asix_halt;
 	eth->priv = ss;
 
+	if (asix_basic_reset(ss))
+		return 0;
+
 	return 1;
 }
-- 
1.7.11.4

[U-Boot] [PATCH 4/6] net: asix: add write_hwaddr function

[Lucas Stach]

All ASIX chipsets aside from AX88172 are able to set the MAC
address on the hardware level. Add a function to expose this
ability.

To differentiate between chip types we now carry flags as driver
private data. Also while touching the asix_dongles array
constify this.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/usb/eth/asix.c | 61 +++++++++++++++++++++++++++++++++++++++++---------
 1 Datei ge?ndert, 50 Zeilen hinzugef?gt(+), 11 Zeilen entfernt(-)

diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
index 50cbbbd..ce1483e 100644
--- a/drivers/usb/eth/asix.c
+++ b/drivers/usb/eth/asix.c
@@ -23,6 +23,7 @@
 #include <usb.h>
 #include <linux/mii.h>
 #include "usb_ether.h"
+#include "malloc.h"
 
 
 /* ASIX AX8817X based USB 2.0 Ethernet Devices */
@@ -35,6 +36,7 @@
 #define AX_CMD_WRITE_RX_CTL		0x10
 #define AX_CMD_WRITE_IPG0		0x12
 #define AX_CMD_READ_NODE_ID		0x13
+#define AX_CMD_WRITE_NODE_ID	0x14
 #define AX_CMD_READ_PHY_ID		0x19
 #define AX_CMD_WRITE_MEDIUM_MODE	0x1b
 #define AX_CMD_WRITE_GPIOS		0x1f
@@ -97,9 +99,19 @@
 #define AX_RX_URB_SIZE 2048
 #define PHY_CONNECT_TIMEOUT 5000
 
+/* asix_flags defines */
+#define FLAG_NONE			0
+#define FLAG_TYPE_AX88172	(1U << 0)
+#define FLAG_TYPE_AX88772	(1U << 1)
+#define FLAG_TYPE_AX88772B	(1U << 2)
 /* local vars */
 static int curr_eth_dev; /* index for name of next device detected */
 
+/* driver private */
+struct asix_private {
+	int flags;
+};
+
 /*
  * Asix infrastructure commands
  */
@@ -284,6 +296,21 @@ static int asix_write_gpio(struct ueth_data *dev, u16 value, int sleep)
 	return ret;
 }
 
+static int asix_write_hwaddr(struct eth_device *eth)
+{
+	struct ueth_data *dev = (struct ueth_data *)eth->priv;
+	int ret;
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, ETH_ALEN);
+
+	memcpy(buf, eth->enetaddr, ETH_ALEN);
+
+	ret = asix_write_cmd(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN, buf);
+	if (ret < 0)
+		debug("Failed to set MAC address: %02x\n", ret);
+
+	return ret;
+}
+
 /*
  * mii commands
  */
@@ -539,19 +566,20 @@ void asix_eth_before_probe(void)
 struct asix_dongle {
 	unsigned short vendor;
 	unsigned short product;
+	int flags;
 };
 
-static struct asix_dongle asix_dongles[] = {
-	{ 0x05ac, 0x1402 },	/* Apple USB Ethernet Adapter */
-	{ 0x07d1, 0x3c05 },	/* D-Link DUB-E100 H/W Ver B1 */
-	{ 0x0b95, 0x772a },	/* Cables-to-Go USB Ethernet Adapter */
-	{ 0x0b95, 0x7720 },	/* Trendnet TU2-ET100 V3.0R */
-	{ 0x0b95, 0x1720 },	/* SMC */
-	{ 0x0db0, 0xa877 },	/* MSI - ASIX 88772a */
-	{ 0x13b1, 0x0018 },	/* Linksys 200M v2.1 */
-	{ 0x1557, 0x7720 },	/* 0Q0 cable ethernet */
-	{ 0x2001, 0x3c05 },	/* DLink DUB-E100 H/W Ver B1 Alternate */
-	{ 0x0000, 0x0000 }	/* END - Do not remove */
+static const struct asix_dongle asix_dongles[] = {
+	{ 0x05ac, 0x1402, FLAG_TYPE_AX88772 },	/* Apple USB Ethernet Adapter */
+	{ 0x07d1, 0x3c05, FLAG_TYPE_AX88772 },	/* D-Link DUB-E100 H/W Ver B1 */
+	{ 0x0b95, 0x772a, FLAG_TYPE_AX88772 },	/* Cables-to-Go USB Ethernet Adapter */
+	{ 0x0b95, 0x7720, FLAG_TYPE_AX88772 },	/* Trendnet TU2-ET100 V3.0R */
+	{ 0x0b95, 0x1720, FLAG_TYPE_AX88172 },	/* SMC */
+	{ 0x0db0, 0xa877, FLAG_TYPE_AX88772 },	/* MSI - ASIX 88772a */
+	{ 0x13b1, 0x0018, FLAG_TYPE_AX88172 },	/* Linksys 200M v2.1 */
+	{ 0x1557, 0x7720, FLAG_TYPE_AX88772 },	/* 0Q0 cable ethernet */
+	{ 0x2001, 0x3c05, FLAG_TYPE_AX88772 },	/* DLink DUB-E100 H/W Ver B1 Alternate */
+	{ 0x0000, 0x0000, FLAG_NONE }	/* END - Do not remove */
 };
 
 /* Probe to see if a new device is actually an asix device */
@@ -588,6 +616,13 @@ int asix_eth_probe(struct usb_device *dev, unsigned int ifnum,
 	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.
@@ -629,6 +664,8 @@ int asix_eth_probe(struct usb_device *dev, unsigned int ifnum,
 int asix_eth_get_info(struct usb_device *dev, struct ueth_data *ss,
 				struct eth_device *eth)
 {
+	struct asix_private *priv = (struct asix_private *)ss->dev_priv;
+
 	if (!eth) {
 		debug("%s: missing parameter.\n", __func__);
 		return 0;
@@ -638,6 +675,8 @@ int asix_eth_get_info(struct usb_device *dev, struct ueth_data *ss,
 	eth->send = asix_send;
 	eth->recv = asix_recv;
 	eth->halt = asix_halt;
+	if (!(priv->flags & FLAG_TYPE_AX88172))
+		eth->write_hwaddr = asix_write_hwaddr;
 	eth->priv = ss;
 
 	if (asix_basic_reset(ss))
-- 
1.7.11.4

[U-Boot] [PATCH 5/6] net: asix: add read_mac function

[Lucas Stach]

Initial device MAC should be read while getting info about the
device, so it's wrong to only read it in asix_init().

Add a dedicated function to read the initial MAC, which is also
able to handle devices that have their initial MAC stored in
EEPROM. Call this function inasix_eth_get_info().

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/usb/eth/asix.c | 47 +++++++++++++++++++++++++++++++++++------------
 1 Datei ge?ndert, 35 Zeilen hinzugef?gt(+), 12 Zeilen entfernt(-)

diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
index ce1483e..6ab3e82 100644
--- a/drivers/usb/eth/asix.c
+++ b/drivers/usb/eth/asix.c
@@ -32,6 +32,7 @@
 #define AX_CMD_READ_MII_REG		0x07
 #define AX_CMD_WRITE_MII_REG		0x08
 #define AX_CMD_SET_HW_MII		0x0a
+#define AX_CMD_READ_EEPROM		0x0b
 #define AX_CMD_READ_RX_CTL		0x0f
 #define AX_CMD_WRITE_RX_CTL		0x10
 #define AX_CMD_WRITE_IPG0		0x12
@@ -104,6 +105,8 @@
 #define FLAG_TYPE_AX88172	(1U << 0)
 #define FLAG_TYPE_AX88772	(1U << 1)
 #define FLAG_TYPE_AX88772B	(1U << 2)
+#define FLAG_EEPROM_MAC		(1U << 3) /* initial mac address in eeprom */
+
 /* local vars */
 static int curr_eth_dev; /* index for name of next device detected */
 
@@ -337,6 +340,33 @@ static int mii_nway_restart(struct ueth_data *dev)
 	return r;
 }
 
+static int asix_read_mac(struct eth_device *eth)
+{
+	struct ueth_data *dev = (struct ueth_data *)eth->priv;
+	struct asix_private *priv = (struct asix_private *)dev->dev_priv;
+	int i;
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, ETH_ALEN);
+
+	if (priv->flags & FLAG_EEPROM_MAC) {
+		for (i = 0; i < (ETH_ALEN >> 1); i++) {
+			if (asix_read_cmd(dev, AX_CMD_READ_EEPROM,
+			                  0x04 + i, 0, 2, buf) < 0) {
+				debug("Failed to read SROM address 04h.\n");
+				return -1;
+			}
+			memcpy((eth->enetaddr + i * 2), buf, 2);
+		}
+	} else {
+		if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf) < 0) {
+			debug("Failed to read MAC address.\n");
+			return -1;
+		}
+		memcpy(eth->enetaddr, buf, ETH_ALEN);
+	}
+
+	return 0;
+}
+
 static int asix_basic_reset(struct ueth_data *dev)
 {
 	int embd_phy;
@@ -391,18 +421,6 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
 
 	debug("** %s()\n", __func__);
 
-	/* Get the MAC address */
-	if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
-				0, 0, ETH_ALEN, buf) < 0) {
-		debug("Failed to read MAC address.\n");
-		goto out_err;
-	}
-	memcpy(eth->enetaddr, buf, ETH_ALEN);
-	debug("MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
-		eth->enetaddr[0], eth->enetaddr[1],
-		eth->enetaddr[2], eth->enetaddr[3],
-		eth->enetaddr[4], eth->enetaddr[5]);
-
 	dev->phy_id = asix_get_phy_addr(dev);
 	if (dev->phy_id < 0)
 		debug("Failed to read phy id\n");
@@ -682,5 +700,10 @@ int asix_eth_get_info(struct usb_device *dev, struct ueth_data *ss,
 	if (asix_basic_reset(ss))
 		return 0;
 
+	/* Get the MAC address */
+	if (asix_read_mac(eth))
+		return 0;
+	debug("MAC %pM\n", eth->enetaddr);
+
 	return 1;
 }
-- 
1.7.11.4

[U-Boot] [PATCH 6/6] net: asix: add AX88772B support

[Lucas Stach]

Add AX88772B ID together with two fixes needed to make this work.

1. The packet length check has to be adjusted, as all ASIX chips
only use 11 bits to indicate the length. AX88772B uses the other
bits to indicate unrelated things, which cause the check to fail.
This fix is based on a fix for the Linux kernel by Marek Vasut.
Linux upstream commit: bca0beb9363f8487ac902931a50eb00180a2d14a

2. AX88772B provides several bulk endpoints. Only the first
IN/OUT endpoints work in the default configuration. So stop
enumeration after we found them to avoid overwriting the
endpoint config with a non-working one.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/usb/eth/asix.c | 30 +++++++++++++++++++-----------
 1 Datei ge?ndert, 19 Zeilen hinzugef?gt(+), 11 Zeilen entfernt(-)

diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
index 6ab3e82..79246d8 100644
--- a/drivers/usb/eth/asix.c
+++ b/drivers/usb/eth/asix.c
@@ -543,13 +543,13 @@ static int asix_recv(struct eth_device *eth)
 		}
 		memcpy(&packet_len, buf_ptr, sizeof(packet_len));
 		le32_to_cpus(&packet_len);
-		if (((packet_len >> 16) ^ 0xffff) != (packet_len & 0xffff)) {
+		if (((~packet_len >> 16) & 0x7ff) != (packet_len & 0x7ff)) {
 			debug("Rx: malformed packet length: %#x (%#x:%#x)\n",
-			      packet_len, (packet_len >> 16) ^ 0xffff,
-			      packet_len & 0xffff);
+			      packet_len, (~packet_len >> 16) & 0x7ff,
+			      packet_len & 0x7ff);
 			return -1;
 		}
-		packet_len = packet_len & 0xffff;
+		packet_len = packet_len & 0x7ff;
 		if (packet_len > actual_len - sizeof(packet_len)) {
 			debug("Rx: too large packet: %d\n", packet_len);
 			return -1;
@@ -597,6 +597,7 @@ static const struct asix_dongle asix_dongles[] = {
 	{ 0x13b1, 0x0018, FLAG_TYPE_AX88172 },	/* Linksys 200M v2.1 */
 	{ 0x1557, 0x7720, FLAG_TYPE_AX88772 },	/* 0Q0 cable ethernet */
 	{ 0x2001, 0x3c05, FLAG_TYPE_AX88772 },	/* DLink DUB-E100 H/W Ver B1 Alternate */
+	{ 0x0b95, 0x772b, FLAG_TYPE_AX88772B | FLAG_EEPROM_MAC }, /* ASIX 88772B */
 	{ 0x0000, 0x0000, FLAG_NONE }	/* END - Do not remove */
 };
 
@@ -606,6 +607,7 @@ int asix_eth_probe(struct usb_device *dev, unsigned int ifnum,
 {
 	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 */
@@ -649,13 +651,19 @@ int asix_eth_probe(struct usb_device *dev, unsigned int ifnum,
 		/* is it an BULK endpoint? */
 		if ((iface->ep_desc[i].bmAttributes &
 		     USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) {
-			if (iface->ep_desc[i].bEndpointAddress & USB_DIR_IN)
-				ss->ep_in = iface->ep_desc[i].bEndpointAddress &
-					USB_ENDPOINT_NUMBER_MASK;
-			else
-				ss->ep_out =
-					iface->ep_desc[i].bEndpointAddress &
-					USB_ENDPOINT_NUMBER_MASK;
+			if (iface->ep_desc[i].bEndpointAddress & USB_DIR_IN) {
+				if (!ep_in_found) {
+					ss->ep_in = iface->ep_desc[i].bEndpointAddress &
+					            USB_ENDPOINT_NUMBER_MASK;
+					ep_in_found = 1;
+				}
+			} else {
+				if (!ep_out_found) {
+					ss->ep_out = iface->ep_desc[i].bEndpointAddress &
+					             USB_ENDPOINT_NUMBER_MASK;
+					ep_out_found = 1;
+				}
+			}
 		}
 
 		/* is it an interrupt endpoint? */
-- 
1.7.11.4

[U-Boot] [PATCH 0/6] net: ASIX AX88772B enablement

[Marek Vasut]

Dear Lucas Stach,

> Hi all,
> 
> This series supersedes the earlier posted patches to support the
> AX88772B chip. I've split this up into a series as what started
> as a small patch to get the chip working turned into a journey
> through the usbeth stack and now not only reworks a lot of the
> ASIX code, but also touches other parts of the stack.
> 
> I want to thank Marek, Joe and Mike for their review, valueable
> feedback and guidance.

Marek blushes and his ego explodes ;-D

> I've verified that we are now actually able to override the MAC
> address from the environment. All review feedback so far has been
> incorporated.

Anyway, thank _YOU_ for your effort, that's the important part here! It is, was 
and always will be the community that forges the project.

> Lucas Stach (6):
>   net: introduce transparent driver private in ueth_data
>   net: usbeth: remove misleading warning
>   net: asix: split out basic reset function
>   net: asix: add write_hwaddr function
>   net: asix: add read_mac function
>   net: asix: add AX88772B support
> 
>  drivers/usb/eth/asix.c     | 180
> ++++++++++++++++++++++++++++++++------------- drivers/usb/eth/smsc95xx.c |
>  48 ++++++++----
>  include/usb_ether.h        |   8 +-
>  net/eth.c                  |   2 -
>  4 Dateien ge?ndert, 165 Zeilen hinzugef?gt(+), 73 Zeilen entfernt(-)

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/6] net: introduce transparent driver private in ueth_data

[Marek Vasut]

Dear Lucas Stach,

> Avoid clutter in ueth_data. Individual drivers should not mess
> with structures belonging to the core like this.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>

Reviewed-by: Marek Vasut <marex@denx.de>
Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/usb/eth/smsc95xx.c | 48
> ++++++++++++++++++++++++++++++++-------------- include/usb_ether.h       
> |  8 ++------
>  2 Dateien ge?ndert, 36 Zeilen hinzugef?gt(+), 20 Zeilen entfernt(-)

You might want to fix your locale ... but I'm surprised git was localised at all 
:)

> diff --git a/drivers/usb/eth/smsc95xx.c b/drivers/usb/eth/smsc95xx.c
> index c62a8c1..4bf2a16 100644
> --- a/drivers/usb/eth/smsc95xx.c
> +++ b/drivers/usb/eth/smsc95xx.c
> @@ -25,6 +25,7 @@
>  #include <usb.h>
>  #include <linux/mii.h>
>  #include "usb_ether.h"
> +#include "malloc.h"
> 
>  /* SMSC LAN95xx based USB 2.0 Ethernet Devices */
> 
> @@ -146,6 +147,12 @@
>  /* local vars */
>  static int curr_eth_dev; /* index for name of next device detected */
> 
> +/* driver private */
> +struct smsc95xx_private {
> +	size_t rx_urb_size;  /* maximum USB URB size */
> +	u32 mac_cr;  /* MAC control register value */
> +	int have_hwaddr;  /* 1 if we have a hardware MAC address */
> +};
> 
>  /*
>   * Smsc95xx infrastructure commands
> @@ -377,6 +384,7 @@ static int smsc95xx_init_mac_address(struct eth_device
> *eth, static int smsc95xx_write_hwaddr(struct eth_device *eth)
>  {
>  	struct ueth_data *dev = (struct ueth_data *)eth->priv;
> +	struct smsc95xx_private *priv = dev->dev_priv;
>  	u32 addr_lo = __get_unaligned_le32(&eth->enetaddr[0]);
>  	u32 addr_hi = __get_unaligned_le16(&eth->enetaddr[4]);
>  	int ret;
> @@ -392,7 +400,7 @@ static int smsc95xx_write_hwaddr(struct eth_device
> *eth) return ret;
> 
>  	debug("MAC %pM\n", eth->enetaddr);
> -	dev->have_hwaddr = 1;
> +	priv->have_hwaddr = 1;
>  	return 0;
>  }
> 
> @@ -425,19 +433,22 @@ static int smsc95xx_set_csums(struct ueth_data *dev,
> 
>  static void smsc95xx_set_multicast(struct ueth_data *dev)
>  {
> +	struct smsc95xx_private *priv = dev->dev_priv;
> +
>  	/* No multicast in u-boot */
> -	dev->mac_cr &= ~(MAC_CR_PRMS_ | MAC_CR_MCPAS_ | MAC_CR_HPFILT_);
> +	priv->mac_cr &= ~(MAC_CR_PRMS_ | MAC_CR_MCPAS_ | MAC_CR_HPFILT_);
>  }
> 
>  /* starts the TX path */
>  static void smsc95xx_start_tx_path(struct ueth_data *dev)
>  {
> +	struct smsc95xx_private *priv = dev->dev_priv;
>  	u32 reg_val;
> 
>  	/* Enable Tx at MAC */
> -	dev->mac_cr |= MAC_CR_TXEN_;
> +	priv->mac_cr |= MAC_CR_TXEN_;
> 
> -	smsc95xx_write_reg(dev, MAC_CR, dev->mac_cr);
> +	smsc95xx_write_reg(dev, MAC_CR, priv->mac_cr);
> 
>  	/* Enable Tx at SCSRs */
>  	reg_val = TX_CFG_ON_;
> @@ -447,8 +458,10 @@ static void smsc95xx_start_tx_path(struct ueth_data
> *dev) /* Starts the Receive path */
>  static void smsc95xx_start_rx_path(struct ueth_data *dev)
>  {
> -	dev->mac_cr |= MAC_CR_RXEN_;
> -	smsc95xx_write_reg(dev, MAC_CR, dev->mac_cr);
> +	struct smsc95xx_private *priv = dev->dev_priv;
> +
> +	priv->mac_cr |= MAC_CR_RXEN_;
> +	smsc95xx_write_reg(dev, MAC_CR, priv->mac_cr);
>  }
> 
>  /*
> @@ -462,6 +475,7 @@ static int smsc95xx_init(struct eth_device *eth, bd_t
> *bd) u32 burst_cap;
>  	int timeout;
>  	struct ueth_data *dev = (struct ueth_data *)eth->priv;
> +	struct smsc95xx_private *priv = (struct smsc95xx_private *)dev-
>dev_priv;
>  #define TIMEOUT_RESOLUTION 50	/* ms */
>  	int link_detected;
> 
> @@ -504,9 +518,9 @@ static int smsc95xx_init(struct eth_device *eth, bd_t
> *bd) debug("timeout waiting for PHY Reset\n");
>  		return -1;
>  	}
> -	if (!dev->have_hwaddr && smsc95xx_init_mac_address(eth, dev) == 0)
> -		dev->have_hwaddr = 1;
> -	if (!dev->have_hwaddr) {
> +	if (!priv->have_hwaddr && smsc95xx_init_mac_address(eth, dev) == 0)
> +		priv->have_hwaddr = 1;
> +	if (!priv->have_hwaddr) {
>  		puts("Error: SMSC95xx: No MAC address set - set usbethaddr\n");
>  		return -1;
>  	}
> @@ -532,16 +546,16 @@ static int smsc95xx_init(struct eth_device *eth, bd_t
> *bd) #ifdef TURBO_MODE
>  	if (dev->pusb_dev->speed == USB_SPEED_HIGH) {
>  		burst_cap = DEFAULT_HS_BURST_CAP_SIZE / HS_USB_PKT_SIZE;
> -		dev->rx_urb_size = DEFAULT_HS_BURST_CAP_SIZE;
> +		priv->rx_urb_size = DEFAULT_HS_BURST_CAP_SIZE;
>  	} else {
>  		burst_cap = DEFAULT_FS_BURST_CAP_SIZE / FS_USB_PKT_SIZE;
> -		dev->rx_urb_size = DEFAULT_FS_BURST_CAP_SIZE;
> +		priv->rx_urb_size = DEFAULT_FS_BURST_CAP_SIZE;
>  	}
>  #else
>  	burst_cap = 0;
> -	dev->rx_urb_size = MAX_SINGLE_PACKET_SIZE;
> +	priv->rx_urb_size = MAX_SINGLE_PACKET_SIZE;
>  #endif
> -	debug("rx_urb_size=%ld\n", (ulong)dev->rx_urb_size);
> +	debug("rx_urb_size=%ld\n", (ulong)priv->rx_urb_size);
> 
>  	ret = smsc95xx_write_reg(dev, BURST_CAP, burst_cap);
>  	if (ret < 0)
> @@ -606,7 +620,7 @@ static int smsc95xx_init(struct eth_device *eth, bd_t
> *bd) if (ret < 0)
>  		return ret;
> 
> -	ret = smsc95xx_read_reg(dev, MAC_CR, &dev->mac_cr);
> +	ret = smsc95xx_read_reg(dev, MAC_CR, &priv->mac_cr);
>  	if (ret < 0)
>  		return ret;
> 
> @@ -857,6 +871,12 @@ int smsc95xx_eth_probe(struct usb_device *dev,
> unsigned int ifnum, return 0;
>  	}
>  	dev->privptr = (void *)ss;
> +
> +	/* alloc driver private */
> +	ss->dev_priv = calloc(1, sizeof(struct smsc95xx_private));
> +	if (!ss->dev_priv)
> +		return 0;
> +
>  	return 1;
>  }
> 
> diff --git a/include/usb_ether.h b/include/usb_ether.h
> index a7fb26b..7c7aecb 100644
> --- a/include/usb_ether.h
> +++ b/include/usb_ether.h
> @@ -50,12 +50,8 @@ struct ueth_data {
>  	unsigned char	protocol;		/* .............. */
>  	unsigned char	irqinterval;	/* Intervall for IRQ Pipe */
> 
> -	/* private fields for each driver can go here if needed */
> -#ifdef CONFIG_USB_ETHER_SMSC95XX
> -	size_t rx_urb_size;  /* maximum USB URB size */
> -	u32 mac_cr;  /* MAC control register value */
> -	int have_hwaddr;  /* 1 if we have a hardware MAC address */
> -#endif
> +	/* driver private */
> +	void *dev_priv;
>  };
> 
>  /*

[U-Boot] [PATCH 2/6] net: usbeth: remove misleading warning

[Marek Vasut]

Dear Lucas Stach,

> If the environment doesn't provide a MAC address it's not an
> error to use the one provided by the eth adapter. Actually it
> should be the default case if the adapter exports it's own
> address during get_info().

I think we should warn if the addr in env doesn't match the one in the device. 
Otherwise, if the env doesn't contain ethaddr (or eth1addr, eth2addr ...), pull 
one from the device and inform user.

> So just remove the warning that gets printed in this case.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  net/eth.c | 2 --
>  1 Datei ge?ndert, 2 Zeilen entfernt(-)
> 
> diff --git a/net/eth.c b/net/eth.c
> index 1a11ce1..39cd5da 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -217,8 +217,6 @@ int eth_write_hwaddr(struct eth_device *dev, const char
> *base_name, } else if (is_valid_ether_addr(dev->enetaddr)) {
>  		eth_setenv_enetaddr_by_index(base_name, eth_number,
>  					     dev->enetaddr);
> -		printf("\nWarning: %s using MAC address from net device\n",
> -			dev->name);
>  	}
> 
>  	if (dev->write_hwaddr &&

Best regards,
Marek Vasut

[U-Boot] [PATCH 3/6] net: asix: split out basic reset function

[Marek Vasut]

Dear Lucas Stach,

> The basic device reset ensures that the device is ready to
> service commands and does not need to get redone before each
> network operation.
> 
> Split out the basic reset from asix_init() and instead call it
> from asix_eth_get_info(), so that it only gets called once.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  drivers/usb/eth/asix.c | 44 ++++++++++++++++++++++++++------------------
>  1 Datei ge?ndert, 26 Zeilen hinzugef?gt(+), 18 Zeilen entfernt(-)
> 
> diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> index 8fb7fc8..50cbbbd 100644
> --- a/drivers/usb/eth/asix.c
> +++ b/drivers/usb/eth/asix.c
> @@ -310,55 +310,60 @@ static int mii_nway_restart(struct ueth_data *dev)
>  	return r;
>  }
> 
> -/*
> - * Asix callbacks
> - */
> -static int asix_init(struct eth_device *eth, bd_t *bd)
> +static int asix_basic_reset(struct ueth_data *dev)
>  {
>  	int embd_phy;
> -	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, ETH_ALEN);
>  	u16 rx_ctl;
> -	struct ueth_data	*dev = (struct ueth_data *)eth->priv;
> -	int timeout = 0;
> -#define TIMEOUT_RESOLUTION 50	/* ms */
> -	int link_detected;
> -
> -	debug("** %s()\n", __func__);
> 
>  	if (asix_write_gpio(dev,
>  			AX_GPIO_RSE | AX_GPIO_GPO_2 | AX_GPIO_GPO2EN, 5) < 0)
> -		goto out_err;
> +		return -1;

You might want to use proper errno.h here ... like -ETIMEDOUT etc.

> 
>  	/* 0x10 is the phy id of the embedded 10/100 ethernet phy */
>  	embd_phy = ((asix_get_phy_addr(dev) & 0x1f) == 0x10 ? 1 : 0);
>  	if (asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT,
>  				embd_phy, 0, 0, NULL) < 0) {
>  		debug("Select PHY #1 failed\n");
> -		goto out_err;
> +		return -1;
>  	}
> 
>  	if (asix_sw_reset(dev, AX_SWRESET_IPPD | AX_SWRESET_PRL) < 0)
> -		goto out_err;
> +		return -1;
> 
>  	if (asix_sw_reset(dev, AX_SWRESET_CLEAR) < 0)
> -		goto out_err;
> +		return -1;
> 
>  	if (embd_phy) {
>  		if (asix_sw_reset(dev, AX_SWRESET_IPRL) < 0)
> -			goto out_err;
> +			return -1;
>  	} else {
>  		if (asix_sw_reset(dev, AX_SWRESET_PRTE) < 0)
> -			goto out_err;
> +			return -1;
>  	}
> 
>  	rx_ctl = asix_read_rx_ctl(dev);
>  	debug("RX_CTL is 0x%04x after software reset\n", rx_ctl);
>  	if (asix_write_rx_ctl(dev, 0x0000) < 0)
> -		goto out_err;
> +		return -1;
> 
>  	rx_ctl = asix_read_rx_ctl(dev);
>  	debug("RX_CTL is 0x%04x setting to 0x0000\n", rx_ctl);
> 
> +	return 0;
> +}
> +
> +/*
> + * 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;
> +#define TIMEOUT_RESOLUTION 50	/* ms */
> +	int link_detected;
> +
> +	debug("** %s()\n", __func__);
> +
>  	/* Get the MAC address */
>  	if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
>  				0, 0, ETH_ALEN, buf) < 0) {
> @@ -635,5 +640,8 @@ int asix_eth_get_info(struct usb_device *dev, struct
> ueth_data *ss, eth->halt = asix_halt;
>  	eth->priv = ss;
> 
> +	if (asix_basic_reset(ss))
> +		return 0;
> +
>  	return 1;
>  }

Best regards,
Marek Vasut

[U-Boot] [PATCH 4/6] net: asix: add write_hwaddr function

[Marek Vasut]

Dear Lucas Stach,

> All ASIX chipsets aside from AX88172 are able to set the MAC
> address on the hardware level. Add a function to expose this
> ability.
> 
> To differentiate between chip types we now carry flags as driver
> private data. Also while touching the asix_dongles array
> constify this.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  drivers/usb/eth/asix.c | 61
> +++++++++++++++++++++++++++++++++++++++++--------- 1 Datei ge?ndert, 50
> Zeilen hinzugef?gt(+), 11 Zeilen entfernt(-)
> 
> diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> index 50cbbbd..ce1483e 100644
> --- a/drivers/usb/eth/asix.c
> +++ b/drivers/usb/eth/asix.c
> @@ -23,6 +23,7 @@
>  #include <usb.h>
>  #include <linux/mii.h>
>  #include "usb_ether.h"
> +#include "malloc.h"

#include <malloc.h>

> 
>  /* ASIX AX8817X based USB 2.0 Ethernet Devices */
> @@ -35,6 +36,7 @@
>  #define AX_CMD_WRITE_RX_CTL		0x10
>  #define AX_CMD_WRITE_IPG0		0x12
>  #define AX_CMD_READ_NODE_ID		0x13
> +#define AX_CMD_WRITE_NODE_ID	0x14
>  #define AX_CMD_READ_PHY_ID		0x19
>  #define AX_CMD_WRITE_MEDIUM_MODE	0x1b
>  #define AX_CMD_WRITE_GPIOS		0x1f
> @@ -97,9 +99,19 @@
>  #define AX_RX_URB_SIZE 2048
>  #define PHY_CONNECT_TIMEOUT 5000
> 
> +/* asix_flags defines */
> +#define FLAG_NONE			0
> +#define FLAG_TYPE_AX88172	(1U << 0)
> +#define FLAG_TYPE_AX88772	(1U << 1)
> +#define FLAG_TYPE_AX88772B	(1U << 2)
>  /* local vars */
>  static int curr_eth_dev; /* index for name of next device detected */
> 
> +/* driver private */
> +struct asix_private {
> +	int flags;
> +};
> +
>  /*
>   * Asix infrastructure commands
>   */
> @@ -284,6 +296,21 @@ static int asix_write_gpio(struct ueth_data *dev, u16
> value, int sleep) return ret;
>  }
> 
> +static int asix_write_hwaddr(struct eth_device *eth)
> +{
> +	struct ueth_data *dev = (struct ueth_data *)eth->priv;
> +	int ret;
> +	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, ETH_ALEN);
> +
> +	memcpy(buf, eth->enetaddr, ETH_ALEN);
> +
> +	ret = asix_write_cmd(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN, buf);
> +	if (ret < 0)
> +		debug("Failed to set MAC address: %02x\n", ret);
> +
> +	return ret;
> +}
> +
>  /*
>   * mii commands
>   */
> @@ -539,19 +566,20 @@ void asix_eth_before_probe(void)
>  struct asix_dongle {
>  	unsigned short vendor;
>  	unsigned short product;
> +	int flags;
>  };
> 
> -static struct asix_dongle asix_dongles[] = {
> -	{ 0x05ac, 0x1402 },	/* Apple USB Ethernet Adapter */
> -	{ 0x07d1, 0x3c05 },	/* D-Link DUB-E100 H/W Ver B1 */
> -	{ 0x0b95, 0x772a },	/* Cables-to-Go USB Ethernet Adapter */
> -	{ 0x0b95, 0x7720 },	/* Trendnet TU2-ET100 V3.0R */
> -	{ 0x0b95, 0x1720 },	/* SMC */
> -	{ 0x0db0, 0xa877 },	/* MSI - ASIX 88772a */
> -	{ 0x13b1, 0x0018 },	/* Linksys 200M v2.1 */
> -	{ 0x1557, 0x7720 },	/* 0Q0 cable ethernet */
> -	{ 0x2001, 0x3c05 },	/* DLink DUB-E100 H/W Ver B1 Alternate */
> -	{ 0x0000, 0x0000 }	/* END - Do not remove */
> +static const struct asix_dongle asix_dongles[] = {

Shouldn't this be static const __type const __name[] ?

> +	{ 0x05ac, 0x1402, FLAG_TYPE_AX88772 },	/* Apple USB Ethernet Adapter */
> +	{ 0x07d1, 0x3c05, FLAG_TYPE_AX88772 },	/* D-Link DUB-E100 H/W Ver B1 */
> +	{ 0x0b95, 0x772a, FLAG_TYPE_AX88772 },	/* Cables-to-Go USB Ethernet
> Adapter */ +	{ 0x0b95, 0x7720, FLAG_TYPE_AX88772 },	/* Trendnet TU2-ET100
> V3.0R */ +	{ 0x0b95, 0x1720, FLAG_TYPE_AX88172 },	/* SMC */
> +	{ 0x0db0, 0xa877, FLAG_TYPE_AX88772 },	/* MSI - ASIX 88772a */
> +	{ 0x13b1, 0x0018, FLAG_TYPE_AX88172 },	/* Linksys 200M v2.1 */
> +	{ 0x1557, 0x7720, FLAG_TYPE_AX88772 },	/* 0Q0 cable ethernet */
> +	{ 0x2001, 0x3c05, FLAG_TYPE_AX88772 },	/* DLink DUB-E100 H/W Ver B1
> Alternate */ +	{ 0x0000, 0x0000, FLAG_NONE }	/* END - Do not remove 
*/
>  };
> 
>  /* Probe to see if a new device is actually an asix device */
> @@ -588,6 +616,13 @@ int asix_eth_probe(struct usb_device *dev, unsigned
> int ifnum, 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.
> @@ -629,6 +664,8 @@ int asix_eth_probe(struct usb_device *dev, unsigned int
> ifnum, int asix_eth_get_info(struct usb_device *dev, struct ueth_data *ss,
> struct eth_device *eth)
>  {
> +	struct asix_private *priv = (struct asix_private *)ss->dev_priv;
> +
>  	if (!eth) {
>  		debug("%s: missing parameter.\n", __func__);
>  		return 0;
> @@ -638,6 +675,8 @@ int asix_eth_get_info(struct usb_device *dev, struct
> ueth_data *ss, eth->send = asix_send;
>  	eth->recv = asix_recv;
>  	eth->halt = asix_halt;
> +	if (!(priv->flags & FLAG_TYPE_AX88172))
> +		eth->write_hwaddr = asix_write_hwaddr;
>  	eth->priv = ss;
> 
>  	if (asix_basic_reset(ss))

Best regards,
Marek Vasut

[U-Boot] [PATCH 5/6] net: asix: add read_mac function

[Marek Vasut]

Dear Lucas Stach,

> Initial device MAC should be read while getting info about the
> device, so it's wrong to only read it in asix_init().
> 
> Add a dedicated function to read the initial MAC, which is also
> able to handle devices that have their initial MAC stored in
> EEPROM. Call this function inasix_eth_get_info().
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>

Reviewed-by: Marek Vasut <marex@denx.de>
Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/usb/eth/asix.c | 47
> +++++++++++++++++++++++++++++++++++------------ 1 Datei ge?ndert, 35
> Zeilen hinzugef?gt(+), 12 Zeilen entfernt(-)
> 
> diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> index ce1483e..6ab3e82 100644
> --- a/drivers/usb/eth/asix.c
> +++ b/drivers/usb/eth/asix.c
> @@ -32,6 +32,7 @@
>  #define AX_CMD_READ_MII_REG		0x07
>  #define AX_CMD_WRITE_MII_REG		0x08
>  #define AX_CMD_SET_HW_MII		0x0a
> +#define AX_CMD_READ_EEPROM		0x0b
>  #define AX_CMD_READ_RX_CTL		0x0f
>  #define AX_CMD_WRITE_RX_CTL		0x10
>  #define AX_CMD_WRITE_IPG0		0x12
> @@ -104,6 +105,8 @@
>  #define FLAG_TYPE_AX88172	(1U << 0)
>  #define FLAG_TYPE_AX88772	(1U << 1)
>  #define FLAG_TYPE_AX88772B	(1U << 2)
> +#define FLAG_EEPROM_MAC		(1U << 3) /* initial mac address in 
eeprom */
> +
>  /* local vars */
>  static int curr_eth_dev; /* index for name of next device detected */
> 
> @@ -337,6 +340,33 @@ static int mii_nway_restart(struct ueth_data *dev)
>  	return r;
>  }
> 
> +static int asix_read_mac(struct eth_device *eth)
> +{
> +	struct ueth_data *dev = (struct ueth_data *)eth->priv;
> +	struct asix_private *priv = (struct asix_private *)dev->dev_priv;
> +	int i;
> +	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, ETH_ALEN);
> +
> +	if (priv->flags & FLAG_EEPROM_MAC) {
> +		for (i = 0; i < (ETH_ALEN >> 1); i++) {
> +			if (asix_read_cmd(dev, AX_CMD_READ_EEPROM,
> +			                  0x04 + i, 0, 2, buf) < 0) {
> +				debug("Failed to read SROM address 04h.\n");
> +				return -1;
> +			}
> +			memcpy((eth->enetaddr + i * 2), buf, 2);
> +		}
> +	} else {
> +		if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf) 
< 0) {
> +			debug("Failed to read MAC address.\n");
> +			return -1;
> +		}
> +		memcpy(eth->enetaddr, buf, ETH_ALEN);
> +	}
> +
> +	return 0;
> +}
> +
>  static int asix_basic_reset(struct ueth_data *dev)
>  {
>  	int embd_phy;
> @@ -391,18 +421,6 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
> 
>  	debug("** %s()\n", __func__);
> 
> -	/* Get the MAC address */
> -	if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
> -				0, 0, ETH_ALEN, buf) < 0) {
> -		debug("Failed to read MAC address.\n");
> -		goto out_err;
> -	}
> -	memcpy(eth->enetaddr, buf, ETH_ALEN);
> -	debug("MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
> -		eth->enetaddr[0], eth->enetaddr[1],
> -		eth->enetaddr[2], eth->enetaddr[3],
> -		eth->enetaddr[4], eth->enetaddr[5]);
> -
>  	dev->phy_id = asix_get_phy_addr(dev);
>  	if (dev->phy_id < 0)
>  		debug("Failed to read phy id\n");
> @@ -682,5 +700,10 @@ int asix_eth_get_info(struct usb_device *dev, struct
> ueth_data *ss, if (asix_basic_reset(ss))
>  		return 0;
> 
> +	/* Get the MAC address */
> +	if (asix_read_mac(eth))
> +		return 0;
> +	debug("MAC %pM\n", eth->enetaddr);
> +
>  	return 1;
>  }

Best regards,
Marek Vasut

[U-Boot] [PATCH 6/6] net: asix: add AX88772B support

[Marek Vasut]

Dear Lucas Stach,

> Add AX88772B ID together with two fixes needed to make this work.
> 
> 1. The packet length check has to be adjusted, as all ASIX chips
> only use 11 bits to indicate the length. AX88772B uses the other
> bits to indicate unrelated things, which cause the check to fail.
> This fix is based on a fix for the Linux kernel by Marek Vasut.
> Linux upstream commit: bca0beb9363f8487ac902931a50eb00180a2d14a
> 
> 2. AX88772B provides several bulk endpoints. Only the first
> IN/OUT endpoints work in the default configuration. So stop
> enumeration after we found them to avoid overwriting the
> endpoint config with a non-working one.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>

Reviewed-by: Marek Vasut <marex@denx.de>
Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/usb/eth/asix.c | 30 +++++++++++++++++++-----------
>  1 Datei ge?ndert, 19 Zeilen hinzugef?gt(+), 11 Zeilen entfernt(-)
> 
> diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> index 6ab3e82..79246d8 100644
> --- a/drivers/usb/eth/asix.c
> +++ b/drivers/usb/eth/asix.c
> @@ -543,13 +543,13 @@ static int asix_recv(struct eth_device *eth)
>  		}
>  		memcpy(&packet_len, buf_ptr, sizeof(packet_len));
>  		le32_to_cpus(&packet_len);
> -		if (((packet_len >> 16) ^ 0xffff) != (packet_len & 0xffff)) {
> +		if (((~packet_len >> 16) & 0x7ff) != (packet_len & 0x7ff)) {
>  			debug("Rx: malformed packet length: %#x (%#x:%#x)\n",
> -			      packet_len, (packet_len >> 16) ^ 0xffff,
> -			      packet_len & 0xffff);
> +			      packet_len, (~packet_len >> 16) & 0x7ff,
> +			      packet_len & 0x7ff);
>  			return -1;
>  		}
> -		packet_len = packet_len & 0xffff;
> +		packet_len = packet_len & 0x7ff;
>  		if (packet_len > actual_len - sizeof(packet_len)) {
>  			debug("Rx: too large packet: %d\n", packet_len);
>  			return -1;
> @@ -597,6 +597,7 @@ static const struct asix_dongle asix_dongles[] = {
>  	{ 0x13b1, 0x0018, FLAG_TYPE_AX88172 },	/* Linksys 200M v2.1 */
>  	{ 0x1557, 0x7720, FLAG_TYPE_AX88772 },	/* 0Q0 cable ethernet */
>  	{ 0x2001, 0x3c05, FLAG_TYPE_AX88772 },	/* DLink DUB-E100 H/W Ver B1
> Alternate */ +	{ 0x0b95, 0x772b, FLAG_TYPE_AX88772B | FLAG_EEPROM_MAC 
},
> /* ASIX 88772B */ { 0x0000, 0x0000, FLAG_NONE }	/* END - Do not remove 
*/
>  };
> 
> @@ -606,6 +607,7 @@ int asix_eth_probe(struct usb_device *dev, unsigned int
> ifnum, {
>  	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 */
> @@ -649,13 +651,19 @@ int asix_eth_probe(struct usb_device *dev, unsigned
> int ifnum, /* is it an BULK endpoint? */
>  		if ((iface->ep_desc[i].bmAttributes &
>  		     USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) {
> -			if (iface->ep_desc[i].bEndpointAddress & USB_DIR_IN)
> -				ss->ep_in = iface->ep_desc[i].bEndpointAddress &
> -					USB_ENDPOINT_NUMBER_MASK;
> -			else
> -				ss->ep_out =
> -					iface->ep_desc[i].bEndpointAddress &
> -					USB_ENDPOINT_NUMBER_MASK;
> +			if (iface->ep_desc[i].bEndpointAddress & USB_DIR_IN) {
> +				if (!ep_in_found) {
> +					ss->ep_in = iface-
>ep_desc[i].bEndpointAddress &
> +					            USB_ENDPOINT_NUMBER_MASK;
> +					ep_in_found = 1;
> +				}
> +			} else {
> +				if (!ep_out_found) {
> +					ss->ep_out = iface-
>ep_desc[i].bEndpointAddress &
> +					             USB_ENDPOINT_NUMBER_MASK;
> +					ep_out_found = 1;
> +				}
> +			}
>  		}
> 
>  		/* is it an interrupt endpoint? */

Best regards,
Marek Vasut

[U-Boot] [PATCH 2/6] net: usbeth: remove misleading warning

[Lucas Stach]

Am Mittwoch, den 22.08.2012, 20:22 +0200 schrieb Marek Vasut:
> Dear Lucas Stach,
> 
> > If the environment doesn't provide a MAC address it's not an
> > error to use the one provided by the eth adapter. Actually it
> > should be the default case if the adapter exports it's own
> > address during get_info().
> 
> I think we should warn if the addr in env doesn't match the one in the device. 
> Otherwise, if the env doesn't contain ethaddr (or eth1addr, eth2addr ...), pull 
> one from the device and inform user.
> 
Sadly it's not obvious from the diff context, but the warning if MAC in
env does not match the one from the device is just above this hunk. Joe
explained to me that the ueth core should always pull the address from
the device and if this succeeds it always sets this address back if none
is found in env. As this is the default case (and we are not actually
changing the MAC of the device) there is no point in bothering the user
with this info.

This is the whole point of this patch. That no one has seen this warning
with the current code is simply because no usb eth driver had
implemented the mac fetch at the right spot. ASIX now does this.

> > So just remove the warning that gets printed in this case.
> > 
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> >  net/eth.c | 2 --
> >  1 Datei ge?ndert, 2 Zeilen entfernt(-)
> > 
> > diff --git a/net/eth.c b/net/eth.c
> > index 1a11ce1..39cd5da 100644
> > --- a/net/eth.c
> > +++ b/net/eth.c
> > @@ -217,8 +217,6 @@ int eth_write_hwaddr(struct eth_device *dev, const char
> > *base_name, } else if (is_valid_ether_addr(dev->enetaddr)) {
> >  		eth_setenv_enetaddr_by_index(base_name, eth_number,
> >  					     dev->enetaddr);
> > -		printf("\nWarning: %s using MAC address from net device\n",
> > -			dev->name);
> >  	}
> > 
> >  	if (dev->write_hwaddr &&
> 
> Best regards,
> Marek Vasut
> 

[U-Boot] [PATCH 2/6] net: usbeth: remove misleading warning

[Joe Hershberger]

Hi Lucas / Marek,

On Wed, Aug 22, 2012 at 1:32 PM, Lucas Stach <dev@lynxeye.de> wrote:
> Am Mittwoch, den 22.08.2012, 20:22 +0200 schrieb Marek Vasut:
>> Dear Lucas Stach,
>>
>> > If the environment doesn't provide a MAC address it's not an
>> > error to use the one provided by the eth adapter. Actually it
>> > should be the default case if the adapter exports it's own
>> > address during get_info().
>>
>> I think we should warn if the addr in env doesn't match the one in the device.
>> Otherwise, if the env doesn't contain ethaddr (or eth1addr, eth2addr ...), pull
>> one from the device and inform user.

Agreed... we also do that.

>>
> Sadly it's not obvious from the diff context, but the warning if MAC in
> env does not match the one from the device is just above this hunk. Joe
> explained to me that the ueth core should always pull the address from
> the device and if this succeeds it always sets this address back if none
> is found in env. As this is the default case (and we are not actually
> changing the MAC of the device) there is no point in bothering the user
> with this info.
>
> This is the whole point of this patch. That no one has seen this warning
> with the current code is simply because no usb eth driver had
> implemented the mac fetch at the right spot. ASIX now does this.
>

That's a good thing.  It's not saying "Error:"... it's a warning.
It's just letting the user know that the MAC is coming from the
EEPROM.

Nak.

-Joe

[U-Boot] [PATCH 1/6] net: introduce transparent driver private in ueth_data

[Joe Hershberger]

Hi Lucas,

On Wed, Aug 22, 2012 at 5:09 AM, Lucas Stach <dev@lynxeye.de> wrote:
> Avoid clutter in ueth_data. Individual drivers should not mess
> with structures belonging to the core like this.
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 3/6] net: asix: split out basic reset function

[Joe Hershberger]

Hi Lucas,

On Wed, Aug 22, 2012 at 5:09 AM, Lucas Stach <dev@lynxeye.de> wrote:
> The basic device reset ensures that the device is ready to
> service commands and does not need to get redone before each
> network operation.
>
> Split out the basic reset from asix_init() and instead call it
> from asix_eth_get_info(), so that it only gets called once.
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 5/6] net: asix: add read_mac function

[Joe Hershberger]

Hi Lucas,

On Wed, Aug 22, 2012 at 5:09 AM, Lucas Stach <dev@lynxeye.de> wrote:
> Initial device MAC should be read while getting info about the
> device, so it's wrong to only read it in asix_init().
>
> Add a dedicated function to read the initial MAC, which is also
> able to handle devices that have their initial MAC stored in
> EEPROM. Call this function inasix_eth_get_info().
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 6/6] net: asix: add AX88772B support

[Joe Hershberger]

Hi Lucas,

On Wed, Aug 22, 2012 at 5:09 AM, Lucas Stach <dev@lynxeye.de> wrote:
> Add AX88772B ID together with two fixes needed to make this work.
>
> 1. The packet length check has to be adjusted, as all ASIX chips
> only use 11 bits to indicate the length. AX88772B uses the other
> bits to indicate unrelated things, which cause the check to fail.
> This fix is based on a fix for the Linux kernel by Marek Vasut.
> Linux upstream commit: bca0beb9363f8487ac902931a50eb00180a2d14a
>
> 2. AX88772B provides several bulk endpoints. Only the first
> IN/OUT endpoints work in the default configuration. So stop
> enumeration after we found them to avoid overwriting the
> endpoint config with a non-working one.
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 4/6] net: asix: add write_hwaddr function

[Joe Hershberger]

Hi Lucas,

On Wed, Aug 22, 2012 at 5:09 AM, Lucas Stach <dev@lynxeye.de> wrote:
> All ASIX chipsets aside from AX88172 are able to set the MAC
> address on the hardware level. Add a function to expose this
> ability.
>
> To differentiate between chip types we now carry flags as driver
> private data. Also while touching the asix_dongles array
> constify this.
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  drivers/usb/eth/asix.c | 61 +++++++++++++++++++++++++++++++++++++++++---------
>  1 Datei ge?ndert, 50 Zeilen hinzugef?gt(+), 11 Zeilen entfernt(-)
>
> diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> index 50cbbbd..ce1483e 100644
> --- a/drivers/usb/eth/asix.c
> +++ b/drivers/usb/eth/asix.c
> @@ -23,6 +23,7 @@
>  #include <usb.h>
>  #include <linux/mii.h>
>  #include "usb_ether.h"
> +#include "malloc.h"
>
>
>  /* ASIX AX8817X based USB 2.0 Ethernet Devices */
> @@ -35,6 +36,7 @@
>  #define AX_CMD_WRITE_RX_CTL            0x10
>  #define AX_CMD_WRITE_IPG0              0x12
>  #define AX_CMD_READ_NODE_ID            0x13
> +#define AX_CMD_WRITE_NODE_ID   0x14
>  #define AX_CMD_READ_PHY_ID             0x19
>  #define AX_CMD_WRITE_MEDIUM_MODE       0x1b
>  #define AX_CMD_WRITE_GPIOS             0x1f
> @@ -97,9 +99,19 @@
>  #define AX_RX_URB_SIZE 2048
>  #define PHY_CONNECT_TIMEOUT 5000
>
> +/* asix_flags defines */
> +#define FLAG_NONE                      0
> +#define FLAG_TYPE_AX88172      (1U << 0)
> +#define FLAG_TYPE_AX88772      (1U << 1)
> +#define FLAG_TYPE_AX88772B     (1U << 2)

Add this flag in the last patch where you add support for this chip.

>  /* local vars */
>  static int curr_eth_dev; /* index for name of next device detected */
>
> +/* driver private */
> +struct asix_private {
> +       int flags;
> +};
> +
>  /*
>   * Asix infrastructure commands
>   */
> @@ -284,6 +296,21 @@ static int asix_write_gpio(struct ueth_data *dev, u16 value, int sleep)
>         return ret;
>  }
>
> +static int asix_write_hwaddr(struct eth_device *eth)
> +{
> +       struct ueth_data *dev = (struct ueth_data *)eth->priv;
> +       int ret;
> +       ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buf, ETH_ALEN);
> +
> +       memcpy(buf, eth->enetaddr, ETH_ALEN);
> +
> +       ret = asix_write_cmd(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN, buf);
> +       if (ret < 0)
> +               debug("Failed to set MAC address: %02x\n", ret);
> +
> +       return ret;
> +}
> +
>  /*
>   * mii commands
>   */
> @@ -539,19 +566,20 @@ void asix_eth_before_probe(void)
>  struct asix_dongle {
>         unsigned short vendor;
>         unsigned short product;
> +       int flags;
>  };
>
> -static struct asix_dongle asix_dongles[] = {
> -       { 0x05ac, 0x1402 },     /* Apple USB Ethernet Adapter */
> -       { 0x07d1, 0x3c05 },     /* D-Link DUB-E100 H/W Ver B1 */
> -       { 0x0b95, 0x772a },     /* Cables-to-Go USB Ethernet Adapter */
> -       { 0x0b95, 0x7720 },     /* Trendnet TU2-ET100 V3.0R */
> -       { 0x0b95, 0x1720 },     /* SMC */
> -       { 0x0db0, 0xa877 },     /* MSI - ASIX 88772a */
> -       { 0x13b1, 0x0018 },     /* Linksys 200M v2.1 */
> -       { 0x1557, 0x7720 },     /* 0Q0 cable ethernet */
> -       { 0x2001, 0x3c05 },     /* DLink DUB-E100 H/W Ver B1 Alternate */
> -       { 0x0000, 0x0000 }      /* END - Do not remove */
> +static const struct asix_dongle asix_dongles[] = {

Agreed with Marek... const array and const elements.

> +       { 0x05ac, 0x1402, FLAG_TYPE_AX88772 },  /* Apple USB Ethernet Adapter */
> +       { 0x07d1, 0x3c05, FLAG_TYPE_AX88772 },  /* D-Link DUB-E100 H/W Ver B1 */
> +       { 0x0b95, 0x772a, FLAG_TYPE_AX88772 },  /* Cables-to-Go USB Ethernet Adapter */
> +       { 0x0b95, 0x7720, FLAG_TYPE_AX88772 },  /* Trendnet TU2-ET100 V3.0R */
> +       { 0x0b95, 0x1720, FLAG_TYPE_AX88172 },  /* SMC */
> +       { 0x0db0, 0xa877, FLAG_TYPE_AX88772 },  /* MSI - ASIX 88772a */
> +       { 0x13b1, 0x0018, FLAG_TYPE_AX88172 },  /* Linksys 200M v2.1 */
> +       { 0x1557, 0x7720, FLAG_TYPE_AX88772 },  /* 0Q0 cable ethernet */
> +       { 0x2001, 0x3c05, FLAG_TYPE_AX88772 },  /* DLink DUB-E100 H/W Ver B1 Alternate */
> +       { 0x0000, 0x0000, FLAG_NONE }   /* END - Do not remove */
>  };
>
>  /* Probe to see if a new device is actually an asix device */
> @@ -588,6 +616,13 @@ int asix_eth_probe(struct usb_device *dev, unsigned int ifnum,
>         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.
> @@ -629,6 +664,8 @@ int asix_eth_probe(struct usb_device *dev, unsigned int ifnum,
>  int asix_eth_get_info(struct usb_device *dev, struct ueth_data *ss,
>                                 struct eth_device *eth)
>  {
> +       struct asix_private *priv = (struct asix_private *)ss->dev_priv;
> +
>         if (!eth) {
>                 debug("%s: missing parameter.\n", __func__);
>                 return 0;
> @@ -638,6 +675,8 @@ int asix_eth_get_info(struct usb_device *dev, struct ueth_data *ss,
>         eth->send = asix_send;
>         eth->recv = asix_recv;
>         eth->halt = asix_halt;
> +       if (!(priv->flags & FLAG_TYPE_AX88172))
> +               eth->write_hwaddr = asix_write_hwaddr;
>         eth->priv = ss;
>
>         if (asix_basic_reset(ss))


Thanks,
-Joe

[U-Boot] [PATCH 3/6] net: asix: split out basic reset function

[Mike Frysinger]

On Wednesday 22 August 2012 06:09:04 Lucas Stach wrote:
> The basic device reset ensures that the device is ready to
> service commands and does not need to get redone before each
> network operation.
> 
> Split out the basic reset from asix_init() and instead call it
> from asix_eth_get_info(), so that it only gets called once.

i'm afraid this is wrong.  the register step (which asix_eth_get_info is 
afaict) should not touch the hardware (other than to extract the MAC address).  
the init func is supposed to bring up the hardware from scratch since the 
expectation is that the halt func brings it down completely.  if it's not 
really possible to completely "bring down" the hardware, then have the 
asix_init func declare a local static int so that it does the "full" bring up 
only once.

so NAK this patch as is
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120822/628fa7d0/attachment.pgp>

[U-Boot] [PATCH 3/6] net: asix: split out basic reset function

[Lucas Stach]

Hi Mike,

Am Mittwoch, den 22.08.2012, 23:01 -0400 schrieb Mike Frysinger:
> On Wednesday 22 August 2012 06:09:04 Lucas Stach wrote:
> > The basic device reset ensures that the device is ready to
> > service commands and does not need to get redone before each
> > network operation.
> > 
> > Split out the basic reset from asix_init() and instead call it
> > from asix_eth_get_info(), so that it only gets called once.
> 
> i'm afraid this is wrong.  the register step (which asix_eth_get_info is 
> afaict) should not touch the hardware (other than to extract the MAC address).  
> the init func is supposed to bring up the hardware from scratch since the 
> expectation is that the halt func brings it down completely.  if it's not 
> really possible to completely "bring down" the hardware, then have the 
> asix_init func declare a local static int so that it does the "full" bring up 
> only once.
> 
> so NAK this patch as is
> -mike

I still think the patch does the right thing. Let's look at the call
chain. When doing a usb start of the controller where the ethernet
device is attached we do the following to register the device:
1. probe (just check if we found suitable hardware, don't touch it yet)
2. get_info (at this point we extract the MAC, which means we have to
bring up the hardware at a basic level, to even be able to touch the
registers)
3. write_hwaddr (eth core sets the MAC address of the device, remember
this is only done _once_ for each device and also needs basic init)

Every network operation basically does first a init() and then a halt().
If we would bring down the device here to a point where it's completely
reset, we would also lose the MAC address set in the register step. This
is clearly not what we want, but also we don't want to set the MAC over
and over again with each network operation. So IMHO halt() should only
bring down the device to a state where the current ethernet connection
is gone. Therefore init() only needs to bring up the ethernet connection
from this state. The basic device initialisation (including the MAC)
should be persistent across multiple network operations.

This is the behaviour this patch implements, aside from halt() not
really doing it's job in the asix driver, but that is for another patch.

Thanks,
Lucas