Thread (55 messages) 55 messages, 4 authors, 2020-10-09

Re: [PATCH v4 1/2] dt-bindings: usb: Add binding for discrete onboard USB hubs

From: Matthias Kaehlcke <mka@chromium.org>
Date: 2020-10-07 17:28:52
Also in: linux-usb, lkml

On Wed, Oct 07, 2020 at 12:38:38PM -0400, Alan Stern wrote:
On Wed, Oct 07, 2020 at 09:03:36AM -0700, Matthias Kaehlcke wrote:
quoted
On Tue, Oct 06, 2020 at 09:00:23PM -0400, Alan Stern wrote:
quoted
On Tue, Oct 06, 2020 at 12:25:36PM -0700, Matthias Kaehlcke wrote:
quoted
On Tue, Oct 06, 2020 at 01:15:24PM -0400, Alan Stern wrote:
quoted
You don't need a platform device or a new driver to do this.  The code 
can go in the existing hub driver.
Maybe. IIUC currently USB drivers don't support/use suspend_late. Could that
be added? It would simplify matters, otherwise all hubs need to know their
peers and check in suspend if they are the last hub standing, only then the
power can be switched off. It would be simpler if a single instance (e.g. the
hub with the DT entries) is in control.
Adding suspend_late would be a little painful.  But you don't really 
need it; you just need to make the "master" hub wait for its peer to 
suspend, which is easy to do.
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(), or would the primary wait
forever if the peer hub isn't suspended yet?
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).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help