From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752531AbdLEQT1 (ORCPT ); Tue, 5 Dec 2017 11:19:27 -0500 Received: from mout.kundenserver.de ([212.227.126.134]:58923 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752171AbdLEQTX (ORCPT ); Tue, 5 Dec 2017 11:19:23 -0500 Subject: Re: [PATCH v3 1/2] usb: dwc2: host: Don't retry NAKed transactions right away To: balbi@kernel.org, johnyoun@synopsys.com Cc: Douglas Anderson , amstan@chromium.org, linux-rockchip@lists.infradead.org, gregkh@linuxfoundation.org, johan@kernel.org, eric@anholt.net, mka@chromium.org, john.stultz@linaro.org, linux-rpi-kernel@lists.infradead.org, jwerner@chromium.org, stable@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org References: <20171030170802.14489-1-dianders@chromium.org> From: Stefan Wahren Message-ID: Date: Tue, 5 Dec 2017 17:18:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20171030170802.14489-1-dianders@chromium.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Provags-ID: V03:K0:fVk8wRLlJDOx0Ft6sltwL/p75qx3/wkAqSckQqYov8uK2f+PeHD HhQsaqZODgCodZjizceXDpEAPj5A6GzSSh0khLrSPPVC8pPvFOSmT7eUatHB6ylmlL8H4GW CUBqrIFZeXwl4zVP2KXgbR2yuyecUSTUlI4b78dh1tKltqZL9+B9PAy+17B0o6oMgYEjCo3 6DbQoFcEGCxrtx4faun+Q== X-UI-Out-Filterresults: notjunk:1;V01:K0:/sDanvGxSMs=:YqdutYxkYqapUw96VHrVZH PnkJpRZ9vwBZnOsfh+gaKrUisypD3R++G1Sz5a9F1EzFIEhRYghH+/hGucnVtMOmJYtWB11mv CG96/+w+fhYcUXYUZf1aZyE+13WGmOD81prOuA5wj09uUwcZPUn3F1BvNjjjjH4H5F08pxyEz uq5yUoGti6a1StyE+lfoeUbPDz//RXHfA0SePnXKZEvXUyQF4ngzjRc/MV+5I+6ukai/uJk/b sz/p3620XO0VcNCsi27Px249Lkl/zvm8fGtXOkJ9Oh/0IwRwWEmBMCYx0Io5EPQbS3HWefIuC Z+Csj0XJen+/pczVLBOW7u/ywkNawM1VmG7ytDbkP2wbiGBV7WFLETnJilCePBRiFJRbc4SHy SlZ9xc3uZLypwYI4Ij2NyYussYJ0X/pRRddFvEg101bv9vEJS39DETdzmSywA+WBLiAxCz6/3 uG2CDOVpeMKVniwMvTDvqw+o0Ghxhgt5Ah16yvJMwe5MNRo/J+w0F+QmlvxEnlsldqXzCxXsp 6gNkOc2UGS4CwUH1SEJMllkAZV4gKqYVtxyIuAEMnUrsgLhz/iOUYHtGh60fH8wJUPg1OGpF0 upYDsO6oITRVjJu05aYUIBHw62W0hA1Btm42yI8GKLOtvlaflnXDRPz4pVx8Q7uyJZLMlGcme x5f3gf0nPeNZxdp/OBbBaKXiEzaSyfqUsBrWjYK1iJSkuAzXOF9TY+RqBS97tp/e2jwrtZxdY z2ycW72qWLtY98/xuZuAogZRLIXI1oiB2DiYuZXFyC2+8VVN/NNF/ozQ7e4= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Felipe, Hi John, Am 30.10.2017 um 18:08 schrieb Douglas Anderson: > On rk3288-veyron devices on Chrome OS it was found that plugging in an > Arduino-based USB device could cause the system to lockup, especially > if the CPU Frequency was at one of the slower operating points (like > 100 MHz / 200 MHz). > > Upon tracing, I found that the following was happening: > * The USB device (full speed) was connected to a high speed hub and > then to the rk3288. Thus, we were dealing with split transactions, > which is all handled in software on dwc2. > * Userspace was initiating a BULK IN transfer > * When we sent the SSPLIT (to start the split transaction), we got an > ACK. Good. Then we issued the CSPLIT. > * When we sent the CSPLIT, we got back a NAK. We immediately (from > the interrupt handler) started to retry and sent another SSPLIT. > * The device kept NAKing our CSPLIT, so we kept ping-ponging between > sending a SSPLIT and a CSPLIT, each time sending from the interrupt > handler. > * The handling of the interrupts was (because of the low CPU speed and > the inefficiency of the dwc2 interrupt handler) was actually taking > _longer_ than it took the other side to send the ACK/NAK. Thus we > were _always_ in the USB interrupt routine. > * The fact that USB interrupts were always going off was preventing > other things from happening in the system. This included preventing > the system from being able to transition to a higher CPU frequency. > > As I understand it, there is no requirement to retry super quickly > after a NAK, we just have to retry sometime in the future. Thus one > solution to the above is to just add a delay between getting a NAK and > retrying the transmission. If this delay is sufficiently long to get > out of the interrupt routine then the rest of the system will be able > to make forward progress. Even a 25 us delay would probably be > enough, but we'll be extra conservative and try to delay 1 ms (the > exact amount depends on HZ and the accuracy of the jiffy and how close > the current jiffy is to ticking, but could be as much as 20 ms or as > little as 1 ms). > > Presumably adding a delay like this could impact the USB throughput, > so we only add the delay with repeated NAKs. > > NOTE: Upon further testing of a pl2303 serial adapter, I found that > this fix may help with problems there. Specifically I found that the > pl2303 serial adapters tend to respond with a NAK when they have > nothing to say and thus we end with this same sequence. > > Signed-off-by: Douglas Anderson > Cc: stable@vger.kernel.org > Reviewed-by: Julius Werner > Tested-by: Stefan Wahren > --- > > Changes in v3: > - Add tested-by for Stefan Wahren > - Sent to Felipe Balbi as candiate to land this. > - Add Cc for stable (it's always been broken so go as far is as easy) > > Changes in v2: > - Address http://crosreview.com/737520 feedback > does it need a resend?