Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Mathias Nyman]

On 11.04.2016 18:44, Peter Griffin wrote:

Hi Mathias,

On Fri, 25 Mar 2016, Peter Griffin wrote:


Otherwise generic-xhci and xhci-platform which have no data get wrongly
detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().

This fixes a regression in v4.5 for STiH407 family SoC's which use the
synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
gets added.

I suspect this will also fix other dwc3 DT platforms such as Exynos,
although I've only tested on STih410 SoC.

Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")


Can you take this 1 charcter fix for the next -rc?



added to my tree and sent forward to Greg

-Mathias

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Mathias Nyman]

On 11.04.2016 18:44, Peter Griffin wrote:

Hi Mathias,

On Fri, 25 Mar 2016, Peter Griffin wrote:


Otherwise generic-xhci and xhci-platform which have no data get wrongly
detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().

This fixes a regression in v4.5 for STiH407 family SoC's which use the
synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
gets added.

I suspect this will also fix other dwc3 DT platforms such as Exynos,
although I've only tested on STih410 SoC.

Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")


Can you take this 1 charcter fix for the next -rc?



added to my tree and sent forward to Greg

-Mathias

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Peter Griffin]

Hi Mathias,

On Fri, 25 Mar 2016, Peter Griffin wrote:

> Otherwise generic-xhci and xhci-platform which have no data get wrongly
> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
> 
> This fixes a regression in v4.5 for STiH407 family SoC's which use the
> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
> gets added.
> 
> I suspect this will also fix other dwc3 DT platforms such as Exynos,
> although I've only tested on STih410 SoC.
> 
> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")

Can you take this 1 charcter fix for the next -rc?

It fixes a regression introduced in commit 4efb2f694114 for usb3 for some
DT platforms.

Then Felipes series which re-works this code will hopefully be merged in the
next merge window.

regards,

Peter.

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Peter Griffin]

Hi Mathias,

On Fri, 25 Mar 2016, Peter Griffin wrote:

> Otherwise generic-xhci and xhci-platform which have no data get wrongly
> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
> 
> This fixes a regression in v4.5 for STiH407 family SoC's which use the
> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
> gets added.
> 
> I suspect this will also fix other dwc3 DT platforms such as Exynos,
> although I've only tested on STih410 SoC.
> 
> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")

Can you take this 1 charcter fix for the next -rc?

It fixes a regression introduced in commit 4efb2f694114 for usb3 for some
DT platforms.

Then Felipes series which re-works this code will hopefully be merged in the
next merge window.

regards,

Peter.

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Lee Jones]

On Sat, 26 Mar 2016, Felipe Balbi wrote:
> Peter Griffin  writes:
> > On Fri, 25 Mar 2016, Felipe Balbi wrote:
> >> Gregory CLEMENT  writes:
> >> >> Peter Griffin  writes:
> >> >>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
> >> >>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
> >> >>>
> >> >>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
> >> >>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
> >> >>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd 
> >> >>> never
> >> >>> gets added.
> >> >>>
> >> >>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
> >> >>> although I've only tested on STih410 SoC.
> >> >>>
> >> >>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
> >> >>> Cc: sta...@vger.kernel.org
> >> >>> Cc: gregory.clem...@free-electrons.com
> >> >>> Cc: yoshihiro.shimoda...@renesas.com
> >> >>> Signed-off-by: Peter Griffin 
> >> >>> ---
> >> >>>  drivers/usb/host/xhci-plat.h | 2 +-
> >> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>>
> >> >>> diff --git a/drivers/usb/host/xhci-plat.h 
> >> >>> b/drivers/usb/host/xhci-plat.h
> >> >>> index 5a2e2e3..529c3c4 100644
> >> >>> --- a/drivers/usb/host/xhci-plat.h
> >> >>> +++ b/drivers/usb/host/xhci-plat.h
> >> >>> @@ -14,7 +14,7 @@
> >> >>>  #include "xhci.h" /* for hcd_to_xhci() */
> >> >>>  
> >> >>>  enum xhci_plat_type {
> >> >>> -  XHCI_PLAT_TYPE_MARVELL_ARMADA,
> >> >>> +  XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
> >> >>>XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
> >> >>>XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
> >> >>
> >> >> aren't these platforms using device tree ? Why aren't these just
> >> >> different compatible strings ?
> >> >
> >> > According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
> >> > xhci-plat: add struct xhci_plat_priv" :
> >> >
> >> > This patch adds struct xhci_plat_priv to simplify the code to match
> >> > platform specific variables. For now, this patch adds a member "type" in
> >> > the structure
> >> 
> >> that's fine but the answer doesn't exactly match my question ;-)
> >> 
> >> My point is that this enum shouldn't be necessary at all. We have
> >> compatible flags to make these checks instead. How about below ?
> >> (untested, uncompiled, yada yada yada). Note that we DON'T need this
> >> xhci_plat_type trickery, just need to be a little bit smarter about how
> >> we use driver_data:
> >
> > Your solution certainly looks more elegant.
> 
> cool thanks. Now that I think about this more carefully, we might wanna
> take $subject anyway for the -rc and get my version applied for v4.7
> merge window. What do you think ?

+1 for a simple/quick fix.

Acked-by: Lee Jones 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Lee Jones]

On Sat, 26 Mar 2016, Felipe Balbi wrote:
> Peter Griffin  writes:
> > On Fri, 25 Mar 2016, Felipe Balbi wrote:
> >> Gregory CLEMENT  writes:
> >> >> Peter Griffin  writes:
> >> >>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
> >> >>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
> >> >>>
> >> >>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
> >> >>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
> >> >>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd 
> >> >>> never
> >> >>> gets added.
> >> >>>
> >> >>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
> >> >>> although I've only tested on STih410 SoC.
> >> >>>
> >> >>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
> >> >>> Cc: sta...@vger.kernel.org
> >> >>> Cc: gregory.clem...@free-electrons.com
> >> >>> Cc: yoshihiro.shimoda...@renesas.com
> >> >>> Signed-off-by: Peter Griffin 
> >> >>> ---
> >> >>>  drivers/usb/host/xhci-plat.h | 2 +-
> >> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>>
> >> >>> diff --git a/drivers/usb/host/xhci-plat.h 
> >> >>> b/drivers/usb/host/xhci-plat.h
> >> >>> index 5a2e2e3..529c3c4 100644
> >> >>> --- a/drivers/usb/host/xhci-plat.h
> >> >>> +++ b/drivers/usb/host/xhci-plat.h
> >> >>> @@ -14,7 +14,7 @@
> >> >>>  #include "xhci.h" /* for hcd_to_xhci() */
> >> >>>  
> >> >>>  enum xhci_plat_type {
> >> >>> -  XHCI_PLAT_TYPE_MARVELL_ARMADA,
> >> >>> +  XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
> >> >>>XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
> >> >>>XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
> >> >>
> >> >> aren't these platforms using device tree ? Why aren't these just
> >> >> different compatible strings ?
> >> >
> >> > According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
> >> > xhci-plat: add struct xhci_plat_priv" :
> >> >
> >> > This patch adds struct xhci_plat_priv to simplify the code to match
> >> > platform specific variables. For now, this patch adds a member "type" in
> >> > the structure
> >> 
> >> that's fine but the answer doesn't exactly match my question ;-)
> >> 
> >> My point is that this enum shouldn't be necessary at all. We have
> >> compatible flags to make these checks instead. How about below ?
> >> (untested, uncompiled, yada yada yada). Note that we DON'T need this
> >> xhci_plat_type trickery, just need to be a little bit smarter about how
> >> we use driver_data:
> >
> > Your solution certainly looks more elegant.
> 
> cool thanks. Now that I think about this more carefully, we might wanna
> take $subject anyway for the -rc and get my version applied for v4.7
> merge window. What do you think ?

+1 for a simple/quick fix.

Acked-by: Lee Jones 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

RE: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Yoshihiro Shimoda]

Hi,

> Sent: Monday, March 28, 2016 5:30 PM
> 
> Hi,
> 
> Yoshihiro Shimoda  writes:
> >> > ps: there might be bugs there, but it's a holiday and I really shouldn't
> >> > be spending time on this right now ;-)
> >>
> >> I'm also off on holiday now until Sunday 10th April... yay :-)
> >> >
> >> > Anyway, have fun testing. Let me know if it doesn't work.
> >>
> >> I only have access to STi platforms which were broken by this change.
> >> Not any of the platforms which rely on the functionality which
> >> was introduced (although I can't see any reason why your patch wouldn't 
> >> work).
> >>
> >> Maybe Yoshihiro (on CC) could test this on the Renesas platforms and 
> >> confirm?
> >
> > Thank you for sending the email to me on CC.
> >
> > I tested Felipe's patch on Renesas platfroms (R-Car Gen2 and Gen3) and
> > I fixed the patch like the following.
> >
> > However, my fixes patch might need to clean the code up more.
> >
> > Changes from Felipe's patch:
> > - Change function names of xhci_rcar_init_quirk() to xhci_rcar_setup_quirk()
> 
> I'm not sure renaming that function fits on the same patch ;-) Sounds
> like it should be a separate patch altogether. I'll work on this
> tomorrow if it's okay for you guys to test on your respective platforms :-)

Thank you for the comment!
I also think it should be separate patch ;)
Also I'm okay to test your patch(es) on my platforms :)

Best regards,
Yoshihiro Shimoda

> --
> balbi

RE: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Yoshihiro Shimoda]

Hi,

> Sent: Monday, March 28, 2016 5:30 PM
> 
> Hi,
> 
> Yoshihiro Shimoda  writes:
> >> > ps: there might be bugs there, but it's a holiday and I really shouldn't
> >> > be spending time on this right now ;-)
> >>
> >> I'm also off on holiday now until Sunday 10th April... yay :-)
> >> >
> >> > Anyway, have fun testing. Let me know if it doesn't work.
> >>
> >> I only have access to STi platforms which were broken by this change.
> >> Not any of the platforms which rely on the functionality which
> >> was introduced (although I can't see any reason why your patch wouldn't 
> >> work).
> >>
> >> Maybe Yoshihiro (on CC) could test this on the Renesas platforms and 
> >> confirm?
> >
> > Thank you for sending the email to me on CC.
> >
> > I tested Felipe's patch on Renesas platfroms (R-Car Gen2 and Gen3) and
> > I fixed the patch like the following.
> >
> > However, my fixes patch might need to clean the code up more.
> >
> > Changes from Felipe's patch:
> > - Change function names of xhci_rcar_init_quirk() to xhci_rcar_setup_quirk()
> 
> I'm not sure renaming that function fits on the same patch ;-) Sounds
> like it should be a separate patch altogether. I'll work on this
> tomorrow if it's okay for you guys to test on your respective platforms :-)

Thank you for the comment!
I also think it should be separate patch ;)
Also I'm okay to test your patch(es) on my platforms :)

Best regards,
Yoshihiro Shimoda

> --
> balbi

RE: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Felipe Balbi]

Hi,

Yoshihiro Shimoda  writes:
>> > ps: there might be bugs there, but it's a holiday and I really shouldn't
>> > be spending time on this right now ;-)
>> 
>> I'm also off on holiday now until Sunday 10th April... yay :-)
>> >
>> > Anyway, have fun testing. Let me know if it doesn't work.
>> 
>> I only have access to STi platforms which were broken by this change.
>> Not any of the platforms which rely on the functionality which
>> was introduced (although I can't see any reason why your patch wouldn't 
>> work).
>> 
>> Maybe Yoshihiro (on CC) could test this on the Renesas platforms and confirm?
>
> Thank you for sending the email to me on CC.
>
> I tested Felipe's patch on Renesas platfroms (R-Car Gen2 and Gen3) and
> I fixed the patch like the following.
>
> However, my fixes patch might need to clean the code up more.
>
> Changes from Felipe's patch:
> - Change function names of xhci_rcar_init_quirk() to xhci_rcar_setup_quirk()

I'm not sure renaming that function fits on the same patch ;-) Sounds
like it should be a separate patch altogether. I'll work on this
tomorrow if it's okay for you guys to test on your respective platforms :-)

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Felipe Balbi]

Hi,

Yoshihiro Shimoda  writes:
>> > ps: there might be bugs there, but it's a holiday and I really shouldn't
>> > be spending time on this right now ;-)
>> 
>> I'm also off on holiday now until Sunday 10th April... yay :-)
>> >
>> > Anyway, have fun testing. Let me know if it doesn't work.
>> 
>> I only have access to STi platforms which were broken by this change.
>> Not any of the platforms which rely on the functionality which
>> was introduced (although I can't see any reason why your patch wouldn't 
>> work).
>> 
>> Maybe Yoshihiro (on CC) could test this on the Renesas platforms and confirm?
>
> Thank you for sending the email to me on CC.
>
> I tested Felipe's patch on Renesas platfroms (R-Car Gen2 and Gen3) and
> I fixed the patch like the following.
>
> However, my fixes patch might need to clean the code up more.
>
> Changes from Felipe's patch:
> - Change function names of xhci_rcar_init_quirk() to xhci_rcar_setup_quirk()

I'm not sure renaming that function fits on the same patch ;-) Sounds
like it should be a separate patch altogether. I'll work on this
tomorrow if it's okay for you guys to test on your respective platforms :-)

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Yoshihiro Shimoda]

Hi,

> Sent: Saturday, March 26, 2016 6:11 PM
< snip >
> > ps: there might be bugs there, but it's a holiday and I really shouldn't
> > be spending time on this right now ;-)
> 
> I'm also off on holiday now until Sunday 10th April... yay :-)
> >
> > Anyway, have fun testing. Let me know if it doesn't work.
> 
> I only have access to STi platforms which were broken by this change.
> Not any of the platforms which rely on the functionality which
> was introduced (although I can't see any reason why your patch wouldn't work).
> 
> Maybe Yoshihiro (on CC) could test this on the Renesas platforms and confirm?

Thank you for sending the email to me on CC.
I tested Felipe's patch on Renesas platfroms (R-Car Gen2 and Gen3) and I fixed 
the patch like the following.
However, my fixes patch might need to clean the code up more.

Changes from Felipe's patch:
- Change function names of xhci_rcar_init_quirk() to xhci_rcar_setup_quirk()
- Add setup_quirk() member and use it for rcar's function.
- Some minor fixes.

---
diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
index 1eefc98..1ea6c18 100644
--- a/drivers/usb/host/xhci-mvebu.c
+++ b/drivers/usb/host/xhci-mvebu.c
@@ -41,8 +41,9 @@ static void xhci_mvebu_mbus_config(void __iomem *base,
}
 }
 
-int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
+int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
 {
+   struct platform_device *pdev = to_platform_device(hcd->self.controller);
struct resource *res;
void __iomem *base;
const struct mbus_dram_target_info *dram;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d39d6bf..d28513a 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -47,43 +47,57 @@ static void xhci_plat_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_PLAT;
 }
 
+static void xhci_priv_plat_start(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (priv->plat_start)
+   priv->plat_start(hcd);
+}
+
+static int xhci_priv_setup_quirk(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (!priv->setup_quirk)
+   return 0;
+
+   return priv->setup_quirk(hcd);
+}
+
 /* called during probe() after chip reset completes */
 static int xhci_plat_setup(struct usb_hcd *hcd)
 {
int ret;
 
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
-   ret = xhci_rcar_init_quirk(hcd);
-   if (ret)
-   return ret;
-   }
+   ret = xhci_priv_setup_quirk(hcd);
+   if (ret)
+   return ret;
 
return xhci_gen_setup(hcd, xhci_plat_quirks);
 }
 
 static int xhci_plat_start(struct usb_hcd *hcd)
 {
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
-   xhci_rcar_start(hcd);
-
+   xhci_priv_plat_start(hcd);
return xhci_run(hcd);
 }
 
 #ifdef CONFIG_OF
 static const struct xhci_plat_priv xhci_plat_marvell_armada = {
-   .type = XHCI_PLAT_TYPE_MARVELL_ARMADA,
+   .init_quirk = xhci_mvebu_mbus_init_quirk,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
-   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
+   .setup_quirk = xhci_rcar_setup_quirk,
+   .plat_start = xhci_rcar_start,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
-   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V2,
+   .setup_quirk = xhci_rcar_setup_quirk,
+   .plat_start = xhci_rcar_start,
 };
 
 static const struct of_device_id usb_xhci_of_match[] = {
@@ -119,6 +133,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 {
struct device_node  *node = pdev->dev.of_node;
struct usb_xhci_pdata   *pdata = dev_get_platdata(>dev);
+   struct xhci_plat_priv   *priv;
const struct of_device_id *match;
const struct hc_driver  *driver;
struct xhci_hcd *xhci;
@@ -178,18 +193,18 @@ static int xhci_plat_probe(struct platform_device *pdev)
}
 
xhci = hcd_to_xhci(hcd);
+   priv = hcd_to_xhci_priv(hcd);
match = of_match_node(usb_xhci_of_match, node);
if (match) {
const struct xhci_plat_priv *priv_match = match->data;
-   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
 
/* Just copy data for now */
if (priv_match)
*priv = *priv_match;
}
 
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_MARVELL_ARMADA)) {
-   ret = xhci_mvebu_mbus_init_quirk(pdev);
+   if (priv->init_quirk) {
+   ret = 

RE: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Yoshihiro Shimoda]

Hi,

> Sent: Saturday, March 26, 2016 6:11 PM
< snip >
> > ps: there might be bugs there, but it's a holiday and I really shouldn't
> > be spending time on this right now ;-)
> 
> I'm also off on holiday now until Sunday 10th April... yay :-)
> >
> > Anyway, have fun testing. Let me know if it doesn't work.
> 
> I only have access to STi platforms which were broken by this change.
> Not any of the platforms which rely on the functionality which
> was introduced (although I can't see any reason why your patch wouldn't work).
> 
> Maybe Yoshihiro (on CC) could test this on the Renesas platforms and confirm?

Thank you for sending the email to me on CC.
I tested Felipe's patch on Renesas platfroms (R-Car Gen2 and Gen3) and I fixed 
the patch like the following.
However, my fixes patch might need to clean the code up more.

Changes from Felipe's patch:
- Change function names of xhci_rcar_init_quirk() to xhci_rcar_setup_quirk()
- Add setup_quirk() member and use it for rcar's function.
- Some minor fixes.

---
diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
index 1eefc98..1ea6c18 100644
--- a/drivers/usb/host/xhci-mvebu.c
+++ b/drivers/usb/host/xhci-mvebu.c
@@ -41,8 +41,9 @@ static void xhci_mvebu_mbus_config(void __iomem *base,
}
 }
 
-int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
+int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
 {
+   struct platform_device *pdev = to_platform_device(hcd->self.controller);
struct resource *res;
void __iomem *base;
const struct mbus_dram_target_info *dram;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d39d6bf..d28513a 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -47,43 +47,57 @@ static void xhci_plat_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_PLAT;
 }
 
