Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751718AbdGRUML (ORCPT ); Tue, 18 Jul 2017 16:12:11 -0400 Received: from mail-qt0-f172.google.com ([209.85.216.172]:33245 "EHLO mail-qt0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751413AbdGRUMJ (ORCPT ); Tue, 18 Jul 2017 16:12:09 -0400 MIME-Version: 1.0 In-Reply-To: <2B3535C5ECE8B5419E3ECBE300772909021B534712@US01WEMBX2.internal.synopsys.com> References: <410670D7E743164D87FA6160E7907A56FDA5E05E@am04wembxa.internal.synopsys.com> <2B3535C5ECE8B5419E3ECBE300772909021B534712@US01WEMBX2.internal.synopsys.com> From: John Stultz Date: Tue, 18 Jul 2017 13:12:07 -0700 Message-ID: Subject: Re: bug? dwc2: insufficient fifo memory To: John Youn Cc: Minas Harutyunyan , lkml , Felipe Balbi Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8067 Lines: 188 On Mon, Jun 5, 2017 at 10:50 AM, John Youn wrote: > On 6/5/2017 5:32 AM, Minas Harutyunyan wrote: >> On 6/2/2017 10:20 PM, John Stultz wrote: >>> On Mon, Apr 17, 2017 at 3:36 PM, John Stultz wrote: >>>> On Fri, Feb 24, 2017 at 2:46 PM, John Stultz wrote: >>>>> Hey John, >>>>> So after the USB tree landed in 4.11-rc, I've been seeing the >>>>> following warning at bootup. >>>>> >>>>> It seems the fifo_mem/total_fifo_size value on hikey is 1920, but I'm >>>>> seeing the addresses zip upward quickly as the txfsz values are all >>>>> 2048. Not exactly sure whats wrong here. Things still seem to work >>>>> properly. >>>>> >>>>> thanks >>>>> -john >>>>> >>>>> >>>>> [ 8.944987] dwc2 f72c0000.usb: bound driver configfs-gadget >>>>> [ 8.956651] insufficient fifo memory >>>>> [ 8.956703] ------------[ cut here ]------------ >>>>> [ 8.964906] WARNING: CPU: 7 PID: 1 at drivers/usb/dwc2/gadget.c:330 >>>>> dwc2_hsotg_init_fifo+0x1a8/0x1c8 >>>> >>>> >>>> Hey John, >>>> So I finally got a bit of time to look deeper into this, and it >>>> seems like this issue was introduced by commit 3c6aea7344c3 ("usb: >>>> dwc2: gadget: Add checking for g-tx-fifo-size parameter"), as that >>>> change added the following snippit: >>>> >>>> if (hsotg->params.g_tx_fifo_size[fifo] < min || >>>> hsotg->params.g_tx_fifo_size[fifo] > dptxfszn) { >>>> dev_warn(hsotg->dev, "%s: Invalid parameter >>>> g_tx_fifo_size[%d]=%d\n", >>>> __func__, fifo, >>>> hsotg->params.g_tx_fifo_size[fifo]); >>>> hsotg->params.g_tx_fifo_size[fifo] = dptxfszn; >>>> } >>>> >>>> On HiKey, we have g-tx-fifo-size = <128 128 128 128 128 128> in the >>>> dtsi, and the fifo_mem value ends up initialized at 1920. >>>> >>>> Unfortunately, in the above, it sees other entries in the >>>> g_tx_fifo_size[] array are initialized to zero, which then causes them >>>> to be set to the "default" value of dptxfszn which is 2048. So then >>>> later in dwc2_hsotg_init_fifo() the addr value (which adds the >>>> fifo_size array value each step) quickly grows beyond the fifo_mem >>>> value, causing the warning. >>>> >>>> Not sure what the right fix is here? Should the min value be used >>>> instead of the "default" dptxfszn value? Again, I'm not sure I see >>>> any side-effects from this warning, but wanted to try to figure out >>>> how to fix it properly. >>> >>> Hey John, Minas, >>> Wanted to follow up on this again. I'm using a patch that looks as >>> follows (sorry for the copy-paste whitespace damage) in the meantime >>> to work around this issue: >>> >>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c >>> index 9cd8722..13a911b 100644 >>> --- a/drivers/usb/dwc2/params.c >>> +++ b/drivers/usb/dwc2/params.c >>> @@ -475,7 +475,7 @@ static void dwc2_check_param_tx_fifo_sizes(struct >>> dwc2_hsotg *hsotg) >>> dev_warn(hsotg->dev, "%s: Invalid parameter >>> g_tx_fifo_size[%d]=%d\n", >>> __func__, fifo, >>> hsotg->params.g_tx_fifo_size[fifo]); >>> - hsotg->params.g_tx_fifo_size[fifo] = dptxfszn; >>> + hsotg->params.g_tx_fifo_size[fifo] = min; >>> } >>> } >>> } >>> >>> Any suggestions on what a proper fix would look like? >>> >>> thanks >>> -john >>> >> >> Hi John, >> >> The patch series: "[PATCH 0/4] usb: dwc2: gadget: Fix dynamic FIFO >> initialization flow" from Sevak Arakelyan submitted to LKML at 4/26/2017 >> should resolve this issue. >> >> Thanks, >> Minas >> > > Hi John, > > See if this patch helps and let us know. > > Unfortunately, the author of this patch (and expert on this part of > the code) has left Synopsys. So it might take us a bit to look into > this. Hey Folks, I realized I didn't get back to you, as I got pulled out to other work. I was testing with 4.13-rc, and noticed the same issue, and figured I should get back to you. I've applied the 4 patches from the patch series you've pointed me to. Though with this patchset I still hit the same warning, but this time from dwc2_hsotg_core_init_disconnected() rather then dwc2_hsotg_init_fifo(). [ 8.016067] init: processing action (sys.usb.config=adb && sys.usb.configfs=1 && sys.usb.ffs.ready=1) from (/init.usb.configfs.rc:21) [ 8.029367] dwc2 f72c0000.usb: bound driver configfs-gadget [ 8.035140] ------------[ cut here ]------------ [ 8.043357] WARNING: CPU: 7 PID: 1 at drivers/usb/dwc2/gadget.c:317 dwc2_hsotg_core_init_disconnected+0x52c/0x560 [ 8.053643] CPU: 7 PID: 1 Comm: init Not tainted 4.13.0-rc1-00038-ga257c7e #3331 [ 8.061042] Hardware name: HiKey Development Board (DT) [ 8.066285] task: ffffffc005f68000 task.stack: ffffffc005f70000 [ 8.072230] PC is at dwc2_hsotg_core_init_disconnected+0x52c/0x560 [ 8.078415] LR is at dwc2_hsotg_core_init_disconnected+0x52c/0x560 [ 8.084593] pc : [] lr : [] pstate: 600001c5 [ 8.091987] sp : ffffffc005f73be0 [ 8.095298] x29: ffffffc005f73be0 x28: ffffff8008dc9a18 [ 8.100629] x27: ffffffc074d1907c x26: 000000000000000f [ 8.105949] x25: 0000000000000007 x24: 000000000000011c [ 8.111263] x23: 0000000000000580 x22: 0000000000000000 [ 8.116577] x21: 0000000000000007 x20: 0000000008000580 [ 8.121902] x19: ffffffc074d19018 x18: 0000000000000010 [ 8.127229] x17: aaaaaaaaaaaaaaab x16: ffffff80081da680 [ 8.132548] x15: ffffff8088e5b3a7 x14: 0000000000000006 [ 8.137863] x13: ffffff8008e5b3b5 x12: ffffff8008d89118 [ 8.143176] x11: 0000000000000000 x10: ffffffc005f73be0 [ 8.148493] x9 : ffffffc005f73be0 x8 : 79726f6d656d206f [ 8.153807] x7 : 66696620746e6569 x6 : 0000000000000340 [ 8.159121] x5 : 0000000000000015 x4 : 0000000000000000 [ 8.164439] x3 : 0000000000000002 x2 : 0000000000000002 [ 8.169769] x1 : 0000000000000001 x0 : 0000000000000018 [ 8.175099] Call trace: [ 8.177552] Exception stack(0xffffffc005f73a10 to 0xffffffc005f73b40) [ 8.183990] 3a00: ffffffc074d19018 0000008000000000 [ 8.191829] 3a20: ffffffc005f73be0 ffffff80086a65ac 0000000000000007 000000000000000f [ 8.199664] 3a40: ffffffc074d1907c ffffff8008dc9a18 ffffffc005f73a90 0000000000000000 [ 8.207512] 3a60: ffffffc005f73be0 ffffffc005f73be0 ffffffc005f73ba0 00000000ffffffc8 [ 8.215362] 3a80: ffffffc005f73ab0 ffffff80081057a4 ffffffc005f73be0 ffffffc005f73be0 [ 8.223210] 3aa0: ffffffc005f73ba0 00000000ffffffc8 0000000000000018 0000000000000001 [ 8.231065] 3ac0: 0000000000000002 0000000000000002 0000000000000000 0000000000000015 [ 8.238919] 3ae0: 0000000000000340 66696620746e6569 79726f6d656d206f ffffffc005f73be0 [ 8.246774] 3b00: ffffffc005f73be0 0000000000000000 ffffff8008d89118 ffffff8008e5b3b5 [ 8.254627] 3b20: 0000000000000006 ffffff8088e5b3a7 ffffff80081da680 aaaaaaaaaaaaaaab [ 8.262487] [] dwc2_hsotg_core_init_disconnected+0x52c/0x560 [ 8.269740] [] dwc2_hsotg_pullup+0x90/0x100 [ 8.275495] [] usb_udc_connect_control.isra.0+0x78/0x90 [ 8.282308] [] udc_bind_to_driver+0xec/0x110 [ 8.288166] [] usb_gadget_probe_driver+0xa0/0x150 [ 8.294459] [] gadget_dev_desc_UDC_store+0xe4/0x100 [ 8.300931] [] configfs_write_file+0xc0/0x198 [ 8.306881] [] __vfs_write+0x1c/0x118 [ 8.312133] [] vfs_write+0xa0/0x1b0 [ 8.317201] [] SyS_write+0x44/0xa0 [ 8.322193] [] el0_svc_naked+0x24/0x28 [ 8.327508] ---[ end trace c6da4c029bccb75e ]--- The same hack I was using earlier - hsotg->params.g_tx_fifo_size[fifo] = dptxfszn; + hsotg->params.g_tx_fifo_size[fifo] = min; still avoids the issue. thanks -john