[PATCH v2 0/3] USB: gadget: f_hid: Add Feature reports

[PATCH v2 0/3] USB: gadget: f_hid: Add Feature reports

[Vicki Pfau]

This patchset adds support for Set- and Get-Feature report handling in
the f_hid gadget subsystem.

Following up on a patchset from mid-2022[1] which stalled out due in
part to neither Maxim Devaev nor I properly testing the interaction with
the use_out_ep feature he added at some point, I've revised it based on
Greg's original feedback.

Unfortuantely, it was never quite clear to me how to properly test this
patchset per Maxim's concerns, so I was hoping to resubmit it with
Greg's feedback addressed and see if Maxim is able to test it now.

Note also that this is based on my original patchset, not the revised
version submitted by David Sands[2] at a later point. He may have some
opinions that would make sense to address before anything exposed to
usermode lands.

[1]: https://lore.kernel.org/linux-usb/20220726005824.2817646-1-vi@endrift.com/
[2]: https://lore.kernel.org/linux-usb/20230215231529.2513236-1-david.sands@biamp.com/

Vicki Pfau (3):
  USB: gadget: Move gadget-related ioctl codes to gadget-ioctl.h
  USB: gadget: f_hid: Add Get-Feature report
  USB: gadget: f_hid: Add Set-Feature report

 drivers/usb/gadget/function/f_hid.c   | 231 ++++++++++++++++++++++++--
 include/uapi/linux/usb/g_hid.h        |  19 +++
 include/uapi/linux/usb/g_printer.h    |  23 +--
 include/uapi/linux/usb/g_uvc.h        |   4 +-
 include/uapi/linux/usb/gadget-ioctl.h |  39 +++++
 include/uapi/linux/usb/gadgetfs.h     |  27 +--
 6 files changed, 279 insertions(+), 64 deletions(-)
 create mode 100644 include/uapi/linux/usb/g_hid.h
 create mode 100644 include/uapi/linux/usb/gadget-ioctl.h

-- 
2.43.0

[PATCH v2 1/3] USB: gadget: Move gadget-related ioctl codes to gadget-ioctl.h

[Vicki Pfau]

Since multiple different gadget types use similar ranges of ioctl IDs,
put all of them in the same file to avoid accidentally creating
overlaps.

Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 include/uapi/linux/usb/g_printer.h    | 23 ++----------------
 include/uapi/linux/usb/g_uvc.h        |  4 +--
 include/uapi/linux/usb/gadget-ioctl.h | 35 +++++++++++++++++++++++++++
 include/uapi/linux/usb/gadgetfs.h     | 27 +--------------------
 4 files changed, 39 insertions(+), 50 deletions(-)
 create mode 100644 include/uapi/linux/usb/gadget-ioctl.h

diff --git a/include/uapi/linux/usb/g_printer.h b/include/uapi/linux/usb/g_printer.h
index 7fc20e4b82f5..fc411ee3f5cc 100644
--- a/include/uapi/linux/usb/g_printer.h
+++ b/include/uapi/linux/usb/g_printer.h
@@ -3,34 +3,15 @@
  * g_printer.h -- Header file for USB Printer gadget driver
  *
  * Copyright (C) 2007 Craig W. Nadler
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
 #ifndef __LINUX_USB_G_PRINTER_H
 #define __LINUX_USB_G_PRINTER_H
 
+#include <linux/usb/gadget-ioctl.h>
+
 #define PRINTER_NOT_ERROR	0x08
 #define PRINTER_SELECTED	0x10
 #define PRINTER_PAPER_EMPTY	0x20
 
-/* The 'g' code is also used by gadgetfs ioctl requests.
- * Don't add any colliding codes to either driver, and keep
- * them in unique ranges (size 0x20 for now).
- */
-#define GADGET_GET_PRINTER_STATUS	_IOR('g', 0x21, unsigned char)
-#define GADGET_SET_PRINTER_STATUS	_IOWR('g', 0x22, unsigned char)
-
 #endif /* __LINUX_USB_G_PRINTER_H */
diff --git a/include/uapi/linux/usb/g_uvc.h b/include/uapi/linux/usb/g_uvc.h
index 8d7824dde1b2..7142e05abab1 100644
--- a/include/uapi/linux/usb/g_uvc.h
+++ b/include/uapi/linux/usb/g_uvc.h
@@ -8,9 +8,9 @@
 #ifndef __LINUX_USB_G_UVC_H
 #define __LINUX_USB_G_UVC_H
 
-#include <linux/ioctl.h>
 #include <linux/types.h>
 #include <linux/usb/ch9.h>
+#include <linux/usb/gadget-ioctl.h>
 
 #define UVC_EVENT_FIRST			(V4L2_EVENT_PRIVATE_START + 0)
 #define UVC_EVENT_CONNECT		(V4L2_EVENT_PRIVATE_START + 0)
@@ -37,6 +37,4 @@ struct uvc_event {
 	};
 };
 
-#define UVCIOC_SEND_RESPONSE		_IOW('U', 1, struct uvc_request_data)
-
 #endif /* __LINUX_USB_G_UVC_H */
diff --git a/include/uapi/linux/usb/gadget-ioctl.h b/include/uapi/linux/usb/gadget-ioctl.h
new file mode 100644
index 000000000000..b5f8f7894db7
--- /dev/null
+++ b/include/uapi/linux/usb/gadget-ioctl.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef __LINUX_USB_GADGET_IOCTL_H
+#define __LINUX_USB_GADGET_IOCTL_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+/* gadgetfs endpoint ioctls */
+
+/* IN transfers may be reported to the gadget driver as complete
+ *	when the fifo is loaded, before the host reads the data;
+ * OUT transfers may be reported to the host's "client" driver as
+ *	complete when they're sitting in the FIFO unread.
+ * THIS returns how many bytes are "unclaimed" in the endpoint fifo
+ * (needed for precise fault handling, when the hardware allows it)
+ */
+#define	GADGETFS_FIFO_STATUS	_IO('g', 1)
+
+/* discards any unclaimed data in the fifo. */
+#define	GADGETFS_FIFO_FLUSH	_IO('g', 2)
+
+/* resets endpoint halt+toggle; used to implement set_interface.
+ * some hardware (like pxa2xx) can't support this.
+ */
+#define	GADGETFS_CLEAR_HALT	_IO('g', 3)
+
+/* g_printer ioctls */
+#define GADGET_GET_PRINTER_STATUS	_IOR('g', 0x21, unsigned char)
+#define GADGET_SET_PRINTER_STATUS	_IOWR('g', 0x22, unsigned char)
+
+/* g_uvc ioctls */
+#define UVCIOC_SEND_RESPONSE		_IOW('U', 1, struct uvc_request_data)
+
+#endif /* __LINUX_USB_GADGET_IOCTL_H */
diff --git a/include/uapi/linux/usb/gadgetfs.h b/include/uapi/linux/usb/gadgetfs.h
index 835473910a49..e8629943d249 100644
--- a/include/uapi/linux/usb/gadgetfs.h
+++ b/include/uapi/linux/usb/gadgetfs.h
@@ -20,9 +20,9 @@
 #define __LINUX_USB_GADGETFS_H
 
 #include <linux/types.h>
-#include <linux/ioctl.h>
 
 #include <linux/usb/ch9.h>
+#include <linux/usb/gadget-ioctl.h>
 
 /*
  * Events are delivered on the ep0 file descriptor, when the user mode driver
@@ -61,29 +61,4 @@ struct usb_gadgetfs_event {
 	enum usb_gadgetfs_event_type	type;
 };
 
-
-/* The 'g' code is also used by printer gadget ioctl requests.
- * Don't add any colliding codes to either driver, and keep
- * them in unique ranges (size 0x20 for now).
- */
-
-/* endpoint ioctls */
-
-/* IN transfers may be reported to the gadget driver as complete
- *	when the fifo is loaded, before the host reads the data;
- * OUT transfers may be reported to the host's "client" driver as
- *	complete when they're sitting in the FIFO unread.
- * THIS returns how many bytes are "unclaimed" in the endpoint fifo
- * (needed for precise fault handling, when the hardware allows it)
- */
-#define	GADGETFS_FIFO_STATUS	_IO('g', 1)
-
-/* discards any unclaimed data in the fifo. */
-#define	GADGETFS_FIFO_FLUSH	_IO('g', 2)
-
-/* resets endpoint halt+toggle; used to implement set_interface.
- * some hardware (like pxa2xx) can't support this.
- */
-#define	GADGETFS_CLEAR_HALT	_IO('g', 3)
-
 #endif /* __LINUX_USB_GADGETFS_H */
