[PATCH] USB: cdc-wdm: implement IOCTL_WDM_MAX_COMMAND

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Userspace applications need to know the maximum supported message
size.

The cdc-wdm driver translates between a character device stream
and a message based protocol.  Each message is transported as a
usb control message with no further encapsulation or syncronization.
Each read or write on the character device should translate to
exactly one usb control message to ensure that message boundaries
are kept intact.  That means that the userspace application must
know the maximum message size supported by the device and driver,
making this size a vital part of the cdc-wdm character device API.

CDC WDM and CDC MBIM functions export the maximum supported
message size through CDC functional descriptors.  The cdc-wdm and
cdc_mbim drivers will parse these descriptors and use the value
chosen by the device.  The only current way for a userspace
application to retrive the value is by duplicating the descriptor
parsing. This is an unnecessary complex task, and application
writers are likely to postpone it, using a fixed value and adding
a "todo" item.

QMI functions have no way to tell the host what message size they
support.  The qmi_wwan driver use a fixed value based on protocol
recommendations and observed device behaviour.  Userspace
applications must know and hard code the same value.  This scheme
will break if we ever encounter a QMI device needing a device
specific message size quirk.  We are currently unable to support
such a device because using a non default size would break the
implicit userspace API.

The message size is currently a hidden attribute of the cdc-wdm
userspace API.  Retrieving it is unnecessarily complex, increasing
the possibility of drivers and applications using different limits.
The resulting errors are hard to debug, and can only be replicated
on identical hardware.

Exporting the maximum message size from the driver simplifies the
task for the userspace application, and creates a unified
information source independent of device and function class. It also
serves to document that the message size is part of the cdc-wdm
userspace API.

This proposed API extension has been presented for the authors of
userspace applications and libraries using the current API: libmbim,
libqmi, uqmi, oFono and ModemManager.  The replies were:

Aleksander Morgado:
 "We do really need max message size for MBIM; and as you say, it may be
  good to have the max message size info also for QMI, so the new ioctl
  seems a good addition. So +1 from my side, for what it's worth."

Dan Williams:
 "Yeah, +1 here.  I'd prefer the sysfs file, but the fact that that
  doesn't work for fd passing pretty much kills it."

No negative replies are so far received.

Cc: Aleksander Morgado <aleksander@xxxxxxxxxx>
Cc: Dan Williams <dcbw@xxxxxxxxxx>
Signed-off-by: Bjørn Mork <bjorn@xxxxxxx>
---
 Documentation/ioctl/ioctl-number.txt |    1 +
 drivers/usb/class/cdc-wdm.c          |   19 +++++++++++++++++++
 include/linux/usb/cdc-wdm.h          |    2 ++
 include/uapi/linux/usb/cdc-wdm.h     |   21 +++++++++++++++++++++
 4 files changed, 43 insertions(+)
 create mode 100644 include/uapi/linux/usb/cdc-wdm.h

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 3210540..237acab 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -131,6 +131,7 @@ Code  Seq#(hex)	Include File		Comments
 'H'	40-4F	sound/hdspm.h		conflict!
 'H'	40-4F	sound/hdsp.h		conflict!
 'H'	90	sound/usb/usx2y/usb_stream.h
+'H'	A0	uapi/linux/usb/cdc-wdm.h
 'H'	C0-F0	net/bluetooth/hci.h	conflict!
 'H'	C0-DF	net/bluetooth/hidp/hidp.h	conflict!
 'H'	C0-DF	net/bluetooth/cmtp/cmtp.h	conflict!
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 5f0cb41..6463d8c 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -13,6 +13,7 @@
  */
 #include <linux/kernel.h>
 #include <linux/errno.h>
+#include <linux/ioctl.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -628,6 +629,22 @@ static int wdm_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
+static long wdm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct wdm_device *desc = file->private_data;
+	int rv = 0;
+
+	switch (cmd) {
+	case IOCTL_WDM_MAX_COMMAND:
+		if (copy_to_user((void __user *)arg, &desc->wMaxCommand, sizeof(desc->wMaxCommand)))
+			rv = -EFAULT;
+		break;
+	default:
+		rv = -ENOTTY;
+	}
+	return rv;
+}
+
 static const struct file_operations wdm_fops = {
 	.owner =	THIS_MODULE,
 	.read =		wdm_read,
@@ -636,6 +653,8 @@ static const struct file_operations wdm_fops = {
 	.flush =	wdm_flush,
 	.release =	wdm_release,
 	.poll =		wdm_poll,
+	.unlocked_ioctl = wdm_ioctl,
+	.compat_ioctl = wdm_ioctl,
 	.llseek =	noop_llseek,
 };
 
diff --git a/include/linux/usb/cdc-wdm.h b/include/linux/usb/cdc-wdm.h
index 719c332..0b3f429 100644
--- a/include/linux/usb/cdc-wdm.h
+++ b/include/linux/usb/cdc-wdm.h
@@ -11,6 +11,8 @@
 #ifndef __LINUX_USB_CDC_WDM_H
 #define __LINUX_USB_CDC_WDM_H
 
+#include <uapi/linux/usb/cdc-wdm.h>
+
 extern struct usb_driver *usb_cdc_wdm_register(struct usb_interface *intf,
 					struct usb_endpoint_descriptor *ep,
 					int bufsize,
diff --git a/include/uapi/linux/usb/cdc-wdm.h b/include/uapi/linux/usb/cdc-wdm.h
new file mode 100644
index 0000000..f03134f
--- /dev/null
+++ b/include/uapi/linux/usb/cdc-wdm.h
@@ -0,0 +1,21 @@
+/*
+ * USB CDC Device Management userspace API definitions
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#ifndef _UAPI__LINUX_USB_CDC_WDM_H
+#define _UAPI__LINUX_USB_CDC_WDM_H
+
+/*
+ * This IOCTL is used to retrieve the wMaxCommand for the device,
+ * defining the message limit for both reading and writing.
+ *
+ * For CDC WDM functions this will be the wMaxCommand field of the
+ * Device Management Functional Descriptor.
+ */
+#define IOCTL_WDM_MAX_COMMAND _IOR('H', 0xA0, __u16)
+
+#endif /* _UAPI__LINUX_USB_CDC_WDM_H */
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux