| Message ID | 87mvom7zo5.fsf@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
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
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
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 }, {}, };