All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] usb: gadget: User space URBs for FunctionFS
@ 2020-11-11 17:07 Ingo Rohloff
  2020-11-11 17:07 ` [PATCH 1/2] usb: gadget: ffs: Implement user URBs for USB bulk endpoints Ingo Rohloff
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Ingo Rohloff @ 2020-11-11 17:07 UTC (permalink / raw)
  To: balbi; +Cc: gregkh, linux-usb, Ingo Rohloff

I am working on a platform (Xilinx Zynq Ultrascale+), which
is supposed to work as a pure USB Device (not dual-role).

To get fast USB bulk transfers I wanted to have something similar
like USBDEVFS_SUBMITURB/USBDEVFS_REAPURB, but for an USB Device.

I now implement two new ioctls for FunctionFS:
  FUNCTIONFS_SUBMITBULKURB
  FUNCTIONFS_REAPBULKURB
which provide simliar functionality.

A similar functionality is already implemented via AIO. But: To use this
API, your user space environment needs to know how to use these system
calls.
Additionally the semantics of the ioctls is slightly different:
Usually you can only access a FunctionFS file if the FunctionFS is
already bound to an UDC (USB Device Controller) and the USB Device is
connected to a USB Host (which then enables the appropriate configuration
and USB endpoints).
These new ioctls behave different: You already can submit URBs before the
Function is bound to an UDC and before the USB Device is connected.
These "pending" URBs will be activated once the endpoints become active.

When the endpoints become deactivated (either by a disconnect from the
USB Host or by unbinding the UDC), active URBs are cancelled.

A user space program will then get a notification, that the URBs have
been cancelled and the status will indicate to the user space program,
that the connection was lost.
Via this mechanism a user space program can keep precise track, which
URBs succeeded and which URBs failed.

The final goal here is to be able to directly let user space provide data
buffers (via mmap I guess), which are then transferred via USB; but this
is the next step.

I implemented test code to demonstrate how to use these new ioctls.

Ingo Rohloff (2):
  usb: gadget: ffs: Implement user URBs for USB bulk endpoints
  usb: gadget: ffs: tools: test applications for user URBs.

 drivers/usb/gadget/function/f_fs.c            | 478 ++++++++++++++++++
 include/uapi/linux/usb/functionfs.h           |  14 +
 .../device_app/usb_func_echo.c                | 474 +++++++++++++++++
 .../ffs-urb-example/device_app/usb_gadget_mk  |  79 +++
 .../ffs-urb-example/host_app/usb_test_echo.c  | 370 ++++++++++++++
 5 files changed, 1415 insertions(+)
 create mode 100644 tools/usb/ffs-urb-example/device_app/usb_func_echo.c
 create mode 100644 tools/usb/ffs-urb-example/device_app/usb_gadget_mk
 create mode 100644 tools/usb/ffs-urb-example/host_app/usb_test_echo.c

-- 
2.17.1


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

* [PATCH 1/2] usb: gadget: ffs: Implement user URBs for USB bulk endpoints
  2020-11-11 17:07 [PATCH 0/2] usb: gadget: User space URBs for FunctionFS Ingo Rohloff
@ 2020-11-11 17:07 ` Ingo Rohloff
  2020-11-15 10:59     ` kernel test robot
  2020-11-15 10:59     ` kernel test robot
  2020-11-11 17:07 ` [PATCH 2/2] usb: gadget: ffs: tools: test applications for user URBs Ingo Rohloff
  2020-11-11 18:40 ` [PATCH 0/2] usb: gadget: User space URBs for FunctionFS Greg KH
  2 siblings, 2 replies; 11+ messages in thread
From: Ingo Rohloff @ 2020-11-11 17:07 UTC (permalink / raw)
  To: balbi; +Cc: gregkh, linux-usb, Ingo Rohloff

URB aka USB Request Block: This patch offers similar functionality as
USBDEVFS_SUBMITURB / USBDEVFS_REAPURB, for USB FunctionFS.  This is
intended to be used by user space programs, which want to implement a
user space gadget driver.

Only bulk endpoints are currently supported.

Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com>
---
 drivers/usb/gadget/function/f_fs.c  | 478 ++++++++++++++++++++++++++++
 include/uapi/linux/usb/functionfs.h |  14 +
 2 files changed, 492 insertions(+)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 046f770a76da..194d45d94031 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -135,6 +135,26 @@ struct ffs_epfile {
 
 	struct dentry			*dentry;
 
+	/* wake up when an async_bulkurb completed */
+	wait_queue_head_t async_waitq;
+
+	/* protects the async URB lists */
+	spinlock_t async_lock;
+
+	/* P: async_lock */
+	/*
+	 *  URB lists:
+	 *  async_pending
+	 *    Not yet passed to UDC driver
+	 *  async_active
+	 *    Passed to UDC driver already, but not yet completed
+	 *  async_completed
+	 *    Completed URBs
+	 */
+	struct list_head  async_pending;
+	struct list_head  async_active;
+	struct list_head  async_completed;
+
 	/*
 	 * Buffer for holding data from partial reads which may happen since
 	 * we’re rounding user read requests to a multiple of a max packet size.
@@ -207,6 +227,35 @@ struct ffs_buffer {
 	char storage[];
 };
 
+/*
+ * async_bulkurb: handling of
+ *    FUNCTIONFS_SUBMITBULKURB
+ *    FUNCTIONFS_REAPBULKURB
+ */
+enum {
+	FFS_BULKURB_STATE_COMPLETE = 0,
+	FFS_BULKURB_STATE_PENDING  = 1,
+	FFS_BULKURB_STATE_ACTIVE   = 2
+};
+DEFINE_SPINLOCK(async_bulkurb_active_lock);
+struct async_bulkurb {
+	struct ffs_epfile *epfile;
+	struct list_head urblist;
+
+	int status;
+	int orig_length;
+	int buffer_length;
+	int actual_length;
+	int num_sgs;
+	struct scatterlist *sg;
+
+	struct usb_ep      *usbep;
+	struct usb_request *usbreq;
+
+	void __user *userurb;
+	void __user *userdata_outep;
+};
+
 /*  ffs_io_data structure ***************************************************/
 
 struct ffs_io_data {
@@ -1262,6 +1311,380 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, struct iov_iter *to)
 	return res;
 }
 
