Hi,
Sandeep Maheswaram <[email protected]> writes:
> During suspend read the status of all port and make sure the PHYs
> are in the correct mode based on current speed.
> Phy interrupt masks are set based on this mode. 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.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> 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 b1e875c..cecd278 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1123,6 +1123,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,
> +};
we're passing data using device_properties, why do you want this here?
> @@ -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);
there's a big assumption here that xhci is still alive. Why isn't this
quirk implemented in xhci-plat itself?
> +int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd)
who calls this?
--
balbi
On Wed, Apr 28, 2021 at 12:55:21PM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Sandeep Maheswaram <[email protected]> writes:
> > During suspend read the status of all port and make sure the PHYs
> > are in the correct mode based on current speed.
> > Phy interrupt masks are set based on this mode. 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.
> >
> > Signed-off-by: Sandeep Maheswaram <[email protected]>
> > ---
> > 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 b1e875c..cecd278 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -1123,6 +1123,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,
> > +};
>
> we're passing data using device_properties, why do you want this here?
>
> > @@ -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);
>
> there's a big assumption here that xhci is still alive. Why isn't this
> quirk implemented in xhci-plat itself?
That should work for determining which types of devices are connected to
the PHYs, however IIUC the xhci-plat doesn't know about the PHY topology.
Are you suggesting to move that info into the xhci-plat driver so that it
can set the corresponding PHY modes?
Hi,
Matthias Kaehlcke <[email protected]> writes:
>> > @@ -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);
>>
>> there's a big assumption here that xhci is still alive. Why isn't this
>> quirk implemented in xhci-plat itself?
>
> That should work for determining which types of devices are connected to
> the PHYs, however IIUC the xhci-plat doesn't know about the PHY topology.
> Are you suggesting to move that info into the xhci-plat driver so that it
> can set the corresponding PHY modes?
Yes, if xHCI needs to know about PHYs in order to properly configure the
PHYs, so be it :-)
--
balbi