diff mbox series

usb: gadget: composite: Remove dedicated OS Feature Descriptors request

Message ID 20200703083534.5292-1-christopher.a.dickens@gmail.com (mailing list archive)
State New, archived
Headers show
Series usb: gadget: composite: Remove dedicated OS Feature Descriptors request | expand

Commit Message

Chris Dickens July 3, 2020, 8:35 a.m. UTC
Currently Microsoft OS Feature Descriptors are handled using a
separately allocated USB request, however everything about this USB
request is identical to the USB request used for all other control
responses. Simplify the code by removing this separate USB request and
using the same USB request as all other control responses.

While at it, simplify the composite_ep0_queue() function by removing the
req and gfp_flags arguments. The former is no longer necessary with a
single USB request and the latter is always GFP_ATOMIC.

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
---
 drivers/usb/gadget/composite.c | 69 +++++-----------------------------
 drivers/usb/gadget/configfs.c  | 11 ------
 include/linux/usb/composite.h  |  9 -----
 3 files changed, 9 insertions(+), 80 deletions(-)

Comments

Greg KH July 3, 2020, 8:57 a.m. UTC | #1
On Fri, Jul 03, 2020 at 01:35:34AM -0700, Chris Dickens wrote:
> Currently Microsoft OS Feature Descriptors are handled using a
> separately allocated USB request, however everything about this USB
> request is identical to the USB request used for all other control
> responses. Simplify the code by removing this separate USB request and
> using the same USB request as all other control responses.
> 
> While at it, simplify the composite_ep0_queue() function by removing the
> req and gfp_flags arguments. The former is no longer necessary with a
> single USB request and the latter is always GFP_ATOMIC.
> 
> Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
> ---
>  drivers/usb/gadget/composite.c | 69 +++++-----------------------------
>  drivers/usb/gadget/configfs.c  | 11 ------
>  include/linux/usb/composite.h  |  9 -----
>  3 files changed, 9 insertions(+), 80 deletions(-)

Did you confirm by testing that this actually works with a real device
that wants to talk to Windows with those feature descriptors?

thanks,

greg k-h
Felipe Balbi July 25, 2020, 6:12 a.m. UTC | #2
Hi,

Chris Dickens <christopher.a.dickens@gmail.com> writes:

> Currently Microsoft OS Feature Descriptors are handled using a
> separately allocated USB request, however everything about this USB
> request is identical to the USB request used for all other control
> responses. Simplify the code by removing this separate USB request and
> using the same USB request as all other control responses.
>
> While at it, simplify the composite_ep0_queue() function by removing the
> req and gfp_flags arguments. The former is no longer necessary with a
> single USB request and the latter is always GFP_ATOMIC.

I would rather move the removal of the extra arguments to a separate
patch.
diff mbox series

Patch

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 5c1eb96a5c57..020801e70caf 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1443,26 +1443,17 @@  static void composite_setup_complete(struct usb_ep *ep, struct usb_request *req)
 
 	if (cdev->req == req)
 		cdev->setup_pending = false;
-	else if (cdev->os_desc_req == req)
-		cdev->os_desc_pending = false;
 	else
 		WARN(1, "unknown request %p\n", req);
 }
 
-static int composite_ep0_queue(struct usb_composite_dev *cdev,
-		struct usb_request *req, gfp_t gfp_flags)
+static int composite_ep0_queue(struct usb_composite_dev *cdev)
 {
 	int ret;
 
-	ret = usb_ep_queue(cdev->gadget->ep0, req, gfp_flags);
-	if (ret == 0) {
-		if (cdev->req == req)
-			cdev->setup_pending = true;
-		else if (cdev->os_desc_req == req)
-			cdev->os_desc_pending = true;
-		else
-			WARN(1, "unknown request %p\n", req);
-	}
+	ret = usb_ep_queue(cdev->gadget->ep0, cdev->req, GFP_ATOMIC);
+	if (ret == 0)
+		cdev->setup_pending = true;
 
 	return ret;
 }
@@ -1519,7 +1510,7 @@  static int fill_ext_compat(struct usb_configuration *c, u8 *buf)
 				buf += 23;
 			}
 			count += 24;
-			if (count + 24 >= USB_COMP_EP0_OS_DESC_BUFSIZ)
+			if (count + 24 >= USB_COMP_EP0_BUFSIZ)
 				return count;
 		}
 	}
@@ -1581,7 +1572,7 @@  static int fill_ext_prop(struct usb_configuration *c, int interface, u8 *buf)
 			list_for_each_entry(ext_prop, &d->ext_prop, entry) {
 				n = ext_prop->data_len +
 					ext_prop->name_len + 14;
-				if (count + n >= USB_COMP_EP0_OS_DESC_BUFSIZ)
+				if (count + n >= USB_COMP_EP0_BUFSIZ)
 					return count;
 				usb_ext_prop_put_size(buf, n);
 				usb_ext_prop_put_type(buf, ext_prop->type);
@@ -1893,12 +1884,9 @@  composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 			int				interface;
 			int				count = 0;
 
-			req = cdev->os_desc_req;
-			req->context = cdev;
-			req->complete = composite_setup_complete;
 			buf = req->buf;
 			os_desc_cfg = cdev->os_desc_config;
-			w_length = min_t(u16, w_length, USB_COMP_EP0_OS_DESC_BUFSIZ);
+			w_length = min_t(u16, w_length, USB_COMP_EP0_BUFSIZ);
 			memset(buf, 0, w_length);
 			buf[5] = 0x01;
 			switch (ctrl->bRequestType & USB_RECIP_MASK) {
@@ -2019,7 +2007,7 @@  composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 		req->length = value;
 		req->context = cdev;
 		req->zero = value < w_length;
-		value = composite_ep0_queue(cdev, req, GFP_ATOMIC);
+		value = composite_ep0_queue(cdev);
 		if (value < 0) {
 			DBG(cdev, "ep_queue --> %d\n", value);
 			req->status = 0;
@@ -2187,30 +2175,6 @@  int composite_dev_prepare(struct usb_composite_driver *composite,
 	return ret;
 }
 
-int composite_os_desc_req_prepare(struct usb_composite_dev *cdev,
-				  struct usb_ep *ep0)
-{
-	int ret = 0;
-
-	cdev->os_desc_req = usb_ep_alloc_request(ep0, GFP_KERNEL);
-	if (!cdev->os_desc_req) {
-		ret = -ENOMEM;
-		goto end;
-	}
-
-	cdev->os_desc_req->buf = kmalloc(USB_COMP_EP0_OS_DESC_BUFSIZ,
-					 GFP_KERNEL);
-	if (!cdev->os_desc_req->buf) {
-		ret = -ENOMEM;
-		usb_ep_free_request(ep0, cdev->os_desc_req);
-		goto end;
-	}
-	cdev->os_desc_req->context = cdev;
-	cdev->os_desc_req->complete = composite_setup_complete;
-end:
-	return ret;
-}
-
 void composite_dev_cleanup(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget_string_container *uc, *tmp;
@@ -2220,15 +2184,6 @@  void composite_dev_cleanup(struct usb_composite_dev *cdev)
 		list_del(&uc->list);
 		kfree(uc);
 	}
-	if (cdev->os_desc_req) {
-		if (cdev->os_desc_pending)
-			usb_ep_dequeue(cdev->gadget->ep0, cdev->os_desc_req);
-
-		kfree(cdev->os_desc_req->buf);
-		cdev->os_desc_req->buf = NULL;
-		usb_ep_free_request(cdev->gadget->ep0, cdev->os_desc_req);
-		cdev->os_desc_req = NULL;
-	}
 	if (cdev->req) {
 		if (cdev->setup_pending)
 			usb_ep_dequeue(cdev->gadget->ep0, cdev->req);
@@ -2286,12 +2241,6 @@  static int composite_bind(struct usb_gadget *gadget,
 	if (status < 0)
 		goto fail;
 
-	if (cdev->use_os_string) {
-		status = composite_os_desc_req_prepare(cdev, gadget->ep0);
-		if (status)
-			goto fail;
-	}
-
 	update_unchanged_dev_desc(&cdev->desc, composite->dev);
 
 	/* has userspace failed to provide a serial number? */
@@ -2460,7 +2409,7 @@  void usb_composite_setup_continue(struct usb_composite_dev *cdev)
 		DBG(cdev, "%s: Completing delayed status\n", __func__);
 		req->length = 0;
 		req->context = cdev;
-		value = composite_ep0_queue(cdev, req, GFP_ATOMIC);
+		value = composite_ep0_queue(cdev);
 		if (value < 0) {
 			DBG(cdev, "ep_queue --> %d\n", value);
 			req->status = 0;
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 9dc06a4e1b30..24a86de6293c 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1232,12 +1232,6 @@  static int configfs_do_nothing(struct usb_composite_dev *cdev)
 	return -EINVAL;
 }
 
-int composite_dev_prepare(struct usb_composite_driver *composite,
-		struct usb_composite_dev *dev);
-
-int composite_os_desc_req_prepare(struct usb_composite_dev *cdev,
-				  struct usb_ep *ep0);
-
 static void purge_configs_funcs(struct gadget_info *gi)
 {
 	struct usb_configuration	*c;
@@ -1393,11 +1387,6 @@  static int configfs_composite_bind(struct usb_gadget *gadget,
 		}
 		usb_ep_autoconfig_reset(cdev->gadget);
 	}
-	if (cdev->use_os_string) {
-		ret = composite_os_desc_req_prepare(cdev, gadget->ep0);
-		if (ret)
-			goto err_purge_funcs;
-	}
 
 	usb_ep_autoconfig_reset(cdev->gadget);
 	return 0;
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 2040696d75b6..505d3509a8bb 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -54,9 +54,6 @@ 
 /* big enough to hold our biggest descriptor */
 #define USB_COMP_EP0_BUFSIZ	4096
 
-/* OS feature descriptor length <= 4kB */
-#define USB_COMP_EP0_OS_DESC_BUFSIZ	4096
-
 #define USB_MS_TO_HS_INTERVAL(x)	(ilog2((x * 1000 / 125)) + 1)
 struct usb_configuration;
 
@@ -423,8 +420,6 @@  extern void usb_composite_unregister(struct usb_composite_driver *driver);
 extern void usb_composite_setup_continue(struct usb_composite_dev *cdev);
 extern int composite_dev_prepare(struct usb_composite_driver *composite,
 		struct usb_composite_dev *cdev);
-extern int composite_os_desc_req_prepare(struct usb_composite_dev *cdev,
-					 struct usb_ep *ep0);
 void composite_dev_cleanup(struct usb_composite_dev *cdev);
 
 static inline struct usb_composite_driver *to_cdriver(
@@ -440,14 +435,12 @@  static inline struct usb_composite_driver *to_cdriver(
  * struct usb_composite_device - represents one composite usb gadget
  * @gadget: read-only, abstracts the gadget's usb peripheral controller
  * @req: used for control responses; buffer is pre-allocated
- * @os_desc_req: used for OS descriptors responses; buffer is pre-allocated
  * @config: the currently active configuration
  * @qw_sign: qwSignature part of the OS string
  * @b_vendor_code: bMS_VendorCode part of the OS string
  * @use_os_string: false by default, interested gadgets set it
  * @os_desc_config: the configuration to be used with OS descriptors
  * @setup_pending: true when setup request is queued but not completed
- * @os_desc_pending: true when os_desc request is queued but not completed
  *
  * One of these devices is allocated and initialized before the
  * associated device driver's bind() is called.
@@ -478,7 +471,6 @@  static inline struct usb_composite_driver *to_cdriver(
 struct usb_composite_dev {
 	struct usb_gadget		*gadget;
 	struct usb_request		*req;
-	struct usb_request		*os_desc_req;
 
 	struct usb_configuration	*config;
 
@@ -513,7 +505,6 @@  struct usb_composite_dev {
 
 	/* public: */
 	unsigned int			setup_pending:1;
-	unsigned int			os_desc_pending:1;
 };
 
 extern int usb_string_id(struct usb_composite_dev *c);