All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] net: asix: Add support for AX88772B
@ 2014-10-09  3:43 Alexandre Courbot
  2014-10-09  5:56 ` Simon Glass
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Alexandre Courbot @ 2014-10-09  3:43 UTC (permalink / raw)
  To: u-boot

This USB device works as-is on this driver.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---

 drivers/usb/eth/asix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
index 6557055e02b8..11811094ede8 100644
--- a/drivers/usb/eth/asix.c
+++ b/drivers/usb/eth/asix.c
@@ -580,6 +580,7 @@ static const struct asix_dongle asix_dongles[] = {
 	{ 0x2001, 0x3c05, FLAG_TYPE_AX88772 },
 	/* ASIX 88772B */
 	{ 0x0b95, 0x772b, FLAG_TYPE_AX88772B | FLAG_EEPROM_MAC },
+	{ 0x0b95, 0x7e2b, FLAG_TYPE_AX88772B },
 	{ 0x0000, 0x0000, FLAG_NONE }	/* END - Do not remove */
 };
 
-- 
2.1.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: asix: Add support for AX88772B
  2014-10-09  3:43 [U-Boot] [PATCH] net: asix: Add support for AX88772B Alexandre Courbot
@ 2014-10-09  5:56 ` Simon Glass
  2014-10-09 15:10 ` Marek Vasut
  2014-10-22 18:17 ` Marek Vasut
  2 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2014-10-09  5:56 UTC (permalink / raw)
  To: u-boot

On 8 October 2014 21:43, Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> This USB device works as-is on this driver.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>
>  drivers/usb/eth/asix.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> index 6557055e02b8..11811094ede8 100644
> --- a/drivers/usb/eth/asix.c
> +++ b/drivers/usb/eth/asix.c
> @@ -580,6 +580,7 @@ static const struct asix_dongle asix_dongles[] = {
>         { 0x2001, 0x3c05, FLAG_TYPE_AX88772 },
>         /* ASIX 88772B */
>         { 0x0b95, 0x772b, FLAG_TYPE_AX88772B | FLAG_EEPROM_MAC },
> +       { 0x0b95, 0x7e2b, FLAG_TYPE_AX88772B },
>         { 0x0000, 0x0000, FLAG_NONE }   /* END - Do not remove */
>  };
>
> --
> 2.1.2
>

Well that seems OK then.

Acked-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: asix: Add support for AX88772B
  2014-10-09  3:43 [U-Boot] [PATCH] net: asix: Add support for AX88772B Alexandre Courbot
  2014-10-09  5:56 ` Simon Glass
@ 2014-10-09 15:10 ` Marek Vasut
  2014-10-22  5:03   ` Alexandre Courbot
  2014-10-22 18:17 ` Marek Vasut
  2 siblings, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2014-10-09 15:10 UTC (permalink / raw)
  To: u-boot

On Thursday, October 09, 2014 at 05:43:30 AM, Alexandre Courbot wrote:
> This USB device works as-is on this driver.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

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

It's a network device, but it's also USB. Joe, do you want to pick
this up please? Alexandre, please keep an eye on the patch and if
it doesn't get picked up for a week or so, poke me and I'll pick
it through USB.

I would be even OK if such a trivial thing made it into 2014.10, but
I'm not in position to vouch on that right now.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: asix: Add support for AX88772B
  2014-10-09 15:10 ` Marek Vasut
@ 2014-10-22  5:03   ` Alexandre Courbot
  0 siblings, 0 replies; 9+ messages in thread
From: Alexandre Courbot @ 2014-10-22  5:03 UTC (permalink / raw)
  To: u-boot

Hi Marek,

I am still not seeing this patch in mainline ; maybe you will want to 
take it as you suggested?

Thanks,
Alex.

On 10/10/2014 12:10 AM, Marek Vasut wrote:
> On Thursday, October 09, 2014 at 05:43:30 AM, Alexandre Courbot wrote:
>> This USB device works as-is on this driver.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Acked-by: Marek Vasut <marex@denx.de>
>
> It's a network device, but it's also USB. Joe, do you want to pick
> this up please? Alexandre, please keep an eye on the patch and if
> it doesn't get picked up for a week or so, poke me and I'll pick
> it through USB.
>
> I would be even OK if such a trivial thing made it into 2014.10, but
> I'm not in position to vouch on that right now.
>
> Best regards,
> Marek Vasut
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: asix: Add support for AX88772B
  2014-10-09  3:43 [U-Boot] [PATCH] net: asix: Add support for AX88772B Alexandre Courbot
  2014-10-09  5:56 ` Simon Glass
  2014-10-09 15:10 ` Marek Vasut
@ 2014-10-22 18:17 ` Marek Vasut
  2 siblings, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2014-10-22 18:17 UTC (permalink / raw)
  To: u-boot

On Thursday, October 09, 2014 at 05:43:30 AM, Alexandre Courbot wrote:
> This USB device works as-is on this driver.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> 
>  drivers/usb/eth/asix.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> index 6557055e02b8..11811094ede8 100644
> --- a/drivers/usb/eth/asix.c
> +++ b/drivers/usb/eth/asix.c
> @@ -580,6 +580,7 @@ static const struct asix_dongle asix_dongles[] = {
>  	{ 0x2001, 0x3c05, FLAG_TYPE_AX88772 },
>  	/* ASIX 88772B */
>  	{ 0x0b95, 0x772b, FLAG_TYPE_AX88772B | FLAG_EEPROM_MAC },
> +	{ 0x0b95, 0x7e2b, FLAG_TYPE_AX88772B },
>  	{ 0x0000, 0x0000, FLAG_NONE }	/* END - Do not remove */
>  };

Applied, thanks!

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: asix: add support for AX88772B
  2012-08-20 14:36   ` Lucas Stach
