Hi,
On Mon, Dec 04, 2023, Guilherme G. Piccoli wrote:
> On 22/11/2023 13:51, Guilherme G. Piccoli 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?
> > 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?
> >
> > Even by masking all maskable ACPI GPEs, the problem still reproduces; also
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?
> > by having the PCIe PME mechanism using non-MSI interrupts, the symptom is
> > observed as the HW succeeding to S3/suspend but waking up right after.
> >
> > By changing the PRTCAP mode to DWC3_GCTL_PRTCAP_HOST in the controller
> > (as one of the latest steps on gadget common suspend), we managed to
> > circumvent the issue. The mode restore is already done in the common
> > resume function. Notice that this patch does not change the in-driver
> > port state on suspend, or else the resume path "thinks" the port was
> > in host mode prior to suspend and resume it on a wrong fashion.
> >
> > Cc: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>
> > Cc: Vivek Dasmohapatra <vivek@xxxxxxxxxxxxx>
> > Signed-off-by: Guilherme G. Piccoli <gpiccoli@xxxxxxxxxx>
> > ---
> >
> >
> > Hi folks, first of all thanks in advance for reviews/feedback on this one.
> >
> > This patch is the result of a debug approach with no datasheet access.
> > With that said, I'm not 100% sure writing to this register is free of
> > undesired side-effects; one thing I'm considering is the following scenario:
> > imagine a device A with the USB port on device/peripheral mode, and a device B
> > connected to it, acting as host. What if we want device B be able to wakeup
> > device A? Would this patch prevent that for all cases, always?
> >
> > I appreciate ideas in case this is not the best approach to fix the
> > problem. We could also gate this approach to the HW board as a first step,
> > for example.
> >
> > Thanks,
> >
> >
> > Guilherme
> >
> >
> > drivers/usb/dwc3/core.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 0328c86ef806..5801d3ebbbcb 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -104,7 +104,7 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
> > return 0;
> > }
> >
> > -void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
> > +static void __dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
> > {
> > u32 reg;
> >
> > @@ -112,7 +112,11 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
> > reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG));
> > reg |= DWC3_GCTL_PRTCAPDIR(mode);
> > dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> > +}
> >
> > +void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
> > +{
> > + __dwc3_set_prtcap(dwc, mode);
> > dwc->current_dr_role = mode;
> > }
> >
> > @@ -2128,6 +2132,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> > break;
> > dwc3_gadget_suspend(dwc);
> > synchronize_irq(dwc->irq_gadget);
> > + __dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
> > dwc3_core_exit(dwc);
> > break;
> > case DWC3_GCTL_PRTCAP_HOST:
>
> Hi folks, friendly ping on this one.
> Thanks,
>
>
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.
BR,
Thinh
