Re: [PATCH v9 1/5] usb: host: xhci: plat: Add suspend quirk for dwc3 controller

[Matthias Kaehlcke]

Hi Sandeep,

On Mon, Nov 01, 2021 at 01:23:40PM +0530, Sandeep Maheswaram wrote:
> During suspend check if any wakeup capable devices are connected to the
> controller (directly or through hubs), and set the wakeup capable property
> for xhci plat device.
> 
> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@xxxxxxxxxxx>
> ---
>  drivers/usb/host/xhci-plat.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index c1edcc9..7ab272b 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -431,6 +431,14 @@ static int xhci_plat_remove(struct platform_device *dev)
>  	return 0;
>  }
>  
> +static void xhci_dwc3_suspend_quirk(struct usb_hcd *hcd, struct device *dev)
> +{
> +	if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> +		device_set_wakeup_capable(dev, true);
> +	else
> +		device_set_wakeup_capable(dev, false);

IIUC wakeup capability is typically a static property that reflects the
actual hardware (or firmware) support for wakeup. In that sense it doesn't
seem a good idea to change it dynamically at suspend time, depending on
what is connected to the bus. I understand though that the odd split
of the dwc3 driver makes it hard to do things properly ...

Earlier in this discussion Felipe suggested to add a function like
device_children_wakeup_capable(), to avoid having to call
usb_wakeup_enabled_descendants() from the dwc3 drivers.

Below is an initial implementation for device_children_wakeup_capable(),
could you try if calling it from dwc3_suspend/resume_common() and
dwc3_qcom_suspend() would work instead of relying on the wakeup
capability?

Thanks

Matthias