[PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM

[PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM

[Lucas Stach]

The device comes up with a MAC address of all zeros. We need to read the
initial device MAC from EEPROM so it can be set properly later.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
A similar fix was added into U-Boot:
http://patchwork.ozlabs.org/patch/179409/
---
 drivers/net/usb/asix_devices.c | 29 ++++++++++++++++++++++++++---
 include/linux/usb/usbnet.h     |  1 +
 2 Dateien geändert, 27 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 7a6e758..06f7f7cb 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -422,14 +422,25 @@ static const struct net_device_ops ax88772_netdev_ops = {
 
 static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-	int ret, embd_phy;
+	int ret, embd_phy, i;
 	u8 buf[ETH_ALEN];
 	u32 phyid;
 
 	usbnet_get_endpoints(dev,intf);
 
 	/* Get the MAC address */
-	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
+	if (dev->driver_info->flags & FLAG_EEPROM_MAC) {
+		for (i = 0; i < (ETH_ALEN >> 1); i++) {
+			ret = asix_read_cmd(dev, AX_CMD_READ_EEPROM, 0x04 + i,
+					0, 2, buf + i * 2);
+			if (ret < 0)
+				break;
+		}
+	} else {
+		ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
+				0, 0, ETH_ALEN, buf);
+	}
+
 	if (ret < 0) {
 		netdev_dbg(dev->net, "Failed to read MAC address: %d\n", ret);
 		return ret;
@@ -872,6 +883,18 @@ static const struct driver_info ax88772_info = {
 	.tx_fixup = asix_tx_fixup,
 };
 
+static const struct driver_info ax88772b_info = {
+	.description = "ASIX AX88772B USB 2.0 Ethernet",
+	.bind = ax88772_bind,
+	.status = asix_status,
+	.link_reset = ax88772_link_reset,
+	.reset = ax88772_reset,
+	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
+	         FLAG_MULTI_PACKET | FLAG_EEPROM_MAC,
+	.rx_fixup = asix_rx_fixup,
+	.tx_fixup = asix_tx_fixup,
+};
+
 static const struct driver_info ax88178_info = {
 	.description = "ASIX AX88178 USB 2.0 Ethernet",
 	.bind = ax88178_bind,
@@ -953,7 +976,7 @@ static const struct usb_device_id	products [] = {
 }, {
 	// ASIX AX88772B 10/100
 	USB_DEVICE (0x0b95, 0x772b),
-	.driver_info = (unsigned long) &ax88772_info,
+	.driver_info = (unsigned long) &ax88772b_info,
 }, {
 	// ASIX AX88772 10/100
 	USB_DEVICE (0x0b95, 0x7720),
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9bbeabf..8e9516f 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -106,6 +106,7 @@ struct driver_info {
  */
 #define FLAG_MULTI_PACKET	0x2000
 #define FLAG_RX_ASSEMBLE	0x4000	/* rx packets may span >1 frames */
+#define FLAG_EEPROM_MAC		0x8000	/* initialize device MAC from eeprom */
 
 	/* init device ... can sleep, or cause probe() failure */
 	int	(*bind)(struct usbnet *, struct usb_interface *);
-- 
1.7.11.7

[PATCH 2/2] net: asix: handle packets crossing URB boundaries

[Lucas Stach]

ASIX AX88772B started to pack data even more tightly. Packets and the ASIX packet
header may now cross URB boundaries. To handle this we have to introduce
some state between individual calls to asix_rx_fixup().

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
I've running this patch for some weeks already now and it gets rid of all
the commonly seen rx failures with AX88772B.
---
 drivers/net/usb/asix.h         | 10 ++++++
 drivers/net/usb/asix_common.c  | 81 +++++++++++++++++++++++++++++-------------
 drivers/net/usb/asix_devices.c |  8 +++++
 3 Dateien geändert, 75 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)

diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index e889631..3b4f7a8 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -167,6 +167,16 @@ struct asix_data {
 	u8 res;
 };
 
+struct asix_rx_fixup_info {
+	struct sk_buff *ax_skb;
+	u32 header;
+	u16 size;
+	bool split_head;
+};
+struct asix_private {
+	struct asix_rx_fixup_info rx_fixup_info;
+};
+
 int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 		  u16 size, void *data);
 
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 50d1673..17f9801 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -53,44 +53,77 @@ void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 
 int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
+	struct asix_private *dp = dev->driver_priv;
+	struct asix_rx_fixup_info *rx = &dp->rx_fixup_info;
 	int offset = 0;
 
-	while (offset + sizeof(u32) < skb->len) {
-		struct sk_buff *ax_skb;
-		u16 size;
-		u32 header = get_unaligned_le32(skb->data + offset);
-
-		offset += sizeof(u32);
-
-		/* get the packet length */
-		size = (u16) (header & 0x7ff);
-		if (size != ((~header >> 16) & 0x07ff)) {
-			netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
-			return 0;
+	while (offset + sizeof(u16) <= skb->len) {
+		u16 remaining = 0;
+		unsigned char *data;
+
+		if (!rx->size) {
+			if ((skb->len - offset == sizeof(u16)) ||
+			    rx->split_head) {
+				if(!rx->split_head) {
+					rx->header = get_unaligned_le16(
+							skb->data + offset);
+					rx->split_head = true;
+					offset += sizeof(u16);
+					break;
+				} else {
+					rx->header |= (get_unaligned_le16(
+							skb->data + offset)
+							<< 16);
+					rx->split_head = false;
+					offset += sizeof(u16);
+				}
+			} else {
+				rx->header = get_unaligned_le32(skb->data +
+								offset);
+				offset += sizeof(u32);
+			}
+
+			/* get the packet length */
+			rx->size = (u16) (rx->header & 0x7ff);
+			if (rx->size != ((~rx->header >> 16) & 0x7ff)) {
+				netdev_err(dev->net, "asix_rx_fixup() Bad Header Length 0x%x, offset %d\n",
+					   rx->header, offset);
+				rx->size = 0;
+				return 0;
+			}
+			rx->ax_skb = netdev_alloc_skb_ip_align(dev->net,
+							       rx->size);
+			if (!rx->ax_skb)
+				return 0;
 		}
 
-		if ((size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) ||
-		    (size + offset > skb->len)) {
+		if (rx->size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) {
 			netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
-				   size);
+				   rx->size);
+			kfree_skb(rx->ax_skb);
 			return 0;
 		}
-		ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
-		if (!ax_skb)
-			return 0;
 
-		skb_put(ax_skb, size);
-		memcpy(ax_skb->data, skb->data + offset, size);
-		usbnet_skb_return(dev, ax_skb);
+		if (rx->size > skb->len - offset) {
+			remaining = rx->size - (skb->len - offset);
+			rx->size = skb->len - offset;
+		}
+
+		data = skb_put(rx->ax_skb, rx->size);
+		memcpy(data, skb->data + offset, rx->size);
+		if (!remaining)
+			usbnet_skb_return(dev, rx->ax_skb);
 
-		offset += (size + 1) & 0xfffe;
+		offset += (rx->size + 1) & 0xfffe;
+		rx->size = remaining;
 	}
 
 	if (skb->len != offset) {
-		netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
-			   skb->len);
+		netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d, %d\n",
+			   skb->len, offset);
 		return 0;
 	}
+
 	return 1;
 }
 
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 06f7f7cb..2e1f3ec 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -495,6 +495,10 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->rx_urb_size = 2048;
 	}
 
+	dev->driver_priv = kzalloc(sizeof(struct asix_private), GFP_KERNEL);
+	if (!dev->driver_priv)
+		return -ENOMEM;
+
 	return 0;
 }
 
