Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On Sun, 26 Jul 2015, Petr Cvek wrote: What about higher speeds (not relevant on PXA, but ep_matches() is called from usb_ep_autoconfig_ss() )? According to http://wiki.osdev.org/Universal_Serial_Bus#Maximum_Data_Payload_Size_2 High speed INT endpoint has a maximum data payload 1024 B and BULK only 512 B (are other attributes of the data phase similar?). What about superspeed? It's true that high speed interrupt endpoints can have higher maxpacket values than bulk endpoints. But this is okay, since ep_matches() checks that the hardware maxpacket value is at least as large as the value in the descriptor: if (ep-maxpacket_limit max) return 0; * modprobe g_serial use_acm=1 n_ports=1 * original version of ep_matches() (returns bulk and int) * compatible EP configuration/definition for UDC side http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L2352 USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_ISO(3), USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), * modified EP configuration for UDC side (just changed EP3 ISO to BULK, so there is one free BULK) USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_BULK(3), //change USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), ===results=== * original configuration is OK, all endpoints are found (in order ep2in-bulk, ep1out-bulk, ep3in-int), INT notification seems to work I don't understand. Above you said that the EP definition in the UDC is USB_EP_IN_ISO(3). So how can you end up with ep3in-int? int != ISO. You should have ended up with the third endpoint being ep5in-int, because ep_matches() doesn't allow an isochronous to match a request for an interrupt endpoint. I have changed definition of ISO to BULK only to accomplish minimal change of driver code (for my demonstration free BULK must be defined before INT - inserting new EP would require to reindex all next EPs and modifying links from PXA side endpoints). The USB_EP_IN_ISO(3) is just unused endpoint. This doesn't answer my question. I was asking about the original configuration, not your changed configuration. You wrote: * original configuration is OK, all endpoints are found (in order ep2in-bulk, ep1out-bulk, ep3in-int), INT notification seems to work But that isn't possible, because in the original configuration ep3in is iso, not int. Did you intend to write ep5in-int rather than ep3in-int? * modified configuration fails: [ 4259.609088] pxa27x-udc pxa27x-udc: ep15:pxa_ep_enable: type mismatch by this condition: http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L1416 because ep_matches() returns BULK. Okay, that's a problem in pxa27x-udc. Why does it insist on an exact match between the hardware endpoint type and the type contained in the descriptor? It should accept an interrupt descriptor if the hardware type is bulk. Hmm, making BULK EP equivalent with INT EP (when INT is requested) would made debugging (there is special bitfield in the config registers) and configuration preset (not anymore unordered set, but definition in the specific sequence) hell. But in other ways it can be OK (specification does not say that using EP marked INT as BULK will fail). This business about using bulk endpoints in place of interrupt endpoints goes back to the days when most UDCs had a very limited selection of endpoints. The idea was that a gadget could work even if there weren't enough interrupt endpoints in the hardware, provided there were extra bulk endpoints. I think optimal idea is custom matcher function. It would eliminate codes for superspeed checking on SoC which known only fullspeed ;-). Wuldn't it also mean duplicating a lot of code? Each custom matcher function would essentially have to include most of ep_matches(). P.S. I did a basic research where UDCs differ between BULK and INT handling (just searching for usb_endpoint_type(), USB_ENDPOINT_XFER_* and usb_endpoint_xfer_*() so it returned BULK on a software side - irrelevant): r8a66597-udc.c (using different constants, dedicated structure entries) m66592-udc.c (same as r8a66597-udc.c) dummy_hcd.c (well it is only dummy, says bulk is OK for INT, but has different matching rules for HS BULK and INT) atmel_usba_udc.c (only by write of some flag) net2272.c (fails with BULK, USB_SPEED_HIGH and maxpacket != 512) at91_udc.c (only BULK is only OK with 8,16,32,64 values) pxa25x_udc.c (setting one flag when hw (?) endpoint is BULK and another if nonBULK, INT FIFO size defined as 8, BULK FIFO as 64) net2280.c (INT path has erratum, different maxpacket matching) pxa27x_udc.h (INT_FIFO_SIZE defined as 16B, BULK_FIFO_SIZE defined as 64B) mv_udc_core.c (different codepath) gr_udc.c (using mode of
Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On 07/26/2015 05:14 PM, Alan Stern wrote: On Sun, 26 Jul 2015, Petr Cvek wrote: What about higher speeds (not relevant on PXA, but ep_matches() is called from usb_ep_autoconfig_ss() )? According to http://wiki.osdev.org/Universal_Serial_Bus#Maximum_Data_Payload_Size_2 High speed INT endpoint has a maximum data payload 1024 B and BULK only 512 B (are other attributes of the data phase similar?). What about superspeed? It's true that high speed interrupt endpoints can have higher maxpacket values than bulk endpoints. But this is okay, since ep_matches() checks that the hardware maxpacket value is at least as large as the value in the descriptor: if (ep-maxpacket_limit max) return 0; * modprobe g_serial use_acm=1 n_ports=1 * original version of ep_matches() (returns bulk and int) * compatible EP configuration/definition for UDC side http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L2352 USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_ISO(3), USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), * modified EP configuration for UDC side (just changed EP3 ISO to BULK, so there is one free BULK) USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_BULK(3), //change USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), ===results=== * original configuration is OK, all endpoints are found (in order ep2in-bulk, ep1out-bulk, ep3in-int), INT notification seems to work I don't understand. Above you said that the EP definition in the UDC is USB_EP_IN_ISO(3). So how can you end up with ep3in-int? int != ISO. You should have ended up with the third endpoint being ep5in-int, because ep_matches() doesn't allow an isochronous to match a request for an interrupt endpoint. I have changed definition of ISO to BULK only to accomplish minimal change of driver code (for my demonstration free BULK must be defined before INT - inserting new EP would require to reindex all next EPs and modifying links from PXA side endpoints). The USB_EP_IN_ISO(3) is just unused endpoint. This doesn't answer my question. I was asking about the original configuration, not your changed configuration. You wrote: * original configuration is OK, all endpoints are found (in order ep2in-bulk, ep1out-bulk, ep3in-int), INT notification seems to work But that isn't possible, because in the original configuration ep3in is iso, not int. Did you intend to write ep5in-int rather than ep3in-int? * modified configuration fails: [ 4259.609088] pxa27x-udc pxa27x-udc: ep15:pxa_ep_enable: type mismatch by this condition: http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L1416 because ep_matches() returns BULK. Okay, that's a problem in pxa27x-udc. Why does it insist on an exact match between the hardware endpoint type and the type contained in the descriptor? It should accept an interrupt descriptor if the hardware type is bulk. Hmm, making BULK EP equivalent with INT EP (when INT is requested) would made debugging (there is special bitfield in the config registers) and configuration preset (not anymore unordered set, but definition in the specific sequence) hell. But in other ways it can be OK (specification does not say that using EP marked INT as BULK will fail). This business about using bulk endpoints in place of interrupt endpoints goes back to the days when most UDCs had a very limited selection of endpoints. The idea was that a gadget could work even if there weren't enough interrupt endpoints in the hardware, provided there were extra bulk endpoints. I think optimal idea is custom matcher function. It would eliminate codes for superspeed checking on SoC which known only fullspeed ;-). Wuldn't it also mean duplicating a lot of code? Each custom matcher function would essentially have to include most of ep_matches(). I have solved this problem making ep_matches() public helper function, so we can avoid duplicated code. BTW this entire discussion is in reply to my patch doing this :) P.S. I did a basic research where UDCs differ between BULK and INT handling (just searching for usb_endpoint_type(), USB_ENDPOINT_XFER_* and usb_endpoint_xfer_*() so it returned BULK on a software side - irrelevant): r8a66597-udc.c (using different constants, dedicated structure entries) m66592-udc.c (same as r8a66597-udc.c) dummy_hcd.c (well it is only dummy, says bulk is OK for INT, but has different matching rules for HS BULK and INT) atmel_usba_udc.c (only by write of some flag) net2272.c (fails with BULK, USB_SPEED_HIGH and maxpacket != 512) at91_udc.c (only BULK is only OK with 8,16,32,64 values) pxa25x_udc.c (setting one flag when hw (?) endpoint is BULK and another if nonBULK, INT FIFO size defined as 8, BULK FIFO as 64) net2280.c (INT path has
Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On 26.7.2015 17:14, Alan Stern wrote: On Sun, 26 Jul 2015, Petr Cvek wrote: What about higher speeds (not relevant on PXA, but ep_matches() is called from usb_ep_autoconfig_ss() )? According to http://wiki.osdev.org/Universal_Serial_Bus#Maximum_Data_Payload_Size_2 High speed INT endpoint has a maximum data payload 1024 B and BULK only 512 B (are other attributes of the data phase similar?). What about superspeed? It's true that high speed interrupt endpoints can have higher maxpacket values than bulk endpoints. But this is okay, since ep_matches() checks that the hardware maxpacket value is at least as large as the value in the descriptor: if (ep-maxpacket_limit max) return 0; OK * modprobe g_serial use_acm=1 n_ports=1 * original version of ep_matches() (returns bulk and int) * compatible EP configuration/definition for UDC side http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L2352 USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_ISO(3), USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), * modified EP configuration for UDC side (just changed EP3 ISO to BULK, so there is one free BULK) USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_BULK(3), //change USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), ===results=== * original configuration is OK, all endpoints are found (in order ep2in-bulk, ep1out-bulk, ep3in-int), INT notification seems to work I don't understand. Above you said that the EP definition in the UDC is USB_EP_IN_ISO(3). So how can you end up with ep3in-int? int != ISO. You should have ended up with the third endpoint being ep5in-int, because ep_matches() doesn't allow an isochronous to match a request for an interrupt endpoint. I have changed definition of ISO to BULK only to accomplish minimal change of driver code (for my demonstration free BULK must be defined before INT - inserting new EP would require to reindex all next EPs and modifying links from PXA side endpoints). The USB_EP_IN_ISO(3) is just unused endpoint. This doesn't answer my question. I was asking about the original configuration, not your changed configuration. You wrote: * original configuration is OK, all endpoints are found (in order ep2in-bulk, ep1out-bulk, ep3in-int), INT notification seems to work But that isn't possible, because in the original configuration ep3in is iso, not int. Did you intend to write ep5in-int rather than ep3in-int? Oh my bad, It seems I got confused by my debug printks (warning about bulk as int and end of search). It should be: original: ep2in-bulk ep1out-bulk ep5in-int modified (free bulk before int): ep2in-bulk ep1out-bulk ep3in-bulk ... pxa27x-udc pxa27x-udc: ep15:pxa_ep_enable: type mismatch ... g_serial gadget: acm ttyGS0 can't notify serial state, -22 * modified configuration fails: [ 4259.609088] pxa27x-udc pxa27x-udc: ep15:pxa_ep_enable: type mismatch by this condition: http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L1416 because ep_matches() returns BULK. Okay, that's a problem in pxa27x-udc. Why does it insist on an exact match between the hardware endpoint type and the type contained in the descriptor? It should accept an interrupt descriptor if the hardware type is bulk. Hmm, making BULK EP equivalent with INT EP (when INT is requested) would made debugging (there is special bitfield in the config registers) and configuration preset (not anymore unordered set, but definition in the specific sequence) hell. But in other ways it can be OK (specification does not say that using EP marked INT as BULK will fail). This business about using bulk endpoints in place of interrupt endpoints goes back to the days when most UDCs had a very limited selection of endpoints. The idea was that a gadget could work even if there weren't enough interrupt endpoints in the hardware, provided there were extra bulk endpoints. I thought so, but on the PXA27x you can configure any endpoint for any type and at the same time you must configure them for the exact matching order. Maybe matching with BULK as INT functionality could work if all predefined INT was first in the array (.udc_usb_ep). This would prioritize predefined INT endpoints and use BULK only after INT exhaustion. (ideal way would be resetting UDC and generating EP configuration for every set_configuration/interface, but I don't know if possible). I think optimal idea is custom matcher function. It would eliminate codes for superspeed checking on SoC which known only fullspeed ;-). Wuldn't it also mean duplicating a lot of code? Each custom matcher function would essentially have to include most of ep_matches(). There can be global generic matching function (striped of quirks) which
Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
Alan Stern st...@rowland.harvard.edu writes: Hi Alan, On Sat, 25 Jul 2015, Robert Jarzmik wrote: Petr Cvek petr.c...@tul.cz writes: On 23.7.2015 21:46, Alan Stern wrote: It seems that it allows using a BULK endpoint for requested INT endpoint. For my PXA27x machine the original code returns BULK EP even with valid INT endpoint definition (because BULK EPs are defined earlier than INT EPs). Yes, it does allow a bulk endpoint to be used when an interrupt endpoint was requested. However, it won't return a bulk endpoint if all the bulk endpoints are already in use. This cannot work for pxa27x. Do you mean that on pxa27x, a bulk endpoint cannot be used as an interrupt endpoint? Why not? From the device controller's point of view, there is no difference between bulk and interrupt (except possibly for the maxpacket sizes and high-bandwidth usage when running at high speed). That's the point, maxpacket size and priority. As you said, it's not that it won't work, it won't work with the priority expected by the software stack, ie. higher priority for ISO endpoint. The pxa27x IP has a hardware limitation which prevents an endpoint from changing its type once the UDC is enabled (see the comment at the beginning of pxa27x_udc.c). If that patchset implies that for a requested INT endpoint a BULK endpoint can be returned, that won't work. Felipe and Robert, is that what this patchset implies ? Sort of. The matching code has always behaved that way and this patchset does not change the behavior. Then all is fine I suppose, if it was working before and nothing changes, it will continue to work, won't it ? A default PXA27x configuration returns BULK for requested INT. Which is unfortunate, because PXA27x supports INT endpoints and has one predefined, but this function find BULK first (one BULK is allocated and INT is never used). See above. See response above. Besides, let's say the pxa27x has one bulk and one interrupt endpoint. Now suppose the gadget driver requests a bulk endpoint first. The matching code will allocate the single bulk endpoint. Then the gadget driver requests an interrupt endpoint. The matching code cannot allocate the bulk endpoint, because that endpoint is already allocated. So it will allocate the interrupt endpoint. Thus, as you can see, under the right conditions everything will work as desired. Let me give you another example : - pxa27x_udc offers 3 endpoints : ep-in, ep-out, ep-iso-in - a gadget driver does : - request an ep-in - request an ep-out - request an ep-in - request an ep-iso-in In that case, the ep-iso-in request will fail, right ? Yet I would have expected the second ep-in request to fail, as that's the one which cannot be serviced. Of course, this hypothetical case implies that pxa27x_udc is not compatible with this gadget driver, so it's not really relevant, is it ... Because if they do, the ep_matches() function works poorly. It returns a BULK for device (gadget) side, but host side (PC) thinks that this endpoint is an INT and handles it in this way. But the PXA27x thinks the endpoint is a BULK and handles it in its way (according to datasheet, settings for a BULK and an INT transfers are not 100% compatible). How do they differ? One example I have in mind is chapter 12.4.2 of pxa27x developer manual Endpoint Memory Configuration, quote follows : If the USB host controller transmits more OUT data than the maximum size packet for a bulk or interrupt endpoint, the UDC does not send any handshake to the host controller causing the host controller to time-out. If the USB host controller transmits more OUT data than the maximum size packet for an isochronous endpoint, the UDC sets the data packet error (DPE) bit in the Endpoint Control/Status register, UDCCSRx[DPE]. Perhaps you could submit a patch that adds a do not allocate a bulk endpoint when an interrupt endpoint is requested quirk flag to the usb_gadget structure, and modify the matching code to take the new flag into account. Well, if it was working that way already in the past, I don't see overloading the code with a quirk a necessity. My only need is that it continues to work. Cheers. -- Robert -- 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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On Sat, 25 Jul 2015, Robert Jarzmik wrote: Petr Cvek petr.c...@tul.cz writes: On 23.7.2015 21:46, Alan Stern wrote: It seems that it allows using a BULK endpoint for requested INT endpoint. For my PXA27x machine the original code returns BULK EP even with valid INT endpoint definition (because BULK EPs are defined earlier than INT EPs). Yes, it does allow a bulk endpoint to be used when an interrupt endpoint was requested. However, it won't return a bulk endpoint if all the bulk endpoints are already in use. This cannot work for pxa27x. Do you mean that on pxa27x, a bulk endpoint cannot be used as an interrupt endpoint? Why not? From the device controller's point of view, there is no difference between bulk and interrupt (except possibly for the maxpacket sizes and high-bandwidth usage when running at high speed). The pxa27x IP has a hardware limitation which prevents an endpoint from changing its type once the UDC is enabled (see the comment at the beginning of pxa27x_udc.c). If that patchset implies that for a requested INT endpoint a BULK endpoint can be returned, that won't work. Felipe and Robert, is that what this patchset implies ? Sort of. The matching code has always behaved that way and this patchset does not change the behavior. A default PXA27x configuration returns BULK for requested INT. Which is unfortunate, because PXA27x supports INT endpoints and has one predefined, but this function find BULK first (one BULK is allocated and INT is never used). See above. See response above. Besides, let's say the pxa27x has one bulk and one interrupt endpoint. Now suppose the gadget driver requests a bulk endpoint first. The matching code will allocate the single bulk endpoint. Then the gadget driver requests an interrupt endpoint. The matching code cannot allocate the bulk endpoint, because that endpoint is already allocated. So it will allocate the interrupt endpoint. Thus, as you can see, under the right conditions everything will work as desired. Because if they do, the ep_matches() function works poorly. It returns a BULK for device (gadget) side, but host side (PC) thinks that this endpoint is an INT and handles it in this way. But the PXA27x thinks the endpoint is a BULK and handles it in its way (according to datasheet, settings for a BULK and an INT transfers are not 100% compatible). How do they differ? I cannot test INT as BULK behavior for the gadget functions, because all gadgets which works on PXA27x does not use INT endpoints (some allocate the endpoint but never use it). Ah a bit of history here. At least gadget zero does, and it's my main testing point for pxa27x_udc. Then there should be g_serial (no acm nor obex), but that's something I have not tried since 2009 ... For history also, there was already an attempt a long time ago for epautoconf revamping, done by Rodolfo Giometti IIRC. Anyway, I need pxa27x_udc to remain functional, so I'd like to understand if something will stop working, Robert B. Perhaps you could submit a patch that adds a do not allocate a bulk endpoint when an interrupt endpoint is requested quirk flag to the usb_gadget structure, and modify the matching code to take the new flag into account. Alan Stern -- 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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On Sat, 25 Jul 2015, Petr Cvek wrote: On 25.7.2015 19:04, Robert Jarzmik wrote: Alan Stern st...@rowland.harvard.edu writes: Hi Alan, On Sat, 25 Jul 2015, Robert Jarzmik wrote: Petr Cvek petr.c...@tul.cz writes: On 23.7.2015 21:46, Alan Stern wrote: It seems that it allows using a BULK endpoint for requested INT endpoint. For my PXA27x machine the original code returns BULK EP even with valid INT endpoint definition (because BULK EPs are defined earlier than INT EPs). Yes, it does allow a bulk endpoint to be used when an interrupt endpoint was requested. However, it won't return a bulk endpoint if all the bulk endpoints are already in use. This cannot work for pxa27x. Do you mean that on pxa27x, a bulk endpoint cannot be used as an interrupt endpoint? Why not? From the device controller's point of view, there is no difference between bulk and interrupt (except possibly for the maxpacket sizes and high-bandwidth usage when running at high speed). That's the point, maxpacket size and priority. As you said, it's not that it won't work, it won't work with the priority expected by the software stack, ie. higher priority for ISO endpoint. Yes, maxpacket could be problem. Datasheet has listed range (1-64) for INT and specific values (8, 16, 32, 64) for BULK. In practice I doubt this will matter. Using a larger maxpacket size than the gadget driver expects is rarely important for interrupt transfers, since they almost never involve more than one packet's worth of data. So for example, if the gadget driver wants an interrupt endpoint with maxpacket 42, it almost certainly will work okay if it gets a bulk endpoint with maxpacket 64. If that patchset implies that for a requested INT endpoint a BULK endpoint can be returned, that won't work. Felipe and Robert, is that what this patchset implies ? Sort of. The matching code has always behaved that way and this patchset does not change the behavior. Then all is fine I suppose, if it was working before and nothing changes, it will continue to work, won't it ? Yes functional behavior of this patch is same as in vanilla, I only began this thread, because I have found out that someone is sending patchset. But I found this behavior when I was trying to use g_webcam gadget. I have finally gathered enough information and luck (unstable machine) to try test g_serial so configuration: * modprobe g_serial use_acm=1 n_ports=1 * original version of ep_matches() (returns bulk and int) * compatible EP configuration/definition for UDC side http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L2352 USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_ISO(3), USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), * modified EP configuration for UDC side (just changed EP3 ISO to BULK, so there is one free BULK) USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_BULK(3), //change USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), ===results=== * original configuration is OK, all endpoints are found (in order ep2in-bulk, ep1out-bulk, ep3in-int), INT notification seems to work I don't understand. Above you said that the EP definition in the UDC is USB_EP_IN_ISO(3). So how can you end up with ep3in-int? int != ISO. You should have ended up with the third endpoint being ep5in-int, because ep_matches() doesn't allow an isochronous to match a request for an interrupt endpoint. * modified configuration fails: [ 4259.609088] pxa27x-udc pxa27x-udc: ep15:pxa_ep_enable: type mismatch by this condition: http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L1416 because ep_matches() returns BULK. Okay, that's a problem in pxa27x-udc. Why does it insist on an exact match between the hardware endpoint type and the type contained in the descriptor? It should accept an interrupt descriptor if the hardware type is bulk. g_serial later disables INT notification [ 4259.609871] g_serial gadget: acm ttyGS0 can't notify serial state, -22 So this function is waiting regression, all it takes is just one change into the PXA27x EP configuration or change of allocation order for endpoints in a gadget. And it limits other existing gadget from being supported too (PXA can have only 23 endpoints including different altsetting/interface/cfg combinations). It could be easily fixed by gadget_is_pxa27x() function. Or one of the other techniques we have mentioned. The inability to use the same endpoint in more than one alternate setting is quite a nasty limitation, however. Alan Stern -- 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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On Sat, 25 Jul 2015, Robert Jarzmik wrote: Alan Stern st...@rowland.harvard.edu writes: Hi Alan, On Sat, 25 Jul 2015, Robert Jarzmik wrote: Petr Cvek petr.c...@tul.cz writes: On 23.7.2015 21:46, Alan Stern wrote: It seems that it allows using a BULK endpoint for requested INT endpoint. For my PXA27x machine the original code returns BULK EP even with valid INT endpoint definition (because BULK EPs are defined earlier than INT EPs). Yes, it does allow a bulk endpoint to be used when an interrupt endpoint was requested. However, it won't return a bulk endpoint if all the bulk endpoints are already in use. This cannot work for pxa27x. Do you mean that on pxa27x, a bulk endpoint cannot be used as an interrupt endpoint? Why not? From the device controller's point of view, there is no difference between bulk and interrupt (except possibly for the maxpacket sizes and high-bandwidth usage when running at high speed). That's the point, maxpacket size and priority. As you said, it's not that it won't work, it won't work with the priority expected by the software stack, ie. higher priority for ISO endpoint. As Robert Baldyga pointed out, this isn't relevant to a UDC driver. Only to a host controller driver. The pxa27x IP has a hardware limitation which prevents an endpoint from changing its type once the UDC is enabled (see the comment at the beginning of pxa27x_udc.c). If that patchset implies that for a requested INT endpoint a BULK endpoint can be returned, that won't work. Felipe and Robert, is that what this patchset implies ? Sort of. The matching code has always behaved that way and this patchset does not change the behavior. Then all is fine I suppose, if it was working before and nothing changes, it will continue to work, won't it ? It should. A default PXA27x configuration returns BULK for requested INT. Which is unfortunate, because PXA27x supports INT endpoints and has one predefined, but this function find BULK first (one BULK is allocated and INT is never used). See above. See response above. Besides, let's say the pxa27x has one bulk and one interrupt endpoint. Now suppose the gadget driver requests a bulk endpoint first. The matching code will allocate the single bulk endpoint. Then the gadget driver requests an interrupt endpoint. The matching code cannot allocate the bulk endpoint, because that endpoint is already allocated. So it will allocate the interrupt endpoint. Thus, as you can see, under the right conditions everything will work as desired. Let me give you another example : - pxa27x_udc offers 3 endpoints : ep-in, ep-out, ep-iso-in - a gadget driver does : - request an ep-in - request an ep-out - request an ep-in - request an ep-iso-in In that case, the ep-iso-in request will fail, right ? Yet I would have expected the second ep-in request to fail, as that's the one which cannot be serviced. In this example, the second ep-in request _will_ fail. An isochronous endpoint will not be allocated when the gadget driver requests a bulk endpoint. The bahavior that Petr didn't like was quite different: The matching code will sometimes allocate a bulk endpoint when the gadget driver requests an interrupt endpoint. Of course, this hypothetical case implies that pxa27x_udc is not compatible with this gadget driver, so it's not really relevant, is it ... Because if they do, the ep_matches() function works poorly. It returns a BULK for device (gadget) side, but host side (PC) thinks that this endpoint is an INT and handles it in this way. But the PXA27x thinks the endpoint is a BULK and handles it in its way (according to datasheet, settings for a BULK and an INT transfers are not 100% compatible). How do they differ? One example I have in mind is chapter 12.4.2 of pxa27x developer manual Endpoint Memory Configuration, quote follows : If the USB host controller transmits more OUT data than the maximum size packet for a bulk or interrupt endpoint, the UDC does not send any handshake to the host controller causing the host controller to time-out. If the USB host controller transmits more OUT data than the maximum size packet for an isochronous endpoint, the UDC sets the data packet error (DPE) bit in the Endpoint Control/Status register, UDCCSRx[DPE]. That's a difference between isochronous and bulk/interrupt. We are talking about the difference between bulk and interrupt. Alan Stern -- 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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
Hi, On 07/25/2015 07:04 PM, Robert Jarzmik wrote: Alan Stern st...@rowland.harvard.edu writes: Hi Alan, On Sat, 25 Jul 2015, Robert Jarzmik wrote: Petr Cvek petr.c...@tul.cz writes: On 23.7.2015 21:46, Alan Stern wrote: It seems that it allows using a BULK endpoint for requested INT endpoint. For my PXA27x machine the original code returns BULK EP even with valid INT endpoint definition (because BULK EPs are defined earlier than INT EPs). Yes, it does allow a bulk endpoint to be used when an interrupt endpoint was requested. However, it won't return a bulk endpoint if all the bulk endpoints are already in use. This cannot work for pxa27x. Do you mean that on pxa27x, a bulk endpoint cannot be used as an interrupt endpoint? Why not? From the device controller's point of view, there is no difference between bulk and interrupt (except possibly for the maxpacket sizes and high-bandwidth usage when running at high speed). That's the point, maxpacket size and priority. As you said, it's not that it won't work, it won't work with the priority expected by the software stack, ie. higher priority for ISO endpoint. Priority is not dependent on UDC hardware capabilities. Only USB host decides about priority, so there is no problem from UDC point of view. The pxa27x IP has a hardware limitation which prevents an endpoint from changing its type once the UDC is enabled (see the comment at the beginning of pxa27x_udc.c). If that patchset implies that for a requested INT endpoint a BULK endpoint can be returned, that won't work. Felipe and Robert, is that what this patchset implies ? Sort of. The matching code has always behaved that way and this patchset does not change the behavior. Then all is fine I suppose, if it was working before and nothing changes, it will continue to work, won't it ? A default PXA27x configuration returns BULK for requested INT. Which is unfortunate, because PXA27x supports INT endpoints and has one predefined, but this function find BULK first (one BULK is allocated and INT is never used). See above. See response above. Besides, let's say the pxa27x has one bulk and one interrupt endpoint. Now suppose the gadget driver requests a bulk endpoint first. The matching code will allocate the single bulk endpoint. Then the gadget driver requests an interrupt endpoint. The matching code cannot allocate the bulk endpoint, because that endpoint is already allocated. So it will allocate the interrupt endpoint. Thus, as you can see, under the right conditions everything will work as desired. Let me give you another example : - pxa27x_udc offers 3 endpoints : ep-in, ep-out, ep-iso-in - a gadget driver does : - request an ep-in - request an ep-out - request an ep-in - request an ep-iso-in In that case, the ep-iso-in request will fail, right ? Yet I would have expected the second ep-in request to fail, as that's the one which cannot be serviced. Gadget driver cannot simply request ep-in. Endpoints are matched with ep descriptors containing complete information about direction, type, maxpacketsize etc. of requested endpoint. So described situation can never take a place. However if gadget driver requests more endpoints than UDC driver supplies it will do fail ;) Current matching mechanism is very simple and surely will not always return optimal endpont set. Maybe we should try to develop something more sophisticated. Of course, this hypothetical case implies that pxa27x_udc is not compatible with this gadget driver, so it's not really relevant, is it ... Because if they do, the ep_matches() function works poorly. It returns a BULK for device (gadget) side, but host side (PC) thinks that this endpoint is an INT and handles it in this way. But the PXA27x thinks the endpoint is a BULK and handles it in its way (according to datasheet, settings for a BULK and an INT transfers are not 100% compatible). How do they differ? One example I have in mind is chapter 12.4.2 of pxa27x developer manual Endpoint Memory Configuration, quote follows : If the USB host controller transmits more OUT data than the maximum size packet for a bulk or interrupt endpoint, the UDC does not send any handshake to the host controller causing the host controller to time-out. If the USB host controller transmits more OUT data than the maximum size packet for an isochronous endpoint, the UDC sets the data packet error (DPE) bit in the Endpoint Control/Status register, UDCCSRx[DPE]. Perhaps you could submit a patch that adds a do not allocate a bulk endpoint when an interrupt endpoint is requested quirk flag to the usb_gadget structure, and modify the matching code to take the new flag into account. Well, if it was working that way already in the past, I don't see overloading the code with a quirk a necessity. My only need is that it continues to work. In this patchset I'm adding
Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On 25.7.2015 20:08, Robert Baldyga wrote: Let me give you another example : - pxa27x_udc offers 3 endpoints : ep-in, ep-out, ep-iso-in - a gadget driver does : - request an ep-in - request an ep-out - request an ep-in - request an ep-iso-in In that case, the ep-iso-in request will fail, right ? Yet I would have expected the second ep-in request to fail, as that's the one which cannot be serviced. Gadget driver cannot simply request ep-in. Endpoints are matched with ep descriptors containing complete information about direction, type, maxpacketsize etc. of requested endpoint. So described situation can never take a place. However if gadget driver requests more endpoints than UDC driver supplies it will do fail ;) Yes and returning of BULK instead of INT can cause it (only defined BULK gets eaten by requested INT). Current matching mechanism is very simple and surely will not always return optimal endpont set. Maybe we should try to develop something more sophisticated. I can test it (as I'm trying to get to work other gadgets like g_webcam, g_audio, g_hid and possibly function composites). Of course, this hypothetical case implies that pxa27x_udc is not compatible with this gadget driver, so it's not really relevant, is it ... Because if they do, the ep_matches() function works poorly. It returns a BULK for device (gadget) side, but host side (PC) thinks that this endpoint is an INT and handles it in this way. But the PXA27x thinks the endpoint is a BULK and handles it in its way (according to datasheet, settings for a BULK and an INT transfers are not 100% compatible). How do they differ? One example I have in mind is chapter 12.4.2 of pxa27x developer manual Endpoint Memory Configuration, quote follows : If the USB host controller transmits more OUT data than the maximum size packet for a bulk or interrupt endpoint, the UDC does not send any handshake to the host controller causing the host controller to time-out. If the USB host controller transmits more OUT data than the maximum size packet for an isochronous endpoint, the UDC sets the data packet error (DPE) bit in the Endpoint Control/Status register, UDCCSRx[DPE]. Perhaps you could submit a patch that adds a do not allocate a bulk endpoint when an interrupt endpoint is requested quirk flag to the usb_gadget structure, and modify the matching code to take the new flag into account. Well, if it was working that way already in the past, I don't see overloading the code with a quirk a necessity. My only need is that it continues to work. In this patchset I'm adding 'ep_match' callback to usb_gadget_ops, which can be used to supply non-standard matching algorithm, so there is no need for new quirk. Yeah that would be better, every UDC to handle its way. Cheers, Robert Petr -- 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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On 25.7.2015 19:04, Robert Jarzmik wrote: Alan Stern st...@rowland.harvard.edu writes: Hi Alan, On Sat, 25 Jul 2015, Robert Jarzmik wrote: Petr Cvek petr.c...@tul.cz writes: On 23.7.2015 21:46, Alan Stern wrote: It seems that it allows using a BULK endpoint for requested INT endpoint. For my PXA27x machine the original code returns BULK EP even with valid INT endpoint definition (because BULK EPs are defined earlier than INT EPs). Yes, it does allow a bulk endpoint to be used when an interrupt endpoint was requested. However, it won't return a bulk endpoint if all the bulk endpoints are already in use. This cannot work for pxa27x. Do you mean that on pxa27x, a bulk endpoint cannot be used as an interrupt endpoint? Why not? From the device controller's point of view, there is no difference between bulk and interrupt (except possibly for the maxpacket sizes and high-bandwidth usage when running at high speed). That's the point, maxpacket size and priority. As you said, it's not that it won't work, it won't work with the priority expected by the software stack, ie. higher priority for ISO endpoint. Yes, maxpacket could be problem. Datasheet has listed range (1-64) for INT and specific values (8, 16, 32, 64) for BULK. The pxa27x IP has a hardware limitation which prevents an endpoint from changing its type once the UDC is enabled (see the comment at the beginning of pxa27x_udc.c). If that patchset implies that for a requested INT endpoint a BULK endpoint can be returned, that won't work. Felipe and Robert, is that what this patchset implies ? Sort of. The matching code has always behaved that way and this patchset does not change the behavior. Then all is fine I suppose, if it was working before and nothing changes, it will continue to work, won't it ? Yes functional behavior of this patch is same as in vanilla, I only began this thread, because I have found out that someone is sending patchset. But I found this behavior when I was trying to use g_webcam gadget. A default PXA27x configuration returns BULK for requested INT. Which is unfortunate, because PXA27x supports INT endpoints and has one predefined, but this function find BULK first (one BULK is allocated and INT is never used). See above. See response above. Besides, let's say the pxa27x has one bulk and one interrupt endpoint. Now suppose the gadget driver requests a bulk endpoint first. The matching code will allocate the single bulk endpoint. Then the gadget driver requests an interrupt endpoint. The matching code cannot allocate the bulk endpoint, because that endpoint is already allocated. So it will allocate the interrupt endpoint. Thus, as you can see, under the right conditions everything will work as desired. Let me give you another example : - pxa27x_udc offers 3 endpoints : ep-in, ep-out, ep-iso-in - a gadget driver does : - request an ep-in - request an ep-out - request an ep-in - request an ep-iso-in In that case, the ep-iso-in request will fail, right ? Yet I would have expected the second ep-in request to fail, as that's the one which cannot be serviced. Of course, this hypothetical case implies that pxa27x_udc is not compatible with this gadget driver, so it's not really relevant, is it ... I have finally gathered enough information and luck (unstable machine) to try test g_serial so configuration: * modprobe g_serial use_acm=1 n_ports=1 * original version of ep_matches() (returns bulk and int) * compatible EP configuration/definition for UDC side http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L2352 USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_ISO(3), USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), * modified EP configuration for UDC side (just changed EP3 ISO to BULK, so there is one free BULK) USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_BULK(3), //change USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), ===results=== * original configuration is OK, all endpoints are found (in order ep2in-bulk, ep1out-bulk, ep3in-int), INT notification seems to work * modified configuration fails: [ 4259.609088] pxa27x-udc pxa27x-udc: ep15:pxa_ep_enable: type mismatch by this condition: http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L1416 because ep_matches() returns BULK. g_serial later disables INT notification [ 4259.609871] g_serial gadget: acm ttyGS0 can't notify serial state, -22 So this function is waiting regression, all it takes is just one change into the PXA27x EP configuration or change of allocation order for endpoints in a gadget. And it limits other existing gadget from being supported too (PXA can have only 23 endpoints including different altsetting/interface/cfg combinations). It could be easily
Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On 25.7.2015 13:04, Robert Jarzmik wrote: Petr Cvek petr.c...@tul.cz writes: On 23.7.2015 21:46, Alan Stern wrote: It seems that it allows using a BULK endpoint for requested INT endpoint. For my PXA27x machine the original code returns BULK EP even with valid INT endpoint definition (because BULK EPs are defined earlier than INT EPs). Yes, it does allow a bulk endpoint to be used when an interrupt endpoint was requested. However, it won't return a bulk endpoint if all the bulk endpoints are already in use. This cannot work for pxa27x. The pxa27x IP has a hardware limitation which prevents an endpoint from changing its type once the UDC is enabled (see the comment at the beginning of pxa27x_udc.c). Just crazy idea (based on how much I have to recompile kernel, when switching between testing gadgets), how much possible (EP allocation, no resource errors return, configuration/interface settings change) it would be to generate an EP configuration on the fly and only enabling the PXA27x when a set_configuration usb request is received? Petr -- 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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On 25.7.2015 23:36, Alan Stern wrote: On Sat, 25 Jul 2015, Petr Cvek wrote: On 25.7.2015 19:04, Robert Jarzmik wrote: Alan Stern st...@rowland.harvard.edu writes: Hi Alan, On Sat, 25 Jul 2015, Robert Jarzmik wrote: Petr Cvek petr.c...@tul.cz writes: On 23.7.2015 21:46, Alan Stern wrote: It seems that it allows using a BULK endpoint for requested INT endpoint. For my PXA27x machine the original code returns BULK EP even with valid INT endpoint definition (because BULK EPs are defined earlier than INT EPs). Yes, it does allow a bulk endpoint to be used when an interrupt endpoint was requested. However, it won't return a bulk endpoint if all the bulk endpoints are already in use. This cannot work for pxa27x. Do you mean that on pxa27x, a bulk endpoint cannot be used as an interrupt endpoint? Why not? From the device controller's point of view, there is no difference between bulk and interrupt (except possibly for the maxpacket sizes and high-bandwidth usage when running at high speed). That's the point, maxpacket size and priority. As you said, it's not that it won't work, it won't work with the priority expected by the software stack, ie. higher priority for ISO endpoint. Yes, maxpacket could be problem. Datasheet has listed range (1-64) for INT and specific values (8, 16, 32, 64) for BULK. In practice I doubt this will matter. Using a larger maxpacket size than the gadget driver expects is rarely important for interrupt transfers, since they almost never involve more than one packet's worth of data. So for example, if the gadget driver wants an interrupt endpoint with maxpacket 42, it almost certainly will work okay if it gets a bulk endpoint with maxpacket 64. What about higher speeds (not relevant on PXA, but ep_matches() is called from usb_ep_autoconfig_ss() )? According to http://wiki.osdev.org/Universal_Serial_Bus#Maximum_Data_Payload_Size_2 High speed INT endpoint has a maximum data payload 1024 B and BULK only 512 B (are other attributes of the data phase similar?). What about superspeed? If that patchset implies that for a requested INT endpoint a BULK endpoint can be returned, that won't work. Felipe and Robert, is that what this patchset implies ? Sort of. The matching code has always behaved that way and this patchset does not change the behavior. Then all is fine I suppose, if it was working before and nothing changes, it will continue to work, won't it ? Yes functional behavior of this patch is same as in vanilla, I only began this thread, because I have found out that someone is sending patchset. But I found this behavior when I was trying to use g_webcam gadget. I have finally gathered enough information and luck (unstable machine) to try test g_serial so configuration: * modprobe g_serial use_acm=1 n_ports=1 * original version of ep_matches() (returns bulk and int) * compatible EP configuration/definition for UDC side http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L2352 USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_ISO(3), USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), * modified EP configuration for UDC side (just changed EP3 ISO to BULK, so there is one free BULK) USB_EP_CTRL, USB_EP_OUT_BULK(1), USB_EP_IN_BULK(2), USB_EP_IN_BULK(3), //change USB_EP_OUT_ISO(4), USB_EP_IN_INT(5), ===results=== * original configuration is OK, all endpoints are found (in order ep2in-bulk, ep1out-bulk, ep3in-int), INT notification seems to work I don't understand. Above you said that the EP definition in the UDC is USB_EP_IN_ISO(3). So how can you end up with ep3in-int? int != ISO. You should have ended up with the third endpoint being ep5in-int, because ep_matches() doesn't allow an isochronous to match a request for an interrupt endpoint. I have changed definition of ISO to BULK only to accomplish minimal change of driver code (for my demonstration free BULK must be defined before INT - inserting new EP would require to reindex all next EPs and modifying links from PXA side endpoints). The USB_EP_IN_ISO(3) is just unused endpoint. * modified configuration fails: [ 4259.609088] pxa27x-udc pxa27x-udc: ep15:pxa_ep_enable: type mismatch by this condition: http://lxr.free-electrons.com/source/drivers/usb/gadget/udc/pxa27x_udc.c#L1416 because ep_matches() returns BULK. Okay, that's a problem in pxa27x-udc. Why does it insist on an exact match between the hardware endpoint type and the type contained in the descriptor? It should accept an interrupt descriptor if the hardware type is bulk. Hmm, making BULK EP equivalent with INT EP (when INT is requested) would made debugging (there is special bitfield in the config registers) and configuration preset (not anymore unordered set, but definition in the specific sequence) hell. But in other
Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
Petr Cvek petr.c...@tul.cz writes: On 23.7.2015 21:46, Alan Stern wrote: It seems that it allows using a BULK endpoint for requested INT endpoint. For my PXA27x machine the original code returns BULK EP even with valid INT endpoint definition (because BULK EPs are defined earlier than INT EPs). Yes, it does allow a bulk endpoint to be used when an interrupt endpoint was requested. However, it won't return a bulk endpoint if all the bulk endpoints are already in use. This cannot work for pxa27x. The pxa27x IP has a hardware limitation which prevents an endpoint from changing its type once the UDC is enabled (see the comment at the beginning of pxa27x_udc.c). If that patchset implies that for a requested INT endpoint a BULK endpoint can be returned, that won't work. Felipe and Robert, is that what this patchset implies ? A default PXA27x configuration returns BULK for requested INT. Which is unfortunate, because PXA27x supports INT endpoints and has one predefined, but this function find BULK first (one BULK is allocated and INT is never used). See above. Because if they do, the ep_matches() function works poorly. It returns a BULK for device (gadget) side, but host side (PC) thinks that this endpoint is an INT and handles it in this way. But the PXA27x thinks the endpoint is a BULK and handles it in its way (according to datasheet, settings for a BULK and an INT transfers are not 100% compatible). I cannot test INT as BULK behavior for the gadget functions, because all gadgets which works on PXA27x does not use INT endpoints (some allocate the endpoint but never use it). Ah a bit of history here. At least gadget zero does, and it's my main testing point for pxa27x_udc. Then there should be g_serial (no acm nor obex), but that's something I have not tried since 2009 ... For history also, there was already an attempt a long time ago for epautoconf revamping, done by Rodolfo Giometti IIRC. Anyway, I need pxa27x_udc to remain functional, so I'd like to understand if something will stop working, Robert B. Cheers. -- Robert -- 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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On 23.7.2015 16:36, Felipe Balbi wrote:
Hi,
On Thu, Jul 23, 2015 at 06:40:40AM +0200, Petr Cvek wrote:
Hello,
Is this:
case USB_ENDPOINT_XFER_INT:
/* Bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if (!ep-caps.type_int !ep-caps.type_bulk)
return 0;
... or original:
switch (type) {
case USB_ENDPOINT_XFER_INT:
/* bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if ('s' == tmp[2]) {// == -iso
return 0;
code still valid?
It seems that it allows using a BULK endpoint for requested INT
endpoint. For my PXA27x machine the original code returns BULK EP even
with valid INT endpoint definition (because BULK EPs are defined
earlier than INT EPs).
This part of the code is from pre git era
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
before pxa27x driver was written and only few chips was supported.
Does anyone know if the INT endpoints works now?
it's very difficult to read your reply when you remove all context.
Ah sorry, I was hacking around PXA UDC and found possible bug in one
ep_matches() function:
http://lxr.free-electrons.com/source/drivers/usb/gadget/epautoconf.c#L75
when searching for origin of this bug I have found about this new patch series
(someone could know how that part of code was created).
Petr Cvek
--
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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On 23.7.2015 21:46, Alan Stern wrote:
On Thu, 23 Jul 2015, Petr Cvek wrote:
Hello,
Is this:
case USB_ENDPOINT_XFER_INT:
/* Bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if (!ep-caps.type_int !ep-caps.type_bulk)
return 0;
... or original:
switch (type) {
case USB_ENDPOINT_XFER_INT:
/* bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if ('s' == tmp[2]) {// == -iso
return 0;
code still valid?
It's hard to understand your question. Are you asking whether this
code is still present in the current version of the kernel? You can
find out for yourself easily enough.
I'm asking if there are still UDCs which require this quirk. If not, we should
change it to:
if ('n' != tmp[2]) {// != -int
so it will only return INT endpoints. Or if there are one or two which does not
support interrupt endpoints, it would be (in my opinion) more practical to make
a special case for them than force all other UDCs to use a BULK endpoint when
they have an INT endpoint.
It seems that it allows using a BULK endpoint for requested INT
endpoint. For my PXA27x machine the original code returns BULK EP
even with valid INT endpoint definition (because BULK EPs are defined
earlier than INT EPs).
Yes, it does allow a bulk endpoint to be used when an interrupt
endpoint was requested. However, it won't return a bulk endpoint if
all the bulk endpoints are already in use.
A default PXA27x configuration returns BULK for requested INT. Which is
unfortunate, because PXA27x supports INT endpoints and has one predefined, but
this function find BULK first (one BULK is allocated and INT is never used).
This part of the code is from pre git era
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
before pxa27x driver was written and only few chips was supported.
Does anyone know if the INT endpoints works now?
What makes you think they might not work?
Because if they do, the ep_matches() function works poorly. It returns a BULK
for device (gadget) side, but host side (PC) thinks that this endpoint is an
INT and handles it in this way. But the PXA27x thinks the endpoint is a BULK
and handles it in its way (according to datasheet, settings for a BULK and an
INT transfers are not 100% compatible).
I cannot test INT as BULK behavior for the gadget functions, because all
gadgets which works on PXA27x does not use INT endpoints (some allocate the
endpoint but never use it).
Alan Stern
Petr Cvek
--
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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On Thu, 23 Jul 2015, Petr Cvek wrote:
Hello,
Is this:
case USB_ENDPOINT_XFER_INT:
/* Bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if (!ep-caps.type_int !ep-caps.type_bulk)
return 0;
... or original:
switch (type) {
case USB_ENDPOINT_XFER_INT:
/* bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if ('s' == tmp[2]) {// == -iso
return 0;
code still valid?
It's hard to understand your question. Are you asking whether this
code is still present in the current version of the kernel? You can
find out for yourself easily enough.
It seems that it allows using a BULK endpoint for requested INT
endpoint. For my PXA27x machine the original code returns BULK EP
even with valid INT endpoint definition (because BULK EPs are defined
earlier than INT EPs).
Yes, it does allow a bulk endpoint to be used when an interrupt
endpoint was requested. However, it won't return a bulk endpoint if
all the bulk endpoints are already in use.
This part of the code is from pre git era
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
before pxa27x driver was written and only few chips was supported.
Does anyone know if the INT endpoints works now?
What makes you think they might not work?
Alan Stern
--
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: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
Hi,
On Thu, Jul 23, 2015 at 06:40:40AM +0200, Petr Cvek wrote:
Hello,
Is this:
case USB_ENDPOINT_XFER_INT:
/* Bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if (!ep-caps.type_int !ep-caps.type_bulk)
return 0;
... or original:
switch (type) {
case USB_ENDPOINT_XFER_INT:
/* bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if ('s' == tmp[2]) {// == -iso
return 0;
code still valid?
It seems that it allows using a BULK endpoint for requested INT
endpoint. For my PXA27x machine the original code returns BULK EP even
with valid INT endpoint definition (because BULK EPs are defined
earlier than INT EPs).
This part of the code is from pre git era
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
before pxa27x driver was written and only few chips was supported.
Does anyone know if the INT endpoints works now?
it's very difficult to read your reply when you remove all context.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
Hello,
Is this:
case USB_ENDPOINT_XFER_INT:
/* Bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if (!ep-caps.type_int !ep-caps.type_bulk)
return 0;
... or original:
switch (type) {
case USB_ENDPOINT_XFER_INT:
/* bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if ('s' == tmp[2]) {// == -iso
return 0;
code still valid?
It seems that it allows using a BULK endpoint for requested INT endpoint. For
my PXA27x machine the original code returns BULK EP even with valid INT
endpoint definition (because BULK EPs are defined earlier than INT EPs).
This part of the code is from pre git era
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
before pxa27x driver was written and only few chips was supported. Does anyone
know if the INT endpoints works now?
Petr Cvek
--
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