-- 
2.43.0

[PATCH v2 2/3] USB: gadget: f_hid: Add Get-Feature report

[Vicki Pfau]

While the HID gadget implementation has been sufficient for devices that only
use INTERRUPT transfers, the USB HID standard includes provisions for Set- and
Get-Feature report CONTROL transfers that go over endpoint 0. These were
previously impossible with the existing implementation, and would either send
an empty reply, or stall out.

As the feature is a standard part of USB HID, it stands to reason that devices
would use it, and that the HID gadget should support it. This patch adds
support for (polled) device-to-host Get-Feature reports through a new ioctl
interface to the hidg class dev nodes.

Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/usb/gadget/function/f_hid.c   | 121 ++++++++++++++++++++++++--
 include/uapi/linux/usb/g_hid.h        |  19 ++++
 include/uapi/linux/usb/gadget-ioctl.h |   3 +
 3 files changed, 137 insertions(+), 6 deletions(-)
 create mode 100644 include/uapi/linux/usb/g_hid.h

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 3c8a9dd585c0..5a097ea718e8 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -16,6 +16,7 @@
 #include <linux/wait.h>
 #include <linux/sched.h>
 #include <linux/usb/g_hid.h>
+#include <uapi/linux/usb/g_hid.h>
 
 #include "u_f.h"
 #include "u_hid.h"
@@ -75,6 +76,13 @@ struct f_hidg {
 	wait_queue_head_t		write_queue;
 	struct usb_request		*req;
 
+	/* get report */
+	struct usb_request		*get_req;
+	struct usb_hidg_report		get_report;
+	spinlock_t			get_spinlock;
+	bool				get_pending;
+	wait_queue_head_t		get_queue;
+
 	struct device			dev;
 	struct cdev			cdev;
 	struct usb_function		func;
@@ -524,6 +532,64 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
 	return status;
 }
 
+
+static int f_hidg_get_report(struct file *file, struct usb_hidg_report __user *buffer)
+{
+	struct f_hidg			*hidg = file->private_data;
+	struct usb_composite_dev	*cdev = hidg->func.config->cdev;
+
+	int		status = 0;
+	unsigned long	flags;
+
+	spin_lock_irqsave(&hidg->get_spinlock, flags);
+
+#define GET_REPORT_COND (!hidg->get_pending)
+
+	while (!GET_REPORT_COND) {
+		spin_unlock_irqrestore(&hidg->get_spinlock, flags);
+
+		if (file->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		if (wait_event_interruptible_exclusive(hidg->get_queue,
+						       GET_REPORT_COND))
+			return -ERESTARTSYS;
+
+		spin_lock_irqsave(&hidg->get_spinlock, flags);
+		if (!hidg->get_pending) {
+			spin_unlock_irqrestore(&hidg->get_spinlock, flags);
+			return -EINVAL;
+		}
+	}
+
+	hidg->get_pending = true;
+	spin_unlock_irqrestore(&hidg->get_spinlock, flags);
+
+	status = copy_from_user(&hidg->get_report, buffer,
+				sizeof(struct usb_hidg_report));
+	if (status != 0) {
+		ERROR(cdev, "copy_from_user error\n");
+		status = -EINVAL;
+	}
+
+	spin_lock_irqsave(&hidg->get_spinlock, flags);
+	hidg->get_pending = false;
+	spin_unlock_irqrestore(&hidg->get_spinlock, flags);
+
+	wake_up(&hidg->get_queue);
+	return status;
+}
+
+static long f_hidg_ioctl(struct file *file, unsigned int code, unsigned long arg)
+{
+	switch (code) {
+	case GADGET_HID_WRITE_GET_REPORT:
+		return f_hidg_get_report(file, (struct usb_hidg_report __user *)arg);
+	default:
+		return -ENOTTY;
+	}
+}
+
 static __poll_t f_hidg_poll(struct file *file, poll_table *wait)
 {
 	struct f_hidg	*hidg  = file->private_data;
@@ -549,6 +615,7 @@ static __poll_t f_hidg_poll(struct file *file, poll_table *wait)
 #undef WRITE_COND
 #undef READ_COND_SSREPORT
 #undef READ_COND_INTOUT
+#undef GET_REPORT_COND
 
 static int f_hidg_release(struct inode *inode, struct file *fd)
 {
@@ -641,6 +708,10 @@ static void hidg_ssreport_complete(struct usb_ep *ep, struct usb_request *req)
 	wake_up(&hidg->read_queue);
 }
 
+static void hidg_get_report_complete(struct usb_ep *ep, struct usb_request *req)
+{
+}
+
 static int hidg_setup(struct usb_function *f,
 		const struct usb_ctrlrequest *ctrl)
 {
@@ -648,6 +719,8 @@ static int hidg_setup(struct usb_function *f,
 	struct usb_composite_dev	*cdev = f->config->cdev;
 	struct usb_request		*req  = cdev->req;
 	int status = 0;
+	unsigned long flags;
+	bool do_wake = false;
 	__u16 value, length;
 
 	value	= __le16_to_cpu(ctrl->wValue);
@@ -660,14 +733,29 @@ static int hidg_setup(struct usb_function *f,
 	switch ((ctrl->bRequestType << 8) | ctrl->bRequest) {
 	case ((USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
 		  | HID_REQ_GET_REPORT):
-		VDBG(cdev, "get_report\n");
+		VDBG(cdev, "get_report | wLength=%d\n", ctrl->wLength);
 
-		/* send an empty report */
-		length = min_t(unsigned, length, hidg->report_length);
-		memset(req->buf, 0x0, length);
+		req = hidg->get_req;
+		req->zero = 0;
+		req->length = min_t(unsigned, length, hidg->report_length);
+		status = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
+		if (status < 0) {
+			ERROR(cdev, "usb_ep_queue error on get_report %d\n",
+			      status);
 
-		goto respond;
-		break;
+			spin_lock_irqsave(&hidg->get_spinlock, flags);
+			if (hidg->get_pending) {
+				hidg->get_pending = false;
+				do_wake = true;
+			}
+			spin_unlock_irqrestore(&hidg->get_spinlock, flags);
+
+			if (do_wake) {
+				wake_up(&hidg->get_queue);
+			}
+		}
+
+		return status;
 
 	case ((USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
 		  | HID_REQ_GET_PROTOCOL):
@@ -801,6 +889,14 @@ static void hidg_disable(struct usb_function *f)
 
 	hidg->req = NULL;
 	spin_unlock_irqrestore(&hidg->write_spinlock, flags);
+
+	spin_lock_irqsave(&hidg->get_spinlock, flags);
+	if (!hidg->get_pending) {
+		usb_ep_free_request(f->config->cdev->gadget->ep0, hidg->get_req);
+		hidg->get_pending = true;
+	}
+	hidg->get_req = NULL;
+	spin_unlock_irqrestore(&hidg->get_spinlock, flags);
 }
 
 static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
@@ -909,6 +1005,7 @@ static const struct file_operations f_hidg_fops = {
 	.write		= f_hidg_write,
 	.read		= f_hidg_read,
 	.poll		= f_hidg_poll,
+	.unlocked_ioctl	= f_hidg_ioctl,
 	.llseek		= noop_llseek,
 };
 
@@ -919,6 +1016,14 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
 	struct usb_string	*us;
 	int			status;
 
+	hidg->get_req = usb_ep_alloc_request(c->cdev->gadget->ep0, GFP_ATOMIC);
+	if (!hidg->get_req)
+		return -ENOMEM;
+	hidg->get_req->buf = hidg->get_report.data;
+	hidg->get_req->zero = 0;
+	hidg->get_req->complete = hidg_get_report_complete;
+	hidg->get_req->context = hidg;
+
 	/* maybe allocate device-global string IDs, and patch descriptors */
 	us = usb_gstrings_attach(c->cdev, ct_func_strings,
 				 ARRAY_SIZE(ct_func_string_defs));
@@ -1004,8 +1109,10 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
 	hidg->write_pending = 1;
 	hidg->req = NULL;
 	spin_lock_init(&hidg->read_spinlock);
+	spin_lock_init(&hidg->get_spinlock);
 	init_waitqueue_head(&hidg->write_queue);
 	init_waitqueue_head(&hidg->read_queue);
+	init_waitqueue_head(&hidg->get_queue);
 	INIT_LIST_HEAD(&hidg->completed_out_req);
 
 	/* create char device */
@@ -1022,6 +1129,8 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
 	if (hidg->req != NULL)
 		free_ep_req(hidg->in_ep, hidg->req);
 
+	usb_ep_free_request(c->cdev->gadget->ep0, hidg->get_req);
+
 	return status;
 }
 
diff --git a/include/uapi/linux/usb/g_hid.h b/include/uapi/linux/usb/g_hid.h
new file mode 100644
index 000000000000..187224dcc0a1
--- /dev/null
+++ b/include/uapi/linux/usb/g_hid.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+
+#ifndef __UAPI_LINUX_USB_G_HID_H
+#define __UAPI_LINUX_USB_G_HID_H
+
+#include <linux/types.h>
+
+#include <linux/usb/gadget-ioctl.h>
+
+/* This value is based on the maximum value of wMaxPacketSize for an endpoint,
+ * which in USB 3.2 is defined as 512 for control packets in section 9.6.6 */
+#define HIDG_REPORT_SIZE_MAX 512
+
+struct usb_hidg_report {
+	__u16 length;
+	__u8 data[HIDG_REPORT_SIZE_MAX];
+};
+
+#endif /* __UAPI_LINUX_USB_G_HID_H */
diff --git a/include/uapi/linux/usb/gadget-ioctl.h b/include/uapi/linux/usb/gadget-ioctl.h
index b5f8f7894db7..81f488643b53 100644
--- a/include/uapi/linux/usb/gadget-ioctl.h
+++ b/include/uapi/linux/usb/gadget-ioctl.h
@@ -25,6 +25,9 @@
  */
 #define	GADGETFS_CLEAR_HALT	_IO('g', 3)
 
+/* g_hid ioctls */
+#define GADGET_HID_WRITE_GET_REPORT	_IOW('g', 0x42, struct usb_hidg_report)
+
 /* g_printer ioctls */
 #define GADGET_GET_PRINTER_STATUS	_IOR('g', 0x21, unsigned char)
 #define GADGET_SET_PRINTER_STATUS	_IOWR('g', 0x22, unsigned char)
-- 
2.43.0

[PATCH v2 3/3] USB: gadget: f_hid: Add Set-Feature report

[Vicki Pfau]

While the HID gadget implementation has been sufficient for devices that only
use INTERRUPT transfers, the USB HID standard includes provisions for Set- and
Get-Feature report CONTROL transfers that go over endpoint 0. These were
previously impossible with the existing implementation, and would either send
an empty reply, or stall out.

As the feature is a standard part of USB HID, it stands to reason that devices
would use it, and that the HID gadget should support it. This patch adds
support for host-to-device Set-Feature reports through a new ioctl
interface to the hidg class dev nodes.

Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/usb/gadget/function/f_hid.c   | 110 ++++++++++++++++++++++++--
 include/uapi/linux/usb/gadget-ioctl.h |   1 +
 2 files changed, 103 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 5a097ea718e8..18e679dc52e3 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -76,6 +76,11 @@ struct f_hidg {
 	wait_queue_head_t		write_queue;
 	struct usb_request		*req;
 
+	/* set report */
+	struct list_head		completed_set_req;
+	spinlock_t			set_spinlock;
+	wait_queue_head_t		set_queue;
+
 	/* get report */
 	struct usb_request		*get_req;
 	struct usb_hidg_report		get_report;
@@ -532,6 +537,54 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
 	return status;
 }
 
+static int f_hidg_set_report(struct file *file, struct usb_hidg_report __user *buffer)
+{
+	struct f_hidg		*hidg = file->private_data;
+	struct f_hidg_req_list	*list;
+	struct usb_request	*req;
+	unsigned long		flags;
+	unsigned short		length;
+	int			status;
+
+	spin_lock_irqsave(&hidg->set_spinlock, flags);
+
+#define SET_REPORT_COND (!list_empty(&hidg->completed_set_req))
+
+	/* wait for at least one buffer to complete */
+	while (!SET_REPORT_COND) {
+		spin_unlock_irqrestore(&hidg->set_spinlock, flags);
+		if (file->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		if (wait_event_interruptible(hidg->set_queue, SET_REPORT_COND))
+			return -ERESTARTSYS;
+
+		spin_lock_irqsave(&hidg->set_spinlock, flags);
+	}
+
+	/* pick the first one */
+	list = list_first_entry(&hidg->completed_set_req,
+				struct f_hidg_req_list, list);
+
+	/*
+	 * Remove this from list to protect it from being free()
+	 * while host disables our function
+	 */
+	list_del(&list->list);
+
+	req = list->req;
+	spin_unlock_irqrestore(&hidg->set_spinlock, flags);
+
+	/* copy to user outside spinlock */
+	length = min_t(unsigned short, sizeof(buffer->data), req->actual);
+	status = copy_to_user(&buffer->length, &length, sizeof(buffer->length));
+	if (!status) {
+	    status = copy_to_user(&buffer->data, req->buf, length);
+	}
+	kfree(list);
+	free_ep_req(hidg->func.config->cdev->gadget->ep0, req);
+	return status;
+}
 
 static int f_hidg_get_report(struct file *file, struct usb_hidg_report __user *buffer)
 {
@@ -583,6 +636,8 @@ static int f_hidg_get_report(struct file *file, struct usb_hidg_report __user *b
 static long f_hidg_ioctl(struct file *file, unsigned int code, unsigned long arg)
 {
 	switch (code) {
+	case GADGET_HID_READ_SET_REPORT:
+		return f_hidg_set_report(file, (struct usb_hidg_report __user *)arg);
 	case GADGET_HID_WRITE_GET_REPORT:
 		return f_hidg_get_report(file, (struct usb_hidg_report __user *)arg);
 	default:
@@ -597,6 +652,7 @@ static __poll_t f_hidg_poll(struct file *file, poll_table *wait)
 
 	poll_wait(file, &hidg->read_queue, wait);
 	poll_wait(file, &hidg->write_queue, wait);
+	poll_wait(file, &hidg->set_queue, wait);
 
 	if (WRITE_COND)
 		ret |= EPOLLOUT | EPOLLWRNORM;
@@ -609,12 +665,16 @@ static __poll_t f_hidg_poll(struct file *file, poll_table *wait)
 			ret |= EPOLLIN | EPOLLRDNORM;
 	}
 
+	if (SET_REPORT_COND)
+		ret |= EPOLLPRI;
+
 	return ret;
 }
 
 #undef WRITE_COND
 #undef READ_COND_SSREPORT
 #undef READ_COND_INTOUT
+#undef SET_REPORT_COND
 #undef GET_REPORT_COND
 
 static int f_hidg_release(struct inode *inode, struct file *fd)
@@ -659,11 +719,19 @@ static void hidg_intout_complete(struct usb_ep *ep, struct usb_request *req)
 
 		req_list->req = req;
 
-		spin_lock_irqsave(&hidg->read_spinlock, flags);
-		list_add_tail(&req_list->list, &hidg->completed_out_req);
-		spin_unlock_irqrestore(&hidg->read_spinlock, flags);
+		if (ep == cdev->gadget->ep0) {
+			spin_lock_irqsave(&hidg->set_spinlock, flags);
+			list_add_tail(&req_list->list, &hidg->completed_set_req);
+			spin_unlock_irqrestore(&hidg->set_spinlock, flags);
 
-		wake_up(&hidg->read_queue);
+			wake_up(&hidg->set_queue);
+		} else {
+			spin_lock_irqsave(&hidg->read_spinlock, flags);
+			list_add_tail(&req_list->list, &hidg->completed_out_req);
+			spin_unlock_irqrestore(&hidg->read_spinlock, flags);
+
+			wake_up(&hidg->read_queue);
+		}
 		break;
 	default:
 		ERROR(cdev, "Set report failed %d\n", req->status);
@@ -776,12 +844,27 @@ 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);
-		if (hidg->use_out_ep)
+		if (!hidg->use_out_ep) {
+			req->complete = hidg_ssreport_complete;
+			req->context  = hidg;
+			goto respond;
+		}
+		if (!length)
 			goto stall;
-		req->complete = hidg_ssreport_complete;
+		req = alloc_ep_req(cdev->gadget->ep0, GFP_ATOMIC);
+		if (!req)
+			return -ENOMEM;
+		req->complete = hidg_intout_complete;
 		req->context  = hidg;
-		goto respond;
-		break;
+		req->zero = 0;
+		req->length = length;
+		status = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
+		if (status < 0) {
+			ERROR(cdev, "usb_ep_queue error on set_report %d\n", status);
+			free_ep_req(cdev->gadget->ep0, req);
+		}
+
+		return status;
 
 	case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8
 		  | HID_REQ_SET_PROTOCOL):
@@ -881,6 +964,14 @@ static void hidg_disable(struct usb_function *f)
 		spin_unlock_irqrestore(&hidg->read_spinlock, flags);
 	}
 
+	spin_lock_irqsave(&hidg->set_spinlock, flags);
+	list_for_each_entry_safe(list, next, &hidg->completed_set_req, list) {
+		free_ep_req(f->config->cdev->gadget->ep0, list->req);
+		list_del(&list->list);
+		kfree(list);
+	}
+	spin_unlock_irqrestore(&hidg->set_spinlock, flags);
+
 	spin_lock_irqsave(&hidg->write_spinlock, flags);
 	if (!hidg->write_pending) {
 		free_ep_req(hidg->in_ep, hidg->req);
@@ -1109,11 +1200,14 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
 	hidg->write_pending = 1;
 	hidg->req = NULL;
 	spin_lock_init(&hidg->read_spinlock);
+	spin_lock_init(&hidg->set_spinlock);
 	spin_lock_init(&hidg->get_spinlock);
 	init_waitqueue_head(&hidg->write_queue);
 	init_waitqueue_head(&hidg->read_queue);
+	init_waitqueue_head(&hidg->set_queue);
 	init_waitqueue_head(&hidg->get_queue);
 	INIT_LIST_HEAD(&hidg->completed_out_req);
+	INIT_LIST_HEAD(&hidg->completed_set_req);
 
 	/* create char device */
 	cdev_init(&hidg->cdev, &f_hidg_fops);
diff --git a/include/uapi/linux/usb/gadget-ioctl.h b/include/uapi/linux/usb/gadget-ioctl.h
index 81f488643b53..f447699148e4 100644
--- a/include/uapi/linux/usb/gadget-ioctl.h
+++ b/include/uapi/linux/usb/gadget-ioctl.h
@@ -26,6 +26,7 @@
 #define	GADGETFS_CLEAR_HALT	_IO('g', 3)
 
 /* g_hid ioctls */
+#define GADGET_HID_READ_SET_REPORT	_IOR('g', 0x41, struct usb_hidg_report)
 #define GADGET_HID_WRITE_GET_REPORT	_IOW('g', 0x42, struct usb_hidg_report)
 
 /* g_printer ioctls */
-- 
2.43.0

Re: [PATCH v2 1/3] USB: gadget: Move gadget-related ioctl codes to gadget-ioctl.h

[Greg Kroah-Hartman]

On Wed, Jan 17, 2024 at 04:37:55PM -0800, Vicki Pfau wrote:
> Since multiple different gadget types use similar ranges of ioctl IDs,
> put all of them in the same file to avoid accidentally creating
> overlaps.
> 
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Vicki Pfau <vi@endrift.com>
> ---
>  include/uapi/linux/usb/g_printer.h    | 23 ++----------------
>  include/uapi/linux/usb/g_uvc.h        |  4 +--
>  include/uapi/linux/usb/gadget-ioctl.h | 35 +++++++++++++++++++++++++++
>  include/uapi/linux/usb/gadgetfs.h     | 27 +--------------------
>  4 files changed, 39 insertions(+), 50 deletions(-)
>  create mode 100644 include/uapi/linux/usb/gadget-ioctl.h
> 
> diff --git a/include/uapi/linux/usb/g_printer.h b/include/uapi/linux/usb/g_printer.h
> index 7fc20e4b82f5..fc411ee3f5cc 100644
> --- a/include/uapi/linux/usb/g_printer.h
> +++ b/include/uapi/linux/usb/g_printer.h
> @@ -3,34 +3,15 @@
>   * g_printer.h -- Header file for USB Printer gadget driver
>   *
>   * Copyright (C) 2007 Craig W. Nadler
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */

Note, this "boiler-plate" text should be removed, you are right, but not
in this commit, as it has nothing to do with it, and should be a
stand-alone change.

So can you please split this out?

Otherwise, nice work,

greg k-h