On Wed, Oct 07, 2020 at 10:28:47AM -0700, Matthias Kaehlcke wrote:

On Wed, Oct 07, 2020 at 12:38:38PM -0400, Alan Stern wrote:

quoted

On Wed, Oct 07, 2020 at 09:03:36AM -0700, Matthias Kaehlcke wrote:

quoted

Ok, I wasn't sure if the hubs suspend asynchronously from each other. If they
do it should indeed not be a problem to have the "master" wait for its peers.

Well, order of suspending is selectable by the user.  It can be either 
asynchronous or reverse order of device registration, which might pose a 
problem.  We don't know in advance which of two peer hubs will be 
registered first.  It might be necessary to introduce some additional 
explicit synchronization.

I'm not sure we are understanding each other completely. I agree that
synchronization is needed to have the primary hub wait for its peers, that
was one of my initial concerns.

Lets use an example to clarify my secondary concern: a hub chip provides a
USB 3 and a USB 2 hub, lets say the USB 3 hub is the primary.

Here is some pseudo-code for the suspend function:

hub_suspend(hub)
  ...

  if (hub->primary) {
    device_pm_wait_for_dev(hub->peer)

    // check for connected devices and turn regulator off
  }

  ...
}

What I meant with 'asynchronous suspend' in this context:

Can hub_suspend() of the peer hub be executed (asynchronously) while the
primary is blocked on device_pm_wait_for_dev(),

Yes, that's exactly what would happen with async suspend.

 or would the primary wait
forever if the peer hub isn't suspended yet?

That wouldn't happen.  device_pm_wait_for_dev is smart; it will return 
immediately if neither device uses async suspend.  But in that case you 
could end up removing power from the peer hub before it had suspended.

That's why I said you might need to add additional synchronization.  The 
suspend routines for the two hubs could each check to see whether the 
other device had suspended yet, and the last one would handle the power 
regulator.  The additional synchronization is for the case where the two 
checks end up being concurrent.

quoted

quoted

quoted

And hubs would need to know their peers in any case, because you have to
check if any devices attached to the peer have wakeup enabled.

My concern was about all hubs (including 'secondaries') having to know their
peers and check on each other, in the scenario we are now talking about only
the "master" hub needs to know and check on its peers, which is fine.

Not all hubs would need this.  Only ones marked in DT as having a power 
regulator.

Sure, as long as the primary (with a power regulator) can wait for its peers
to suspend without the risk of blocking forever (my doubt above).

If we take this approach, we'll have to give up on the idea that the 
primary can always suspend after the peer.

Alan Stern