This refactoring allows drivers like dwc3 host glue driver to
specify thier xhci quirks.
Pavankumar Kondeti (1):
usb: xhci: refactor quirks and plat private data
Sandeep Maheswaram (1):
usb: dwc: host: add xhci_plat_priv quirk XHCI_SKIP_PHY_INIT
drivers/usb/cdns3/host.c | 2 +-
drivers/usb/dwc3/host.c | 15 +++++++++
drivers/usb/host/xhci-plat.c | 3 +-
drivers/usb/host/xhci-plat.h | 25 ---------------
drivers/usb/host/xhci-rcar.c | 3 +-
drivers/usb/host/xhci.h | 60 ++++--------------------------------
include/linux/usb/xhci-plat.h | 24 +++++++++++++++
include/linux/usb/xhci-quirks.h | 67 +++++++++++++++++++++++++++++++++++++++++
8 files changed, 117 insertions(+), 82 deletions(-)
delete mode 100644 drivers/usb/host/xhci-plat.h
create mode 100644 include/linux/usb/xhci-plat.h
create mode 100644 include/linux/usb/xhci-quirks.h
--
2.7.4
dwc3 manages PHY by own DRD driver, so skip the management by
HCD core.
During runtime suspend phy was not getting suspend because
runtime_usage value is 2.
Signed-off-by: Sandeep Maheswaram <[email protected]>
---
drivers/usb/dwc3/host.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index eda8719..4a035a8 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -13,6 +13,14 @@
#include <linux/platform_device.h>
#include "core.h"
+#include <linux/usb/hcd.h>
+#include <linux/usb/xhci-plat.h>
+#include <linux/usb/xhci-quirks.h>
+
+
+static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
+ .quirks = XHCI_SKIP_PHY_INIT,
+};
static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc,
int irq, char *name)
@@ -122,6 +130,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");
--
2.7.4
Sandeep Maheswaram <[email protected]> 于2022年2月16日周三 14:58写道：
>
> dwc3 manages PHY by own DRD driver, so skip the management by
> HCD core.
> During runtime suspend phy was not getting suspend because
> runtime_usage value is 2.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> drivers/usb/dwc3/host.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index eda8719..4a035a8 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -13,6 +13,14 @@
> #include <linux/platform_device.h>
>
> #include "core.h"
> +#include <linux/usb/hcd.h>
> +#include <linux/usb/xhci-plat.h>
> +#include <linux/usb/xhci-quirks.h>
> +
> +
> +static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
> + .quirks = XHCI_SKIP_PHY_INIT,
> +};
It's better to create this xhci_plat_priv by each dwc3 glue layer,
with that, we can use this priv to pass other flags and possibly
override APIs by each glue driver which may not apply to all dwc3
platforms.
thanks
Li Jun
>
> static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc,
> int irq, char *name)
> @@ -122,6 +130,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");
> --
> 2.7.4
>
On Wed, Feb 16, 2022 at 03:16:40PM +0800, Jun Li wrote:
> Sandeep Maheswaram <[email protected]> 于2022年2月16日周三 14:58写道：
> >
> > dwc3 manages PHY by own DRD driver, so skip the management by
> > HCD core.
> > During runtime suspend phy was not getting suspend because
> > runtime_usage value is 2.
> >
> > Signed-off-by: Sandeep Maheswaram <[email protected]>
> > ---
> > drivers/usb/dwc3/host.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> > index eda8719..4a035a8 100644
> > --- a/drivers/usb/dwc3/host.c
> > +++ b/drivers/usb/dwc3/host.c
> > @@ -13,6 +13,14 @@
> > #include <linux/platform_device.h>
> >
> > #include "core.h"
> > +#include <linux/usb/hcd.h>
> > +#include <linux/usb/xhci-plat.h>
> > +#include <linux/usb/xhci-quirks.h>
> > +
> > +
> > +static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
> > + .quirks = XHCI_SKIP_PHY_INIT,
> > +};
>
> It's better to create this xhci_plat_priv by each dwc3 glue layer,
> with that, we can use this priv to pass other flags and possibly
> override APIs by each glue driver which may not apply to all dwc3
> platforms.
>
Do you see a need for any glue driver to know about this xHC platform data?
AFAICT, glue driver has no direction connection with the dwc3 core. All
the required data is coming from dT on ARM based boards. Adding a private
interface between dwc3 core and glue for passing xhci platform data seems
to be overkill. If there is a pressing need, why not?
Thanks,
Pavan
Pavan Kondeti <[email protected]> 于2022年2月16日周三 16:00写道：
>
> On Wed, Feb 16, 2022 at 03:16:40PM +0800, Jun Li wrote:
> > Sandeep Maheswaram <[email protected]> 于2022年2月16日周三 14:58写道：
> > >
> > > dwc3 manages PHY by own DRD driver, so skip the management by
> > > HCD core.
> > > During runtime suspend phy was not getting suspend because
> > > runtime_usage value is 2.
> > >
> > > Signed-off-by: Sandeep Maheswaram <[email protected]>
> > > ---
> > > drivers/usb/dwc3/host.c | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> > > index eda8719..4a035a8 100644
> > > --- a/drivers/usb/dwc3/host.c
> > > +++ b/drivers/usb/dwc3/host.c
> > > @@ -13,6 +13,14 @@
> > > #include <linux/platform_device.h>
> > >
> > > #include "core.h"
> > > +#include <linux/usb/hcd.h>
> > > +#include <linux/usb/xhci-plat.h>
> > > +#include <linux/usb/xhci-quirks.h>
> > > +
> > > +
> > > +static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
> > > + .quirks = XHCI_SKIP_PHY_INIT,
> > > +};
> >
> > It's better to create this xhci_plat_priv by each dwc3 glue layer,
> > with that, we can use this priv to pass other flags and possibly
> > override APIs by each glue driver which may not apply to all dwc3
> > platforms.
> >
>
> Do you see a need for any glue driver to know about this xHC platform data?
Yes. I have some xhci quirks which are specifix to NXP iMX platforms.
> AFAICT, glue driver has no direction connection with the dwc3 core. All
> the required data is coming from dT on ARM based boards. Adding a private
> interface between dwc3 core and glue for passing xhci platform data seems
> to be overkill. If there is a pressing need, why not?
And looking at xhci_plat_priv members
-struct xhci_plat_priv {
- const char *firmware_name;
- unsigned long long quirks;
- int (*plat_setup)(struct usb_hcd *);
- void (*plat_start)(struct usb_hcd *);
- int (*init_quirk)(struct usb_hcd *);
- int (*suspend_quirk)(struct usb_hcd *);
- int (*resume_quirk)(struct usb_hcd *);
-};
Are we going to share the same all those quirks and APIs
implementation across all dwc3 platforms?
Thanks
Li Jun
>
> Thanks,
> Pavan
On Wed, Feb 16, 2022 at 04:49:32PM +0800, Jun Li wrote:
> Pavan Kondeti <[email protected]> 于2022年2月16日周三 16:00写道：
> >
> > On Wed, Feb 16, 2022 at 03:16:40PM +0800, Jun Li wrote:
> > > Sandeep Maheswaram <[email protected]> 于2022年2月16日周三 14:58写道：
> > > >
> > > > dwc3 manages PHY by own DRD driver, so skip the management by
> > > > HCD core.
> > > > During runtime suspend phy was not getting suspend because
> > > > runtime_usage value is 2.
> > > >
> > > > Signed-off-by: Sandeep Maheswaram <[email protected]>
> > > > ---
> > > > drivers/usb/dwc3/host.c | 15 +++++++++++++++
> > > > 1 file changed, 15 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> > > > index eda8719..4a035a8 100644
> > > > --- a/drivers/usb/dwc3/host.c
> > > > +++ b/drivers/usb/dwc3/host.c
> > > > @@ -13,6 +13,14 @@
> > > > #include <linux/platform_device.h>
> > > >
> > > > #include "core.h"
> > > > +#include <linux/usb/hcd.h>
> > > > +#include <linux/usb/xhci-plat.h>
> > > > +#include <linux/usb/xhci-quirks.h>
> > > > +
> > > > +
> > > > +static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
> > > > + .quirks = XHCI_SKIP_PHY_INIT,
> > > > +};
> > >
> > > It's better to create this xhci_plat_priv by each dwc3 glue layer,
> > > with that, we can use this priv to pass other flags and possibly
> > > override APIs by each glue driver which may not apply to all dwc3
> > > platforms.
> > >
> >
> > Do you see a need for any glue driver to know about this xHC platform data?
>
> Yes. I have some xhci quirks which are specifix to NXP iMX platforms.
>
> > AFAICT, glue driver has no direction connection with the dwc3 core. All
> > the required data is coming from dT on ARM based boards. Adding a private
> > interface between dwc3 core and glue for passing xhci platform data seems
> > to be overkill. If there is a pressing need, why not?
>
> And looking at xhci_plat_priv members
>
> -struct xhci_plat_priv {
> - const char *firmware_name;
> - unsigned long long quirks;
> - int (*plat_setup)(struct usb_hcd *);
> - void (*plat_start)(struct usb_hcd *);
> - int (*init_quirk)(struct usb_hcd *);
> - int (*suspend_quirk)(struct usb_hcd *);
> - int (*resume_quirk)(struct usb_hcd *);
> -};
>
> Are we going to share the same all those quirks and APIs
> implementation across all dwc3 platforms?
>
Currently Yes. Thats why I am asking if there is a pressing need to
make this more complex than it needs to be..
Thanks,
Pavan