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

[Felipe Balbi]

Hi,

Yoshihiro Shimoda <yoshihiro.shimoda...@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: <sta...@vger.kernel.org> # v3.17+
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda...@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

signature.asc
Description: PGP signature