On Thu, Feb 27, 2014 at 10:48 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Thu, 27 Feb 2014, Dan Williams wrote: > >> > If the port's ACPI data agrees with the default matching, there's no >> > issue. But if they disagree, don't accept the default match. That way >> > you never have to correct a mistaken match. >> >> So it turns out this simplifies the patch a bit, by getting rid of >> invalidate_dependent_peers(). However, we still need >> enumerate_dependent_peers() to handle cases where descendants are >> registered prior to peering the parents via location data. The big > > How could that happen? > > Suppose B is a child of A, and they both have been registered for some > time but don't have any peers assigned yet. Now along comes A', and > the firmware location data tells us that A' and A are peers. > > B still doesn't have a matching peer, though. Not until B' (a child of > A') is registered. At that time, B' will automatically be peered with > B, so no extra work is needed. > > The main point is that the location data for A' is available before any > descendants of A' can be created. Therefore descendants cannot be > registered prior to peering the parents via location data. Ah, yes, there's no possible work for enumerate_dependent_peers() to do because by definition if it could find a dependent peer, that same peer would have done the same association in reverse via its default mapping. Ok, that code can die too. >> complexity savings is killing the need for a solution like the one >> proposed in "[RFC PATCH v5 16/16] usb, xhci: flush initial hub >> discovery to gate port power control" >> >> i went ahead and changed 'struct usb_port_location' to 'typedef u32 >> usb_port_location_t', and am preparing to release a v6 addressing the >> current comments. > > Okay. I suggest that when v6 is ready, you start by posting just the > equivalents to v5 patches 1-5 (or 1-6). Once we agree on the details > of the peering, we'll move forward to consider synchronization of hub > and port PM operations. Will do. > BTW, after some more thought, I decided the right way to avoid races in > the peer assignments is to do some of the work under hub.c's > device_state_lock. Add a "usb_" prefix to the name and share it with > port.c. We already are guaranteed that all changes to port->child are > protected by this lock, which is exactly what we need. > I had wrapped locations that set port->child to NULL in the peer_lock, but I'll look at just re-using the device_state_lock for that scenario. I'm still thinking the start of a disconnect operation should flush in-flight peering operations. Otherwise we'll need to take a reference under the device_state_lock, but I'll give it more thought. -- 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
