On Fri, Jan 05, 2007 at 02:39:21PM +0100, Oliver Neukum wrote:
> +/* --- prototypes --- */
> +
> +static int __init wdm_init(void);
> +static void __exit wdm_exit(void);
> +
> +static int wdm_probe(struct usb_interface *intf, const struct usb_device_id 
> *id);
> +static void wdm_disconnect(struct usb_interface *intf);
> +
> +static ssize_t wdm_read(struct file *file, char __user *buffer, size_t 
> count, loff_t *ppos);
> +static ssize_t wdm_write(struct file *file, const char __user *buffer, 
> size_t count, loff_t *ppos);
> +static int wdm_release(struct inode *inode, struct file *file);
> +static int wdm_open(struct inode *inode, struct file *file);
> +static int wdm_flush (struct file * file, fl_owner_t id);
> +
> +static void wdm_out_callback (struct urb *urb);
> +static void wdm_in_callback (struct urb *urb);
> +
> +static void free_urbs(struct wdm_device *desc);
> +static void cleanup (struct wdm_device *desc);
> +static void kill_urbs (struct wdm_device *desc);
> +static int run_poll(struct wdm_device *desc, gfp_t gfp);

Are all of these prototypes really needed?

> +/* --- table of supported interfaces ---*/
> +
> +static struct usb_device_id wdm_ids[] = {
> +     {
> +             .match_flags = USB_DEVICE_ID_MATCH_INT_CLASS | 
> USB_DEVICE_ID_MATCH_INT_SUBCLASS,
> +             .bInterfaceClass = USB_CLASS_COMM,
> +             .bInterfaceSubClass = USB_CDC_SUBCLASS_DMM
> +     },
> +     { }
> +};
> +
> +MODULE_DEVICE_TABLE (usb, wdm_ids);
> +
> +#define WDM_MINOR_BASE 32

Where did you get this minor number from?  I think it conflicts with a
very old usb driver, but you still need to reserve a real number from me
:)

Oh, and this will not work with the dynamic minors for usb devices, you
need to fix that up.

> --- /dev/null 2005-03-19 23:01:40.000000000 +0100
> +++ drivers/usb/class/cdc-wdm.c       2007-01-05 12:52:33.000000000 +0100
> @@ -0,0 +1,576 @@
> +/* cdc-wdm.c
> +
> +This driver supports USB CDC WCM Device Management.
> +
> +Copyright (c) 2007 Oliver Neukum

Need an email and a license here.

> +
> +Some code taken from cdc-acm.c
> +*/
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/smp_lock.h>
> +#include <linux/mutex.h>
> +#include <asm/uaccess.h>
> +#include <linux/usb.h>
> +#include <linux/usb/cdc.h>
> +#include <asm/byteorder.h>
> +#include <asm/bitops.h>
> +#include <asm/unaligned.h>
> +
> +#include "cdc-wdm.h"

Do we really need a separate .h file for this driver?

> +#include "cdc-acm.h" /* for request types */
> +
> +#define DEBUG

This should not be needed due to the CONFIG_USB_DEBUG config option.

> +static ssize_t wdm_write(struct file *file, const char __user *buffer, 
> size_t count, loff_t *ppos)
> +{
> +     u8 *buf;
> +     int rv = -EMSGSIZE, r, we;
> +     unsigned long flags;
> +     struct wdm_device *desc = file->private_data;
> +     struct usb_ctrlrequest *req;
> +
> +     if (count > desc->wMaxCommand)
> +             count = desc->wMaxCommand;
> +
> +     spin_lock_irqsave(&desc->iuspin, flags);
> +     we = desc->werr;
> +     desc->werr = 0;
> +     spin_unlock_irqrestore(&desc->iuspin, flags);
> +     if (we < 0)
> +             return -EIO;
> +
> +     run_poll(desc, GFP_KERNEL); /* read responses right on so that the 
> output buffer not stall */
> +     r = mutex_lock_interruptible(&desc->wlock); /* concurrent writes */
> +     rv = -ERESTARTSYS;
> +     if (r) {
> +             goto outnl;
> +     }

Minor nit, no { } needed for one line if statements.  You do this in a
few other places.

> +
> +     r = wait_event_interruptible(desc->wait, !test_bit(WDM_IN_USE, 
> &desc->flags));
> +     if (r < 0) {
> +             goto out;
> +     }
> +
> +     if (test_bit(WDM_DISCONNECTING, &desc->flags)) {
> +             rv = -ENODEV;
> +             goto out;
> +     }
> +
> +     desc->outbuf = buf = kmalloc(count, GFP_KERNEL);
> +     if (!buf) {
> +             rv = -ENOMEM;
> +             goto out;
> +     }
> +
> +     r = copy_from_user(buf, buffer, count);
> +     if (r > 0) {
> +             kfree(buf);
> +             rv = -EFAULT;
> +             goto out;
> +     }
> +
> +     req = desc->orq;
> +     usb_fill_control_urb(
> +             desc->command,
> +             interface_to_usbdev(desc->intf),
> +             usb_sndctrlpipe(interface_to_usbdev(desc->intf), 0), /* using 
> common endpoint 0 */
> +             (unsigned char *)req,
> +             buf,
> +             count,
> +             wdm_out_callback,
> +             desc
> +     );
> +     req->bRequestType = USB_RT_ACM;
> +     req->bRequest = USB_CDC_SEND_ENCAPSULATED_COMMAND;
> +     req->wValue = 0;
> +     req->wIndex = desc->inum;
> +     req->wLength = cpu_to_le16(count);
> +     set_bit(WDM_IN_USE, &desc->flags);
> +
> +     rv = usb_submit_urb(desc->command, GFP_KERNEL);
> +     if (rv < 0) {
> +             kfree(buf);
> +             clear_bit(WDM_IN_USE, &desc->flags);
> +     } else {
> +             info("Tx URB has been submitted");
> +     }
> +out:
> +     mutex_unlock(&desc->wlock);
> +outnl:
> +     return rv < 0 ? rv : count;
> +}
> +
> +static ssize_t wdm_read(struct file *file, char __user *buffer, size_t 
> count, loff_t *ppos)
> +{
> +     int rv, cntr;
> +     unsigned long flags;
> +     struct wdm_device *desc = file->private_data;
> +
> +     rv = mutex_lock_interruptible(&desc->rlock); /*concurrent reads */
> +     if (rv < 0)
> +             return -ERESTARTSYS;
> +
> +     if (desc->length == 0) {
> +             desc->read = 0;
> +             run_poll(desc, GFP_KERNEL);
> +retry:
> +             rv = wait_event_interruptible(desc->wait, test_bit(WDM_READ, 
> &desc->flags));
> +             

Trailing whitespace :(

> +static void cleanup (struct wdm_device *desc)
> +{
> +     //usb_buffer_free(interface_to_usbdev(desc->intf), sizeof(struct 
> usb_cdc_notification), desc->sbuf, desc->shandle);
> +     //usb_buffer_free(interface_to_usbdev(desc->intf), desc->wMaxCommand, 
> desc->inbuf, desc->ihandle);
> +kfree(desc->sbuf);
> +kfree(desc->inbuf);
> +     kfree(desc->orq);
> +     kfree(desc->irq);
> +     kfree(desc->ubuf);
> +     free_urbs(desc);
> +     kfree(desc);
> +}

Odd indentation...

thanks,

greg k-h

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to