[PATCH v2] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[PATCH v2] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Yoshihiro Shimoda]

The firmware of R-Car USB 3.0 host controller will control the reset.
So, if the xhci driver doesn't do firmware downloading (e.g. kernel
configuration is CONFIG_USB_XHCI_PLATFORM=y and CONFIG_USB_XHCI_RCAR
is not set), the reset of USB 3.0 host controller doesn't work
correctly. Then, the host controller will cause long wait in
xhci_reset() because the CMD_RESET bit of op_regs->command is not
cleared for 10 seconds.

So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
to exit the probe function immediately.

Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers)
Cc: <stable@vger.kernel.org> # v3.17+
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 Changes from v1:
  - Revise the commit log.
    (http://www.spinics.net/lists/stable/msg130007.html)

 drivers/usb/host/xhci-rcar.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
index 2941a25..2afed68 100644
--- a/drivers/usb/host/xhci-rcar.h
+++ b/drivers/usb/host/xhci-rcar.h
@@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
 
 static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
 {
-	return 0;
+	/*
+	 * To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
+	 * disabled, this function fails.
+	 */
+	return -ENODEV;
 }
 #endif
 #endif /* _XHCI_RCAR_H */
-- 
1.9.1

Re: [PATCH v2] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2831 bytes --]


Hi,

Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
> The firmware of R-Car USB 3.0 host controller will control the reset.
> So, if the xhci driver doesn't do firmware downloading (e.g. kernel
> configuration is CONFIG_USB_XHCI_PLATFORM=y and CONFIG_USB_XHCI_RCAR
> is not set), the reset of USB 3.0 host controller doesn't work
> correctly. Then, the host controller will cause long wait in
> xhci_reset() because the CMD_RESET bit of op_regs->command is not
> cleared for 10 seconds.
>
> So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
> to exit the probe function immediately.
>
> Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers)
> Cc: <stable@vger.kernel.org> # v3.17+
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  Changes from v1:
>   - Revise the commit log.
>     (http://www.spinics.net/lists/stable/msg130007.html)
>
>  drivers/usb/host/xhci-rcar.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
> index 2941a25..2afed68 100644
> --- a/drivers/usb/host/xhci-rcar.h
> +++ b/drivers/usb/host/xhci-rcar.h
> @@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
>  
>  static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
>  {
> -	return 0;
> +	/*
> +	 * To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
> +	 * disabled, this function fails.
> +	 */
> +	return -ENODEV;

okay, if I understood correctly the thing is that you have a kernel
built with XHCI platform support but without XHCI RCAR support. Then, if
you run this kernel on RCAR board, you see this CMD_RESET problem,
right?

Isn't this pointing to the fact that xhci-plat.ko built without RCAR
isn't exactly compatible with RCAR ?

IOW:

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 676ea458148b..3e39320564ce 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -112,6 +112,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
 	}, {
 		.compatible = "marvell,armada-380-xhci",
 		.data = &xhci_plat_marvell_armada,
+#if IS_ENABLED(CONFIG_USB_XHCI_RCAR)
 	}, {
 		.compatible = "renesas,xhci-r8a7790",
 		.data = &xhci_plat_renesas_rcar_gen2,
@@ -130,6 +131,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
 	}, {
 		.compatible = "renesas,rcar-gen3-xhci",
 		.data = &xhci_plat_renesas_rcar_gen3,
+#endif
 	},
 	{},
 };

Rob, should we limit compatible flags like this ? Without
CONFIG_USB_XHCI_RCAR, this driver won't work on RCAR but, as you can
see, this driver might still work on other non-RCAR platforms.

What's your take on this ?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v2] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Rob Herring]

On Fri, Apr 22, 2016 at 4:36 AM, Felipe Balbi
<felipe.balbi@linux.intel.com> wrote:
>
> Hi,
>
> Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
>> The firmware of R-Car USB 3.0 host controller will control the reset.
>> So, if the xhci driver doesn't do firmware downloading (e.g. kernel
>> configuration is CONFIG_USB_XHCI_PLATFORM=y and CONFIG_USB_XHCI_RCAR
>> is not set), the reset of USB 3.0 host controller doesn't work
>> correctly. Then, the host controller will cause long wait in
>> xhci_reset() because the CMD_RESET bit of op_regs->command is not
>> cleared for 10 seconds.
>>
>> So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
>> to exit the probe function immediately.
>>
>> Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers)
>> Cc: <stable@vger.kernel.org> # v3.17+
>> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> ---
>>  Changes from v1:
>>   - Revise the commit log.
>>     (http://www.spinics.net/lists/stable/msg130007.html)
>>
>>  drivers/usb/host/xhci-rcar.h | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
>> index 2941a25..2afed68 100644
>> --- a/drivers/usb/host/xhci-rcar.h
>> +++ b/drivers/usb/host/xhci-rcar.h
>> @@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
>>
>>  static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
>>  {
>> -     return 0;
>> +     /*
>> +      * To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
>> +      * disabled, this function fails.
>> +      */
>> +     return -ENODEV;
>
> okay, if I understood correctly the thing is that you have a kernel
> built with XHCI platform support but without XHCI RCAR support. Then, if
> you run this kernel on RCAR board, you see this CMD_RESET problem,
> right?
>
> Isn't this pointing to the fact that xhci-plat.ko built without RCAR
> isn't exactly compatible with RCAR ?
>
> IOW:
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 676ea458148b..3e39320564ce 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -112,6 +112,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
>         }, {
>                 .compatible = "marvell,armada-380-xhci",
>                 .data = &xhci_plat_marvell_armada,
> +#if IS_ENABLED(CONFIG_USB_XHCI_RCAR)
>         }, {
>                 .compatible = "renesas,xhci-r8a7790",
>                 .data = &xhci_plat_renesas_rcar_gen2,
> @@ -130,6 +131,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
>         }, {
>                 .compatible = "renesas,rcar-gen3-xhci",
>                 .data = &xhci_plat_renesas_rcar_gen3,
> +#endif
>         },
>         {},
>  };
>
> Rob, should we limit compatible flags like this ? Without
> CONFIG_USB_XHCI_RCAR, this driver won't work on RCAR but, as you can
> see, this driver might still work on other non-RCAR platforms.
>
> What's your take on this ?

We should fix this in kconfig to always enable the option when RCAR is
enabled IMO.

Rob

RE: [PATCH v2] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Yoshihiro Shimoda]

Hi,

> From: Rob Herring
> Sent: Friday, April 22, 2016 10:29 PM
> 
> On Fri, Apr 22, 2016 at 4:36 AM, Felipe Balbi
> <felipe.balbi@linux.intel.com> wrote:
> >
> > Hi,
> >
> > Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
> >> The firmware of R-Car USB 3.0 host controller will control the reset.
> >> So, if the xhci driver doesn't do firmware downloading (e.g. kernel
> >> configuration is CONFIG_USB_XHCI_PLATFORM=y and CONFIG_USB_XHCI_RCAR
> >> is not set), the reset of USB 3.0 host controller doesn't work
> >> correctly. Then, the host controller will cause long wait in
> >> xhci_reset() because the CMD_RESET bit of op_regs->command is not
> >> cleared for 10 seconds.
> >>
> >> So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
> >> to exit the probe function immediately.
> >>
> >> Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers)
> >> Cc: <stable@vger.kernel.org> # v3.17+
> >> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >> ---
> >>  Changes from v1:
> >>   - Revise the commit log.
> >>     (http://www.spinics.net/lists/stable/msg130007.html)
> >>
> >>  drivers/usb/host/xhci-rcar.h | 6 +++++-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
> >> index 2941a25..2afed68 100644
> >> --- a/drivers/usb/host/xhci-rcar.h
> >> +++ b/drivers/usb/host/xhci-rcar.h
> >> @@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
> >>
> >>  static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
> >>  {
> >> -     return 0;
> >> +     /*
> >> +      * To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
> >> +      * disabled, this function fails.
> >> +      */
> >> +     return -ENODEV;
> >
> > okay, if I understood correctly the thing is that you have a kernel
> > built with XHCI platform support but without XHCI RCAR support. Then, if
> > you run this kernel on RCAR board, you see this CMD_RESET problem,
> > right?
> >
> > Isn't this pointing to the fact that xhci-plat.ko built without RCAR
> > isn't exactly compatible with RCAR ?
> >
> > IOW:
> >
> > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> > index 676ea458148b..3e39320564ce 100644
> > --- a/drivers/usb/host/xhci-plat.c
> > +++ b/drivers/usb/host/xhci-plat.c
> > @@ -112,6 +112,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
> >         }, {
> >                 .compatible = "marvell,armada-380-xhci",
> >                 .data = &xhci_plat_marvell_armada,
> > +#if IS_ENABLED(CONFIG_USB_XHCI_RCAR)
> >         }, {
> >                 .compatible = "renesas,xhci-r8a7790",
> >                 .data = &xhci_plat_renesas_rcar_gen2,
> > @@ -130,6 +131,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
> >         }, {
> >                 .compatible = "renesas,rcar-gen3-xhci",
> >                 .data = &xhci_plat_renesas_rcar_gen3,
> > +#endif
> >         },
> >         {},
> >  };
> >
> > Rob, should we limit compatible flags like this ? Without
> > CONFIG_USB_XHCI_RCAR, this driver won't work on RCAR but, as you can
> > see, this driver might still work on other non-RCAR platforms.
> >
> > What's your take on this ?
> 
> We should fix this in kconfig to always enable the option when RCAR is
> enabled IMO.

I could fix this in kconfig like the followings:

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index f2bc5c3..905d1d2 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -46,6 +46,7 @@ menuconfig ARCH_RENESAS
 	select PINCTRL
 	select ARCH_REQUIRE_GPIOLIB
 	select ZONE_DMA if ARM_LPAE
+	select USB_XHCI_RCAR if USB_XHCI_HCD
 
 if ARCH_RENESAS
 
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index efa77c1..010d2b7 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -105,6 +105,7 @@ config ARCH_RENESAS
 	select PM
 	select PM_GENERIC_DOMAINS
 	select RENESAS_IRQC
+	select USB_XHCI_RCAR if USB_XHCI_HCD
 	help
 	  This enables support for the ARMv8 based Renesas SoCs.

If this is acceptable, I will send each patch to arm / arm64.

Best regards,
Yoshihiro Shimoda

> Rob

RE: [PATCH v2] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 4449 bytes --]


Hi,

Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
>> > Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
>> >> The firmware of R-Car USB 3.0 host controller will control the reset.
>> >> So, if the xhci driver doesn't do firmware downloading (e.g. kernel
>> >> configuration is CONFIG_USB_XHCI_PLATFORM=y and CONFIG_USB_XHCI_RCAR
>> >> is not set), the reset of USB 3.0 host controller doesn't work
>> >> correctly. Then, the host controller will cause long wait in
>> >> xhci_reset() because the CMD_RESET bit of op_regs->command is not
>> >> cleared for 10 seconds.
>> >>
>> >> So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
>> >> to exit the probe function immediately.
>> >>
>> >> Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers)
>> >> Cc: <stable@vger.kernel.org> # v3.17+
>> >> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> >> ---
>> >>  Changes from v1:
>> >>   - Revise the commit log.
>> >>     (http://www.spinics.net/lists/stable/msg130007.html)
>> >>
>> >>  drivers/usb/host/xhci-rcar.h | 6 +++++-
>> >>  1 file changed, 5 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
>> >> index 2941a25..2afed68 100644
>> >> --- a/drivers/usb/host/xhci-rcar.h
>> >> +++ b/drivers/usb/host/xhci-rcar.h
>> >> @@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
>> >>
>> >>  static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
>> >>  {
>> >> -     return 0;
>> >> +     /*
>> >> +      * To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
>> >> +      * disabled, this function fails.
>> >> +      */
>> >> +     return -ENODEV;
>> >
>> > okay, if I understood correctly the thing is that you have a kernel
>> > built with XHCI platform support but without XHCI RCAR support. Then, if
>> > you run this kernel on RCAR board, you see this CMD_RESET problem,
>> > right?
>> >
>> > Isn't this pointing to the fact that xhci-plat.ko built without RCAR
>> > isn't exactly compatible with RCAR ?
>> >
>> > IOW:
>> >
>> > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> > index 676ea458148b..3e39320564ce 100644
>> > --- a/drivers/usb/host/xhci-plat.c
>> > +++ b/drivers/usb/host/xhci-plat.c
>> > @@ -112,6 +112,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
>> >         }, {
>> >                 .compatible = "marvell,armada-380-xhci",
>> >                 .data = &xhci_plat_marvell_armada,
>> > +#if IS_ENABLED(CONFIG_USB_XHCI_RCAR)
>> >         }, {
>> >                 .compatible = "renesas,xhci-r8a7790",
>> >                 .data = &xhci_plat_renesas_rcar_gen2,
>> > @@ -130,6 +131,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
>> >         }, {
>> >                 .compatible = "renesas,rcar-gen3-xhci",
>> >                 .data = &xhci_plat_renesas_rcar_gen3,
>> > +#endif
>> >         },
>> >         {},
>> >  };
>> >
>> > Rob, should we limit compatible flags like this ? Without
>> > CONFIG_USB_XHCI_RCAR, this driver won't work on RCAR but, as you can
>> > see, this driver might still work on other non-RCAR platforms.
>> >
>> > What's your take on this ?
>> 
>> We should fix this in kconfig to always enable the option when RCAR is
>> enabled IMO.
>
> I could fix this in kconfig like the followings:
>
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index f2bc5c3..905d1d2 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -46,6 +46,7 @@ menuconfig ARCH_RENESAS
>  	select PINCTRL
>  	select ARCH_REQUIRE_GPIOLIB
>  	select ZONE_DMA if ARM_LPAE
> +	select USB_XHCI_RCAR if USB_XHCI_HCD
>  
>  if ARCH_RENESAS
>  
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index efa77c1..010d2b7 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -105,6 +105,7 @@ config ARCH_RENESAS
>  	select PM
>  	select PM_GENERIC_DOMAINS
>  	select RENESAS_IRQC
> +	select USB_XHCI_RCAR if USB_XHCI_HCD
>  	help
>  	  This enables support for the ARMv8 based Renesas SoCs.
>
> If this is acceptable, I will send each patch to arm / arm64.

I'm okay with that, not sure what the ARM folks will think ;-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v2] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Geert Uytterhoeven]

Hi Shimoda-san,

On Mon, Apr 25, 2016 at 4:10 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>> From: Rob Herring
>> Sent: Friday, April 22, 2016 10:29 PM
>> On Fri, Apr 22, 2016 at 4:36 AM, Felipe Balbi
>> <felipe.balbi@linux.intel.com> wrote:
>> > Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
>> >> The firmware of R-Car USB 3.0 host controller will control the reset.
>> >> So, if the xhci driver doesn't do firmware downloading (e.g. kernel
>> >> configuration is CONFIG_USB_XHCI_PLATFORM=y and CONFIG_USB_XHCI_RCAR
>> >> is not set), the reset of USB 3.0 host controller doesn't work
>> >> correctly. Then, the host controller will cause long wait in
>> >> xhci_reset() because the CMD_RESET bit of op_regs->command is not
>> >> cleared for 10 seconds.
>> >>
>> >> So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
>> >> to exit the probe function immediately.
>> >>
>> >> Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers)
>> >> Cc: <stable@vger.kernel.org> # v3.17+
>> >> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> >> ---
>> >>  Changes from v1:
>> >>   - Revise the commit log.
>> >>     (http://www.spinics.net/lists/stable/msg130007.html)
>> >>
>> >>  drivers/usb/host/xhci-rcar.h | 6 +++++-
>> >>  1 file changed, 5 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
>> >> index 2941a25..2afed68 100644
>> >> --- a/drivers/usb/host/xhci-rcar.h
>> >> +++ b/drivers/usb/host/xhci-rcar.h
>> >> @@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
>> >>
>> >>  static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
>> >>  {
>> >> -     return 0;
>> >> +     /*
>> >> +      * To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
>> >> +      * disabled, this function fails.
>> >> +      */
>> >> +     return -ENODEV;
>> >
>> > okay, if I understood correctly the thing is that you have a kernel
>> > built with XHCI platform support but without XHCI RCAR support. Then, if
>> > you run this kernel on RCAR board, you see this CMD_RESET problem,
>> > right?

[...]

>> We should fix this in kconfig to always enable the option when RCAR is
>> enabled IMO.
>
> I could fix this in kconfig like the followings:
>
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index f2bc5c3..905d1d2 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -46,6 +46,7 @@ menuconfig ARCH_RENESAS
>         select PINCTRL
>         select ARCH_REQUIRE_GPIOLIB
>         select ZONE_DMA if ARM_LPAE
> +       select USB_XHCI_RCAR if USB_XHCI_HCD
>
>  if ARCH_RENESAS
>
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index efa77c1..010d2b7 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -105,6 +105,7 @@ config ARCH_RENESAS
>         select PM
>         select PM_GENERIC_DOMAINS
>         select RENESAS_IRQC
> +       select USB_XHCI_RCAR if USB_XHCI_HCD
>         help
>           This enables support for the ARMv8 based Renesas SoCs.
>
> If this is acceptable, I will send each patch to arm / arm64.

What about enforcing this in a single place instead, i.e. in
drivers/usb/host/Kconfig?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v2] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven
> Sent: Monday, April 25, 2016 3:58 PM
> 
> Hi Shimoda-san,
> 
> On Mon, Apr 25, 2016 at 4:10 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> >> From: Rob Herring
> >> Sent: Friday, April 22, 2016 10:29 PM
> >> On Fri, Apr 22, 2016 at 4:36 AM, Felipe Balbi
> >> <felipe.balbi@linux.intel.com> wrote:
> >> > Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
> >> >> The firmware of R-Car USB 3.0 host controller will control the reset.
> >> >> So, if the xhci driver doesn't do firmware downloading (e.g. kernel
> >> >> configuration is CONFIG_USB_XHCI_PLATFORM=y and CONFIG_USB_XHCI_RCAR
> >> >> is not set), the reset of USB 3.0 host controller doesn't work
> >> >> correctly. Then, the host controller will cause long wait in
> >> >> xhci_reset() because the CMD_RESET bit of op_regs->command is not
> >> >> cleared for 10 seconds.
> >> >>
> >> >> So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
> >> >> to exit the probe function immediately.
> >> >>
> >> >> Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers)
> >> >> Cc: <stable@vger.kernel.org> # v3.17+
> >> >> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >> >> ---
> >> >>  Changes from v1:
> >> >>   - Revise the commit log.
> >> >>     (http://www.spinics.net/lists/stable/msg130007.html)
> >> >>
> >> >>  drivers/usb/host/xhci-rcar.h | 6 +++++-
> >> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
> >> >> index 2941a25..2afed68 100644
> >> >> --- a/drivers/usb/host/xhci-rcar.h
> >> >> +++ b/drivers/usb/host/xhci-rcar.h
> >> >> @@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
> >> >>
> >> >>  static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
> >> >>  {
> >> >> -     return 0;
> >> >> +     /*
> >> >> +      * To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
> >> >> +      * disabled, this function fails.
> >> >> +      */
> >> >> +     return -ENODEV;
> >> >
> >> > okay, if I understood correctly the thing is that you have a kernel
> >> > built with XHCI platform support but without XHCI RCAR support. Then, if
> >> > you run this kernel on RCAR board, you see this CMD_RESET problem,
> >> > right?
> 
> [...]
> 
> >> We should fix this in kconfig to always enable the option when RCAR is
> >> enabled IMO.
> >
> > I could fix this in kconfig like the followings:
> >
> > diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> > index f2bc5c3..905d1d2 100644
> > --- a/arch/arm/mach-shmobile/Kconfig
> > +++ b/arch/arm/mach-shmobile/Kconfig
> > @@ -46,6 +46,7 @@ menuconfig ARCH_RENESAS
> >         select PINCTRL
> >         select ARCH_REQUIRE_GPIOLIB
> >         select ZONE_DMA if ARM_LPAE
> > +       select USB_XHCI_RCAR if USB_XHCI_HCD
> >
> >  if ARCH_RENESAS
> >
> > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> > index efa77c1..010d2b7 100644
> > --- a/arch/arm64/Kconfig.platforms
> > +++ b/arch/arm64/Kconfig.platforms
> > @@ -105,6 +105,7 @@ config ARCH_RENESAS
> >         select PM
> >         select PM_GENERIC_DOMAINS
> >         select RENESAS_IRQC
> > +       select USB_XHCI_RCAR if USB_XHCI_HCD
> >         help
> >           This enables support for the ARMv8 based Renesas SoCs.
> >
> > If this is acceptable, I will send each patch to arm / arm64.
> 
> What about enforcing this in a single place instead, i.e. in
> drivers/usb/host/Kconfig?

Thank you for the suggestion. I could make such a patch.
I prefer this than each arm / arm64 patch.

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 3050b18..e9d4dde 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -35,6 +35,7 @@ config USB_XHCI_PCI
 
 config USB_XHCI_PLATFORM
 	tristate "Generic xHCI driver for a platform device"
+	select USB_XHCI_RCAR if ARCH_RENESAS
 	---help---
 	  Adds an xHCI host driver for a generic platform device, which
 	  provides a memory space and an irq.
@@ -63,7 +64,7 @@ config USB_XHCI_MVEBU
 
 config USB_XHCI_RCAR
 	tristate "xHCI support for Renesas R-Car SoCs"
-	select USB_XHCI_PLATFORM
+	depends on USB_XHCI_PLATFORM
 	depends on ARCH_RENESAS || COMPILE_TEST
 	---help---
 	  Say 'Y' to enable the support for the xHCI host controller

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

RE: [PATCH v2] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Yoshihiro Shimoda]

Hi,

> From: Felipe Balbi
> Sent: Friday, April 22, 2016 6:37 PM
> 
> Hi,
> 
> Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
> > The firmware of R-Car USB 3.0 host controller will control the reset.
> > So, if the xhci driver doesn't do firmware downloading (e.g. kernel
> > configuration is CONFIG_USB_XHCI_PLATFORM=y and CONFIG_USB_XHCI_RCAR
> > is not set), the reset of USB 3.0 host controller doesn't work
> > correctly. Then, the host controller will cause long wait in
> > xhci_reset() because the CMD_RESET bit of op_regs->command is not
> > cleared for 10 seconds.
> >
> > So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
> > to exit the probe function immediately.
> >
> > Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers)
> > Cc: <stable@vger.kernel.org> # v3.17+
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  Changes from v1:
> >   - Revise the commit log.
> >     (http://www.spinics.net/lists/stable/msg130007.html)
> >
> >  drivers/usb/host/xhci-rcar.h | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
> > index 2941a25..2afed68 100644
> > --- a/drivers/usb/host/xhci-rcar.h
> > +++ b/drivers/usb/host/xhci-rcar.h
> > @@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
> >
> >  static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
> >  {
> > -	return 0;
> > +	/*
> > +	 * To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
> > +	 * disabled, this function fails.
> > +	 */
> > +	return -ENODEV;
> 
> okay, if I understood correctly the thing is that you have a kernel
> built with XHCI platform support but without XHCI RCAR support. Then, if
> you run this kernel on RCAR board, you see this CMD_RESET problem,
> right?

Yes, that's right.

> Isn't this pointing to the fact that xhci-plat.ko built without RCAR
> isn't exactly compatible with RCAR ?

You're correct.

> IOW:
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 676ea458148b..3e39320564ce 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -112,6 +112,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
>  	}, {
>  		.compatible = "marvell,armada-380-xhci",
>  		.data = &xhci_plat_marvell_armada,
> +#if IS_ENABLED(CONFIG_USB_XHCI_RCAR)
>  	}, {
>  		.compatible = "renesas,xhci-r8a7790",
>  		.data = &xhci_plat_renesas_rcar_gen2,
> @@ -130,6 +131,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
>  	}, {
>  		.compatible = "renesas,rcar-gen3-xhci",
>  		.data = &xhci_plat_renesas_rcar_gen3,
> +#endif
>  	},
>  	{},
>  };

I see. This can avoid probing the xhci-plat.ko if RCAR is disabled.

Best regards,
Yoshihiro Shimoda

> Rob, should we limit compatible flags like this ? Without
> CONFIG_USB_XHCI_RCAR, this driver won't work on RCAR but, as you can
> see, this driver might still work on other non-RCAR platforms.
> 
> What's your take on this ?
> 
> --
> balbi