@@ -829,6 +833,10 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->rx_urb_size = 2048;
 	}
 
+	dev->driver_priv = kzalloc(sizeof(struct asix_private), GFP_KERNEL);
+	if (!dev->driver_priv)
+			return -ENOMEM;
+
 	return 0;
 }
 
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM

[Oliver Neukum]

On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> index 9bbeabf..8e9516f 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -106,6 +106,7 @@ struct driver_info {
>   */
>  #define FLAG_MULTI_PACKET      0x2000
>  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */

Hi,

this looks sensible, but
why are you adding a flag unused in usbnet to usbnet.h?

	Regards
		Oliver

Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM

[Lucas Stach]

Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum:
> On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> > index 9bbeabf..8e9516f 100644
> > --- a/include/linux/usb/usbnet.h
> > +++ b/include/linux/usb/usbnet.h
> > @@ -106,6 +106,7 @@ struct driver_info {
> >   */
> >  #define FLAG_MULTI_PACKET      0x2000
> >  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> > +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */
> 
> Hi,
> 
> this looks sensible, but
> why are you adding a flag unused in usbnet to usbnet.h?

Right, this might not be the right place to add this. Could you point me
to a more appropriate place? The data member of usbnet might be a good
place to stuff this into, but why is this a plain long and not some kind
of pointer? It is used for a different purpose on other ASIX chips
already.

Regards,
Lucas

Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM

[Oliver Neukum]

On Tuesday 18 December 2012 14:24:32 Lucas Stach wrote:
> Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum:
> > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> > > index 9bbeabf..8e9516f 100644
> > > --- a/include/linux/usb/usbnet.h
> > > +++ b/include/linux/usb/usbnet.h
> > > @@ -106,6 +106,7 @@ struct driver_info {
> > >   */
> > >  #define FLAG_MULTI_PACKET      0x2000
> > >  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> > > +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */
> > 
> > Hi,
> > 
> > this looks sensible, but
> > why are you adding a flag unused in usbnet to usbnet.h?
> 
> Right, this might not be the right place to add this. Could you point me
> to a more appropriate place? The data member of usbnet might be a good

driver_priv is intended for such stuff

> place to stuff this into, but why is this a plain long and not some kind
> of pointer? It is used for a different purpose on other ASIX chips
> already.

But there is another pointer.

	Regards
		Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM

[Lucas Stach]

Am Dienstag, den 18.12.2012, 14:33 +0100 schrieb Oliver Neukum:
> On Tuesday 18 December 2012 14:24:32 Lucas Stach wrote:
> > Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum:
> > > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> > > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> > > > index 9bbeabf..8e9516f 100644
> > > > --- a/include/linux/usb/usbnet.h
> > > > +++ b/include/linux/usb/usbnet.h
> > > > @@ -106,6 +106,7 @@ struct driver_info {
> > > >   */
> > > >  #define FLAG_MULTI_PACKET      0x2000
> > > >  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> > > > +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */
> > > 
> > > Hi,
> > > 
> > > this looks sensible, but
> > > why are you adding a flag unused in usbnet to usbnet.h?
> > 
> > Right, this might not be the right place to add this. Could you point me
> > to a more appropriate place? The data member of usbnet might be a good
> 
> driver_priv is intended for such stuff
> 
I'm not talking about the usbnet struct, but the driver_info struct. I
need a way to pass this flag from the static driver info to the bind
function. I don't even have a usbnet device at this point, where I could
hang on driver_priv data.

Regards,
Lucas

Re: [PATCH 2/2] net: asix: handle packets crossing URB boundaries

[Oliver Neukum]

On Tuesday 18 December 2012 13:10:26 Lucas Stach wrote:
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -495,6 +495,10 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>                 dev->rx_urb_size = 2048;
>         }
>  
> +       dev->driver_priv = kzalloc(sizeof(struct asix_private), GFP_KERNEL);
> +       if (!dev->driver_priv)
> +               return -ENOMEM;
> +
>         return 0;

Where is this freed?

	Regards
		Oliver

Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM

[Oliver Neukum]

On Tuesday 18 December 2012 14:38:32 Lucas Stach wrote:
> Am Dienstag, den 18.12.2012, 14:33 +0100 schrieb Oliver Neukum:
> > On Tuesday 18 December 2012 14:24:32 Lucas Stach wrote:
> > > Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum:
> > > > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> > > > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> > > > > index 9bbeabf..8e9516f 100644
> > > > > --- a/include/linux/usb/usbnet.h
> > > > > +++ b/include/linux/usb/usbnet.h
> > > > > @@ -106,6 +106,7 @@ struct driver_info {
> > > > >   */
> > > > >  #define FLAG_MULTI_PACKET      0x2000
> > > > >  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> > > > > +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */
> > > > 
> > > > Hi,
> > > > 
> > > > this looks sensible, but
> > > > why are you adding a flag unused in usbnet to usbnet.h?
> > > 
> > > Right, this might not be the right place to add this. Could you point me
> > > to a more appropriate place? The data member of usbnet might be a good
> > 
> > driver_priv is intended for such stuff
> > 
> I'm not talking about the usbnet struct, but the driver_info struct. I
> need a way to pass this flag from the static driver info to the bind
> function. I don't even have a usbnet device at this point, where I could
> hang on driver_priv data.

True, sorry I misread the patch. You could split up ax88772_bind()
implicitly encoding the flag. That would be reasonably clean. If you
really don't want to do that, please add another private field. But
we cannot pollute the flags.

	Regards
		Oliver