> diff -x sysfs.c -uNr linux-2.6.8-1.521/drivers/usb/class/cdc-acm.h
> linux-2.6.8-1.521-ak/drivers/usb/class/cdc-acm.h
> --- linux-2.6.8-1.521/drivers/usb/class/cdc-acm.h�������2004-08-14
> 08:38:09.000000000 +0300
> +++ linux-2.6.8-1.521-ak/drivers/usb/class/cdc-acm.h����2004-09-22
> 01:07:03.000000000 +0300
> @@ -117,6 +117,7 @@
> �} __attribute__ ((packed));
> �
> �/* class specific descriptor types */
> +#define CDC_HEADER_TYPE������������������������0x00
> �#define CDC_CALL_MANAGEMENT_TYPE�������0x01
> �#define CDC_AC_MANAGEMENT_TYPE���������0x02
> �#define CDC_UNION_TYPE�����������������0x06
Could you split cdc-acm.h into the part defining the spec and the headers
of cdc-acm.c?
> +/* table of devices that work with this driver */
> +static struct usb_device_id obex_table [] = {
> +�������{ USB_INTERFACE_INFO(USB_CLASS_COMM, 11, 0) },
Should be a symbolic constant, not 11
> +/* Structure to hold all of our device specific stuff */
> +struct usb_obex {
> +�������struct usb_device *�����udev;�������������������/* save off the usb device
> pointer */
> +�������struct usb_interface *��interface;��������������/* the interface for this
> device */
> +�������struct usb_interface * �data_interface;�����������������/* the data
> interface for this device */
> +�������unsigned char�����������minor;������������������/* the starting minor number
> for this device */
use int
> +�������unsigned char�����������num_ports;��������������/* the number of ports this
> device has */
> +�������char��������������������num_interrupt_in;�������/* number of interrupt in
> endpoints we have */
> +�������char��������������������num_bulk_in;������������/* number of bulk in
> endpoints we have */
> +�������char��������������������num_bulk_out;�����������/* number of bulk out
> endpoints we have */
> +
> +�������unsigned char *���������bulk_in_buffer;���������/* the buffer to receive
> data */
u8 *
> +static inline int obex_open_data_interface (struct usb_obex *dev)
> +{
> +�������int i;
> +�������int retval = -ENOMEM;
> +�������struct usb_host_interface *iface_desc;
> + � � � �struct usb_endpoint_descriptor *endpoint;
> + � � � �size_t buffer_size;
> + � � � �struct usb_device *udev = interface_to_usbdev(dev->data_interface);
> +
> +�������/* choose correct interface setting */
> +�������for (i = 0; i < dev->data_interface->num_altsetting; i++) {
> +���������������iface_desc = &dev->data_interface->altsetting[i];
> +���������������if (iface_desc->desc.bNumEndpoints == 2)
> +�����������������������goto found;
Don't do this. You should do this in probe(), do a sanity check and
store the numerical values.
> +�������usb_fill_bulk_urb(dev->write_urb, udev, usb_sndbulkpipe(udev,
> dev->bulk_out_endpointAddr),
> +����������������������� �dev->bulk_out_buffer, dev->bulk_out_size,
> obex_write_bulk_callback, dev);
> +�������dev->write_urb->transfer_flags |= URB_ASYNC_UNLINK | URB_NO_TRANSFER_DMA_MAP;
why async?
> + *�����obex_write
> + *
> + *�����This writes the data to a device. The typical packet size on a bulk
> + * ����endpoint is rather small, 64 bytes or something like that, so we're
> + * ����writing all of user-supplied data in a loop, and not just the
> + * ����first chunk. Another reason is that openobex userspace library expects
> + *�����the transport to support packets sizes defined by the OBEX standard,
> + *�����and the minimum which the device has to support is 255 bytes.
> + *�����If you need to transfer large items, then CONNECT response from a
> + *�����device contains the maximum packet size, use it for your transfers.
> + *�����No double-buffering to keep things simple.
> + *
> + *�����A device driver has to decide how to report I/O errors back to the
> + *�����user. �The safest course is to wait for the transfer to finish before
> + *�����returning so that any errors will be reported reliably. �obex_read()
> + *�����works like this. �But waiting for I/O is slow, so many drivers only
> + *�����check for errors during I/O initiation and do not report problems
> + *�����that occur during the actual transfer. �That's what we will do here.
> + *
> + */
> +static ssize_t obex_write (struct file *file, const char *buffer, size_t count,
> loff_t *ppos)
> +{
> +�������struct usb_obex *dev;
> +�������ssize_t bytes_written = 0;
> +�������ssize_t bytes_to_write = 0;
> +�������int retval = 0;
> +
> +�������dev = (struct usb_obex *)file->private_data;
> +
> +�������dbg("%s - minor %d, count = %d", __FUNCTION__, dev->minor, count);
> +
> +�������/* lock this object */
> +�������down (&dev->sem);
If you can not give an upper bound below use down_interruptible()
> +
> +�������/* verify that the device wasn't unplugged */
> +�������if (!dev->present || !dev->open) {
> +���������������retval = -ENODEV;
> +���������������goto exit;
> +�������}
> +
> +�������/* verify that we actually have some data to write */
> +�������if (count == 0) {
> +���������������dbg("%s - write request of 0 bytes", __FUNCTION__);
> +���������������goto exit;
> +�������}
> +
> +�������while (bytes_written < count) {
> +���������������/* wait for a previous write to finish up; we don't use a timeout
> +��������������� * and so a nonresponsive device can delay us indefinitely.
> +��������������� */
> +���������������if (atomic_read (&dev->write_busy))
> +�����������������������wait_for_completion (&dev->write_finished);
> +
> +���������������/* we can only write as much as our buffer will hold */
> +���������������bytes_to_write = min (dev->bulk_out_size, count - bytes_written);
> +
> +���������������/* copy the data from userspace into our transfer buffer;
> +��������������� * this is the only copy required.
> +��������������� */
> +���������������if (copy_from_user(dev->write_urb->transfer_buffer,
> buffer+bytes_written,
> +������������������������������� � �bytes_to_write)) {
> +�����������������������retval = -EFAULT;
> +�����������������������goto exit;
> +���������������}
> +���������������bytes_written += bytes_to_write;
> +
> +���������������usb_obex_debug_data (__FUNCTION__, bytes_written,
> +������������������������������� � � dev->write_urb->transfer_buffer);
> +
> +���������������/* this urb was already set up, except for this write size */
> +���������������dev->write_urb->transfer_buffer_length = bytes_to_write;
> +
> +���������������/* send the data out the bulk port */
> +���������������/* a character device write uses GFP_KERNEL,
> +��������������� � unless a spinlock is held */
> +���������������init_completion (&dev->write_finished);
> +���������������atomic_set (&dev->write_busy, 1);
> +���������������retval = usb_submit_urb(dev->write_urb, GFP_KERNEL);
> +���������������if (retval) {
> +�����������������������atomic_set (&dev->write_busy, 0);
> +�����������������������err("%s - failed submitting write urb, error %d",
> +����������������������� � �__FUNCTION__, retval);
> +�����������������������goto exit;
Incorrect. If you have written data, you must report that.
> +���������������} else {
> +�����������������������retval = bytes_written;
> +���������������}
> +�������}
> +exit:
> +�������/* unlock the device */
> +�������up (&dev->sem);
> +
> +�������return retval;
> +}
> +
> +
> +/**
> + *�����obex_write_bulk_callback
> + *�����
> + *�����Writes are asynchronous, so this function is called when they're over.
> + */
> +static void obex_write_bulk_callback (struct urb *urb, struct pt_regs *regs)
> +{
> +�������struct usb_obex *dev = (struct usb_obex *)urb->context;
> +
> +�������dbg("%s - minor %d", __FUNCTION__, dev->minor);
> +
> +�������/* sync/async unlink faults aren't errors */
> +�������if (urb->status && !(urb->status == -ENOENT ||
> +�������������������������������urb->status == -ECONNRESET)) {
> +���������������dbg("%s - nonzero write bulk status received: %d",
> +��������������� � �__FUNCTION__, urb->status);
> +�������}
> +
> +�������/* notify anyone waiting that the write has finished */
> +�������atomic_set (&dev->write_busy, 0);
> +�������complete (&dev->write_finished);
This is a race. You can race complete() and init_completion().
> +}
> +
> +
> +/**
> + *�����obex_probe
> + *
> + *�����Called by the usb core when a new device is connected that it thinks
> + *�����this driver might be interested in. CDC OBEX devices have two
> + *�����interfaces: a master interface which is basically useless except it
> + *�����can contain a textual description of itself, and a slave interface
> + * ����which contains the actual data endpoints. Here, we search for the
> + *�����latter, but don't set it up until the device file is actually opened.
> + */
> +static int obex_probe(struct usb_interface *interface, const struct usb_device_id
> *id)
> +{
> + � � � �struct union_desc *union_header = NULL;
> + � � � �char *buffer = interface->altsetting->extra;
> + � � � �int buflen = interface->altsetting->extralen;
> +�������struct usb_device *udev = interface_to_usbdev(interface);
> +�������struct usb_obex *dev = NULL;
> + � � � �struct usb_interface *data_interface;
> + � � � �int data_interface_num;
> + � � � �int retval = -ENOMEM;
> +
> +�������/* allocate memory for our device state and initialize it */
> +�������dev = kmalloc (sizeof(struct usb_obex), GFP_KERNEL);
> +�������if (dev == NULL) {
> +���������������err ("Out of memory");
> +���������������return -ENOMEM;
> +�������}
> +�������memset (dev, 0x00, sizeof (*dev));
> +
> +�������init_MUTEX (&dev->sem);
> +�������dev->udev = udev;
> +�������dev->interface = interface;
> +
> + � � � �if (!buffer) {
> + � � � � � � � �err("Weird descriptor references");
> + � � � � � � � �return -EINVAL;
> + � � � �}
> +
> + � � � �while (buflen > 0) {
> + � � � � � � � �if (buffer [1] != USB_DT_CS_INTERFACE) {
> + � � � � � � � � � � � �err("skipping garbage");
> + � � � � � � � � � � � �goto next_desc;
> + � � � � � � � �}
> +
> + � � � � � � � �switch (buffer [2]) {
> + � � � � � � � � � � � �case CDC_UNION_TYPE: /* we've found it */
> + � � � � � � � � � � � � � � � �if (union_header) {
> + � � � � � � � � � � � � � � � � � � � �err("More than one union descriptor,
> skipping ...");
> + � � � � � � � � � � � � � � � � � � � �goto next_desc;
> + � � � � � � � � � � � � � � � �}
> + � � � � � � � � � � � � � � � �union_header = (struct union_desc *)buffer;
> + � � � � � � � � � � � � � � � �break;
> + � � � � � � � � � � � �case CDC_OBEX_TYPE: /* maybe check version */
> +�����������������������case CDC_HEADER_TYPE:
> + � � � � � � � � � � � � � � � �break; /* for now we ignore it */
> + � � � � � � � � � � � �default:
> + � � � � � � � � � � � � � � � �err("Ignoring extra header, type %d, length %d",
> buffer[2], buffer[0]);
> + � � � � � � � � � � � � � � � �break;
> + � � � � � � � � � � � �}
> +next_desc:
> + � � � � � � � �buflen -= buffer[0];
> + � � � � � � � �buffer += buffer[0];
> + � � � �}
> +
> + � � � �if (!union_header) {
> + � � � � � � � �dev_dbg(&interface->dev,"No union descriptor, giving up\n");
> + � � � � � � � �return -ENODEV;
> + � � � �}
> +�������/* Found the slave interface */
> +�������data_interface = usb_ifnum_to_if(udev, (data_interface_num =
> union_header->bSlaveInterface0));
> +
> + � � � �if (!data_interface) {
> + � � � � � � � �dev_dbg(&interface->dev,"no interfaces\n");
> + � � � � � � � �return -ENODEV;
> + � � � �}
> +
> + � � � �if (usb_interface_claimed(data_interface)) { /* valid in this context */
> + � � � � � � � �dev_dbg(&interface->dev,"The data interface isn't available\n");
> + � � � � � � � �return -EBUSY;
> + � � � �}
> +
> +�������dev->data_interface = data_interface;
> +�������/* we can register the device now, as it is ready */
> +�������usb_driver_claim_interface(&obex_driver, data_interface, dev);
> +�������usb_set_intfdata (interface, dev);
> +�������retval = usb_register_dev (interface, &obex_class);
> +�������if (retval) {
> +���������������/* something prevented us from registering this driver */
> +���������������err ("Not able to get a minor for this device.");
> +���������������usb_set_intfdata (interface, NULL);
> +���������������goto error;
Resource leak in error case. unclaim the second interface and free the
memory.
Regards
Oliver