diff mbox series

[v10,2/6] usb: gadget: configfs: Check USB configuration before adding

Message ID 1623923899-16759-3-git-send-email-wcheng@codeaurora.org
State New
Headers show
Series None | expand

Commit Message

Wesley Cheng June 17, 2021, 9:58 a.m. UTC
Ensure that the USB gadget is able to support the configuration being
added based on the number of endpoints required from all interfaces.  This
is for accounting for any bandwidth or space limitations.

Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
---
 drivers/usb/gadget/configfs.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Wesley Cheng June 22, 2021, 5:27 a.m. UTC | #1
On 6/17/2021 4:07 AM, Greg KH wrote:
> On Thu, Jun 17, 2021 at 02:58:15AM -0700, Wesley Cheng wrote:
>> Ensure that the USB gadget is able to support the configuration being
>> added based on the number of endpoints required from all interfaces.  This
>> is for accounting for any bandwidth or space limitations.
>>
>> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
>> ---
>>  drivers/usb/gadget/configfs.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
>> index 15a607c..76b9983 100644
>> --- a/drivers/usb/gadget/configfs.c
>> +++ b/drivers/usb/gadget/configfs.c
>> @@ -1374,6 +1374,7 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
>>  		struct usb_function *f;
>>  		struct usb_function *tmp;
>>  		struct gadget_config_name *cn;
>> +		unsigned long ep_map = 0;
>>  
>>  		if (gadget_is_otg(gadget))
>>  			c->descriptors = otg_desc;
>> @@ -1403,7 +1404,28 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
>>  				list_add(&f->list, &cfg->func_list);
>>  				goto err_purge_funcs;
>>  			}
>> +			if (f->fs_descriptors) {
>> +				struct usb_descriptor_header **d;
>> +
>> +				d = f->fs_descriptors;
>> +				for (; *d; ++d) {

Hi Greg,

Thanks for the review and feedback.

> 
> With this check, there really is not a need to check for
> f->fs_descriptors above in the if statement, right?
> 

f->fs_descriptor will carry the table of descriptors that a particular
function driver has assigned to it.  The for loop here, will dereference
the individual descriptors within that descriptor array, so we need to
first ensure the descriptor array is present before traversing through
the individual entries/elements.

>> +					struct usb_endpoint_descriptor *ep;
>> +					int addr;
>> +
>> +					if ((*d)->bDescriptorType != USB_DT_ENDPOINT)
>> +						continue;
>> +
>> +					ep = (struct usb_endpoint_descriptor *)*d;
>> +					addr = ((ep->bEndpointAddress & 0x80) >> 3) |
>> +						(ep->bEndpointAddress & 0x0f);
> 
> Don't we have direction macros for this type of check?
> 

I don't believe we have a macro which would be able to convert the
bEndpointAddress field into the bit which needs to be set, assuming that
the 32bit ep_map has the lower 16bits carrying OUT EPs, and the upper
16bits carrying the IN EPs.

>> +					set_bit(addr, &ep_map);
>> +				}
> 
> What is this loop trying to do?  Please document it as I can not figure
> it out at all.
> 

Sure I can add some comments to this loop.  All its trying to do is to
set corresponding EPs being used (both IN and OUT eps) by a particular
USB configuration, then passing this bit mapped value to the DWC3 gadget
to evaluate.  As mentioned in the previous comment, the upper 16 bits
carries all IN eps used, and the lower 16 bits carries all the OUT eps used.

Thanks
Wesley Cheng

> thanks,
> 
> greg k-h
>
Greg KH June 22, 2021, 6:05 a.m. UTC | #2
On Mon, Jun 21, 2021 at 10:27:09PM -0700, Wesley Cheng wrote:
> 
> 
> On 6/17/2021 4:07 AM, Greg KH wrote:
> > On Thu, Jun 17, 2021 at 02:58:15AM -0700, Wesley Cheng wrote:
> >> Ensure that the USB gadget is able to support the configuration being
> >> added based on the number of endpoints required from all interfaces.  This
> >> is for accounting for any bandwidth or space limitations.
> >>
> >> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
> >> ---
> >>  drivers/usb/gadget/configfs.c | 22 ++++++++++++++++++++++
> >>  1 file changed, 22 insertions(+)
> >>
> >> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> >> index 15a607c..76b9983 100644
> >> --- a/drivers/usb/gadget/configfs.c
> >> +++ b/drivers/usb/gadget/configfs.c
> >> @@ -1374,6 +1374,7 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
> >>  		struct usb_function *f;
> >>  		struct usb_function *tmp;
> >>  		struct gadget_config_name *cn;
> >> +		unsigned long ep_map = 0;
> >>  
> >>  		if (gadget_is_otg(gadget))
> >>  			c->descriptors = otg_desc;
> >> @@ -1403,7 +1404,28 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
> >>  				list_add(&f->list, &cfg->func_list);
> >>  				goto err_purge_funcs;
> >>  			}
> >> +			if (f->fs_descriptors) {
> >> +				struct usb_descriptor_header **d;
> >> +
> >> +				d = f->fs_descriptors;
> >> +				for (; *d; ++d) {
> 
> Hi Greg,
> 
> Thanks for the review and feedback.
> 
> > 
> > With this check, there really is not a need to check for
> > f->fs_descriptors above in the if statement, right?
> > 
> 
> f->fs_descriptor will carry the table of descriptors that a particular
> function driver has assigned to it.  The for loop here, will dereference
> the individual descriptors within that descriptor array, so we need to
> first ensure the descriptor array is present before traversing through
> the individual entries/elements.

Ah, it's a dereference of an array element.  Subtle.  Tricky.  Messy :(

> >> +					struct usb_endpoint_descriptor *ep;
> >> +					int addr;
> >> +
> >> +					if ((*d)->bDescriptorType != USB_DT_ENDPOINT)
> >> +						continue;
> >> +
> >> +					ep = (struct usb_endpoint_descriptor *)*d;
> >> +					addr = ((ep->bEndpointAddress & 0x80) >> 3) |
> >> +						(ep->bEndpointAddress & 0x0f);
> > 
> > Don't we have direction macros for this type of check?
> > 
> 
> I don't believe we have a macro which would be able to convert the
> bEndpointAddress field into the bit which needs to be set, assuming that
> the 32bit ep_map has the lower 16bits carrying OUT EPs, and the upper
> 16bits carrying the IN EPs.

We have macros to tell if an endpoint is IN or OUT, please use those.

And this "cram the whole thing into 64 bits" is not obvious at all.
Just pass around the original pointer to the descriptors if someone
wants to use it or not, don't make up yet-another-data-structure here
for no good reason.  We aren't so memory constrained we need to pack
stuff into bits here.

thanks,

greg k-h
Wesley Cheng June 23, 2021, 9:38 a.m. UTC | #3
On 6/21/2021 11:05 PM, Greg KH wrote:
> On Mon, Jun 21, 2021 at 10:27:09PM -0700, Wesley Cheng wrote:
>>
>>
>> On 6/17/2021 4:07 AM, Greg KH wrote:
>>> On Thu, Jun 17, 2021 at 02:58:15AM -0700, Wesley Cheng wrote:
>>>> Ensure that the USB gadget is able to support the configuration being
>>>> added based on the number of endpoints required from all interfaces.  This
>>>> is for accounting for any bandwidth or space limitations.
>>>>
>>>> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
>>>> ---
>>>>  drivers/usb/gadget/configfs.c | 22 ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
>>>> index 15a607c..76b9983 100644
>>>> --- a/drivers/usb/gadget/configfs.c
>>>> +++ b/drivers/usb/gadget/configfs.c
>>>> @@ -1374,6 +1374,7 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
>>>>  		struct usb_function *f;
>>>>  		struct usb_function *tmp;
>>>>  		struct gadget_config_name *cn;
>>>> +		unsigned long ep_map = 0;
>>>>  
>>>>  		if (gadget_is_otg(gadget))
>>>>  			c->descriptors = otg_desc;
>>>> @@ -1403,7 +1404,28 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
>>>>  				list_add(&f->list, &cfg->func_list);
>>>>  				goto err_purge_funcs;
>>>>  			}
>>>> +			if (f->fs_descriptors) {
>>>> +				struct usb_descriptor_header **d;
>>>> +
>>>> +				d = f->fs_descriptors;
>>>> +				for (; *d; ++d) {
>>
>> Hi Greg,
>>
>> Thanks for the review and feedback.
>>
>>>
>>> With this check, there really is not a need to check for
>>> f->fs_descriptors above in the if statement, right?
>>>
>>
>> f->fs_descriptor will carry the table of descriptors that a particular
>> function driver has assigned to it.  The for loop here, will dereference
>> the individual descriptors within that descriptor array, so we need to
>> first ensure the descriptor array is present before traversing through
>> the individual entries/elements.
> 
> Ah, it's a dereference of an array element.  Subtle.  Tricky.  Messy :(
> 
>>>> +					struct usb_endpoint_descriptor *ep;
>>>> +					int addr;
>>>> +
>>>> +					if ((*d)->bDescriptorType != USB_DT_ENDPOINT)
>>>> +						continue;
>>>> +
>>>> +					ep = (struct usb_endpoint_descriptor *)*d;
>>>> +					addr = ((ep->bEndpointAddress & 0x80) >> 3) |
>>>> +						(ep->bEndpointAddress & 0x0f);
>>>
>>> Don't we have direction macros for this type of check?
>>>
>>
>> I don't believe we have a macro which would be able to convert the
>> bEndpointAddress field into the bit which needs to be set, assuming that
>> the 32bit ep_map has the lower 16bits carrying OUT EPs, and the upper
>> 16bits carrying the IN EPs.

Hi Greg,

> 
> We have macros to tell if an endpoint is IN or OUT, please use those.
> 
> And this "cram the whole thing into 64 bits" is not obvious at all.
> Just pass around the original pointer to the descriptors if someone
> wants to use it or not, don't make up yet-another-data-structure here
> for no good reason.  We aren't so memory constrained we need to pack
> stuff into bits here.
> 

Hmm ok, what I can do is to move this logic into the check_config()
callback itself, which is implemented by the UDC driver.  So now, the
DWC3 will have to do something similar to what is done here, ie loop the
EP descriptors for each function to determine the number of IN endpoints
being used.

Thanks
Wesley Cheng

> thanks,
> 
> greg k-h
>
Wesley Cheng June 23, 2021, 9:44 p.m. UTC | #4
On 6/23/2021 4:35 AM, Greg KH wrote:
> On Wed, Jun 23, 2021 at 02:38:55AM -0700, Wesley Cheng wrote:
>>
>>
>> On 6/21/2021 11:05 PM, Greg KH wrote:
>>> On Mon, Jun 21, 2021 at 10:27:09PM -0700, Wesley Cheng wrote:
>>>>
>>>>
>>>> On 6/17/2021 4:07 AM, Greg KH wrote:
>>>>> On Thu, Jun 17, 2021 at 02:58:15AM -0700, Wesley Cheng wrote:
>>>>>> Ensure that the USB gadget is able to support the configuration being
>>>>>> added based on the number of endpoints required from all interfaces.  This
>>>>>> is for accounting for any bandwidth or space limitations.
>>>>>>
>>>>>> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
>>>>>> ---
>>>>>>  drivers/usb/gadget/configfs.c | 22 ++++++++++++++++++++++
>>>>>>  1 file changed, 22 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
>>>>>> index 15a607c..76b9983 100644
>>>>>> --- a/drivers/usb/gadget/configfs.c
>>>>>> +++ b/drivers/usb/gadget/configfs.c
>>>>>> @@ -1374,6 +1374,7 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
>>>>>>  		struct usb_function *f;
>>>>>>  		struct usb_function *tmp;
>>>>>>  		struct gadget_config_name *cn;
>>>>>> +		unsigned long ep_map = 0;
>>>>>>  
>>>>>>  		if (gadget_is_otg(gadget))
>>>>>>  			c->descriptors = otg_desc;
>>>>>> @@ -1403,7 +1404,28 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
>>>>>>  				list_add(&f->list, &cfg->func_list);
>>>>>>  				goto err_purge_funcs;
>>>>>>  			}
>>>>>> +			if (f->fs_descriptors) {
>>>>>> +				struct usb_descriptor_header **d;
>>>>>> +
>>>>>> +				d = f->fs_descriptors;
>>>>>> +				for (; *d; ++d) {
>>>>
>>>> Hi Greg,
>>>>
>>>> Thanks for the review and feedback.
>>>>
>>>>>
>>>>> With this check, there really is not a need to check for
>>>>> f->fs_descriptors above in the if statement, right?
>>>>>
>>>>
>>>> f->fs_descriptor will carry the table of descriptors that a particular
>>>> function driver has assigned to it.  The for loop here, will dereference
>>>> the individual descriptors within that descriptor array, so we need to
>>>> first ensure the descriptor array is present before traversing through
>>>> the individual entries/elements.
>>>
>>> Ah, it's a dereference of an array element.  Subtle.  Tricky.  Messy :(
>>>
>>>>>> +					struct usb_endpoint_descriptor *ep;
>>>>>> +					int addr;
>>>>>> +
>>>>>> +					if ((*d)->bDescriptorType != USB_DT_ENDPOINT)
>>>>>> +						continue;
>>>>>> +
>>>>>> +					ep = (struct usb_endpoint_descriptor *)*d;
>>>>>> +					addr = ((ep->bEndpointAddress & 0x80) >> 3) |
>>>>>> +						(ep->bEndpointAddress & 0x0f);
>>>>>
>>>>> Don't we have direction macros for this type of check?
>>>>>
>>>>
>>>> I don't believe we have a macro which would be able to convert the
>>>> bEndpointAddress field into the bit which needs to be set, assuming that
>>>> the 32bit ep_map has the lower 16bits carrying OUT EPs, and the upper
>>>> 16bits carrying the IN EPs.
>>
>> Hi Greg,
>>
>>>
>>> We have macros to tell if an endpoint is IN or OUT, please use those.
>>>
>>> And this "cram the whole thing into 64 bits" is not obvious at all.
>>> Just pass around the original pointer to the descriptors if someone
>>> wants to use it or not, don't make up yet-another-data-structure here
>>> for no good reason.  We aren't so memory constrained we need to pack
>>> stuff into bits here.
>>>
>>
>> Hmm ok, what I can do is to move this logic into the check_config()
>> callback itself, which is implemented by the UDC driver.  So now, the
>> DWC3 will have to do something similar to what is done here, ie loop the
>> EP descriptors for each function to determine the number of IN endpoints
>> being used.

Hi Greg,

> 
> We have common USB core functions for this, why can't you just use them?
> 

So, I've tried to use pre-existing mechanisms there in the USB core, but
they are not populated correctly at the time of function binding.  I
will highlight some of the things I've tried, and why they do not work.
 If possible, if you could point which core functions can achieve what
we are trying to do here, that would help as well.

  - f->endpoints - This is a bitmap which carries the endpoints used by
a particular function driver.  This does not work, as this is set during
receiving the SET_CONFIG packet.  (we need this during the function
driver binding stage)

  - gadget->in_epnum/gadget->out_epnum - This carries the count of
endpoints used per configuration.  This would be perfect, but this count
is only incremented when we are not matching EPs using the EP name.  So
in designs where the EP name is used to match, it can not be used.

 - gadget->ep_list - I can use this now in the check_config() to iterate
through the list of eps to see which ones have been claimed for a
particular configuration.

So just to re-iterate, the TXFIFO resize logic kicks in when the host
sends the SET_CONFIG packet, which is the "end" of USB enumeration.  We
had discussed a concern previously where, what if we run the resize
logic, and there is not enough internal memory.  We'd end up with an
enumerated device w/ certain functions broken.

This is where the check_config() comes into the picture.  It uses the
number of endpoints collected during the bind() stage, and checks to
make sure the resize logic can at least allocate 1 TXFIFO per endpoint.
 If it can not, then it will fail the bind sequence.

Thanks
Wesley Cheng
> Please do not take data that we already have in one format, and convert
> it to another one just for a single driver to consume.  That's
> pointless.
> 
> thanks,
> 
> greg k-h
>
Greg KH June 24, 2021, 12:09 p.m. UTC | #5
On Wed, Jun 23, 2021 at 02:44:31PM -0700, Wesley Cheng wrote:
> 
> 
> On 6/23/2021 4:35 AM, Greg KH wrote:
> > On Wed, Jun 23, 2021 at 02:38:55AM -0700, Wesley Cheng wrote:
> >>
> >>
> >> On 6/21/2021 11:05 PM, Greg KH wrote:
> >>> On Mon, Jun 21, 2021 at 10:27:09PM -0700, Wesley Cheng wrote:
> >>>>
> >>>>
> >>>> On 6/17/2021 4:07 AM, Greg KH wrote:
> >>>>> On Thu, Jun 17, 2021 at 02:58:15AM -0700, Wesley Cheng wrote:
> >>>>>> Ensure that the USB gadget is able to support the configuration being
> >>>>>> added based on the number of endpoints required from all interfaces.  This
> >>>>>> is for accounting for any bandwidth or space limitations.
> >>>>>>
> >>>>>> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
> >>>>>> ---
> >>>>>>  drivers/usb/gadget/configfs.c | 22 ++++++++++++++++++++++
> >>>>>>  1 file changed, 22 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> >>>>>> index 15a607c..76b9983 100644
> >>>>>> --- a/drivers/usb/gadget/configfs.c
> >>>>>> +++ b/drivers/usb/gadget/configfs.c
> >>>>>> @@ -1374,6 +1374,7 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
> >>>>>>  		struct usb_function *f;
> >>>>>>  		struct usb_function *tmp;
> >>>>>>  		struct gadget_config_name *cn;
> >>>>>> +		unsigned long ep_map = 0;
> >>>>>>  
> >>>>>>  		if (gadget_is_otg(gadget))
> >>>>>>  			c->descriptors = otg_desc;
> >>>>>> @@ -1403,7 +1404,28 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
> >>>>>>  				list_add(&f->list, &cfg->func_list);
> >>>>>>  				goto err_purge_funcs;
> >>>>>>  			}
> >>>>>> +			if (f->fs_descriptors) {
> >>>>>> +				struct usb_descriptor_header **d;
> >>>>>> +
> >>>>>> +				d = f->fs_descriptors;
> >>>>>> +				for (; *d; ++d) {
> >>>>
> >>>> Hi Greg,
> >>>>
> >>>> Thanks for the review and feedback.
> >>>>
> >>>>>
> >>>>> With this check, there really is not a need to check for
> >>>>> f->fs_descriptors above in the if statement, right?
> >>>>>
> >>>>
> >>>> f->fs_descriptor will carry the table of descriptors that a particular
> >>>> function driver has assigned to it.  The for loop here, will dereference
> >>>> the individual descriptors within that descriptor array, so we need to
> >>>> first ensure the descriptor array is present before traversing through
> >>>> the individual entries/elements.
> >>>
> >>> Ah, it's a dereference of an array element.  Subtle.  Tricky.  Messy :(
> >>>
> >>>>>> +					struct usb_endpoint_descriptor *ep;
> >>>>>> +					int addr;
> >>>>>> +
> >>>>>> +					if ((*d)->bDescriptorType != USB_DT_ENDPOINT)
> >>>>>> +						continue;
> >>>>>> +
> >>>>>> +					ep = (struct usb_endpoint_descriptor *)*d;
> >>>>>> +					addr = ((ep->bEndpointAddress & 0x80) >> 3) |
> >>>>>> +						(ep->bEndpointAddress & 0x0f);
> >>>>>
> >>>>> Don't we have direction macros for this type of check?
> >>>>>
> >>>>
> >>>> I don't believe we have a macro which would be able to convert the
> >>>> bEndpointAddress field into the bit which needs to be set, assuming that
> >>>> the 32bit ep_map has the lower 16bits carrying OUT EPs, and the upper
> >>>> 16bits carrying the IN EPs.
> >>
> >> Hi Greg,
> >>
> >>>
> >>> We have macros to tell if an endpoint is IN or OUT, please use those.
> >>>
> >>> And this "cram the whole thing into 64 bits" is not obvious at all.
> >>> Just pass around the original pointer to the descriptors if someone
> >>> wants to use it or not, don't make up yet-another-data-structure here
> >>> for no good reason.  We aren't so memory constrained we need to pack
> >>> stuff into bits here.
> >>>
> >>
> >> Hmm ok, what I can do is to move this logic into the check_config()
> >> callback itself, which is implemented by the UDC driver.  So now, the
> >> DWC3 will have to do something similar to what is done here, ie loop the
> >> EP descriptors for each function to determine the number of IN endpoints
> >> being used.
> 
> Hi Greg,
> 
> > 
> > We have common USB core functions for this, why can't you just use them?
> > 
> 
> So, I've tried to use pre-existing mechanisms there in the USB core, but
> they are not populated correctly at the time of function binding.  I
> will highlight some of the things I've tried, and why they do not work.
>  If possible, if you could point which core functions can achieve what
> we are trying to do here, that would help as well.

We have functions to detect the IN/OUT of an endpoint, use them.

We also have functions that determine how many endpoints of each type
that a device has, why can you not use them as well?  Are they only
valid for driver structures, not gadget ones?

>   - f->endpoints - This is a bitmap which carries the endpoints used by
> a particular function driver.  This does not work, as this is set during
> receiving the SET_CONFIG packet.  (we need this during the function
> driver binding stage)
> 
>   - gadget->in_epnum/gadget->out_epnum - This carries the count of
> endpoints used per configuration.  This would be perfect, but this count
> is only incremented when we are not matching EPs using the EP name.  So
> in designs where the EP name is used to match, it can not be used.
> 
>  - gadget->ep_list - I can use this now in the check_config() to iterate
> through the list of eps to see which ones have been claimed for a
> particular configuration.
> 
> So just to re-iterate, the TXFIFO resize logic kicks in when the host
> sends the SET_CONFIG packet, which is the "end" of USB enumeration.  We
> had discussed a concern previously where, what if we run the resize
> logic, and there is not enough internal memory.  We'd end up with an
> enumerated device w/ certain functions broken.

Where are you running out of memory?  In the gadget kernel?  Or
somewhere else?

> This is where the check_config() comes into the picture.  It uses the
> number of endpoints collected during the bind() stage, and checks to
> make sure the resize logic can at least allocate 1 TXFIFO per endpoint.
>  If it can not, then it will fail the bind sequence.

That's fine, I am just worried about your crazy "pack all the bits into
a u64 for no good reason" logic here.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 15a607c..76b9983 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1374,6 +1374,7 @@  static int configfs_composite_bind(struct usb_gadget *gadget,
 		struct usb_function *f;
 		struct usb_function *tmp;
 		struct gadget_config_name *cn;
+		unsigned long ep_map = 0;
 
 		if (gadget_is_otg(gadget))
 			c->descriptors = otg_desc;
@@ -1403,7 +1404,28 @@  static int configfs_composite_bind(struct usb_gadget *gadget,
 				list_add(&f->list, &cfg->func_list);
 				goto err_purge_funcs;
 			}
+			if (f->fs_descriptors) {
+				struct usb_descriptor_header **d;
+
+				d = f->fs_descriptors;
+				for (; *d; ++d) {
+					struct usb_endpoint_descriptor *ep;
+					int addr;
+
+					if ((*d)->bDescriptorType != USB_DT_ENDPOINT)
+						continue;
+
+					ep = (struct usb_endpoint_descriptor *)*d;
+					addr = ((ep->bEndpointAddress & 0x80) >> 3) |
+						(ep->bEndpointAddress & 0x0f);
+					set_bit(addr, &ep_map);
+				}
+			}
 		}
+		ret = usb_gadget_check_config(cdev->gadget, ep_map);
+		if (ret)
+			goto err_purge_funcs;
+
 		usb_ep_autoconfig_reset(cdev->gadget);
 	}
 	if (cdev->use_os_string) {