| Message ID | 20200523232304.23976-8-peter.chen@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: some PM changes for cdns3 and xhci-plat | expand |
Hi > -----Original Message----- > From: Peter Chen <peter.chen@nxp.com> > Sent: 2020年5月24日 7:23 > To: balbi@kernel.org; mathias.nyman@intel.com > Cc: linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > pawell@cadence.com; rogerq@ti.com; gregkh@linuxfoundation.org; Jun Li > <jun.li@nxp.com>; Peter Chen <peter.chen@nxp.com> > Subject: [PATCH v2 7/9] usb: host: xhci-plat: add priv flag for > skip_phy_initialization > > Some DRD controllers (eg, dwc3 & cdns3) have PHY management at their own driver > to cover both device and host mode, so add one priv flag for such users to skip > PHY management from HCD core. Can this flag be set directly in __cdns3_host_init()? Li Jun > > Signed-off-by: Peter Chen <peter.chen@nxp.com> > --- > drivers/usb/host/xhci-plat.c | 8 ++++++-- drivers/usb/host/xhci-plat.h | 1 + > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index > 03d6bbe51919..a3d6cb464186 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -183,6 +183,8 @@ static int xhci_plat_probe(struct platform_device *pdev) > struct usb_hcd *hcd; > int ret; > int irq; > + struct xhci_plat_priv *priv = NULL; > + > > if (usb_disabled()) > return -ENODEV; > @@ -280,8 +282,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > priv_match = dev_get_platdata(&pdev->dev); > > if (priv_match) { > - struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); > - > + priv = hcd_to_xhci_priv(hcd); > /* Just copy data for now */ > *priv = *priv_match; > } > @@ -329,6 +330,9 @@ static int xhci_plat_probe(struct platform_device *pdev) > > hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); > xhci->shared_hcd->tpl_support = hcd->tpl_support; > + if (priv && priv->skip_phy_initialization) > + hcd->skip_phy_initialization = 1; > + > ret = usb_add_hcd(hcd, irq, IRQF_SHARED); > if (ret) > goto disable_usb_phy; > diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h index > 1fb149d1fbce..8825e8eb28d6 100644 > --- a/drivers/usb/host/xhci-plat.h > +++ b/drivers/usb/host/xhci-plat.h > @@ -13,6 +13,7 @@ > struct xhci_plat_priv { > const char *firmware_name; > unsigned long long quirks; > + unsigned int skip_phy_initialization:1; > void (*plat_start)(struct usb_hcd *); > int (*init_quirk)(struct usb_hcd *); > int (*suspend_quirk)(struct usb_hcd *); > -- > 2.17.1
On 20-05-24 07:40:34, Jun Li wrote: > Hi > > > -----Original Message----- > > From: Peter Chen <peter.chen@nxp.com> > > Sent: 2020年5月24日 7:23 > > To: balbi@kernel.org; mathias.nyman@intel.com > > Cc: linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > > pawell@cadence.com; rogerq@ti.com; gregkh@linuxfoundation.org; Jun Li > > <jun.li@nxp.com>; Peter Chen <peter.chen@nxp.com> > > Subject: [PATCH v2 7/9] usb: host: xhci-plat: add priv flag for > > skip_phy_initialization > > > > Some DRD controllers (eg, dwc3 & cdns3) have PHY management at their own driver > > to cover both device and host mode, so add one priv flag for such users to skip > > PHY management from HCD core. > > Can this flag be set directly in __cdns3_host_init()? No, since both HCD creation and usb_add_hcd invoking are at xhci_plat_probe, glue layer has no chance to change hcd flags. Peter > > Li Jun > > > > Signed-off-by: Peter Chen <peter.chen@nxp.com> > > --- > > drivers/usb/host/xhci-plat.c | 8 ++++++-- drivers/usb/host/xhci-plat.h | 1 + > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index > > 03d6bbe51919..a3d6cb464186 100644 > > --- a/drivers/usb/host/xhci-plat.c > > +++ b/drivers/usb/host/xhci-plat.c > > @@ -183,6 +183,8 @@ static int xhci_plat_probe(struct platform_device *pdev) > > struct usb_hcd *hcd; > > int ret; > > int irq; > > + struct xhci_plat_priv *priv = NULL; > > + > > > > if (usb_disabled()) > > return -ENODEV; > > @@ -280,8 +282,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > > priv_match = dev_get_platdata(&pdev->dev); > > > > if (priv_match) { > > - struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); > > - > > + priv = hcd_to_xhci_priv(hcd); > > /* Just copy data for now */ > > *priv = *priv_match; > > } > > @@ -329,6 +330,9 @@ static int xhci_plat_probe(struct platform_device *pdev) > > > > hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); > > xhci->shared_hcd->tpl_support = hcd->tpl_support; > > + if (priv && priv->skip_phy_initialization) > > + hcd->skip_phy_initialization = 1; > > + > > ret = usb_add_hcd(hcd, irq, IRQF_SHARED); > > if (ret) > > goto disable_usb_phy; > > diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h index > > 1fb149d1fbce..8825e8eb28d6 100644 > > --- a/drivers/usb/host/xhci-plat.h > > +++ b/drivers/usb/host/xhci-plat.h > > @@ -13,6 +13,7 @@ > > struct xhci_plat_priv { > > const char *firmware_name; > > unsigned long long quirks; > > + unsigned int skip_phy_initialization:1; > > void (*plat_start)(struct usb_hcd *); > > int (*init_quirk)(struct usb_hcd *); > > int (*suspend_quirk)(struct usb_hcd *); > > -- > > 2.17.1 >
> -----Original Message----- > From: Peter Chen <peter.chen@nxp.com> > Sent: 2020年5月25日 11:04 > To: Jun Li <jun.li@nxp.com> > Cc: balbi@kernel.org; mathias.nyman@intel.com; linux-usb@vger.kernel.org; > dl-linux-imx <linux-imx@nxp.com>; pawell@cadence.com; rogerq@ti.com; > gregkh@linuxfoundation.org > Subject: Re: [PATCH v2 7/9] usb: host: xhci-plat: add priv flag for > skip_phy_initialization > > On 20-05-24 07:40:34, Jun Li wrote: > > Hi > > > > > -----Original Message----- > > > From: Peter Chen <peter.chen@nxp.com> > > > Sent: 2020年5月24日 7:23 > > > To: balbi@kernel.org; mathias.nyman@intel.com > > > Cc: linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > > > pawell@cadence.com; rogerq@ti.com; gregkh@linuxfoundation.org; Jun > > > Li <jun.li@nxp.com>; Peter Chen <peter.chen@nxp.com> > > > Subject: [PATCH v2 7/9] usb: host: xhci-plat: add priv flag for > > > skip_phy_initialization > > > > > > Some DRD controllers (eg, dwc3 & cdns3) have PHY management at their > > > own driver to cover both device and host mode, so add one priv flag > > > for such users to skip PHY management from HCD core. > > > > Can this flag be set directly in __cdns3_host_init()? > > No, since both HCD creation and usb_add_hcd invoking are at xhci_plat_probe, glue > layer has no chance to change hcd flags. I see after patch[2/9], @@ -43,6 +45,11 @@ static int __cdns3_host_init(struct cdns3 *cdns) goto err1; } + /* Glue needs to access xHCI region register for Power management */ + hcd = platform_get_drvdata(xhci); + if (hcd) + cdns->xhci_regs = hcd->regs; + You already can access hcd directly. Li Jun > > Peter > > > > Li Jun > > > > > > Signed-off-by: Peter Chen <peter.chen@nxp.com> > > > --- > > > drivers/usb/host/xhci-plat.c | 8 ++++++-- > > > drivers/usb/host/xhci-plat.h | 1 + > > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/usb/host/xhci-plat.c > > > b/drivers/usb/host/xhci-plat.c index > > > 03d6bbe51919..a3d6cb464186 100644 > > > --- a/drivers/usb/host/xhci-plat.c > > > +++ b/drivers/usb/host/xhci-plat.c > > > @@ -183,6 +183,8 @@ static int xhci_plat_probe(struct platform_device *pdev) > > > struct usb_hcd *hcd; > > > int ret; > > > int irq; > > > + struct xhci_plat_priv *priv = NULL; > > > + > > > > > > if (usb_disabled()) > > > return -ENODEV; > > > @@ -280,8 +282,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > > > priv_match = dev_get_platdata(&pdev->dev); > > > > > > if (priv_match) { > > > - struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); > > > - > > > + priv = hcd_to_xhci_priv(hcd); > > > /* Just copy data for now */ > > > *priv = *priv_match; > > > } > > > @@ -329,6 +330,9 @@ static int xhci_plat_probe(struct > > > platform_device *pdev) > > > > > > hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); > > > xhci->shared_hcd->tpl_support = hcd->tpl_support; > > > + if (priv && priv->skip_phy_initialization) > > > + hcd->skip_phy_initialization = 1; > > > + > > > ret = usb_add_hcd(hcd, irq, IRQF_SHARED); > > > if (ret) > > > goto disable_usb_phy; > > > diff --git a/drivers/usb/host/xhci-plat.h > > > b/drivers/usb/host/xhci-plat.h index > > > 1fb149d1fbce..8825e8eb28d6 100644 > > > --- a/drivers/usb/host/xhci-plat.h > > > +++ b/drivers/usb/host/xhci-plat.h > > > @@ -13,6 +13,7 @@ > > > struct xhci_plat_priv { > > > const char *firmware_name; > > > unsigned long long quirks; > > > + unsigned int skip_phy_initialization:1; > > > void (*plat_start)(struct usb_hcd *); > > > int (*init_quirk)(struct usb_hcd *); > > > int (*suspend_quirk)(struct usb_hcd *); > > > -- > > > 2.17.1 > > > > -- > > Thanks, > Peter Chen
On 20-05-25 12:30:49, Jun Li wrote: > > > > -----Original Message----- > > From: Peter Chen <peter.chen@nxp.com> > > Sent: 2020年5月25日 11:04 > > To: Jun Li <jun.li@nxp.com> > > Cc: balbi@kernel.org; mathias.nyman@intel.com; linux-usb@vger.kernel.org; > > dl-linux-imx <linux-imx@nxp.com>; pawell@cadence.com; rogerq@ti.com; > > gregkh@linuxfoundation.org > > Subject: Re: [PATCH v2 7/9] usb: host: xhci-plat: add priv flag for > > skip_phy_initialization > > > > On 20-05-24 07:40:34, Jun Li wrote: > > > Hi > > > > > > > -----Original Message----- > > > > From: Peter Chen <peter.chen@nxp.com> > > > > Sent: 2020年5月24日 7:23 > > > > To: balbi@kernel.org; mathias.nyman@intel.com > > > > Cc: linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > > > > pawell@cadence.com; rogerq@ti.com; gregkh@linuxfoundation.org; Jun > > > > Li <jun.li@nxp.com>; Peter Chen <peter.chen@nxp.com> > > > > Subject: [PATCH v2 7/9] usb: host: xhci-plat: add priv flag for > > > > skip_phy_initialization > > > > > > > > Some DRD controllers (eg, dwc3 & cdns3) have PHY management at their > > > > own driver to cover both device and host mode, so add one priv flag > > > > for such users to skip PHY management from HCD core. > > > > > > Can this flag be set directly in __cdns3_host_init()? > > > > No, since both HCD creation and usb_add_hcd invoking are at xhci_plat_probe, glue > > layer has no chance to change hcd flags. > > I see after patch[2/9], > > @@ -43,6 +45,11 @@ static int __cdns3_host_init(struct cdns3 *cdns) > goto err1; > } > > + /* Glue needs to access xHCI region register for Power management */ > + hcd = platform_get_drvdata(xhci); > + if (hcd) > + cdns->xhci_regs = hcd->regs; > + > > You already can access hcd directly. > At that time, usb_add_hcd was called, and the power was powered on if the skip flat was not set.
On 24.5.2020 2.23, Peter Chen wrote: > Some DRD controllers (eg, dwc3 & cdns3) have PHY management at > their own driver to cover both device and host mode, so add one > priv flag for such users to skip PHY management from HCD core. > > Signed-off-by: Peter Chen <peter.chen@nxp.com> > --- > drivers/usb/host/xhci-plat.c | 8 ++++++-- > drivers/usb/host/xhci-plat.h | 1 + > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index 03d6bbe51919..a3d6cb464186 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -183,6 +183,8 @@ static int xhci_plat_probe(struct platform_device *pdev) > struct usb_hcd *hcd; > int ret; > int irq; > + struct xhci_plat_priv *priv = NULL; > + > > if (usb_disabled()) > return -ENODEV; > @@ -280,8 +282,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > priv_match = dev_get_platdata(&pdev->dev); > > if (priv_match) { > - struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);h > - > + priv = hcd_to_xhci_priv(hcd); > /* Just copy data for now */ > *priv = *priv_match; > } > @@ -329,6 +330,9 @@ static int xhci_plat_probe(struct platform_device *pdev) > > hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); > xhci->shared_hcd->tpl_support = hcd->tpl_support; > + if (priv && priv->skip_phy_initialization) > + hcd->skip_phy_initialization = 1; > + > ret = usb_add_hcd(hcd, irq, IRQF_SHARED); > if (ret) > goto disable_usb_phy; > diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h > index 1fb149d1fbce..8825e8eb28d6 100644 > --- a/drivers/usb/host/xhci-plat.h > +++ b/drivers/usb/host/xhci-plat.h > @@ -13,6 +13,7 @@ > struct xhci_plat_priv { > const char *firmware_name; > unsigned long long quirks; > + unsigned int skip_phy_initialization:1; Any specific reason why this approach was chosen instead of adding a new flag to the "long long quirks" above? -Mathias
> > On 24.5.2020 2.23, Peter Chen wrote: > > Some DRD controllers (eg, dwc3 & cdns3) have PHY management at their > > own driver to cover both device and host mode, so add one priv flag > > for such users to skip PHY management from HCD core. > > > > Signed-off-by: Peter Chen <peter.chen@nxp.com> > > --- > > drivers/usb/host/xhci-plat.c | 8 ++++++-- > > drivers/usb/host/xhci-plat.h | 1 + > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-plat.c > > b/drivers/usb/host/xhci-plat.c index 03d6bbe51919..a3d6cb464186 100644 > > --- a/drivers/usb/host/xhci-plat.c > > +++ b/drivers/usb/host/xhci-plat.c > > @@ -183,6 +183,8 @@ static int xhci_plat_probe(struct platform_device *pdev) > > struct usb_hcd *hcd; > > int ret; > > int irq; > > + struct xhci_plat_priv *priv = NULL; > > + > > > > if (usb_disabled()) > > return -ENODEV; > > @@ -280,8 +282,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > > priv_match = dev_get_platdata(&pdev->dev); > > > > if (priv_match) { > > - struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);h > > - > > + priv = hcd_to_xhci_priv(hcd); > > /* Just copy data for now */ > > *priv = *priv_match; > > } > > @@ -329,6 +330,9 @@ static int xhci_plat_probe(struct platform_device > > *pdev) > > > > hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); > > xhci->shared_hcd->tpl_support = hcd->tpl_support; > > + if (priv && priv->skip_phy_initialization) > > + hcd->skip_phy_initialization = 1; > > + > > ret = usb_add_hcd(hcd, irq, IRQF_SHARED); > > if (ret) > > goto disable_usb_phy; > > diff --git a/drivers/usb/host/xhci-plat.h > > b/drivers/usb/host/xhci-plat.h index 1fb149d1fbce..8825e8eb28d6 100644 > > --- a/drivers/usb/host/xhci-plat.h > > +++ b/drivers/usb/host/xhci-plat.h > > @@ -13,6 +13,7 @@ > > struct xhci_plat_priv { > > const char *firmware_name; > > unsigned long long quirks; > > + unsigned int skip_phy_initialization:1; > > Any specific reason why this approach was chosen instead of adding a new flag to > the "long long quirks" above? I thought the quirks are for hardware, but it seems it could be used for software too. I will change it by using quirks, thanks. Peter
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 03d6bbe51919..a3d6cb464186 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -183,6 +183,8 @@ static int xhci_plat_probe(struct platform_device *pdev) struct usb_hcd *hcd; int ret; int irq; + struct xhci_plat_priv *priv = NULL; + if (usb_disabled()) return -ENODEV; @@ -280,8 +282,7 @@ static int xhci_plat_probe(struct platform_device *pdev) priv_match = dev_get_platdata(&pdev->dev); if (priv_match) { - struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); - + priv = hcd_to_xhci_priv(hcd); /* Just copy data for now */ *priv = *priv_match; } @@ -329,6 +330,9 @@ static int xhci_plat_probe(struct platform_device *pdev) hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); xhci->shared_hcd->tpl_support = hcd->tpl_support; + if (priv && priv->skip_phy_initialization) + hcd->skip_phy_initialization = 1; + ret = usb_add_hcd(hcd, irq, IRQF_SHARED); if (ret) goto disable_usb_phy; diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h index 1fb149d1fbce..8825e8eb28d6 100644 --- a/drivers/usb/host/xhci-plat.h +++ b/drivers/usb/host/xhci-plat.h @@ -13,6 +13,7 @@ struct xhci_plat_priv { const char *firmware_name; unsigned long long quirks; + unsigned int skip_phy_initialization:1; void (*plat_start)(struct usb_hcd *); int (*init_quirk)(struct usb_hcd *); int (*suspend_quirk)(struct usb_hcd *);
Some DRD controllers (eg, dwc3 & cdns3) have PHY management at their own driver to cover both device and host mode, so add one priv flag for such users to skip PHY management from HCD core. Signed-off-by: Peter Chen <peter.chen@nxp.com> --- drivers/usb/host/xhci-plat.c | 8 ++++++-- drivers/usb/host/xhci-plat.h | 1 + 2 files changed, 7 insertions(+), 2 deletions(-)