| Message ID | 1616434280-32635-3-git-send-email-sanm@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB DWC3 host wake up support from system suspend | expand |
On Mon, Mar 22, 2021 at 11:01:18PM +0530, Sandeep Maheswaram wrote: > Adding suspend quirk function for dwc3 host which will be called > during xhci suspend. What does xhci have to do with this? And where is the user of this quirk function in this series? > Setting hs_phy_mode, ss_phy_mode , phy_power_off flags and phy mode > during host suspend. > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> > --- > drivers/usb/dwc3/core.h | 3 +++ > drivers/usb/dwc3/host.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index ce6bd84..f05546c 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1113,6 +1113,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..3db042c 100644 > --- a/drivers/usb/dwc3/host.c > +++ b/drivers/usb/dwc3/host.c > @@ -11,6 +11,13 @@ > #include <linux/platform_device.h> > > #include "core.h" > +#include "../host/xhci.h" > +#include "../host/xhci-plat.h" Hah, really? > +int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd); Didn't checkpatch complain about this? Put function prototypes in .h files please, this isn't the 1980's... thanks, greg k-h
On Mon, Mar 22, 2021 at 11:01:18PM +0530, Sandeep Maheswaram wrote: > 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. > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> > --- > drivers/usb/dwc3/core.h | 3 +++ > drivers/usb/dwc3/host.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index ce6bd84..f05546c 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1113,6 +1113,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..3db042c 100644 > --- a/drivers/usb/dwc3/host.c > +++ b/drivers/usb/dwc3/host.c > @@ -11,6 +11,13 @@ > #include <linux/platform_device.h> > > #include "core.h" > +#include "../host/xhci.h" > +#include "../host/xhci-plat.h" > +int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd); Wait, this is right below down in this file, shouldn't it be static?
On Mon, Mar 22, 2021 at 11:01:18PM +0530, Sandeep Maheswaram wrote: > 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. > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> > --- > drivers/usb/dwc3/core.h | 3 +++ > drivers/usb/dwc3/host.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index ce6bd84..f05546c 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1113,6 +1113,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..3db042c 100644 > --- a/drivers/usb/dwc3/host.c > +++ b/drivers/usb/dwc3/host.c > @@ -11,6 +11,13 @@ > #include <linux/platform_device.h> > > #include "core.h" > +#include "../host/xhci.h" > +#include "../host/xhci-plat.h" > +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 +122,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 +141,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); nit: remove empty line between the two lines above. 'reg' is read to determine the number of ports and discarded afterwards, so it makes sense to establish that relationship visually. You could (optionally) add an empty line after assigning 'num_ports'. > + 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); > + > + dwc->phy_power_off = true; it seems silly to assign this field only to overwrite it two lines later if wakeup capable descendants are connected. Making the assignment in an 'else' branch would be clearer IMO. > + > + if (usb_wakeup_enabled_descendants(hcd->self.root_hub)) > + dwc->phy_power_off = false; > + > + return 0; > +} > + > void dwc3_host_exit(struct dwc3 *dwc) > { > platform_device_unregister(dwc->xhci);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index ce6bd84..f05546c 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -1113,6 +1113,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..3db042c 100644 --- a/drivers/usb/dwc3/host.c +++ b/drivers/usb/dwc3/host.c @@ -11,6 +11,13 @@ #include <linux/platform_device.h> #include "core.h" +#include "../host/xhci.h" +#include "../host/xhci-plat.h" +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 +122,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 +141,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); + + dwc->phy_power_off = true; + + if (usb_wakeup_enabled_descendants(hcd->self.root_hub)) + dwc->phy_power_off = false; + + return 0; +} + void dwc3_host_exit(struct dwc3 *dwc) { platform_device_unregister(dwc->xhci);
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. Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org> --- drivers/usb/dwc3/core.h | 3 +++ drivers/usb/dwc3/host.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+)