On Fri, 2011-08-05 at 09:33 -0700, Sarah Sharp wrote: > > > > > > > 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. > I'm converting it to HIRD(BESL), which will only be used by USB2 LPM. Driver can always retrieve the host controller exit latency at any time, like this: + xhci->u2del = HCS_U2_LATENCY(xhci->hcs_params3); > > > 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. > Yes, Table 13 is confusing me when I check the 1.0 errata. As you notice, it does not only rename the field to BESL, but also re-define the decoding value of the field: a value of 15 (1111h) represents 1.2ms for HIRD in xHCI 1.0, while it decodes as 9950us(HIRD) in 1.0 errata. > 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? > Yes, I think that's OK. > > 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); > Sure, I'll modify it. Thanks a lot for your elaborate clarification and review, it's really appreciated. Do you think USB2 hardware LPM can be applied to devices behind external hubs? Thanks, Andiry -- 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
