I was recently looking at the root hub logic in 2.6 and noticed that UHCI was mis-advertising itself as supporting per-port power switching; the hardware doesn't do that. OK, easy to fix...
Then I noticed that it was advertising itself as supporting global over-current protection, but was using per-port status! Not quite as obvious a fix, since the UHCI 1.1d spec doesn't even list the USBPORTSC_OC bit the driver uses.
Google reports some of Intel's first UHCI controllers ([1], [2]) define not just OC, but also a status change bit that uhci-hcd doesn't know about. Do you know anything about this? I'd expect that those two bits are implemented in subsequent UHCI hardware, both from Intel and from other vendors.
Well to make a long story short, here's a patch that makes the UHCI root hub more conformant to the USB hub spec. Tested only lightly (on a 440BX, not VIA). This obviously isn't a 2.6.2 candidate.
- Dave
[1] http://www.intel.com/design/intarch/techinfo/430tx/usbreg.htm [2] http://www.intel.com/design/intarch/techinfo/440bx/usbreg.htm
UHCI root hub updates ... minor bugfixes and cleanups, improving conformance with the USB hub specification.
- UHCI doesn't support any kind of power switching; so modify the
hub descriptor to stop claiming it does! Likewise fail attempts
to disable power on any port.
- Intel defined both overcurrent status overcurrent-change bits, but
the current code only knows about one. Modify hub descriptor to
report per-port overcurrent protection; and use both bits.
- Modify the port status set/clear macros to know about the bits
that must always be written as zero, and the write-to-clear bits.
Update callers which wrote "set" instead of "clear".
- Rewrote code returning port status; magic numbers are gone.
- Driver can't really support 8 root hub ports; don't try.
Also moves the #define DEBUG earlier so that it can kick in any of
the various debug macros ... like pr_debug() and dev_dbg().
--- 1.50/drivers/usb/host/uhci-hcd.c Tue Jan 27 08:37:55 2004
+++ edited/drivers/usb/host/uhci-hcd.c Tue Feb 3 09:22:28 2004
@@ -27,6 +27,11 @@
*/
#include <linux/config.h>
+#ifdef CONFIG_USB_DEBUG
+#define DEBUG
+#else
+#undef DEBUG
+#endif
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/kernel.h>
@@ -41,11 +46,6 @@
#include <linux/interrupt.h>
#include <linux/spinlock.h>
#include <linux/proc_fs.h>
-#ifdef CONFIG_USB_DEBUG
-#define DEBUG
-#else
-#undef DEBUG
-#endif
#include <linux/usb.h>
#include <asm/uaccess.h>
@@ -2308,7 +2308,7 @@
/* This is experimental so anything less than 2 or greater than 8 is */
/* something weird and we'll ignore it */
- if (port < 2 || port > 8) {
+ if (port < 2 || port > UHCI_RH_MAXCHILD) {
info("port count misdetected? forcing to 2 ports");
port = 2;
}
--- 1.10/drivers/usb/host/uhci-hcd.h Fri Jul 18 06:22:32 2003
+++ edited/drivers/usb/host/uhci-hcd.h Tue Feb 3 09:26:50 2004
@@ -49,12 +49,19 @@
#define USBPORTSC_CSC 0x0002 /* Connect Status Change */
#define USBPORTSC_PE 0x0004 /* Port Enable */
#define USBPORTSC_PEC 0x0008 /* Port Enable Change */
-#define USBPORTSC_LS 0x0030 /* Line Status */
+#define USBPORTSC_DPLUS 0x0010 /* D+ high (line status) */
+#define USBPORTSC_DMINUS 0x0020 /* D- high (line status) */
#define USBPORTSC_RD 0x0040 /* Resume Detect */
+#define USBPORTSC_RES1 0x0080 /* reserved, always 1 */
#define USBPORTSC_LSDA 0x0100 /* Low Speed Device Attached */
#define USBPORTSC_PR 0x0200 /* Port Reset */
+/* OC and OCC from Intel 430TX and later (not UHCI 1.1d spec) */
#define USBPORTSC_OC 0x0400 /* Over Current condition */
+#define USBPORTSC_OCC 0x0800 /* Over Current Change R/WC */
#define USBPORTSC_SUSP 0x1000 /* Suspend */
+#define USBPORTSC_RES2 0x2000 /* reserved, write zeroes */
+#define USBPORTSC_RES3 0x4000 /* reserved, write zeroes */
+#define USBPORTSC_RES4 0x8000 /* reserved, write zeroes */
/* Legacy support register */
#define USBLEGSUP 0xc0
--- 1.2/drivers/usb/host/uhci-hub.c Wed Jun 5 02:57:47 2002
+++ edited/drivers/usb/host/uhci-hub.c Tue Feb 3 09:58:33 2004
@@ -16,14 +16,22 @@
0x09, /* __u8 bLength; */
0x29, /* __u8 bDescriptorType; Hub-descriptor */
0x02, /* __u8 bNbrPorts; */
- 0x00, /* __u16 wHubCharacteristics; */
- 0x00,
+ 0x0a, /* __u16 wHubCharacteristics; */
+ 0x00, /* (per-port OC, no power switching) */
0x01, /* __u8 bPwrOn2pwrGood; 2ms */
0x00, /* __u8 bHubContrCurrent; 0 mA */
0x00, /* __u8 DeviceRemovable; *** 7 Ports max *** */
0xff /* __u8 PortPwrCtrlMask; *** 7 ports max *** */
};
+#define UHCI_RH_MAXCHILD 7
+
+/* must write as zeroes */
+#define WZ_BITS (USBPORTSC_RES2 | USBPORTSC_RES3 | USBPORTSC_RES4)
+
+/* status change bits: nonzero writes will clear */
+#define RWC_BITS (USBPORTSC_OCC | USBPORTSC_PEC | USBPORTSC_CSC)
+
static int uhci_hub_status_data(struct usb_hcd *hcd, char *buf)
{
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
@@ -32,7 +40,9 @@
*buf = 0;
for (i = 0; i < uhci->rh_numports; i++) {
- *buf |= ((inw(io_addr + USBPORTSC1 + i * 2) & 0xa) > 0 ? (1 << (i +
1)) : 0);
+ *buf |= (inw(io_addr + USBPORTSC1 + i * 2) & RWC_BITS) != 0
+ ? (1 << (i + 1))
+ : 0;
len = (i + 1) / 8 + 1;
}
@@ -43,12 +53,15 @@
#define CLR_RH_PORTSTAT(x) \
status = inw(io_addr + USBPORTSC1 + 2 * (wIndex-1)); \
- status = (status & 0xfff5) & ~(x); \
+ status &= ~(RWC_BITS|WZ_BITS); \
+ status &= ~(x); \
+ status |= RWC_BITS & (x); \
outw(status, io_addr + USBPORTSC1 + 2 * (wIndex-1))
#define SET_RH_PORTSTAT(x) \
status = inw(io_addr + USBPORTSC1 + 2 * (wIndex-1)); \
- status = (status & 0xfff5) | (x); \
+ status |= (x); \
+ status &= ~(RWC_BITS|WZ_BITS); \
outw(status, io_addr + USBPORTSC1 + 2 * (wIndex-1))
@@ -57,13 +70,9 @@
u16 wIndex, char *buf, u16 wLength)
{
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
- int i, status, retval = 0, len = 0;
+ int status, retval = 0, len = 0;
unsigned int io_addr = uhci->io_addr;
- __u16 cstatus;
- char c_p_r[8];
-
- for (i = 0; i < 8; i++)
- c_p_r[i] = 0;
+ u16 wPortChange, wPortStatus;
switch (typeReq) {
/* Request Destination:
@@ -79,18 +88,39 @@
OK(4); /* hub power */
case GetPortStatus:
status = inw(io_addr + USBPORTSC1 + 2 * (wIndex - 1));
- cstatus = ((status & USBPORTSC_CSC) >> (1 - 0)) |
- ((status & USBPORTSC_PEC) >> (3 - 1)) |
- (c_p_r[wIndex - 1] << (0 + 4));
- status = (status & USBPORTSC_CCS) |
- ((status & USBPORTSC_PE) >> (2 - 1)) |
- ((status & USBPORTSC_SUSP) >> (12 - 2)) |
- ((status & USBPORTSC_PR) >> (9 - 4)) |
- (1 << 8) | /* power on */
- ((status & USBPORTSC_LSDA) << (-8 + 9));
- *(__u16 *)buf = cpu_to_le16(status);
- *(__u16 *)(buf + 2) = cpu_to_le16(cstatus);
+ /* C_SUSPEND and C_RESET are always false */
+ wPortChange = 0;
+ if (status & USBPORTSC_CSC)
+ wPortChange |= 1 << (USB_PORT_FEAT_C_CONNECTION - 16);
+ if (status & USBPORTSC_PEC)
+ wPortChange |= 1 << (USB_PORT_FEAT_C_ENABLE - 16);
+ if (status & USBPORTSC_OCC)
+ wPortChange |= 1 << (USB_PORT_FEAT_C_OVER_CURRENT - 16);
+
+ /* UHCI has no power switching (always on) */
+ wPortStatus = 1 << USB_PORT_FEAT_POWER;
+ if (status & USBPORTSC_CCS)
+ wPortStatus |= 1 << USB_PORT_FEAT_CONNECTION;
+ if (status & USBPORTSC_PE) {
+ wPortStatus |= 1 << USB_PORT_FEAT_ENABLE;
+ if (status & (USBPORTSC_SUSP | USBPORTSC_RD))
+ wPortStatus |= 1 << USB_PORT_FEAT_SUSPEND;
+ }
+ if (status & USBPORTSC_OC)
+ wPortStatus |= 1 << USB_PORT_FEAT_OVER_CURRENT;
+ if (status & USBPORTSC_PR)
+ wPortStatus |= 1 << USB_PORT_FEAT_RESET;
+ if (status & USBPORTSC_LSDA)
+ wPortStatus |= 1 << USB_PORT_FEAT_LOWSPEED;
+
+ if (wPortChange)
+ dev_dbg (hcd->controller,
+ "port %d portsc %04x\n",
+ wIndex, status);
+
+ *(__u16 *)buf = cpu_to_le16(wPortStatus);
+ *(__u16 *)(buf + 2) = cpu_to_le16(wPortChange);
OK(4);
case SetHubFeature:
switch (wValue) {
@@ -104,6 +134,7 @@
case ClearHubFeature:
switch (wValue) {
case C_HUB_OVER_CURRENT:
+ case C_HUB_LOCAL_POWER:
OK(0); /* hub power over current */
default:
goto err;
@@ -120,18 +151,15 @@
case USB_PORT_FEAT_RESET:
SET_RH_PORTSTAT(USBPORTSC_PR);
mdelay(50); /* USB v1.1 7.1.7.3 */
- c_p_r[wIndex - 1] = 1;
CLR_RH_PORTSTAT(USBPORTSC_PR);
udelay(10);
SET_RH_PORTSTAT(USBPORTSC_PE);
mdelay(10);
- SET_RH_PORTSTAT(0xa);
+ CLR_RH_PORTSTAT(USBPORTSC_PEC|USBPORTSC_CSC);
OK(0);
case USB_PORT_FEAT_POWER:
+ /* UHCI has no power switching */
OK(0); /* port power ** */
- case USB_PORT_FEAT_ENABLE:
- SET_RH_PORTSTAT(USBPORTSC_PE);
- OK(0);
default:
goto err;
}
@@ -145,23 +173,25 @@
CLR_RH_PORTSTAT(USBPORTSC_PE);
OK(0);
case USB_PORT_FEAT_C_ENABLE:
- SET_RH_PORTSTAT(USBPORTSC_PEC);
+ CLR_RH_PORTSTAT(USBPORTSC_PEC);
OK(0);
case USB_PORT_FEAT_SUSPEND:
CLR_RH_PORTSTAT(USBPORTSC_SUSP);
OK(0);
case USB_PORT_FEAT_C_SUSPEND:
- /*** WR_RH_PORTSTAT(RH_PS_PSSC); */
+ /* this driver won't report these */
OK(0);
case USB_PORT_FEAT_POWER:
- OK(0); /* port power */
+ /* UHCI has no power switching */
+ goto err;
case USB_PORT_FEAT_C_CONNECTION:
- SET_RH_PORTSTAT(USBPORTSC_CSC);
+ CLR_RH_PORTSTAT(USBPORTSC_CSC);
OK(0);
case USB_PORT_FEAT_C_OVER_CURRENT:
+ CLR_RH_PORTSTAT(USBPORTSC_OCC);
OK(0); /* port power over current */
case USB_PORT_FEAT_C_RESET:
- c_p_r[wIndex - 1] = 0;
+ /* this driver won't report these */
OK(0);
default:
goto err;
