Userspace applications need to know the maximum supported message
size.
Using a character device as interface to USB control messages hides
the fact that these messages have a strict size limit. The userspace
application must in most cases still be aware of this limit. It must
allocate sufficiently large read buffers, and it must prevent the
driver from splitting messages if the protocol requires more
advanced fragmentation like e.g. CDC MBIM does.
The limit could be read from CDC functional descriptors for CDC WDM
and CDC MBIM devices, duplicating the parsing already done by the
driver probe. For other devices, where the limit is based on a
hardcoded default, the application will need hardcode this default
as well. Such hidden and implicit kernel dependencies are bad and
makes it impossible to ever change the defaults.
All this is unnecessarily complex and likely to make drivers and
applications end up using different limits, causing errors which
are hard to debug and replicate.
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.
Signed-off-by: Bjørn Mork <bjorn@xxxxxxx>
---
Oliver Neukum <oliver@xxxxxxxxxx> writes:
> On Saturday 09 February 2013 20:16:20 Bjørn Mork wrote:
>> Oliver Neukum <oliver@xxxxxxxxxx> writes:
>> > On Saturday 09 February 2013 18:41:52 Bjørn Mork wrote:
>
>> Well, OK..., "generic" then. In the sense that the attribute stays the
>> same regardless of whether the value is hardcoded in the driver (QMI),
>> or parsed from wMaxCommand (CDC WDM) or wMaxControlMessage (CDC MBIM)
>>
>> Not sure I understand what you want here... I am trying to avoid having
>> three different attributes for the three drivers currently needing this
>> number.
>
> I would say that the most generic solution would be an ioctl()
I am not exactly sure how to do this, but does this look like something
that could be submitted?
The issue just came up again, after I tried to get a user with an
Ericsson H5521gw modem to run mbimcli. It failed because the hard coded
message size is too big. We need a simple way for userspace applications
to check the message size.
Bjørn
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