@ 2012-08-20 15:15     ` Marek Vasut
  0 siblings, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2012-08-20 15:15 UTC (permalink / raw)
  To: u-boot

Dear Lucas Stach,

> Hello Marek,
> 
> Am Montag, den 20.08.2012, 16:13 +0200 schrieb Marek Vasut:
> > Dear Lucas Stach,
> > 
> > > There are multiple changes needed to make AX88772B work:
> > > 
> > > 1. add vendor and product ID (trivial)
> > > 
> > > 2. We need to read out the MAC address from the EEPROM and write
> > > it into the NodeID register manually.
> > > 
> > > 3. 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
> > 
> > Yea, I enjoyed my share with this one. Though the linux patch still
> > doesn't work on my hardware completely ... I get some random trouble. Do
> > you get any mess in dmesg?
> 
> Currently I'm mostly doing U-Boot work, so I didn't test Linux more than
> some simple boot tests.

Ok, it might have been just my hardware though.

> > > 4. 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.
> > 
> > ;-)
> > 
> > > This was tested to work on a Colibri T20 board.
> > > Patch is based on u-boot-net/next.
> > > 
> > > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > > ---
> > > 
> > >  drivers/usb/eth/asix.c | 62
> > > 
> > > +++++++++++++++++++++++++++++++++++++++++--------- 1 Datei ge?ndert, 51
> > > Zeilen hinzugef?gt(+), 11 Zeilen entfernt(-)
> > > 
> > > diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> > > index 8fb7fc8..86e0f0f 100644
> > > --- a/drivers/usb/eth/asix.c
> > > +++ b/drivers/usb/eth/asix.c
> > > @@ -31,10 +31,12 @@
> > > 
> > >  #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
> > >  #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
> > > 
> > > @@ -322,6 +324,7 @@ static int asix_init(struct eth_device *eth, bd_t
> > > *bd)
> > > 
> > >  	int timeout = 0;
> > >  
> > >  #define TIMEOUT_RESOLUTION 50	/* ms */
> > >  
> > >  	int link_detected;
> > > 
> > > +	int i;
> > > 
> > >  	debug("** %s()\n", __func__);
> > > 
> > > @@ -359,6 +362,35 @@ static int asix_init(struct eth_device *eth, bd_t
> > > *bd)
> > > 
> > >  	rx_ctl = asix_read_rx_ctl(dev);
> > >  	debug("RX_CTL is 0x%04x setting to 0x0000\n", rx_ctl);
> > > 
> > > +	/*
> > > +	 * For AX88772B to work we have to read the MAC address from the
> > > EEPROM +	 * and set the node ID manually.
> > > +	 */
> > > +	if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) &&
> > > +	    (dev->pusb_dev->descriptor.idProduct == 0x772b)) {
> > > +
> > > +		ALLOC_CACHE_ALIGN_BUFFER(unsigned char, read_buf, 2);
> > > +		memset(buf, 0, ETH_ALEN);
> > > +
> > > +		for (i = 0; i < (ETH_ALEN >> 1); i++) {
> > > +			memset(read_buf, 0, 2);
> > > +			if (asix_read_cmd(dev, AX_CMD_READ_EEPROM,
> > > +			                  0x04 + i, 0, 2, read_buf)) {
> > > +				debug("Failed to read SROM address 04h.\n");
> > > +				goto out_err;
> > > +			}
> > > +			memcpy((buf + i*2), read_buf, 2);
> > > +		}
> > > +
> > > +		debug("MAC read from EEPROM: %02x:%02x:%02x:%02x:%02x:%02x\n",
> > > +		       buf[0], buf[1], buf[2], buf[3], buf[4], buf[5]);
> > > +
> > > +		if (asix_write_cmd(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN,
> > 
> > buf)) {
> > 
> > > +			printf("Failed to set MAC address.\n");
> > > +			goto out_err;
> > > +		}
> > > +	}
> > 
> > Can this be split into separate function?
> 
> I thought about this, but decided not to do so as I felt it doesn't do
> much harm in there. But I can certainly change this if you think it
> really should be split out.

It should, since there'll eventually land other such cards with this issue or 
other issues.

> > >  	/* Get the MAC address */
> > >  	if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
> > >  	
> > >  				0, 0, ETH_ALEN, buf) < 0) {
> > > 
> > > @@ -493,13 +525,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;
> > > 
> > > @@ -546,6 +578,7 @@ static struct asix_dongle asix_dongles[] = {
> > > 
> > >  	{ 0x13b1, 0x0018 },	/* Linksys 200M v2.1 */
> > >  	{ 0x1557, 0x7720 },	/* 0Q0 cable ethernet */
> > >  	{ 0x2001, 0x3c05 },	/* DLink DUB-E100 H/W Ver B1 Alternate */
> > > 
> > > +	{ 0x0b95, 0x772b },	/* ASIX 88772B */
> > 
> > Maybe this list should be extended by caps as it is in linux?
> 
> Hm, I don't know what that would buy us. It's not like we are trying to
> make use of the maximum feature set of each chip to get the last bit of
> performance. IMHO it's better to just use a safe subset of the features
> to keep this driver small and understandable.

That's right, but this chip is really weird, right? It's not like the others, so 
we should flag it behaves slightly different. What do you think?

> > >  	{ 0x0000, 0x0000 }	/* END - Do not remove */
> > >  
> > >  };
> > > 
> > > @@ -555,6 +588,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 */
> > > 
> > > @@ -591,13 +625,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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: asix: add support for AX88772B
  2012-08-20 14:13 ` Marek Vasut
@ 2012-08-20 14:36   ` Lucas Stach
  2012-08-20 15:15     ` Marek Vasut
  0 siblings, 1 reply; 9+ messages in thread
From: Lucas Stach @ 2012-08-20 14:36 UTC (permalink / raw)
  To: u-boot

Hello Marek,

Am Montag, den 20.08.2012, 16:13 +0200 schrieb Marek Vasut:
> Dear Lucas Stach,
> 
> > There are multiple changes needed to make AX88772B work:
> > 
> > 1. add vendor and product ID (trivial)
> > 
> > 2. We need to read out the MAC address from the EEPROM and write
> > it into the NodeID register manually.
> > 
> > 3. 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
> 
> Yea, I enjoyed my share with this one. Though the linux patch still doesn't work 
> on my hardware completely ... I get some random trouble. Do you get any mess in 
> dmesg?
> 
Currently I'm mostly doing U-Boot work, so I didn't test Linux more than
some simple boot tests.

> > 4. 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.
> 
> ;-)
> 
> > This was tested to work on a Colibri T20 board.
> > Patch is based on u-boot-net/next.
> > 
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> >  drivers/usb/eth/asix.c | 62
> > +++++++++++++++++++++++++++++++++++++++++--------- 1 Datei ge?ndert, 51
> > Zeilen hinzugef?gt(+), 11 Zeilen entfernt(-)
> > 
> > diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> > index 8fb7fc8..86e0f0f 100644
> > --- a/drivers/usb/eth/asix.c
> > +++ b/drivers/usb/eth/asix.c
> > @@ -31,10 +31,12 @@
> >  #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
> >  #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
> > @@ -322,6 +324,7 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
> >  	int timeout = 0;
> >  #define TIMEOUT_RESOLUTION 50	/* ms */
> >  	int link_detected;
> > +	int i;
> > 
> >  	debug("** %s()\n", __func__);
> > 
> > @@ -359,6 +362,35 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
> >  	rx_ctl = asix_read_rx_ctl(dev);
> >  	debug("RX_CTL is 0x%04x setting to 0x0000\n", rx_ctl);
> > 
> > +	/*
> > +	 * For AX88772B to work we have to read the MAC address from the EEPROM
> > +	 * and set the node ID manually.
> > +	 */
> > +	if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) &&
> > +	    (dev->pusb_dev->descriptor.idProduct == 0x772b)) {
> > +
> > +		ALLOC_CACHE_ALIGN_BUFFER(unsigned char, read_buf, 2);
> > +		memset(buf, 0, ETH_ALEN);
> > +
> > +		for (i = 0; i < (ETH_ALEN >> 1); i++) {
> > +			memset(read_buf, 0, 2);
> > +			if (asix_read_cmd(dev, AX_CMD_READ_EEPROM,
> > +			                  0x04 + i, 0, 2, read_buf)) {
> > +				debug("Failed to read SROM address 04h.\n");
> > +				goto out_err;
> > +			}
> > +			memcpy((buf + i*2), read_buf, 2);
> > +		}
> > +
> > +		debug("MAC read from EEPROM: %02x:%02x:%02x:%02x:%02x:%02x\n",
> > +		       buf[0], buf[1], buf[2], buf[3], buf[4], buf[5]);
> > +
> > +		if (asix_write_cmd(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN, 
> buf)) {
> > +			printf("Failed to set MAC address.\n");
> > +			goto out_err;
> > +		}
> > +	}
> 
> Can this be split into separate function?
> 
I thought about this, but decided not to do so as I felt it doesn't do
much harm in there. But I can certainly change this if you think it
really should be split out.

> >  	/* Get the MAC address */
> >  	if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
> >  				0, 0, ETH_ALEN, buf) < 0) {
> > @@ -493,13 +525,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;
> > @@ -546,6 +578,7 @@ static struct asix_dongle asix_dongles[] = {
> >  	{ 0x13b1, 0x0018 },	/* Linksys 200M v2.1 */
> >  	{ 0x1557, 0x7720 },	/* 0Q0 cable ethernet */
> >  	{ 0x2001, 0x3c05 },	/* DLink DUB-E100 H/W Ver B1 Alternate */
> > +	{ 0x0b95, 0x772b },	/* ASIX 88772B */
> 
> Maybe this list should be extended by caps as it is in linux?
> 
Hm, I don't know what that would buy us. It's not like we are trying to
make use of the maximum feature set of each chip to get the last bit of
performance. IMHO it's better to just use a safe subset of the features
to keep this driver small and understandable.

> >  	{ 0x0000, 0x0000 }	/* END - Do not remove */
> >  };
> > 
> > @@ -555,6 +588,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 */
> > @@ -591,13 +625,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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: asix: add support for AX88772B
  2012-08-20 14:07 [U-Boot] [PATCH] net: asix: add " Lucas Stach
@ 2012-08-20 14:13 ` Marek Vasut
  2012-08-20 14:36   ` Lucas Stach
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2012-08-20 14:13 UTC (permalink / raw)
  To: u-boot

Dear Lucas Stach,

> There are multiple changes needed to make AX88772B work:
> 
> 1. add vendor and product ID (trivial)
> 
> 2. We need to read out the MAC address from the EEPROM and write
> it into the NodeID register manually.
> 
> 3. 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

Yea, I enjoyed my share with this one. Though the linux patch still doesn't work 
on my hardware completely ... I get some random trouble. Do you get any mess in 
dmesg?

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

;-)

> This was tested to work on a Colibri T20 board.
> Patch is based on u-boot-net/next.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  drivers/usb/eth/asix.c | 62
> +++++++++++++++++++++++++++++++++++++++++--------- 1 Datei ge?ndert, 51
> Zeilen hinzugef?gt(+), 11 Zeilen entfernt(-)
> 
> diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> index 8fb7fc8..86e0f0f 100644
> --- a/drivers/usb/eth/asix.c
> +++ b/drivers/usb/eth/asix.c
> @@ -31,10 +31,12 @@
>  #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
>  #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
> @@ -322,6 +324,7 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
>  	int timeout = 0;
>  #define TIMEOUT_RESOLUTION 50	/* ms */
>  	int link_detected;
> +	int i;
> 
>  	debug("** %s()\n", __func__);
> 
> @@ -359,6 +362,35 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
>  	rx_ctl = asix_read_rx_ctl(dev);
>  	debug("RX_CTL is 0x%04x setting to 0x0000\n", rx_ctl);
> 
> +	/*
> +	 * For AX88772B to work we have to read the MAC address from the EEPROM
> +	 * and set the node ID manually.
> +	 */
> +	if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) &&
> +	    (dev->pusb_dev->descriptor.idProduct == 0x772b)) {
> +
> +		ALLOC_CACHE_ALIGN_BUFFER(unsigned char, read_buf, 2);
> +		memset(buf, 0, ETH_ALEN);
> +
> +		for (i = 0; i < (ETH_ALEN >> 1); i++) {
> +			memset(read_buf, 0, 2);
> +			if (asix_read_cmd(dev, AX_CMD_READ_EEPROM,
> +			                  0x04 + i, 0, 2, read_buf)) {
> +				debug("Failed to read SROM address 04h.\n");
> +				goto out_err;
> +			}
> +			memcpy((buf + i*2), read_buf, 2);
> +		}
> +
> +		debug("MAC read from EEPROM: %02x:%02x:%02x:%02x:%02x:%02x\n",
> +		       buf[0], buf[1], buf[2], buf[3], buf[4], buf[5]);
> +
> +		if (asix_write_cmd(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN, 
buf)) {
> +			printf("Failed to set MAC address.\n");
> +			goto out_err;
> +		}
> +	}

Can this be split into separate function?

>  	/* Get the MAC address */
>  	if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
>  				0, 0, ETH_ALEN, buf) < 0) {
> @@ -493,13 +525,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;
> @@ -546,6 +578,7 @@ static struct asix_dongle asix_dongles[] = {
>  	{ 0x13b1, 0x0018 },	/* Linksys 200M v2.1 */
>  	{ 0x1557, 0x7720 },	/* 0Q0 cable ethernet */
>  	{ 0x2001, 0x3c05 },	/* DLink DUB-E100 H/W Ver B1 Alternate */
> +	{ 0x0b95, 0x772b },	/* ASIX 88772B */

Maybe this list should be extended by caps as it is in linux?

>  	{ 0x0000, 0x0000 }	/* END - Do not remove */
>  };
> 
> @@ -555,6 +588,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 */
> @@ -591,13 +625,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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: asix: add support for AX88772B
@ 2012-08-20 14:07 Lucas Stach
  2012-08-20 14:13 ` Marek Vasut
  0 siblings, 1 reply; 9+ messages in thread
From: Lucas Stach @ 2012-08-20 14:07 UTC (permalink / raw)
  To: u-boot

There are multiple changes needed to make AX88772B work:

1. add vendor and product ID (trivial)

2. We need to read out the MAC address from the EEPROM and write
it into the NodeID register manually.

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

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

This was tested to work on a Colibri T20 board.
Patch is based on u-boot-net/next.

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

diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
index 8fb7fc8..86e0f0f 100644
--- a/drivers/usb/eth/asix.c
+++ b/drivers/usb/eth/asix.c
@@ -31,10 +31,12 @@
 #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
 #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
@@ -322,6 +324,7 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
 	int timeout = 0;
 #define TIMEOUT_RESOLUTION 50	/* ms */
 	int link_detected;
+	int i;
 
 	debug("** %s()\n", __func__);
 
@@ -359,6 +362,35 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
 	rx_ctl = asix_read_rx_ctl(dev);
 	debug("RX_CTL is 0x%04x setting to 0x0000\n", rx_ctl);
 
+	/*
+	 * For AX88772B to work we have to read the MAC address from the EEPROM
+	 * and set the node ID manually.
+	 */
+	if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) &&
+	    (dev->pusb_dev->descriptor.idProduct == 0x772b)) {
+
+		ALLOC_CACHE_ALIGN_BUFFER(unsigned char, read_buf, 2);
+		memset(buf, 0, ETH_ALEN);
+
+		for (i = 0; i < (ETH_ALEN >> 1); i++) {
+			memset(read_buf, 0, 2);
+			if (asix_read_cmd(dev, AX_CMD_READ_EEPROM,
+			                  0x04 + i, 0, 2, read_buf)) {
+				debug("Failed to read SROM address 04h.\n");
+				goto out_err;
+			}
+			memcpy((buf + i*2), read_buf, 2);
+		}
+
+		debug("MAC read from EEPROM: %02x:%02x:%02x:%02x:%02x:%02x\n",
+		       buf[0], buf[1], buf[2], buf[3], buf[4], buf[5]);
+
+		if (asix_write_cmd(dev, AX_CMD_WRITE_NODE_ID, 0, 0, ETH_ALEN, buf)) {
+			printf("Failed to set MAC address.\n");
+			goto out_err;
+		}
+	}
+
 	/* Get the MAC address */
 	if (asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
 				0, 0, ETH_ALEN, buf) < 0) {
@@ -493,13 +525,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;
@@ -546,6 +578,7 @@ static struct asix_dongle asix_dongles[] = {
 	{ 0x13b1, 0x0018 },	/* Linksys 200M v2.1 */
 	{ 0x1557, 0x7720 },	/* 0Q0 cable ethernet */
 	{ 0x2001, 0x3c05 },	/* DLink DUB-E100 H/W Ver B1 Alternate */
+	{ 0x0b95, 0x772b },	/* ASIX 88772B */
 	{ 0x0000, 0x0000 }	/* END - Do not remove */
 };
 
@@ -555,6 +588,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 */
@@ -591,13 +625,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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-10-22 18:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-09  3:43 [U-Boot] [PATCH] net: asix: Add support for AX88772B Alexandre Courbot
2014-10-09  5:56 ` Simon Glass
2014-10-09 15:10 ` Marek Vasut
2014-10-22  5:03   ` Alexandre Courbot
2014-10-22 18:17 ` Marek Vasut
  -- strict thread matches above, loose matches on Subject: below --
2012-08-20 14:07 [U-Boot] [PATCH] net: asix: add " Lucas Stach
2012-08-20 14:13 ` Marek Vasut
2012-08-20 14:36   ` Lucas Stach
2012-08-20 15:15     ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.