+static void xhci_priv_plat_start(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (priv->plat_start)
+   priv->plat_start(hcd);
+}
+
+static int xhci_priv_setup_quirk(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (!priv->setup_quirk)
+   return 0;
+
+   return priv->setup_quirk(hcd);
+}
+
 /* called during probe() after chip reset completes */
 static int xhci_plat_setup(struct usb_hcd *hcd)
 {
int ret;
 
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
-   ret = xhci_rcar_init_quirk(hcd);
-   if (ret)
-   return ret;
-   }
+   ret = xhci_priv_setup_quirk(hcd);
+   if (ret)
+   return ret;
 
return xhci_gen_setup(hcd, xhci_plat_quirks);
 }
 
 static int xhci_plat_start(struct usb_hcd *hcd)
 {
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
-   xhci_rcar_start(hcd);
-
+   xhci_priv_plat_start(hcd);
return xhci_run(hcd);
 }
 
 #ifdef CONFIG_OF
 static const struct xhci_plat_priv xhci_plat_marvell_armada = {
-   .type = XHCI_PLAT_TYPE_MARVELL_ARMADA,
+   .init_quirk = xhci_mvebu_mbus_init_quirk,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
-   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
+   .setup_quirk = xhci_rcar_setup_quirk,
+   .plat_start = xhci_rcar_start,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
-   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V2,
+   .setup_quirk = xhci_rcar_setup_quirk,
+   .plat_start = xhci_rcar_start,
 };
 
 static const struct of_device_id usb_xhci_of_match[] = {
@@ -119,6 +133,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 {
struct device_node  *node = pdev->dev.of_node;
struct usb_xhci_pdata   *pdata = dev_get_platdata(>dev);
+   struct xhci_plat_priv   *priv;
const struct of_device_id *match;
const struct hc_driver  *driver;
struct xhci_hcd *xhci;
@@ -178,18 +193,18 @@ static int xhci_plat_probe(struct platform_device *pdev)
}
 
xhci = hcd_to_xhci(hcd);
+   priv = hcd_to_xhci_priv(hcd);
match = of_match_node(usb_xhci_of_match, node);
if (match) {
const struct xhci_plat_priv *priv_match = match->data;
-   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
 
/* Just copy data for now */
if (priv_match)
*priv = *priv_match;
}
 
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_MARVELL_ARMADA)) {
-   ret = xhci_mvebu_mbus_init_quirk(pdev);
+   if (priv->init_quirk) {
+   ret = 

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Felipe Balbi]

Hi,

Peter Griffin  writes:
> Hi Felipe,
>
> On Fri, 25 Mar 2016, Felipe Balbi wrote:
>
>> 
>> Hi,
>> 
>> Gregory CLEMENT  writes:
>> >> Peter Griffin  writes:
>> >>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
>> >>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
>> >>>
>> >>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
>> >>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
>> >>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
>> >>> gets added.
>> >>>
>> >>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
>> >>> although I've only tested on STih410 SoC.
>> >>>
>> >>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
>> >>> Cc: sta...@vger.kernel.org
>> >>> Cc: gregory.clem...@free-electrons.com
>> >>> Cc: yoshihiro.shimoda...@renesas.com
>> >>> Signed-off-by: Peter Griffin 
>> >>> ---
>> >>>  drivers/usb/host/xhci-plat.h | 2 +-
>> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
>> >>> index 5a2e2e3..529c3c4 100644
>> >>> --- a/drivers/usb/host/xhci-plat.h
>> >>> +++ b/drivers/usb/host/xhci-plat.h
>> >>> @@ -14,7 +14,7 @@
>> >>>  #include "xhci.h"   /* for hcd_to_xhci() */
>> >>>  
>> >>>  enum xhci_plat_type {
>> >>> -XHCI_PLAT_TYPE_MARVELL_ARMADA,
>> >>> +XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
>> >>>  XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>> >>>  XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
>> >>
>> >> aren't these platforms using device tree ? Why aren't these just
>> >> different compatible strings ?
>> >
>> > According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
>> > xhci-plat: add struct xhci_plat_priv" :
>> >
>> > This patch adds struct xhci_plat_priv to simplify the code to match
>> > platform specific variables. For now, this patch adds a member "type" in
>> > the structure
>> 
>> that's fine but the answer doesn't exactly match my question ;-)
>> 
>> My point is that this enum shouldn't be necessary at all. We have
>> compatible flags to make these checks instead. How about below ?
>> (untested, uncompiled, yada yada yada). Note that we DON'T need this
>> xhci_plat_type trickery, just need to be a little bit smarter about how
>> we use driver_data:
>
> Your solution certainly looks more elegant.

cool thanks. Now that I think about this more carefully, we might wanna
take $subject anyway for the -rc and get my version applied for v4.7
merge window. What do you think ?

>> diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
>> index 1eefc988192d..1ea6c18b74f3 100644
>> --- a/drivers/usb/host/xhci-mvebu.c
>> +++ b/drivers/usb/host/xhci-mvebu.c
>> @@ -41,8 +41,9 @@ static void xhci_mvebu_mbus_config(void __iomem *base,
>>  }
>>  }
>>  
>> -int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
>> +int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
>>  {
>> +struct platform_device *pdev = to_platform_device(hcd->self.controller);
>>  struct resource *res;
>>  void __iomem *base;
>>  const struct mbus_dram_target_info *dram;
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 5c15e9bc5f7a..adb77c60a9ae 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -47,43 +47,56 @@ static void xhci_plat_quirks(struct device *dev, struct 
>> xhci_hcd *xhci)
>>  xhci->quirks |= XHCI_PLAT;
>>  }
>>  
>> +static void xhci_priv_plat_start(struct usb_hcd *hcd)
>> +{
>> +struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
>> +
>> +if (priv->plat_start)
>> +priv->plat_start(hcd);
>> +}
>> +
>> +static int xhci_priv_init_quirk(struct usb_hcd *hcd)
>> +{
>> +struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
>> +
>> +if (!priv->init_quirk)
>> +return 0;
>> +
>> +return priv->init_quirk(hcd);
>> +}
>> +
>>  /* called during probe() after chip reset completes */
>>  static int xhci_plat_setup(struct usb_hcd *hcd)
>>  {
>>  int ret;
>>  
>> -if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
>> -xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
>> -ret = xhci_rcar_init_quirk(hcd);
>> -if (ret)
>> -return ret;
>> -}
>> +ret = xhci_priv_init_quirk(hcd);
>> +if (ret)
>> +return ret;
>>  
>>  return xhci_gen_setup(hcd, xhci_plat_quirks);
>>  }
>>  
>>  static int xhci_plat_start(struct usb_hcd *hcd)
>>  {
>> -if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
>> -xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
>> -xhci_rcar_start(hcd);
>> -
>> +xhci_priv_plat_start(hcd);
>>  return xhci_run(hcd);
>>  }

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Felipe Balbi]

Hi,

Peter Griffin  writes:
> Hi Felipe,
>
> On Fri, 25 Mar 2016, Felipe Balbi wrote:
>
>> 
>> Hi,
>> 
>> Gregory CLEMENT  writes:
>> >> Peter Griffin  writes:
>> >>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
>> >>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
>> >>>
>> >>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
>> >>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
>> >>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
>> >>> gets added.
>> >>>
>> >>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
>> >>> although I've only tested on STih410 SoC.
>> >>>
>> >>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
>> >>> Cc: sta...@vger.kernel.org
>> >>> Cc: gregory.clem...@free-electrons.com
>> >>> Cc: yoshihiro.shimoda...@renesas.com
>> >>> Signed-off-by: Peter Griffin 
>> >>> ---
>> >>>  drivers/usb/host/xhci-plat.h | 2 +-
>> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
>> >>> index 5a2e2e3..529c3c4 100644
>> >>> --- a/drivers/usb/host/xhci-plat.h
>> >>> +++ b/drivers/usb/host/xhci-plat.h
>> >>> @@ -14,7 +14,7 @@
>> >>>  #include "xhci.h"   /* for hcd_to_xhci() */
>> >>>  
>> >>>  enum xhci_plat_type {
>> >>> -XHCI_PLAT_TYPE_MARVELL_ARMADA,
>> >>> +XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
>> >>>  XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>> >>>  XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
>> >>
>> >> aren't these platforms using device tree ? Why aren't these just
>> >> different compatible strings ?
>> >
>> > According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
>> > xhci-plat: add struct xhci_plat_priv" :
>> >
>> > This patch adds struct xhci_plat_priv to simplify the code to match
>> > platform specific variables. For now, this patch adds a member "type" in
>> > the structure
>> 
>> that's fine but the answer doesn't exactly match my question ;-)
>> 
>> My point is that this enum shouldn't be necessary at all. We have
>> compatible flags to make these checks instead. How about below ?
>> (untested, uncompiled, yada yada yada). Note that we DON'T need this
>> xhci_plat_type trickery, just need to be a little bit smarter about how
>> we use driver_data:
>
> Your solution certainly looks more elegant.

cool thanks. Now that I think about this more carefully, we might wanna
take $subject anyway for the -rc and get my version applied for v4.7
merge window. What do you think ?

>> diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
>> index 1eefc988192d..1ea6c18b74f3 100644
>> --- a/drivers/usb/host/xhci-mvebu.c
>> +++ b/drivers/usb/host/xhci-mvebu.c
>> @@ -41,8 +41,9 @@ static void xhci_mvebu_mbus_config(void __iomem *base,
>>  }
>>  }
>>  
>> -int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
>> +int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
>>  {
>> +struct platform_device *pdev = to_platform_device(hcd->self.controller);
>>  struct resource *res;
>>  void __iomem *base;
>>  const struct mbus_dram_target_info *dram;
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 5c15e9bc5f7a..adb77c60a9ae 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -47,43 +47,56 @@ static void xhci_plat_quirks(struct device *dev, struct 
>> xhci_hcd *xhci)
>>  xhci->quirks |= XHCI_PLAT;
>>  }
>>  
>> +static void xhci_priv_plat_start(struct usb_hcd *hcd)
>> +{
>> +struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
>> +
>> +if (priv->plat_start)
>> +priv->plat_start(hcd);
>> +}
>> +
>> +static int xhci_priv_init_quirk(struct usb_hcd *hcd)
>> +{
>> +struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
>> +
>> +if (!priv->init_quirk)
>> +return 0;
>> +
>> +return priv->init_quirk(hcd);
>> +}
>> +
>>  /* called during probe() after chip reset completes */
>>  static int xhci_plat_setup(struct usb_hcd *hcd)
>>  {
>>  int ret;
>>  
>> -if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
>> -xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
>> -ret = xhci_rcar_init_quirk(hcd);
>> -if (ret)
>> -return ret;
>> -}
>> +ret = xhci_priv_init_quirk(hcd);
>> +if (ret)
>> +return ret;
>>  
>>  return xhci_gen_setup(hcd, xhci_plat_quirks);
>>  }
>>  
>>  static int xhci_plat_start(struct usb_hcd *hcd)
>>  {
>> -if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
>> -xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
>> -xhci_rcar_start(hcd);
>> -
>> +xhci_priv_plat_start(hcd);
>>  return xhci_run(hcd);
>>  }
>>  
>>  #ifdef CONFIG_OF
>>  static const struct xhci_plat_priv xhci_plat_marvell_armada = {
>> -.type = 

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Peter Griffin]

Hi Felipe,

On Fri, 25 Mar 2016, Felipe Balbi wrote:

> 
> Hi,
> 
> Gregory CLEMENT  writes:
> >> Peter Griffin  writes:
> >>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
> >>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
> >>>
> >>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
> >>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
> >>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
> >>> gets added.
> >>>
> >>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
> >>> although I've only tested on STih410 SoC.
> >>>
> >>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
> >>> Cc: sta...@vger.kernel.org
> >>> Cc: gregory.clem...@free-electrons.com
> >>> Cc: yoshihiro.shimoda...@renesas.com
> >>> Signed-off-by: Peter Griffin 
> >>> ---
> >>>  drivers/usb/host/xhci-plat.h | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
> >>> index 5a2e2e3..529c3c4 100644
> >>> --- a/drivers/usb/host/xhci-plat.h
> >>> +++ b/drivers/usb/host/xhci-plat.h
> >>> @@ -14,7 +14,7 @@
> >>>  #include "xhci.h"/* for hcd_to_xhci() */
> >>>  
> >>>  enum xhci_plat_type {
> >>> - XHCI_PLAT_TYPE_MARVELL_ARMADA,
> >>> + XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
> >>>   XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
> >>>   XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
> >>
> >> aren't these platforms using device tree ? Why aren't these just
> >> different compatible strings ?
> >
> > According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
> > xhci-plat: add struct xhci_plat_priv" :
> >
> > This patch adds struct xhci_plat_priv to simplify the code to match
> > platform specific variables. For now, this patch adds a member "type" in
> > the structure
> 
> that's fine but the answer doesn't exactly match my question ;-)
> 
> My point is that this enum shouldn't be necessary at all. We have
> compatible flags to make these checks instead. How about below ?
> (untested, uncompiled, yada yada yada). Note that we DON'T need this
> xhci_plat_type trickery, just need to be a little bit smarter about how
> we use driver_data:

Your solution certainly looks more elegant.

> 
> diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
> index 1eefc988192d..1ea6c18b74f3 100644
> --- a/drivers/usb/host/xhci-mvebu.c
> +++ b/drivers/usb/host/xhci-mvebu.c
> @@ -41,8 +41,9 @@ static void xhci_mvebu_mbus_config(void __iomem *base,
>   }
>  }
>  
> -int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
> +int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
>  {
> + struct platform_device *pdev = to_platform_device(hcd->self.controller);
>   struct resource *res;
>   void __iomem *base;
>   const struct mbus_dram_target_info *dram;
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 5c15e9bc5f7a..adb77c60a9ae 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -47,43 +47,56 @@ static void xhci_plat_quirks(struct device *dev, struct 
> xhci_hcd *xhci)
>   xhci->quirks |= XHCI_PLAT;
>  }
>  
> +static void xhci_priv_plat_start(struct usb_hcd *hcd)
> +{
> + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
> +
> + if (priv->plat_start)
> + priv->plat_start(hcd);
> +}
> +
> +static int xhci_priv_init_quirk(struct usb_hcd *hcd)
> +{
> + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
> +
> + if (!priv->init_quirk)
> + return 0;
> +
> + return priv->init_quirk(hcd);
> +}
> +
>  /* called during probe() after chip reset completes */
>  static int xhci_plat_setup(struct usb_hcd *hcd)
>  {
>   int ret;
>  
> - if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
> - xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
> - ret = xhci_rcar_init_quirk(hcd);
> - if (ret)
> - return ret;
> - }
> + ret = xhci_priv_init_quirk(hcd);
> + if (ret)
> + return ret;
>  
>   return xhci_gen_setup(hcd, xhci_plat_quirks);
>  }
>  
>  static int xhci_plat_start(struct usb_hcd *hcd)
>  {
> - if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
> - xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
> - xhci_rcar_start(hcd);
> -
> + xhci_priv_plat_start(hcd);
>   return xhci_run(hcd);
>  }
>  
>  #ifdef CONFIG_OF
>  static const struct xhci_plat_priv xhci_plat_marvell_armada = {
> - .type = XHCI_PLAT_TYPE_MARVELL_ARMADA,
> + .init_quirk = xhci_mvebu_mbus_init_quirk,
>  };
>  
>  static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
> - .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>   .firmware_name = 

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Peter Griffin]

Hi Felipe,

On Fri, 25 Mar 2016, Felipe Balbi wrote:

> 
> Hi,
> 
> Gregory CLEMENT  writes:
> >> Peter Griffin  writes:
> >>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
> >>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
> >>>
> >>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
> >>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
> >>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
> >>> gets added.
> >>>
> >>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
> >>> although I've only tested on STih410 SoC.
> >>>
> >>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
> >>> Cc: sta...@vger.kernel.org
> >>> Cc: gregory.clem...@free-electrons.com
> >>> Cc: yoshihiro.shimoda...@renesas.com
> >>> Signed-off-by: Peter Griffin 
> >>> ---
> >>>  drivers/usb/host/xhci-plat.h | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
> >>> index 5a2e2e3..529c3c4 100644
> >>> --- a/drivers/usb/host/xhci-plat.h
> >>> +++ b/drivers/usb/host/xhci-plat.h
> >>> @@ -14,7 +14,7 @@
> >>>  #include "xhci.h"/* for hcd_to_xhci() */
> >>>  
> >>>  enum xhci_plat_type {
> >>> - XHCI_PLAT_TYPE_MARVELL_ARMADA,
> >>> + XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
> >>>   XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
> >>>   XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
> >>
> >> aren't these platforms using device tree ? Why aren't these just
> >> different compatible strings ?
> >
> > According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
> > xhci-plat: add struct xhci_plat_priv" :
> >
> > This patch adds struct xhci_plat_priv to simplify the code to match
> > platform specific variables. For now, this patch adds a member "type" in
> > the structure
> 
> that's fine but the answer doesn't exactly match my question ;-)
> 
> My point is that this enum shouldn't be necessary at all. We have
> compatible flags to make these checks instead. How about below ?
> (untested, uncompiled, yada yada yada). Note that we DON'T need this
> xhci_plat_type trickery, just need to be a little bit smarter about how
> we use driver_data:

Your solution certainly looks more elegant.

> 
> diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
> index 1eefc988192d..1ea6c18b74f3 100644
> --- a/drivers/usb/host/xhci-mvebu.c
> +++ b/drivers/usb/host/xhci-mvebu.c
> @@ -41,8 +41,9 @@ static void xhci_mvebu_mbus_config(void __iomem *base,
>   }
>  }
>  
> -int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
> +int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
>  {
> + struct platform_device *pdev = to_platform_device(hcd->self.controller);
>   struct resource *res;
>   void __iomem *base;
>   const struct mbus_dram_target_info *dram;
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 5c15e9bc5f7a..adb77c60a9ae 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -47,43 +47,56 @@ static void xhci_plat_quirks(struct device *dev, struct 
> xhci_hcd *xhci)
>   xhci->quirks |= XHCI_PLAT;
>  }
>  
> +static void xhci_priv_plat_start(struct usb_hcd *hcd)
> +{
> + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
> +
> + if (priv->plat_start)
> + priv->plat_start(hcd);
> +}
> +
> +static int xhci_priv_init_quirk(struct usb_hcd *hcd)
> +{
> + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
> +
> + if (!priv->init_quirk)
> + return 0;
> +
> + return priv->init_quirk(hcd);
> +}
> +
>  /* called during probe() after chip reset completes */
>  static int xhci_plat_setup(struct usb_hcd *hcd)
>  {
>   int ret;
>  
> - if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
> - xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
> - ret = xhci_rcar_init_quirk(hcd);
> - if (ret)
> - return ret;
> - }
> + ret = xhci_priv_init_quirk(hcd);
> + if (ret)
> + return ret;
>  
>   return xhci_gen_setup(hcd, xhci_plat_quirks);
>  }
>  
>  static int xhci_plat_start(struct usb_hcd *hcd)
>  {
> - if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
> - xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
> - xhci_rcar_start(hcd);
> -
> + xhci_priv_plat_start(hcd);
>   return xhci_run(hcd);
>  }
>  
>  #ifdef CONFIG_OF
>  static const struct xhci_plat_priv xhci_plat_marvell_armada = {
> - .type = XHCI_PLAT_TYPE_MARVELL_ARMADA,
> + .init_quirk = xhci_mvebu_mbus_init_quirk,
>  };
>  
>  static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
> - .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>   .firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
> + .init_quirk = xhci_rcar_init_quirk,
>  };
>  
>  static const struct 

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Felipe Balbi]

Hi,

Gregory CLEMENT  writes:
>> Peter Griffin  writes:
>>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
>>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
>>>
>>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
>>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
>>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
>>> gets added.
>>>
>>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
>>> although I've only tested on STih410 SoC.
>>>
>>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
>>> Cc: sta...@vger.kernel.org
>>> Cc: gregory.clem...@free-electrons.com
>>> Cc: yoshihiro.shimoda...@renesas.com
>>> Signed-off-by: Peter Griffin 
>>> ---
>>>  drivers/usb/host/xhci-plat.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
>>> index 5a2e2e3..529c3c4 100644
>>> --- a/drivers/usb/host/xhci-plat.h
>>> +++ b/drivers/usb/host/xhci-plat.h
>>> @@ -14,7 +14,7 @@
>>>  #include "xhci.h"  /* for hcd_to_xhci() */
>>>  
>>>  enum xhci_plat_type {
>>> -   XHCI_PLAT_TYPE_MARVELL_ARMADA,
>>> +   XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
>>> XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>>> XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
>>
>> aren't these platforms using device tree ? Why aren't these just
>> different compatible strings ?
>
> According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
> xhci-plat: add struct xhci_plat_priv" :
>
> This patch adds struct xhci_plat_priv to simplify the code to match
> platform specific variables. For now, this patch adds a member "type" in
> the structure

that's fine but the answer doesn't exactly match my question ;-)

My point is that this enum shouldn't be necessary at all. We have
compatible flags to make these checks instead. How about below ?
(untested, uncompiled, yada yada yada). Note that we DON'T need this
xhci_plat_type trickery, just need to be a little bit smarter about how
we use driver_data:

diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
index 1eefc988192d..1ea6c18b74f3 100644
--- a/drivers/usb/host/xhci-mvebu.c
+++ b/drivers/usb/host/xhci-mvebu.c
@@ -41,8 +41,9 @@ static void xhci_mvebu_mbus_config(void __iomem *base,
}
 }
 
-int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
+int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
 {
+   struct platform_device *pdev = to_platform_device(hcd->self.controller);
struct resource *res;
void __iomem *base;
const struct mbus_dram_target_info *dram;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 5c15e9bc5f7a..adb77c60a9ae 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -47,43 +47,56 @@ static void xhci_plat_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_PLAT;
 }
 
+static void xhci_priv_plat_start(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (priv->plat_start)
+   priv->plat_start(hcd);
+}
+
+static int xhci_priv_init_quirk(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (!priv->init_quirk)
+   return 0;
+
+   return priv->init_quirk(hcd);
+}
+
 /* called during probe() after chip reset completes */
 static int xhci_plat_setup(struct usb_hcd *hcd)
 {
int ret;
 
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
-   ret = xhci_rcar_init_quirk(hcd);
-   if (ret)
-   return ret;
-   }
+   ret = xhci_priv_init_quirk(hcd);
+   if (ret)
+   return ret;
 
return xhci_gen_setup(hcd, xhci_plat_quirks);
 }
 
 static int xhci_plat_start(struct usb_hcd *hcd)
 {
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
-   xhci_rcar_start(hcd);
-
+   xhci_priv_plat_start(hcd);
return xhci_run(hcd);
 }
 
 #ifdef CONFIG_OF
 static const struct xhci_plat_priv xhci_plat_marvell_armada = {
-   .type = XHCI_PLAT_TYPE_MARVELL_ARMADA,
+   .init_quirk = xhci_mvebu_mbus_init_quirk,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
-   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
+   .init_quirk = xhci_rcar_init_quirk,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
-   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V2,
+   .init_quirk = xhci_rcar_init_quirk,
+   .plat_start = 

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Felipe Balbi]

Hi,

Gregory CLEMENT  writes:
>> Peter Griffin  writes:
>>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
>>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
>>>
>>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
>>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
>>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
>>> gets added.
>>>
>>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
>>> although I've only tested on STih410 SoC.
>>>
>>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
>>> Cc: sta...@vger.kernel.org
>>> Cc: gregory.clem...@free-electrons.com
>>> Cc: yoshihiro.shimoda...@renesas.com
>>> Signed-off-by: Peter Griffin 
>>> ---
>>>  drivers/usb/host/xhci-plat.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
>>> index 5a2e2e3..529c3c4 100644
>>> --- a/drivers/usb/host/xhci-plat.h
>>> +++ b/drivers/usb/host/xhci-plat.h
>>> @@ -14,7 +14,7 @@
>>>  #include "xhci.h"  /* for hcd_to_xhci() */
>>>  
>>>  enum xhci_plat_type {
>>> -   XHCI_PLAT_TYPE_MARVELL_ARMADA,
>>> +   XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
>>> XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>>> XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
>>
>> aren't these platforms using device tree ? Why aren't these just
>> different compatible strings ?
>
> According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
> xhci-plat: add struct xhci_plat_priv" :
>
> This patch adds struct xhci_plat_priv to simplify the code to match
> platform specific variables. For now, this patch adds a member "type" in
> the structure

that's fine but the answer doesn't exactly match my question ;-)

My point is that this enum shouldn't be necessary at all. We have
compatible flags to make these checks instead. How about below ?
(untested, uncompiled, yada yada yada). Note that we DON'T need this
xhci_plat_type trickery, just need to be a little bit smarter about how
we use driver_data:

diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
index 1eefc988192d..1ea6c18b74f3 100644
--- a/drivers/usb/host/xhci-mvebu.c
+++ b/drivers/usb/host/xhci-mvebu.c
@@ -41,8 +41,9 @@ static void xhci_mvebu_mbus_config(void __iomem *base,
}
 }
 
-int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
+int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
 {
+   struct platform_device *pdev = to_platform_device(hcd->self.controller);
struct resource *res;
void __iomem *base;
const struct mbus_dram_target_info *dram;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 5c15e9bc5f7a..adb77c60a9ae 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -47,43 +47,56 @@ static void xhci_plat_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_PLAT;
 }
 
