On Thu, Aug 04, 2011 at 04:39:19PM +0800, Xu, Andiry wrote: > > -----Original Message----- > > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb- > > owner@xxxxxxxxxxxxxxx] On Behalf Of Sarah Sharp > > Sent: Thursday, August 04, 2011 2:28 AM > > To: Xu, Andiry > > Cc: linux-usb@xxxxxxxxxxxxxxx; gregkh@xxxxxxx; > > stern@xxxxxxxxxxxxxxxxxxx > > Subject: Re: [PATCH 6/8] xHCI: calculate HIRD > > > > On Tue, Jul 26, 2011 at 08:37:36AM +0800, Andiry Xu wrote: > > > Calculate HIRD(Host Initiated Resume Duration) in USB2 PORTPMSC > based > > on > > > the host's U2 Device Exit Latency field. > > > > > > Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx> > > > --- > > > drivers/usb/host/xhci-mem.c | 17 +++++++++++++++++ > > > drivers/usb/host/xhci.h | 2 ++ > > > 2 files changed, 19 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci- > > mem.c > > > index 2b4cff9..83ba59b 100644 > > > --- a/drivers/usb/host/xhci-mem.c > > > +++ b/drivers/usb/host/xhci-mem.c > > > @@ -1927,6 +1927,21 @@ static int xhci_setup_port_arrays(struct > > xhci_hcd *xhci, gfp_t flags) > > > return 0; > > > } > > > > > > +/* Calculate HIRD for USB2 PORTPMSC*/ > > > +static void xhci_calculate_hird(struct xhci_hcd *xhci) > > > +{ > > > + int u2del; > > > + > > > + u2del = HCS_U2_LATENCY(xhci->hcs_params3); > > > + if (u2del <= 50) > > > + xhci->hird = 0; > > > + else > > > + xhci->hird = (u2del - 51) / 75 + 1; > > > + > > > + if (xhci->hird > 15) > > > + xhci->hird = 15; > > > +} > > > + > > > > Can you store the hird in microseconds instead of converting it for > the > > USB 2.0 port PM register format? The USB 3.0 exit latency in the port > > PM status register is going to want to use the unconverted value, so > it > > makes sense to move the conversion into (or into a new function above) > > the hub code that turns on USB 2.0 LPM. > > > > Well, I suppose USB 3.0 devices should use the wU2DevExitLat field in > their SuperSpeed Device Capabilities descriptor instead. The xHCI driver will need to use the host controller exit latency, on top of the wU2DevExitLat field, in order to calculate the max exit latency for a device. The driver will need to sum up the exit latencies for all parents of the device, including the roothub. I think it makes sense to store the host controller exit latency in its unconverted value. You can always convert it before you use it. > > Also, the latest version of the 1.0 spec that I have seems to be a bit > > different from what you're using to code this. It renames the port PM > > status register HIRD field to BESL, and adds some fields in a new > > register, Port Hardware LPM Control Register. It also adds some text > > to > > section 4.23.5.1.1.1 about how to handle these fields. I think you'll > > need to revise your patchset to handle that change. Writing to the > > fourth port status register should have no effect for xHCI 1.0 hosts > > that don't support it, since they should ignore those writes. > > > > I think you're referring to xHCI 1.0 errata. It has a lot of changes > from xHCI 1.0 spec, which causes pain to driver developers. Errata changes are supposed to be pretty non-invasive from a driver standpoint. Either new features are added to register spaces that were originally marked as reserved, or a change is made in such a way that both old and new software can run on any host controller with that revision number. At least, that's how I've been told it's supposed to work. :) I just want you to double check that the code you added would still work on these host controllers. Otherwise we'll need to talk to the xHCI spec architect and get it changed. > I don't > have the HW which is compliant with xHCI 1.0 errata so I can not verify > the code after modification; That's fine, neither do I (hardware engineers are not *that* speedy). However, I do want to be sure that when host controllers that do implement this errata arrive, we have a plan for how we can handle them. Or at the very least, a note in the code around setting HIRD that some host controllers may implement a different interface, so that when USB 2.0 LPM breaks on these new hosts, we have some sort of clue as to why. > also it changes the definition of USB2 > PORTPMSC register. If the HCIVERSION of new HW is still 0100h, how > does driver distinguish them? I think you shouldn't have to distinguish them. For most errata I've seen, you write to an extra register field (which the older host controllers will ignore), and maybe write to a now-reserved register that the newer host controllers will ignore. I think that's the case for the LPM errata. I think the HIRD field in the PORTPMSC register was only renamed to BESL, and the meaning was not changed. It gets a little confusing with table 13, and I need to double check that with the xHCI spec architect. After reading the new sections, here's how I think it's supposed to work: - Set the field that used to be called HIRD (called BESL now) to what you would normally set it to. - Leave the HIRDM in the new Port Hardware LPM Control Register set to zero. That will force the host to use the BESL only. - Leave BESLD set to zero. Now the only question in my mind is how to set the L1 timeout. It looks like if software leaves it set to zero, the timeout is 128 microseconds, which is probably a reasonable number. So it seems like the errata will still allow older software to work on hosts that implement that errata, aside from the question of the BESL encoding. I'll double check this with the xHCI spec architect. Do you think my interpretation looks correct from your reading of the spec? > I'm afraid I don't have the resource to implement xHCI 1.0 errata, > since I may switch to other tasks. Yes, we're all busy, and I don't want you to implement something that no hardware will use. :) But I do want to make sure we have a plan for hardware that does implement it, and that the xHCI spec errata is sane enough to allow old software to work. At least put a note in the code about the errata changes, and we'll make any changes to the code once we confirm with the xHCI spec architect. Perhaps something like: + /* Test USB 2.0 software LPM */ + /* FIXME: Some xHCI 1.0 hosts may implement a new register + * to set up hardware-controlled USB 2.0 LPM. See section + * 5.4.11 and 4.23.5.1.1.1 in the June 2011 errata release. + */ + xhci_dbg(xhci, "test port %d software LPM\n", port_num); + /* Set L1 Device Slot and HIRD */ + pm_addr = port_array[port_num] + 1; + temp = PORT_L1DS(udev->slot_id) | PORT_HIRD(xhci->hird); + xhci_writel(xhci, temp, pm_addr); Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
