Re: [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  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 +
> Marcel Ziswiler  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  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 
>>> 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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alban Bedel]

On Tue, 9 Aug 2016 14:32:14 +0200
Marek Vasut  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 +
> >>> Marcel Ziswiler  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  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 
> > Tested-on: Colibri T20/T30 on Colibri Evaluation Board
> > 

Will this be applied for the upcoming release?

Alban


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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  wrote:
> 
>> On 08/11/2016 10:52 AM, Alban Bedel wrote:
>>> On Tue, 9 Aug 2016 14:32:14 +0200
>>> Marek Vasut  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 +
>>> Marcel Ziswiler  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  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 
> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alban Bedel]

On Thu, 11 Aug 2016 11:26:23 +0200
Marek Vasut  wrote:

> On 08/11/2016 10:52 AM, Alban Bedel wrote:
> > On Tue, 9 Aug 2016 14:32:14 +0200
> > Marek Vasut  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 +
> > Marcel Ziswiler  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  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 
> >>> 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


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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 +
> > Marcel Ziswiler  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  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 
Tested-on: Colibri T20/T30 on Colibri Evaluation Board
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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 +
>>> Marcel Ziswiler  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  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 
> Tested-on: Colibri T20/T30 on Colibri Evaluation Board
> 
Thanks!

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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 +
> Marcel Ziswiler  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  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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alban Bedel]

On Wed, 3 Aug 2016 15:23:30 +
Marcel Ziswiler  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  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



signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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  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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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  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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alban Bedel]

On Wed, 3 Aug 2016 09:00:42 +0200
Marek Vasut  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


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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 
> ---
>  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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[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 
---
 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_ID0x772b
 
 /* 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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot