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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Felipe,

> From: Felipe Balbi
> Sent: Wednesday, June 01, 2016 4:01 PM
> 
> Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> 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.balbi@xxxxxxxxxxxxxxx>
> > 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.uh@xxxxxxxxxxx>
> >     Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
> >     Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> >     Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> >
> > < 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);

Thank you for the comment!
You're correct. This also can resolve the issue.
Do you prefer such a simple patch?
At least, I prefer such a simple patch :)
Why I wrote this patch set is I thought I should implement similar flow before regression.

Best regards,
Yoshihiro Shimoda




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux