On Tue, Sep 29, 2020 at 04:17:29PM +0530, Sandeep Maheswaram wrote:
> Adding suspend quirk function for dwc3 host which will be called
> during xhci suspend.
> Setting hs_phy_flags, ss_phy_flags and phy mode during host suspend.
> 
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
>  drivers/usb/dwc3/host.c | 49 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index e195176..7f316fa 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,41 @@ int dwc3_host_init(struct dwc3 *dwc)
>       return ret;
>  }
>  
> +static void dwc3_set_phy_speed_flags(struct usb_hcd *hcd)

The main thing this function does is setting the PHY mode (see
the phy_set_mode() calls), please rename it to dwc3_set_phy_mode()
to reflect this.

> +{
> +
> +     int i, num_ports;
> +     u32 reg;
> +     struct device *dev = hcd->self.controller;
> +     struct dwc3 *dwc = dev_get_drvdata(dev->parent);
> +     struct xhci_hcd *xhci_hcd = hcd_to_xhci(hcd);
> +
> +     dwc->hs_phy_flags = 0;

What about 'dwc->ss_phy_flags'?

I suggested in another patch to use a local variable instead, so you
probably have to initialize it anyway or the compiler will be unhappy ;-)

> +
> +     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_flags |= PHY_MODE_USB_HOST_HS;
> +                     else if (DEV_LOWSPEED(reg))
> +                             dwc->hs_phy_flags |= PHY_MODE_USB_HOST_LS;

nit: add empty line to visually separate HS from SS

> +                     if (DEV_SUPERSPEED(reg))
> +                             dwc->ss_phy_flags |= PHY_MODE_USB_HOST_SS;
> +             }
> +     }
> +     phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
> +     phy_set_mode(dwc->usb3_generic_phy, dwc->ss_phy_flags);

Check return values and return any errors?

Reply via email to