+static void xhci_priv_plat_start(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (priv->plat_start)
+   priv->plat_start(hcd);
+}
+
+static int xhci_priv_init_quirk(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (!priv->init_quirk)
+   return 0;
+
+   return priv->init_quirk(hcd);
+}
+
 /* called during probe() after chip reset completes */
 static int xhci_plat_setup(struct usb_hcd *hcd)
 {
int ret;
 
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
-   ret = xhci_rcar_init_quirk(hcd);
-   if (ret)
-   return ret;
-   }
+   ret = xhci_priv_init_quirk(hcd);
+   if (ret)
+   return ret;
 
return xhci_gen_setup(hcd, xhci_plat_quirks);
 }
 
 static int xhci_plat_start(struct usb_hcd *hcd)
 {
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
-   xhci_rcar_start(hcd);
-
+   xhci_priv_plat_start(hcd);
return xhci_run(hcd);
 }
 
 #ifdef CONFIG_OF
 static const struct xhci_plat_priv xhci_plat_marvell_armada = {
-   .type = XHCI_PLAT_TYPE_MARVELL_ARMADA,
+   .init_quirk = xhci_mvebu_mbus_init_quirk,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
-   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
+   .init_quirk = xhci_rcar_init_quirk,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
-   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V2,
+   .init_quirk = xhci_rcar_init_quirk,
+   .plat_start = xhci_rcar_start,
 };
 
 static const struct of_device_id usb_xhci_of_match[] = {
diff --git 

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Gregory CLEMENT]

Hi Felipe,
 
 On ven., mars 25 2016, Felipe Balbi  wrote:

> Hi,
>
> Peter Griffin  writes:
>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
>>
>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
>> gets added.
>>
>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
>> although I've only tested on STih410 SoC.
>>
>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
>> Cc: sta...@vger.kernel.org
>> Cc: gregory.clem...@free-electrons.com
>> Cc: yoshihiro.shimoda...@renesas.com
>> Signed-off-by: Peter Griffin 
>> ---
>>  drivers/usb/host/xhci-plat.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
>> index 5a2e2e3..529c3c4 100644
>> --- a/drivers/usb/host/xhci-plat.h
>> +++ b/drivers/usb/host/xhci-plat.h
>> @@ -14,7 +14,7 @@
>>  #include "xhci.h"   /* for hcd_to_xhci() */
>>  
>>  enum xhci_plat_type {
>> -XHCI_PLAT_TYPE_MARVELL_ARMADA,
>> +XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
>>  XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>>  XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
>
> aren't these platforms using device tree ? Why aren't these just
> different compatible strings ?

According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
xhci-plat: add struct xhci_plat_priv" :

This patch adds struct xhci_plat_priv to simplify the code to match
platform specific variables. For now, this patch adds a member "type" in
the structure

Gregory


>
> -- 
> balbi

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Gregory CLEMENT]

Hi Felipe,
 
 On ven., mars 25 2016, Felipe Balbi  wrote:

> Hi,
>
> Peter Griffin  writes:
>> Otherwise generic-xhci and xhci-platform which have no data get wrongly
>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
>>
>> This fixes a regression in v4.5 for STiH407 family SoC's which use the
>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
>> gets added.
>>
>> I suspect this will also fix other dwc3 DT platforms such as Exynos,
>> although I've only tested on STih410 SoC.
>>
>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
>> Cc: sta...@vger.kernel.org
>> Cc: gregory.clem...@free-electrons.com
>> Cc: yoshihiro.shimoda...@renesas.com
>> Signed-off-by: Peter Griffin 
>> ---
>>  drivers/usb/host/xhci-plat.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
>> index 5a2e2e3..529c3c4 100644
>> --- a/drivers/usb/host/xhci-plat.h
>> +++ b/drivers/usb/host/xhci-plat.h
>> @@ -14,7 +14,7 @@
>>  #include "xhci.h"   /* for hcd_to_xhci() */
>>  
>>  enum xhci_plat_type {
>> -XHCI_PLAT_TYPE_MARVELL_ARMADA,
>> +XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
>>  XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>>  XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
>
> aren't these platforms using device tree ? Why aren't these just
> different compatible strings ?

According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host:
xhci-plat: add struct xhci_plat_priv" :

This patch adds struct xhci_plat_priv to simplify the code to match
platform specific variables. For now, this patch adds a member "type" in
the structure

Gregory


>
> -- 
> balbi

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Felipe Balbi]

Hi,

Peter Griffin  writes:
> Otherwise generic-xhci and xhci-platform which have no data get wrongly
> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
>
> This fixes a regression in v4.5 for STiH407 family SoC's which use the
> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
> gets added.
>
> I suspect this will also fix other dwc3 DT platforms such as Exynos,
> although I've only tested on STih410 SoC.
>
> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
> Cc: sta...@vger.kernel.org
> Cc: gregory.clem...@free-electrons.com
> Cc: yoshihiro.shimoda...@renesas.com
> Signed-off-by: Peter Griffin 
> ---
>  drivers/usb/host/xhci-plat.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
> index 5a2e2e3..529c3c4 100644
> --- a/drivers/usb/host/xhci-plat.h
> +++ b/drivers/usb/host/xhci-plat.h
> @@ -14,7 +14,7 @@
>  #include "xhci.h"/* for hcd_to_xhci() */
>  
>  enum xhci_plat_type {
> - XHCI_PLAT_TYPE_MARVELL_ARMADA,
> + XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
>   XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>   XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,

aren't these platforms using device tree ? Why aren't these just
different compatible strings ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value

[Felipe Balbi]

Hi,

Peter Griffin  writes:
> Otherwise generic-xhci and xhci-platform which have no data get wrongly
> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is().
>
> This fixes a regression in v4.5 for STiH407 family SoC's which use the
> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to
> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never
> gets added.
>
> I suspect this will also fix other dwc3 DT platforms such as Exynos,
> although I've only tested on STih410 SoC.
>
> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv")
> Cc: sta...@vger.kernel.org
> Cc: gregory.clem...@free-electrons.com
> Cc: yoshihiro.shimoda...@renesas.com
> Signed-off-by: Peter Griffin 
> ---
>  drivers/usb/host/xhci-plat.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
> index 5a2e2e3..529c3c4 100644
> --- a/drivers/usb/host/xhci-plat.h
> +++ b/drivers/usb/host/xhci-plat.h
> @@ -14,7 +14,7 @@
>  #include "xhci.h"/* for hcd_to_xhci() */
>  
>  enum xhci_plat_type {
> - XHCI_PLAT_TYPE_MARVELL_ARMADA,
> + XHCI_PLAT_TYPE_MARVELL_ARMADA = 1,
>   XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>   XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,

aren't these platforms using device tree ? Why aren't these just
different compatible strings ?

-- 
balbi


signature.asc
Description: PGP signature