On Thu, Apr 15, 2004 at 05:29:37PM -0700, David Brownell wrote:
> Greg KH wrote:
> >On Thu, Apr 15, 2004 at 10:10:00AM -0400, Alan Stern wrote:
> >
> >>Greg:
> >>
> >>I just noticed your new usb_get/put_intf() functions. Creating them is a
> >>tacit admission that drivers will need to continue using interfaces after
> >>their disconnect() routine has returned. Is this wise?
>
> Sysfs may hold references; it's not just (or mostly) drivers ...
Exactly.
> >>The current assumption in usbcore is that when disconnect() returns the
> >>driver will have relinquished its interface and no URBs will be active.
> >>The core is then free to bind another driver to that interface. If the
> >>old driver is still using the interface, it's liable to cause problems.
> >
> >
> >Yes, but we can't guarantee that the driver is done with the device, or
> >the interface after disconnect() is over. And we don't want to do that
> >either, otherwise it makes the driver's lives a big pain.
>
> That doesn't seem quite right. Here's what I thought the (new) story was
> going to be:
>
> - Most drivers rely on the implicit claim that came from probe() success,
> which is implicitly released when the disconnect() returns. So those
> drivers have a simple rule: they can't have (or use) any pointers to
> that interface after disconnect() returns.
And that causes a lot of gyrations in the driver to ensure it. Look at
the mess of locks in something as simple as usb-skeleton.c. Ugly and a
pain in the butt to debug correctly. Look how long it took us to get it
right.
> - After disconnect(), the driver could keep an interface reference
> IF (!!) it explicitly refcounting it ... which wasn't possible
> before this patch, though device refcounting could until recently
> be a proxy for it.
Well, drivers currently use usb_get_dev() to hold this reference today.
And they could use usb_get_intf() instead now.
> And the current story for trying to talk to the device after disconnect()
> wouldn't change: it's not allowed.
Um no. It should fail. It will happen, and is allowed. This works
correctly today, and we can not break this.
> Although when disconnect() happens because of physical device
> disconnect (instead of config change, or other disconnect/rebind
> cases), such requests can/do fail cleanly.
Yeah, I hate the "rebind" nonsense... bleah.
> >So, if you really want to give an interface to another driver, we need
> >to just unregister it and create a new one. That way it will all "just
> >work" properly.
>
> Yes ... but that if the first driver were still allowed to talk to the
> device after disconnect() returns, then it couldn't work "properly".
We need to mark the interface as "gone" so that the usb core knows this
and doesn't allow any future accesses.
> Anything it did could easily stomp all over the second driver's work.
Without such a flag, yes. That's why we need to do that.
> >The reason I created those functions is we never completed the usb
> >device -> usb interface conversion fully during 2.5. Drivers should not
> >care at all about struct usb_device, which is not quite true today (we
> >still need it for initializing urbs, but that's it.) Also, with your
> >upcoming changes for making the interfaces individual structures,
> >drivers can grab a reference to the interface instead of grabbing the
> >reference to the device (like they do today.)
>
> When development starts on 2.7, one possibility would be to submit
> urbs directly to endpoints. Endpoints are logical children of the
> interfaces, as well as the real targets of the urbs, so they could
> be cleanly disabled as part of interface disconnect() processing.
Yes, that would be nice to do.
Oh, I've rewritten the usb-skeleton driver so that it contains no locks
now (with the exception of the BKL to protect on disconnect.) It's a
world simpler, but I've probably forgotten something here. I've
attached it below if anyone wants to look at it.
thanks,
greg k-h
------------------
/*
* USB Skeleton driver - 2.0
*
* Copyright (C) 2001-2004 Greg Kroah-Hartman ([EMAIL PROTECTED])
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License as
* published by the Free Software Foundation, version 2.
*
* This driver is based on the 2.6.3 version of drivers/usb/usb-skeleton.c
* but has been rewritten to be easy to read and use, as no locks are now
* needed anymore.
*
*/
#include <linux/config.h>
#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/kref.h>
#include <asm/uaccess.h>
#include <linux/usb.h>
/* Define these values to match your devices */
#define USB_SKEL_VENDOR_ID 0xfff0
#define USB_SKEL_PRODUCT_ID 0xfff0
/* table of devices that work with this driver */
static struct usb_device_id skel_table [] = {
{ USB_DEVICE(USB_SKEL_VENDOR_ID, USB_SKEL_PRODUCT_ID) },
{ } /* Terminating entry */
};
MODULE_DEVICE_TABLE (usb, skel_table);
/* Get a minor range for your devices from the usb maintainer */
#define USB_SKEL_MINOR_BASE 192
/* Structure to hold all of our device specific stuff */
struct usb_skel {
struct usb_device * udev; /* the usb device for this
device */
struct usb_interface * interface; /* the interface for this
device */
unsigned char * bulk_in_buffer; /* the buffer to receive data
*/
size_t bulk_in_size; /* the size of the receive
buffer */
__u8 bulk_in_endpointAddr; /* the address of the bulk in
endpoint */
__u8 bulk_out_endpointAddr; /* the address of the bulk out
endpoint */
struct kref kref;
};
#define to_skel_dev(d) container_of(d, struct usb_skel, kref)
static struct usb_driver skel_driver;
static void skel_delete(struct kref *kref)
{
struct usb_skel *dev = to_skel_dev(kref);
usb_put_dev(dev->udev);
kfree (dev->bulk_in_buffer);
kfree (dev);
}
static int skel_open(struct inode *inode, struct file *file)
{
struct usb_skel *dev;
struct usb_interface *interface;
int subminor;
int retval = 0;
subminor = iminor(inode);
interface = usb_find_interface(&skel_driver, subminor);
if (!interface) {
err ("%s - error, can't find device for minor %d",
__FUNCTION__, subminor);
retval = -ENODEV;
goto exit;
}
dev = usb_get_intfdata(interface);
if (!dev) {
retval = -ENODEV;
goto exit;
}
/* increment our usage count for the device */
kref_get(&dev->kref);
/* save our object in the file's private structure */
file->private_data = dev;
exit:
return retval;
}
static int skel_release(struct inode *inode, struct file *file)
{
struct usb_skel *dev;
dev = (struct usb_skel *)file->private_data;
if (dev == NULL)
return -ENODEV;
/* decrement the count on our device */
kref_put(&dev->kref);
return 0;
}
static ssize_t skel_read(struct file *file, char *buffer, size_t count, loff_t *ppos)
{
struct usb_skel *dev;
int retval = 0;
dev = (struct usb_skel *)file->private_data;
/* do a blocking bulk read to get data from the device */
retval = usb_bulk_msg(dev->udev,
usb_rcvbulkpipe(dev->udev, dev->bulk_in_endpointAddr),
dev->bulk_in_buffer,
min(dev->bulk_in_size, count),
&count, HZ*10);
/* if the read was successful, copy the data to userspace */
if (!retval) {
if (copy_to_user(buffer, dev->bulk_in_buffer, count))
retval = -EFAULT;
else
retval = count;
}
return retval;
}
static void skel_write_bulk_callback(struct urb *urb, struct pt_regs *regs)
{
struct usb_skel *dev;
dev = (struct usb_skel *)urb->context;
/* 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);
}
/* free up our allocated buffer */
usb_buffer_free(urb->dev, urb->transfer_buffer_length,
urb->transfer_buffer, urb->transfer_dma);
}
static ssize_t skel_write(struct file *file, const char *user_buffer, size_t count,
loff_t *ppos)
{
struct usb_skel *dev;
int retval = 0;
struct urb *urb = NULL;
char *buf = NULL;
dev = (struct usb_skel *)file->private_data;
/* verify that we actually have some data to write */
if (count == 0)
goto exit;
/* create a urb, and a buffer for it, and copy the data to the urb */
urb = usb_alloc_urb(0, GFP_KERNEL);
if (!urb) {
retval = -ENOMEM;
goto error;
}
buf = usb_buffer_alloc(dev->udev, count, GFP_KERNEL, &urb->transfer_dma);
if (!buf) {
retval = -ENOMEM;
goto error;
}
if (copy_from_user(buf, user_buffer, count)) {
retval = -EFAULT;
goto error;
}
/* initialize the urb properly */
usb_fill_bulk_urb(urb, dev->udev,
usb_sndbulkpipe(dev->udev, dev->bulk_out_endpointAddr),
buf, count, skel_write_bulk_callback, dev);
urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
/* send the data out the bulk port */
retval = usb_submit_urb(urb, GFP_KERNEL);
if (retval) {
err("%s - failed submitting write urb, error %d", __FUNCTION__,
retval);
goto error;
}
/* release our reference to this urb, the USB core will eventually free it
entirely */
usb_free_urb(urb);
exit:
return count;
error:
usb_free_urb(urb);
kfree(buf);
return retval;
}
static struct file_operations skel_fops = {
.owner = THIS_MODULE,
.read = skel_read,
.write = skel_write,
.open = skel_open,
.release = skel_release,
};
/*
* usb class driver info in order to get a minor number from the usb core,
* and to have the device registered with devfs and the driver core
*/
static struct usb_class_driver skel_class = {
.name = "usb/skel%d",
.fops = &skel_fops,
.mode = S_IFCHR | S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH,
.minor_base = USB_SKEL_MINOR_BASE,
};
static int skel_probe(struct usb_interface *interface, const struct usb_device_id *id)
{
struct usb_skel *dev = NULL;
struct usb_host_interface *iface_desc;
struct usb_endpoint_descriptor *endpoint;
size_t buffer_size;
int i;
int retval = -ENOMEM;
/* allocate memory for our device state and initialize it */
dev = kmalloc(sizeof(struct usb_skel), GFP_KERNEL);
if (dev == NULL) {
err("Out of memory");
goto error;
}
memset(dev, 0x00, sizeof (*dev));
kref_init(&dev->kref, skel_delete);
dev->udev = usb_get_dev(interface_to_usbdev(interface));
dev->interface = interface;
/* set up the endpoint information */
/* use only the first bulk-in and bulk-out endpoints */
iface_desc = interface->cur_altsetting;
for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
endpoint = &iface_desc->endpoint[i].desc;
if (!dev->bulk_in_endpointAddr &&
(endpoint->bEndpointAddress & USB_DIR_IN) &&
((endpoint->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
== USB_ENDPOINT_XFER_BULK)) {
/* we found a bulk in endpoint */
buffer_size = endpoint->wMaxPacketSize;
dev->bulk_in_size = buffer_size;
dev->bulk_in_endpointAddr = endpoint->bEndpointAddress;
dev->bulk_in_buffer = kmalloc(buffer_size, GFP_KERNEL);
if (!dev->bulk_in_buffer) {
err("Couldn't allocate bulk_in_buffer");
goto error;
}
}
if (!dev->bulk_out_endpointAddr &&
!(endpoint->bEndpointAddress & USB_DIR_IN) &&
((endpoint->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
== USB_ENDPOINT_XFER_BULK)) {
/* we found a bulk out endpoint */
dev->bulk_out_endpointAddr = endpoint->bEndpointAddress;
}
}
if (!(dev->bulk_in_endpointAddr && dev->bulk_out_endpointAddr)) {
err("Couldn't find both bulk-in and bulk-out endpoints");
goto error;
}
/* we can register the device now, as it is ready */
usb_set_intfdata(interface, dev);
retval = usb_register_dev(interface, &skel_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;
}
/* let the user know what node this device is now attached to */
info ("USB Skeleton device now attached to USBSkel-%d", interface->minor);
return 0;
error:
if (dev)
kref_put(&dev->kref);
return retval;
}
static void skel_disconnect(struct usb_interface *interface)
{
struct usb_skel *dev;
int minor = interface->minor;
/* prevent skel_open() from racing skel_disconnect() */
lock_kernel();
dev = usb_get_intfdata(interface);
usb_set_intfdata(interface, NULL);
/* give back our minor */
usb_deregister_dev(interface, &skel_class);
unlock_kernel();
/* decrement our usage count */
kref_put(&dev->kref);
info("USB Skeleton #%d now disconnected", minor);
}
static struct usb_driver skel_driver = {
.owner = THIS_MODULE,
.name = "skeleton",
.probe = skel_probe,
.disconnect = skel_disconnect,
.id_table = skel_table,
};
static int __init usb_skel_init(void)
{
int result;
/* register this driver with the USB subsystem */
result = usb_register(&skel_driver);
if (result)
err("usb_register failed. Error number %d", result);
return 0;
}
static void __exit usb_skel_exit(void)
{
/* deregister this driver with the USB subsystem */
usb_deregister(&skel_driver);
}
module_init (usb_skel_init);
module_exit (usb_skel_exit);
MODULE_LICENSE("GPL");
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
