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 Thinh, thanks again for the help and apologies for my delay.
Responses inline below:


On 22/01/2024 23:12, Thinh Nguyen wrote:
> [...]
> 
> The tracepoints indicated that no gadget driver was binded. So the
> controller actually never started (ie. soft-connection never happened if
> the controller doesn't start).
> 
>>
>> (2) Plugged the USB cable connecting the Deck to my laptop - results at
>> {trace,regdump}.2 and as we can see, traces are empty.
> 
> Right, because as noted above, no activity is seen.
> 

Okay, that makes sense then and explains why I see nothing related to
the Deck on my laptop! Do you know a SW way to measure that the USB port
is providing power / "energy"? It's just out of curiosity, not that we
need that in this case (knowing the lack of a gadget driver).


>> [...]
> 
> I can see the device was resumed right after.
> 
>   kworker/u32:10-1078    [002] ...1.   179.453703: dwc3_pci_suspend <-pci_pm_suspend
>   kworker/u32:10-1078    [002] ...1.   179.453704: dwc3_pci_dsm <-pci_pm_suspend
>   kworker/u32:19-1087    [005] ...1.   179.939836: dwc3_pci_resume <-dpm_run_callback
> 
>>
>> (4) [bonus] Collected the same results of 3 (after rebooting the Deck)
>> but running dwc3 with this patch/quirk - it's easy to notice in the
>> trace, as we can see the extras readl/writel prior to suspend. In this
>> case, suspend works...and results are on {trace,regdump}.4-PATCHED
> 
> Even in the patched version, device was resumed right after. The resume
> here may not trigger the system resume, but it resumed nonetheless.
> 
>    kworker/u32:6-356     [006] ...1.    69.600400: dwc3_pci_suspend <-pci_pm_suspend
>    kworker/u32:6-356     [006] ...1.    69.600401: dwc3_pci_dsm <-pci_pm_suspend
>   kworker/u32:22-1028    [001] ...1.    70.125294: dwc3_pci_resume <-dpm_run_callback
>

Yeah, in both cases the resume happened. But they differ: without the
patch, resume was automatic - the device effectively can't suspend since
it auto-resumes instantly. Whereas with the patch (scenario 4), I've
triggered the resume by pressing the power button on Steam Deck.

And ftrace timestamps unfortunately don't help with that, it's not
showing how much time the device stay suspended, so it might be
confusing and both cases could appear as the same exact scenario, even
if they are completely different (fail vs success cases).


>> [...]
> Thanks for the logs and data points. The tracepoints indicate that the
> workaround _only_ prevented the system wakeup, not the controller.
> The wakeup still happen there as you can see the controller got woken up
> immediately after request to go into suspend in both cases, patched or
> not. So the workaround you provided doesn't help keeping the controller
> in suspend.

Yeah, this is not really the case - as mentioned above, though they
appear the same in traces, without the workaround we have an immediate
auto-resume, preventing the suspend. With the patch, device suspends and
we can keep it this way for the amount of time we want, only resuming
when we press the power button (which is exactly the expected behavior).


> 
> We need to look into why that's the case, and we need to figure out
> where the source of the wake came from. Do you have a connector
> controller that can also wakeup the system?

I'm not sure about this, what do you mean by a connector controller?!


> 
> As for the workaround, if the wakeup source is the usb controller, did
> you try to disable wakeup?
> 
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index 6604845c397c..e395d7518022 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -359,7 +359,7 @@ static int dwc3_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
>  		goto err;
>  	}
>  
> -	device_init_wakeup(dev, true);
> +	device_init_wakeup(dev, false);
>  	pci_set_drvdata(pci, dwc);
>  	pm_runtime_put(dev);
>  #ifdef CONFIG_PM
> 
>  If it works, you can fine tune to only disable wakeup when in device
>  mode and enable when in host mode with device_set_wakeup_enable().
> 

Oh, great suggestion - thanks! And...it worked!

So, with your patch, I'm able to properly suspend the Deck, even with
dwc3 in device/gadget mode.

I'll try to cook a patch with this approach but restricted to this
specific USB controller and only when in gadget mode - do you think this
is the way to go?

Any other debug / logs that might be useful?
Cheers,

Guilherme




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

  Powered by Linux