On Fri, Apr 16, 2021 at 03:31:49PM +0530, Sandeep Maheswaram wrote: > Subject: usb: dwc3: host: Add suspend_quirk for dwc3 host > > Adding suspend quirk function for dwc3 host which will be called > during xhci suspend. > Setting hs_phy_mode, ss_phy_mode , phy_power_off flags and phy mode > during host suspend. This describes in other words what the code already tells us, but doesn't really explain why this change is needed. An attempt to be a bit clearer: Subject: usb: dwc3: host: Set PHY mode during suspend During suspend read the status of all port and make sure the PHYs are in the correct mode (mka@: why is it necessary to call phy_set_mode(), shouldn't the PHYs already be in the correct mode if they are operational?). Keep track of the mode of the HS PHY to be able to configure wakeup properly. Also check during suspend if any wakeup capable devices are connected to the controller (directly or through hubs), if there are none set a flag to indicate that the PHY should be powered down during suspend. Just a starting point, I'm sure it has room for improvement. > > Signed-off-by: Sandeep Maheswaram <sanm@xxxxxxxxxxxxxx> > --- > drivers/usb/dwc3/core.h | 3 +++ > drivers/usb/dwc3/host.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 62 insertions(+) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 6e9abfb..f409dc1 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1111,6 +1111,9 @@ struct dwc3 { > > bool phys_ready; > > + unsigned int hs_phy_mode; > + bool phy_power_off; > + > struct ulpi *ulpi; > bool ulpi_ready; > > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c > index f29a264..527f04c 100644 > --- a/drivers/usb/dwc3/host.c > +++ b/drivers/usb/dwc3/host.c > @@ -11,6 +11,14 @@ > #include <linux/platform_device.h> > > #include "core.h" > +#include "../host/xhci.h" > +#include "../host/xhci-plat.h" > + > +static int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd); > + > +static const struct xhci_plat_priv xhci_plat_dwc3_xhci = { > + .suspend_quirk = xhci_dwc3_suspend_quirk, > +}; > > static int dwc3_host_get_irq(struct dwc3 *dwc) > { > @@ -115,6 +123,13 @@ int dwc3_host_init(struct dwc3 *dwc) > } > } > > + ret = platform_device_add_data(xhci, &xhci_plat_dwc3_xhci, > + sizeof(struct xhci_plat_priv)); > + if (ret) { > + dev_err(dwc->dev, "failed to add data to xHCI\n"); > + goto err; > + } > + > ret = platform_device_add(xhci); > if (ret) { > dev_err(dwc->dev, "failed to register xHCI device\n"); > @@ -127,6 +142,50 @@ int dwc3_host_init(struct dwc3 *dwc) > return ret; > } > > +static void dwc3_set_phy_mode(struct usb_hcd *hcd) > +{ > + > + int i, num_ports; > + u32 reg; > + unsigned int ss_phy_mode = 0; > + struct dwc3 *dwc = dev_get_drvdata(hcd->self.controller->parent); > + struct xhci_hcd *xhci_hcd = hcd_to_xhci(hcd); > + > + dwc->hs_phy_mode = 0; > + > + reg = readl(&xhci_hcd->cap_regs->hcs_params1); > + num_ports = HCS_MAX_PORTS(reg); > + > + for (i = 0; i < num_ports; i++) { > + reg = readl(&xhci_hcd->op_regs->port_status_base + i * 0x04); > + if (reg & PORT_PE) { > + if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg)) > + dwc->hs_phy_mode |= PHY_MODE_USB_HOST_HS; > + else if (DEV_LOWSPEED(reg)) > + dwc->hs_phy_mode |= PHY_MODE_USB_HOST_LS; > + > + if (DEV_SUPERSPEED(reg)) > + ss_phy_mode |= PHY_MODE_USB_HOST_SS; > + } > + } > + phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_mode); > + phy_set_mode(dwc->usb3_generic_phy, ss_phy_mode); > +} > + > +int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd) > +{ > + struct dwc3 *dwc = dev_get_drvdata(hcd->self.controller->parent); > + > + dwc3_set_phy_mode(hcd); > + > + if (usb_wakeup_enabled_descendants(hcd->self.root_hub)) > + dwc->phy_power_off = false; > + else > + dwc->phy_power_off = true; > + > + return 0; > +} > + > void dwc3_host_exit(struct dwc3 *dwc) > { > platform_device_unregister(dwc->xhci); > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation >