net: asix: Fix ASIX 88772B with driver model

PATCH New Alban Bedel 3 participants 13 messages Aug 3, 2016 – Aug 11, 2016 USB

Share & tools

Alban Bedel

author

2016-08-03 05:32 UTC #0 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Commit 147271209a9d ("net: asix: fix operation without eeprom") added a special handling for ASIX 88772B that enable another type of header. This break the driver in DM mode as the extra handling needed in the receive path is missing.

However this new header mode is not required and only seems to increase the code complexity, so this patch revert this part of commit 147271209a9d.

Change-Id I7edc89f5714ead60e5204340ba05caf21b155593

Fixes 147271209a9d ("net: asix: fix operation without eeprom")

Signed-off-by Alban Bedel <[email protected]>

---
 drivers/usb/eth/asix.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

drivers/usb/eth/asix.c +3 ~~-19~~

index ad083cf8ae4a..1c6e967db1a0 100644
@@ -67,11 +67,8 @@
 	 AX_MEDIUM_AC | AX_MEDIUM_RE)
 
 /* AX88772 & AX88178 RX_CTL values */
-#define AX_RX_CTL_RH2M		0x0200	/* 32-bit aligned RX IP header */
-#define AX_RX_CTL_RH1M		0x0100	/* Enable RX header format type 1 */
-#define AX_RX_CTL_SO		0x0080
-#define AX_RX_CTL_AB		0x0008
-#define AX_RX_HEADER_DEFAULT	(AX_RX_CTL_RH1M | AX_RX_CTL_RH2M)
+#define AX_RX_CTL_SO			0x0080
+#define AX_RX_CTL_AB			0x0008
 
 #define AX_DEFAULT_RX_CTL	\
 	(AX_RX_CTL_SO | AX_RX_CTL_AB)

@@ -98,8 +95,6 @@
 #define FLAG_TYPE_AX88772B	(1U << 2)
 #define FLAG_EEPROM_MAC		(1U << 3) /* initial mac address in eeprom */
 
-#define ASIX_USB_VENDOR_ID	0x0b95
-#define AX88772B_USB_PRODUCT_ID	0x772b
 
 /* driver private */
 struct asix_private {

@@ -431,15 +426,10 @@ static int asix_init_common(struct ueth_data *dev, uint8_t *enetaddr)
 	int timeout = 0;
 #define TIMEOUT_RESOLUTION 50	/* ms */
 	int link_detected;
-	u32 ctl = AX_DEFAULT_RX_CTL;
 
 	debug("** %s()\n", __func__);
 
-	if ((dev->pusb_dev->descriptor.idVendor == ASIX_USB_VENDOR_ID) &&
-	    (dev->pusb_dev->descriptor.idProduct == AX88772B_USB_PRODUCT_ID))
-		ctl |= AX_RX_HEADER_DEFAULT;
-
-	if (asix_write_rx_ctl(dev, ctl) < 0)
+	if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0)
 		goto out_err;
 
 	if (asix_write_hwaddr_common(dev, enetaddr) < 0)

@@ -572,12 +562,6 @@ static int asix_recv(struct eth_device *eth)
 			return -1;
 		}
 
-		if ((dev->pusb_dev->descriptor.idVendor ==
-		     ASIX_USB_VENDOR_ID) &&
-		    (dev->pusb_dev->descriptor.idProduct ==
-		     AX88772B_USB_PRODUCT_ID))
-			buf_ptr += 2;
-
 		/* Notify net stack */
 		net_process_received_packet(buf_ptr + sizeof(packet_len),
 					    packet_len);

Signature · 1 line

2.9.2

Marek Vasut

2016-08-03 07:00 UTC #1 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 4 lines · Alban Bedel

Commit 147271209a9d ("net: asix: fix operation without eeprom") added a special handling for ASIX 88772B that enable another type of header. This break the driver in DM mode as the extra handling needed in the receive path is missing.

So add the extra handling ?

Quoted 3 lines

However this new header mode is not required and only seems to increase the code complexity, so this patch revert this part of commit 147271209a9d.

Why is it not required ?

Quoted 65 lines

Change-Id I7edc89f5714ead60e5204340ba05caf21b155593

Fixes 147271209a9d ("net: asix: fix operation without eeprom")

