From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:54332 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753628AbeDQOh7 (ORCPT ); Tue, 17 Apr 2018 10:37:59 -0400 Date: Tue, 17 Apr 2018 16:37:52 +0200 From: Greg KH To: leiwan@codeaurora.org Cc: stable@vger.kernel.org Subject: Re: refine xhci-plat-Fix-xhci_plat-shutdown-sequence Message-ID: <20180417143752.GA12398@kroah.com> References: <20180417103226.GA8445@kroah.com> <9445d31a0d3a7e26bb6dc1807539ad6e@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <9445d31a0d3a7e26bb6dc1807539ad6e@codeaurora.org> Sender: stable-owner@vger.kernel.org List-ID: 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 > 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: > Signed-off-by: Lei wang > --- > 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