+static void ffs_async_bulkurb_free(struct async_bulkurb *as)
+{
+	int i;
+
+	if (as->usbreq)
+		usb_ep_free_request(as->usbep, as->usbreq);
+	for (i = 0; i < as->num_sgs; i++) {
+		if (sg_page(&as->sg[i]))
+			kfree(sg_virt(&as->sg[i]));
+	}
+	kfree(as->sg);
+	kfree(as);
+}
+
+static void ffs_async_bulkurb_completion(
+	struct usb_ep *_ep, struct usb_request *req
+)
+{
+	struct async_bulkurb *as = req->context;
+	struct ffs_epfile *epfile;
+	unsigned long flags;
+
+	// avoid race condition with ffs_epfile_async_release()
+	spin_lock_irqsave(&async_bulkurb_active_lock, flags);
+	epfile = as->epfile;
+	if (!epfile) {
+		// as->epfile was released: just free URB to drop it
+		ffs_async_bulkurb_free(as);
+		spin_unlock_irqrestore(&async_bulkurb_active_lock, flags);
+		return;
+	}
+	spin_lock(&epfile->async_lock);
+	spin_unlock(&async_bulkurb_active_lock);
+	if (req->status < 0) {
+		as->status = req->status; // error code
+		as->actual_length = 0;
+	} else {
+		as->status = FFS_BULKURB_STATE_COMPLETE;
+		as->actual_length = req->actual;
+	}
+	list_move_tail(&as->urblist, &epfile->async_completed);
+	spin_unlock_irqrestore(&epfile->async_lock, flags);
+	wake_up(&epfile->async_waitq);
+}
+
+static struct async_bulkurb *ffs_async_bulkurb_getcompleted(
+	struct ffs_epfile *epfile
+)
+{
+	struct async_bulkurb *as = NULL;
+
+	spin_lock_irq(&epfile->async_lock);
+	if (!list_empty(&epfile->async_completed)) {
+		as = list_first_entry(
+			&epfile->async_completed,
+			struct async_bulkurb,
+			urblist
+		);
+		list_del_init(&as->urblist);
+	}
+	spin_unlock_irq(&epfile->async_lock);
+	return as;
+}
+
+static struct async_bulkurb *ffs_async_bulkurb_reap(
+	struct ffs_epfile *epfile
+)
+{
+	DECLARE_WAITQUEUE(wait, current);
+	struct async_bulkurb *as = NULL;
+
+	add_wait_queue(&epfile->async_waitq, &wait);
+	for (;;) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		as = ffs_async_bulkurb_getcompleted(epfile);
+		if (as)
+			break;
+		if (signal_pending(current))
+			break;
+		schedule();
+	}
+	set_current_state(TASK_RUNNING);
+	remove_wait_queue(&epfile->async_waitq, &wait);
+
+	return as;
+}
+
+static long ffs_async_bulburb_copy_outep(
+	void __user *userdata, int data_len,
+	struct scatterlist *sg, int num_sgs
+)
+{
+	int i, sz;
+	long err;
+
+	for (i = 0; i < num_sgs && data_len > 0; i++) {
+		sz = data_len;
+		if (sz > PAGE_SIZE)
+			sz = PAGE_SIZE;
+		err = copy_to_user(userdata, sg_virt(sg), sz);
+		if (err)
+			return err;
+		userdata += sz;
+		data_len -= sz;
+		sg++;
+	}
+	return 0;
+}
+
+static long ffs_epfile_reapbulkurb(
+	struct file *file, void __user * __user *p
+)
+{
+	struct ffs_epfile *epfile = (struct ffs_epfile *)file->private_data;
+	struct async_bulkurb *as;
+	struct usb_functionfs_bulkurb __user *uurb;
+	long err;
+
+	as = ffs_async_bulkurb_reap(epfile);
+	if (!as)
+		return -EINTR;
+	err = 0;
+	uurb = as->userurb;
+	if (put_user(as->status, &uurb->status)) {
+		err = -EFAULT;
+		goto funcend;
+	}
+	if (as->userdata_outep && as->actual_length > 0) {
+		void __user *userdata;
+		int data_len;
+
+		userdata = as->userdata_outep;
+		data_len = as->actual_length;
+		if (data_len > as->orig_length)
+			data_len = as->orig_length;
+		err = ffs_async_bulburb_copy_outep(
+			userdata, data_len,
+			as->sg, as->num_sgs
+		);
+		if (err) {
+			err = -EFAULT;
+			goto funcend;
+		}
+	}
+	if (put_user(as->actual_length, &uurb->actual_length)) {
+		err = -EFAULT;
+		goto funcend;
+	}
+	if (put_user(as->userurb, p)) {
+		err = -EFAULT;
+		goto funcend;
+	}
+
+funcend:
+	ffs_async_bulkurb_free(as);
+	return err;
+}
+
+static int ffs_async_bulkurb_alloc_sg(
+	struct scatterlist *sg, int num_sgs,
+	void __user *userdata, int data_len
+)
+{
+	int i, sz;
+	void *buf;
+
+	// allocate num_sgs*PAGE_SIZE bytes for data
+	// but make sure only data_len bytes are used
+	sg_init_table(sg, num_sgs);
+	for (i = 0; i < num_sgs; i++) {
+		buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+		sz = data_len;
+		if (sz > PAGE_SIZE)
+			sz = PAGE_SIZE;
+		sg_set_buf(&sg[i], buf, sz);
+		if (userdata && data_len > 0) {
+			// transfer device -> host:
+			// copy user data to kernel memory
+			if (copy_from_user(buf, userdata, sz))
+				return -EFAULT;
+			userdata += sz;
+		}
+		data_len -= sz;
+	}
+	return 0;
+}
+
+static long ffs_async_bulkurb_prepare(
+	struct async_bulkurb *as,
+	struct ffs_epfile *epfile,
+	void __user *userurb
+)
+{
+	int num_sgs;
+	int data_len;
+	int err;
+	struct usb_functionfs_bulkurb urb;
+
+	if (copy_from_user(&urb, userurb, sizeof(urb)))
+		return -EFAULT;
+	data_len = urb.buffer_length;
+	if (data_len <= 0)
+		return -EINVAL;
+	as->orig_length = data_len;
+	num_sgs = DIV_ROUND_UP(data_len, PAGE_SIZE);
+	as->epfile = epfile;
+	as->sg = kmalloc_array(
+		num_sgs, sizeof(struct scatterlist), GFP_KERNEL
+	);
+	if (!as->sg)
+		return -ENOMEM;
+	as->num_sgs = num_sgs;
+	as->buffer_length = num_sgs*PAGE_SIZE;
+	err = ffs_async_bulkurb_alloc_sg(
+		as->sg, num_sgs,
+		(epfile->in) ? urb.buffer : NULL, data_len
+	);
+	if (err)
+		return err;
+	as->userurb = userurb;
+	if (!epfile->in)
+		as->userdata_outep = urb.buffer;
+	as->status = FFS_BULKURB_STATE_PENDING;
+	return 0;
+}
+
+// you must hold as->epfile->ffs->epslock,
+// when calling ffs_async_bulkurb_activate.
+// IRQs should be disabled.
+static void ffs_async_bulkurb_activate(struct async_bulkurb *as)
+{
+	struct ffs_epfile *epfile = as->epfile;
+	struct ffs_ep *ep = epfile->ep;
+	struct usb_gadget *gadget = epfile->ffs->gadget;
+	struct usb_request *req;
+	ssize_t data_len;
+	int err;
+
+	if (!ep) {
+		as->status = -ENODEV;
+		goto error;
+	}
+	if (!gadget->sg_supported || epfile->isoc) {
+		as->status = -EPERM;
+		goto error;
+	}
+	data_len = as->orig_length;
+	if (!epfile->in) {
+		/* transfer host -> device */
+		data_len = usb_ep_align_maybe(gadget, ep->ep, data_len);
+		if (data_len > as->buffer_length) {
+			as->status = -ENOMEM;
+			goto error;
+		}
+	}
+	req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC);
+	if (!req) {
+		as->status = -ENOMEM;
+		goto error;
+	}
+	req->context = as;
+	req->length = data_len;
+	req->buf = NULL;
+	req->num_sgs = as->num_sgs;
+	req->sg = as->sg;
+	req->complete = ffs_async_bulkurb_completion;
+	err = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
+	if (unlikely(err)) {
+		as->status = err;
+		goto error;
+	}
+	as->status = FFS_BULKURB_STATE_ACTIVE;
+	as->usbep = ep->ep;
+	as->usbreq = req;
+	spin_lock(&epfile->async_lock);
+	list_add_tail(&as->urblist, &epfile->async_active);
+	spin_unlock(&epfile->async_lock);
+	return;
+
+error:
+	spin_lock(&epfile->async_lock);
+	list_add_tail(&as->urblist, &epfile->async_completed);
+	spin_unlock(&epfile->async_lock);
+}
+
+
+static long ffs_epfile_submitbulkurb(struct file *file, void __user *p)
+{
+	struct ffs_epfile *epfile = file->private_data;
+	struct async_bulkurb *as;
+	long err;
+
+	as = kzalloc(sizeof(struct async_bulkurb), GFP_KERNEL);
+	if (!as)
+		return -ENOMEM;
+	err = ffs_async_bulkurb_prepare(as, epfile, p);
+	if (err) {
+		ffs_async_bulkurb_free(as);
+		return err;
+	}
+	spin_lock_irq(&epfile->ffs->eps_lock);
+	if (epfile->ep)
+		ffs_async_bulkurb_activate(as);
+	else {
+		spin_lock(&epfile->async_lock);
+		list_add_tail(&as->urblist, &epfile->async_pending);
+		spin_unlock(&epfile->async_lock);
+	}
+	spin_unlock_irq(&epfile->ffs->eps_lock);
+	return 0;
+}
+
+static __poll_t ffs_epfile_poll(
+	struct file *file, struct poll_table_struct *wait
+)
+{
+	struct ffs_epfile *epfile = file->private_data;
+	__poll_t mask = 0;
+
+	poll_wait(file, &epfile->async_waitq, wait);
+	spin_lock_irq(&epfile->async_lock);
+	if (
+		(file->f_mode & FMODE_WRITE) &&
+		!list_empty(&epfile->async_completed)
+	)
+		mask |= EPOLLOUT | EPOLLWRNORM;
+	spin_unlock_irq(&epfile->async_lock);
+	return mask;
+}
+
+static void ffs_epfile_async_release(struct ffs_epfile *epfile)
+{
+	struct async_bulkurb *as;
+	unsigned long flags;
+
+	// avoid race condition with ffs_async_bulkurb_completion()
+	// we will modify as->epfile of active URBs
+	spin_lock_irqsave(&async_bulkurb_active_lock, flags);
+	spin_lock(&epfile->async_lock);
+	while (!list_empty(&epfile->async_active)) {
+		as = list_first_entry(
+			&epfile->async_active,
+			struct async_bulkurb,
+			urblist
+		);
+		list_del_init(&as->urblist);
+		as->epfile = NULL;
+		// "as" will be freed in completion handler
+	}
+	// done with all active URBs: release lock for them
+	spin_unlock(&async_bulkurb_active_lock);
+	while (!list_empty(&epfile->async_pending)) {
+		as = list_first_entry(
+			&epfile->async_pending,
+			struct async_bulkurb,
+			urblist
+		);
+		list_del_init(&as->urblist);
+		ffs_async_bulkurb_free(as);
+	}
+	while (!list_empty(&epfile->async_completed)) {
+		as = list_first_entry(
+			&epfile->async_completed,
+			struct async_bulkurb,
+			urblist
+		);
+		list_del_init(&as->urblist);
+		ffs_async_bulkurb_free(as);
+	}
+	spin_unlock_irqrestore(&epfile->async_lock, flags);
+}
+
 static int
 ffs_epfile_release(struct inode *inode, struct file *file)
 {
@@ -1269,6 +1692,7 @@ ffs_epfile_release(struct inode *inode, struct file *file)
 
 	ENTER();
 
+	ffs_epfile_async_release(epfile);
 	__ffs_epfile_read_buffer_free(epfile);
 	ffs_data_closed(epfile->ffs);
 
@@ -1287,6 +1711,22 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
 	if (WARN_ON(epfile->ffs->state != FFS_ACTIVE))
 		return -ENODEV;
 
+	// The following ioctls should work even without an enabled endpoint
+	switch (code) {
+	case FUNCTIONFS_SUBMITBULKURB:
+	{
+		ret = ffs_epfile_submitbulkurb(file, (void __user *)value);
+		return ret;
+	}
+	case FUNCTIONFS_REAPBULKURB:
+	{
+		ret = ffs_epfile_reapbulkurb(
+			file, (void __user * __user *)value
+		);
+		return ret;
+	}
+	}
+
 	/* Wait for endpoint to be enabled */
 	ep = epfile->ep;
 	if (!ep) {
@@ -1359,6 +1799,7 @@ static const struct file_operations ffs_epfile_operations = {
 	.write_iter =	ffs_epfile_write_iter,
 	.read_iter =	ffs_epfile_read_iter,
 	.release =	ffs_epfile_release,
+	.poll =  ffs_epfile_poll,
 	.unlocked_ioctl =	ffs_epfile_ioctl,
 	.compat_ioctl = compat_ptr_ioctl,
 };
@@ -1881,6 +2322,13 @@ static int ffs_epfiles_create(struct ffs_data *ffs)
 	for (i = 1; i <= count; ++i, ++epfile) {
 		epfile->ffs = ffs;
 		mutex_init(&epfile->mutex);
+
+		spin_lock_init(&epfile->async_lock);
+		init_waitqueue_head(&epfile->async_waitq);
+		INIT_LIST_HEAD(&epfile->async_pending);
+		INIT_LIST_HEAD(&epfile->async_active);
+		INIT_LIST_HEAD(&epfile->async_completed);
+
 		if (ffs->user_flags & FUNCTIONFS_VIRTUAL_ADDR)
 			sprintf(epfile->name, "ep%02x", ffs->eps_addrmap[i]);
 		else
@@ -1906,6 +2354,7 @@ static void ffs_epfiles_destroy(struct ffs_epfile *epfiles, unsigned count)
 
 	for (; count; --count, ++epfile) {
 		BUG_ON(mutex_is_locked(&epfile->mutex));
+		ffs_epfile_async_release(epfile);
 		if (epfile->dentry) {
 			d_delete(epfile->dentry);
 			dput(epfile->dentry);
@@ -1972,6 +2421,35 @@ static int ffs_func_eps_enable(struct ffs_function *func)
 		++epfile;
 	}
 
+	/*
+	 * loop through all epfiles and convert
+	 * pending async requests into active async requests
+	 */
+	epfile = ffs->epfiles;
+	count  = ffs->eps_count;
+	while (count--) {
+		if (!epfile->ep) {
+			++epfile;
+			continue;
+		}
+		spin_lock(&epfile->async_lock);
+		while (!list_empty(&epfile->async_pending)) {
+			struct async_bulkurb *as;
+
+			as = list_first_entry(
+				&epfile->async_pending,
+				struct async_bulkurb,
+				urblist
+			);
+			list_del_init(&as->urblist);
+			spin_unlock(&epfile->async_lock);
+			ffs_async_bulkurb_activate(as);
+			spin_lock(&epfile->async_lock);
+		}
+		spin_unlock(&epfile->async_lock);
+		++epfile;
+	}
+
 	wake_up_interruptible(&ffs->wait);
 	spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
 
diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
index d77ee6b65328..09e19cdd1604 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -288,6 +288,20 @@ struct usb_functionfs_event {
 #define	FUNCTIONFS_ENDPOINT_DESC	_IOR('g', 130, \
 					     struct usb_endpoint_descriptor)
 
+/*
+ * Submit an USB Request Block for a bulk endpoint
+ */
+struct usb_functionfs_bulkurb {
+	int status;
+	int buffer_length;
+	int actual_length;
+	void __user *buffer;
+};
+
+#define FUNCTIONFS_SUBMITBULKURB  \
+	_IOW('g', 131, struct usb_functionfs_bulkurb)
 
+#define FUNCTIONFS_REAPBULKURB  \
+	_IOW('g', 132, void *)
 
 #endif /* _UAPI__LINUX_FUNCTIONFS_H__ */
-- 
2.17.1


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

* [PATCH 2/2] usb: gadget: ffs: tools: test applications for user URBs.
  2020-11-11 17:07 [PATCH 0/2] usb: gadget: User space URBs for FunctionFS Ingo Rohloff
  2020-11-11 17:07 ` [PATCH 1/2] usb: gadget: ffs: Implement user URBs for USB bulk endpoints Ingo Rohloff
@ 2020-11-11 17:07 ` Ingo Rohloff
  2020-11-11 18:40 ` [PATCH 0/2] usb: gadget: User space URBs for FunctionFS Greg KH
  2 siblings, 0 replies; 11+ messages in thread
From: Ingo Rohloff @ 2020-11-11 17:07 UTC (permalink / raw)
  To: balbi; +Cc: gregkh, linux-usb, Ingo Rohloff

An echo test applications for USB Linux Host and USB Linux Device, to
test ioctls FUNCTIONFS_SUBMITBULKURB and FUNCTIONFS_REAPBULKURB.

Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com>
---
 .../device_app/usb_func_echo.c                | 474 ++++++++++++++++++
 .../ffs-urb-example/device_app/usb_gadget_mk  |  79 +++
 .../ffs-urb-example/host_app/usb_test_echo.c  | 370 ++++++++++++++
 3 files changed, 923 insertions(+)
 create mode 100644 tools/usb/ffs-urb-example/device_app/usb_func_echo.c
 create mode 100644 tools/usb/ffs-urb-example/device_app/usb_gadget_mk
 create mode 100644 tools/usb/ffs-urb-example/host_app/usb_test_echo.c

diff --git a/tools/usb/ffs-urb-example/device_app/usb_func_echo.c b/tools/usb/ffs-urb-example/device_app/usb_func_echo.c
new file mode 100644
index 000000000000..a8197b840c05
--- /dev/null
+++ b/tools/usb/ffs-urb-example/device_app/usb_func_echo.c
@@ -0,0 +1,474 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * This sample code implements a USB function with 2 bulk endpoints,
+ * employing two IOCTLs:
+ *     FUNCTIONFS_SUBMITBULKURB
+ *     FUNCTIONFS_REAPBULKURB
+ *
+ * The two bulk endpoints are used for:
+ *  - IN  EP  transfer  device -> host  this code: write direction
+ *  - OUT EP  transfer  host -> device  this code: read direction
+ *
+ * The code waits to receive up to 0x3FFC bytes on the OUT EP.
+ * It then modifies the data and sends it back (see modify_data()).
+ * The idea here is, that the host always uses USB transaction sizes,
+ * which are NOT a multiple of MaxPacketSize.
+ *
+ * This means the end of each USB transaction is marked by a short
+ * packet.
+ *
+ * TL;DR   This implements a simple echo test function on a USB device
+ */
+
+#define _BSD_SOURCE /* for endian.h */
+
+#include <endian.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <poll.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <linux/usb/functionfs.h>
+
+#define DBG_PRINTF(...) do { \
+	printf("%s : ", __func__); \
+	printf(__VA_ARGS__); \
+	fflush(stdout); \
+} while ((0))
+
+struct tst_descriptors {
+	struct usb_functionfs_descs_head_v2 header;
+	__le32 fs_count;
+	__le32 hs_count;
+	__le32 ss_count;
+	struct {
+		struct usb_interface_descriptor intf;
+		struct usb_endpoint_descriptor_no_audio sink;
+		struct usb_endpoint_descriptor_no_audio source;
+	} __attribute__((packed)) fs_descs, hs_descs;
+	struct {
+		struct usb_interface_descriptor intf;
+		struct usb_endpoint_descriptor_no_audio sink;
+		struct usb_ss_ep_comp_descriptor sink_comp;
+		struct usb_endpoint_descriptor_no_audio source;
+		struct usb_ss_ep_comp_descriptor source_comp;
+	} ss_descs;
+} __attribute__((packed));
+
+static struct tst_descriptors gSetupFFS = {
+	.header = {
+		.magic = htole32(FUNCTIONFS_DESCRIPTORS_MAGIC_V2),
+		.flags = htole32(
+			FUNCTIONFS_HAS_FS_DESC |
+			FUNCTIONFS_HAS_HS_DESC |
+			FUNCTIONFS_HAS_SS_DESC
+		),
+		.length = htole32(sizeof(gSetupFFS))
+	},
+	.fs_count = htole32(3),
+	.fs_descs = {
+		.intf = {
+			.bLength = sizeof(gSetupFFS.fs_descs.intf),
+			.bDescriptorType = USB_DT_INTERFACE,
+			.bNumEndpoints = 2,
+			.bInterfaceClass = USB_CLASS_VENDOR_SPEC,
+			.iInterface = 1,
+		},
+		.sink = {
+			.bLength = sizeof(gSetupFFS.fs_descs.sink),
+			.bDescriptorType = USB_DT_ENDPOINT,
+			.bEndpointAddress = 1 | USB_DIR_IN,
+			.bmAttributes = USB_ENDPOINT_XFER_BULK,
+			/* .wMaxPacketSize = autoconfiguration (kernel) */
+		},
+		.source = {
+			.bLength = sizeof(gSetupFFS.fs_descs.source),
+			.bDescriptorType = USB_DT_ENDPOINT,
+			.bEndpointAddress = 2 | USB_DIR_OUT,
+			.bmAttributes = USB_ENDPOINT_XFER_BULK,
+			/* .wMaxPacketSize = autoconfiguration (kernel) */
+		},
+	},
+	.hs_count = htole32(3),
+	.hs_descs = {
+		.intf = {
+			.bLength = sizeof(gSetupFFS.hs_descs.intf),
+			.bDescriptorType = USB_DT_INTERFACE,
+			.bNumEndpoints = 2,
+			.bInterfaceClass = USB_CLASS_VENDOR_SPEC,
+			.iInterface = 1,
+		},
+		.sink = {
+			.bLength = sizeof(gSetupFFS.hs_descs.sink),
+			.bDescriptorType = USB_DT_ENDPOINT,
+			.bEndpointAddress = 1 | USB_DIR_IN,
+			.bmAttributes = USB_ENDPOINT_XFER_BULK,
+			.wMaxPacketSize = htole16(512),
+		},
+		.source = {
+			.bLength = sizeof(gSetupFFS.hs_descs.source),
+			.bDescriptorType = USB_DT_ENDPOINT,
+			.bEndpointAddress = 2 | USB_DIR_OUT,
+			.bmAttributes = USB_ENDPOINT_XFER_BULK,
+			.wMaxPacketSize = htole16(512),
+			.bInterval = 1, /* NAK every 1 uframe */
+		},
+	},
+	.ss_count = htole32(5),
+	.ss_descs = {
+		.intf = {
+			.bLength = sizeof(gSetupFFS.ss_descs.intf),
+			.bDescriptorType = USB_DT_INTERFACE,
+			.bNumEndpoints = 2,
+			.bInterfaceClass = USB_CLASS_VENDOR_SPEC,
+			.iInterface = 1,
+		},
+		.sink = {
+			.bLength = sizeof(gSetupFFS.ss_descs.sink),
+			.bDescriptorType = USB_DT_ENDPOINT,
+			.bEndpointAddress = 1 | USB_DIR_IN,
+			.bmAttributes = USB_ENDPOINT_XFER_BULK,
+			.wMaxPacketSize = htole16(1024),
+		},
+		.sink_comp = {
+			.bLength = USB_DT_SS_EP_COMP_SIZE,
+			.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
+			.bMaxBurst = 0,
+			.bmAttributes = 0,
+			.wBytesPerInterval = 0,
+		},
+		.source = {
+			.bLength = sizeof(gSetupFFS.ss_descs.source),
+			.bDescriptorType = USB_DT_ENDPOINT,
+			.bEndpointAddress = 2 | USB_DIR_OUT,
+			.bmAttributes = USB_ENDPOINT_XFER_BULK,
+			.wMaxPacketSize = htole16(1024),
+			.bInterval = 1, /* NAK every 1 uframe */
+		},
+		.source_comp = {
+			.bLength = USB_DT_SS_EP_COMP_SIZE,
+			.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
+			.bMaxBurst = 0,
+			.bmAttributes = 0,
+			.wBytesPerInterval = 0,
+		},
+	},
+};
+
+#define STR_INTERFACE_ "ECHO GADGET"
+struct tst_usb_strings {
+	struct usb_functionfs_strings_head header;
+	struct {
+		__le16 code;
+		const char str1[sizeof(STR_INTERFACE_)];
+	} __attribute__((packed)) lang0;
+} __attribute__((packed));
+
+static struct tst_usb_strings gStrings = {
+	.header = {
+		.magic = FUNCTIONFS_STRINGS_MAGIC,
+		.length = sizeof(struct tst_usb_strings),
+		.str_count = 1,
+		.lang_count = 1,
+	},
+	.lang0 = {
+		0x0409, /* en-us */
+		STR_INTERFACE_,
+	},
+};
+#define STR_INTERFACE gStrings.lang0.str1
+
+// define number of URBs used in total
+// MUST be a power of 2
+#define URB_NR 8
+static struct {
+	int ep0fd;
+
+	int ep_in_fd;     // WRITE: device -> host     ( IN EP)
+	int ep_out_fd;    // READ:  host   -> device   (OUT EP)
+
+	uint8_t dataBufSpace[0x4000*URB_NR + 0x2000];
+	uint8_t *dataBuf;
+
+	int rdUrbWrIdx;
+	int rdUrbRdIdx;
+	int rdPending;
+	struct usb_functionfs_bulkurb rdUrb[URB_NR];
+
+	int wrUrbWrIdx;
+	int wrUrbRdIdx;
+	int wrPending;
+	struct usb_functionfs_bulkurb wrUrb[URB_NR];
+} gs;
+#define URB_NEXT_IDX(idx_) do { \
+	idx_ = ((idx_)+1)&(URB_NR-1); \
+} while ((0))
+
+static void display_event(struct usb_functionfs_event *event)
+{
+	static const char *const names[] = {
+		[FUNCTIONFS_BIND] = "BIND",
+		[FUNCTIONFS_UNBIND] = "UNBIND",
+		[FUNCTIONFS_ENABLE] = "ENABLE",
+		[FUNCTIONFS_DISABLE] = "DISABLE",
+		[FUNCTIONFS_SETUP] = "SETUP",
+		[FUNCTIONFS_SUSPEND] = "SUSPEND",
+		[FUNCTIONFS_RESUME] = "RESUME",
+	};
+	switch (event->type) {
+	case FUNCTIONFS_BIND:
+	case FUNCTIONFS_UNBIND:
+	case FUNCTIONFS_ENABLE:
+	case FUNCTIONFS_DISABLE:
+	case FUNCTIONFS_SETUP:
+	case FUNCTIONFS_SUSPEND:
+	case FUNCTIONFS_RESUME:
+		printf("Event %s\n", names[event->type]);
+	default:
+		break;
+	}
+}
+
+static void handle_ep0(void)
+{
+	struct usb_functionfs_event event;
+	int ret;
+
+	ret = read(gs.ep0fd, &event, sizeof(event));
+	if (!ret) {
+		perror("unable to read event from ep0");
+		return;
+	}
+	display_event(&event);
+	switch (event.type) {
+	case FUNCTIONFS_SETUP:
+		if (event.u.setup.bRequestType & USB_DIR_IN)
+			write(gs.ep0fd, NULL, 0);
+		else
+			read(gs.ep0fd, NULL, 0);
+		break;
+	default:
+		break;
+	}
+}
+
+static void modify_data(uint32_t *buffer, int byteLen)
+{
+	// modify received data
+	// to be fast, this transformation is not depenent on endianness
+	while (byteLen > 0) {
+		*buffer = *buffer ^ 0xAAAAAAAA;
+		buffer++;
+		byteLen -= 4;
+	}
+}
+
+// simple echo test + modification of received data
+static void start_in_ep(struct usb_functionfs_bulkurb *rdUrb)
+{
+	struct usb_functionfs_bulkurb *wrUrb;
+	int err;
+
+	if (rdUrb->actual_length <= 0)
+		return;
+	modify_data(rdUrb->buffer, rdUrb->actual_length);
+	// send back to USB host
+	wrUrb = &(gs.wrUrb[gs.wrUrbWrIdx]);
+	memset(wrUrb, 0, sizeof(struct usb_functionfs_bulkurb));
+	wrUrb->buffer = rdUrb->buffer;
+	wrUrb->buffer_length = rdUrb->actual_length;
+	err = ioctl(gs.ep_in_fd, FUNCTIONFS_SUBMITBULKURB, wrUrb);
+	if (err) {
+		DBG_PRINTF("error FUNCTIONFS_SUBMITBULKURB\n");
+		printf("    err == %d, errno == %d\n", err, errno);
+		exit(1);
+	}
+	gs.wrPending++;
+	URB_NEXT_IDX(gs.wrUrbWrIdx);
+}
+
+// write request finished == device->host USB transfer finished
+// Sanity check and keep tally what has happened so far.
+static void complete_in_ep(void)
+{
+	struct usb_functionfs_bulkurb *urb;
+	struct usb_functionfs_bulkurb *wrUrb;
+	int err;
+
+	if (!gs.wrPending) {
+		DBG_PRINTF("no write pending\n");
+		exit(1);
+	}
+	err = ioctl(gs.ep_in_fd, FUNCTIONFS_REAPBULKURB, &urb);
+	if (err) {
+		DBG_PRINTF("error FUNCTIONFS_REAPBULKURB\n");
+		printf("    err == %d, errno == %d\n", err, errno);
+		exit(1);
+	}
+	wrUrb = &(gs.wrUrb[gs.wrUrbRdIdx]);
+	if (urb != wrUrb) {
+		DBG_PRINTF("unexpected WRITE URB\n");
+		printf("   urb == %p, should == %p\n", urb, wrUrb);
+		exit(1);
+	}
+	if (urb->status) {
+		// will be -ESHUTDOWN, when device is
+		// disconnected from host or UDC is unbound,
+		// while write is in progress
+		DBG_PRINTF("WRITE error: %d\n", urb->status);
+	} else if (urb->actual_length != urb->buffer_length) {
+		DBG_PRINTF("length mismatch\n");
+		printf("   requestd %d bytes, wrote %d bytes\n",
+			urb->buffer_length, urb->actual_length
+		);
+	}
+	URB_NEXT_IDX(gs.wrUrbRdIdx);
+}
+
+// start read requests for host->device transfers
+// we can start these requests even before
+// the UDC is bound or the USB device is connected
+static void start_out_ep(void)
+{
+	struct usb_functionfs_bulkurb *rdUrb;
+	int err;
+
+	while (gs.rdPending < 2) {
+		rdUrb = &(gs.rdUrb[gs.rdUrbWrIdx]);
+		memset(rdUrb, 0, sizeof(struct usb_functionfs_bulkurb));
+		rdUrb->buffer_length = 0x4000;
+		rdUrb->buffer = gs.dataBuf + (gs.rdUrbWrIdx * 0x4000);
+		err = ioctl(
+			gs.ep_out_fd, FUNCTIONFS_SUBMITBULKURB, rdUrb
+		);
+		if (err) {
+			DBG_PRINTF("Error FUNCTIONFS_SUBMITBULKURB\n");
+			printf("    err == %d, errno == %d\n", err, errno);
+			exit(1);
+		}
+		URB_NEXT_IDX(gs.rdUrbWrIdx);
+		gs.rdPending++;
+	}
+}
+
+// read finished == host->device USB transfer finished
+// handle received data (simple echo)
+static void complete_out_ep(void)
+{
+	struct usb_functionfs_bulkurb *urb;
+	struct usb_functionfs_bulkurb *rdUrb;
+	int err;
+
+	err = ioctl(gs.ep_out_fd, FUNCTIONFS_REAPBULKURB, &urb);
+	if (err) {
+		DBG_PRINTF("FUNCTIONFS_REAPBULKURB\n");
+		printf("    err == %d, errno == %d\n", err, errno);
+		exit(1);
+	}
+	rdUrb = &(gs.rdUrb[gs.rdUrbRdIdx]);
+	if (urb != rdUrb) {
+		DBG_PRINTF("unexpected READ URB\n");
+		printf("   urb == %p, should == %p\n", urb, rdUrb);
+		exit(1);
+	}
+	URB_NEXT_IDX(gs.rdUrbRdIdx);
+	gs.rdPending--;
+
+	// Start new read requests, before echoing data
+	start_out_ep();
+	if (!urb->status)
+		start_in_ep(urb);
+	else {
+		// will be -ESHUTDOWN, when device is
+		// disconnected from host or UDC is unbound,
+		// while read is in progress
+		DBG_PRINTF("READ error: %d\n", urb->status);
+	}
+}
+
+static uint8_t *alignTo8K(uint8_t *buf)
+{
+	uintptr_t bufAddr;
+
+	bufAddr  =   (uintptr_t)buf;
+	bufAddr +=   (uintptr_t)0x1FFF;
+	bufAddr &= ~((uintptr_t)0x1FFF);
+	return (uint8_t *)bufAddr;
+}
+
+int main(void)
+{
+	ssize_t ret;
+
+	gs.dataBuf = alignTo8K(gs.dataBufSpace);
+	gs.ep0fd = open("ep0", O_RDWR);
+	if (gs.ep0fd < 0) {
+		printf("Can't open ep0\n");
+		exit(1);
+	}
+	// Tell FunctionFS about descriptors/endpoints
+	ret = write(gs.ep0fd, &gSetupFFS, sizeof(gSetupFFS));
+	if (ret < 0) {
+		printf("Can't setup Interface/Endpoint descriptors\n");
+		exit(1);
+	}
+
+	// Tell function FS about strings
+	ret = write(gs.ep0fd, &gStrings, sizeof(gStrings));
+	if (ret < 0) {
+		printf("Can't setup string descriptors\n");
+		exit(1);
+	}
+
+	// now endpoints should be created and active...
+	gs.ep_in_fd = open("ep1", O_RDWR);
+	if (gs.ep_in_fd < 0) {
+		printf("Can't open ep1\n");
+		exit(1);
+	}
+	gs.ep_out_fd = open("ep2", O_RDWR);
+	if (gs.ep_out_fd < 0) {
+		printf("Can't open ep2\n");
+		exit(1);
+	}
+
+	// We are able to start URBs (USB Request Blocks)
+	// even while the device is disconnected
+	// and/or no UDC is bound to this function
+	start_out_ep();
+	for (;;) {
+		int status;
+		struct pollfd pollfds[3];
+
+		memset(pollfds, 0, sizeof(pollfds));
+		pollfds[0].fd = gs.ep0fd;
+		pollfds[0].events = POLLIN|POLLRDNORM;
+		// URB events are indicated via POLLOUT/POLLWRNORM
+		pollfds[1].fd = gs.ep_in_fd;
+		pollfds[1].events = POLLOUT|POLLWRNORM;
+		pollfds[2].fd = gs.ep_out_fd;
+		pollfds[2].events = POLLOUT|POLLWRNORM;
+		status = poll(pollfds, 3, 1000);
+		if (status == 0) {
+			printf("poll timeout...\n");
+			continue;
+		}
+		if (status < 0) {
+			printf("poll error. errno = %d\n", errno);
+			continue;
+		}
+		if (pollfds[0].revents & (POLLIN|POLLRDNORM))
+			handle_ep0();
+		if (pollfds[1].revents & (POLLOUT|POLLWRNORM))
+			complete_in_ep();
+		if (pollfds[2].revents & (POLLOUT|POLLWRNORM))
+			complete_out_ep();
+	}
+	return 0;
+}
diff --git a/tools/usb/ffs-urb-example/device_app/usb_gadget_mk b/tools/usb/ffs-urb-example/device_app/usb_gadget_mk
new file mode 100644
index 000000000000..95f71d0f7a9d
--- /dev/null
+++ b/tools/usb/ffs-urb-example/device_app/usb_gadget_mk
@@ -0,0 +1,79 @@
+#!/bin/ash
+# SPDX-License-Identifier: MIT
+
+# If you run this in a shell on a USB device, this script might
+# be used to play around with the Linux USB Gadget Driver and FunctionFS
+# This script needs to be run AS ROOT.
+#
+# This script will:
+#  - create the directory /config  and mount ConfigFS on top of it
+#  - modprobe "libcomposite" to get a /config/usb_gadget entry
+#  - create /config/usb_gadget/g1   to configure a USB gadget
+#  - create .../g1/configs/c.1  as USB configuration
+#  - create .../g1/functions/ffs.usb0
+#    and attach it to configuration c.1
+#  - create /usbfunc and mount FunctionFS for "usb0" on to of it
+#
+# After this setup you should be able to use /usbfunc/ep0
+# to configure your function (via software).
+#
+# To bind your gadget to a UDC (USB Device Controller)
+# echo the name of the UDC to /config/usb_gadget/g1/UDC
+
+if [ ! -d /config ] ; then
+	mkdir /config
+	mount -t configfs none /config
+fi
+
+if [ ! -d /config/usb_gadget ] ; then
+	modprobe libcomposite
+fi
+
+cd /config/usb_gadget
+
+if [ ! -d g1 ] ; then
+	mkdir g1
+	cd g1
+else
+	cd g1
+fi
+
+if [ ! -d strings/0x409 ] ; then
+	mkdir strings/0x409
+fi
+
+echo "0x1209"       > idVendor
+echo "0x4567"       > idProduct
+echo "Generic"      > strings/0x409/manufacturer
+echo "Echo Test"    > strings/0x409/product
+echo "112233445566" > strings/0x409/serialnumber
+
+if [ ! -d configs/c.1 ] ; then
+	mkdir configs/c.1
+fi
+
+if [ ! -d configs/c.1/strings/0x409 ] ; then
+	mkdir configs/c.1/strings/0x409
+fi
+
+echo "default" > configs/c.1/strings/0x409/configuration
+
+# The assumption is, that your USB device is self powered
+# If not: Change here
+echo 0 > configs/c.1/MaxPower
+
+if [ ! -d functions/ffs.usb0 ] ; then
+	mkdir functions/ffs.usb0
+	ln -s functions/ffs.usb0 configs/c.1
+fi
+
+if [ ! -d /usbfunc ] ; then
+	mkdir /usbfunc
+	mount -t functionfs usb0 /usbfunc
+fi
+
+echo 'To start function: Example'
+echo '   cd /usbfunc'
+echo '   usb_func_echo &'
+echo 'To bind to a UDC (USB Device Controller): Example'
+echo '   echo "fe200000.dwc3" > /config/usb_gadget/g1/UDC'
diff --git a/tools/usb/ffs-urb-example/host_app/usb_test_echo.c b/tools/usb/ffs-urb-example/host_app/usb_test_echo.c
new file mode 100644
index 000000000000..6de6aabb498e
--- /dev/null
+++ b/tools/usb/ffs-urb-example/host_app/usb_test_echo.c
@@ -0,0 +1,370 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * This code implements the PC counterpart to usb_func_echo.c
+ *
+ * It sends data to usb_func_echo.c, receives returned data
+ * and checks that the returned data is correct.
+ *
+ * Usage:
+ *   usb_func_echo  <USB device node>
+ *
+ * To find out, which USB device node to use, watch /var/log/messages
+ *
+ * Example, in /var/log/messages you get
+ *   usb 1-9: new high-speed USB device number 13 using xhci_hcd
+ *   usb 1-9: New USB device found, idVendor=1209, idProduct=4567 (...)
+ *   usb 1-9: New USB device strings: Mfr=1, Product=2, SerialNumber=3
+ *   usb 1-9: Product: Echo Test
+ *   usb 1-9: Manufacturer: Generic
+ *   usb 1-9: SerialNumber: 112233445566
+ * Then
+ *    $ cat /sys/bus/usb/devices/1-9/busnum
+ *     1
+ *    $ cat /sys/bus/usb/devices/1-9/devnum
+ *     13
+ * There should be a device node under /dev/bus/usb/001/013
+ * repesenting your USB device. You might now test with
+ *    $ usb_func_echo /dev/bus/usb/001/013
+ * You might need ROOT access to run this test.
+ */
+
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <poll.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <linux/usbdevice_fs.h>
+#include <time.h>
+
+#define DBG_PRINTF(...) do { \
+	printf("%s : ", __func__); \
+	printf(__VA_ARGS__); \
+	fflush(stdout); \
+} while ((0))
+
+#define USB_INTF_EP_OUT 0x01
+#define USB_INTF_EP_IN  0x81
+
+// define number of URBs
+// MUST be a power of 2
+#define URB_NR 8
+
+struct checkT {
+	uint32_t initValue;
+	int byteLen;
+};
+
+static struct {
+	int usbFd;
+
+	int checkTotalBytes;
+	int checkWrIdx;
+	int checkRdIdx;
+	int checkPending;
+	struct checkT checks[URB_NR];
+
+	uint8_t  rdDataBufSpace[0x4000*URB_NR + 0x2000];
+	uint8_t *rdDataBuf;
+	int rdUrbWrIdx;
+	int rdUrbRdIdx;
+	int rdPending;
+	struct usbdevfs_urb rdUrb[URB_NR];
+
+	uint8_t  wrDataBufSpace[0x4000*URB_NR + 0x2000];
+	uint8_t *wrDataBuf;
+	int wrUrbWrIdx;
+	int wrUrbRdIdx;
+	int wrPending;
+	struct usbdevfs_urb wrUrb[URB_NR];
+} gs;
+#define URB_NEXT_IDX(idx_) do { \
+	idx_ = ((idx_)+1)&(URB_NR-1); \
+} while ((0))
+
+static void fill_out_data(uint32_t *data, int byteLen)
+{
+	struct checkT *check;
+	uint32_t v;
+
+	// this ultra simple initialization tries to be fast
+	check = &(gs.checks[gs.checkWrIdx]);
+	v = random();
+	check->initValue = v;
+	check->byteLen = byteLen;
+	while (byteLen > 0) {
+		*data = v;
+		data++;
+		v += 0x5abc8203;
+		byteLen -= 4;
+	}
+	URB_NEXT_IDX(gs.checkWrIdx);
+	gs.checkPending++;
+}
+
+static void start_out_urb(void)
+{
+	struct usbdevfs_urb *outUrb;
+	uint32_t *data;
+	int length;
+	int err;
+
+
+	// make sure:
+	// * 0x10 <= length <= 0x3FFC
+	// * length is a multiple of 4 == sizeof(uint32_t)
+	// * length is NOT a multiple of max packet size
+	length  = random();
+	length &= 0x3FFC;
+	length |= 0x10;
+
+	data = (uint32_t *)(gs.wrDataBuf + (gs.wrUrbWrIdx*0x4000));
+	outUrb = &(gs.wrUrb[gs.wrUrbWrIdx]);
+	fill_out_data(data, length);
+
+retry:
+	memset(outUrb, 0, sizeof(struct usbdevfs_urb));
+	outUrb->type     = USBDEVFS_URB_TYPE_BULK;
+	outUrb->endpoint = USB_INTF_EP_OUT;
+	outUrb->buffer   = data;
+	outUrb->buffer_length = length;
+	err = ioctl(gs.usbFd, USBDEVFS_SUBMITURB, outUrb);
+	if (err < 0) {
+		int errnoValue;
+
+		errnoValue = errno;
+		if (errnoValue == EINTR)
+			goto retry;
+		DBG_PRINTF("ioctl error: errno = %d\n", errnoValue);
+		exit(1);
+	}
+	URB_NEXT_IDX(gs.wrUrbWrIdx);
+	gs.wrPending++;
+}
+
+static void complete_out_urb(struct usbdevfs_urb *urb)
+{
+	struct usbdevfs_urb *outUrb;
+
+	if (!gs.wrPending) {
+		DBG_PRINTF("No write pending\n");
+		exit(1);
+	}
+	outUrb = &(gs.wrUrb[gs.wrUrbRdIdx]);
+	if (urb != outUrb) {
+		DBG_PRINTF("Unexpected URB\n");
+		exit(1);
+	}
+	if (urb->actual_length != urb->buffer_length) {
+		DBG_PRINTF("Wrong length:\n");
+		printf("    is     %d\n", urb->actual_length);
+		printf("    should %d\n", urb->buffer_length);
+		exit(1);
+	}
+	URB_NEXT_IDX(gs.wrUrbRdIdx);
+	gs.wrPending--;
+}
+
+static void start_in_urb(void)
+{
+	struct usbdevfs_urb *inUrb;
+	uint32_t *data;
+	int err;
+
+	data = (uint32_t *)(gs.rdDataBuf + (gs.rdUrbWrIdx*0x4000));
+	inUrb = &(gs.rdUrb[gs.rdUrbWrIdx]);
+
+retry:
+	memset(inUrb, 0, sizeof(struct usbdevfs_urb));
+	inUrb->type     = USBDEVFS_URB_TYPE_BULK;
+	inUrb->endpoint = USB_INTF_EP_IN;
+	inUrb->buffer   = data;
+	inUrb->buffer_length = 0x4000;
+	err = ioctl(gs.usbFd, USBDEVFS_SUBMITURB, inUrb);
+	if (err < 0) {
+		int errnoValue;
+
+		errnoValue = errno;
+		if (errnoValue == EINTR)
+			goto retry;
+		DBG_PRINTF("ioctl error: errno = %d\n", errnoValue);
+		exit(1);
+	}
+	URB_NEXT_IDX(gs.rdUrbWrIdx);
+	gs.rdPending++;
+}
+
+static void check_in_data(struct usbdevfs_urb *urb)
+{
+	struct checkT *check;
+	uint32_t v, c, *data;
+	int byteLen;
+
+	check = &(gs.checks[gs.checkRdIdx]);
+	v = check->initValue;
+	byteLen = check->byteLen;
+	if (urb->actual_length != byteLen) {
+		DBG_PRINTF("unexpected length:\n");
+		printf("    is     %d\n", urb->actual_length);
+		printf("    should %d\n", byteLen);
+		exit(1);
+	}
+	gs.checkTotalBytes += byteLen;
+	data = (uint32_t *)urb->buffer;
+	while (byteLen > 0) {
+		c = *data;
+		data++;
+		if (c != (v^0xAAAAAAAA)) {
+			DBG_PRINTF("unexpected data:\n");
+			printf("    is     0x%08x\n", c);
+			printf("    should 0x%08x\n", v^0xAAAAAAAA);
+			exit(1);
+		}
+		v += 0x5abc8203;
+		byteLen -= 4;
+	}
+	URB_NEXT_IDX(gs.checkRdIdx);
+	gs.checkPending--;
+}
+
+static void complete_in_urb(struct usbdevfs_urb *urb)
+{
+	struct usbdevfs_urb *inUrb;
+
+	if (!gs.rdPending) {
+		DBG_PRINTF("no reads pending\n");
+		exit(1);
+	}
+	inUrb = &(gs.rdUrb[gs.rdUrbRdIdx]);
+	if (urb != inUrb) {
+		DBG_PRINTF("unexpected urb\n");
+		exit(1);
+	}
+	if (urb->actual_length <= 0) {
+		DBG_PRINTF("illegal length: %d\n", urb->actual_length);
+		exit(1);
+	}
+	check_in_data(urb);
+	URB_NEXT_IDX(gs.rdUrbRdIdx);
+	gs.rdPending--;
+}
+
+static uint8_t *alignTo8K(uint8_t *buf)
+{
+	uintptr_t bufAddr;
+
+	bufAddr  =   (uintptr_t)buf;
+	bufAddr +=   (uintptr_t)0x1FFF;
+	bufAddr &= ~((uintptr_t)0x1FFF);
+	return (uint8_t *)bufAddr;
+}
+
+#define ECHO_TEST_TRANSFERS 10000
+static void do_echo_test(void)
+{
+	int status;
+	struct pollfd pollfds[1];
+	struct usbdevfs_urb *urb;
+	int checksSend;
+	int checksRecv;
+
+	checksSend = 0;
+	checksRecv = 0;
+	for (;;) {
+
+		while (
+			checksSend < ECHO_TEST_TRANSFERS &&
+			gs.checkPending < 2 &&
+			gs.wrPending < 2
+		) {
+			checksSend++;
+			start_out_urb();
+		}
+		while (
+			checksRecv < ECHO_TEST_TRANSFERS &&
+			gs.rdPending < 2
+		) {
+			checksRecv++;
+			start_in_urb();
+		}
+		if (gs.wrPending == 0 && gs.rdPending == 0)
+			break;
+		memset(pollfds, 0, sizeof(pollfds));
+		pollfds[0].fd = gs.usbFd;
+		pollfds[0].events = POLLOUT|POLLWRNORM;
+		status = poll(pollfds, 1, 1000);
+		if (status == 0) {
+			printf("poll timeout...\n");
+			continue;
+		}
+		if (status < 0)
+			continue;
+		if (!pollfds[0].revents)
+			continue;
+		status = ioctl(gs.usbFd, USBDEVFS_REAPURB, &urb);
+		if (status < 0) {
+			printf("USBDEVFS_REAPURB error\n");
+			exit(1);
+		}
+		if (urb->endpoint == USB_INTF_EP_OUT)
+			complete_out_urb(urb);
+		else if (urb->endpoint == USB_INTF_EP_IN)
+			complete_in_urb(urb);
+	}
+}
+
+int main(int argc, char **argv)
+{
+	int intf, err;
+	struct timespec tp_start, tp_stop;
+	int64_t tDiff;
+
+	if (argc < 2) {
+		printf("Usage:\n");
+		printf("    usb_test_echo <usb device node>\n");
+		return 1;
+	}
+	gs.usbFd = open(argv[1], O_RDWR);
+	if (gs.usbFd < 0) {
+		printf("Can't open %s\nAborting.\n", argv[1]);
+		return 2;
+	}
+
+	intf = 0;
+	err = ioctl(gs.usbFd, USBDEVFS_CLAIMINTERFACE, &intf);
+	if (err) {
+		printf("Can't claim interface 0. Aborting\n");
+		return 3;
+	}
+	printf("Claimed interface 0...\n");
+
+	gs.rdDataBuf = alignTo8K(gs.rdDataBufSpace);
+	gs.wrDataBuf = alignTo8K(gs.wrDataBufSpace);
+
+
+
+	clock_gettime(CLOCK_MONOTONIC_RAW, &tp_start);
+	do_echo_test();
+	clock_gettime(CLOCK_MONOTONIC_RAW, &tp_stop);
+	tDiff  = (int64_t)(tp_stop.tv_sec)*(int64_t)1000000000;
+	tDiff += (int64_t)(tp_stop.tv_nsec);
+	tDiff -= (int64_t)(tp_start.tv_sec)*(int64_t)1000000000;
+	tDiff -= (int64_t)(tp_start.tv_nsec);
+	tDiff /= 1000;
+	printf("transferred %d bytes back and forth\n", gs.checkTotalBytes);
+	printf("%d iterations took %ld usec\n", ECHO_TEST_TRANSFERS, tDiff);
+
+	intf = 0;
+	err = ioctl(gs.usbFd, USBDEVFS_RELEASEINTERFACE, &intf);
+	if (err) {
+		printf("Can't release interface 0. Aborting\n");
+		return 4;
+	}
+	printf("Released interface 0...\n");
+	return 0;
+}
-- 
2.17.1


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

* Re: [PATCH 0/2] usb: gadget: User space URBs for FunctionFS
  2020-11-11 17:07 [PATCH 0/2] usb: gadget: User space URBs for FunctionFS Ingo Rohloff
  2020-11-11 17:07 ` [PATCH 1/2] usb: gadget: ffs: Implement user URBs for USB bulk endpoints Ingo Rohloff
  2020-11-11 17:07 ` [PATCH 2/2] usb: gadget: ffs: tools: test applications for user URBs Ingo Rohloff
@ 2020-11-11 18:40 ` Greg KH
  2020-11-12 17:05   ` Ingo Rohloff
  2 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2020-11-11 18:40 UTC (permalink / raw)
  To: Ingo Rohloff; +Cc: balbi, linux-usb

On Wed, Nov 11, 2020 at 06:07:16PM +0100, Ingo Rohloff wrote:
> I am working on a platform (Xilinx Zynq Ultrascale+), which
> is supposed to work as a pure USB Device (not dual-role).
> 
> To get fast USB bulk transfers I wanted to have something similar
> like USBDEVFS_SUBMITURB/USBDEVFS_REAPURB, but for an USB Device.
> 
> I now implement two new ioctls for FunctionFS:
>   FUNCTIONFS_SUBMITBULKURB
>   FUNCTIONFS_REAPBULKURB
> which provide simliar functionality.
> 
> A similar functionality is already implemented via AIO. But: To use this
> API, your user space environment needs to know how to use these system
> calls.

So instead you created a new interface which requires different system
calls?

Doing it in a different way is "interesting", but you are duplicating
existing functionality here.  What is wrong with the AIO interface that
we currently have that keeps you from using it, other than it being
"different" than some other user/kernel interfaces that people are
familiar with?

> Additionally the semantics of the ioctls is slightly different:
> Usually you can only access a FunctionFS file if the FunctionFS is
> already bound to an UDC (USB Device Controller) and the USB Device is
> connected to a USB Host (which then enables the appropriate configuration
> and USB endpoints).
> These new ioctls behave different: You already can submit URBs before the
> Function is bound to an UDC and before the USB Device is connected.
> These "pending" URBs will be activated once the endpoints become active.
> 
> When the endpoints become deactivated (either by a disconnect from the
> USB Host or by unbinding the UDC), active URBs are cancelled.
> 
> A user space program will then get a notification, that the URBs have
> been cancelled and the status will indicate to the user space program,
> that the connection was lost.
> Via this mechanism a user space program can keep precise track, which
> URBs succeeded and which URBs failed.

So, it implements AIO in a different way?

> The final goal here is to be able to directly let user space provide data
> buffers (via mmap I guess), which are then transferred via USB; but this
> is the next step.

Isn't that kind of what the AIO inteface provides today?  :)

thanks,

greg k-h

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

* Re: [PATCH 0/2] usb: gadget: User space URBs for FunctionFS
  2020-11-11 18:40 ` [PATCH 0/2] usb: gadget: User space URBs for FunctionFS Greg KH
@ 2020-11-12 17:05   ` Ingo Rohloff
  2020-11-13 14:20     ` Greg KH
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Rohloff @ 2020-11-12 17:05 UTC (permalink / raw)
  To: Greg KH; +Cc: balbi, linux-usb

Hello Greg,

After writing this response: 
I want to redo some parts of the patch, so please ignore the current
version.
I feel that what I did is not extensible enough.

I still want to know if you think, if it makes sense at all to publish
any of that.


On Wed, 11 Nov 2020 19:40:54 +0100
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Wed, Nov 11, 2020 at 06:07:16PM +0100, Ingo Rohloff wrote:
> ...
> > I now implement two new ioctls for FunctionFS:
> >   FUNCTIONFS_SUBMITBULKURB
> >   FUNCTIONFS_REAPBULKURB
> > which provide simliar functionality.
> > 
> > A similar functionality is already implemented via AIO. But: To use
> > this API, your user space environment needs to know how to use
> > these system calls.  

OK I now understand why I was confused myself:
I was looking into the POSIX AIO functions ("man 7 aio").
At least in the C library which I use, the implementation of
POSIX AIO does not use the native Linux system calls.
What I meant above was: This C library does not know about the native
Linux system calls.

Now because of your comment I learned about "libaio", which uses the
Linux native system calls.

The first problem here: Where do you get this library from ?
At the end I got it from here:
  https://pagure.io/libaio 
  version 0.3.111
I am still not sure if this is the right place, because at least
Debian provides a 0.3.112 version, which seems to use an extra
system call (io_pgetevents); no clue if I should use the Debian
version or not.

Of course you could call the system calls directly (without using
libaio), but that's even worse I think.

> 
> So instead you created a new interface which requires different system
> calls?
> 

I created a new interface which uses the old IOCTL system call,
by extending it.
Of course the C library I use knows how to do an IOCTL system call;
which means I do not need another library (like "libaio") and extra
knowledge about other system calls.

Granted: Adding new IOCTL codes is very similar to adding new
system calls, but at least I understand the interface here.
In contrast: It was not exactly easy to get my hands on "libaio".
The fact that there seem to exist different versions of this library,
which employ different system calls also does not make me feel very
comfortable.

> Doing it in a different way is "interesting", but you are duplicating
> existing functionality here.  What is wrong with the AIO interface
> that we currently have that keeps you from using it, other than it
> being "different" than some other user/kernel interfaces that people
> are familiar with?

The first problem was: I did not know about "libaio" :)

But there is more: The existing AIO interface, does not give you the
amount of control over USB transactions which I want.

Example: I want to send 8192 bytes   USB Device -> USB Host

Because this should resemble one USB Transaction,
I want to add a Zero Length Packet at the end.

How would I do that via the AIO interface ?

Another example of this kind of missing control can be found in
comments in "f_fs.c":

   /*
    * Dear user space developer!
    *
    * TL;DR: To stop getting below error message in your kernel ...

so this tells you, you need to be aware of some USB specific quirks,
when using AIO together with FunctionFS.
I much rather want to use an interface which already is specific for
USB instead of using a generic interface (AIO), which then exhibits 
USB specific quirks.
I want to be able to get as close to the USB layer as possible with 
user space; AIO adds another abstraction layer between user space and
USB, which in turn means I loose some of the control over USB.

There are more FunctionFS AIO quirks, which I don't like at all:
Here is some output of a slightly patched 
  <linux kernel>/tools/usb/ffs-aio-example/simple/device_app
test application using libaio in conjunction with FuncionFS:

// cable connected and host test application started:
  submit: out
  ev=in; ret=8192   // ret := struct ioevent.res
  submit: in
  ev=out; ret=8192
  submit: out
// cable disconnected
  Event DISABLE  

// The submitted AIOs are not cancelled ?

// cable connected again
  Event ENABLE   
  ev=in; ret=-108  // -ESHUTDOWN from  struct ioevent.res
  ev=out; ret=-108
  submit: in
  submit: out

As you can see from above: The AIO completes with an "-ESHUTDOWN"
error, once you *reconnect* the cable (not after the disconnect).
I think this is the wrong point of time.


This also is another example, why I don't like using a generic
interface (AIO) for triggering USB specific operations:
The "ret" output above comes from a "struct ioevent.res" member;
I guess this is completely specific for FuncionFS. 
This "res" member seems to convey length information (how much 
was actually transferred) plus status/error information when it 
is negative.
If I get this correctly normal AIO operations are not supposed to
complete prematurely ? 
"prematurely": I mean transferring less than the specified length.
Maybe I just don't understand the libaio interface well enough.

> > ...
> > Via this mechanism a user space program can keep precise track,
> > which URBs succeeded and which URBs failed.  
> 
> So, it implements AIO in a different way?

Yes and No:
It does not exhibit the behavior described above.
When the cable gets disconnected, the URBs will complete with
a status which tells you the cable was disconnected.

I find it much clearer to have *specific* URB struct members called
"status" and "actual_length", than accessing a *generic* "struct
ioevent.res" member, which only seems to have this particular meaning
for FunctionFS file descriptors.

To implement 
  <linux kernel>/tools/usb/ffs-aio-example/simple/device_app
it seems you also need "eventfd" to be able to use "poll";
so this adds yet another layer of complexity.

Using an IOCTL call, you get direct control over USB transactions.
So you are much closer to the USB layer than if you are using AIO.
This means you can easily extend the functionality to support 
USB/FunctionFS specific things:

> > The final goal here is to be able to directly let user space
> > provide data buffers (via mmap I guess), which are then transferred
> > via USB; but this is the next step.  
> 
> Isn't that kind of what the AIO inteface provides today?  :)

I think my explanation was not clear at all:
What I want to have is a "zero copy" transfer.
As far as I know, "devio.c" already has that for the USB host side:
User space is able to "mmap" access to coherent DMA buffers.
When you then pass the URB to "devio.c" no copying is going on at all;
the "mmap" buffers are directly used as data buffers for the USB host
controller; at least that's what I understood from the "devio.c" code.
(In "devio.c" you need to grep "as->usbm" to find the code.)

The FunctionFS AIO does copy user data to/from kernel buffers.
I want to be able to *completely* get rid of these copy operations.

Why: Because I want to be able to provide >400MB/s to a PC,
when using USB SuperSpeed; each copy operation hurts here.

so long
  Ingo

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

* Re: [PATCH 0/2] usb: gadget: User space URBs for FunctionFS
  2020-11-12 17:05   ` Ingo Rohloff
@ 2020-11-13 14:20     ` Greg KH
  2020-11-13 15:20       ` Ingo Rohloff
  0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2020-11-13 14:20 UTC (permalink / raw)
  To: Ingo Rohloff; +Cc: balbi, linux-usb

On Thu, Nov 12, 2020 at 06:05:35PM +0100, Ingo Rohloff wrote:
> On Wed, 11 Nov 2020 19:40:54 +0100
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > On Wed, Nov 11, 2020 at 06:07:16PM +0100, Ingo Rohloff wrote:
> > ...
> > > I now implement two new ioctls for FunctionFS:
> > >   FUNCTIONFS_SUBMITBULKURB
> > >   FUNCTIONFS_REAPBULKURB
> > > which provide simliar functionality.
> > > 
> > > A similar functionality is already implemented via AIO. But: To use
> > > this API, your user space environment needs to know how to use
> > > these system calls.  
> 
> OK I now understand why I was confused myself:
> I was looking into the POSIX AIO functions ("man 7 aio").
> At least in the C library which I use, the implementation of
> POSIX AIO does not use the native Linux system calls.
> What I meant above was: This C library does not know about the native
> Linux system calls.

Then use a new library :)

> Now because of your comment I learned about "libaio", which uses the
> Linux native system calls.

Great!

> The first problem here: Where do you get this library from ?
> At the end I got it from here:
>   https://pagure.io/libaio 
>   version 0.3.111
> I am still not sure if this is the right place, because at least
> Debian provides a 0.3.112 version, which seems to use an extra
> system call (io_pgetevents); no clue if I should use the Debian
> version or not.

I do not know if this is the latest one or not, sorry.  Ask your Linux
distro about this, or use the "raw" kernel aio syscalls.

The gadget code has always used AIO since the very beginning, this is
nothing new (decades old).  While it might feel "odd", I recommend
working with it first before trying to create new kernel apis that
duplicate existing functionality for the only reason being that AIO is
"different".

But if you find bugs in the current implementation, we will gladly
accept patches.

thanks,

greg k-h

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

* Re: [PATCH 0/2] usb: gadget: User space URBs for FunctionFS
  2020-11-13 14:20     ` Greg KH
@ 2020-11-13 15:20       ` Ingo Rohloff
  0 siblings, 0 replies; 11+ messages in thread
From: Ingo Rohloff @ 2020-11-13 15:20 UTC (permalink / raw)
  To: Greg KH; +Cc: balbi, linux-usb

Hi Greg,

> The gadget code has always used AIO since the very beginning, this is
> nothing new (decades old).  While it might feel "odd", I recommend
> working with it first before trying to create new kernel apis that
> duplicate existing functionality for the only reason being that AIO is
> "different".

Forget my patch: You are right: I am now convinced, that using the AIO
of the kernel should really provide the same.

For me it was the other way round: I wrote code talking to "devio.c" on
the USB Host and using "ioctl"; so that sounded natural to me.
I think "devio.c" does not support async I/O ?
So when starting, I was not aware at all that the gadget side supports
AIO; and then I got misled by the POSIX aio interface (which did hide
the native Linux aio system calls from me.)

The "bug" I found is a bug in the example code (not in the kernel)
as far as I can tell.

The other thing I want in the future:
> > > The final goal here is to be able to directly let user space
> > > provide data buffers (via mmap I guess), which are then
> > > transferred via USB; but this is the next step.    
> 
> > Isn't that kind of what the AIO inteface provides today?  :)  
>
> I think my explanation was not clear at all:
> What I want to have is a "zero copy" transfer.

I now think this can be implemented within the already existing
AIO framework in f_fs.c by implementing a suitable mmap call.

But before doing any of that I need test code.

So my plan right now:
- Write a working (fast) echo example using libaio

- Write a working (fast) echo example using liburing
  (https://github.com/axboe/liburing)
  because this should result in even faster AIO.
  Another big reason for not doing extra ioctls;
  using the existing AIO framework in the kernel
  should allow to use liburing :)

- Once I have done that I will look into extending
  f_fs.c with a mmap call so that complete zero copy
  transfers to/from USB bulk endpoints should become possible.

Then I should be able to do some performance tests via USB 3.0
to see how much this helps.

I will post again once I have some working code.

Thank you for your time and comments; 
that really helps me to find a better solution :-)

so long
  Ingo


PS: Not important:
> >   https://pagure.io/libaio 
> >   version 0.3.111
>
> I do not know if this is the latest one or not, sorry.  Ask your Linux
> distro about this, or use the "raw" kernel aio syscalls.

Thanks to gentoo, I found out there is 
  https://releases.pagure.org/libaio/
and this one has the 0.3.112 release; and of course there is always the
git repository (which unfortunately does not include tags for anything
more recent than 0.3.110).

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

* Re: [PATCH 1/2] usb: gadget: ffs: Implement user URBs for USB bulk endpoints
  2020-11-11 17:07 ` [PATCH 1/2] usb: gadget: ffs: Implement user URBs for USB bulk endpoints Ingo Rohloff
@ 2020-11-15 10:59     ` kernel test robot
  2020-11-15 10:59     ` kernel test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-11-15 10:59 UTC (permalink / raw)
  To: Ingo Rohloff, balbi; +Cc: kbuild-all, gregkh, linux-usb, Ingo Rohloff

[-- Attachment #1: Type: text/plain, Size: 2639 bytes --]

Hi Ingo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on balbi-usb/testing/next]
[also build test WARNING on usb/usb-testing peter.chen-usb/ci-for-usb-next linus/master v5.10-rc3 next-20201113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ingo-Rohloff/usb-gadget-User-space-URBs-for-FunctionFS/20201112-011456
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
config: x86_64-randconfig-s032-20201111 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-107-gaf3512a6-dirty
        # https://github.com/0day-ci/linux/commit/9f07add950a345cd51a7d56e685577b93da8a5f0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ingo-Rohloff/usb-gadget-User-space-URBs-for-FunctionFS/20201112-011456
        git checkout 9f07add950a345cd51a7d56e685577b93da8a5f0
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


"sparse warnings: (new ones prefixed by >>)"
>> drivers/usb/gadget/function/f_fs.c:240:1: sparse: sparse: symbol 'async_bulkurb_active_lock' was not declared. Should it be static?
   drivers/usb/gadget/function/f_fs.c:3336:32: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short [usertype] wMaxPacketSize @@     got restricted __le16 [usertype] wMaxPacketSize @@
   drivers/usb/gadget/function/f_fs.c:3336:32: sparse:     expected unsigned short [usertype] wMaxPacketSize
   drivers/usb/gadget/function/f_fs.c:3336:32: sparse:     got restricted __le16 [usertype] wMaxPacketSize
   drivers/usb/gadget/function/f_fs.c:3361:36: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] wMaxPacketSize @@     got unsigned short [usertype] wMaxPacketSize @@
   drivers/usb/gadget/function/f_fs.c:3361:36: sparse:     expected restricted __le16 [usertype] wMaxPacketSize
   drivers/usb/gadget/function/f_fs.c:3361:36: sparse:     got unsigned short [usertype] wMaxPacketSize

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35860 bytes --]

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

* Re: [PATCH 1/2] usb: gadget: ffs: Implement user URBs for USB bulk endpoints
@ 2020-11-15 10:59     ` kernel test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-11-15 10:59 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2683 bytes --]

Hi Ingo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on balbi-usb/testing/next]
[also build test WARNING on usb/usb-testing peter.chen-usb/ci-for-usb-next linus/master v5.10-rc3 next-20201113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ingo-Rohloff/usb-gadget-User-space-URBs-for-FunctionFS/20201112-011456
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
config: x86_64-randconfig-s032-20201111 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-107-gaf3512a6-dirty
        # https://github.com/0day-ci/linux/commit/9f07add950a345cd51a7d56e685577b93da8a5f0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ingo-Rohloff/usb-gadget-User-space-URBs-for-FunctionFS/20201112-011456
        git checkout 9f07add950a345cd51a7d56e685577b93da8a5f0
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


"sparse warnings: (new ones prefixed by >>)"
>> drivers/usb/gadget/function/f_fs.c:240:1: sparse: sparse: symbol 'async_bulkurb_active_lock' was not declared. Should it be static?
   drivers/usb/gadget/function/f_fs.c:3336:32: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short [usertype] wMaxPacketSize @@     got restricted __le16 [usertype] wMaxPacketSize @@
   drivers/usb/gadget/function/f_fs.c:3336:32: sparse:     expected unsigned short [usertype] wMaxPacketSize
   drivers/usb/gadget/function/f_fs.c:3336:32: sparse:     got restricted __le16 [usertype] wMaxPacketSize
   drivers/usb/gadget/function/f_fs.c:3361:36: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] wMaxPacketSize @@     got unsigned short [usertype] wMaxPacketSize @@
   drivers/usb/gadget/function/f_fs.c:3361:36: sparse:     expected restricted __le16 [usertype] wMaxPacketSize
   drivers/usb/gadget/function/f_fs.c:3361:36: sparse:     got unsigned short [usertype] wMaxPacketSize

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35860 bytes --]

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

* [RFC PATCH] usb: gadget: ffs: async_bulkurb_active_lock can be static
  2020-11-11 17:07 ` [PATCH 1/2] usb: gadget: ffs: Implement user URBs for USB bulk endpoints Ingo Rohloff
@ 2020-11-15 10:59     ` kernel test robot
  2020-11-15 10:59     ` kernel test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-11-15 10:59 UTC (permalink / raw)
  To: Ingo Rohloff, balbi; +Cc: kbuild-all, gregkh, linux-usb, Ingo Rohloff


Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---
 f_fs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 194d45d940313..b59e144ad7e0e 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -237,7 +237,7 @@ enum {
 	FFS_BULKURB_STATE_PENDING  = 1,
 	FFS_BULKURB_STATE_ACTIVE   = 2
 };
-DEFINE_SPINLOCK(async_bulkurb_active_lock);
+static DEFINE_SPINLOCK(async_bulkurb_active_lock);
 struct async_bulkurb {
 	struct ffs_epfile *epfile;
 	struct list_head urblist;

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

* [RFC PATCH] usb: gadget: ffs: async_bulkurb_active_lock can be static
@ 2020-11-15 10:59     ` kernel test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-11-15 10:59 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]


Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---
 f_fs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 194d45d940313..b59e144ad7e0e 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -237,7 +237,7 @@ enum {
 	FFS_BULKURB_STATE_PENDING  = 1,
 	FFS_BULKURB_STATE_ACTIVE   = 2
 };
-DEFINE_SPINLOCK(async_bulkurb_active_lock);
+static DEFINE_SPINLOCK(async_bulkurb_active_lock);
 struct async_bulkurb {
 	struct ffs_epfile *epfile;
 	struct list_head urblist;

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

end of thread, other threads:[~2020-11-15 11:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 17:07 [PATCH 0/2] usb: gadget: User space URBs for FunctionFS Ingo Rohloff
2020-11-11 17:07 ` [PATCH 1/2] usb: gadget: ffs: Implement user URBs for USB bulk endpoints Ingo Rohloff
2020-11-15 10:59   ` kernel test robot
2020-11-15 10:59     ` kernel test robot
2020-11-15 10:59   ` [RFC PATCH] usb: gadget: ffs: async_bulkurb_active_lock can be static kernel test robot
2020-11-15 10:59     ` kernel test robot
2020-11-11 17:07 ` [PATCH 2/2] usb: gadget: ffs: tools: test applications for user URBs Ingo Rohloff
2020-11-11 18:40 ` [PATCH 0/2] usb: gadget: User space URBs for FunctionFS Greg KH
2020-11-12 17:05   ` Ingo Rohloff
2020-11-13 14:20     ` Greg KH
2020-11-13 15:20       ` Ingo Rohloff

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.