* [Bug] usb: dwc2: host function broken in OTG mode on bcm283x
@ 2016-08-20 13:02 Stefan Wahren
2016-08-22 20:06 ` John Youn
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Wahren @ 2016-08-20 13:02 UTC (permalink / raw)
To: linux-arm-kernel
Hi John,
on bcm2835 in OTG mode the USB host function broken (USB devices don't
enumerate) with the following settings:
CONFIG_USB=y
CONFIG_USB_OTG=y
CONFIG_USB_DWC2=y
CONFIG_USB_DWC2_DUAL_ROLE=y
CONFIG_USB_PHY=y
CONFIG_NOP_USB_XCEIV=y
CONFIG_USB_GADGET=y
CONFIG_USB_ZERO=y
and DT don't provide any optional gadget fifo settings. During boot the kernel
give a warning about insufficient fifo memory.
Here is the relevant log with some debug messages:
[ 2.373718] dwc2_lowlevel_hw_init()
[ 2.389306] dwc2_set_all_params()
[ 2.404927] dwc2_lowlevel_hw_enable()
[ 2.420331] dwc2_get_dr_mode()
[ 2.434894] dwc2_get_dr_mode: OF suggests 3
[ 2.450592] dwc2_get_dr_mode: No PHY found
[ 2.466161] dwc2_core_reset_and_force_dr_mode()
[ 2.517779] mmc0: new SDHC card at address 1234
[ 2.535005] mmcblk0: mmc0:1234 SA32G 29.3 GiB
[ 2.553421] mmcblk0: p1 p2
[ 2.619213] dwc2_get_hwparams()
[ 2.633611] dwc2 20980000.usb: Core Release: 2.80a (snpsid=4f54280a)
[ 2.633628] dwc2 20980000.usb: hwcfg1=00000000
[ 2.633640] dwc2 20980000.usb: hwcfg2=228ddd50
[ 2.633651] dwc2 20980000.usb: hwcfg3=0ff000e8
[ 2.633663] dwc2 20980000.usb: hwcfg4=1ff00020
[ 2.633674] dwc2 20980000.usb: grxfsiz=00001000
[ 2.633688] dwc2 20980000.usb: gnptxfsiz=01001000
[ 2.633698] dwc2 20980000.usb: hptxfsiz=02002000
[ 2.633710] dwc2 20980000.usb: Forcing mode to device
[ 2.679241] dwc2 20980000.usb: gnptxfsiz=01001000
[ 2.729220] dwc2 20980000.usb: Detected values from hardware:
[ 2.729237] dwc2 20980000.usb: op_mode=0
[ 2.729249] dwc2 20980000.usb: arch=2
[ 2.729259] dwc2 20980000.usb: dma_desc_enable=0
[ 2.729271] dwc2 20980000.usb: power_optimized=0
[ 2.729285] dwc2 20980000.usb: i2c_enable=0
[ 2.729294] dwc2 20980000.usb: hs_phy_type=1
[ 2.729305] dwc2 20980000.usb: fs_phy_type=1
[ 2.729315] dwc2 20980000.usb: utmi_phy_data_width=0
[ 2.729325] dwc2 20980000.usb: num_dev_ep=7
[ 2.729335] dwc2 20980000.usb: num_dev_perio_in_ep=0
[ 2.729345] dwc2 20980000.usb: host_channels=8
[ 2.729356] dwc2 20980000.usb: max_transfer_size=524287
[ 2.729367] dwc2 20980000.usb: max_packet_count=1023
[ 2.729377] dwc2 20980000.usb: nperio_tx_q_depth=0x4
[ 2.729387] dwc2 20980000.usb: host_perio_tx_q_depth=0x4
[ 2.729398] dwc2 20980000.usb: dev_token_q_depth=0x8
[ 2.729408] dwc2 20980000.usb: enable_dynamic_fifo=1
[ 2.729418] dwc2 20980000.usb: en_multiple_tx_fifo=1
[ 2.729429] dwc2 20980000.usb: total_fifo_size=4080
[ 2.729439] dwc2 20980000.usb: host_rx_fifo_size=4096
[ 2.729450] dwc2 20980000.usb: host_nperio_tx_fifo_size=256
[ 2.729461] dwc2 20980000.usb: host_perio_tx_fifo_size=512
[ 2.729469] dwc2 20980000.usb:
[ 2.729474] dwc2_set_parameters()
[ 2.743824] dwc2 20980000.usb: dwc2_set_parameters()
[ 2.743848] dwc2 20980000.usb: Setting dma_desc_fs_enable to 0
[ 2.743881] dwc2 20980000.usb: Setting external_id_pin_ctl to 0
[ 2.743895] dwc2 20980000.usb: Setting hibernation to 0
[ 2.743899] dwc2_force_dr_mode()
[ 2.869216] dwc2_gadget_init()
[ 2.883027] dwc2 20980000.usb: Specified GNPTXFDEP=1024 > 256
[ 2.899480] dwc2 20980000.usb: NonPeriodic TXFIFO size: 256
[ 2.899497] dwc2 20980000.usb: RXFIFO size: 2048
[ 2.899511] dwc2 20980000.usb: Periodic TXFIFO 0 size: 0
[ 2.899523] dwc2 20980000.usb: Periodic TXFIFO 1 size: 256
[ 2.899536] dwc2 20980000.usb: Periodic TXFIFO 2 size: 256
[ 2.899547] dwc2 20980000.usb: Periodic TXFIFO 3 size: 256
[ 2.899559] dwc2 20980000.usb: Periodic TXFIFO 4 size: 256
[ 2.899572] dwc2 20980000.usb: Periodic TXFIFO 5 size: 768
[ 2.899584] dwc2 20980000.usb: Periodic TXFIFO 6 size: 768
[ 2.899595] dwc2 20980000.usb: Periodic TXFIFO 7 size: 768
[ 2.899607] dwc2 20980000.usb: Periodic TXFIFO 8 size: 768
[ 2.899619] dwc2 20980000.usb: Periodic TXFIFO 9 size: 0
[ 2.899630] dwc2 20980000.usb: Periodic TXFIFO10 size: 0
[ 2.899640] dwc2 20980000.usb: Periodic TXFIFO11 size: 0
[ 2.899652] dwc2 20980000.usb: Periodic TXFIFO12 size: 0
[ 2.899662] dwc2 20980000.usb: Periodic TXFIFO13 size: 0
[ 2.899674] dwc2 20980000.usb: Periodic TXFIFO14 size: 0
[ 2.899686] dwc2 20980000.usb: Periodic TXFIFO15 size: 0
[ 2.899720] dwc2 20980000.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
[ 2.918197] zero gadget: Gadget Zero, version: Cinco de Mayo 2008
[ 2.935109] zero gadget: zero ready
[ 2.949235] ------------[ cut here ]------------
[ 2.964357] WARNING: CPU: 0 PID: 6 at drivers/usb/dwc2/gadget.c:227
dwc2_hsotg_init_fifo+0xd0/0x1ac
[ 2.984089] insufficient fifo memory
[ 2.987649] CPU: 0 PID: 6 Comm: kworker/u2:0 Not tainted
4.7.0-rc7-next-20160719+ #29
[ 3.016940] Hardware name: BCM2835
[ 3.031145] Workqueue: deferwq deferred_probe_work_func
[ 3.047436] [<c010e480>] (unwind_backtrace) from [<c010b97c>]
(show_stack+0x20/0x24)
[ 3.066350] [<c010b97c>] (show_stack) from [<c035dc54>]
(dump_stack+0x20/0x28)
[ 3.084726] [<c035dc54>] (dump_stack) from [<c011c96c>] (__warn+0xe0/0x10c)
[ 3.102866] [<c011c96c>] (__warn) from [<c011c9e0>]
(warn_slowpath_fmt+0x48/0x50)
[ 3.121564] [<c011c9e0>] (warn_slowpath_fmt) from [<c0480cd0>]
(dwc2_hsotg_init_fifo+0xd0/0x1ac)
[ 3.141678] [<c0480cd0>] (dwc2_hsotg_init_fifo) from [<c0483f18>]
(dwc2_hsotg_udc_start+0x1e4/0x2b8)
[ 3.162222] [<c0483f18>] (dwc2_hsotg_udc_start) from [<c0490a40>]
(udc_bind_to_driver+0x60/0x108)
[ 3.182462] [<c0490a40>] (udc_bind_to_driver) from [<c04913d8>]
(usb_add_gadget_udc_release+0x144/0x244)
[ 3.203338] [<c04913d8>] (usb_add_gadget_udc_release) from [<c0491578>]
(usb_add_gadget_udc+0x1c/0x20)
[ 3.224024] [<c0491578>] (usb_add_gadget_udc) from [<c0484f90>]
(dwc2_gadget_init+0x460/0x4f8)
[ 3.244022] [<c0484f90>] (dwc2_gadget_init) from [<c0474698>]
(dwc2_driver_probe+0x360/0x4bc)
[ 3.264058] [<c0474698>] (dwc2_driver_probe) from [<c040236c>]
(platform_drv_probe+0x68/0xb0)
[ 3.284186] [<c040236c>] (platform_drv_probe) from [<c0400b2c>]
(driver_probe_device+0x180/0x42c)
[ 3.304727] [<c0400b2c>] (driver_probe_device) from [<c04010e0>]
(__device_attach_driver+0xc0/0x108)
[ 3.325628] [<c04010e0>] (__device_attach_driver) from [<c03ff1e8>]
(bus_for_each_drv+0x5c/0xa4)
[ 3.346260] [<c03ff1e8>] (bus_for_each_drv) from [<c04007ec>]
(__device_attach+0x9c/0x138)
[ 3.366460] [<c04007ec>] (__device_attach) from [<c0401164>]
(device_initial_probe+0x1c/0x20)
[ 3.386971] [<c0401164>] (device_initial_probe) from [<c03ff3fc>]
(bus_probe_device+0x38/0x90)
[ 3.407720] [<c03ff3fc>] (bus_probe_device) from [<c0400218>]
(deferred_probe_work_func+0x5c/0xa4)
[ 3.429054] [<c0400218>] (deferred_probe_work_func) from [<c0130e28>]
(process_one_work+0x1d0/0x37c)
[ 3.450717] [<c0130e28>] (process_one_work) from [<c01312a8>]
(worker_thread+0x294/0x414)
[ 3.471563] [<c01312a8>] (worker_thread) from [<c01363e4>]
(kthread+0xec/0x100)
[ 3.491662] [<c01363e4>] (kthread) from [<c01080f8>]
(ret_from_fork+0x14/0x3c)
[ 3.511770] ---[ end trace 00841c75e25fed43 ]---
[ 3.532627] dwc2 20980000.usb: bound driver zero
...
[ 4.092616] dwc2_hcd_init()
[ 4.108117] dwc2 20980000.usb: DWC OTG Controller
[ 4.125318] dwc2 20980000.usb: new USB bus registered, assigned bus number 1
[ 4.144876] dwc2 20980000.usb: irq 33, io mem 0x00000000
[ 4.164367] hub 1-0:1.0: USB hub found
[ 4.180746] hub 1-0:1.0: 1 port detected
Since there are no gadget fifo sizes specified the dwc2 drivers uses legacy
settings.
I think there are 2 causes for this warning:
1. The array behind DWC2_G_P_LEGACY_TX_FIFO_SIZE specify 8 values plus the
separate ep0 which makes in sum 9 EPs. So dwc2_hsotg_init_fifo tries to setup 9
EPs with legacy settings. But bcm2835 only support 8 EPs. I don't know of any
platform with less EPs.
2. The legacy settings for gadget are to high. They need to be adjusted to the
"smallest" platform.
Here is a patch which fixes the problem (USB devices enumerate and warning
disappear) for me:
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 9fae029..289ad3c 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -263,8 +263,8 @@ enum dwc2_lx_state {
* Gadget periodic tx fifo sizes as used by legacy driver
* EP0 is not included
*/
-#define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, \
- 768, 0, 0, 0, 0, 0, 0, 0}
+#define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 128, 128, 64, 64, 64, 64, \
+ 0, 0, 0, 0, 0, 0, 0, 0}
/* Gadget ep0 states */
enum dwc2_ep0_state {
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index af46adf..2d0bc2a 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3868,8 +3868,8 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
u32 p_tx_fifo[] = DWC2_G_P_LEGACY_TX_FIFO_SIZE;
/* Initialize to legacy fifo configuration values */
- hsotg->g_rx_fifo_sz = 2048;
- hsotg->g_np_g_tx_fifo_sz = 1024;
+ hsotg->g_rx_fifo_sz = 256;
+ hsotg->g_np_g_tx_fifo_sz = 128;
memcpy(&hsotg->g_tx_fifo_sz[1], p_tx_fifo, sizeof(p_tx_fifo));
/* Device tree specific probe */
dwc2_hsotg_of_probe(hsotg);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Bug] usb: dwc2: host function broken in OTG mode on bcm283x
2016-08-20 13:02 [Bug] usb: dwc2: host function broken in OTG mode on bcm283x Stefan Wahren
@ 2016-08-22 20:06 ` John Youn
2016-08-22 20:47 ` Stefan Wahren
0 siblings, 1 reply; 7+ messages in thread
From: John Youn @ 2016-08-22 20:06 UTC (permalink / raw)
To: linux-arm-kernel
On 8/20/2016 6:03 AM, Stefan Wahren wrote:
> Hi John,
>
> on bcm2835 in OTG mode the USB host function broken (USB devices don't
> enumerate) with the following settings:
>
> CONFIG_USB=y
> CONFIG_USB_OTG=y
> CONFIG_USB_DWC2=y
> CONFIG_USB_DWC2_DUAL_ROLE=y
> CONFIG_USB_PHY=y
> CONFIG_NOP_USB_XCEIV=y
> CONFIG_USB_GADGET=y
> CONFIG_USB_ZERO=y
>
> and DT don't provide any optional gadget fifo settings. During boot the kernel
> give a warning about insufficient fifo memory.
>
> Here is the relevant log with some debug messages:
>
> [ 2.373718] dwc2_lowlevel_hw_init()
> [ 2.389306] dwc2_set_all_params()
> [ 2.404927] dwc2_lowlevel_hw_enable()
> [ 2.420331] dwc2_get_dr_mode()
> [ 2.434894] dwc2_get_dr_mode: OF suggests 3
> [ 2.450592] dwc2_get_dr_mode: No PHY found
> [ 2.466161] dwc2_core_reset_and_force_dr_mode()
> [ 2.517779] mmc0: new SDHC card at address 1234
> [ 2.535005] mmcblk0: mmc0:1234 SA32G 29.3 GiB
> [ 2.553421] mmcblk0: p1 p2
> [ 2.619213] dwc2_get_hwparams()
> [ 2.633611] dwc2 20980000.usb: Core Release: 2.80a (snpsid=4f54280a)
> [ 2.633628] dwc2 20980000.usb: hwcfg1=00000000
> [ 2.633640] dwc2 20980000.usb: hwcfg2=228ddd50
> [ 2.633651] dwc2 20980000.usb: hwcfg3=0ff000e8
> [ 2.633663] dwc2 20980000.usb: hwcfg4=1ff00020
> [ 2.633674] dwc2 20980000.usb: grxfsiz=00001000
> [ 2.633688] dwc2 20980000.usb: gnptxfsiz=01001000
> [ 2.633698] dwc2 20980000.usb: hptxfsiz=02002000
> [ 2.633710] dwc2 20980000.usb: Forcing mode to device
> [ 2.679241] dwc2 20980000.usb: gnptxfsiz=01001000
> [ 2.729220] dwc2 20980000.usb: Detected values from hardware:
> [ 2.729237] dwc2 20980000.usb: op_mode=0
> [ 2.729249] dwc2 20980000.usb: arch=2
> [ 2.729259] dwc2 20980000.usb: dma_desc_enable=0
> [ 2.729271] dwc2 20980000.usb: power_optimized=0
> [ 2.729285] dwc2 20980000.usb: i2c_enable=0
> [ 2.729294] dwc2 20980000.usb: hs_phy_type=1
> [ 2.729305] dwc2 20980000.usb: fs_phy_type=1
> [ 2.729315] dwc2 20980000.usb: utmi_phy_data_width=0
> [ 2.729325] dwc2 20980000.usb: num_dev_ep=7
> [ 2.729335] dwc2 20980000.usb: num_dev_perio_in_ep=0
> [ 2.729345] dwc2 20980000.usb: host_channels=8
> [ 2.729356] dwc2 20980000.usb: max_transfer_size=524287
> [ 2.729367] dwc2 20980000.usb: max_packet_count=1023
> [ 2.729377] dwc2 20980000.usb: nperio_tx_q_depth=0x4
> [ 2.729387] dwc2 20980000.usb: host_perio_tx_q_depth=0x4
> [ 2.729398] dwc2 20980000.usb: dev_token_q_depth=0x8
> [ 2.729408] dwc2 20980000.usb: enable_dynamic_fifo=1
> [ 2.729418] dwc2 20980000.usb: en_multiple_tx_fifo=1
> [ 2.729429] dwc2 20980000.usb: total_fifo_size=4080
> [ 2.729439] dwc2 20980000.usb: host_rx_fifo_size=4096
> [ 2.729450] dwc2 20980000.usb: host_nperio_tx_fifo_size=256
> [ 2.729461] dwc2 20980000.usb: host_perio_tx_fifo_size=512
> [ 2.729469] dwc2 20980000.usb:
> [ 2.729474] dwc2_set_parameters()
> [ 2.743824] dwc2 20980000.usb: dwc2_set_parameters()
> [ 2.743848] dwc2 20980000.usb: Setting dma_desc_fs_enable to 0
> [ 2.743881] dwc2 20980000.usb: Setting external_id_pin_ctl to 0
> [ 2.743895] dwc2 20980000.usb: Setting hibernation to 0
> [ 2.743899] dwc2_force_dr_mode()
> [ 2.869216] dwc2_gadget_init()
> [ 2.883027] dwc2 20980000.usb: Specified GNPTXFDEP=1024 > 256
> [ 2.899480] dwc2 20980000.usb: NonPeriodic TXFIFO size: 256
> [ 2.899497] dwc2 20980000.usb: RXFIFO size: 2048
> [ 2.899511] dwc2 20980000.usb: Periodic TXFIFO 0 size: 0
> [ 2.899523] dwc2 20980000.usb: Periodic TXFIFO 1 size: 256
> [ 2.899536] dwc2 20980000.usb: Periodic TXFIFO 2 size: 256
> [ 2.899547] dwc2 20980000.usb: Periodic TXFIFO 3 size: 256
> [ 2.899559] dwc2 20980000.usb: Periodic TXFIFO 4 size: 256
> [ 2.899572] dwc2 20980000.usb: Periodic TXFIFO 5 size: 768
> [ 2.899584] dwc2 20980000.usb: Periodic TXFIFO 6 size: 768
> [ 2.899595] dwc2 20980000.usb: Periodic TXFIFO 7 size: 768
> [ 2.899607] dwc2 20980000.usb: Periodic TXFIFO 8 size: 768
> [ 2.899619] dwc2 20980000.usb: Periodic TXFIFO 9 size: 0
> [ 2.899630] dwc2 20980000.usb: Periodic TXFIFO10 size: 0
> [ 2.899640] dwc2 20980000.usb: Periodic TXFIFO11 size: 0
> [ 2.899652] dwc2 20980000.usb: Periodic TXFIFO12 size: 0
> [ 2.899662] dwc2 20980000.usb: Periodic TXFIFO13 size: 0
> [ 2.899674] dwc2 20980000.usb: Periodic TXFIFO14 size: 0
> [ 2.899686] dwc2 20980000.usb: Periodic TXFIFO15 size: 0
> [ 2.899720] dwc2 20980000.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
> [ 2.918197] zero gadget: Gadget Zero, version: Cinco de Mayo 2008
> [ 2.935109] zero gadget: zero ready
> [ 2.949235] ------------[ cut here ]------------
> [ 2.964357] WARNING: CPU: 0 PID: 6 at drivers/usb/dwc2/gadget.c:227
> dwc2_hsotg_init_fifo+0xd0/0x1ac
> [ 2.984089] insufficient fifo memory
> [ 2.987649] CPU: 0 PID: 6 Comm: kworker/u2:0 Not tainted
> 4.7.0-rc7-next-20160719+ #29
> [ 3.016940] Hardware name: BCM2835
> [ 3.031145] Workqueue: deferwq deferred_probe_work_func
> [ 3.047436] [<c010e480>] (unwind_backtrace) from [<c010b97c>]
> (show_stack+0x20/0x24)
> [ 3.066350] [<c010b97c>] (show_stack) from [<c035dc54>]
> (dump_stack+0x20/0x28)
> [ 3.084726] [<c035dc54>] (dump_stack) from [<c011c96c>] (__warn+0xe0/0x10c)
> [ 3.102866] [<c011c96c>] (__warn) from [<c011c9e0>]
> (warn_slowpath_fmt+0x48/0x50)
> [ 3.121564] [<c011c9e0>] (warn_slowpath_fmt) from [<c0480cd0>]
> (dwc2_hsotg_init_fifo+0xd0/0x1ac)
> [ 3.141678] [<c0480cd0>] (dwc2_hsotg_init_fifo) from [<c0483f18>]
> (dwc2_hsotg_udc_start+0x1e4/0x2b8)
> [ 3.162222] [<c0483f18>] (dwc2_hsotg_udc_start) from [<c0490a40>]
> (udc_bind_to_driver+0x60/0x108)
> [ 3.182462] [<c0490a40>] (udc_bind_to_driver) from [<c04913d8>]
> (usb_add_gadget_udc_release+0x144/0x244)
> [ 3.203338] [<c04913d8>] (usb_add_gadget_udc_release) from [<c0491578>]
> (usb_add_gadget_udc+0x1c/0x20)
> [ 3.224024] [<c0491578>] (usb_add_gadget_udc) from [<c0484f90>]
> (dwc2_gadget_init+0x460/0x4f8)
> [ 3.244022] [<c0484f90>] (dwc2_gadget_init) from [<c0474698>]
> (dwc2_driver_probe+0x360/0x4bc)
> [ 3.264058] [<c0474698>] (dwc2_driver_probe) from [<c040236c>]
> (platform_drv_probe+0x68/0xb0)
> [ 3.284186] [<c040236c>] (platform_drv_probe) from [<c0400b2c>]
> (driver_probe_device+0x180/0x42c)
> [ 3.304727] [<c0400b2c>] (driver_probe_device) from [<c04010e0>]
> (__device_attach_driver+0xc0/0x108)
> [ 3.325628] [<c04010e0>] (__device_attach_driver) from [<c03ff1e8>]
> (bus_for_each_drv+0x5c/0xa4)
> [ 3.346260] [<c03ff1e8>] (bus_for_each_drv) from [<c04007ec>]
> (__device_attach+0x9c/0x138)
> [ 3.366460] [<c04007ec>] (__device_attach) from [<c0401164>]
> (device_initial_probe+0x1c/0x20)
> [ 3.386971] [<c0401164>] (device_initial_probe) from [<c03ff3fc>]
> (bus_probe_device+0x38/0x90)
> [ 3.407720] [<c03ff3fc>] (bus_probe_device) from [<c0400218>]
> (deferred_probe_work_func+0x5c/0xa4)
> [ 3.429054] [<c0400218>] (deferred_probe_work_func) from [<c0130e28>]
> (process_one_work+0x1d0/0x37c)
> [ 3.450717] [<c0130e28>] (process_one_work) from [<c01312a8>]
> (worker_thread+0x294/0x414)
> [ 3.471563] [<c01312a8>] (worker_thread) from [<c01363e4>]
> (kthread+0xec/0x100)
> [ 3.491662] [<c01363e4>] (kthread) from [<c01080f8>]
> (ret_from_fork+0x14/0x3c)
> [ 3.511770] ---[ end trace 00841c75e25fed43 ]---
> [ 3.532627] dwc2 20980000.usb: bound driver zero
> ...
> [ 4.092616] dwc2_hcd_init()
> [ 4.108117] dwc2 20980000.usb: DWC OTG Controller
> [ 4.125318] dwc2 20980000.usb: new USB bus registered, assigned bus number 1
> [ 4.144876] dwc2 20980000.usb: irq 33, io mem 0x00000000
> [ 4.164367] hub 1-0:1.0: USB hub found
> [ 4.180746] hub 1-0:1.0: 1 port detected
>
> Since there are no gadget fifo sizes specified the dwc2 drivers uses legacy
> settings.
>
> I think there are 2 causes for this warning:
>
> 1. The array behind DWC2_G_P_LEGACY_TX_FIFO_SIZE specify 8 values plus the
> separate ep0 which makes in sum 9 EPs. So dwc2_hsotg_init_fifo tries to setup 9
> EPs with legacy settings. But bcm2835 only support 8 EPs. I don't know of any
> platform with less EPs.
>
> 2. The legacy settings for gadget are to high. They need to be adjusted to the
> "smallest" platform.
>
> Here is a patch which fixes the problem (USB devices enumerate and warning
> disappear) for me:
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 9fae029..289ad3c 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -263,8 +263,8 @@ enum dwc2_lx_state {
> * Gadget periodic tx fifo sizes as used by legacy driver
> * EP0 is not included
> */
> -#define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, \
> - 768, 0, 0, 0, 0, 0, 0, 0}
> +#define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 128, 128, 64, 64, 64, 64, \
> + 0, 0, 0, 0, 0, 0, 0, 0}
>
> /* Gadget ep0 states */
> enum dwc2_ep0_state {
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index af46adf..2d0bc2a 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3868,8 +3868,8 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> u32 p_tx_fifo[] = DWC2_G_P_LEGACY_TX_FIFO_SIZE;
>
> /* Initialize to legacy fifo configuration values */
> - hsotg->g_rx_fifo_sz = 2048;
> - hsotg->g_np_g_tx_fifo_sz = 1024;
> + hsotg->g_rx_fifo_sz = 256;
> + hsotg->g_np_g_tx_fifo_sz = 128;
> memcpy(&hsotg->g_tx_fifo_sz[1], p_tx_fifo, sizeof(p_tx_fifo));
> /* Device tree specific probe */
> dwc2_hsotg_of_probe(hsotg);
>
Hi Stefan,
Why doesn't DT work? I think all the properties are there to set
these.
I hesitate to change the legacy/default settings in case it breaks any
existing drivers that depend on them.
Regards,
John
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug] usb: dwc2: host function broken in OTG mode on bcm283x
2016-08-22 20:06 ` John Youn
@ 2016-08-22 20:47 ` Stefan Wahren
2016-08-22 22:08 ` John Youn
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Wahren @ 2016-08-22 20:47 UTC (permalink / raw)
To: linux-arm-kernel
Hi John,
> John Youn <John.Youn@synopsys.com> hat am 22. August 2016 um 22:06
> geschrieben:
>
>
> On 8/20/2016 6:03 AM, Stefan Wahren wrote:
> > Hi John,
> >
> >
>
> Hi Stefan,
>
> Why doesn't DT work? I think all the properties are there to set
> these.
it only works for the future releases, not for existing DT blobs. The DT is part
of the ABI.
Yes, i already send a patch to fix the DT [1], but it would be better to fix the
issue in deep.
>
> I hesitate to change the legacy/default settings in case it breaks any
> existing drivers that depend on them.
Platforms? I didn't expected that other drivers use these settings.
Another option would be to add the gadget fifo sizes to platform data, but it's
not a nice solution.
Regards
Stefan
[1] -
http://lists.infradead.org/pipermail/linux-rpi-kernel/2016-August/004225.html
>
> Regards,
> John
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug] usb: dwc2: host function broken in OTG mode on bcm283x
2016-08-22 20:47 ` Stefan Wahren
@ 2016-08-22 22:08 ` John Youn
2016-08-23 4:40 ` Stefan Wahren
0 siblings, 1 reply; 7+ messages in thread
From: John Youn @ 2016-08-22 22:08 UTC (permalink / raw)
To: linux-arm-kernel
On 8/22/2016 1:47 PM, Stefan Wahren wrote:
> Hi John,
>
>> John Youn <John.Youn@synopsys.com> hat am 22. August 2016 um 22:06
>> geschrieben:
>>
>>
>> On 8/20/2016 6:03 AM, Stefan Wahren wrote:
>>> Hi John,
>>>
>>>
>>
>> Hi Stefan,
>>
>> Why doesn't DT work? I think all the properties are there to set
>> these.
>
> it only works for the future releases, not for existing DT blobs. The DT is part
> of the ABI.
>
> Yes, i already send a patch to fix the DT [1], but it would be better to fix the
> issue in deep.
I don't know much about DT issues. So I'm not sure what the issue is
with this. The properties for the gadget fifo sizes already exist so
there's no change to the ABI, correct?
Also, the patch you linked doesn't seem to have settings for the FIFO
sizes.
>
>>
>> I hesitate to change the legacy/default settings in case it breaks any
>> existing drivers that depend on them.
>
> Platforms? I didn't expected that other drivers use these settings.
Yes, I mean platforms.
I believe these values were hard-coded since before the unified "dwc2"
existed. So for sure there are platforms using these settings and to
lower them will negatively impact those platforms. The DT properties
were introduced to override these.
I think it is best to override them in the broken platforms. Either
that or fix it so that the defaults are more intelligently determined
in a way that maximizes the fifo sizes and cannot fail, as opposed to
some other arbitrary values.
>
> Another option would be to add the gadget fifo sizes to platform data, but it's
> not a nice solution.
I agree, that wouldn't be good.
Regards,
John
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug] usb: dwc2: host function broken in OTG mode on bcm283x
2016-08-22 22:08 ` John Youn
@ 2016-08-23 4:40 ` Stefan Wahren
2016-08-23 18:37 ` John Youn
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Wahren @ 2016-08-23 4:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi John,
> John Youn <John.Youn@synopsys.com> hat am 23. August 2016 um 00:08
> geschrieben:
>
>
> On 8/22/2016 1:47 PM, Stefan Wahren wrote:
> > Hi John,
> >
> >> John Youn <John.Youn@synopsys.com> hat am 22. August 2016 um 22:06
> >> geschrieben:
> >>
> >>
> >> On 8/20/2016 6:03 AM, Stefan Wahren wrote:
> >>> Hi John,
> >>>
> >>>
> >>
> >> Hi Stefan,
> >>
> >> Why doesn't DT work? I think all the properties are there to set
> >> these.
> >
> > it only works for the future releases, not for existing DT blobs. The DT is
> > part
> > of the ABI.
> >
> > Yes, i already send a patch to fix the DT [1], but it would be better to fix
> > the
> > issue in deep.
>
> I don't know much about DT issues. So I'm not sure what the issue is
> with this.The properties for the gadget fifo sizes already exist so
> there's no change to the ABI, correct?
It's correct the ABI doesn't change, but it also means that newer kernel must
work with older DTB files. So the fix should be better in the driver and not in
the DT sources.
>
> Also, the patch you linked doesn't seem to have settings for the FIFO
> sizes.
It wouldn't make sense to provide gadget fifo sizes for a host-only setup. The
mentioned patch doesn't really fix this issue, that's why i mentioned that we
should fix this issue first.
>
> >
> >>
> >> I hesitate to change the legacy/default settings in case it breaks any
> >> existing drivers that depend on them.
> >
> > Platforms? I didn't expected that other drivers use these settings.
>
> Yes, I mean platforms.
>
> I believe these values were hard-coded since before the unified "dwc2"
> existed. So for sure there are platforms using these settings and to
> lower them will negatively impact those platforms. The DT properties
> were introduced to override these.
Unfortunately the gadget fifo sizes should have been marked as required for OTG
and gadget mode. But now it's too late. We only can improve DT binding here or
at least add a warning during probe.
>
> I think it is best to override them in the broken platforms. Either
> that or fix it so that the defaults are more intelligently determined
> in a way that maximizes the fifo sizes and cannot fail, as opposed to
> some other arbitrary values.
>
I don't have the knowledge for the second solution and i wouldn't prefer much
more detection intelligence.
> >
> > Another option would be to add the gadget fifo sizes to platform data, but
> > it's
> > not a nice solution.
>
> I agree, that wouldn't be good.
>
> Regards,
> John
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug] usb: dwc2: host function broken in OTG mode on bcm283x
2016-08-23 4:40 ` Stefan Wahren
@ 2016-08-23 18:37 ` John Youn
2016-08-23 18:55 ` Stefan Wahren
0 siblings, 1 reply; 7+ messages in thread
From: John Youn @ 2016-08-23 18:37 UTC (permalink / raw)
To: linux-arm-kernel
On 8/22/2016 9:41 PM, Stefan Wahren wrote:
> Hi John,
>
>> John Youn <John.Youn@synopsys.com> hat am 23. August 2016 um 00:08
>> geschrieben:
>>
>>
>> On 8/22/2016 1:47 PM, Stefan Wahren wrote:
>>> Hi John,
>>>
>>>> John Youn <John.Youn@synopsys.com> hat am 22. August 2016 um 22:06
>>>> geschrieben:
>>>>
>>>>
>>>> On 8/20/2016 6:03 AM, Stefan Wahren wrote:
>>>>> Hi John,
>>>>>
>>>>>
>>>>
>>>> Hi Stefan,
>>>>
>>>> Why doesn't DT work? I think all the properties are there to set
>>>> these.
>>>
>>> it only works for the future releases, not for existing DT blobs. The DT is
>>> part
>>> of the ABI.
>>>
>>> Yes, i already send a patch to fix the DT [1], but it would be better to fix
>>> the
>>> issue in deep.
>>
>> I don't know much about DT issues. So I'm not sure what the issue is
>> with this.The properties for the gadget fifo sizes already exist so
>> there's no change to the ABI, correct?
>
> It's correct the ABI doesn't change, but it also means that newer kernel must
> work with older DTB files. So the fix should be better in the driver and not in
> the DT sources.
>
>>
>> Also, the patch you linked doesn't seem to have settings for the FIFO
>> sizes.
>
> It wouldn't make sense to provide gadget fifo sizes for a host-only setup. The
> mentioned patch doesn't really fix this issue, that's why i mentioned that we
> should fix this issue first.
>
>>
>>>
>>>>
>>>> I hesitate to change the legacy/default settings in case it breaks any
>>>> existing drivers that depend on them.
>>>
>>> Platforms? I didn't expected that other drivers use these settings.
>>
>> Yes, I mean platforms.
>>
>> I believe these values were hard-coded since before the unified "dwc2"
>> existed. So for sure there are platforms using these settings and to
>> lower them will negatively impact those platforms. The DT properties
>> were introduced to override these.
>
> Unfortunately the gadget fifo sizes should have been marked as required for OTG
> and gadget mode. But now it's too late. We only can improve DT binding here or
> at least add a warning during probe.
>
>>
>> I think it is best to override them in the broken platforms. Either
>> that or fix it so that the defaults are more intelligently determined
>> in a way that maximizes the fifo sizes and cannot fail, as opposed to
>> some other arbitrary values.
>>
>
> I don't have the knowledge for the second solution and i wouldn't prefer much
> more detection intelligence.
>
How about we fall back to smaller defaults if the original values
fail? And don't configure more than the amount of endpoints.
That should take care of most of the cases. If an irregular
configuration shows up, require it to use the DT properties.
Regards,
John
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug] usb: dwc2: host function broken in OTG mode on bcm283x
2016-08-23 18:37 ` John Youn
@ 2016-08-23 18:55 ` Stefan Wahren
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Wahren @ 2016-08-23 18:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi John,
> John Youn <John.Youn@synopsys.com> hat am 23. August 2016 um 20:37
> geschrieben:
>
>
>
> How about we fall back to smaller defaults if the original values
> fail? And don't configure more than the amount of endpoints.
this sounds good. I would prefer to calculate the estimated gadget fifo size
before configuring the hardware, so we can bail out without breaking the host
function.
Regards
Stefan
>
> That should take care of most of the cases. If an irregular
> configuration shows up, require it to use the DT properties.
>
> Regards,
> John
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-08-23 18:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-20 13:02 [Bug] usb: dwc2: host function broken in OTG mode on bcm283x Stefan Wahren
2016-08-22 20:06 ` John Youn
2016-08-22 20:47 ` Stefan Wahren
2016-08-22 22:08 ` John Youn
2016-08-23 4:40 ` Stefan Wahren
2016-08-23 18:37 ` John Youn
2016-08-23 18:55 ` Stefan Wahren
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.