From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756063AbcEWPT7 (ORCPT ); Mon, 23 May 2016 11:19:59 -0400 Received: from mail1.bemta14.messagelabs.com ([193.109.254.106]:48885 "EHLO mail1.bemta14.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753221AbcEWPTz convert rfc822-to-8bit (ORCPT ); Mon, 23 May 2016 11:19:55 -0400 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrMKsWRWlGSWpSXmKPExsVyU6Zrou4Eeed wg4MnJSz+31nEatG8eD2bxeVdc9gsFi1rZbY4s+oWu8WCjY8YHdg8Hkz9z+Sxf+4ado/jN7Yz eXzeJBfAEsWamZeUX5HAmnFmzlbGgs1SFXe+T2BuYFwk2sXIxSEksIlR4u2ru+xdjJxAzl5Gi f93C0FsNgEziV2v1oHFRQQiJE4suMgM0sAs8JpR4v6F6UwgCWGBQIlt15dAFQVJLD3eCWX7SW xo2MkCYrMIqEqcvjyLFcTmFXCVuP7tNRPE5n9MEo3rv4IVcQq4SVyedRGsmVFAVuJL42pmEJt ZQFzi1pP5YMskBAQkluw5zwxhi0q8fPyPFcI2kNi6dB8LhK0g8aP3FFSNtcS/B3eZIOboSCzY /YkNwtaWWLbwNTPEQYISJ2c+YZnAKDYLybpZSFpmIWmZhaRlASPLKkaN4tSistQiXUMLvaSiz PSMktzEzBxdQ0MTvdzU4uLE9NScxKRiveT83E2MwMisZ2Bg3MF4ZLvnIUZJDiYlUd6TO5zChf iS8lMqMxKLM+KLSnNSiw8xynBwKEnwPpZzDhcSLEpNT61Iy8wBpgiYtAQHj5II71GQNG9xQWJ ucWY6ROoUo6KUOC8vMLEICYAkMkrz4NpgaekSo6yUMC8jAwODEE9BalFuZgmq/CtGcQ5GJWHe dyDjeTLzSuCmvwJazAS0+KG0A8jikkSElFQDo23fzA+fdqRVTos++a4qwsJKSbv62rk/azIL5 stv5rrXUKR61eNx2z3Vyu+lk5sXnHG07d5UYBKp+7vvadCB4v3bv+zWfcz5Yrfhp4b7SU+Kzl 7Q55H+4xL+7lGVh39skd/+r19XWN1llWV50SiV33uE8d56tl/S0+z2Z5x5GxNVNCvko4SRmBJ LcUaioRZzUXEiAEQgQp9GAwAA X-Env-Sender: andrew.goodbody@cambrionix.com X-Msg-Ref: server-9.tower-92.messagelabs.com!1464016780!32730201!2 X-Originating-IP: [217.28.138.145] X-StarScan-Received: X-StarScan-Version: 8.34; banners=-,-,- X-VirusChecked: Checked From: Andrew Goodbody To: Sergei Shtylyov , "b-liu@ti.com" CC: "gregkh@linuxfoundation.org" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" Subject: RE: [PATCH V2 2/2] usb: musb: Stop bulk endpoint while queue is rotated Thread-Topic: [PATCH V2 2/2] usb: musb: Stop bulk endpoint while queue is rotated Thread-Index: AQHRtOrNDeZOdsfoq0qoHTZEXPmD3Z/GZYgAgAAaspD///Z1gIAAD9yAgAAb/AA= Date: Mon, 23 May 2016 15:19:39 +0000 Message-ID: <4b83d63f052640148935c1ff4015486c@SEHSTE15D2BE2.hs20.net> References: <1464004844-26553-1-git-send-email-andrew.goodbody@cambrionix.com> <1464004844-26553-3-git-send-email-andrew.goodbody@cambrionix.com> <6b25b4a5cc104ea0adc5bd006040529b@SEHSTE15D2BE2.hs20.net> <6a1d9c4c-5501-bf4f-9ddc-9ffe6e2371a1@cogentembedded.com> In-Reply-To: <6a1d9c4c-5501-bf4f-9ddc-9ffe6e2371a1@cogentembedded.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [46.17.166.30] x-exclaimer-md-config: 266e7a57-cddd-49fd-bdea-19bca6d40303 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com] > On 05/23/2016 04:40 PM, Sergei Shtylyov wrote: > > >>>> Ensure that the endpoint is stopped by clearing REQPKT before > >>>> clearing DATAERR_NAKTIMEOUT before rotating the queue on the > >>>> dedicated bulk endpoint. > >>>> This addresses an issue where a race could result in the endpoint > >>>> receiving data before it was reprogrammed resulting in a warning > >>>> about such data from musb_rx_reinit before it was thrown away. > >>>> The data thrown away was a valid packet that had been correctly > >>>> ACKed which meant the host and device got out of sync. > >>>> > >>>> Signed-off-by: Andrew Goodbody > > >>>> Cc: stable@vger.kernel.org > >>>> --- > >>>> V2 added comment about clearing REQPKT before > DATAERR_NAKTIMEOUT > >>>> > >>>> drivers/usb/musb/musb_host.c | 6 ++++++ > >>>> 1 file changed, 6 insertions(+) > >>>> > >>>> diff --git a/drivers/usb/musb/musb_host.c > >>>> b/drivers/usb/musb/musb_host.c index 2966596..676cb98 100644 > >>>> --- a/drivers/usb/musb/musb_host.c > >>>> +++ b/drivers/usb/musb/musb_host.c > >>>> @@ -997,6 +997,12 @@ static void musb_bulk_nak_timeout(struct > musb > >>> *musb, struct musb_hw_ep *ep, > >>>> /* clear nak timeout bit */ > >>>> rx_csr = musb_readw(epio, MUSB_RXCSR); > >>>> rx_csr |= MUSB_RXCSR_H_WZC_BITS; > >>>> + /* > >>>> + * Need to stop the transaction by clearing REQPKT before > >>> > >>> Transcation? Maybe transfer? > >> > >> The quote from the TRM is "If the DATAERR_NAKTIMEOUT bit is set, the > >> controller can be directed either to continue trying this transaction > >> (until it times out again) by clearing the DATAERR_NAKTIMEOUT bit or > >> to abort the transaction by clearing REQPKT bit before clearing the > DATAERR_NAKTIMEOUT bit." > >> So 'transaction' is correct. > > > > OK. > > > >>>> + * DATAERR_NAKTIMEOUT ref TRM section 16.3.8.2.2.1.2 > >>> > >>> Which TRM? Do you understand that the MUSB core is used by > >>> multiple SoCs? > >>> I'd recommend referring just to the "abstract" manual or the MUSB > >>> programmer's guide (section 9.2.2 if you want an exact ref.). > > > >> That would be the AM335x Sitara Processors TRM. I don't have the MUSB > >> programmer's guide, is it available online somewhere? > > > > AFAIK, no. > > > >> I do not understand what you mean by the "abstract" manual. > > > > Just saying "the manual" or "the manuals". > > > >> Would you be OK with "ref MUSB Programmer's Guide section 9.2.2" > > > > Yes. > > The full document name is "MUSBMHDRC USB 2.0 HIGH-SPEED DUAL-ROLE > CONTROLLER Programmer's Guide". I moved the comment up to replace the one at the top so the patch now looks like this if (is_in) { dma = is_dma_capable() ? ep->rx_channel : NULL; - /* clear nak timeout bit */ + /* + * Need to stop the transaction by clearing REQPKT first + * then the NAK Timeout bit ref MUSBMHDRC USB 2.0 HIGH-SPEED + * DUAL-ROLE CONTROLLER Programmer's Guide, section 9.2.2 + */ rx_csr = musb_readw(epio, MUSB_RXCSR); rx_csr |= MUSB_RXCSR_H_WZC_BITS; + rx_csr &= ~MUSB_RXCSR_H_REQPKT; + musb_writew(epio, MUSB_RXCSR, rx_csr); rx_csr &= ~MUSB_RXCSR_DATAERROR; musb_writew(epio, MUSB_RXCSR, rx_csr); Are you OK with that now? Andrew