On Thu, Feb 22, 2024 at 7:55 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Feb 21, 2024 at 02:40:05PM -0800, Kevin Rowland wrote:
> > Alan, first, apologies for the extremely late response.
> >
> > I successfully hacked around the original issue, things have been
> > working for a long
> > time, but now I have to revisit it. Responses inline.
> >
> > On Tue, Jun 7, 2022 at 7:46 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Mon, Jun 06, 2022 at 03:12:58PM -0700, Kevin Rowland wrote:
> > > > Hello all,
> > > >
> > > > I've got a USB 3.0 host (an NXP i.MX8QM running 5.10.72 with Cadence
> > > > XHCI host controller IP) connected to a USB device on the same PCB,
> > > > which also happens to run Linux (although I think that's beside the
> > > > point here). The quirk is that, although D+/D- are routed directly
> > > > from host to device, VBUS is actually controlled by a separate GPIO on
> > > > the host. The dedicated VBUS pin on the USB host controller is pulled
> > > > high. ID is pulled high on the PBC but driven low by a GPIO from the
> > > > i.MX8, so we can imagine it's tied to ground.
> > > >
> > > > I've made a little schematic drawing [1] to help visualize the connections.
> > > >
> > > > We've run into an issue where, on resume from STR, the following
> > > > sequence occurs:
> > > > - the GPIO peripheral on the host is powered back on, VBUS is
> > > > immediately driven high
> > >
> > > Why wasn't the GPIO turned on the whole time the system was suspended?
> > > How can remote wakeup work without VBUS power?
> > We don't currently use remote wakeup. The device is self-powered and enters
> > suspend-to-RAM itself when the host is suspended. The device can then wake
> > from an external source, at which point it will wake the host (i.MX8)
> > by asserting a
> > GPIO.
> >
> > We've discussed maintaining VBUS through suspend and using USB remote
> > wakeup from the device, but that's off the table right now. Part of
> > the reason is
> > that our SoC can't get down into its lowest power state without powering off all
> > on-chip peripherals like the USB host controller - it can, however,
> > get to its lowest
> > power state and still wake via GPIO events.
> >
> > > > - the device signals attach on DP/DN, but _I believe_ the host
> > > > controller on the host is not yet powered on
> > Slight correction, we have a 3.0 link so the device signals attach on
> > the SS lines,
> > then I think it falls back to signaling attach on DP/DN. Hopefully that doesn't
> > change things too much.
> >
> > > > - the host controller is then powered on and the {hub, hcd, xhci}
> > > > drivers all resume, but no port status change is detected; I believe
> > > > that attach signaling was missed by the host controller
>
> You mean the port is physically attached to the device but the host
> controller reports that it is not connected?
>
> As part of its resume handling, the hub driver polls all the port
> statuses. If there was a status change, it would be aware unless the
> controller's root hub simply did not report the new port status at all.
Right, in my original investigation from 18 months ago, on return from
`check_port_resume_type()` in `usb_port_resume()` I saw that BIT(0) in
both portstatus and portchange was cleared.
Then, if I hacked in an extra LOW->HIGH toggle of VBUS in
`usb_port_resume()`, right before `wait_for_connected()` /
`check_port_resume_type()`, I saw BIT(0) in both of those two port
registers was set, and enumeration succeeded. My conclusion was that
the host controller hardware was simply not powered on when VBUS was
first driven HIGH, so the hardware missed ATTACH signaling by the
device, and therefore BIT(0) remained clear. Hacking in an extra
rising edge on VBUS forced the device to signal ATTACH again, and at
that point the hardware was powered and ready to notice it.
I'm still pretty confident that that conclusion is correct, but I'm
open to clarifications.
>
> > > > I'd like for the host controller driver (or the root hub driver??) to
> > > > have explicit control of VBUS, so that it's driven high only when the
> > > > host controller regains power and is ready to detect attach signaling.
>
> In fact, the hub driver is aware of none of this. The host controller
> driver, or better, the host controller's platform driver is where all
> the knowledge about the VBUS GPIO resides.
>
> How does your resume sequence work? Does the PM core call a resume
> routine in the platform driver, which then tells the xHCI core to do its
> resume processing? If that's the case then all you have to is turn on
> the GPIO after the xHCI resume processing is finished, just before the
> platform driver's resume routine returns.
>
> If that's not how it works then you might have to add some
> platform-specific glue code to the end of the xHCI core resume routine.
> Maybe controlled by a quirk flag.
>
I'm not sure if "platform driver" here means the i.MX8-specific driver
or the glue driver in `xhci-plat.c`. I see that the glue driver in
`xhci-plat.c` registers a resume hook. That resume hook indeed calls
`xhci_resume()` to tell the xHCI core to resume.
The i.MX8-specific driver is just the `cdns3` driver, which doesn't
seem to call into the xHCI core directly. Rather, the `cdns3` driver
creates the xHCI platform device with a
`platform_device_{alloc,add}()` pair. So since the xHCI platform
device is a child of the`cdns3` device, the latter will be resumed
before the former (just because of their ordering in `dpm_list`).
Writing that out makes me think I need to drive VBUS in the glue
driver, in `xhci-plat.c:xhci_plat_resume()`, after returning from
`xhci_resume()`, exactly as you've suggested. I can structure it in
the same way as the existing `suspend_quirk()`, such that a callback
is included in the "private" data passed by the `cdns3` driver.
> > > > I see device-tree documentation about the USB connector node and
> > > > `vbus-supply`, but I'm having a hard time understanding how to square
> > > > my use-case with `drivers/usb/common/usb-conn-gpio.c`, which reacts to
> > > > state changes on VBUS or ID.
> > > >
> > > > Any thoughts on where I should stick the logic that enables VBUS on
> > > > resume? My current (very hacky) fix is to initialize a global (argh!)
> > > > gpio_desc to refer to the VBUS GPIO, then to call
> > > > `gpiod_set_value(<gpio_desc>, 0); gpiod_set_value(<gpio_desc>, 1);` in
> > > > `usb_port_resume()`, which is where I ended up when tracking the
> > > > original issue. This toggles VBUS and allows us to catch the new round
> > > > of attach signaling from the device.
> > >
> > > The hub driver already knows to turn on port power when a hub is
> > > initialized or during a reset-resume. It doesn't do so during a
> > > regular resume because it assumes power was on the whole time. You can
> > > change this, if necessary.
> > Via set_port_feature(PORT_FEAT_POWER) in hub_power_on? Does this mean
> > I should patch in some extra logic to ask the platform-specific driver
> > to assert the
> > external VBUS GPIO? I'm happy to do that, I just don't want to miss logic that's
> > already built-in to the drivers.
> >
> > If I'm reading the indirection correctly, I'll need:
> > hcd_to_xhci(bus_to_hcd(hub->hdev->bus))
> >
> > to access the struct xhci_hcd. I don't yet see how to go from there to
> > the platform
> > driver.
>
> This is a good indication that you're trying to do things in the wrong
> place. Since what you're talking about is all platform-specific stuff,
> that best place to put it all is in the platform driver.
>
Okay great, avoiding the indirection game is always nice :-)
> Alan Stern
>
> > > > I'm happy to use the fixed-regulator framework instead, I'm just not
> > > > sure which driver should own the gpio_desc / regulator and where it
> > > > should be disabled / enabled during suspend / resume.
> > >
> > > Probably whichever platform-specific driver manages your xHCI controller
> > > should be the one to interact with the GPIO. But it should make changes
> > > only when told to do so by a higher layer (such as the hub driver).
> > This helps, thanks.
> >
> > Best,
> > Kevin
> >
> >
> >
> > > Alan Stern
> > >
> > > > Best,
> > > > Kevin
> > > >
> > > > [1]
> > > >
> > > > i.MX8 device
> > > > .----------------. .-------------.
> > > > | GPIOX.Y ---|-------->| VBUS (in) |
> > > > | | | |
> > > > | USB | _ | |
> > > > | .------------. | | | |
> > > > | | VBUS ---|-|----' | |
> > > > | | DP <---|-|-------->| DP |
> > > > | | DP <---|-|-------->| DN |
> > > > | | ID ----|-|----. '-------------'
> > > > | '------------' | |
> > > > '----------------' v
