RE: USB Ethernet/RNDIS Gadget issue
Hi, > > I finally bisect the kernel (I didn't known that command, thanks for the > > tip) > > and found that it doesn't work anymore from this commit : > > > > b0bac2581c1918cc4ab0aca01977ad69f0bc127a is the first bad commit > > commit b0bac2581c1918cc4ab0aca01977ad69f0bc127a > > Author: Robert Baldyga > > Date: Wed Sep 16 12:10:42 2015 +0200 > > > > usb: gadget: introduce 'enabled' flag in struct usb_ep > > > > This patch introduces 'enabled' flag in struct usb_ep, and modifies > > usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint > > enabled/disabled state. It helps to avoid enabling endpoints which are > > already enabled, and disabling endpoints which are already disables. > > > > From now USB functions don't have to remember current endpoint > > enable/disable state, as this state is now handled automatically which > > makes this API less bug-prone. > > > > Signed-off-by: Robert Baldyga > > Signed-off-by: Felipe Balbi > > > > > > If I comment the test in function usb_ep_enable and usb_ep_disable to avoid > > enabling or disabling endpoints that already are, it works. > > But I guess the real issue is that this function is called more than once > > at some point? > > This is the second bug report involving Robert's series, we really need > to give some deeper attention to this. Robert, care to comment ? > > So, this means that if you let endpoints be enabled or disabled more > than once, then it works ? Sounds like a bug in the UDC controller to > me. Robert's patch just helped trigger the problem. Which controller > were you using again ? Was it Atmel UDC ? Yes that's it, and issue occurs when I unplug/plug the cable... On first connection it works. And yes I am using using atmel_usba_udc.c module. Thanks, Gil -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: USB Ethernet/RNDIS Gadget issue
Hi,
I finally bisect the kernel (I didn't known that command, thanks for the
tip)
and found that it doesn't work anymore from this commit :
b0bac2581c1918cc4ab0aca01977ad69f0bc127a is the first bad commit
commit b0bac2581c1918cc4ab0aca01977ad69f0bc127a
Author: Robert Baldyga
Date: Wed Sep 16 12:10:42 2015 +0200
usb: gadget: introduce 'enabled' flag in struct usb_ep
This patch introduces 'enabled' flag in struct usb_ep, and modifies
usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint
enabled/disabled state. It helps to avoid enabling endpoints which are
already enabled, and disabling endpoints which are already disables.
From now USB functions don't have to remember current endpoint
enable/disable state, as this state is now handled automatically which
makes this API less bug-prone.
Signed-off-by: Robert Baldyga
Signed-off-by: Felipe Balbi
If I comment the test in function usb_ep_enable and usb_ep_disable to avoid
enabling or disabling endpoints that already are, it works.
But I guess the real issue is that this function is called more than once
at some point?
>>>
>>> This is the second bug report involving Robert's series, we really need
>>> to give some deeper attention to this. Robert, care to comment ?
>>>
>>> So, this means that if you let endpoints be enabled or disabled more
>>> than once, then it works ? Sounds like a bug in the UDC controller to
>>> me. Robert's patch just helped trigger the problem. Which controller
>>> were you using again ? Was it Atmel UDC ?
>>
>> Yes that's it, and issue occurs when I unplug/plug the cable... On first
>> connection it works.
>> And yes I am using using atmel_usba_udc.c module.
>
>I wonder if there's a bug on atmel (or rndis function) where on cable
>disconnect, endpoint is left enabled. Can you check ?
Ok you are right, and indeed it seems related to Atmel udc driver.
Call to the function usba_ep_disable in atmel_usba_udc.c fail because
of this code :
if (!ep->ep.desc) {
spin_unlock_irqrestore(&udc->lock, flags);
/* REVISIT because this driver disables endpoints in
* reset_all_endpoints() before calling disconnect(),
* most gadget drivers would trigger this non-error ...
*/
if (udc->gadget.speed != USB_SPEED_UNKNOWN)
DBG(DBG_ERR, "ep_disable: %s not enabled\n",
ep->ep.name);
return -EINVAL;
}
That means that the function usb_ep_disable will not reset the enabled
flag so we think that endpoint is still enabled.
To confirm I just test without this part of code and it seems to work
but I don't know how it should be fixed, I am not very familiar with this...
Regards,
Gil
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: USB Ethernet/RNDIS Gadget issue
Hi, > interesting. Wonder what Windows is doing differently. Yeah, I guess the > best way forward is to find which commit caused the regression and git > bisect is the way to do that. Would be nice to see v3.13, v3.14, > v3.15... are working or not. The idea is to find the smallest interval > for bisection. > I finally bisect the kernel (I didn't known that command, thanks for the tip) and found that it doesn't work anymore from this commit : b0bac2581c1918cc4ab0aca01977ad69f0bc127a is the first bad commit commit b0bac2581c1918cc4ab0aca01977ad69f0bc127a Author: Robert Baldyga Date: Wed Sep 16 12:10:42 2015 +0200 usb: gadget: introduce 'enabled' flag in struct usb_ep This patch introduces 'enabled' flag in struct usb_ep, and modifies usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint enabled/disabled state. It helps to avoid enabling endpoints which are already enabled, and disabling endpoints which are already disables. From now USB functions don't have to remember current endpoint enable/disable state, as this state is now handled automatically which makes this API less bug-prone. Signed-off-by: Robert Baldyga Signed-off-by: Felipe Balbi If I comment the test in function usb_ep_enable and usb_ep_disable to avoid enabling or disabling endpoints that already are, it works. But I guess the real issue is that this function is called more than once at some point? Regards, Gil -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: USB Ethernet/RNDIS Gadget issue
Hi, Gil Weber writes: >> interesting. Wonder what Windows is doing differently. Yeah, I guess the >> best way forward is to find which commit caused the regression and git >> bisect is the way to do that. Would be nice to see v3.13, v3.14, >> v3.15... are working or not. The idea is to find the smallest interval >> for bisection. >> > > I finally bisect the kernel (I didn't known that command, thanks for the tip) > and found that it doesn't work anymore from this commit : > > b0bac2581c1918cc4ab0aca01977ad69f0bc127a is the first bad commit > commit b0bac2581c1918cc4ab0aca01977ad69f0bc127a > Author: Robert Baldyga > Date: Wed Sep 16 12:10:42 2015 +0200 > > usb: gadget: introduce 'enabled' flag in struct usb_ep > > This patch introduces 'enabled' flag in struct usb_ep, and modifies > usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint > enabled/disabled state. It helps to avoid enabling endpoints which are > already enabled, and disabling endpoints which are already disables. > > From now USB functions don't have to remember current endpoint > enable/disable state, as this state is now handled automatically which > makes this API less bug-prone. > > Signed-off-by: Robert Baldyga > Signed-off-by: Felipe Balbi > > > If I comment the test in function usb_ep_enable and usb_ep_disable to avoid > enabling or disabling endpoints that already are, it works. > But I guess the real issue is that this function is called more than once > at some point? This is the second bug report involving Robert's series, we really need to give some deeper attention to this. Robert, care to comment ? So, this means that if you let endpoints be enabled or disabled more than once, then it works ? Sounds like a bug in the UDC controller to me. Robert's patch just helped trigger the problem. Which controller were you using again ? Was it Atmel UDC ? -- balbi signature.asc Description: PGP signature
RE: USB Ethernet/RNDIS Gadget issue
Hi, Gil Weber writes: >> > I finally bisect the kernel (I didn't known that command, thanks for the >> > tip) >> > and found that it doesn't work anymore from this commit : >> > >> > b0bac2581c1918cc4ab0aca01977ad69f0bc127a is the first bad commit >> > commit b0bac2581c1918cc4ab0aca01977ad69f0bc127a >> > Author: Robert Baldyga >> > Date: Wed Sep 16 12:10:42 2015 +0200 >> > >> > usb: gadget: introduce 'enabled' flag in struct usb_ep >> > >> > This patch introduces 'enabled' flag in struct usb_ep, and modifies >> > usb_ep_enable() and usb_ep_disable() functions to encapsulate endpoint >> > enabled/disabled state. It helps to avoid enabling endpoints which are >> > already enabled, and disabling endpoints which are already disables. >> > >> > From now USB functions don't have to remember current endpoint >> > enable/disable state, as this state is now handled automatically which >> > makes this API less bug-prone. >> > >> > Signed-off-by: Robert Baldyga >> > Signed-off-by: Felipe Balbi >> > >> > >> > If I comment the test in function usb_ep_enable and usb_ep_disable to avoid >> > enabling or disabling endpoints that already are, it works. >> > But I guess the real issue is that this function is called more than once >> > at some point? >> >> This is the second bug report involving Robert's series, we really need >> to give some deeper attention to this. Robert, care to comment ? >> >> So, this means that if you let endpoints be enabled or disabled more >> than once, then it works ? Sounds like a bug in the UDC controller to >> me. Robert's patch just helped trigger the problem. Which controller >> were you using again ? Was it Atmel UDC ? > > Yes that's it, and issue occurs when I unplug/plug the cable... On first > connection it works. > And yes I am using using atmel_usba_udc.c module. I wonder if there's a bug on atmel (or rndis function) where on cable disconnect, endpoint is left enabled. Can you check ? -- balbi signature.asc Description: PGP signature
RE: USB Ethernet/RNDIS Gadget issue
Hi, Gil Weber writes: >> from your logs below, it seems like you're using atmel_usba_udc.c, >> right? That driver was merged during v3.17 merge window, how come it was >> working for you on v3.12 ? >> >> Can you try v3.17, when that driver was merged ? > > It was already in previous versions as I used it in 3.12 and 3.5. > From what I see in 3.17 it has just been moved to drivers/usb/gadget/udc. > Before that, all udc drivers were in drivers/usb/gadget. oh, you're right. It was just a rename patch. Sorry. > In 3.12 it was working good, I could try to merge my code to intermediate > versions. okay. So one way would be to bisect what happened between v3.12 and v4.4. >> > <7>[ 275.026000] composite_suspend:2122: g_ether gadget: suspend >> > <7>[ 275.141000] composite_suspend:2122: g_ether gadget: suspend >> > ... >> >> what happened during these almost 5 seconds ?? You shouldn't trim log >> like that, there might be important information ;-) > > It was always the same log. > It appears in loop because I removed power detection in atmel_usba_udc.c > driver for testing. Here is a cleaner log with that power detection when > I remove the cable: > > <7>[ 2540.513000] reset_config:626: g_ether gadget: reset config > <7>[ 2540.513000] rndis_disable:616: g_ether gadget: rndis deactivated > <7>[ 2549.289000] composite_suspend:2122: g_ether gadget: suspend > <6>[ 2549.735000] g_ether gadget: high-speed config #2: RNDIS > <7>[ 2549.735000] usba_ep_disable:655: udc: ep_disable: ep3 not enabled > <7>[ 2549.735000] rndis_set_alt:560: g_ether gadget: reset rndis > <7>[ 2549.735000] usba_ep_disable:655: udc: ep_disable: ep1 not enabled > <7>[ 2549.735000] usba_ep_disable:655: udc: ep_disable: ep2 not enabled > <7>[ 2549.735000] rndis_set_alt:565: g_ether gadget: init rndis > <7>[ 2549.735000] rndis_set_alt:593: g_ether gadget: RNDIS RX/TX early > activation ... > <7>[ 2549.735000] rndis_open:638: g_ether gadget: rndis_open > <7>[ 2549.735000] rndis_set_param_medium:981: rndis_set_param_medium: 0 > 4259840 > <7>[ 2549.735000] rndis_set_param_dev:951: rndis_set_param_dev: > <7>[ 2549.736000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l24 > <7>[ 2549.736000] rndis_msg_parser:810: rndis_msg_parser: RNDIS_MSG_INIT > <7>[ 2549.745000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l24 > <7>[ 2549.746000] rndis_msg_parser:810: rndis_msg_parser: RNDIS_MSG_INIT > > After that there is nothing, it just stop. > >> unlikely. RNDIS works fine at least on dwc3 and MUSB (though I haven't >> tested against windows for a few months) >> >> > I cannot say from which version it is not working anymore as the >> > previous version I used was 3.12. >> >> and that was an atmel provided kernel ? Are you, now, using an atmel >> provided kernel or actual mainline ? Does it work against a Linux host ? >> > > I am using mainline kernel. > > I just did a test with a Linux host and it seems to work. > > To be sure, I also test on other Windows hosts and all are having the > issue. interesting. Wonder what Windows is doing differently. Yeah, I guess the best way forward is to find which commit caused the regression and git bisect is the way to do that. Would be nice to see v3.13, v3.14, v3.15... are working or not. The idea is to find the smallest interval for bisection. cheers -- balbi signature.asc Description: PGP signature
RE: USB Ethernet/RNDIS Gadget issue
Hi, > from your logs below, it seems like you're using atmel_usba_udc.c, > right? That driver was merged during v3.17 merge window, how come it was > working for you on v3.12 ? > > Can you try v3.17, when that driver was merged ? It was already in previous versions as I used it in 3.12 and 3.5. >From what I see in 3.17 it has just been moved to drivers/usb/gadget/udc. Before that, all udc drivers were in drivers/usb/gadget. In 3.12 it was working good, I could try to merge my code to intermediate versions. > > <7>[ 275.026000] composite_suspend:2122: g_ether gadget: suspend > > <7>[ 275.141000] composite_suspend:2122: g_ether gadget: suspend > > ... > > what happened during these almost 5 seconds ?? You shouldn't trim log > like that, there might be important information ;-) It was always the same log. It appears in loop because I removed power detection in atmel_usba_udc.c driver for testing. Here is a cleaner log with that power detection when I remove the cable: <7>[ 2540.513000] reset_config:626: g_ether gadget: reset config <7>[ 2540.513000] rndis_disable:616: g_ether gadget: rndis deactivated <7>[ 2549.289000] composite_suspend:2122: g_ether gadget: suspend <6>[ 2549.735000] g_ether gadget: high-speed config #2: RNDIS <7>[ 2549.735000] usba_ep_disable:655: udc: ep_disable: ep3 not enabled <7>[ 2549.735000] rndis_set_alt:560: g_ether gadget: reset rndis <7>[ 2549.735000] usba_ep_disable:655: udc: ep_disable: ep1 not enabled <7>[ 2549.735000] usba_ep_disable:655: udc: ep_disable: ep2 not enabled <7>[ 2549.735000] rndis_set_alt:565: g_ether gadget: init rndis <7>[ 2549.735000] rndis_set_alt:593: g_ether gadget: RNDIS RX/TX early activation ... <7>[ 2549.735000] rndis_open:638: g_ether gadget: rndis_open <7>[ 2549.735000] rndis_set_param_medium:981: rndis_set_param_medium: 0 4259840 <7>[ 2549.735000] rndis_set_param_dev:951: rndis_set_param_dev: <7>[ 2549.736000] rndis_setup:525: g_ether gadget: rndis req21.00 v i l24 <7>[ 2549.736000] rndis_msg_parser:810: rndis_msg_parser: RNDIS_MSG_INIT <7>[ 2549.745000] rndis_setup:525: g_ether gadget: rndis req21.00 v i l24 <7>[ 2549.746000] rndis_msg_parser:810: rndis_msg_parser: RNDIS_MSG_INIT After that there is nothing, it just stop. > unlikely. RNDIS works fine at least on dwc3 and MUSB (though I haven't > tested against windows for a few months) > > > I cannot say from which version it is not working anymore as the > > previous version I used was 3.12. > > and that was an atmel provided kernel ? Are you, now, using an atmel > provided kernel or actual mainline ? Does it work against a Linux host ? > I am using mainline kernel. I just did a test with a Linux host and it seems to work. To be sure, I also test on other Windows hosts and all are having the issue. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB Ethernet/RNDIS Gadget issue
Hi, (please break your lines at 80-columns) Gil Weber writes: > I am experiencing some issues related USB Ethernet gadget with RNDIS > on Kernel 4.4. > > On first connection everything seems OK, the device is correctly > detected on my Windows 7 host. > > If I unplug/replug the USB cable, the device is detected again but not > working anymore, Windows status is "This device cannot start. (Code > 10)". > > Everything was working with Kernel 3.12. from your logs below, it seems like you're using atmel_usba_udc.c, right? That driver was merged during v3.17 merge window, how come it was working for you on v3.12 ? Can you try v3.17, when that driver was merged ? > Here is the trace log when it is first detected : > > <6>[ 126.437000] g_ether gadget: high-speed config #2: RNDIS > <7>[ 126.437000] rndis_set_alt:565: g_ether gadget: init rndis > <7>[ 126.437000] rndis_set_alt:593: g_ether gadget: RNDIS RX/TX early > activation ... > <7>[ 126.437000] rndis_open:638: g_ether gadget: rndis_open > <7>[ 126.437000] rndis_set_param_medium:981: rndis_set_param_medium: 0 > 4259840 > <7>[ 126.437000] rndis_set_param_dev:951: rndis_set_param_dev: > <7>[ 126.438000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l24 > <7>[ 126.439000] rndis_msg_parser:810: rndis_msg_parser: RNDIS_MSG_INIT > <7>[ 126.443000] rndis_setup:525: g_ether gadget: rndis reqa1.01 v i > l4096 > <7>[ 126.443000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l28 > <7>[ 126.443000] gen_ndis_query_resp:214: gen_ndis_query_resp: > RNDIS_OID_GEN_SUPPORTED_LIST > <7>[ 126.447000] rndis_setup:525: g_ether gadget: rndis reqa1.01 v i > l4096 > <7>[ 126.447000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l32 > <7>[ 126.447000] gen_ndis_query_resp:308: gen_ndis_query_resp: > RNDIS_OID_GEN_VENDOR_DRIVER_VERSION > <7>[ 126.451000] rndis_setup:525: g_ether gadget: rndis reqa1.01 v i > l4096 > <7>[ 126.451000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l32 > <7>[ 126.451000] gen_ndis_query_resp:252: gen_ndis_query_resp: > RNDIS_OID_GEN_MAXIMUM_FRAME_SIZE > <7>[ 126.455000] rndis_setup:525: g_ether gadget: rndis reqa1.01 v i > l4096 > <7>[ 126.455000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l32 > <7>[ 126.455000] gen_ndis_query_resp:252: gen_ndis_query_resp: > RNDIS_OID_GEN_MAXIMUM_FRAME_SIZE > <7>[ 126.459000] rndis_setup:525: g_ether gadget: rndis reqa1.01 v i > l4096 > <7>[ 126.459000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l32 > <7>[ 126.459000] gen_ndis_query_resp:439: gen_ndis_query_resp: > RNDIS_OID_802_3_MAXIMUM_LIST_SIZE > <7>[ 126.463000] rndis_setup:525: g_ether gadget: rndis reqa1.01 v i > l4096 > <7>[ 126.463000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l34 > <7>[ 126.463000] gen_ndis_query_resp:421: gen_ndis_query_resp: > RNDIS_OID_802_3_CURRENT_ADDRESS > <7>[ 126.467000] rndis_setup:525: g_ether gadget: rndis reqa1.01 v i > l4096 > <7>[ 126.467000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l34 > <7>[ 126.467000] gen_ndis_query_resp:411: gen_ndis_query_resp: > RNDIS_OID_802_3_PERMANENT_ADDRESS > <7>[ 126.471000] rndis_setup:525: g_ether gadget: rndis reqa1.01 v i > l4096 > > And here is the trace log when I unplug/plug the cable: > > <7>[ 275.026000] composite_suspend:2122: g_ether gadget: suspend > <7>[ 275.141000] composite_suspend:2122: g_ether gadget: suspend > ... what happened during these almost 5 seconds ?? You shouldn't trim log like that, there might be important information ;-) > <7>[ 279.756000] composite_suspend:2122: g_ether gadget: suspend > <7>[ 279.871000] reset_config:626: g_ether gadget: reset config > <7>[ 279.871000] rndis_disable:616: g_ether gadget: rndis deactivated > <6>[ 279.995000] g_ether gadget: high-speed config #2: RNDIS > <7>[ 279.995000] usba_ep_disable:655: udc: ep_disable: ep3 not enabled > <7>[ 279.995000] rndis_set_alt:560: g_ether gadget: reset rndis > <7>[ 279.995000] usba_ep_disable:655: udc: ep_disable: ep1 not enabled > <7>[ 279.995000] usba_ep_disable:655: udc: ep_disable: ep2 not enabled > <7>[ 279.995000] rndis_set_alt:565: g_ether gadget: init rndis > <7>[ 279.995000] rndis_set_alt:593: g_ether gadget: RNDIS RX/TX early > activation ... > <7>[ 279.995000] rndis_open:638: g_ether gadget: rndis_open > <7>[ 279.995000] rndis_set_param_medium:981: rndis_set_param_medium: 0 > 4259840 > <7>[ 279.995000] rndis_set_param_dev:951: rndis_set_param_dev: > <7>[ 279.996000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l24 > <7>[ 279.996000] rndis_msg_parser:810: rndis_msg_parser: RNDIS_MSG_INIT > <7>[ 280.011000] rndis_setup:525: g_ether gadget: rndis req21.00 v i > l24 > <7>[ 280.011000] rndis_msg_parser:810: rndis_msg_parser: RNDIS_MSG_INIT > > Does some body have an idea of what is going on? Is
