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,
> 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).
>
> Even by masking all maskable ACPI GPEs, the problem still reproduces; also
> 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,
Guilherme
