[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Felipe Balbi]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150723/b205a82f/attachment.sig>

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Alan Stern]

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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Petr Cvek]

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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Petr Cvek]

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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Robert Jarzmik]

Petr Cvek <petr.cvek@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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Alan Stern]

On Sat, 25 Jul 2015, Robert Jarzmik wrote:

> Petr Cvek <petr.cvek@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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Robert Jarzmik]

Alan Stern <stern@rowland.harvard.edu> writes:

Hi Alan,

> On Sat, 25 Jul 2015, Robert Jarzmik wrote:
>
>> Petr Cvek <petr.cvek@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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Robert Baldyga]

Hi,

On 07/25/2015 07:04 PM, Robert Jarzmik wrote:
> Alan Stern <stern@rowland.harvard.edu> writes:
>
> Hi Alan,
>
>> On Sat, 25 Jul 2015, Robert Jarzmik wrote:
>>
>>> Petr Cvek <petr.cvek@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 '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.

Cheers,
Robert

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Petr Cvek]

On 25.7.2015 19:04, Robert Jarzmik wrote:
> Alan Stern <stern@rowland.harvard.edu> writes:
> 
> Hi Alan,
> 
>> On Sat, 25 Jul 2015, Robert Jarzmik wrote:
>>
>>> Petr Cvek <petr.cvek@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 fixed by gadget_is_pxa27x() function. 

Petr

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Petr Cvek]

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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Petr Cvek]

On 25.7.2015 13:04, Robert Jarzmik wrote:
> Petr Cvek <petr.cvek@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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Alan Stern]

On Sat, 25 Jul 2015, Robert Jarzmik wrote:

> Alan Stern <stern@rowland.harvard.edu> writes:
> 
> Hi Alan,
> 
> > On Sat, 25 Jul 2015, Robert Jarzmik wrote:
> >
> >> Petr Cvek <petr.cvek@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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Alan Stern]

On Sat, 25 Jul 2015, Petr Cvek wrote:

> On 25.7.2015 19:04, Robert Jarzmik wrote:
> > Alan Stern <stern@rowland.harvard.edu> writes:
> > 
> > Hi Alan,
> > 
> >> On Sat, 25 Jul 2015, Robert Jarzmik wrote:
> >>
> >>> Petr Cvek <petr.cvek@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

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Petr Cvek]

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 <stern@rowland.harvard.edu> writes:
>>>
>>> Hi Alan,
>>>
>>>> On Sat, 25 Jul 2015, Robert Jarzmik wrote:
>>>>
>>>>> Petr Cvek <petr.cvek@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 ways it can be OK (specification does not say that using EP marked INT as BULK will fail).

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

I think optimal idea is custom matcher function. It would eliminate codes for superspeed checking on SoC which known only fullspeed ;-).


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 endpoint to print messages and register setting)
	fsl_qe_udc.c (different maxpacket sizes for highspeed)
	udc-xilinx.c (maching of different maxpacket sizes)
	omap_udc.c (maxpacket size definitions)
	
> 
> Alan Stern
> 

Petr Cvek

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Alan Stern]

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 endpoint to print messages and register setting)
> 	fsl_qe_udc.c (different maxpacket sizes for highspeed)
> 	udc-xilinx.c (maching of different maxpacket sizes)
> 	omap_udc.c (maxpacket size definitions)

The fact that bulk and interrupt are handled differently doesn't mean 
that you aren't allowed to allocate a bulk endpoint when the gadget 
driver quested an interrupt endpoint.

On the other hand, it certainly would be better to do this only when no 
interrupt endpoints remain available.  As it stands now, 
usb_ep_autoconfig_ss() uses the first match it finds even if there is a 
better match later on.

Alan Stern

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Petr Cvek]

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 can be called even from custom matcher (only quirks).

>> 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 endpoint to print messages and register setting)
>> 	fsl_qe_udc.c (different maxpacket sizes for highspeed)
>> 	udc-xilinx.c (maching of different maxpacket sizes)
>> 	omap_udc.c (maxpacket size definitions)
> 
> The fact that bulk and interrupt are handled differently doesn't mean 
> that you aren't allowed to allocate a bulk endpoint when the gadget 
> driver quested an interrupt endpoint.

Yeah, I was just curious, although PXA25x have endpoint HW handling really different. It has exception in ep_matches() and it cannot use INT endpoint but only BULK endpoint for INT communication. As PXA25x was one of few UDCs at the time of using "BULK as INT". I thought that newer UDCs does not have these problems anymore (like not enough INT endpoints).

> 
> On the other hand, it certainly would be better to do this only when no 
> interrupt endpoints remain available.  As it stands now, 
> usb_ep_autoconfig_ss() uses the first match it finds even if there is a 
> better match later on.

There could be two passes, one for INT and second for BULK. But it would require ep_matches() to return only INT for INT request. ;-)

> 
> Alan Stern
> 

Petr Cvek

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Robert Baldyga]

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 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 endpoint to print messages and register setting)
>> 	fsl_qe_udc.c (different maxpacket sizes for highspeed)
>> 	udc-xilinx.c (maching of different maxpacket sizes)
>> 	omap_udc.c (maxpacket size definitions)
>
> The fact that bulk and interrupt are handled differently doesn't mean
> that you aren't allowed to allocate a bulk endpoint when the gadget
> driver quested an interrupt endpoint.
>
> On the other hand, it certainly would be better to do this only when no
> interrupt endpoints remain available.  As it stands now,
> usb_ep_autoconfig_ss() uses the first match it finds even if there is a
> better match later on.

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Robert Baldyga]

Move ep_matches() function to udc-core and rename it to
usb_gadget_ep_match_desc(). This function can be used by UDC drivers
in 'match_ep' callback to avoid writing lots of repetitive code.

Replace all calls of ep_matches() with usb_gadget_ep_match_desc().

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/epautoconf.c   | 95 +++++----------------------------------
 drivers/usb/gadget/udc/udc-core.c | 69 ++++++++++++++++++++++++++++
 include/linux/usb/gadget.h        |  8 ++++
 3 files changed, 88 insertions(+), 84 deletions(-)

diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index 1b1fee0..3f0a380 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -22,82 +22,6 @@
 
 #include "gadget_chips.h"
 
-static int
-ep_matches (
-	struct usb_gadget		*gadget,
-	struct usb_ep			*ep,
-	struct usb_endpoint_descriptor	*desc,
-	struct usb_ss_ep_comp_descriptor *ep_comp
-)
-{
-	u8              type;
-	u16             max;
-	int             num_req_streams = 0;
-
-	/* endpoint already claimed? */
-	if (ep->claimed)
-		return 0;
-
-	type = usb_endpoint_type(desc);
-	max = 0x7ff & usb_endpoint_maxp(desc);
-
-	if (usb_endpoint_dir_in(desc) && !ep->caps.dir_in)
-		return 0;
-	else if (!ep->caps.dir_out)
-		return 0;
-
-	if (max > ep->maxpacket_limit)
-		return 0;
-
-	/* "high bandwidth" works only at high speed */
-	if (!gadget_is_dualspeed(gadget) && usb_endpoint_maxp(desc) & (3<<11))
-		return 0;
-
-	switch (type) {
-	case USB_ENDPOINT_XFER_CONTROL:
-		/* only support ep0 for portable CONTROL traffic */
-		return 0;
-	case USB_ENDPOINT_XFER_ISOC:
-		if (!ep->caps.type_iso)
-			return 0;
-		/* ISO:  limit 1023 bytes full speed,
-		 * 1024 high/super speed
-		 */
-		if (!gadget_is_dualspeed(gadget) && max > 1023)
-			return 0;
-		break;
-	case USB_ENDPOINT_XFER_BULK:
-		if (!ep->caps.type_bulk)
-			return 0;
-		if (ep_comp && gadget_is_superspeed(gadget)) {
-			/* Get the number of required streams from the
-			 * EP companion descriptor and see if the EP
-			 * matches it
-			 */
-			num_req_streams = ep_comp->bmAttributes & 0x1f;
-			if (num_req_streams > ep->max_streams)
-				return 0;
-		}
-		break;
-	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;
-		/* INT:  limit 64 bytes full speed,
-		 * 1024 high/super speed
-		 */
-		if (!gadget_is_dualspeed(gadget) && max > 64)
-			return 0;
-		break;
-	}
-
-	/* MATCH!! */
-
-	return 1;
-}
-
 static struct usb_ep *
 find_ep (struct usb_gadget *gadget, const char *name)
 {
@@ -180,10 +104,12 @@ struct usb_ep *usb_ep_autoconfig_ss(
 		if (type == USB_ENDPOINT_XFER_INT) {
 			/* ep-e, ep-f are PIO with only 64 byte fifos */
 			ep = find_ep(gadget, "ep-e");
-			if (ep && ep_matches(gadget, ep, desc, ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 			ep = find_ep(gadget, "ep-f");
-			if (ep && ep_matches(gadget, ep, desc, ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 		}
 
@@ -191,20 +117,21 @@ struct usb_ep *usb_ep_autoconfig_ss(
 		snprintf(name, sizeof(name), "ep%d%s", usb_endpoint_num(desc),
 				usb_endpoint_dir_in(desc) ? "in" : "out");
 		ep = find_ep(gadget, name);
-		if (ep && ep_matches(gadget, ep, desc, ep_comp))
+		if (ep && usb_gadget_ep_match_desc(gadget, ep, desc, ep_comp))
 			goto found_ep;
 	} else if (gadget_is_goku (gadget)) {
 		if (USB_ENDPOINT_XFER_INT == type) {
 			/* single buffering is enough */
 			ep = find_ep(gadget, "ep3-bulk");
-			if (ep && ep_matches(gadget, ep, desc, ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 		} else if (USB_ENDPOINT_XFER_BULK == type
 				&& (USB_DIR_IN & desc->bEndpointAddress)) {
 			/* DMA may be available */
 			ep = find_ep(gadget, "ep2-bulk");
-			if (ep && ep_matches(gadget, ep, desc,
-					      ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 		}
 
@@ -223,14 +150,14 @@ struct usb_ep *usb_ep_autoconfig_ss(
 				ep = find_ep(gadget, "ep2out");
 		} else
 			ep = NULL;
-		if (ep && ep_matches(gadget, ep, desc, ep_comp))
+		if (ep && usb_gadget_ep_match_desc(gadget, ep, desc, ep_comp))
 			goto found_ep;
 #endif
 	}
 
 	/* Second, look at endpoints until an unclaimed one looks usable */
 	list_for_each_entry (ep, &gadget->ep_list, ep_list) {
-		if (ep_matches(gadget, ep, desc, ep_comp))
+		if (usb_gadget_ep_match_desc(gadget, ep, desc, ep_comp))
 			goto found_ep;
 	}
 
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index d69c355..2dff8df 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -129,6 +129,75 @@ EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
 
 /* ------------------------------------------------------------------------- */
 
+int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
+		struct usb_ep *ep, struct usb_endpoint_descriptor *desc,
+		struct usb_ss_ep_comp_descriptor *ep_comp)
+{
+	u8              type;
+	u16             max;
+	int             num_req_streams = 0;
+
+	/* endpoint already claimed? */
+	if (ep->claimed)
+		return 0;
+
+	type = usb_endpoint_type(desc);
+	max = 0x7ff & usb_endpoint_maxp(desc);
+
+	if (usb_endpoint_dir_in(desc) && !ep->caps.dir_in)
+		return 0;
+	else if (!ep->caps.dir_out)
+		return 0;
+
+	if (max > ep->maxpacket_limit)
+		return 0;
+
+	/* "high bandwidth" works only at high speed */
+	if (!gadget_is_dualspeed(gadget) && usb_endpoint_maxp(desc) & (3<<11))
+		return 0;
+
+	switch (type) {
+	case USB_ENDPOINT_XFER_CONTROL:
+		/* only support ep0 for portable CONTROL traffic */
+		return 0;
+	case USB_ENDPOINT_XFER_ISOC:
+		if (!ep->caps.type_iso)
+			return 0;
+		/* ISO:  limit 1023 bytes full speed, 1024 high/super speed */
+		if (!gadget_is_dualspeed(gadget) && max > 1023)
+			return 0;
+		break;
+	case USB_ENDPOINT_XFER_BULK:
+		if (!ep->caps.type_bulk)
+			return 0;
+		if (ep_comp && gadget_is_superspeed(gadget)) {
+			/* Get the number of required streams from the
+			 * EP companion descriptor and see if the EP
+			 * matches it
+			 */
+			num_req_streams = ep_comp->bmAttributes & 0x1f;
+			if (num_req_streams > ep->max_streams)
+				return 0;
+		}
+		break;
+	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;
+		/* INT:  limit 64 bytes full speed, 1024 high/super speed */
+		if (!gadget_is_dualspeed(gadget) && max > 64)
+			return 0;
+		break;
+	}
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_ep_match_desc);
+
+/* ------------------------------------------------------------------------- */
+
 static void usb_gadget_state_work(struct work_struct *work)
 {
 	struct usb_gadget *gadget = work_to_gadget(work);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 0041bb9..77e2c1e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1079,6 +1079,14 @@ extern void usb_gadget_giveback_request(struct usb_ep *ep,
 
 /*-------------------------------------------------------------------------*/
 
+/* utility to check if endpoint caps match descriptor needs */
+
+extern int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
+		struct usb_ep *ep, struct usb_endpoint_descriptor *desc,
+		struct usb_ss_ep_comp_descriptor *ep_comp);
+
+/*-------------------------------------------------------------------------*/
+
 /* utility to update vbus status for udc core, it may be scheduled */
 extern void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status);
 
-- 
1.9.1

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Robert Baldyga]

Move ep_matches() function to udc-core and rename it to
usb_gadget_ep_match_desc(). This function can be used by UDC drivers
in 'match_ep' callback to avoid writing lots of repetitive code.

Replace all calls of ep_matches() with usb_gadget_ep_match_desc().

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/epautoconf.c   | 95 +++++----------------------------------
 drivers/usb/gadget/udc/udc-core.c | 69 ++++++++++++++++++++++++++++
 include/linux/usb/gadget.h        |  8 ++++
 3 files changed, 88 insertions(+), 84 deletions(-)

diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index 1b1fee0..3f0a380 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -22,82 +22,6 @@
 
 #include "gadget_chips.h"
 
-static int
-ep_matches (
-	struct usb_gadget		*gadget,
-	struct usb_ep			*ep,
-	struct usb_endpoint_descriptor	*desc,
-	struct usb_ss_ep_comp_descriptor *ep_comp
-)
-{
-	u8              type;
-	u16             max;
-	int             num_req_streams = 0;
-
-	/* endpoint already claimed? */
-	if (ep->claimed)
-		return 0;
-
-	type = usb_endpoint_type(desc);
-	max = 0x7ff & usb_endpoint_maxp(desc);
-
-	if (usb_endpoint_dir_in(desc) && !ep->caps.dir_in)
-		return 0;
-	else if (!ep->caps.dir_out)
-		return 0;
-
-	if (max > ep->maxpacket_limit)
-		return 0;
-
-	/* "high bandwidth" works only at high speed */
-	if (!gadget_is_dualspeed(gadget) && usb_endpoint_maxp(desc) & (3<<11))
-		return 0;
-
-	switch (type) {
-	case USB_ENDPOINT_XFER_CONTROL:
-		/* only support ep0 for portable CONTROL traffic */
-		return 0;
-	case USB_ENDPOINT_XFER_ISOC:
-		if (!ep->caps.type_iso)
-			return 0;
-		/* ISO:  limit 1023 bytes full speed,
-		 * 1024 high/super speed
-		 */
-		if (!gadget_is_dualspeed(gadget) && max > 1023)
-			return 0;
-		break;
-	case USB_ENDPOINT_XFER_BULK:
-		if (!ep->caps.type_bulk)
-			return 0;
-		if (ep_comp && gadget_is_superspeed(gadget)) {
-			/* Get the number of required streams from the
-			 * EP companion descriptor and see if the EP
-			 * matches it
-			 */
-			num_req_streams = ep_comp->bmAttributes & 0x1f;
-			if (num_req_streams > ep->max_streams)
-				return 0;
-		}
-		break;
-	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;
-		/* INT:  limit 64 bytes full speed,
-		 * 1024 high/super speed
-		 */
-		if (!gadget_is_dualspeed(gadget) && max > 64)
-			return 0;
-		break;
-	}
-
-	/* MATCH!! */
-
-	return 1;
-}
-
 static struct usb_ep *
 find_ep (struct usb_gadget *gadget, const char *name)
 {
@@ -180,10 +104,12 @@ struct usb_ep *usb_ep_autoconfig_ss(
 		if (type == USB_ENDPOINT_XFER_INT) {
 			/* ep-e, ep-f are PIO with only 64 byte fifos */
 			ep = find_ep(gadget, "ep-e");
-			if (ep && ep_matches(gadget, ep, desc, ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 			ep = find_ep(gadget, "ep-f");
-			if (ep && ep_matches(gadget, ep, desc, ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 		}
 
@@ -191,20 +117,21 @@ struct usb_ep *usb_ep_autoconfig_ss(
 		snprintf(name, sizeof(name), "ep%d%s", usb_endpoint_num(desc),
 				usb_endpoint_dir_in(desc) ? "in" : "out");
 		ep = find_ep(gadget, name);
-		if (ep && ep_matches(gadget, ep, desc, ep_comp))
+		if (ep && usb_gadget_ep_match_desc(gadget, ep, desc, ep_comp))
 			goto found_ep;
 	} else if (gadget_is_goku (gadget)) {
 		if (USB_ENDPOINT_XFER_INT == type) {
 			/* single buffering is enough */
 			ep = find_ep(gadget, "ep3-bulk");
-			if (ep && ep_matches(gadget, ep, desc, ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 		} else if (USB_ENDPOINT_XFER_BULK == type
 				&& (USB_DIR_IN & desc->bEndpointAddress)) {
 			/* DMA may be available */
 			ep = find_ep(gadget, "ep2-bulk");
-			if (ep && ep_matches(gadget, ep, desc,
-					      ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 		}
 
@@ -223,14 +150,14 @@ struct usb_ep *usb_ep_autoconfig_ss(
 				ep = find_ep(gadget, "ep2out");
 		} else
 			ep = NULL;
-		if (ep && ep_matches(gadget, ep, desc, ep_comp))
+		if (ep && usb_gadget_ep_match_desc(gadget, ep, desc, ep_comp))
 			goto found_ep;
 #endif
 	}
 
 	/* Second, look at endpoints until an unclaimed one looks usable */
 	list_for_each_entry (ep, &gadget->ep_list, ep_list) {
-		if (ep_matches(gadget, ep, desc, ep_comp))
+		if (usb_gadget_ep_match_desc(gadget, ep, desc, ep_comp))
 			goto found_ep;
 	}
 
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index d69c355..2dff8df 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -129,6 +129,75 @@ EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
 
 /* ------------------------------------------------------------------------- */
 
+int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
+		struct usb_ep *ep, struct usb_endpoint_descriptor *desc,
+		struct usb_ss_ep_comp_descriptor *ep_comp)
+{
+	u8              type;
+	u16             max;
+	int             num_req_streams = 0;
+
+	/* endpoint already claimed? */
+	if (ep->claimed)
+		return 0;
+
+	type = usb_endpoint_type(desc);
+	max = 0x7ff & usb_endpoint_maxp(desc);
+
+	if (usb_endpoint_dir_in(desc) && !ep->caps.dir_in)
+		return 0;
+	else if (!ep->caps.dir_out)
+		return 0;
+
+	if (max > ep->maxpacket_limit)
+		return 0;
+
+	/* "high bandwidth" works only at high speed */
+	if (!gadget_is_dualspeed(gadget) && usb_endpoint_maxp(desc) & (3<<11))
+		return 0;
+
+	switch (type) {
+	case USB_ENDPOINT_XFER_CONTROL:
+		/* only support ep0 for portable CONTROL traffic */
+		return 0;
+	case USB_ENDPOINT_XFER_ISOC:
+		if (!ep->caps.type_iso)
+			return 0;
+		/* ISO:  limit 1023 bytes full speed, 1024 high/super speed */
+		if (!gadget_is_dualspeed(gadget) && max > 1023)
+			return 0;
+		break;
+	case USB_ENDPOINT_XFER_BULK:
+		if (!ep->caps.type_bulk)
+			return 0;
+		if (ep_comp && gadget_is_superspeed(gadget)) {
+			/* Get the number of required streams from the
+			 * EP companion descriptor and see if the EP
+			 * matches it
+			 */
+			num_req_streams = ep_comp->bmAttributes & 0x1f;
+			if (num_req_streams > ep->max_streams)
+				return 0;
+		}
+		break;
+	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;
+		/* INT:  limit 64 bytes full speed, 1024 high/super speed */
+		if (!gadget_is_dualspeed(gadget) && max > 64)
+			return 0;
+		break;
+	}
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_ep_match_desc);
+
+/* ------------------------------------------------------------------------- */
+
 static void usb_gadget_state_work(struct work_struct *work)
 {
 	struct usb_gadget *gadget = work_to_gadget(work);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 0041bb9..77e2c1e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1079,6 +1079,14 @@ extern void usb_gadget_giveback_request(struct usb_ep *ep,
 
 /*-------------------------------------------------------------------------*/
 
+/* utility to check if endpoint caps match descriptor needs */
+
+extern int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
+		struct usb_ep *ep, struct usb_endpoint_descriptor *desc,
+		struct usb_ss_ep_comp_descriptor *ep_comp);
+
+/*-------------------------------------------------------------------------*/
+
 /* utility to update vbus status for udc core, it may be scheduled */
 extern void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status);
 
-- 
1.9.1

[PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core

[Robert Baldyga]

Move ep_matches() function to udc-core and rename it to
usb_gadget_ep_match_desc(). This function can be used by UDC drivers
in 'match_ep' callback to avoid writing lots of repetitive code.

Replace all calls of ep_matches() with usb_gadget_ep_match_desc().

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/epautoconf.c   | 95 +++++----------------------------------
 drivers/usb/gadget/udc/udc-core.c | 69 ++++++++++++++++++++++++++++
 include/linux/usb/gadget.h        |  8 ++++
 3 files changed, 88 insertions(+), 84 deletions(-)

diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index 1b1fee0..3f0a380 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -22,82 +22,6 @@
 
 #include "gadget_chips.h"
 
-static int
-ep_matches (
-	struct usb_gadget		*gadget,
-	struct usb_ep			*ep,
-	struct usb_endpoint_descriptor	*desc,
-	struct usb_ss_ep_comp_descriptor *ep_comp
-)
-{
-	u8              type;
-	u16             max;
-	int             num_req_streams = 0;
-
-	/* endpoint already claimed? */
-	if (ep->claimed)
-		return 0;
-
-	type = usb_endpoint_type(desc);
-	max = 0x7ff & usb_endpoint_maxp(desc);
-
-	if (usb_endpoint_dir_in(desc) && !ep->caps.dir_in)
-		return 0;
-	else if (!ep->caps.dir_out)
-		return 0;
-
-	if (max > ep->maxpacket_limit)
-		return 0;
-
-	/* "high bandwidth" works only at high speed */
-	if (!gadget_is_dualspeed(gadget) && usb_endpoint_maxp(desc) & (3<<11))
-		return 0;
-
-	switch (type) {
-	case USB_ENDPOINT_XFER_CONTROL:
-		/* only support ep0 for portable CONTROL traffic */
-		return 0;
-	case USB_ENDPOINT_XFER_ISOC:
-		if (!ep->caps.type_iso)
-			return 0;
-		/* ISO:  limit 1023 bytes full speed,
-		 * 1024 high/super speed
-		 */
-		if (!gadget_is_dualspeed(gadget) && max > 1023)
-			return 0;
-		break;
-	case USB_ENDPOINT_XFER_BULK:
-		if (!ep->caps.type_bulk)
-			return 0;
-		if (ep_comp && gadget_is_superspeed(gadget)) {
-			/* Get the number of required streams from the
-			 * EP companion descriptor and see if the EP
-			 * matches it
-			 */
-			num_req_streams = ep_comp->bmAttributes & 0x1f;
-			if (num_req_streams > ep->max_streams)
-				return 0;
-		}
-		break;
-	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;
-		/* INT:  limit 64 bytes full speed,
-		 * 1024 high/super speed
-		 */
-		if (!gadget_is_dualspeed(gadget) && max > 64)
-			return 0;
-		break;
-	}
-
-	/* MATCH!! */
-
-	return 1;
-}
-
 static struct usb_ep *
 find_ep (struct usb_gadget *gadget, const char *name)
 {
@@ -180,10 +104,12 @@ struct usb_ep *usb_ep_autoconfig_ss(
 		if (type == USB_ENDPOINT_XFER_INT) {
 			/* ep-e, ep-f are PIO with only 64 byte fifos */
 			ep = find_ep(gadget, "ep-e");
-			if (ep && ep_matches(gadget, ep, desc, ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 			ep = find_ep(gadget, "ep-f");
-			if (ep && ep_matches(gadget, ep, desc, ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 		}
 
@@ -191,20 +117,21 @@ struct usb_ep *usb_ep_autoconfig_ss(
 		snprintf(name, sizeof(name), "ep%d%s", usb_endpoint_num(desc),
 				usb_endpoint_dir_in(desc) ? "in" : "out");
 		ep = find_ep(gadget, name);
-		if (ep && ep_matches(gadget, ep, desc, ep_comp))
+		if (ep && usb_gadget_ep_match_desc(gadget, ep, desc, ep_comp))
 			goto found_ep;
 	} else if (gadget_is_goku (gadget)) {
 		if (USB_ENDPOINT_XFER_INT == type) {
 			/* single buffering is enough */
 			ep = find_ep(gadget, "ep3-bulk");
-			if (ep && ep_matches(gadget, ep, desc, ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 		} else if (USB_ENDPOINT_XFER_BULK == type
 				&& (USB_DIR_IN & desc->bEndpointAddress)) {
 			/* DMA may be available */
 			ep = find_ep(gadget, "ep2-bulk");
-			if (ep && ep_matches(gadget, ep, desc,
-					      ep_comp))
+			if (ep && usb_gadget_ep_match_desc(gadget,
+					ep, desc, ep_comp))
 				goto found_ep;
 		}
 
@@ -223,14 +150,14 @@ struct usb_ep *usb_ep_autoconfig_ss(
 				ep = find_ep(gadget, "ep2out");
 		} else
 			ep = NULL;
-		if (ep && ep_matches(gadget, ep, desc, ep_comp))
+		if (ep && usb_gadget_ep_match_desc(gadget, ep, desc, ep_comp))
 			goto found_ep;
 #endif
 	}
 
 	/* Second, look at endpoints until an unclaimed one looks usable */
 	list_for_each_entry (ep, &gadget->ep_list, ep_list) {
-		if (ep_matches(gadget, ep, desc, ep_comp))
+		if (usb_gadget_ep_match_desc(gadget, ep, desc, ep_comp))
 			goto found_ep;
 	}
 
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index d69c355..2dff8df 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -129,6 +129,75 @@ EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
 
 /* ------------------------------------------------------------------------- */
 
+int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
+		struct usb_ep *ep, struct usb_endpoint_descriptor *desc,
+		struct usb_ss_ep_comp_descriptor *ep_comp)
+{
+	u8              type;
+	u16             max;
+	int             num_req_streams = 0;
+
+	/* endpoint already claimed? */
+	if (ep->claimed)
+		return 0;
+
+	type = usb_endpoint_type(desc);
+	max = 0x7ff & usb_endpoint_maxp(desc);
+
+	if (usb_endpoint_dir_in(desc) && !ep->caps.dir_in)
+		return 0;
+	else if (!ep->caps.dir_out)
+		return 0;
+
+	if (max > ep->maxpacket_limit)
+		return 0;
+
+	/* "high bandwidth" works only at high speed */
+	if (!gadget_is_dualspeed(gadget) && usb_endpoint_maxp(desc) & (3<<11))
+		return 0;
+
+	switch (type) {
+	case USB_ENDPOINT_XFER_CONTROL:
+		/* only support ep0 for portable CONTROL traffic */
+		return 0;
+	case USB_ENDPOINT_XFER_ISOC:
+		if (!ep->caps.type_iso)
+			return 0;
+		/* ISO:  limit 1023 bytes full speed, 1024 high/super speed */
+		if (!gadget_is_dualspeed(gadget) && max > 1023)
+			return 0;
+		break;
+	case USB_ENDPOINT_XFER_BULK:
+		if (!ep->caps.type_bulk)
+			return 0;
+		if (ep_comp && gadget_is_superspeed(gadget)) {
+			/* Get the number of required streams from the
+			 * EP companion descriptor and see if the EP
+			 * matches it
+			 */
+			num_req_streams = ep_comp->bmAttributes & 0x1f;
+			if (num_req_streams > ep->max_streams)
+				return 0;
+		}
+		break;
+	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;
+		/* INT:  limit 64 bytes full speed, 1024 high/super speed */
+		if (!gadget_is_dualspeed(gadget) && max > 64)
+			return 0;
+		break;
+	}
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_ep_match_desc);
+
+/* ------------------------------------------------------------------------- */
+
 static void usb_gadget_state_work(struct work_struct *work)
 {
 	struct usb_gadget *gadget = work_to_gadget(work);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 0041bb9..77e2c1e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1079,6 +1079,14 @@ extern void usb_gadget_giveback_request(struct usb_ep *ep,
 
 /*-------------------------------------------------------------------------*/
 
+/* utility to check if endpoint caps match descriptor needs */
+
+extern int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
+		struct usb_ep *ep, struct usb_endpoint_descriptor *desc,
+		struct usb_ss_ep_comp_descriptor *ep_comp);
+
+/*-------------------------------------------------------------------------*/
+
 /* utility to update vbus status for udc core, it may be scheduled */
 extern void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status);
 
-- 
1.9.1