linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] usb: gadget: f_hid: optional SETUP/SET_REPORT mode
@ 2021-08-14  3:12 Maxim Devaev
  2021-08-19 21:13 ` Maciej Żenczykowski
  0 siblings, 1 reply; 3+ messages in thread
From: Maxim Devaev @ 2021-08-14  3:12 UTC (permalink / raw)
  To: balbi
  Cc: stern, gregkh, mdevaev, ruslan.bilovol, mika.westerberg, maze,
	jj251510319013, linux-usb, linux-kernel

f_hid provides the OUT Endpoint as only way for receiving reports
from the host. SETUP/SET_REPORT method is not supported, and this causes
a number of compatibility problems with various host drivers, especially
in the case of keyboard emulation using f_hid.

- Some hosts do not support the OUT Endpoint and ignore it,
  so it becomes impossible for the gadget to receive a report
  from the host. In the case of a keyboard, the gadget loses the ability
  to get the status of the LEDs.

- Some BIOSes/UEFIs can't work with HID with the OUT Endpoint at all.
  This may be due to their bugs or incomplete implementation
  of the HID standard.
  For example, absolutely all Apple UEFIs can't handle the OUT Endpoint
  if it goes after IN Endpoint in the descriptor and require the reverse
  order (OUT, IN) which is a violation of the standard.
  Other hosts either do not initialize gadgets with a descriptor
  containing the OUT Endpoint completely (like some HP and DELL BIOSes
  and embedded firmwares like on KVM switches), or initialize them,
  but will not poll.

This patch adds option no_out_endpoint=1 to disable the OUT Endpoint
and allow f_hid to receive reports from the host via SETUP/SET_REPORT.

Previously, there was such a feature in f_hid, but it was replaced
by the OUT Endpoint [1] in the commit 99c515005857 ("usb: gadget: hidg:
register OUT INT endpoint for SET_REPORT"). So this patch actually returns
the removed functionality making it optional. For backward compatibility
reasons, the OUT Endpoint mode remains the default behaviour.

- The OUT Endpoint mode provides the report queue and reduces USB overhead
  (eliminating SETUP routine) on transmitting a report from the host.

- If the SETUP/SET_REPORT mode is used, there is no report queue,
  so the userspace will only read the last report. For classic HID
  devices like keyboard this is not a problem, since it is intended
  to transmit the status of the LEDs and only the last report
  is important. This mode provides better compatibility with strange
  and buggy host drivers.

Both modes passed USBCV tests. Checking with the USB protocol analyzer
also confirmed that everything is working as it should and the new mode
ensures operability in all of the described cases.

Signed-off-by: Maxim Devaev <mdevaev@gmail.com>
Link: https://www.spinics.net/lists/linux-usb/msg65494.html [1]
---
 drivers/usb/gadget/function/f_hid.c | 217 +++++++++++++++++++++++-----
 drivers/usb/gadget/function/u_hid.h |   1 +
 2 files changed, 185 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index bb476e121eae..e3fb73ed696d 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -45,12 +45,17 @@ struct f_hidg {
 	unsigned short			report_desc_length;
 	char				*report_desc;
 	unsigned short			report_length;
+	bool				use_out_ep;
 
 	/* recv report */
-	struct list_head		completed_out_req;
 	spinlock_t			read_spinlock;
 	wait_queue_head_t		read_queue;
+	/* recv report - interrupt out only (use_out_ep == 1) */
+	struct list_head		completed_out_req;
 	unsigned int			qlen;
+	/* recv report - setup set_report only (use_out_ep == 0) */
+	char				*set_report_buf;
+	unsigned int			set_report_length;
 
 	/* send report */
 	spinlock_t			write_spinlock;
@@ -79,7 +84,7 @@ static struct usb_interface_descriptor hidg_interface_desc = {
 	.bDescriptorType	= USB_DT_INTERFACE,
 	/* .bInterfaceNumber	= DYNAMIC */
 	.bAlternateSetting	= 0,
-	.bNumEndpoints		= 2,
+	/* .bNumEndpoints	= DYNAMIC */
 	.bInterfaceClass	= USB_CLASS_HID,
 	/* .bInterfaceSubClass	= DYNAMIC */
 	/* .bInterfaceProtocol	= DYNAMIC */
@@ -140,7 +145,7 @@ static struct usb_ss_ep_comp_descriptor hidg_ss_out_comp_desc = {
 	/* .wBytesPerInterval   = DYNAMIC */
 };
 
-static struct usb_descriptor_header *hidg_ss_descriptors[] = {
+static struct usb_descriptor_header *hidg_ss_descriptors_intout[] = {
 	(struct usb_descriptor_header *)&hidg_interface_desc,
 	(struct usb_descriptor_header *)&hidg_desc,
 	(struct usb_descriptor_header *)&hidg_ss_in_ep_desc,
@@ -150,6 +155,14 @@ static struct usb_descriptor_header *hidg_ss_descriptors[] = {
 	NULL,
 };
 
+static struct usb_descriptor_header *hidg_ss_descriptors_ssreport[] = {
+	(struct usb_descriptor_header *)&hidg_interface_desc,
+	(struct usb_descriptor_header *)&hidg_desc,
+	(struct usb_descriptor_header *)&hidg_ss_in_ep_desc,
+	(struct usb_descriptor_header *)&hidg_ss_in_comp_desc,
+	NULL,
+};
+
 /* High-Speed Support */
 
 static struct usb_endpoint_descriptor hidg_hs_in_ep_desc = {
@@ -176,7 +189,7 @@ static struct usb_endpoint_descriptor hidg_hs_out_ep_desc = {
 				      */
 };
 
-static struct usb_descriptor_header *hidg_hs_descriptors[] = {
+static struct usb_descriptor_header *hidg_hs_descriptors_intout[] = {
 	(struct usb_descriptor_header *)&hidg_interface_desc,
 	(struct usb_descriptor_header *)&hidg_desc,
 	(struct usb_descriptor_header *)&hidg_hs_in_ep_desc,
@@ -184,6 +197,13 @@ static struct usb_descriptor_header *hidg_hs_descriptors[] = {
 	NULL,
 };
 
+static struct usb_descriptor_header *hidg_hs_descriptors_ssreport[] = {
+	(struct usb_descriptor_header *)&hidg_interface_desc,
+	(struct usb_descriptor_header *)&hidg_desc,
+	(struct usb_descriptor_header *)&hidg_hs_in_ep_desc,
+	NULL,
+};
+
 /* Full-Speed Support */
 
 static struct usb_endpoint_descriptor hidg_fs_in_ep_desc = {
@@ -210,7 +230,7 @@ static struct usb_endpoint_descriptor hidg_fs_out_ep_desc = {
 				       */
 };
 
-static struct usb_descriptor_header *hidg_fs_descriptors[] = {
+static struct usb_descriptor_header *hidg_fs_descriptors_intout[] = {
 	(struct usb_descriptor_header *)&hidg_interface_desc,
 	(struct usb_descriptor_header *)&hidg_desc,
 	(struct usb_descriptor_header *)&hidg_fs_in_ep_desc,
@@ -218,6 +238,13 @@ static struct usb_descriptor_header *hidg_fs_descriptors[] = {
 	NULL,
 };
 
+static struct usb_descriptor_header *hidg_fs_descriptors_ssreport[] = {
+	(struct usb_descriptor_header *)&hidg_interface_desc,
+	(struct usb_descriptor_header *)&hidg_desc,
+	(struct usb_descriptor_header *)&hidg_fs_in_ep_desc,
+	NULL,
+};
+
 /*-------------------------------------------------------------------------*/
 /*                                 Strings                                 */
 
@@ -241,9 +268,11 @@ static struct usb_gadget_strings *ct_func_strings[] = {
 /*-------------------------------------------------------------------------*/
 /*                              Char Device                                */
 
-static ssize_t f_hidg_read(struct file *file, char __user *buffer,
-			size_t count, loff_t *ptr)
+static ssize_t f_hidg_intout_read(struct file *file, char __user *buffer,
+				  size_t count, loff_t *ptr)
 {
+	/* used only if the OUT endpoint is configured */
+
 	struct f_hidg *hidg = file->private_data;
 	struct f_hidg_req_list *list;
 	struct usb_request *req;
@@ -255,15 +284,15 @@ static ssize_t f_hidg_read(struct file *file, char __user *buffer,
 
 	spin_lock_irqsave(&hidg->read_spinlock, flags);
 
-#define READ_COND (!list_empty(&hidg->completed_out_req))
+#define READ_COND_INTOUT (!list_empty(&hidg->completed_out_req))
 
 	/* wait for at least one buffer to complete */
-	while (!READ_COND) {
+	while (!READ_COND_INTOUT) {
 		spin_unlock_irqrestore(&hidg->read_spinlock, flags);
 		if (file->f_flags & O_NONBLOCK)
 			return -EAGAIN;
 
-		if (wait_event_interruptible(hidg->read_queue, READ_COND))
+		if (wait_event_interruptible(hidg->read_queue, READ_COND_INTOUT))
 			return -ERESTARTSYS;
 
 		spin_lock_irqsave(&hidg->read_spinlock, flags);
@@ -313,6 +342,62 @@ static ssize_t f_hidg_read(struct file *file, char __user *buffer,
 	return count;
 }
 
+#define READ_COND_SSREPORT (hidg->set_report_buf != NULL)
+
+static ssize_t f_hidg_ssreport_read(struct file *file, char __user *buffer,
+				    size_t count, loff_t *ptr)
+{
+	/* used only if the OUT endpoint is NOT configured */
+
+	struct f_hidg *hidg = file->private_data;
+	char *tmp_buf = NULL;
+	unsigned long flags;
+
+	if (!count)
+		return 0;
+
+	spin_lock_irqsave(&hidg->read_spinlock, flags);
+
+	while (!READ_COND_SSREPORT) {
+		spin_unlock_irqrestore(&hidg->read_spinlock, flags);
+		if (file->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		if (wait_event_interruptible(hidg->read_queue, READ_COND_SSREPORT))
+			return -ERESTARTSYS;
+
+		spin_lock_irqsave(&hidg->read_spinlock, flags);
+	}
+
+	count = min_t(unsigned int, count, hidg->set_report_length);
+	tmp_buf = hidg->set_report_buf;
+	hidg->set_report_buf = NULL;
+
+	spin_unlock_irqrestore(&hidg->read_spinlock, flags);
+
+	if (tmp_buf != NULL) {
+		count -= copy_to_user(buffer, tmp_buf, count);
+		kfree(tmp_buf);
+	} else {
+		count = -ENOMEM;
+	}
+
+	wake_up(&hidg->read_queue);
+
+	return count;
+}
+
+static ssize_t f_hidg_read(struct file *file, char __user *buffer,
+			   size_t count, loff_t *ptr)
+{
+	struct f_hidg *hidg = file->private_data;
+
+	if (hidg->use_out_ep)
+		return f_hidg_intout_read(file, buffer, count, ptr);
+	else
+		return f_hidg_ssreport_read(file, buffer, count, ptr);
+}
+
 static void f_hidg_req_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	struct f_hidg *hidg = (struct f_hidg *)ep->driver_data;
@@ -433,14 +518,20 @@ static __poll_t f_hidg_poll(struct file *file, poll_table *wait)
 	if (WRITE_COND)
 		ret |= EPOLLOUT | EPOLLWRNORM;
 
-	if (READ_COND)
-		ret |= EPOLLIN | EPOLLRDNORM;
+	if (hidg->use_out_ep) {
+		if (READ_COND_INTOUT)
+			ret |= EPOLLIN | EPOLLRDNORM;
+	} else {
+		if (READ_COND_SSREPORT)
+			ret |= EPOLLIN | EPOLLRDNORM;
+	}
 
 	return ret;
 }
 
 #undef WRITE_COND
-#undef READ_COND
+#undef READ_COND_SSREPORT
+#undef READ_COND_INTOUT
 
 static int f_hidg_release(struct inode *inode, struct file *fd)
 {
@@ -467,8 +558,10 @@ static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep,
 	return alloc_ep_req(ep, length);
 }
 
-static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request *req)
+static void hidg_intout_complete(struct usb_ep *ep, struct usb_request *req)
 {
+	/* used only if the OUT endpoint is configured */
+
 	struct f_hidg *hidg = (struct f_hidg *) req->context;
 	struct usb_composite_dev *cdev = hidg->func.config->cdev;
 	struct f_hidg_req_list *req_list;
@@ -502,6 +595,39 @@ static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request *req)
 	}
 }
 
+static void hidg_ssreport_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	/* used only if the OUT endpoint is NOT configured */
+
+	struct f_hidg *hidg = (struct f_hidg *)req->context;
+	struct usb_composite_dev *cdev = hidg->func.config->cdev;
+	char *new_buf = NULL;
+	unsigned long flags;
+
+	if (req->status != 0 || req->buf == NULL || req->actual == 0) {
+		ERROR(cdev,
+		      "%s FAILED: status=%d, buf=%p, actual=%d\n",
+		      __func__, req->status, req->buf, req->actual);
+		return;
+	}
+
+	spin_lock_irqsave(&hidg->read_spinlock, flags);
+
+	new_buf = krealloc(hidg->set_report_buf, req->actual, GFP_ATOMIC);
+	if (new_buf == NULL) {
+		spin_unlock_irqrestore(&hidg->read_spinlock, flags);
+		return;
+	}
+	hidg->set_report_buf = new_buf;
+
+	hidg->set_report_length = req->actual;
+	memcpy(hidg->set_report_buf, req->buf, req->actual);
+
+	spin_unlock_irqrestore(&hidg->read_spinlock, flags);
+
+	wake_up(&hidg->read_queue);
+}
+
 static int hidg_setup(struct usb_function *f,
 		const struct usb_ctrlrequest *ctrl)
 {
@@ -549,7 +675,11 @@ static int hidg_setup(struct usb_function *f,
 	case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
 		  | HID_REQ_SET_REPORT):
 		VDBG(cdev, "set_report | wLength=%d\n", ctrl->wLength);
-		goto stall;
+		if (hidg->use_out_ep)
+			goto stall;
+		req->complete = hidg_ssreport_complete;
+		req->context  = hidg;
+		goto respond;
 		break;
 
 	case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
@@ -637,15 +767,18 @@ static void hidg_disable(struct usb_function *f)
 	unsigned long flags;
 
 	usb_ep_disable(hidg->in_ep);
-	usb_ep_disable(hidg->out_ep);
 
-	spin_lock_irqsave(&hidg->read_spinlock, flags);
-	list_for_each_entry_safe(list, next, &hidg->completed_out_req, list) {
-		free_ep_req(hidg->out_ep, list->req);
-		list_del(&list->list);
-		kfree(list);
+	if (hidg->out_ep) {
+		usb_ep_disable(hidg->out_ep);
+
+		spin_lock_irqsave(&hidg->read_spinlock, flags);
+		list_for_each_entry_safe(list, next, &hidg->completed_out_req, list) {
+			free_ep_req(hidg->out_ep, list->req);
+			list_del(&list->list);
+			kfree(list);
+		}
+		spin_unlock_irqrestore(&hidg->read_spinlock, flags);
 	}
-	spin_unlock_irqrestore(&hidg->read_spinlock, flags);
 
 	spin_lock_irqsave(&hidg->write_spinlock, flags);
 	if (!hidg->write_pending) {
@@ -691,8 +824,7 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 		}
 	}
 
-
-	if (hidg->out_ep != NULL) {
+	if (hidg->use_out_ep && hidg->out_ep != NULL) {
 		/* restart endpoint */
 		usb_ep_disable(hidg->out_ep);
 
@@ -717,7 +849,7 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 					hidg_alloc_ep_req(hidg->out_ep,
 							  hidg->report_length);
 			if (req) {
-				req->complete = hidg_set_report_complete;
+				req->complete = hidg_intout_complete;
 				req->context  = hidg;
 				status = usb_ep_queue(hidg->out_ep, req,
 						      GFP_ATOMIC);
@@ -743,7 +875,8 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 	}
 	return 0;
 disable_out_ep:
-	usb_ep_disable(hidg->out_ep);
+	if (hidg->out_ep)
+		usb_ep_disable(hidg->out_ep);
 free_req_in:
 	if (req_in)
 		free_ep_req(hidg->in_ep, req_in);
@@ -795,14 +928,21 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
 		goto fail;
 	hidg->in_ep = ep;
 
-	ep = usb_ep_autoconfig(c->cdev->gadget, &hidg_fs_out_ep_desc);
-	if (!ep)
-		goto fail;
-	hidg->out_ep = ep;
+	hidg->out_ep = NULL;
+	if (hidg->use_out_ep) {
+		ep = usb_ep_autoconfig(c->cdev->gadget, &hidg_fs_out_ep_desc);
+		if (!ep)
+			goto fail;
+		hidg->out_ep = ep;
+	}
+
+	/* used only if use_out_ep == 1 */
+	hidg->set_report_buf = NULL;
 
 	/* set descriptor dynamic values */
 	hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
 	hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
+	hidg_interface_desc.bNumEndpoints = hidg->use_out_ep ? 2 : 1;
 	hidg->protocol = HID_REPORT_PROTOCOL;
 	hidg->idle = 1;
 	hidg_ss_in_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
@@ -833,12 +973,19 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
 	hidg_ss_out_ep_desc.bEndpointAddress =
 		hidg_fs_out_ep_desc.bEndpointAddress;
 
-	status = usb_assign_descriptors(f, hidg_fs_descriptors,
-			hidg_hs_descriptors, hidg_ss_descriptors,
-			hidg_ss_descriptors);
+#define CHOOSE_DESC(prefix)	\
+	(hidg->use_out_ep ? prefix##_intout : prefix##_ssreport)
+
+	status = usb_assign_descriptors(f,
+		CHOOSE_DESC(hidg_fs_descriptors),
+		CHOOSE_DESC(hidg_hs_descriptors),
+		CHOOSE_DESC(hidg_ss_descriptors),
+		CHOOSE_DESC(hidg_ss_descriptors));
 	if (status)
 		goto fail;
 
+#undef CHOOSE_DESC
+
 	spin_lock_init(&hidg->write_spinlock);
 	hidg->write_pending = 1;
 	hidg->req = NULL;
@@ -950,6 +1097,7 @@ CONFIGFS_ATTR(f_hid_opts_, name)
 
 F_HID_OPT(subclass, 8, 255);
 F_HID_OPT(protocol, 8, 255);
+F_HID_OPT(no_out_endpoint, 8, 1);
 F_HID_OPT(report_length, 16, 65535);
 
 static ssize_t f_hid_opts_report_desc_show(struct config_item *item, char *page)
@@ -1009,6 +1157,7 @@ CONFIGFS_ATTR_RO(f_hid_opts_, dev);
 static struct configfs_attribute *hid_attrs[] = {
 	&f_hid_opts_attr_subclass,
 	&f_hid_opts_attr_protocol,
+	&f_hid_opts_attr_no_out_endpoint,
 	&f_hid_opts_attr_report_length,
 	&f_hid_opts_attr_report_desc,
 	&f_hid_opts_attr_dev,
@@ -1093,6 +1242,7 @@ static void hidg_free(struct usb_function *f)
 	hidg = func_to_hidg(f);
 	opts = container_of(f->fi, struct f_hid_opts, func_inst);
 	kfree(hidg->report_desc);
+	kfree(hidg->set_report_buf);
 	kfree(hidg);
 	mutex_lock(&opts->lock);
 	--opts->refcnt;
@@ -1139,6 +1289,7 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi)
 			return ERR_PTR(-ENOMEM);
 		}
 	}
+	hidg->use_out_ep = !opts->no_out_endpoint;
 
 	mutex_unlock(&opts->lock);
 
diff --git a/drivers/usb/gadget/function/u_hid.h b/drivers/usb/gadget/function/u_hid.h
index 98d6af558c03..84bb70292855 100644
--- a/drivers/usb/gadget/function/u_hid.h
+++ b/drivers/usb/gadget/function/u_hid.h
@@ -20,6 +20,7 @@ struct f_hid_opts {
 	int				minor;
 	unsigned char			subclass;
 	unsigned char			protocol;
+	unsigned char			no_out_endpoint;
 	unsigned short			report_length;
 	unsigned short			report_desc_length;
 	unsigned char			*report_desc;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] usb: gadget: f_hid: optional SETUP/SET_REPORT mode
  2021-08-14  3:12 [PATCH v2] usb: gadget: f_hid: optional SETUP/SET_REPORT mode Maxim Devaev
@ 2021-08-19 21:13 ` Maciej Żenczykowski
  2021-08-20 14:36   ` Maxim Devaev
  0 siblings, 1 reply; 3+ messages in thread
From: Maciej Żenczykowski @ 2021-08-19 21:13 UTC (permalink / raw)
  To: Maxim Devaev
  Cc: balbi, stern, Greg Kroah-Hartman, ruslan.bilovol,
	mika.westerberg, jj251510319013, linux-usb, Kernel hackers

I'm a noob when it comes to usb, so some of my questions may well be stupid...

On Sat, Aug 14, 2021 at 5:13 AM Maxim Devaev <mdevaev@gmail.com> wrote:
>
> f_hid provides the OUT Endpoint as only way for receiving reports
> from the host. SETUP/SET_REPORT method is not supported, and this causes
> a number of compatibility problems with various host drivers, especially
> in the case of keyboard emulation using f_hid.
>
> - Some hosts do not support the OUT Endpoint and ignore it,
>   so it becomes impossible for the gadget to receive a report
>   from the host. In the case of a keyboard, the gadget loses the ability
>   to get the status of the LEDs.

perhaps better to rephrase as 'the host ceases to change the status of
the gadget/keyboard LEDs',
unless this is actually driven by the keyboard as opposed to the other
way round (which is what I'd expect from AT, PS/2 keyboards).

> - Some BIOSes/UEFIs can't work with HID with the OUT Endpoint at all.

HID devices with an OUT Endpoint

>   This may be due to their bugs or incomplete implementation
>   of the HID standard.
>   For example, absolutely all Apple UEFIs can't handle the OUT Endpoint
>   if it goes after IN Endpoint in the descriptor and require the reverse
>   order (OUT, IN) which is a violation of the standard.
>   Other hosts either do not initialize gadgets with a descriptor
>   containing the OUT Endpoint completely (like some HP and DELL BIOSes
>   and embedded firmwares like on KVM switches), or initialize them,
>   but will not poll.

Not clear what 'not poll' means here.  Why would they (the host) need
to poll an OUT endpoint?

Additionally it seems like any keyboard gadget would want to default
to the older more compatible mode?
Or are there compatibility problems with it as well?

> This patch adds option no_out_endpoint=1 to disable the OUT Endpoint
> and allow f_hid to receive reports from the host via SETUP/SET_REPORT.

allows

Is this a kcmdline option? module param? sysfs option?
Can one dynamically change this as one reconfigures the gadget for a new mode?

>
> Previously, there was such a feature in f_hid, but it was replaced
> by the OUT Endpoint [1] in the commit 99c515005857 ("usb: gadget: hidg:
> register OUT INT endpoint for SET_REPORT"). So this patch actually returns
> the removed functionality making it optional. For backward compatibility

while making it optional

> reasons, the OUT Endpoint mode remains the default behaviour.
>
> - The OUT Endpoint mode provides the report queue and reduces USB overhead
>   (eliminating SETUP routine) on transmitting a report from the host.
>
> - If the SETUP/SET_REPORT mode is used, there is no report queue,
>   so the userspace will only read the last report. For classic HID

without 'the'

>   devices like keyboard this is not a problem, since it is intended

keyboards

>   to transmit the status of the LEDs and only the last report
>   is important. This mode provides better compatibility with strange
>   and buggy host drivers.
>
> Both modes passed USBCV tests. Checking with the USB protocol analyzer
> also confirmed that everything is working as it should and the new mode
> ensures operability in all of the described cases.
>
> Signed-off-by: Maxim Devaev <mdevaev@gmail.com>
> Link: https://www.spinics.net/lists/linux-usb/msg65494.html [1]
> ---
>  drivers/usb/gadget/function/f_hid.c | 217 +++++++++++++++++++++++-----
>  drivers/usb/gadget/function/u_hid.h |   1 +
>  2 files changed, 185 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
> index bb476e121eae..e3fb73ed696d 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -45,12 +45,17 @@ struct f_hidg {
>         unsigned short                  report_desc_length;
>         char                            *report_desc;
>         unsigned short                  report_length;
> +       bool                            use_out_ep;

this could use a comment along the lines of /* as opposed to SETUP/SET_REPORT */

>
>         /* recv report */
> -       struct list_head                completed_out_req;
>         spinlock_t                      read_spinlock;
>         wait_queue_head_t               read_queue;
> +       /* recv report - interrupt out only (use_out_ep == 1) */
> +       struct list_head                completed_out_req;
>         unsigned int                    qlen;
> +       /* recv report - setup set_report only (use_out_ep == 0) */
> +       char                            *set_report_buf;
> +       unsigned int                    set_report_length;
>
>         /* send report */
>         spinlock_t                      write_spinlock;
> @@ -79,7 +84,7 @@ static struct usb_interface_descriptor hidg_interface_desc = {
>         .bDescriptorType        = USB_DT_INTERFACE,
>         /* .bInterfaceNumber    = DYNAMIC */
>         .bAlternateSetting      = 0,
> -       .bNumEndpoints          = 2,
> +       /* .bNumEndpoints       = DYNAMIC */

if you look down below, this isn't actually dynamic, should we just
have hidg_interface_desc_{intout,ssreport} structs?

>         .bInterfaceClass        = USB_CLASS_HID,
>         /* .bInterfaceSubClass  = DYNAMIC */
>         /* .bInterfaceProtocol  = DYNAMIC */
> @@ -140,7 +145,7 @@ static struct usb_ss_ep_comp_descriptor hidg_ss_out_comp_desc = {
>         /* .wBytesPerInterval   = DYNAMIC */
>  };
>
> -static struct usb_descriptor_header *hidg_ss_descriptors[] = {
> +static struct usb_descriptor_header *hidg_ss_descriptors_intout[] = {
>         (struct usb_descriptor_header *)&hidg_interface_desc,
>         (struct usb_descriptor_header *)&hidg_desc,
>         (struct usb_descriptor_header *)&hidg_ss_in_ep_desc,
> @@ -150,6 +155,14 @@ static struct usb_descriptor_header *hidg_ss_descriptors[] = {
>         NULL,
>  };
>
> +static struct usb_descriptor_header *hidg_ss_descriptors_ssreport[] = {
> +       (struct usb_descriptor_header *)&hidg_interface_desc,
> +       (struct usb_descriptor_header *)&hidg_desc,
> +       (struct usb_descriptor_header *)&hidg_ss_in_ep_desc,
> +       (struct usb_descriptor_header *)&hidg_ss_in_comp_desc,
> +       NULL,
> +};
> +
>  /* High-Speed Support */
>
>  static struct usb_endpoint_descriptor hidg_hs_in_ep_desc = {
> @@ -176,7 +189,7 @@ static struct usb_endpoint_descriptor hidg_hs_out_ep_desc = {
>                                       */
>  };
>
> -static struct usb_descriptor_header *hidg_hs_descriptors[] = {
> +static struct usb_descriptor_header *hidg_hs_descriptors_intout[] = {
>         (struct usb_descriptor_header *)&hidg_interface_desc,
>         (struct usb_descriptor_header *)&hidg_desc,
>         (struct usb_descriptor_header *)&hidg_hs_in_ep_desc,
> @@ -184,6 +197,13 @@ static struct usb_descriptor_header *hidg_hs_descriptors[] = {
>         NULL,
>  };
>
> +static struct usb_descriptor_header *hidg_hs_descriptors_ssreport[] = {
> +       (struct usb_descriptor_header *)&hidg_interface_desc,
> +       (struct usb_descriptor_header *)&hidg_desc,
> +       (struct usb_descriptor_header *)&hidg_hs_in_ep_desc,
> +       NULL,
> +};
> +
>  /* Full-Speed Support */
>
>  static struct usb_endpoint_descriptor hidg_fs_in_ep_desc = {
> @@ -210,7 +230,7 @@ static struct usb_endpoint_descriptor hidg_fs_out_ep_desc = {
>                                        */
>  };
>
> -static struct usb_descriptor_header *hidg_fs_descriptors[] = {
> +static struct usb_descriptor_header *hidg_fs_descriptors_intout[] = {
>         (struct usb_descriptor_header *)&hidg_interface_desc,
>         (struct usb_descriptor_header *)&hidg_desc,
>         (struct usb_descriptor_header *)&hidg_fs_in_ep_desc,
> @@ -218,6 +238,13 @@ static struct usb_descriptor_header *hidg_fs_descriptors[] = {
>         NULL,
>  };
>
> +static struct usb_descriptor_header *hidg_fs_descriptors_ssreport[] = {
> +       (struct usb_descriptor_header *)&hidg_interface_desc,
> +       (struct usb_descriptor_header *)&hidg_desc,
> +       (struct usb_descriptor_header *)&hidg_fs_in_ep_desc,
> +       NULL,
> +};
> +
>  /*-------------------------------------------------------------------------*/
>  /*                                 Strings                                 */
>
> @@ -241,9 +268,11 @@ static struct usb_gadget_strings *ct_func_strings[] = {
>  /*-------------------------------------------------------------------------*/
>  /*                              Char Device                                */
>
> -static ssize_t f_hidg_read(struct file *file, char __user *buffer,
> -                       size_t count, loff_t *ptr)
> +static ssize_t f_hidg_intout_read(struct file *file, char __user *buffer,
> +                                 size_t count, loff_t *ptr)
>  {
> +       /* used only if the OUT endpoint is configured */
> +

comment probably belongs above the function instead of inside it

>         struct f_hidg *hidg = file->private_data;
>         struct f_hidg_req_list *list;
>         struct usb_request *req;
> @@ -255,15 +284,15 @@ static ssize_t f_hidg_read(struct file *file, char __user *buffer,
>
>         spin_lock_irqsave(&hidg->read_spinlock, flags);
>
> -#define READ_COND (!list_empty(&hidg->completed_out_req))
> +#define READ_COND_INTOUT (!list_empty(&hidg->completed_out_req))
>
>         /* wait for at least one buffer to complete */
> -       while (!READ_COND) {
> +       while (!READ_COND_INTOUT) {
>                 spin_unlock_irqrestore(&hidg->read_spinlock, flags);
>                 if (file->f_flags & O_NONBLOCK)
>                         return -EAGAIN;
>
> -               if (wait_event_interruptible(hidg->read_queue, READ_COND))
> +               if (wait_event_interruptible(hidg->read_queue, READ_COND_INTOUT))
>                         return -ERESTARTSYS;
>
>                 spin_lock_irqsave(&hidg->read_spinlock, flags);
> @@ -313,6 +342,62 @@ static ssize_t f_hidg_read(struct file *file, char __user *buffer,
>         return count;
>  }
>
> +#define READ_COND_SSREPORT (hidg->set_report_buf != NULL)
> +
> +static ssize_t f_hidg_ssreport_read(struct file *file, char __user *buffer,
> +                                   size_t count, loff_t *ptr)
> +{
> +       /* used only if the OUT endpoint is NOT configured */
> +
> +       struct f_hidg *hidg = file->private_data;
> +       char *tmp_buf = NULL;
> +       unsigned long flags;
> +
> +       if (!count)
> +               return 0;
> +
> +       spin_lock_irqsave(&hidg->read_spinlock, flags);
> +
> +       while (!READ_COND_SSREPORT) {
> +               spin_unlock_irqrestore(&hidg->read_spinlock, flags);
> +               if (file->f_flags & O_NONBLOCK)
> +                       return -EAGAIN;
> +
> +               if (wait_event_interruptible(hidg->read_queue, READ_COND_SSREPORT))
> +                       return -ERESTARTSYS;
> +
> +               spin_lock_irqsave(&hidg->read_spinlock, flags);
> +       }
> +
> +       count = min_t(unsigned int, count, hidg->set_report_length);
> +       tmp_buf = hidg->set_report_buf;
> +       hidg->set_report_buf = NULL;
> +
> +       spin_unlock_irqrestore(&hidg->read_spinlock, flags);
> +
> +       if (tmp_buf != NULL) {
> +               count -= copy_to_user(buffer, tmp_buf, count);
> +               kfree(tmp_buf);
> +       } else {
> +               count = -ENOMEM;
> +       }
> +
> +       wake_up(&hidg->read_queue);
> +
> +       return count;
> +}
> +
> +static ssize_t f_hidg_read(struct file *file, char __user *buffer,
> +                          size_t count, loff_t *ptr)
> +{
> +       struct f_hidg *hidg = file->private_data;
> +
> +       if (hidg->use_out_ep)
> +               return f_hidg_intout_read(file, buffer, count, ptr);
> +       else
> +               return f_hidg_ssreport_read(file, buffer, count, ptr);
> +}
> +
>  static void f_hidg_req_complete(struct usb_ep *ep, struct usb_request *req)
>  {
>         struct f_hidg *hidg = (struct f_hidg *)ep->driver_data;
> @@ -433,14 +518,20 @@ static __poll_t f_hidg_poll(struct file *file, poll_table *wait)
>         if (WRITE_COND)
>                 ret |= EPOLLOUT | EPOLLWRNORM;
>
> -       if (READ_COND)
> -               ret |= EPOLLIN | EPOLLRDNORM;
> +       if (hidg->use_out_ep) {
> +               if (READ_COND_INTOUT)
> +                       ret |= EPOLLIN | EPOLLRDNORM;
> +       } else {
> +               if (READ_COND_SSREPORT)
> +                       ret |= EPOLLIN | EPOLLRDNORM;
> +       }
>
>         return ret;
>  }
>
>  #undef WRITE_COND
> -#undef READ_COND
> +#undef READ_COND_SSREPORT
> +#undef READ_COND_INTOUT
>
>  static int f_hidg_release(struct inode *inode, struct file *fd)
>  {
> @@ -467,8 +558,10 @@ static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep,
>         return alloc_ep_req(ep, length);
>  }
>
> -static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request *req)
> +static void hidg_intout_complete(struct usb_ep *ep, struct usb_request *req)
>  {
> +       /* used only if the OUT endpoint is configured */
> +

ditto

>         struct f_hidg *hidg = (struct f_hidg *) req->context;
>         struct usb_composite_dev *cdev = hidg->func.config->cdev;
>         struct f_hidg_req_list *req_list;
> @@ -502,6 +595,39 @@ static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request *req)
>         }
>  }
>
> +static void hidg_ssreport_complete(struct usb_ep *ep, struct usb_request *req)
> +{
> +       /* used only if the OUT endpoint is NOT configured */
> +

ditto

> +       struct f_hidg *hidg = (struct f_hidg *)req->context;
> +       struct usb_composite_dev *cdev = hidg->func.config->cdev;
> +       char *new_buf = NULL;
> +       unsigned long flags;
> +
> +       if (req->status != 0 || req->buf == NULL || req->actual == 0) {
> +               ERROR(cdev,
> +                     "%s FAILED: status=%d, buf=%p, actual=%d\n",
> +                     __func__, req->status, req->buf, req->actual);
> +               return;
> +       }
> +
> +       spin_lock_irqsave(&hidg->read_spinlock, flags);
> +
> +       new_buf = krealloc(hidg->set_report_buf, req->actual, GFP_ATOMIC);
> +       if (new_buf == NULL) {
> +               spin_unlock_irqrestore(&hidg->read_spinlock, flags);
> +               return;
> +       }
> +       hidg->set_report_buf = new_buf;
> +
> +       hidg->set_report_length = req->actual;
> +       memcpy(hidg->set_report_buf, req->buf, req->actual);
> +
> +       spin_unlock_irqrestore(&hidg->read_spinlock, flags);
> +
> +       wake_up(&hidg->read_queue);
> +}
> +
>  static int hidg_setup(struct usb_function *f,
>                 const struct usb_ctrlrequest *ctrl)
>  {
> @@ -549,7 +675,11 @@ static int hidg_setup(struct usb_function *f,
>         case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
>                   | HID_REQ_SET_REPORT):
>                 VDBG(cdev, "set_report | wLength=%d\n", ctrl->wLength);
> -               goto stall;
> +               if (hidg->use_out_ep)
> +                       goto stall;
> +               req->complete = hidg_ssreport_complete;
> +               req->context  = hidg;
> +               goto respond;
>                 break;
>
>         case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
> @@ -637,15 +767,18 @@ static void hidg_disable(struct usb_function *f)
>         unsigned long flags;
>
>         usb_ep_disable(hidg->in_ep);
> -       usb_ep_disable(hidg->out_ep);
>
> -       spin_lock_irqsave(&hidg->read_spinlock, flags);
> -       list_for_each_entry_safe(list, next, &hidg->completed_out_req, list) {
> -               free_ep_req(hidg->out_ep, list->req);
> -               list_del(&list->list);
> -               kfree(list);
> +       if (hidg->out_ep) {
> +               usb_ep_disable(hidg->out_ep);
> +
> +               spin_lock_irqsave(&hidg->read_spinlock, flags);
> +               list_for_each_entry_safe(list, next, &hidg->completed_out_req, list) {
> +                       free_ep_req(hidg->out_ep, list->req);
> +                       list_del(&list->list);
> +                       kfree(list);
> +               }
> +               spin_unlock_irqrestore(&hidg->read_spinlock, flags);
>         }
> -       spin_unlock_irqrestore(&hidg->read_spinlock, flags);
>
>         spin_lock_irqsave(&hidg->write_spinlock, flags);
>         if (!hidg->write_pending) {
> @@ -691,8 +824,7 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
>                 }
>         }
>
> -
> -       if (hidg->out_ep != NULL) {
> +       if (hidg->use_out_ep && hidg->out_ep != NULL) {
>                 /* restart endpoint */
>                 usb_ep_disable(hidg->out_ep);
>
> @@ -717,7 +849,7 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
>                                         hidg_alloc_ep_req(hidg->out_ep,
>                                                           hidg->report_length);
>                         if (req) {
> -                               req->complete = hidg_set_report_complete;
> +                               req->complete = hidg_intout_complete;
>                                 req->context  = hidg;
>                                 status = usb_ep_queue(hidg->out_ep, req,
>                                                       GFP_ATOMIC);
> @@ -743,7 +875,8 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
>         }
>         return 0;
>  disable_out_ep:
> -       usb_ep_disable(hidg->out_ep);
> +       if (hidg->out_ep)
> +               usb_ep_disable(hidg->out_ep);
>  free_req_in:
>         if (req_in)
>                 free_ep_req(hidg->in_ep, req_in);
> @@ -795,14 +928,21 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
>                 goto fail;
>         hidg->in_ep = ep;
>
> -       ep = usb_ep_autoconfig(c->cdev->gadget, &hidg_fs_out_ep_desc);
> -       if (!ep)
> -               goto fail;
> -       hidg->out_ep = ep;
> +       hidg->out_ep = NULL;
> +       if (hidg->use_out_ep) {
> +               ep = usb_ep_autoconfig(c->cdev->gadget, &hidg_fs_out_ep_desc);
> +               if (!ep)
> +                       goto fail;
> +               hidg->out_ep = ep;
> +       }
> +
> +       /* used only if use_out_ep == 1 */
> +       hidg->set_report_buf = NULL;
>
>         /* set descriptor dynamic values */
>         hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
>         hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
> +       hidg_interface_desc.bNumEndpoints = hidg->use_out_ep ? 2 : 1;
>         hidg->protocol = HID_REPORT_PROTOCOL;
>         hidg->idle = 1;
>         hidg_ss_in_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
> @@ -833,12 +973,19 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
>         hidg_ss_out_ep_desc.bEndpointAddress =
>                 hidg_fs_out_ep_desc.bEndpointAddress;
>
> -       status = usb_assign_descriptors(f, hidg_fs_descriptors,
> -                       hidg_hs_descriptors, hidg_ss_descriptors,
> -                       hidg_ss_descriptors);
> +#define CHOOSE_DESC(prefix)    \
> +       (hidg->use_out_ep ? prefix##_intout : prefix##_ssreport)
> +
> +       status = usb_assign_descriptors(f,
> +               CHOOSE_DESC(hidg_fs_descriptors),
> +               CHOOSE_DESC(hidg_hs_descriptors),
> +               CHOOSE_DESC(hidg_ss_descriptors),
> +               CHOOSE_DESC(hidg_ss_descriptors));

may be better to just use an if (hidg->use_out_ep) status =
usb_assign_descriptors(...) else status = usb_assign_descriptors(...)

(or just status = hidg->use_out_ep ? usb_assign_descriptors(...) :
usb_assign_descriptors(...)

it's not really any longer (when you count up the extra lines adding
and undefing the macro) and eliminates a macro making things easier to
grep for...

>         if (status)
>                 goto fail;
>
> +#undef CHOOSE_DESC
> +
>         spin_lock_init(&hidg->write_spinlock);
>         hidg->write_pending = 1;
>         hidg->req = NULL;
> @@ -950,6 +1097,7 @@ CONFIGFS_ATTR(f_hid_opts_, name)
>
>  F_HID_OPT(subclass, 8, 255);
>  F_HID_OPT(protocol, 8, 255);
> +F_HID_OPT(no_out_endpoint, 8, 1);
>  F_HID_OPT(report_length, 16, 65535);
>
>  static ssize_t f_hid_opts_report_desc_show(struct config_item *item, char *page)
> @@ -1009,6 +1157,7 @@ CONFIGFS_ATTR_RO(f_hid_opts_, dev);
>  static struct configfs_attribute *hid_attrs[] = {
>         &f_hid_opts_attr_subclass,
>         &f_hid_opts_attr_protocol,
> +       &f_hid_opts_attr_no_out_endpoint,
>         &f_hid_opts_attr_report_length,
>         &f_hid_opts_attr_report_desc,
>         &f_hid_opts_attr_dev,
> @@ -1093,6 +1242,7 @@ static void hidg_free(struct usb_function *f)
>         hidg = func_to_hidg(f);
>         opts = container_of(f->fi, struct f_hid_opts, func_inst);
>         kfree(hidg->report_desc);
> +       kfree(hidg->set_report_buf);
>         kfree(hidg);
>         mutex_lock(&opts->lock);
>         --opts->refcnt;
> @@ -1139,6 +1289,7 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi)
>                         return ERR_PTR(-ENOMEM);
>                 }
>         }
> +       hidg->use_out_ep = !opts->no_out_endpoint;

maybe it would be better to use consistent naming...

hidg->no_out_endpoint = opts->no_out_endpoint

or call the option 'use_ssreport' instead of 'no_out_endpoint'
(negatives are harder to think about)

>
>         mutex_unlock(&opts->lock);
>
> diff --git a/drivers/usb/gadget/function/u_hid.h b/drivers/usb/gadget/function/u_hid.h
> index 98d6af558c03..84bb70292855 100644
> --- a/drivers/usb/gadget/function/u_hid.h
> +++ b/drivers/usb/gadget/function/u_hid.h
> @@ -20,6 +20,7 @@ struct f_hid_opts {
>         int                             minor;
>         unsigned char                   subclass;
>         unsigned char                   protocol;
> +       unsigned char                   no_out_endpoint;
>         unsigned short                  report_length;
>         unsigned short                  report_desc_length;
>         unsigned char                   *report_desc;
> --
> 2.32.0
>

Anyway, nothing in here is particularly important, just loose thoughts.
In general this seems pretty nice.

- Maciej

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] usb: gadget: f_hid: optional SETUP/SET_REPORT mode
  2021-08-19 21:13 ` Maciej Żenczykowski
@ 2021-08-20 14:36   ` Maxim Devaev
  0 siblings, 0 replies; 3+ messages in thread
From: Maxim Devaev @ 2021-08-20 14:36 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: balbi, stern, Greg Kroah-Hartman, ruslan.bilovol,
	mika.westerberg, jj251510319013, linux-usb, Kernel hackers

Maciej Żenczykowski <maze@google.com> wrote:
> perhaps better to rephrase as 'the host ceases to change the status of
> the gadget/keyboard LEDs',
> unless this is actually driven by the keyboard as opposed to the other
> way round (which is what I'd expect from AT, PS/2 keyboards).

Since I'm describing the behavior on the gadget side,
it seemed to me that the explanation about the impossibility
of receiving it was more appropriate.

> Not clear what 'not poll' means here.  Why would they (the host) need
> to poll an OUT endpoint?

Poll the IN Endpoint, fixed

> Additionally it seems like any keyboard gadget would want to default
> to the older more compatible mode?
> Or are there compatibility problems with it as well?

Yes, any keyboard should use SETUP/SET_REPORT if it wants maximum
compatibility. This mode has no problems with hosts. I suggest it
as optional only because for the last 9 years the default behavior
has been OUT Endpoint and in the place of those people who use it,
I would be upset if this was changed, since it could lead to strange
problems like lack of a queue and loss of events if f_hid is used
for data transmission, and not for emulating input devices.

> if you look down below, this isn't actually dynamic, should we just
> have hidg_interface_desc_{intout,ssreport} structs?

DYNAMIC is indirectly provided by the no_out_endpoints flag.
I preferred to avoid duplicating the code here.

> may be better to just use an if (hidg->use_out_ep) status =
> usb_assign_descriptors(...) else status = usb_assign_descriptors(...)

Yep, you're right

> maybe it would be better to use consistent naming...
> 
> hidg->no_out_endpoint = opts->no_out_endpoint
> 
> or call the option 'use_ssreport' instead of 'no_out_endpoint'
> (negatives are harder to think about)

Yea, I also thought about it and made such a name precisely
based on consistency. The rest of the code contains the out_ep
variable, so it was logical to make the use_out_ep flag.
In addition, there are no long names like *_out_endpoint anywhere.
At the same time, abbreviations are not used in configfs
and I didn't want to make a flag there that the user should change
to 0 if he does not want to use out endpoint. I would prefer
to leave it as it is, because it does not use negation logic,
and it will be easy for you to grep something like out_ep\>

> Anyway, nothing in here is particularly important, just loose thoughts.
> In general this seems pretty nice.

Thank you for the review! I will fix this and make a third version of the patch.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-20 14:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-14  3:12 [PATCH v2] usb: gadget: f_hid: optional SETUP/SET_REPORT mode Maxim Devaev
2021-08-19 21:13 ` Maciej Żenczykowski
2021-08-20 14:36   ` Maxim Devaev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).