Signed-off-by Alban Bedel <[email protected]>

---
 drivers/usb/eth/asix.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

drivers/usb/eth/asix.c +3 ~~-19~~

index ad083cf8ae4a..1c6e967db1a0 100644
@@ -67,11 +67,8 @@
 	 AX_MEDIUM_AC | AX_MEDIUM_RE)
 
 /* AX88772 & AX88178 RX_CTL values */
-#define AX_RX_CTL_RH2M		0x0200	/* 32-bit aligned RX IP header */
-#define AX_RX_CTL_RH1M		0x0100	/* Enable RX header format type 1 */
-#define AX_RX_CTL_SO		0x0080
-#define AX_RX_CTL_AB		0x0008
-#define AX_RX_HEADER_DEFAULT	(AX_RX_CTL_RH1M | AX_RX_CTL_RH2M)
+#define AX_RX_CTL_SO			0x0080
+#define AX_RX_CTL_AB			0x0008
 
 #define AX_DEFAULT_RX_CTL	\
 	(AX_RX_CTL_SO | AX_RX_CTL_AB)

@@ -98,8 +95,6 @@
 #define FLAG_TYPE_AX88772B	(1U << 2)
 #define FLAG_EEPROM_MAC		(1U << 3) /* initial mac address in eeprom */
 
-#define ASIX_USB_VENDOR_ID	0x0b95
-#define AX88772B_USB_PRODUCT_ID	0x772b
 
 /* driver private */
 struct asix_private {

@@ -431,15 +426,10 @@ static int asix_init_common(struct ueth_data *dev, uint8_t *enetaddr)
 	int timeout = 0;
 #define TIMEOUT_RESOLUTION 50	/* ms */
 	int link_detected;
-	u32 ctl = AX_DEFAULT_RX_CTL;
 
 	debug("** %s()\n", __func__);
 
-	if ((dev->pusb_dev->descriptor.idVendor == ASIX_USB_VENDOR_ID) &&
-	    (dev->pusb_dev->descriptor.idProduct == AX88772B_USB_PRODUCT_ID))
-		ctl |= AX_RX_HEADER_DEFAULT;
-
-	if (asix_write_rx_ctl(dev, ctl) < 0)
+	if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0)
 		goto out_err;
 
 	if (asix_write_hwaddr_common(dev, enetaddr) < 0)

@@ -572,12 +562,6 @@ static int asix_recv(struct eth_device *eth)
 			return -1;
 		}
 
-		if ((dev->pusb_dev->descriptor.idVendor ==
-		     ASIX_USB_VENDOR_ID) &&
-		    (dev->pusb_dev->descriptor.idProduct ==
-		     AX88772B_USB_PRODUCT_ID))
-			buf_ptr += 2;
-
 		/* Notify net stack */
 		net_process_received_packet(buf_ptr + sizeof(packet_len),
 					    packet_len);

Signature · 2 lines

Best regards,
Marek Vasut

Alban Bedel

author

2016-08-03 09:46 UTC #2 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 7 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:

Quoted 4 lines · Alban Bedel
Commit 147271209a9d ("net: asix: fix operation without eeprom") added a special handling for ASIX 88772B that enable another type of header. This break the driver in DM mode as the extra handling needed in the receive path is missing.
So add the extra handling ?

I can do that too, but I though u-boot preferred to avoid useless code.

Quoted 5 lines

Quoted 3 lines
However this new header mode is not required and only seems to increase the code complexity, so this patch revert this part of commit 147271209a9d.
Why is it not required ?

It works fine without, since 2012. In fact this change is not even mentioned in the log of commit 147271209a9d, so I really don't know why it was added in the first place. As can be seen in the revert all it does is adding 2 bytes to the USB packets that are then just skipped. Seems pretty useless to me.

Alban
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160803/eb919e51/attachment.sig>

Marek Vasut

2016-08-03 13:51 UTC #3 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 4 lines · Alban Bedel

Quoted 1 line · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
On 08/03/2016 07:32 AM, Alban Bedel wrote:

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
Commit 147271209a9d ("net: asix: fix operation without eeprom") added a special handling for ASIX 88772B that enable another type of header. This break the driver in DM mode as the extra handling needed in the receive path is missing.
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless code.

Yes, if it is useless.

Quoted 11 lines

Quoted 5 lines
Quoted 3 lines
However this new header mode is not required and only seems to increase the code complexity, so this patch revert this part of commit 147271209a9d.
Why is it not required ?
It works fine without, since 2012. In fact this change is not even mentioned in the log of commit 147271209a9d, so I really don't know why it was added in the first place. As can be seen in the revert all it does is adding 2 bytes to the USB packets that are then just skipped. Seems pretty useless to me.

I would like to get some feedback on this from Marcel, since he added this stuff.

Quoted 2 lines

Alban

Signature · 2 lines

Best regards,
Marek Vasut

Marcel Ziswiler

2016-08-03 15:23 UTC #4 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 33 lines · Marek Vasut

Quoted 7 lines · Alban Bedel
Quoted 3 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
Quoted 1 line · Alban Bedel
Quoted 8 lines
Quoted 6 lines
Quoted 5 lines
Commit 147271209a9d ("net: asix: fix operation without eeprom") added a special handling for ASIX 88772B that enable another type of header. This break the driver in DM mode as the extra handling
needed in the receive path is missing.
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless code.
Yes, if it is useless.
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
However this new header mode is not required and only seems to increase the code complexity, so this patch revert this part of commit 147271209a9d.
Why is it not required ?
Quoted 8 lines
It works fine without, since 2012. In fact this change is not even mentioned in the log of commit 147271209a9d, so I really don't know why
it was added in the first place. As can be seen in the revert all it
does is adding 2 bytes to the USB packets that are then just skipped.
Seems pretty useless to me.

Quoted 2 lines

I would like to get some feedback on this from Marcel, since he added this stuff.

Yes, sorry. I just came back from vacation and started looking into it now. As far as I remember on our hardware without this Ethernet did not quite work reliably. This also means that with driver model so far it does not work for us which I fed back to Simon once but so far this has not been resolved. That fix came from some early U-Boot work done by Antmicro way back and I am missing some of the history.

Cheers

Marcel

Quoted 1 line

Quoted 1 line
Alban

Alban Bedel

author

2016-08-04 09:07 UTC #5 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 37 lines · On Wed, 3 Aug 2016 15:23:30 +0000 Marcel Ziswiler…

Quoted 33 lines · Marek Vasut
Quoted 7 lines · Alban Bedel
Quoted 3 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
Quoted 1 line · Alban Bedel
Quoted 8 lines
Quoted 6 lines
Quoted 5 lines
Commit 147271209a9d ("net: asix: fix operation without eeprom") added a special handling for ASIX 88772B that enable another type of header. This break the driver in DM mode as the extra handling
needed in the receive path is missing.
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless code.
Yes, if it is useless.
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
However this new header mode is not required and only seems to increase the code complexity, so this patch revert this part of commit 147271209a9d.
Why is it not required ?
Quoted 8 lines
It works fine without, since 2012. In fact this change is not even mentioned in the log of commit 147271209a9d, so I really don't know why
it was added in the first place. As can be seen in the revert all it
does is adding 2 bytes to the USB packets that are then just skipped.
Seems pretty useless to me.
Quoted 2 lines
I would like to get some feedback on this from Marcel, since he added this stuff.

Quoted 6 lines

Yes, sorry. I just came back from vacation and started looking into it now. As far as I remember on our hardware without this Ethernet did not quite work reliably. This also means that with driver model so far it does not work for us which I fed back to Simon once but so far this has not been resolved. That fix came from some early U-Boot work done by Antmicro way back and I am missing some of the history.

Then I'll do a new patch that just fix the driver model receive path.

Alban

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160804/93886a01/attachment.sig>

Marek Vasut

2016-08-04 09:12 UTC #6 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 40 lines · Alban Bedel

Quoted 37 lines · On Wed, 3 Aug 2016 15:23:30 +0000 Marcel Ziswiler…
Quoted 35 lines · Marek Vasut
Quoted 7 lines · Alban Bedel
Quoted 3 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
Quoted 1 line · Alban Bedel
Quoted 8 lines
Quoted 6 lines
Quoted 5 lines
Commit 147271209a9d ("net: asix: fix operation without eeprom") added a special handling for ASIX 88772B that enable another type of header. This break the driver in DM mode as the extra handling
needed in the receive path is missing.
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless code.
Yes, if it is useless.
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
However this new header mode is not required and only seems to increase the code complexity, so this patch revert this part of commit 147271209a9d.
Why is it not required ?
Quoted 8 lines
It works fine without, since 2012. In fact this change is not even mentioned in the log of commit 147271209a9d, so I really don't know why
it was added in the first place. As can be seen in the revert all it
does is adding 2 bytes to the USB packets that are then just skipped.
Seems pretty useless to me.
I would like to get some feedback on this from Marcel, since he added this stuff.

