On Mon, Jan 13, 2014 at 2:56 PM, Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx> wrote: > On Tue, Jan 07, 2014 at 12:29:44PM -0800, Dan Williams wrote: >> ACPI identifies peer ports by setting their 'group_token' and 'group_position' >> _PLD data to the same value. If a platform has tier mismatch (see Appendix D >> figure 131 in the xhci spec), ACPI can override the default peer port >> association (for internal hubs). >> >> Location data is cached as an opaque cookie in usb_port_location data. > > I haven't looked at this too closely, but what happens if: > - the USB 2.0 roothub is registered > - userspace immediately sets autosuspend_delay to zero and > pm_qos_no_port_poweroff to zero > - usb_acpi_find_device changes the connect_type to something other than > unknown Note, in this revision I've kept the established policy of not checking connect_type. We'll immediately suspend on setting pm_qos_no_poweroff to zero and device going idle. > - before usb_acpi_check_port_peer() is called, the port is suspended > > In that case, we'll potentially power off a suspended USB 2.0 port > before we know if it has a USB 3.0 peer port. It seems like you > shouldn't change the connection type until you set the peer port. > Unless you need that connection type for matching the peer port? The sequencing enforced by usb_port_runtime_suspend() is directional in that a usb2 port powering off before usb3 is always permitted (otherwise we run into cyclical dependencies). The implementation expects that the device will enforce the preference to connect on its usb3 interface, and it should as long as VBUS and the RX terminations are maintained, which they would be in this scenario. In the reverse scenario (detecting usb3 before usb2 peer) we have a check for "!peer" to block suspending the usb3 port prematurely. However, I wonder if it would be advisable to detect the case where a usb3 device has downgraded to it's usb2 connection. In that case I think we want to disable port power off because the assumption of "always enforce usb3 powered down last" has been invalidated. I can look into an incremental patch to add that safeguard for flaky devices, but it has not been a problem in testing. > Also, why only match on group token and group position? The xHCI spec > in Appendix D says, "The _UPC declarations for LS/FS/HS and SS ports > that are paired to form a USB 3.0 compatible connector. A 'pair' is > defined by two ports that declare _PLDs with identical Panel, Vertical > Position, Horizontal Position, Shape, Group Orientation and Group Token > parameter values." (Note, I know nothing about what these fields are > actually filled in with, I'm just quoting the spec here.) True, I was going off the ACPI spec (6.1.8) that says that "Group Token" is "Unique numerical value identifying a group". So my understanding is that the other fields are just positional, but group token and group position are sufficient for uniquely identifying connectors. -- 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
