diff mbox series

[RFC,v5,4/6] usb: gadget: configfs: Check USB configuration before adding

Message ID 20200829055846.19034-5-wcheng@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series Re-introduce TX FIFO resize for larger EP bursting | expand

Commit Message

Wesley Cheng Aug. 29, 2020, 5: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

Peter Chen Aug. 31, 2020, 2:29 a.m. UTC | #1
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 <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 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?

> +					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?

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

>  		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
>
Wesley Cheng Aug. 31, 2020, 7:34 a.m. UTC | #2
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 <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 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
>>
>
diff mbox series

Patch

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);
+					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) {