Re: [PATCH] usb: dwc3: Fix spurious wakeup when port is on device mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, Jan 09, 2024, Guilherme G. Piccoli wrote:
> On 05/12/2023 11:40, Guilherme G. Piccoli wrote:
> > Hi Thinh, thanks for your response. I'll clarify inline below:
> > 
> > On 04/12/2023 22:23, Thinh Nguyen wrote:
> >> [...]
> >>>> It was noticed that on plugging a low-power USB source on Steam
> >>>> Deck USB-C port (handled by dwc3), if such port is on device role,
> >>
> >> I'm not clear of the testing sequence here. Can you clarify further? Is
> >> this device operating as host mode but then it switches role to device
> >> mode when no device is connected?
> >>
> > 
> > Exactly this. We have a driver that changes between host/device mode
> > according to ACPI notifications on port connect. But to make
> > tests/discussion easier and eliminate more variables, we just dropped
> > this driver and do it manually.
> > 
> > So the steps are:
> > 
> > (A) host mode test
> > 1) Put the port on host mode using debugfs interface.
> > 2) Wait 30 seconds, plug a cable connecting the Steam Deck to a laptop -
> > we call this connection a "low-power source", since it seems to charge
> > slowly the Deck.

I assume there was a role switch negotiation to switch to device mode
successfully here before the next step?

> > 3) Suspend the Deck after some seconds (S3/deep) - success
> > 
> > (B) device mode test
> > 
> > 1) Put the port on device mode using debugfs interface.
> > 2) Wait 30 seconds, plug a cable connecting the Steam Deck to a laptop.
> > 3) Suspend the Deck after some seconds (S3/deep) - fails
> > 
> > 3a) If pcie_pme is using MSIs, it fails showing that a wakeup interrupt
> > is pending, in this case the Steam Deck effectively doesn't enter suspend.
> > 
> > 3b) if PCIe PME is not using MSIs, Deck suspends and right after (less
> > than a second), wakes up properly.
> > 

Your platform is DRD right? If that's the case, then it should be using
level interrupt. It should not support MSI unless it's host mode only.

> > 
> >>>> the HW somehow keep asseting the PCIe PME line and triggering a
> >>>> wakeup event on S3/deep suspend (that manifests as a PME wakeup
> >>>> interrupt, failing the suspend path). That doesn't happen when the USB
> >>>> port is on host mode or if the USB device connected is not a low-power
> >>>> type (for example, a connected keyboard doesn't reproduce that).
> >>
> >> Is the PME continuously generating non-stop? Did you test this in USB3
> >> speed? Does this happen for every low-power device or just this specific
> >> low-power device?
> > 
> > It seems PME is continuously being generated, yes. I tested by
> > connecting to my laptop as mentioned, I guess others tested different
> > scenarios, not always reproduces. An example: a keyboard or a disk
> > connected when the USB port is on device mode doesn't reproduce. Also, I
> > think I didn't test "in USB3 speed" - could you detail more, not sure if
> > I understood that properly.

I mean to ask whether this test was done while operating in SuperSpeed
or SuperSpeed Plus.

> > 
> > 
> >> [...] 
> >> Even if you masked all the interrupts, and the events are still
> >> generating? Did you check if the driver handled wakeup from PME and
> >> properly restore the controller?
> >>
> > 
> > Ok, let me clarify a bit. From the ACPI perspective, I was able to check
> > from kernel that some GPEs were generated on resume when the issue
> > happens (and potentially even when the issue doesn't happen, in host
> > mode for example). So, what I did was masking all these GPEs using the
> > kernel sysfs interface. After masking, the issue still reproduces but
> > the GPEs count doesn't increase.
> > 
> > Regarding the PME interrupt now: if MSI is used on PME, I can see an
> > increase of 1 in every suspend/resume attempt (checking
> > /proc/interrupts). Now if MSIs are not used, guess what? There was no
> > increase in the interrupt at all. I didn't mask the PME interrupt on
> > PCIe PME driver...but even with PME driver disabled, IIRC the problem
> > reproduces.
> > 
> > "Did you check if the driver handled wakeup from PME and properly
> > restore the controller?" <- I think I didn't - how do you suggest me to
> > check that?

If it's in device mode, and you mentioned PME, that means that the
device was in hibernation. I assume that you're not using the mainline
dwc3 driver if Steam Deck supports hibernation and was in hibernation
before the connection. Otherwise, PME should not be generated. If it
does, something is broken and requires a workaround (as the one you
have).

> > 
> > What I've noticed is that either the system can't suspend, or if no MSIs
> > are used on PCIe PME, it suspends and resumes right after, with success.
> > In this latter case, dwc3 works normally again, resume is successful.
> > Let me know if you want me to check any other path or function called, etc.
> > 
> > 
> >> [...]
> >>
> >> Some platforms may need a soft reset before a change to prtcapdir. This
> >> may break some setups. This seems to be a workaround and should not be
> >> treated as part of a normal flow.
> > 
> > OK, do you have any other idea of a register change that is softer than
> > changing "prtcapdir" and could prevent the issue? Also, would that
> > workaround makes sense as...a quirk?
> > 
> > We could guard it for Deck's HW exclusively, using DMI if you think it
> > does make sense...or the dwc3 quirks (already have some for AMD right?)
> > 

Yes, you can create a specific quirk for this device.

> > I'm CCing Piyush and Huang from AMD, since this is AMD HW. Any other
> > suggestions are much appreciated.
> > Thanks,
> > 
> > 
> > Guilherme
> 
> 
> Hi folks, just curious if you think the way forward would be indeed to
> quirk it based on DMI/device ID, or if should pursue another approach
> here. Suggestions are very welcome, and thanks in advance!
> 

Sorry for the delay response. Just got back from break.

Thanks,
Thinh




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux