On Mon, 8 Nov 2004, David Brownell wrote:
> > Most of the areas of conflict are places where I converted, for example,
> > ohci->hcd into ohci_to_hcd(ohci).
>
> I guess I'd need to see that patch. I was kind of hoping
> to hold off on cleanups in favor of bugfixes for a while,
> at least inside usbcore. We're still shaking out the
> consequences of merging all those other changes ...
You asked for it! :-)
And remember that this is a cleanup Greg asked to see soon...
Attached are just two of the patches: one for the core and one for the
UHCI driver. The OHCI patch is very similar, just with lots more places
to change, so it's not attached here. The changes have the same form:
replace ohci->hcd with hcd (if it's available) or with ohci_to_hcd(ohci).
If you want to see the complete patch I will send it separately (it's a
bit long).
There are three partially-related but still interesting aspects to the
core patch:
hcd->description is removed since we can simply use
hcd->driver->description instead.
hcd->state is explicitly initialized to USB_STATE_HALT when the
hcd structure is created.
usb_bus->class_device is initialized when the bus is, and
then added (not registered) when the bus is registered.
Alan Stern
===== drivers/usb/core/hcd-pci.c 1.68 vs edited =====
--- 1.68/drivers/usb/core/hcd-pci.c 2004-11-05 13:06:30 -05:00
+++ edited/drivers/usb/core/hcd-pci.c 2004-11-08 16:11:41 -05:00
@@ -120,7 +120,7 @@
// driver->reset(), later on, will transfer device from
// control by SMM/BIOS to control by Linux (if needed)
- hcd = driver->hcd_alloc ();
+ hcd = usb_create_hcd (driver);
if (hcd == NULL){
dev_dbg (&dev->dev, "hcd alloc fail\n");
retval = -ENOMEM;
@@ -140,20 +140,16 @@
hcd->region = region;
pci_set_drvdata (dev, hcd);
- hcd->driver = driver;
- hcd->description = driver->description;
hcd->self.bus_name = pci_name(dev);
#ifdef CONFIG_PCI_NAMES
hcd->product_desc = dev->pretty_name;
-#else
- if (hcd->product_desc == NULL)
- hcd->product_desc = "USB Host Controller";
#endif
hcd->self.controller = &dev->dev;
if ((retval = hcd_buffer_create (hcd)) != 0) {
clean_3:
- kfree (hcd);
+ pci_set_drvdata (dev, NULL);
+ usb_put_hcd (hcd);
goto clean_2;
}
@@ -164,7 +160,6 @@
dev_err (hcd->self.controller, "can't reset\n");
goto clean_3;
}
- hcd->state = USB_STATE_HALT;
pci_set_master (dev);
#ifndef __sparc__
@@ -173,7 +168,7 @@
bufp = __irq_itoa(dev->irq);
#endif
retval = request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ,
- hcd->description, hcd);
+ hcd->driver->description, hcd);
if (retval != 0) {
dev_err (hcd->self.controller,
"request interrupt %s failed\n", bufp);
@@ -185,14 +180,6 @@
(driver->flags & HCD_MEMORY) ? "pci mem" : "io base",
resource);
- usb_bus_init (&hcd->self);
- hcd->self.op = &usb_hcd_operations;
- hcd->self.release = &usb_hcd_release;
- hcd->self.hcpriv = (void *) hcd;
- init_timer (&hcd->rh_timer);
-
- INIT_LIST_HEAD (&hcd->dev_list);
-
usb_register_bus (&hcd->self);
if ((retval = driver->start (hcd)) < 0) {
@@ -408,7 +395,7 @@
pci_set_power_state (dev, 0);
dev->dev.power.power_state = 0;
retval = request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ,
- hcd->description, hcd);
+ hcd->driver->description, hcd);
if (retval < 0) {
dev_err (hcd->self.controller,
"can't restore IRQ after resume!\n");
===== drivers/usb/core/hcd.c 1.166 vs edited =====
--- 1.166/drivers/usb/core/hcd.c 2004-11-05 13:06:30 -05:00
+++ edited/drivers/usb/core/hcd.c 2004-11-08 16:11:41 -05:00
@@ -312,7 +312,7 @@
// id 3 == vendor description
} else if (id == 3) {
sprintf (buf, "%s %s %s", UTS_SYSNAME, UTS_RELEASE,
- hcd->description);
+ hcd->driver->description);
// unsupported IDs --> "protocol stall"
} else
@@ -674,6 +674,8 @@
bus->bandwidth_isoc_reqs = 0;
INIT_LIST_HEAD (&bus->bus_list);
+
+ class_device_initialize(&bus->class_dev);
}
EXPORT_SYMBOL (usb_bus_init);
@@ -732,7 +734,7 @@
snprintf(bus->class_dev.class_id, BUS_ID_SIZE, "usb%d", busnum);
bus->class_dev.class = &usb_host_class;
bus->class_dev.dev = bus->controller;
- retval = class_device_register(&bus->class_dev);
+ retval = class_device_add(&bus->class_dev);
if (retval) {
clear_bit(busnum, busmap.busmap);
up(&usb_bus_list_lock);
@@ -1503,12 +1505,8 @@
/*
* usb_hcd_operations - adapts usb_bus framework to HCD framework (bus glue)
- *
- * When registering a USB bus through the HCD framework code, use this
- * usb_operations vector. The PCI glue layer does so automatically; only
- * bus glue for non-PCI system busses will need to use this.
*/
-struct usb_operations usb_hcd_operations = {
+static struct usb_operations usb_hcd_operations = {
.allocate = hcd_alloc_dev,
.get_frame_number = hcd_get_frame_number,
.submit_urb = hcd_submit_urb,
@@ -1522,7 +1520,6 @@
.hub_resume = hcd_hub_resume,
#endif
};
-EXPORT_SYMBOL (usb_hcd_operations);
/*-------------------------------------------------------------------------*/
@@ -1624,11 +1621,52 @@
/*-------------------------------------------------------------------------*/
-void usb_hcd_release(struct usb_bus *bus)
+static void hcd_release (struct usb_bus *bus)
{
struct usb_hcd *hcd;
- hcd = container_of (bus, struct usb_hcd, self);
+ hcd = container_of(bus, struct usb_hcd, self);
kfree(hcd);
}
-EXPORT_SYMBOL (usb_hcd_release);
+
+/**
+ * usb_create_hcd - create and initialize an HCD structure
+ * @driver: HC driver that will use this hcd
+ * Context: !in_interrupt()
+ *
+ * Allocate a struct usb_hcd, with extra space at the end for the
+ * HC driver's private data. Initialize the generic members of the
+ * hcd structure.
+ *
+ * If memory is unavailable, returns NULL.
+ */
+struct usb_hcd *usb_create_hcd (const struct hc_driver *driver)
+{
+ struct usb_hcd *hcd;
+
+ hcd = kcalloc(1, sizeof(*hcd) + driver->hcd_priv_size, GFP_KERNEL);
+ if (!hcd)
+ return NULL;
+
+ usb_bus_init(&hcd->self);
+ hcd->self.op = &usb_hcd_operations;
+ hcd->self.hcpriv = hcd;
+ hcd->self.release = &hcd_release;
+
+ init_timer(&hcd->rh_timer);
+ INIT_LIST_HEAD(&hcd->dev_list);
+
+ hcd->driver = driver;
+ hcd->product_desc = (driver->product_desc) ? driver->product_desc :
+ "USB Host Controller";
+ hcd->state = USB_STATE_HALT;
+
+ return hcd;
+}
+EXPORT_SYMBOL (usb_create_hcd);
+
+void usb_put_hcd (struct usb_hcd *hcd)
+{
+ usb_bus_put(&hcd->self);
+}
+EXPORT_SYMBOL (usb_put_hcd);
===== drivers/usb/core/hcd.h 1.95 vs edited =====
--- 1.95/drivers/usb/core/hcd.h 2004-11-05 13:06:30 -05:00
+++ edited/drivers/usb/core/hcd.h 2004-11-08 16:11:41 -05:00
@@ -63,7 +63,6 @@
struct usb_bus self; /* hcd is-a bus */
const char *product_desc; /* product/vendor string */
- const char *description; /* "ehci-hcd" etc */
struct timer_list rh_timer; /* drives root hub */
struct list_head dev_list; /* devices on this bus */
@@ -71,7 +70,7 @@
/*
* hardware info/state
*/
- struct hc_driver *driver; /* hw-specific hooks */
+ const struct hc_driver *driver; /* hw-specific hooks */
unsigned saw_irq : 1;
unsigned can_wakeup:1; /* hw supports wakeup? */
unsigned remote_wakeup:1;/* sw should use wakeup? */
@@ -104,6 +103,12 @@
* input size of periodic table to an interrupt scheduler.
* (ohci 32, uhci 1024, ehci 256/512/1024).
*/
+
+ /* The HC driver's private data is stored at the end of
+ * this structure.
+ */
+ unsigned long hcd_priv[0]
+ __attribute__ ((aligned (sizeof(unsigned long))));
};
/* 2.4 does this a bit differently ... */
@@ -162,6 +167,8 @@
struct hc_driver {
const char *description; /* "ehci-hcd" etc */
+ const char *product_desc; /* product/vendor string */
+ size_t hcd_priv_size; /* size of private data */
/* irq handler */
irqreturn_t (*irq) (struct usb_hcd *hcd, struct pt_regs *regs);
@@ -190,15 +197,6 @@
/* return current frame number */
int (*get_frame_number) (struct usb_hcd *hcd);
- /* memory lifecycle */
- /* Note: The absence of hcd_free reflects a temporary situation;
- * in the near future hcd_alloc will disappear as well and all
- * allocations/deallocations will be handled by usbcore. For the
- * moment, drivers are required to return a pointer that the core
- * can pass to kfree, i.e., the struct usb_hcd must be the _first_
- * member of a larger driver-specific structure. */
- struct usb_hcd *(*hcd_alloc) (void);
-
/* manage i/o requests, device state */
int (*urb_enqueue) (struct usb_hcd *hcd, struct urb *urb,
int mem_flags);
@@ -221,6 +219,10 @@
extern void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb, struct
pt_regs *regs);
extern void usb_bus_init (struct usb_bus *bus);
+extern struct usb_hcd *usb_create_hcd (const struct hc_driver *driver);
+extern void usb_put_hcd (struct usb_hcd *hcd);
+
+
#ifdef CONFIG_PCI
struct pci_dev;
struct pci_device_id;
@@ -245,7 +247,6 @@
void *addr, dma_addr_t dma);
/* generic bus glue, needed for host controllers that don't use PCI */
-extern struct usb_operations usb_hcd_operations;
extern irqreturn_t usb_hcd_irq (int irq, void *__hcd, struct pt_regs *r);
extern void usb_hc_died (struct usb_hcd *hcd);
@@ -364,8 +365,6 @@
return usb_register_root_hub (usb_dev, hcd->self.controller);
}
-
-extern void usb_hcd_release (struct usb_bus *);
extern void usb_set_device_state(struct usb_device *udev,
enum usb_device_state new_state);
===== drivers/usb/host/uhci-hcd.c 1.144 vs edited =====
--- 1.144/drivers/usb/host/uhci-hcd.c 2004-11-05 13:06:32 -05:00
+++ edited/drivers/usb/host/uhci-hcd.c 2004-11-08 16:12:15 -05:00
@@ -1886,7 +1886,7 @@
uhci->state_end = jiffies + HZ;
outw(USBCMD_RS | USBCMD_CF | USBCMD_MAXP, io_addr + USBCMD);
- uhci->hcd.state = USB_STATE_RUNNING;
+ uhci_to_hcd(uhci)->state = USB_STATE_RUNNING;
return 0;
}
@@ -1926,7 +1926,8 @@
#ifdef CONFIG_PROC_FS
if (uhci->proc_entry) {
- remove_proc_entry(uhci->hcd.self.bus_name, uhci_proc_root);
+ remove_proc_entry(uhci_to_hcd(uhci)->self.bus_name,
+ uhci_proc_root);
uhci->proc_entry = NULL;
}
#endif
@@ -2146,7 +2147,7 @@
udev->speed = USB_SPEED_FULL;
- if (hcd_register_root(udev, &uhci->hcd) != 0) {
+ if (hcd_register_root(udev, hcd) != 0) {
dev_err(uhci_dev(uhci), "unable to start root hub\n");
retval = -ENOMEM;
goto err_start_root_hub;
@@ -2272,24 +2273,11 @@
if ((rc = start_hc(uhci)) != 0)
return rc;
}
- uhci->hcd.state = USB_STATE_RUNNING;
+ hcd->state = USB_STATE_RUNNING;
return 0;
}
#endif
-static struct usb_hcd *uhci_hcd_alloc(void)
-{
- struct uhci_hcd *uhci;
-
- uhci = (struct uhci_hcd *)kmalloc(sizeof(*uhci), GFP_KERNEL);
- if (!uhci)
- return NULL;
-
- memset(uhci, 0, sizeof(*uhci));
- uhci->hcd.product_desc = "UHCI Host Controller";
- return &uhci->hcd;
-}
-
/* Are there any URBs for a particular device/endpoint on a given list? */
static int urbs_for_ep_list(struct list_head *head,
struct hcd_dev *hdev, int ep)
@@ -2339,6 +2327,8 @@
static const struct hc_driver uhci_driver = {
.description = hcd_name,
+ .product_desc = "UHCI Host Controller",
+ .hcd_priv_size = sizeof(struct uhci_hcd),
/* Generic hardware linkage */
.irq = uhci_irq,
@@ -2352,8 +2342,6 @@
.resume = uhci_resume,
#endif
.stop = uhci_stop,
-
- .hcd_alloc = uhci_hcd_alloc,
.urb_enqueue = uhci_urb_enqueue,
.urb_dequeue = uhci_urb_dequeue,
===== drivers/usb/host/uhci-hcd.h 1.41 vs edited =====
--- 1.41/drivers/usb/host/uhci-hcd.h 2004-11-05 13:06:32 -05:00
+++ edited/drivers/usb/host/uhci-hcd.h 2004-11-08 16:12:15 -05:00
@@ -314,9 +314,6 @@
UHCI_RESUMING_2
};
-#define hcd_to_uhci(hcd_ptr) container_of(hcd_ptr, struct uhci_hcd, hcd)
-#define uhci_dev(u) ((u)->hcd.self.controller)
-
/*
* This describes the full uhci information.
*
@@ -324,7 +321,6 @@
* a subset of what the full implementation needs.
*/
struct uhci_hcd {
- struct usb_hcd hcd; /* must come first! */
#ifdef CONFIG_PROC_FS
/* procfs */
@@ -382,6 +378,18 @@
wait_queue_head_t waitqh; /* endpoint_disable waiters */
};
+
+/* Convert between a usb_hcd pointer and the corresponding uhci_hcd */
+static inline struct uhci_hcd *hcd_to_uhci(struct usb_hcd *hcd)
+{
+ return (struct uhci_hcd *) (hcd->hcd_priv);
+}
+static inline struct usb_hcd *uhci_to_hcd(struct uhci_hcd *uhci)
+{
+ return container_of((void *) uhci, struct usb_hcd, hcd_priv);
+}
+
+#define uhci_dev(u) (uhci_to_hcd(u)->self.controller)
struct urb_priv {
struct list_head urb_list;
