[RFC] USB: cdc-wdm: add msgsize sysfs attribute

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

 



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 class.

Signed-off-by: Bjørn Mork <bjorn@xxxxxxx>
---
Hello Oliver,

this is something I should have thought about a long time ago.
It just popped up in connection with the new cdc_mbim driver, but
the question is IMHO just as relevant for CDC WDM devices. Not to
mention QMI devices...  

I realize that there is some tradition for letting USB drivers
add attributes to the USB interface device when they parse
functional descriptors, but I really think it is a better idea
to keep this as a generic cdc-wdm property independent of the
device class.  That's why I propose a generic name and attaching
the attribute to the cdc-wdm device.

Example showing one QMI, two WDM and one MBIM device:

 $ grep . /sys/class/usbmisc/cdc-wdm*/msgsize
 /sys/class/usbmisc/cdc-wdm0/msgsize:4096
 /sys/class/usbmisc/cdc-wdm1/msgsize:2048
 /sys/class/usbmisc/cdc-wdm2/msgsize:2048
 /sys/class/usbmisc/cdc-wdm3/msgsize:1536

Thoughs or comments?


Bjørn

 Documentation/ABI/testing/sysfs-class-usbmisc-cdc-wdm |   14 ++++++++++++++
 drivers/usb/class/cdc-wdm.c                           |   14 ++++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-usbmisc-cdc-wdm

diff --git a/Documentation/ABI/testing/sysfs-class-usbmisc-cdc-wdm b/Documentation/ABI/testing/sysfs-class-usbmisc-cdc-wdm
new file mode 100644
index 0000000..300458d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-usbmisc-cdc-wdm
@@ -0,0 +1,14 @@
+What:		/sys/class/usbmisc/cdc-wdmX/msgsize
+Date:		February 2013
+KernelVersion:	3.10
+Contact:	Bjørn Mork <bjorn@xxxxxxx>
+Description:
+		Maximum message size in bytes supported by the device
+		for both reading and writing, independent of driver
+		and device class.
+
+		The driver will split longer messages before they are
+		sent to the device. The userspace application must
+		ensure proper message fragmention if supported by the
+		protocol in use, and should be prepared to receive
+		messages up to this size in a single read operation.
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 5f0cb41..73690d7 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -667,6 +667,18 @@ static void wdm_rxwork(struct work_struct *work)
 	}
 }
 
+static ssize_t show_msgsize(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev->parent);
+	struct wdm_device *wdm = wdm_find_device(intf);
+
+	if (!wdm)
+		return -ENODEV;
+	return sprintf(buf, "%u\n", wdm->wMaxCommand);
+}
+static DEVICE_ATTR(msgsize, S_IRUGO, show_msgsize, NULL);
+
 /* --- hotplug --- */
 
 static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor *ep,
@@ -766,6 +778,7 @@ static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor
 		goto err;
 	else
 		dev_info(&intf->dev, "%s: USB WDM device\n", dev_name(intf->usb_dev));
+	device_create_file(intf->usb_dev, &dev_attr_msgsize);
 out:
 	return rv;
 err:
@@ -879,6 +892,7 @@ static void wdm_disconnect(struct usb_interface *intf)
 	struct wdm_device *desc;
 	unsigned long flags;
 
+	device_remove_file(intf->usb_dev, &dev_attr_msgsize);
 	usb_deregister_dev(intf, &wdm_class);
 	desc = wdm_find_device(intf);
 	mutex_lock(&wdm_mutex);
-- 
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