On 8/30/2020 7:29 PM, Peter Chen wrote:
> On 20-08-28 22:58:44, 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 <wch...@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 56051bb97349..7c74c04b1d8c 100644
>> --- a/drivers/usb/gadget/configfs.c
>> +++ b/drivers/usb/gadget/configfs.c
>> @@ -1361,6 +1361,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;
>> @@ -1390,7 +1391,28 @@ static int configfs_composite_bind(struct usb_gadget 
>> *gadget,
>>                              list_add(&f->list, &cfg->func_list);
>>                              goto err_purge_funcs;
>>                      }
>> +                    if (f->ss_descriptors) {
>> +                            struct usb_descriptor_header **d;
>> +
>> +                            d = f->ss_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);
> 
> ">> 3" or "<< 3?
> 

Hi Peter,

Thanks for your comments.  It should be ">> 3" as we want to utilize the
corresponding USB_DIR_IN bit in the bitmap to set the correct bit.
(USB_DIR_IN = 0x80)

>> +                                    set_bit(addr, &ep_map);
> 
> You want to record all endpoints on ep_map? Considering there are
> four EP_IN (1-4), and four EP_OUT (1-4), what the value of ep_map
> would like?
> 

So for example, if a configuration uses EP8IN and EP9OUT, then the
ep_map will look like:

EP8-IN:
addr = ((0x88 & 0x80) >> 3) | (0x88 & 0xf) --> 0x18

EP9-OUT:
addr = ((0x9 & 0x80) >> 3) | (0x9 & 0xf) --> 0x9

ep_map = ep_map = 0x01000200

The lower 16 bits will carry the OUT endpoints, whereas the upper 16
bits are the IN endpoints. (ie bit16 = ep0in, bit0 = ep0out)

>> +                            }
>> +                    }
>>              }
>> +            ret = usb_gadget_check_config(cdev->gadget, ep_map);
>> +            if (ret)
>> +                    goto err_purge_funcs;
>> +
> 
> You may move this patch after your 4nd patch to avoid "git bisect"
> issue.
> 

Sure, thanks for the suggestion, will do that in the next rev.

Thanks
Wesley

>>              usb_ep_autoconfig_reset(cdev->gadget);
>>      }
>>      if (cdev->use_os_string) {
>> -- 
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Reply via email to