let's say: it works with VIA as bad as before. ;-) The UHCI driver itself works -- if the ehci driver is/was loaded (before).
If I only load the UHCI driver all external devices get no power at all (measured with a meter).
To get power to the external bus I have to first load both drivers: ehci and uhci. When I remove the ehci driver the power is still on.
I discussed this problem with Dave a while ago and we agreed that time that we leave the problem unresolved. Maybe now it's time to fix it with this patch as well?
/Bernd
Alan Stern wrote:
On Tue, 3 Feb 2004, David Brownell wrote:
Hi,
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
Your changes look good to me. I added a few of my own, nothing very big:
Removed an unused variable in uhci_hub_status_data() and made the code a little more readable -- a little slower too, perhaps, but it's not time-critical;
Stored the port address instead of calculating it many times;
Added a missing check for port number in range;
Merged the code for SetHubFeature and ClearHubFeature, since neither one actually does anything;
Removed some redundant comments;
Removed an idempotent min_t operation (how did that get in there in the first place?).
This works okay on my system, but mine also uses Intel parts. While I would be surprised if another vendor's UHCI implementation differed in the way it handles overcurrent notifications, I don't know for sure. Getting information out of VIA doesn't appear to be easy.
Can someone who has a VIA-based UHCI controller please try out this patch and let us know if anything goes wrong?
Alan Stern
===== uhci-hcd.c 1.76 vs edited =====
--- 1.76/drivers/usb/host/uhci-hcd.c Thu Jan 29 10:15:40 2004
+++ edited/drivers/usb/host/uhci-hcd.c Fri Feb 6 14:11:31 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>
@@ -2309,7 +2309,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;
}
===== uhci-hcd.h 1.15 vs edited =====
--- 1.15/drivers/usb/host/uhci-hcd.h Mon Sep 8 12:21:51 2003
+++ edited/drivers/usb/host/uhci-hcd.h Fri Feb 6 14:11:31 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
===== uhci-hub.c 1.3 vs edited =====
--- 1.3/drivers/usb/host/uhci-hub.c Sun Jun 9 03:03:14 2002
+++ edited/drivers/usb/host/uhci-hub.c Fri Feb 6 15:20:27 2004
@@ -16,40 +16,50 @@
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);
unsigned int io_addr = uhci->io_addr;
- int i, len = 1;
+ int i;
*buf = 0;
for (i = 0; i < uhci->rh_numports; i++) {
- *buf |= ((inw(io_addr + USBPORTSC1 + i * 2) & 0xa) > 0 ? (1 << (i + 1)) : 0);
- len = (i + 1) / 8 + 1;
+ if (inw(io_addr + USBPORTSC1 + i * 2) & RWC_BITS)
+ *buf |= (1 << (i + 1));
}
-
return !!*buf;
}
#define OK(x) len = (x); break
#define CLR_RH_PORTSTAT(x) \
- status = inw(io_addr + USBPORTSC1 + 2 * (wIndex-1)); \
- status = (status & 0xfff5) & ~(x); \
- outw(status, io_addr + USBPORTSC1 + 2 * (wIndex-1))
+ status = inw(port_addr); \
+ status &= ~(RWC_BITS|WZ_BITS); \
+ status &= ~(x); \
+ status |= RWC_BITS & (x); \
+ outw(status, port_addr)
#define SET_RH_PORTSTAT(x) \
- status = inw(io_addr + USBPORTSC1 + 2 * (wIndex-1)); \
- status = (status & 0xfff5) | (x); \
- outw(status, io_addr + USBPORTSC1 + 2 * (wIndex-1))
+ status = inw(port_addr); \
+ status |= (x); \
+ status &= ~(RWC_BITS|WZ_BITS); \
+ outw(status, port_addr)
/* size of returned buffer is part of USB spec */
@@ -57,13 +67,9 @@
u16 wIndex, char *buf, u16 wLength)
{
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
- int i, 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;
+ int status, retval = 0, len = 0;
+ unsigned int port_addr = uhci->io_addr + USBPORTSC1 + 2 * (wIndex-1);
+ __u16 wPortChange, wPortStatus;
switch (typeReq) {
/* Request Destination:
@@ -78,33 +84,49 @@
*(__u32 *)buf = cpu_to_le32(0);
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);
- OK(4);
- case SetHubFeature:
- switch (wValue) {
- case C_HUB_OVER_CURRENT:
- case C_HUB_LOCAL_POWER:
- break;
- default:
+ if (!wIndex || wIndex > uhci->rh_numports)
goto err;
+ status = inw(port_addr);
+
+ /* UHCI doesn't support C_SUSPEND and C_RESET (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;
}
- break;
+ 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: /* We don't implement these */
case ClearHubFeature:
switch (wValue) {
case C_HUB_OVER_CURRENT:
- OK(0); /* hub power over current */
+ case C_HUB_LOCAL_POWER:
+ OK(0);
default:
goto err;
}
@@ -120,17 +142,14 @@
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:
- OK(0); /* port power ** */
- case USB_PORT_FEAT_ENABLE:
- SET_RH_PORTSTAT(USBPORTSC_PE);
+ /* UHCI has no power switching */
OK(0);
default:
goto err;
@@ -145,31 +164,32 @@
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:
- OK(0); /* port power over current */
+ CLR_RH_PORTSTAT(USBPORTSC_OCC);
+ OK(0);
case USB_PORT_FEAT_C_RESET:
- c_p_r[wIndex - 1] = 0;
+ /* this driver won't report these */
OK(0);
default:
goto err;
}
break;
case GetHubDescriptor:
- len = min_t(unsigned int, wLength,
- min_t(unsigned int, sizeof(root_hub_hub_des), wLength));
+ len = min_t(unsigned int, sizeof(root_hub_hub_des), wLength);
memcpy(buf, root_hub_hub_des, len);
if (len > 2)
buf[2] = uhci->rh_numports;
------------------------------------------------------- The SF.Net email is sponsored by EclipseCon 2004 Premiere Conference on Open Tools Development and Integration See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. http://www.eclipsecon.org/osdn _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
------------------------------------------------------- The SF.Net email is sponsored by EclipseCon 2004 Premiere Conference on Open Tools Development and Integration See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. http://www.eclipsecon.org/osdn _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
