| Message ID | a95a4f365e8c6e275ac176942e8391d09cdeea1e.1438609279.git.marcel.ziswiler@toradex.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
On Monday, August 03, 2015 at 04:54:35 PM, Marcel Ziswiler wrote: > From: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > The following commit enforces CONFIG_DM_ETH for USB Ethernet which > breaks any board using CONFIG_USB_HOST_ETHER without CONFIG_DM which > this patch fixes. > > commit 69559093f6173dcfcb041df0995063bdbd07d49b > dm: usb: Avoid using USB ethernet with CONFIG_DM_USB and no DM_ETH > > Tested on Colibri T20/T30 as well as Apalis T30 with > CONFIG_USB_HOST_ETHER and CONFIG_USB_ETHER_ASIX enabled and a LevelOne > USB-0301 ASIX AX88772 dongle. > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > --- Simon, do you have any comment on this particular patch please ? Best regards, Marek Vasut
Hi, On 3 August 2015 at 08:54, Marcel Ziswiler <marcel@ziswiler.com> wrote: > From: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > The following commit enforces CONFIG_DM_ETH for USB Ethernet which > breaks any board using CONFIG_USB_HOST_ETHER without CONFIG_DM which > this patch fixes. I think this commit message might need a few tweaks. This is not a regression as mentioned elsewhere. Also I don't thing it means CONFIG_DM here. Maybe CONFIG_DM_USB? > > commit 69559093f6173dcfcb041df0995063bdbd07d49b > dm: usb: Avoid using USB ethernet with CONFIG_DM_USB and no DM_ETH > > Tested on Colibri T20/T30 as well as Apalis T30 with > CONFIG_USB_HOST_ETHER and CONFIG_USB_ETHER_ASIX enabled and a LevelOne > USB-0301 ASIX AX88772 dongle. > There was a report that this does in fact not work ("CONFIG_DM_ETH USB_ETHER_ASIX Reception Issue on Tegra"). does this patch fix that problem? I'm not sure how but the above suggests that it does. > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > --- > Note: Enabling CONFIG_DM_ETH (which BTW does not even compile without > adding some more errno.h includes to asix.c and usb_ether.c) doesn't > quite work neither due to reception issues which I will report in > separate emails/patches. > > common/cmd_usb.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/common/cmd_usb.c b/common/cmd_usb.c > index 0ade775..6874af7 100644 > --- a/common/cmd_usb.c > +++ b/common/cmd_usb.c > @@ -530,13 +530,16 @@ static void do_usb_start(void) > /* try to recognize storage devices immediately */ > usb_stor_curr_dev = usb_stor_scan(1); > #endif > +#endif > #ifdef CONFIG_USB_HOST_ETHER > # ifdef CONFIG_DM_ETH > -# error "You must use CONFIG_DM_USB if you want to use CONFIG_USB_HOST_ETHER with CONFIG_DM_ETH" > -# endif > +# ifndef CONFIG_DM_USB > +# error "You must use CONFIG_DM_USB if you want to use CONFIG_USB_HOST_ETHER with CONFIG_DM_ETH" > +# endif > +# else > /* try to recognize ethernet devices immediately */ > usb_ether_curr_dev = usb_host_eth_scan(1); > -#endif > +# endif > #endif > #ifdef CONFIG_USB_KEYBOARD > drv_usb_kbd_init(); > -- > 2.4.3 > The logic here was really ugly (sorry!). However even so I'm struggling to understand what this patch actually changes. It seems that the #error is activated: #if !CONFIG_DM_USB && CONFIG_USB_HOST_ETHER && CONFIG_DM_ETH which looks the same as before. So overall I'm a bit confused. Hoping we can figure this out soon. The patches to enable driver model USB Ethernet and Asix were applied a few weeks ago and there may well still be problems. As I mentioned I did not test this on Tegra hardware unfortuantely. Regards, Simon
On 5 August 2015 05:53:44 CEST, Simon Glass <sjg@chromium.org> wrote: >I think this commit message might need a few tweaks. This is not a >regression as mentioned elsewhere. Also I don't thing it means >CONFIG_DM here. Maybe CONFIG_DM_USB? Yes you are right. It's about DM_USB of course, sorry. >There was a report that this does in fact not work ("CONFIG_DM_ETH >USB_ETHER_ASIX Reception Issue on Tegra"). does this patch fix that >problem? I'm not sure how but the above suggests that it does. No, those are two entirely different issues. Fact is right now almost nobody defines DM_ETH but e.g. on Tegra DM_USB is set which currently renders on-module Ethernet defunct on both Colibri T20 as well as Colibri T30. This patch fixes just that and Ethernet will work again. >The logic here was really ugly (sorry!). However even so I'm >struggling to understand what this patch actually changes. It seems >that the #error is activated: > >#if !CONFIG_DM_USB > && CONFIG_USB_HOST_ETHER > && CONFIG_DM_ETH > >which looks the same as before. No, it's not only about the error message. You missed the usb_host_eth_scan() call! Without that being called in the old non-DM_ETH case it simply won't work! Which is the case in current mainline U-Boot on Colibri T20/T30. > >So overall I'm a bit confused. Hoping we can figure this out soon. The >patches to enable driver model USB Ethernet and Asix were applied a >few weeks ago and there may well still be problems. As I mentioned I >did not test this on Tegra hardware unfortuantely. As mentioned before. That's a different story but as DM_ETH is not currently enabled on any Tegras I consider this a secondary issue
diff --git a/common/cmd_usb.c b/common/cmd_usb.c index 0ade775..6874af7 100644 --- a/common/cmd_usb.c +++ b/common/cmd_usb.c @@ -530,13 +530,16 @@ static void do_usb_start(void) /* try to recognize storage devices immediately */ usb_stor_curr_dev = usb_stor_scan(1); #endif +#endif #ifdef CONFIG_USB_HOST_ETHER # ifdef CONFIG_DM_ETH -# error "You must use CONFIG_DM_USB if you want to use CONFIG_USB_HOST_ETHER with CONFIG_DM_ETH" -# endif +# ifndef CONFIG_DM_USB +# error "You must use CONFIG_DM_USB if you want to use CONFIG_USB_HOST_ETHER with CONFIG_DM_ETH" +# endif +# else /* try to recognize ethernet devices immediately */ usb_ether_curr_dev = usb_host_eth_scan(1); -#endif +# endif #endif #ifdef CONFIG_USB_KEYBOARD drv_usb_kbd_init();