Quoted 8 lines

Quoted 6 lines
Yes, sorry. I just came back from vacation and started looking into it now. As far as I remember on our hardware without this Ethernet did not quite work reliably. This also means that with driver model so far it does not work for us which I fed back to Simon once but so far this has not been resolved. That fix came from some early U-Boot work done by Antmicro way back and I am missing some of the history.
Then I'll do a new patch that just fix the driver model receive path.

Hold on. Marcel, can you maybe test if removing this code has any impact on the behavior now ?

Signature · 2 lines

Best regards,
Marek Vasut

Marcel Ziswiler

2016-08-09 12:14 UTC #7 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 70 lines · Marek Vasut

Quoted 69 lines · Alban Bedel
Quoted 65 lines · On Wed, 3 Aug 2016 15:23:30 +0000 Marcel Ziswiler…
Quoted 49 lines · Marek Vasut
Quoted 18 lines · Alban Bedel
Quoted 13 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
Quoted 10 lines · Alban Bedel
Commit 147271209a9d ("net: asix: fix operation without eeprom")
added a special handling for ASIX 88772B that enable another
type of header. This break the driver in DM mode as the extra
handling
needed in the receive path is missing.
Quoted 4 lines
Quoted 1 line
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless
code.
Yes, if it is useless.
Quoted 12 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
However this new header mode is not required and only seems to
increase the code complexity, so this patch revert this part of
commit 147271209a9d.
Why is it not required ?
Quoted 11 lines
It works fine without, since 2012. In fact this change is not even
mentioned in the log of commit 147271209a9d, so I really don't know
why
it was added in the first place. As can be seen in the revert all
it
does is adding 2 bytes to the USB packets that are then just skipped.
Seems pretty useless to me.
Quoted 3 lines
I would like to get some feedback on this from Marcel, since he added
this stuff.
Yes, sorry. I just came back from vacation and started looking into it
now. As far as I remember on our hardware without this Ethernet did not
quite work reliably. This also means that with driver model so far it
does not work for us which I fed back to Simon once but so far this has
not been resolved. That fix came from some early U-Boot work done by
Antmicro way back and I am missing some of the history.

Quoted 5 lines

Quoted 2 lines
Then I'll do a new patch that just fix the driver model receive path.
Hold on. Marcel, can you maybe test if removing this code has any impact
on the behavior now ?

Sorry for the delay. I tested Alban's patch now both on Toradex Colibri T20 as well as T30 and its on-module ASIX USB-to-Ethernet chip actually works perfectly aside from the occasional EHCI timed out on TD - token=0x88008d80 Rx: failed to receive: -5 message which last I checked with Simon is still unresolved but was already there long before any of the driver model work started.

Tested-by Marcel Ziswiler <[email protected]>

Tested-on: Colibri T20/T30 on Colibri Evaluation Board

Marek Vasut

2016-08-09 12:32 UTC #8 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 84 lines · Marcel Ziswiler

Quoted 70 lines · Marek Vasut
Quoted 69 lines · Alban Bedel
Quoted 65 lines · On Wed, 3 Aug 2016 15:23:30 +0000 Marcel Ziswiler…
Quoted 49 lines · Marek Vasut
Quoted 18 lines · Alban Bedel
Quoted 13 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
Quoted 10 lines · Alban Bedel
Commit 147271209a9d ("net: asix: fix operation without eeprom")
added a special handling for ASIX 88772B that enable another
type of header. This break the driver in DM mode as the extra
handling
needed in the receive path is missing.
Quoted 4 lines
Quoted 1 line
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless
code.
Yes, if it is useless.
Quoted 12 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
However this new header mode is not required and only seems to
increase the code complexity, so this patch revert this part of
commit 147271209a9d.
Why is it not required ?
Quoted 11 lines
It works fine without, since 2012. In fact this change is not even
mentioned in the log of commit 147271209a9d, so I really don't know
why
it was added in the first place. As can be seen in the revert all
it
does is adding 2 bytes to the USB packets that are then just skipped.
Seems pretty useless to me.
Quoted 3 lines
I would like to get some feedback on this from Marcel, since he added
this stuff.
Yes, sorry. I just came back from vacation and started looking into it
now. As far as I remember on our hardware without this Ethernet did not
quite work reliably. This also means that with driver model so far it
does not work for us which I fed back to Simon once but so far this has
not been resolved. That fix came from some early U-Boot work done by
Antmicro way back and I am missing some of the history.
Quoted 5 lines
Quoted 2 lines
Then I'll do a new patch that just fix the driver model receive path.
Hold on. Marcel, can you maybe test if removing this code has any impact
on the behavior now ?
Sorry for the delay. I tested Alban's patch now both on Toradex Colibri T20 as well as T30 and its on-module ASIX USB-to-Ethernet chip actually works perfectly aside from the occasional EHCI timed out on TD - token=0x88008d80 Rx: failed to receive: -5 message which last I checked with Simon is still unresolved but was already there long before any of the driver model work started.

Quoted 3 lines

Tested-by Marcel Ziswiler <[email protected]>
Tested-on: Colibri T20/T30 on Colibri Evaluation Board

Thanks!

Signature · 2 lines

Best regards,
Marek Vasut

Alban Bedel

author

2016-08-11 08:52 UTC #9 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 85 lines · On Tue, 9 Aug 2016 14:32:14 +0200 Marek Vasut <[email protected]> wrote:

Quoted 84 lines · Marcel Ziswiler
Quoted 70 lines · Marek Vasut
Quoted 69 lines · Alban Bedel
Quoted 65 lines · On Wed, 3 Aug 2016 15:23:30 +0000 Marcel Ziswiler…
Quoted 49 lines · Marek Vasut
Quoted 18 lines · Alban Bedel
Quoted 13 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
Quoted 10 lines · Alban Bedel
Commit 147271209a9d ("net: asix: fix operation without eeprom")
added a special handling for ASIX 88772B that enable another
type of header. This break the driver in DM mode as the extra
handling
needed in the receive path is missing.
Quoted 4 lines
Quoted 1 line
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless
code.
Yes, if it is useless.
Quoted 12 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
However this new header mode is not required and only seems to
increase the code complexity, so this patch revert this part of
commit 147271209a9d.
Why is it not required ?
Quoted 11 lines
It works fine without, since 2012. In fact this change is not even
mentioned in the log of commit 147271209a9d, so I really don't know
why
it was added in the first place. As can be seen in the revert all
it
does is adding 2 bytes to the USB packets that are then just skipped.
Seems pretty useless to me.
Quoted 3 lines
I would like to get some feedback on this from Marcel, since he added
this stuff.
Yes, sorry. I just came back from vacation and started looking into it
now. As far as I remember on our hardware without this Ethernet did not
quite work reliably. This also means that with driver model so far it
does not work for us which I fed back to Simon once but so far this has
not been resolved. That fix came from some early U-Boot work done by
Antmicro way back and I am missing some of the history.
Quoted 5 lines
Quoted 2 lines
Then I'll do a new patch that just fix the driver model receive path.
Hold on. Marcel, can you maybe test if removing this code has any impact
on the behavior now ?
Sorry for the delay. I tested Alban's patch now both on Toradex Colibri T20 as well as T30 and its on-module ASIX USB-to-Ethernet chip actually works perfectly aside from the occasional EHCI timed out on TD - token=0x88008d80 Rx: failed to receive: -5 message which last I checked with Simon is still unresolved but was already there long before any of the driver model work started.

Quoted 3 lines

Quoted 3 lines
Tested-by Marcel Ziswiler <[email protected]>
Tested-on: Colibri T20/T30 on Colibri Evaluation Board

Will this be applied for the upcoming release?

Marek Vasut

2016-08-11 09:26 UTC #10 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 88 lines · Alban Bedel

Quoted 85 lines · On Tue, 9 Aug 2016 14:32:14 +0200 Marek Vasut <[email protected]> wrote:
Quoted 84 lines · Marcel Ziswiler
Quoted 70 lines · Marek Vasut
Quoted 69 lines · Alban Bedel
Quoted 65 lines · On Wed, 3 Aug 2016 15:23:30 +0000 Marcel Ziswiler…
Quoted 49 lines · Marek Vasut
Quoted 18 lines · Alban Bedel
Quoted 13 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
Quoted 10 lines · Alban Bedel
Commit 147271209a9d ("net: asix: fix operation without eeprom")
added a special handling for ASIX 88772B that enable another
type of header. This break the driver in DM mode as the extra
handling
needed in the receive path is missing.
Quoted 4 lines
Quoted 1 line
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless
code.
Yes, if it is useless.
Quoted 12 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
However this new header mode is not required and only seems to
increase the code complexity, so this patch revert this part of
commit 147271209a9d.
Why is it not required ?
Quoted 11 lines
It works fine without, since 2012. In fact this change is not even
mentioned in the log of commit 147271209a9d, so I really don't know
why
it was added in the first place. As can be seen in the revert all
it
does is adding 2 bytes to the USB packets that are then just skipped.
Seems pretty useless to me.
Quoted 3 lines
I would like to get some feedback on this from Marcel, since he added
this stuff.
Yes, sorry. I just came back from vacation and started looking into it
now. As far as I remember on our hardware without this Ethernet did not
quite work reliably. This also means that with driver model so far it
does not work for us which I fed back to Simon once but so far this has
not been resolved. That fix came from some early U-Boot work done by
Antmicro way back and I am missing some of the history.
Quoted 5 lines
Quoted 2 lines
Then I'll do a new patch that just fix the driver model receive path.
Hold on. Marcel, can you maybe test if removing this code has any impact
on the behavior now ?
Sorry for the delay. I tested Alban's patch now both on Toradex Colibri T20 as well as T30 and its on-module ASIX USB-to-Ethernet chip actually works perfectly aside from the occasional EHCI timed out on TD - token=0x88008d80 Rx: failed to receive: -5 message which last I checked with Simon is still unresolved but was already there long before any of the driver model work started.

Quoted 5 lines

Quoted 3 lines
Quoted 3 lines
Tested-by Marcel Ziswiler <[email protected]>
Tested-on: Colibri T20/T30 on Colibri Evaluation Board
Will this be applied for the upcoming release?

Yeah. Why the hurry though ?

Signature · 2 lines

Best regards,
Marek Vasut

Alban Bedel

author

2016-08-11 10:28 UTC #11 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 89 lines · On Thu, 11 Aug 2016 11:26:23 +0200 Marek Vasut <[email protected]> wrote:

Quoted 88 lines · Alban Bedel
Quoted 85 lines · On Tue, 9 Aug 2016 14:32:14 +0200 Marek Vasut <[email protected]> wrote:
Quoted 84 lines · Marcel Ziswiler
Quoted 70 lines · Marek Vasut
Quoted 69 lines · Alban Bedel
Quoted 65 lines · On Wed, 3 Aug 2016 15:23:30 +0000 Marcel Ziswiler…
Quoted 49 lines · Marek Vasut
Quoted 18 lines · Alban Bedel
Quoted 13 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
Quoted 10 lines · Alban Bedel
Commit 147271209a9d ("net: asix: fix operation without eeprom")
added a special handling for ASIX 88772B that enable another
type of header. This break the driver in DM mode as the extra
handling
needed in the receive path is missing.
Quoted 4 lines
Quoted 1 line
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless
code.
Yes, if it is useless.
Quoted 12 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
However this new header mode is not required and only seems to
increase the code complexity, so this patch revert this part of
commit 147271209a9d.
Why is it not required ?
Quoted 11 lines
It works fine without, since 2012. In fact this change is not even
mentioned in the log of commit 147271209a9d, so I really don't know
why
it was added in the first place. As can be seen in the revert all
it
does is adding 2 bytes to the USB packets that are then just skipped.
Seems pretty useless to me.
Quoted 3 lines
I would like to get some feedback on this from Marcel, since he added
this stuff.
Yes, sorry. I just came back from vacation and started looking into it
now. As far as I remember on our hardware without this Ethernet did not
quite work reliably. This also means that with driver model so far it
does not work for us which I fed back to Simon once but so far this has
not been resolved. That fix came from some early U-Boot work done by
Antmicro way back and I am missing some of the history.
Quoted 5 lines
Quoted 2 lines
Then I'll do a new patch that just fix the driver model receive path.
Hold on. Marcel, can you maybe test if removing this code has any impact
on the behavior now ?
Sorry for the delay. I tested Alban's patch now both on Toradex Colibri T20 as well as T30 and its on-module ASIX USB-to-Ethernet chip actually works perfectly aside from the occasional EHCI timed out on TD - token=0x88008d80 Rx: failed to receive: -5 message which last I checked with Simon is still unresolved but was already there long before any of the driver model work started.

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Quoted 3 lines
Tested-by Marcel Ziswiler <[email protected]>
Tested-on: Colibri T20/T30 on Colibri Evaluation Board
Will this be applied for the upcoming release?
Yeah. Why the hurry though ?

I was just wondering because all the other patches I submitted have been applied but this one still seems to be on hold.

Marek Vasut

2016-08-11 14:18 UTC #12 lore raw

PATCH 1/1 net: asix: Fix ASIX 88772B with driver model

Quoted 96 lines · Alban Bedel

Quoted 93 lines · On Thu, 11 Aug 2016 11:26:23 +0200 Marek Vasut <[email protected]> wrote:
Quoted 92 lines · Alban Bedel
Quoted 88 lines · On Tue, 9 Aug 2016 14:32:14 +0200 Marek Vasut <[email protected]> wrote:
Quoted 87 lines · Marcel Ziswiler
Quoted 70 lines · Marek Vasut
Quoted 69 lines · Alban Bedel
Quoted 65 lines · On Wed, 3 Aug 2016 15:23:30 +0000 Marcel Ziswiler…
Quoted 49 lines · Marek Vasut
Quoted 18 lines · Alban Bedel
Quoted 13 lines · On Wed, 3 Aug 2016 09:00:42 +0200 Marek Vasut <[email protected]> wrote:
Quoted 10 lines · Alban Bedel
Commit 147271209a9d ("net: asix: fix operation without eeprom")
added a special handling for ASIX 88772B that enable another
type of header. This break the driver in DM mode as the extra
handling
needed in the receive path is missing.
Quoted 4 lines
Quoted 1 line
So add the extra handling ?
I can do that too, but I though u-boot preferred to avoid useless
code.
Yes, if it is useless.
Quoted 12 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
However this new header mode is not required and only seems to
increase the code complexity, so this patch revert this part of
commit 147271209a9d.
Why is it not required ?
Quoted 11 lines
It works fine without, since 2012. In fact this change is not even
mentioned in the log of commit 147271209a9d, so I really don't know
why
it was added in the first place. As can be seen in the revert all
it
does is adding 2 bytes to the USB packets that are then just skipped.
Seems pretty useless to me.
Quoted 3 lines
I would like to get some feedback on this from Marcel, since he added
this stuff.
Yes, sorry. I just came back from vacation and started looking into it
now. As far as I remember on our hardware without this Ethernet did not
quite work reliably. This also means that with driver model so far it
does not work for us which I fed back to Simon once but so far this has
not been resolved. That fix came from some early U-Boot work done by
Antmicro way back and I am missing some of the history.
Quoted 5 lines
Quoted 2 lines
Then I'll do a new patch that just fix the driver model receive path.
Hold on. Marcel, can you maybe test if removing this code has any impact
on the behavior now ?
Sorry for the delay. I tested Alban's patch now both on Toradex Colibri T20 as well as T30 and its on-module ASIX USB-to-Ethernet chip actually works perfectly aside from the occasional EHCI timed out on TD - token=0x88008d80 Rx: failed to receive: -5 message which last I checked with Simon is still unresolved but was already there long before any of the driver model work started.
Tested-by Marcel Ziswiler <[email protected]>
Tested-on: Colibri T20/T30 on Colibri Evaluation Board

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
Will this be applied for the upcoming release?
Yeah. Why the hurry though ?
I was just wondering because all the other patches I submitted have been applied but this one still seems to be on hold.

Well because this one was broken, so I had to throw it away. Please do make sure next time that the stuff builds using buildman.

Signature · 2 lines

Best regards,
Marek Vasut