[PATCH] USB: isp116x-hcd: cleanup
The attached patch makes a cleanup of isp116x-hcd. Most of the volume of
the patch comes from 2 sources: moving the code around to get rid of a
few function prototypes and reworking register dumping functions/macros.
Among other things, switched over from using procfs to debugfs.
Cleanup. The following changes were made:
- Rework register dumping code so it can be used for dumping
to both syslog and debugfs.
- Switch from procfs to debugfs..
- Die gracefully on Unrecoverable Error interrupt.
- Fix memory leak in isp116x_urb_enqueue(), if HC happens to
die in a narrow time window.
- Fix a 'sparce' warning (unnecessary cast).
- Report Devices Removable for root hub ports by default
(was Devices Permanently Attached).
- Move bus suspend/resume functions down in code to get rid of
a few function prototypes.
- A number of one-line cleanups.
- Add an entry to MAINTAINERS.
Signed-off-by: Olav Kongas <ok@artecdesign.ee>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
MAINTAINERS | 6
drivers/usb/host/isp116x-hcd.c | 429 ++++++++++++++++-------------------------
drivers/usb/host/isp116x.h | 83 +++++--
3 files changed, 230 insertions(+), 288 deletions(-)
diff --git a/drivers/usb/host/isp116x-hcd.c b/drivers/usb/host/isp116x-hcd.c
index 5f56c4a..342cfad 100644
--- a/drivers/usb/host/isp116x-hcd.c
+++ b/drivers/usb/host/isp116x-hcd.c
@@ -55,19 +55,13 @@
/* enqueuing/finishing log of urbs */
//#define URB_TRACE
-#include <linux/config.h>
#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/kernel.h>
#include <linux/delay.h>
-#include <linux/ioport.h>
-#include <linux/sched.h>
-#include <linux/slab.h>
-#include <linux/smp_lock.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
#include <linux/errno.h>
#include <linux/init.h>
#include <linux/list.h>
-#include <linux/interrupt.h>
#include <linux/usb.h>
#include <linux/usb_isp116x.h>
#include <linux/platform_device.h>
@@ -77,14 +71,10 @@
#include <asm/system.h>
#include <asm/byteorder.h>
-#ifndef DEBUG
-# define STUB_DEBUG_FILE
-#endif
-
#include "../core/hcd.h"
#include "isp116x.h"
-#define DRIVER_VERSION "05 Aug 2005"
+#define DRIVER_VERSION "03 Nov 2005"
#define DRIVER_DESC "ISP116x USB Host Controller Driver"
MODULE_DESCRIPTION(DRIVER_DESC);
@@ -305,9 +295,8 @@
udev = urb->dev;
ptd = &ep->ptd;
cc = PTD_GET_CC(ptd);
-
- spin_lock(&urb->lock);
short_not_ok = 1;
+ spin_lock(&urb->lock);
/* Data underrun is special. For allowed underrun
we clear the error and continue as normal. For
@@ -420,7 +409,7 @@
ep->nextpid = 0;
break;
default:
- BUG_ON(1);
+ BUG();
}
spin_unlock(&urb->lock);
}
@@ -628,8 +617,12 @@
u32 intstat = isp116x_read_reg32(isp116x, HCINTSTAT);
isp116x_write_reg32(isp116x, HCINTSTAT, intstat);
if (intstat & HCINT_UE) {
- ERR("Unrecoverable error\n");
- /* What should we do here? Reset? */
+ ERR("Unrecoverable error, HC is dead!\n");
+ /* IRQ's are off, we do no DMA,
+ perfectly ready to die ... */
+ hcd->state = HC_STATE_HALT;
+ ret = IRQ_HANDLED;
+ goto done;
}
if (intstat & HCINT_RHSC)
/* When root hub or any of its ports is going
@@ -640,7 +633,6 @@
if (intstat & HCINT_RD) {
DBG("---- remote wakeup\n");
usb_hcd_resume_root_hub(hcd);
- ret = IRQ_HANDLED;
}
irqstat &= ~HCuPINT_OPR;
ret = IRQ_HANDLED;
@@ -651,6 +643,7 @@
}
isp116x_write_reg16(isp116x, HCuPINTENB, isp116x->irqenb);
+ done:
spin_unlock(&isp116x->lock);
return ret;
}
@@ -724,6 +717,7 @@
spin_lock_irqsave(&isp116x->lock, flags);
if (!HC_IS_RUNNING(hcd->state)) {
+ kfree(ep);
ret = -ENODEV;
goto fail;
}
@@ -888,7 +882,7 @@
struct usb_host_endpoint *hep)
{
int i;
- struct isp116x_ep *ep = hep->hcpriv;;
+ struct isp116x_ep *ep = hep->hcpriv;
if (!ep)
return;
@@ -916,8 +910,6 @@
return (int)fmnum;
}
-/*----------------------------------------------------------------*/
-
/*
Adapted from ohci-hub.c. Currently we don't support autosuspend.
*/
@@ -968,11 +960,10 @@
desc->bHubContrCurrent = 0;
desc->bNbrPorts = (u8) (reg & 0x3);
/* Power switching, device type, overcurrent. */
- desc->wHubCharacteristics =
- (__force __u16) cpu_to_le16((u16) ((reg >> 8) & 0x1f));
+ desc->wHubCharacteristics = cpu_to_le16((u16) ((reg >> 8) & 0x1f));
desc->bPwrOn2PwrGood = (u8) ((reg >> 24) & 0xff);
/* two bitmaps: ports removable, and legacy PortPwrCtrlMask */
- desc->bitmap[0] = desc->bNbrPorts == 1 ? 1 << 1 : 3 << 1;
+ desc->bitmap[0] = 0;
desc->bitmap[1] = ~0;
}
@@ -1159,145 +1150,9 @@
return ret;
}
-#ifdef CONFIG_PM
-
-static int isp116x_bus_suspend(struct usb_hcd *hcd)
-{
- struct isp116x *isp116x = hcd_to_isp116x(hcd);
- unsigned long flags;
- u32 val;
- int ret = 0;
-
- spin_lock_irqsave(&isp116x->lock, flags);
-
- val = isp116x_read_reg32(isp116x, HCCONTROL);
- switch (val & HCCONTROL_HCFS) {
- case HCCONTROL_USB_OPER:
- hcd->state = HC_STATE_QUIESCING;
- val &= (~HCCONTROL_HCFS & ~HCCONTROL_RWE);
- val |= HCCONTROL_USB_SUSPEND;
- if (hcd->remote_wakeup)
- val |= HCCONTROL_RWE;
- /* Wait for usb transfers to finish */
- mdelay(2);
- isp116x_write_reg32(isp116x, HCCONTROL, val);
- hcd->state = HC_STATE_SUSPENDED;
- /* Wait for devices to suspend */
- mdelay(5);
- case HCCONTROL_USB_SUSPEND:
- break;
- case HCCONTROL_USB_RESUME:
- isp116x_write_reg32(isp116x, HCCONTROL,
- (val & ~HCCONTROL_HCFS) |
- HCCONTROL_USB_RESET);
- case HCCONTROL_USB_RESET:
- ret = -EBUSY;
- break;
- default:
- ret = -EINVAL;
- }
-
- spin_unlock_irqrestore(&isp116x->lock, flags);
- return ret;
-}
-
-/* Get rid of these declarations later in cleanup */
-static int isp116x_reset(struct usb_hcd *hcd);
-static int isp116x_start(struct usb_hcd *hcd);
-
-static int isp116x_bus_resume(struct usb_hcd *hcd)
-{
- struct isp116x *isp116x = hcd_to_isp116x(hcd);
- u32 val;
-
- msleep(5);
- spin_lock_irq(&isp116x->lock);
-
- val = isp116x_read_reg32(isp116x, HCCONTROL);
- switch (val & HCCONTROL_HCFS) {
- case HCCONTROL_USB_SUSPEND:
- val &= ~HCCONTROL_HCFS;
- val |= HCCONTROL_USB_RESUME;
- isp116x_write_reg32(isp116x, HCCONTROL, val);
- case HCCONTROL_USB_RESUME:
- break;
- case HCCONTROL_USB_OPER:
- spin_unlock_irq(&isp116x->lock);
- /* Without setting power_state here the
- SUSPENDED state won't be removed from
- sysfs/usbN/power.state as a response to remote
- wakeup. Maybe in the future. */
- hcd->self.root_hub->dev.power.power_state = PMSG_ON;
- return 0;
- default:
- /* HCCONTROL_USB_RESET: this may happen, when during
- suspension the HC lost power. Reinitialize completely */
- spin_unlock_irq(&isp116x->lock);
- DBG("Chip has been reset while suspended. Reinit from scratch.\n");
- isp116x_reset(hcd);
- isp116x_start(hcd);
- isp116x_hub_control(hcd, SetPortFeature,
- USB_PORT_FEAT_POWER, 1, NULL, 0);
- if ((isp116x->rhdesca & RH_A_NDP) == 2)
- isp116x_hub_control(hcd, SetPortFeature,
- USB_PORT_FEAT_POWER, 2, NULL, 0);
- hcd->self.root_hub->dev.power.power_state = PMSG_ON;
- return 0;
- }
-
- val = isp116x->rhdesca & RH_A_NDP;
- while (val--) {
- u32 stat =
- isp116x_read_reg32(isp116x, val ? HCRHPORT2 : HCRHPORT1);
- /* force global, not selective, resume */
- if (!(stat & RH_PS_PSS))
- continue;
- DBG("%s: Resuming port %d\n", __func__, val);
- isp116x_write_reg32(isp116x, RH_PS_POCI, val
- ? HCRHPORT2 : HCRHPORT1);
- }
- spin_unlock_irq(&isp116x->lock);
-
- hcd->state = HC_STATE_RESUMING;
- mdelay(20);
-
- /* Go operational */
- spin_lock_irq(&isp116x->lock);
- val = isp116x_read_reg32(isp116x, HCCONTROL);
- isp116x_write_reg32(isp116x, HCCONTROL,
- (val & ~HCCONTROL_HCFS) | HCCONTROL_USB_OPER);
- spin_unlock_irq(&isp116x->lock);
- /* see analogous comment above */
- hcd->self.root_hub->dev.power.power_state = PMSG_ON;
- hcd->state = HC_STATE_RUNNING;
-
- return 0;
-}
-
-
-#else
-
-#define isp116x_bus_suspend NULL
-#define isp116x_bus_resume NULL
-
-#endif
-
/*-----------------------------------------------------------------*/
-#ifdef STUB_DEBUG_FILE
-
-static inline void create_debug_file(struct isp116x *isp116x)
-{
-}
-
-static inline void remove_debug_file(struct isp116x *isp116x)
-{
-}
-
-#else
-
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
+#ifdef CONFIG_DEBUG_FS
static void dump_irq(struct seq_file *s, char *label, u16 mask)
{
@@ -1321,13 +1176,9 @@
mask & HCINT_SF ? " sof" : "", mask & HCINT_SO ? " so" : "");
}
-static int proc_isp116x_show(struct seq_file *s, void *unused)
+static int isp116x_show_dbg(struct seq_file *s, void *unused)
{
struct isp116x *isp116x = s->private;
- struct isp116x_ep *ep;
- struct urb *urb;
- unsigned i;
- char *str;
seq_printf(s, "%s\n%s version %s\n",
isp116x_to_hcd(isp116x)->product_desc, hcd_name,
@@ -1343,105 +1194,50 @@
}
spin_lock_irq(&isp116x->lock);
-
dump_irq(s, "hc_irq_enable", isp116x_read_reg16(isp116x, HCuPINTENB));
dump_irq(s, "hc_irq_status", isp116x_read_reg16(isp116x, HCuPINT));
dump_int(s, "hc_int_enable", isp116x_read_reg32(isp116x, HCINTENB));
dump_int(s, "hc_int_status", isp116x_read_reg32(isp116x, HCINTSTAT));
-
- list_for_each_entry(ep, &isp116x->async, schedule) {
-
- switch (ep->nextpid) {
- case USB_PID_IN:
- str = "in";
- break;
- case USB_PID_OUT:
- str = "out";
- break;
- case USB_PID_SETUP:
- str = "setup";
- break;
- case USB_PID_ACK:
- str = "status";
- break;
- default:
- str = "?";
- break;
- };
- seq_printf(s, "%p, ep%d%s, maxpacket %d:\n", ep,
- ep->epnum, str, ep->maxpacket);
- list_for_each_entry(urb, &ep->hep->urb_list, urb_list) {
- seq_printf(s, " urb%p, %d/%d\n", urb,
- urb->actual_length,
- urb->transfer_buffer_length);
- }
- }
- if (!list_empty(&isp116x->async))
- seq_printf(s, "\n");
-
- seq_printf(s, "periodic size= %d\n", PERIODIC_SIZE);
-
- for (i = 0; i < PERIODIC_SIZE; i++) {
- ep = isp116x->periodic[i];
- if (!ep)
- continue;
- seq_printf(s, "%2d [%3d]:\n", i, isp116x->load[i]);
-
- /* DUMB: prints shared entries multiple times */
- do {
- seq_printf(s, " %d/%p (%sdev%d ep%d%s max %d)\n",
- ep->period, ep,
- (ep->udev->speed ==
- USB_SPEED_FULL) ? "" : "ls ",
- ep->udev->devnum, ep->epnum,
- (ep->epnum ==
- 0) ? "" : ((ep->nextpid ==
- USB_PID_IN) ? "in" : "out"),
- ep->maxpacket);
- ep = ep->next;
- } while (ep);
- }
+ isp116x_show_regs_seq(isp116x, s);
spin_unlock_irq(&isp116x->lock);
seq_printf(s, "\n");
return 0;
}
-static int proc_isp116x_open(struct inode *inode, struct file *file)
+static int isp116x_open_seq(struct inode *inode, struct file *file)
{
- return single_open(file, proc_isp116x_show, PDE(inode)->data);
+ return single_open(file, isp116x_show_dbg, inode->u.generic_ip);
}
-static struct file_operations proc_ops = {
- .open = proc_isp116x_open,
+static struct file_operations isp116x_debug_fops = {
+ .open = isp116x_open_seq,
.read = seq_read,
.llseek = seq_lseek,
.release = single_release,
};
-/* expect just one isp116x per system */
-static const char proc_filename[] = "driver/isp116x";
-
-static void create_debug_file(struct isp116x *isp116x)
+static int create_debug_file(struct isp116x *isp116x)
{
- struct proc_dir_entry *pde;
-
- pde = create_proc_entry(proc_filename, 0, NULL);
- if (pde == NULL)
- return;
-
- pde->proc_fops = &proc_ops;
- pde->data = isp116x;
- isp116x->pde = pde;
+ isp116x->dentry = debugfs_create_file(hcd_name,
+ S_IRUGO, NULL, isp116x,
+ &isp116x_debug_fops);
+ if (!isp116x->dentry)
+ return -ENOMEM;
+ return 0;
}
static void remove_debug_file(struct isp116x *isp116x)
{
- if (isp116x->pde)
- remove_proc_entry(proc_filename, NULL);
+ debugfs_remove(isp116x->dentry);
}
-#endif
+#else
+
+#define create_debug_file(d) 0
+#define remove_debug_file(d) do{}while(0)
+
+#endif /* CONFIG_DEBUG_FS */
/*-----------------------------------------------------------------*/
@@ -1476,7 +1272,7 @@
struct isp116x *isp116x = hcd_to_isp116x(hcd);
unsigned long t;
u16 clkrdy = 0;
- int ret = 0, timeout = 15 /* ms */ ;
+ int ret, timeout = 15 /* ms */ ;
ret = isp116x_sw_reset(isp116x);
if (ret)
@@ -1492,7 +1288,7 @@
break;
}
if (!clkrdy) {
- ERR("Clock not ready after 20ms\n");
+ ERR("Clock not ready after %dms\n", timeout);
/* After sw_reset the clock won't report to be ready, if
H_WAKEUP pin is high. */
ERR("Please make sure that the H_WAKEUP pin is pulled low!\n");
@@ -1610,12 +1406,128 @@
isp116x_write_reg32(isp116x, HCRHPORT1, RH_PS_CCS);
isp116x_write_reg32(isp116x, HCRHPORT2, RH_PS_CCS);
- isp116x_show_regs(isp116x);
+ isp116x_show_regs_log(isp116x);
spin_unlock_irqrestore(&isp116x->lock, flags);
return 0;
}
-/*-----------------------------------------------------------------*/
+#ifdef CONFIG_PM
+
+static int isp116x_bus_suspend(struct usb_hcd *hcd)
+{
+ struct isp116x *isp116x = hcd_to_isp116x(hcd);
+ unsigned long flags;
+ u32 val;
+ int ret = 0;
+
+ spin_lock_irqsave(&isp116x->lock, flags);
+
+ val = isp116x_read_reg32(isp116x, HCCONTROL);
+ switch (val & HCCONTROL_HCFS) {
+ case HCCONTROL_USB_OPER:
+ hcd->state = HC_STATE_QUIESCING;
+ val &= (~HCCONTROL_HCFS & ~HCCONTROL_RWE);
+ val |= HCCONTROL_USB_SUSPEND;
+ if (hcd->remote_wakeup)
+ val |= HCCONTROL_RWE;
+ /* Wait for usb transfers to finish */
+ mdelay(2);
+ isp116x_write_reg32(isp116x, HCCONTROL, val);
+ hcd->state = HC_STATE_SUSPENDED;
+ /* Wait for devices to suspend */
+ mdelay(5);
+ case HCCONTROL_USB_SUSPEND:
+ break;
+ case HCCONTROL_USB_RESUME:
+ isp116x_write_reg32(isp116x, HCCONTROL,
+ (val & ~HCCONTROL_HCFS) |
+ HCCONTROL_USB_RESET);
+ case HCCONTROL_USB_RESET:
+ ret = -EBUSY;
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ spin_unlock_irqrestore(&isp116x->lock, flags);
+ return ret;
+}
+
+static int isp116x_bus_resume(struct usb_hcd *hcd)
+{
+ struct isp116x *isp116x = hcd_to_isp116x(hcd);
+ u32 val;
+
+ msleep(5);
+ spin_lock_irq(&isp116x->lock);
+
+ val = isp116x_read_reg32(isp116x, HCCONTROL);
+ switch (val & HCCONTROL_HCFS) {
+ case HCCONTROL_USB_SUSPEND:
+ val &= ~HCCONTROL_HCFS;
+ val |= HCCONTROL_USB_RESUME;
+ isp116x_write_reg32(isp116x, HCCONTROL, val);
+ case HCCONTROL_USB_RESUME:
+ break;
+ case HCCONTROL_USB_OPER:
+ spin_unlock_irq(&isp116x->lock);
+ /* Without setting power_state here the
+ SUSPENDED state won't be removed from
+ sysfs/usbN/power.state as a response to remote
+ wakeup. Maybe in the future. */
+ hcd->self.root_hub->dev.power.power_state = PMSG_ON;
+ return 0;
+ default:
+ /* HCCONTROL_USB_RESET: this may happen, when during
+ suspension the HC lost power. Reinitialize completely */
+ spin_unlock_irq(&isp116x->lock);
+ DBG("Chip has been reset while suspended. Reinit from scratch.\n");
+ isp116x_reset(hcd);
+ isp116x_start(hcd);
+ isp116x_hub_control(hcd, SetPortFeature,
+ USB_PORT_FEAT_POWER, 1, NULL, 0);
+ if ((isp116x->rhdesca & RH_A_NDP) == 2)
+ isp116x_hub_control(hcd, SetPortFeature,
+ USB_PORT_FEAT_POWER, 2, NULL, 0);
+ hcd->self.root_hub->dev.power.power_state = PMSG_ON;
+ return 0;
+ }
+
+ val = isp116x->rhdesca & RH_A_NDP;
+ while (val--) {
+ u32 stat =
+ isp116x_read_reg32(isp116x, val ? HCRHPORT2 : HCRHPORT1);
+ /* force global, not selective, resume */
+ if (!(stat & RH_PS_PSS))
+ continue;
+ DBG("%s: Resuming port %d\n", __func__, val);
+ isp116x_write_reg32(isp116x, RH_PS_POCI, val
+ ? HCRHPORT2 : HCRHPORT1);
+ }
+ spin_unlock_irq(&isp116x->lock);
+
+ hcd->state = HC_STATE_RESUMING;
+ msleep(20);
+
+ /* Go operational */
+ spin_lock_irq(&isp116x->lock);
+ val = isp116x_read_reg32(isp116x, HCCONTROL);
+ isp116x_write_reg32(isp116x, HCCONTROL,
+ (val & ~HCCONTROL_HCFS) | HCCONTROL_USB_OPER);
+ spin_unlock_irq(&isp116x->lock);
+ /* see analogous comment above */
+ hcd->self.root_hub->dev.power.power_state = PMSG_ON;
+ hcd->state = HC_STATE_RUNNING;
+
+ return 0;
+}
+
+#else
+
+#define isp116x_bus_suspend NULL
+#define isp116x_bus_resume NULL
+
+#endif
static struct hc_driver isp116x_hc_driver = {
.description = hcd_name,
@@ -1745,12 +1657,19 @@
}
ret = usb_add_hcd(hcd, irq, SA_INTERRUPT);
- if (ret != 0)
+ if (ret)
goto err6;
- create_debug_file(isp116x);
+ ret = create_debug_file(isp116x);
+ if (ret) {
+ ERR("Couldn't create debugfs entry\n");
+ goto err7;
+ }
+
return 0;
+ err7:
+ usb_remove_hcd(hcd);
err6:
usb_put_hcd(hcd);
err5:
@@ -1772,13 +1691,9 @@
*/
static int isp116x_suspend(struct platform_device *dev, pm_message_t state)
{
- int ret = 0;
-
- VDBG("%s: state %x\n", __func__, state);
-
+ VDBG("%s: state %x\n", __func__, state.event);
dev->dev.power.power_state = state;
-
- return ret;
+ return 0;
}
/*
@@ -1786,13 +1701,9 @@
*/
static int isp116x_resume(struct platform_device *dev)
{
- int ret = 0;
-
- VDBG("%s: state %x\n", __func__, dev->dev.power.power_state);
-
+ VDBG("%s: state %x\n", __func__, dev->power.power_state.event);
dev->dev.power.power_state = PMSG_ON;
-
- return ret;
+ return 0;
}
#else