[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

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

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 <alban.bedel@avionic-design.de>
---
 drivers/usb/eth/asix.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
index ad083cf8ae4a..1c6e967db1a0 100644
--- a/drivers/usb/eth/asix.c
+++ b/drivers/usb/eth/asix.c
@@ -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);
-- 
2.9.2

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Marek Vasut]

On 08/03/2016 07:32 AM, Alban Bedel wrote:
> 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 ?

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

> Change-Id: I7edc89f5714ead60e5204340ba05caf21b155593
> Fixes: 147271209a9d ("net: asix: fix operation without eeprom")
> Signed-off-by: Alban Bedel <alban.bedel@avionic-design.de>
> ---
>  drivers/usb/eth/asix.c | 22 +++-------------------
>  1 file changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
> index ad083cf8ae4a..1c6e967db1a0 100644
> --- a/drivers/usb/eth/asix.c
> +++ b/drivers/usb/eth/asix.c
> @@ -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);
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Alban Bedel]

On Wed, 3 Aug 2016 09:00:42 +0200
Marek Vasut <marex@denx.de> wrote:

> On 08/03/2016 07:32 AM, Alban Bedel wrote:
> > 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.

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

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Marek Vasut]

On 08/03/2016 11:46 AM, Alban Bedel wrote:
> On Wed, 3 Aug 2016 09:00:42 +0200
> Marek Vasut <marex@denx.de> wrote:
> 
>> On 08/03/2016 07:32 AM, Alban Bedel wrote:
>>> 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.

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

> Alban
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Marcel Ziswiler]

On Wed, 2016-08-03 at 15:51 +0200, Marek Vasut wrote:
> On 08/03/2016 11:46 AM, Alban Bedel wrote:
> > 
> > On Wed, 3 Aug 2016 09:00:42 +0200
> > Marek Vasut <marex@denx.de> wrote:
> > 
> > > 
> > > On 08/03/2016 07:32 AM, Alban Bedel wrote:
> > > > 
> > > > 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.
> 
> > 
> > > 
> > > > 
> > > > 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.

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

> > Alban

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Alban Bedel]

On Wed, 3 Aug 2016 15:23:30 +0000
Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote:

> On Wed, 2016-08-03 at 15:51 +0200, Marek Vasut wrote:
> > On 08/03/2016 11:46 AM, Alban Bedel wrote:
> > > 
> > > On Wed, 3 Aug 2016 09:00:42 +0200
> > > Marek Vasut <marex@denx.de> wrote:
> > > 
> > > > 
> > > > On 08/03/2016 07:32 AM, Alban Bedel wrote:
> > > > > 
> > > > > 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.
> > 
> > > 
> > > > 
> > > > > 
> > > > > 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.
> 
> 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>

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Marek Vasut]

On 08/04/2016 11:07 AM, Alban Bedel wrote:
> On Wed, 3 Aug 2016 15:23:30 +0000
> Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote:
> 
>> On Wed, 2016-08-03 at 15:51 +0200, Marek Vasut wrote:
>>> On 08/03/2016 11:46 AM, Alban Bedel wrote:
>>>>
>>>> On Wed, 3 Aug 2016 09:00:42 +0200
>>>> Marek Vasut <marex@denx.de> wrote:
>>>>
>>>>>
>>>>> On 08/03/2016 07:32 AM, Alban Bedel wrote:
>>>>>>
>>>>>> 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.
>>>
>>>>
>>>>>
>>>>>>
>>>>>> 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.
>>
>> 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 ?


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Marcel Ziswiler]

On Thu, 2016-08-04 at 11:12 +0200, Marek Vasut wrote:
> On 08/04/2016 11:07 AM, Alban Bedel wrote:
> > 
> > On Wed, 3 Aug 2016 15:23:30 +0000
> > Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote:
> > 
> > > 
> > > On Wed, 2016-08-03 at 15:51 +0200, Marek Vasut wrote:
> > > > 
> > > > On 08/03/2016 11:46 AM, Alban Bedel wrote:
> > > > > 
> > > > > 
> > > > > On Wed, 3 Aug 2016 09:00:42 +0200
> > > > > Marek Vasut <marex@denx.de> wrote:
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 08/03/2016 07:32 AM, Alban Bedel wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 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.
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 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.
> > > 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 ?

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 <marcel.ziswiler@toradex.com>
Tested-on: Colibri T20/T30 on Colibri Evaluation Board

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Marek Vasut]

On 08/09/2016 02:14 PM, Marcel Ziswiler wrote:
> On Thu, 2016-08-04 at 11:12 +0200, Marek Vasut wrote:
>> On 08/04/2016 11:07 AM, Alban Bedel wrote:
>>>
>>> On Wed, 3 Aug 2016 15:23:30 +0000
>>> Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote:
>>>
>>>>
>>>> On Wed, 2016-08-03 at 15:51 +0200, Marek Vasut wrote:
>>>>>
>>>>> On 08/03/2016 11:46 AM, Alban Bedel wrote:
>>>>>>
>>>>>>
>>>>>> On Wed, 3 Aug 2016 09:00:42 +0200
>>>>>> Marek Vasut <marex@denx.de> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 08/03/2016 07:32 AM, Alban Bedel wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> 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.
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> 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.
>>>> 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 ?
> 
> 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 <marcel.ziswiler@toradex.com>
> Tested-on: Colibri T20/T30 on Colibri Evaluation Board
> 
Thanks!

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Alban Bedel]

On Tue, 9 Aug 2016 14:32:14 +0200
Marek Vasut <marex@denx.de> wrote:

> On 08/09/2016 02:14 PM, Marcel Ziswiler wrote:
> > On Thu, 2016-08-04 at 11:12 +0200, Marek Vasut wrote:
> >> On 08/04/2016 11:07 AM, Alban Bedel wrote:
> >>>
> >>> On Wed, 3 Aug 2016 15:23:30 +0000
> >>> Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote:
> >>>
> >>>>
> >>>> On Wed, 2016-08-03 at 15:51 +0200, Marek Vasut wrote:
> >>>>>
> >>>>> On 08/03/2016 11:46 AM, Alban Bedel wrote:
> >>>>>>
> >>>>>>
> >>>>>> On Wed, 3 Aug 2016 09:00:42 +0200
> >>>>>> Marek Vasut <marex@denx.de> wrote:
> >>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On 08/03/2016 07:32 AM, Alban Bedel wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> 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.
> >>>>>
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> 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.
> >>>> 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 ?
> > 
> > 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 <marcel.ziswiler@toradex.com>
> > Tested-on: Colibri T20/T30 on Colibri Evaluation Board
> > 

Will this be applied for the upcoming release?

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/20160811/9866af3b/attachment.sig>

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Marek Vasut]

On 08/11/2016 10:52 AM, Alban Bedel wrote:
> On Tue, 9 Aug 2016 14:32:14 +0200
> Marek Vasut <marex@denx.de> wrote:
> 
>> On 08/09/2016 02:14 PM, Marcel Ziswiler wrote:
>>> On Thu, 2016-08-04 at 11:12 +0200, Marek Vasut wrote:
>>>> On 08/04/2016 11:07 AM, Alban Bedel wrote:
>>>>>
>>>>> On Wed, 3 Aug 2016 15:23:30 +0000
>>>>> Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote:
>>>>>
>>>>>>
>>>>>> On Wed, 2016-08-03 at 15:51 +0200, Marek Vasut wrote:
>>>>>>>
>>>>>>> On 08/03/2016 11:46 AM, Alban Bedel wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, 3 Aug 2016 09:00:42 +0200
>>>>>>>> Marek Vasut <marex@denx.de> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 08/03/2016 07:32 AM, Alban Bedel wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 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.
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 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.
>>>>>> 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 ?
>>>
>>> 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 <marcel.ziswiler@toradex.com>
>>> Tested-on: Colibri T20/T30 on Colibri Evaluation Board
>>>
> 
> Will this be applied for the upcoming release?

Yeah. Why the hurry though ?


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Alban Bedel]

On Thu, 11 Aug 2016 11:26:23 +0200
Marek Vasut <marex@denx.de> wrote:

> On 08/11/2016 10:52 AM, Alban Bedel wrote:
> > On Tue, 9 Aug 2016 14:32:14 +0200
> > Marek Vasut <marex@denx.de> wrote:
> > 
> >> On 08/09/2016 02:14 PM, Marcel Ziswiler wrote:
> >>> On Thu, 2016-08-04 at 11:12 +0200, Marek Vasut wrote:
> >>>> On 08/04/2016 11:07 AM, Alban Bedel wrote:
> >>>>>
> >>>>> On Wed, 3 Aug 2016 15:23:30 +0000
> >>>>> Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote:
> >>>>>
> >>>>>>
> >>>>>> On Wed, 2016-08-03 at 15:51 +0200, Marek Vasut wrote:
> >>>>>>>
> >>>>>>> On 08/03/2016 11:46 AM, Alban Bedel wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Wed, 3 Aug 2016 09:00:42 +0200
> >>>>>>>> Marek Vasut <marex@denx.de> wrote:
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On 08/03/2016 07:32 AM, Alban Bedel wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> 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.
> >>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> 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.
> >>>>>> 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 ?
> >>>
> >>> 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 <marcel.ziswiler@toradex.com>
> >>> 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.

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/20160811/968587b9/attachment.sig>

[U-Boot] [PATCH] net: asix: Fix ASIX 88772B with driver model

[Marek Vasut]

On 08/11/2016 12:28 PM, Alban Bedel wrote:
> On Thu, 11 Aug 2016 11:26:23 +0200
> Marek Vasut <marex@denx.de> wrote:
> 
>> On 08/11/2016 10:52 AM, Alban Bedel wrote:
>>> On Tue, 9 Aug 2016 14:32:14 +0200
>>> Marek Vasut <marex@denx.de> wrote:
>>>
>>>> On 08/09/2016 02:14 PM, Marcel Ziswiler wrote:
>>>>> On Thu, 2016-08-04 at 11:12 +0200, Marek Vasut wrote:
>>>>>> On 08/04/2016 11:07 AM, Alban Bedel wrote:
>>>>>>>
>>>>>>> On Wed, 3 Aug 2016 15:23:30 +0000
>>>>>>> Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, 2016-08-03 at 15:51 +0200, Marek Vasut wrote:
>>>>>>>>>
>>>>>>>>> On 08/03/2016 11:46 AM, Alban Bedel wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Wed, 3 Aug 2016 09:00:42 +0200
>>>>>>>>>> Marek Vasut <marex@denx.de> wrote:
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 08/03/2016 07:32 AM, Alban Bedel wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> 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.
>>>>>>>> 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 ?
>>>>>
>>>>> 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 <marcel.ziswiler@toradex.com>
>>>>> 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.

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.


-- 
Best regards,
Marek Vasut