Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753681AbcD0Pq3 (ORCPT ); Wed, 27 Apr 2016 11:46:29 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:57249 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753654AbcD0Pq0 (ORCPT ); Wed, 27 Apr 2016 11:46:26 -0400 Date: Wed, 27 Apr 2016 10:46:20 -0500 From: Bin Liu To: Max Uvarov CC: , , , Subject: Re: [PATCHv2] musb_host: fix lockup on rxcsr_h_error Message-ID: <20160427154620.GB5604@uda0271908> Mail-Followup-To: Bin Liu , Max Uvarov , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, gregkh@linuxfoundation.org, sergei.shtylyov@cogentembedded.com References: <1461739918-9583-1-git-send-email-muvarov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <1461739918-9583-1-git-send-email-muvarov@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2529 Lines: 75 Hi, On Wed, Apr 27, 2016 at 09:51:58AM +0300, Max Uvarov wrote: > Fix soft lockup when resetting remote device attached > to usb host. Configuration: > pppd -> musb hub -> usb-serial -> gsm modem I have heard a few reports similar to this symptom, but never been able to reproduce it on my side. > When gsm modem resets, musb rolls in incoming rx interrupts > which does not give any time to other application as result > it totally lock ups. Solution is to keep original logic for RXCSR_H_ERROR Have you looked where exact place in the interrupt routine the execution has stuck in? > and merge RXCSR_DATAERROR and RXCSR_H_ERROR branches to call same code > for setting rx stall with MUSB_RXCSR_H_WZC_BITS. MUSB_RXCSR_H_WZC_BITS itself does not set rx stall, it just ensures MUSB_RXCSR_H_RXSTALL not to be cleared. Please check its comment in musb_regs.h. > > Signed-off-by: Max Uvarov > --- > v2: use bitwise or for error flags before logical and. (Sergei Shtylyov). > > drivers/usb/musb/musb_host.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c > index c3d5fc9..2d9aa78 100644 > --- a/drivers/usb/musb/musb_host.c > +++ b/drivers/usb/musb/musb_host.c > @@ -1592,14 +1592,12 @@ void musb_host_rx(struct musb *musb, u8 epnum) What kernel do you use? This line # is away off from upstream kernel. > > /* stall; record URB status */ > status = -EPIPE; > + } else if (rx_csr & (MUSB_RXCSR_DATAERROR | MUSB_RXCSR_H_ERROR)) { > > - } else if (rx_csr & MUSB_RXCSR_H_ERROR) { > - dev_dbg(musb->controller, "end %d RX proto error\n", epnum); > - > - status = -EPROTO; > - musb_writeb(epio, MUSB_RXINTERVAL, 0); > - > - } else if (rx_csr & MUSB_RXCSR_DATAERROR) { > + if (rx_csr & MUSB_RXCSR_H_ERROR) { > + status = -EPROTO; > + musb_writeb(epio, MUSB_RXINTERVAL, 0); > + } Please help me to understand how this change fixes the issue. I see the most effect of the change here is directly 'goto finish' so that 'done' flag is not set, then musb_advance_schedule() is not called. Is this the case or I missed other important pieces? Thanks, -Bin. > > if (USB_ENDPOINT_XFER_ISOC != qh->type) { > dev_dbg(musb->controller, "RX end %d NAK timeout\n", epnum); > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html