Re: [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT

[Felipe Balbi]

Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com> writes:

> I'm afraid but I found a regression of xhci-rcar in v4.7-rc1.
> This regression is caused by the following commit:
>
> commit b1c127ae990bccf0187d741c1695a61e54de1943
> Author: Felipe Balbi <felipe.ba...@linux.intel.com>
> Date:   Fri Apr 22 13:17:16 2016 +0300
>
>     usb: host: xhci: plat: make use of new methods in xhci_plat_priv
>     
>     Now that the code has been refactored enough,
>     switching over to using ->plat_start() and
>     ->init_quirk() becomes a very simple patch.
>     
>     After this patch, there are no further uses for
>     xhci_plat_type_is() which will be removed in a
>     follow-up patch.
>     
>     Acked-by: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
>     Signed-off-by: Felipe Balbi <felipe.ba...@linux.intel.com>
>     Signed-off-by: Mathias Nyman <mathias.ny...@linux.intel.com>
>     Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
>
> < Overview >
> The regression is the quirks flag "XHCI_NO_64BIT_SUPPORT" will be overwritten
> by xhci_gen_setup(). Then, the driver will not work correctly.
>
> < Detail >
> Since the previous code will do the following, the quirks flag can be set:
>
> xhci_plat_setup()
>  -> xhci_gen_setup(hcd, xhci_plat_quirks);
>   -> xhci->quirks = quirks;
>    -> get_quirks() [This is xhci_plat_quirks]
>     -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT
>
> However, after we applied the patch above, the quirks will disappear:
>
> xhci_plat_setup()
>  -> xhci_priv_init_quirk();
>   -> xhci_rcar_init_quirk();
>    -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT
>   -> xhci_gen_setup(hcd, xhci_plat_quirks);
>    -> xhci->quirks = quirks; <----------------- here
>     -> get_quirks() [This is xhci_plat_quirks]
>
> So, I submitted incremental patches to resolve this issue like the followings:
>
> xhci_plat_setup()
>  -> xhci_priv_init_quirk();
>   -> xhci_rcar_init_quirk();
>   -> xhci_gen_setup(hcd, xhci_plat_quirks);
>    -> xhci->quirks = quirks;
>     -> get_quirks() [This is xhci_plat_quirks]
>      -> xhci->quirks |= priv->quirks (XHCI_NO_64BIT_SUPPORT)

isn't the following enough?

@@ -4886,7 +4886,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t 
get_quirks)
                xhci->hcc_params2 = readl(&xhci->cap_regs->hcc_params2);
        xhci_print_registers(xhci);
 
-       xhci->quirks = quirks;
+       xhci->quirks |= quirks;
 
        get_quirks(dev, xhci);
 
-- 
balbi

signature.asc
Description: PGP signature