* Re: refine xhci-plat-Fix-xhci_plat-shutdown-sequence
2018-04-17 14:32 ` refine xhci-plat-Fix-xhci_plat-shutdown-sequence leiwan
@ 2018-04-17 14:37 ` Greg KH
0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2018-04-17 14:37 UTC (permalink / raw)
To: leiwan; +Cc: stable
On Tue, Apr 17, 2018 at 10:32:28PM +0800, leiwan@codeaurora.org wrote:
> 在 2018-04-17 18:32，Greg KH 写道：
> > On Tue, Apr 17, 2018 at 11:32:42AM +0800, leiwan@codeaurora.org wrote:
> > >
> > > xhci-plat Shutdown callback should check HCD_FLAG_HW_ACCESSIBLE
> > > before accessing any register. This should avoid hung with access
> > > controllers which support runtime suspend
> > >
> > > This can fix for issue of https://patchwork.kernel.org/patch/10339317/
> > > corresponding upload in CAF:
> > > https://source.codeaurora.org/quic/la/kernel/msm-4.4/commit/?h=LV.HB.1.1.5-03810-8x96.0&id=a7a5307ee04ad349d365ad50f304605a9cd9bd0a
> > >
> > > full patch refer attachment.
> > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > > index 9b27798..bdf914d 100644
> > > --- a/drivers/usb/host/xhci.c
> > > +++ b/drivers/usb/host/xhci.c
> > > @@ -702,6 +702,10 @@ static void xhci_shutdown(struct usb_hcd *hcd)
> > > usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
> > >
> > > spin_lock_irq(&xhci->lock);
> > > + if (!HCD_HW_ACCESSIBLE(hcd)) {
> > > + spin_unlock_irq(&xhci->lock);
> > > + return;
> > > + }
> > > xhci_halt(xhci);
> >
> > A blank line after the if statement?
> > >> [lei]yes
> > What about all of the other places in this driver that you should also
> > check for this? Look at the other host controllers, shouldn't you
> > mirror what they are doing?
> > >> [lei]I checked other usb host module shutdown and suspend workflow.
> >> All usb host driver need to check hw accessable before
> >> read/write usb register especially in runtime PM case..
> > And this needs a Fixes: tag, along with a cc: stable so as to properly
> > get backported as this is broken in some stable kernels right now.
> > >> [lei] Added by v2 patch
> > thanks,
> >
> > greg k-h
>
>
> From c03697fa259ab38d1002598ec2ccfac37607ca0b Mon Sep 17 00:00:00 2001
> From: Lei wang <leiwan@codeaurora.org>
> Date: Tue, 17 Apr 2018 10:55:35 +0800
> Subject: [PATCH v2] xhci: plat: Fix xhci_plat shutdown hung
>
> xhci-plat Shutdown callback should check HCD_FLAG_HW_ACCESSIBLE
> before accessing any register. This should avoid hung with access
> controllers which support runtime suspend
>
> Fixes: b07c12517f2a ("xhci: plat: Register shutdown for xhci_plat")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Lei wang <leiwan@codeaurora.org>
> ---
> drivers/usb/host/xhci.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 9b27798..bdf914d 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -702,6 +702,10 @@ static void xhci_shutdown(struct usb_hcd *hcd)
> usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
>
> spin_lock_irq(&xhci->lock);
> + if (!HCD_HW_ACCESSIBLE(hcd)) {
> + spin_unlock_irq(&xhci->lock);
> + return;
> + }
> xhci_halt(xhci);
> /* Workaround for spurious wakeups at shutdown with HSW */
> if (xhci->quirks & XHCI_SPURIOUS_WAKEUP)
> --
> 1.9.1
I totally do not understand, why did you forward a broken patch that was
not accepted upstream to the stable list?
Please go read Documentation/SubmittingPatches for how to do this
correctly. Your first email was close, but it was attached, which isn't
ok. And then I provided code review comments which you seem to have
ignored here...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 2+ messages in thread