RE: Crash in usb_f_mass_storage
> -Original Message- > From: Paul Jones [mailto:p.jo...@teclyn.com] > Sent: Friday, October 16, 2015 3:40 AM > To: Kaukab, Yousaf > Cc: Alan Stern; Felipe Balbi; Linux USB Mailing List > Subject: Re: Crash in usb_f_mass_storage > > > On 15 Oct 2015, at 04:12, Kaukab, Yousaf wrote: > > >> -Original Message- > >> From: Paul Jones [mailto:p.jo...@teclyn.com] > >> Sent: Thursday, October 15, 2015 12:30 AM > >> To: Alan Stern > >> Cc: Kaukab, Yousaf; Felipe Balbi; Linux USB Mailing List > >> Subject: Re: Crash in usb_f_mass_storage > >> > >> > >> On 14 Oct 2015, at 15:37, Alan Stern wrote: > >> > >>> On Wed, 14 Oct 2015, Paul Jones wrote: > >>> > >>>> On 12 Oct 2015, at 14:16, Felipe Balbi wrote: > >>>> > >>>>> > >>>>> Hi, > >>>>> > >>>>> Paul Jones writes: > >>>>>> On 10 Oct 2015, at 16:32, Paul Jones wrote: > >>>>>> > >>>>>>> I came across the following kernel message on the latest 4.3-rc4 > >>>>>>> whilst > >> performance testing on a USB3380 device connected to a Mac (10.9.5): > >>>>>>> > >>>>>>> [ 51.613838] WARNING: CPU: 2 PID: 0 at > >> drivers/usb/gadget/function/f_mass_storage.c:346 > >> fsg_setup+0x12a/0x170 > >> [usb_f_mass_storage]() > >>>>>>> [ 51.613838] Modules linked in: usb_f_mass_storage libcomposite > >> configfs drbg ansi_cprng ctr ccm arc4 snd_hda_codec_hdmi iwlmvm i915 > >> mac80211 snd_hda_codec_realtek snd_hda_codec_generic hid_generic > >> intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp > >> iwlwifi kvm_intel cfg80211 kvm drm_kms_helper drm snd_hda_intel > >> snd_hda_codec btusb btrtl crct10dif_pclmul crc32_pclmul btbcm > >> snd_hda_core ghash_clmulni_intel btintel bluetooth snd_hwdep e1000e > >> aesni_intel > >> aes_x86_64 lrw snd_pcm gf128mul glue_helper ablk_helper cryptd > >> serio_raw alx mei_me lpc_ich usbhid mei snd_timer snd net2280 > >> i2c_algo_bit ptp udc_core fb_sys_fops mdio syscopyarea pps_core > >> sysfillrect soundcore sysimgblt i2c_hid hid video dw_dmac sdhci_acpi > >> shpchp i2c_designware_platform dw_dmac_core spi_pxa2xx_platform sdhci > >> 8250_dw i2c_designware_core acpi_pad lp mac_hid parport > >>>>>>> [ 51.613858] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW > >> 4.3.0-rc4+ #4 > >>>>>>> [ 51.613859] Hardware name: Gigabyte Technology Co., Ltd. H97N- > >> WIFI/H97N-WIFI, BIOS F7 04/21/2015 > >>>>>>> [ 51.613860] a03e9e10 88021eb03d70 81393f5d > >> > >>>>>>> [ 51.613861] 88021eb03da8 81075856 880037be4400 > >> 88020b3023c8 > >>>>>>> [ 51.613862] 880037be4400 ffa1 > > >> 88021eb03db8 > >>>>>>> [ 51.613863] Call Trace: > >>>>>>> [ 51.613864][] dump_stack+0x44/0x57 > >>>>>>> [ 51.613867] [] > warn_slowpath_common+0x86/0xc0 > >>>>>>> [ 51.613868] [] warn_slowpath_null+0x1a/0x20 > >>>>>>> [ 51.613870] [] fsg_setup+0x12a/0x170 > >> [usb_f_mass_storage] > >>>>>>> [ 51.613872] [] composite_setup+0x173/0x16b0 > >> [libcomposite] > >>>>>>> [ 51.613873] [] ? ktime_get+0x3a/0x90 > >>>>>>> [ 51.613874] [] ? lapic_next_deadline+0x29/0x30 > >>>>>>> [ 51.613875] [] ? ktime_get+0x3a/0x90 > >>>>>>> [ 51.613877] [] net2280_irq+0xba2/0x10db > >> [net2280] > >>>>>>> [ 51.613879] [] > >> handle_irq_event_percpu+0x39/0x180 > >>>>>>> [ 51.613880] [] handle_irq_event+0x45/0x70 > >>>>>>> [ 51.613881] [] handle_edge_irq+0x99/0x150 > >>>>>>> [ 51.613883] [] handle_irq+0x1d/0x30 > >>>>>>> [ 51.613883] [] do_IRQ+0x4d/0xd0 > >>>>>>> [ 51.613885] [] common_interrupt+0x87/0x87 > >>>>>>> [ 51.613885][] ? > >> cpuidle_enter_state+0xb8/0x220 > >>>>>>> [ 51.613888] [] cpuidle_enter+0x17/0x20 > >>>>>>> [ 51.613889] [] call_cpuidle+0x32/0x60 >
RE: Crash in usb_f_mass_storage
> -Original Message- > From: Paul Jones [mailto:p.jo...@teclyn.com] > Sent: Thursday, October 15, 2015 12:30 AM > To: Alan Stern > Cc: Kaukab, Yousaf; Felipe Balbi; Linux USB Mailing List > Subject: Re: Crash in usb_f_mass_storage > > > On 14 Oct 2015, at 15:37, Alan Stern wrote: > > > On Wed, 14 Oct 2015, Paul Jones wrote: > > > >> On 12 Oct 2015, at 14:16, Felipe Balbi wrote: > >> > >>> > >>> Hi, > >>> > >>> Paul Jones writes: > >>>> On 10 Oct 2015, at 16:32, Paul Jones wrote: > >>>> > >>>>> I came across the following kernel message on the latest 4.3-rc4 whilst > performance testing on a USB3380 device connected to a Mac (10.9.5): > >>>>> > >>>>> [ 51.613838] WARNING: CPU: 2 PID: 0 at > drivers/usb/gadget/function/f_mass_storage.c:346 fsg_setup+0x12a/0x170 > [usb_f_mass_storage]() > >>>>> [ 51.613838] Modules linked in: usb_f_mass_storage libcomposite > configfs drbg ansi_cprng ctr ccm arc4 snd_hda_codec_hdmi iwlmvm i915 > mac80211 snd_hda_codec_realtek snd_hda_codec_generic hid_generic > intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp iwlwifi > kvm_intel cfg80211 kvm drm_kms_helper drm snd_hda_intel snd_hda_codec > btusb btrtl crct10dif_pclmul crc32_pclmul btbcm snd_hda_core > ghash_clmulni_intel btintel bluetooth snd_hwdep e1000e aesni_intel > aes_x86_64 lrw snd_pcm gf128mul glue_helper ablk_helper cryptd serio_raw > alx mei_me lpc_ich usbhid mei snd_timer snd net2280 i2c_algo_bit ptp > udc_core fb_sys_fops mdio syscopyarea pps_core sysfillrect soundcore > sysimgblt i2c_hid hid video dw_dmac sdhci_acpi shpchp > i2c_designware_platform dw_dmac_core spi_pxa2xx_platform sdhci 8250_dw > i2c_designware_core acpi_pad lp mac_hid parport > >>>>> [ 51.613858] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW > 4.3.0-rc4+ #4 > >>>>> [ 51.613859] Hardware name: Gigabyte Technology Co., Ltd. H97N- > WIFI/H97N-WIFI, BIOS F7 04/21/2015 > >>>>> [ 51.613860] a03e9e10 88021eb03d70 81393f5d > > >>>>> [ 51.613861] 88021eb03da8 81075856 880037be4400 > 88020b3023c8 > >>>>> [ 51.613862] 880037be4400 ffa1 > 88021eb03db8 > >>>>> [ 51.613863] Call Trace: > >>>>> [ 51.613864][] dump_stack+0x44/0x57 > >>>>> [ 51.613867] [] warn_slowpath_common+0x86/0xc0 > >>>>> [ 51.613868] [] warn_slowpath_null+0x1a/0x20 > >>>>> [ 51.613870] [] fsg_setup+0x12a/0x170 > [usb_f_mass_storage] > >>>>> [ 51.613872] [] composite_setup+0x173/0x16b0 > [libcomposite] > >>>>> [ 51.613873] [] ? ktime_get+0x3a/0x90 > >>>>> [ 51.613874] [] ? lapic_next_deadline+0x29/0x30 > >>>>> [ 51.613875] [] ? ktime_get+0x3a/0x90 > >>>>> [ 51.613877] [] net2280_irq+0xba2/0x10db > [net2280] > >>>>> [ 51.613879] [] > handle_irq_event_percpu+0x39/0x180 > >>>>> [ 51.613880] [] handle_irq_event+0x45/0x70 > >>>>> [ 51.613881] [] handle_edge_irq+0x99/0x150 > >>>>> [ 51.613883] [] handle_irq+0x1d/0x30 > >>>>> [ 51.613883] [] do_IRQ+0x4d/0xd0 > >>>>> [ 51.613885] [] common_interrupt+0x87/0x87 > >>>>> [ 51.613885][] ? > cpuidle_enter_state+0xb8/0x220 > >>>>> [ 51.613888] [] cpuidle_enter+0x17/0x20 > >>>>> [ 51.613889] [] call_cpuidle+0x32/0x60 > >>>>> [ 51.613890] [] ? cpuidle_select+0x13/0x20 > >>>>> [ 51.613891] [] cpu_startup_entry+0x21c/0x2e0 > >>>>> [ 51.613891] [] start_secondary+0x104/0x130 > >>>>> [ 51.613892] ---[ end trace bda1c37ade46c57d ] > >>>>> > >>>>> I can also reliable reproduce this by connecting the USB3380 to a USB > port on the same Linux machine. > >>>>> In that case I also see an error: > >>>>> net2280 : net2280_enable: error=-22 net2280_enable is returning EINVAL in more than one places. Can you check which one is it? We need better error reporting from this driver. > >>>>> > >>>>> Perhaps unrelated, but there is also a message: > >>>>> configfs-gadget gadget: common->fsg is NULL in fsg_setup at 511 > >>>> The same crash happens in 4.2 as well but not in 4.1. > >>> > >>> care to run a git bisect a
RE: [PATCH v1 Resend] usb: dwc2: gadget: fix a memory use-after-free bug
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Thursday, October 1, 2015 7:21 PM > To: Kaukab, Yousaf > Cc: ba...@ti.com; linux-usb@vger.kernel.org; john.y...@synopsys.com; > l...@rock-chips.com; he...@sntech.de; c...@rock-chips.com; > h...@rock-chips.com; > y...@rock-chips.com; gaura...@google.com; albe...@google.com; wulf@rock- > chips.com; jwer...@chromium.org; jeffy.c...@rock-chips.com; Herrero, > Gregory; huang...@rock-chips.com; rockchip-disc...@chromium.org; > gre...@linuxfoundation.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH v1 Resend] usb: dwc2: gadget: fix a memory use-after-free > bug > > On Thu, Oct 01, 2015 at 12:01:48PM +, Kaukab, Yousaf wrote: > > > From: Mian Yousaf Kaukab > > > Date: Tue, Sep 29, 2015 at 12:25 PM > > > Subject: [PATCH v1 Resend] usb: dwc2: gadget: fix a memory > > > use-after-free bug > > > To: linux-usb@vger.kernel.org, ba...@ti.com, john.y...@synopsys.com, > > > l...@rock-chips.com > > > Cc: he...@sntech.de, c...@rock-chips.com, h...@rock-chips.com, yk@rock- > > > chips.com, gaura...@google.com, albe...@google.com, > > > w...@rock-chips.com, jwer...@chromium.org, > > > jeffy.c...@rock-chips.com, gregory.herr...@intel.com, > > > huang...@rock-chips.com, rockchip- disc...@chromium.org, > > > gre...@linuxfoundation.org, linux- ker...@vger.kernel.org > > > > > > > > > From: Yunzhi Li > > > > > > When dwc2_hsotg_handle_unaligned_buf_complete() hs_req->req.buf > > > already destroyed, in dwc2_hsotg_unmap_dma(), it touches > > > hs_req->req.dma again, so > > > dwc2_hsotg_unmap_dma() should be called before > > > dwc2_hsotg_handle_unaligned_buf_complete(). Otherwise, it will cause > > > a bad_page BUG, when allocate this memory page next time. > > > > > > This bug led to the following crash: > > > > > > BUG: Bad page state in process swapper/0 pfn:2bdbc > > > [ 26.820440] page:eed76780 count:0 mapcount:0 mapping: (null) > index:0x0 > > > [ 26.854710] page flags: 0x200(arch_1) > > > [ 26.885836] page dumped because: PAGE_FLAGS_CHECK_AT_PREP flag > set > > > [ 26.919179] bad because of flags: > > > [ 26.948917] page flags: 0x200(arch_1) > > > [ 26.979100] Modules linked in: > > > [ 27.008401] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W3.14.0 #17 > > > [ 27.041816] [] (unwind_backtrace) from [] > > > (show_stack+0x20/0x24) > > > [ 27.076108] [] (show_stack) from [] > > > (dump_stack+0x70/0x8c) > > > [ 27.110246] [] (dump_stack) from [] > > > (bad_page+0xfc/0x12c) > > > [ 27.143958] [] (bad_page) from [] > > > (get_page_from_freelist+0x3e4/0x50c) > > > [ 27.179298] [] (get_page_from_freelist) from [] > > > (__alloc_pages_nodemask) > > > [ 27.216296] [] (__alloc_pages_nodemask) from [] > > > (__get_free_pages+0x20/) > > > [ 27.252326] [] (__get_free_pages) from [] > > > (kmalloc_order_trace+0x34/0xa) > > > [ 27.288295] [] (kmalloc_order_trace) from [] > > > (__kmalloc+0x40/0x1ac) > > > [ 27.323751] [] (__kmalloc) from [] > > > (dwc2_hsotg_ep_queue.isra.12+0x7c/0x1) > > > [ 27.359937] [] (dwc2_hsotg_ep_queue.isra.12) from > > > [] (dwc2_hsotg_ep_queue) > > > [ 27.397478] [] (dwc2_hsotg_ep_queue_lock) from > > > [] (rx_submit+0xfc/0x164) > > > [ 27.433619] [] (rx_submit) from [] > > > (rx_complete+0x22c/0x230) > > > [ 27.468872] [] (rx_complete) from [] > > > (dwc2_hsotg_complete_request+0xfc/0) > > > [ 27.506240] [] (dwc2_hsotg_complete_request) from > > > [] (dwc2_hsotg_handle_o) > > > [ 27.545401] [] (dwc2_hsotg_handle_outdone) from > > > [] (dwc2_hsotg_epint+0x2c) > > > [ 27.583689] [] (dwc2_hsotg_epint) from [] > > > (dwc2_hsotg_irq+0x1dc/0x4ac) > > > [ 27.621041] [] (dwc2_hsotg_irq) from [] > > > (handle_irq_event_percpu+0x70/0x) > > > [ 27.659066] [] (handle_irq_event_percpu) from > > > [] (handle_irq_event+0x4c) > > > [ 27.697322] [] (handle_irq_event) from [] > > > (handle_fasteoi_irq+0xc8/0x11) > > > [ 27.735451] [] (handle_fasteoi_irq) from [] > > > (generic_handle_irq+0x30/0x) > > > [ 27.773918] [] (generic_handle_irq) from [] > > > (__handle_domain_irq+0x84/0) > > > [ 27.812018] [] (__handle_domain_irq) from [] > > > (gic_handle_irq+0x48/0x6c) > > > [ 27.849695] [] (gic_handle_irq) from [] > > > (__irq_svc+0x40/0x50) > > > [ 27.886907] Exception stack(0xc0d01ee0 to 0xc0d01f28) > > > > > > Acked-by: John Youn > > > Tested-by: Heiko Stuebner > > > Tested-by: Jeffy Chen > > > Signed-off-by: Yunzhi Li > > > > Hi Felipe, > > This patch has been hanging around for a while now. Can you please apply > this? > > https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/nex > t&id=1c2e3377a933af9102f6c57c414c378a52d4e70d Thanks! > > -- > balbi BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1 Resend] usb: dwc2: gadget: fix a memory use-after-free bug
> From: Mian Yousaf Kaukab > Date: Tue, Sep 29, 2015 at 12:25 PM > Subject: [PATCH v1 Resend] usb: dwc2: gadget: fix a memory use-after-free bug > To: linux-usb@vger.kernel.org, ba...@ti.com, john.y...@synopsys.com, > l...@rock-chips.com > Cc: he...@sntech.de, c...@rock-chips.com, h...@rock-chips.com, yk@rock- > chips.com, gaura...@google.com, albe...@google.com, w...@rock-chips.com, > jwer...@chromium.org, jeffy.c...@rock-chips.com, > gregory.herr...@intel.com, huang...@rock-chips.com, rockchip- > disc...@chromium.org, gre...@linuxfoundation.org, linux- > ker...@vger.kernel.org > > > From: Yunzhi Li > > When dwc2_hsotg_handle_unaligned_buf_complete() hs_req->req.buf already > destroyed, in dwc2_hsotg_unmap_dma(), it touches hs_req->req.dma again, so > dwc2_hsotg_unmap_dma() should be called before > dwc2_hsotg_handle_unaligned_buf_complete(). Otherwise, it will cause a > bad_page BUG, when allocate this memory page next time. > > This bug led to the following crash: > > BUG: Bad page state in process swapper/0 pfn:2bdbc > [ 26.820440] page:eed76780 count:0 mapcount:0 mapping: (null) index:0x0 > [ 26.854710] page flags: 0x200(arch_1) > [ 26.885836] page dumped because: PAGE_FLAGS_CHECK_AT_PREP flag set > [ 26.919179] bad because of flags: > [ 26.948917] page flags: 0x200(arch_1) > [ 26.979100] Modules linked in: > [ 27.008401] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W3.14.0 #17 > [ 27.041816] [] (unwind_backtrace) from [] > (show_stack+0x20/0x24) > [ 27.076108] [] (show_stack) from [] > (dump_stack+0x70/0x8c) > [ 27.110246] [] (dump_stack) from [] > (bad_page+0xfc/0x12c) > [ 27.143958] [] (bad_page) from [] > (get_page_from_freelist+0x3e4/0x50c) > [ 27.179298] [] (get_page_from_freelist) from [] > (__alloc_pages_nodemask) > [ 27.216296] [] (__alloc_pages_nodemask) from [] > (__get_free_pages+0x20/) > [ 27.252326] [] (__get_free_pages) from [] > (kmalloc_order_trace+0x34/0xa) > [ 27.288295] [] (kmalloc_order_trace) from [] > (__kmalloc+0x40/0x1ac) > [ 27.323751] [] (__kmalloc) from [] > (dwc2_hsotg_ep_queue.isra.12+0x7c/0x1) > [ 27.359937] [] (dwc2_hsotg_ep_queue.isra.12) from > [] (dwc2_hsotg_ep_queue) > [ 27.397478] [] (dwc2_hsotg_ep_queue_lock) from > [] (rx_submit+0xfc/0x164) > [ 27.433619] [] (rx_submit) from [] > (rx_complete+0x22c/0x230) > [ 27.468872] [] (rx_complete) from [] > (dwc2_hsotg_complete_request+0xfc/0) > [ 27.506240] [] (dwc2_hsotg_complete_request) from > [] (dwc2_hsotg_handle_o) > [ 27.545401] [] (dwc2_hsotg_handle_outdone) from > [] (dwc2_hsotg_epint+0x2c) > [ 27.583689] [] (dwc2_hsotg_epint) from [] > (dwc2_hsotg_irq+0x1dc/0x4ac) > [ 27.621041] [] (dwc2_hsotg_irq) from [] > (handle_irq_event_percpu+0x70/0x) > [ 27.659066] [] (handle_irq_event_percpu) from > [] (handle_irq_event+0x4c) > [ 27.697322] [] (handle_irq_event) from [] > (handle_fasteoi_irq+0xc8/0x11) > [ 27.735451] [] (handle_fasteoi_irq) from [] > (generic_handle_irq+0x30/0x) > [ 27.773918] [] (generic_handle_irq) from [] > (__handle_domain_irq+0x84/0) > [ 27.812018] [] (__handle_domain_irq) from [] > (gic_handle_irq+0x48/0x6c) > [ 27.849695] [] (gic_handle_irq) from [] > (__irq_svc+0x40/0x50) > [ 27.886907] Exception stack(0xc0d01ee0 to 0xc0d01f28) > > Acked-by: John Youn > Tested-by: Heiko Stuebner > Tested-by: Jeffy Chen > Signed-off-by: Yunzhi Li Hi Felipe, This patch has been hanging around for a while now. Can you please apply this? BR, Yousaf
RE: [PATCH v3 20/32] usb: dwc2: force dr_mode in case of configuration mismatch
> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Wednesday, September 30, 2015 6:26 PM
> To: Sergei Shtylyov
> Cc: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com;
> john.y...@synopsys.com; Herrero, Gregory; he...@sntech.de;
> diand...@chromium.org; r.bald...@samsung.com;
> dingu...@opensource.altera.com; zhangfei@linaro.org;
> david.a.co...@linux.intel.com
> Subject: Re: [PATCH v3 20/32] usb: dwc2: force dr_mode in case of
> configuration mismatch
>
> On Tue, Sep 29, 2015 at 09:58:56PM +0300, Sergei Shtylyov wrote:
> > On 09/29/2015 01:08 PM, Mian Yousaf Kaukab wrote:
> >
> > >If dual role configuration is not selected, check and force dr_mode
> > >based on the selected configuration.
> > >
> > >Signed-off-by: Mian Yousaf Kaukab
> > >Tested-by: Robert Baldyga
> > >Tested-by: Dinh Nguyen
> > >Tested-by: John Youn
> > >Acked-by: John Youn
> > >---
> > > drivers/usb/dwc2/platform.c | 11 +++
> > > 1 file changed, 11 insertions(+)
> > >
> > >diff --git a/drivers/usb/dwc2/platform.c
> > >b/drivers/usb/dwc2/platform.c index ed7a78e..0945e21 100644
> > >--- a/drivers/usb/dwc2/platform.c
> > >+++ b/drivers/usb/dwc2/platform.c
> > >@@ -368,6 +368,17 @@ static int dwc2_driver_probe(struct
> platform_device *dev)
> > > (unsigned long)res->start, hsotg->regs);
> > >
> > > hsotg->dr_mode = usb_get_dr_mode(&dev->dev);
> > >+ if (IS_ENABLED(CONFIG_USB_DWC2_HOST) &&
> > >+ hsotg->dr_mode != USB_DR_MODE_HOST) {
> >
> >Please indent with 2 tabs, so that it's easier on the eyes, not
> > blending with the following...
>
> fixed both myself
>
Thank you for taking care of this!
> --
> balbi
BR,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 00/32] usb: dwc2: various bug fixes
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Wednesday, September 30, 2015 6:24 PM > To: Kaukab, Yousaf > Cc: linux-usb@vger.kernel.org; ba...@ti.com; john.y...@synopsys.com; > Herrero, Gregory; he...@sntech.de; diand...@chromium.org; > r.bald...@samsung.com; dingu...@opensource.altera.com; > zhangfei@linaro.org; sergei.shtyl...@cogentembedded.com; > david.a.co...@linux.intel.com > Subject: Re: [PATCH v3 00/32] usb: dwc2: various bug fixes > > On Tue, Sep 29, 2015 at 12:07:58PM +0200, Mian Yousaf Kaukab wrote: > > Hi, > > This series consists of various bug fixes for both host and gadget > > sides. All patches are verified on dwc2 v3.0a with dedicated fifos. > > It would be good to get some Tested-bys for other platforms. > > > > It is based on testing/next branch in Felipe's git. > > > > Thank you, > > > > Best regards, > > Yousaf > > > > History: > > v3: > > - Fix comment from Sergei Shtylyov > > - Rebase to latest testing/next > > are you sure ? Because I already had up to "intialize op_state for peripheral > only" applied. I'll skip all patches until that one and try to apply. It was a mistake. I should have skipped the patches in this revision that are already applied. Sorry about that. > > -- > balbi BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 20/32] usb: dwc2: force dr_mode in case of configuration mismatch
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Monday, September 28, 2015 10:43 PM > To: Kaukab, Yousaf > Cc: linux-usb@vger.kernel.org; ba...@ti.com; john.y...@synopsys.com; > Herrero, Gregory; he...@sntech.de; diand...@chromium.org; > r.bald...@samsung.com; dingu...@opensource.altera.com; > zhangfei@linaro.org; sergei.shtyl...@cogentembedded.com; > david.a.co...@linux.intel.com > Subject: Re: [PATCH v2 20/32] usb: dwc2: force dr_mode in case of > configuration mismatch > > On Tue, Sep 22, 2015 at 03:16:56PM +0200, Mian Yousaf Kaukab wrote: > > If dual role configuration is not selected, check and force dr_mode > > based on the selected configuration. > > > > Signed-off-by: Mian Yousaf Kaukab > > Tested-by: Robert Baldyga > > this failed to apply. Please rebase on testing/next Ok I will rebase the whole series. BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free bug
> -Original Message- > From: Kaukab, Yousaf > Sent: Friday, September 25, 2015 10:52 AM > To: 'John Youn'; 'Yunzhi Li'; 'Felipe Balbi' > Cc: 'he...@sntech.de'; 'c...@rock-chips.com'; 'h...@rock-chips.com'; 'yk@rock- > chips.com'; 'gaura...@google.com'; 'albe...@google.com'; 'wulf@rock- > chips.com'; 'jwer...@chromium.org'; 'jeffy.c...@rock-chips.com'; Herrero, > Gregory; 'huang...@rock-chips.com'; 'rockchip-disc...@chromium.org'; 'Greg > Kroah-Hartman'; 'linux-usb@vger.kernel.org'; 'linux-ker...@vger.kernel.org' > Subject: RE: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free bug > > > -Original Message- > > From: Kaukab, Yousaf > > Sent: Tuesday, September 22, 2015 2:24 PM > > To: John Youn; Yunzhi Li; Felipe Balbi > > Cc: he...@sntech.de; c...@rock-chips.com; h...@rock-chips.com; yk@rock- > > chips.com; gaura...@google.com; albe...@google.com; > > w...@rock-chips.com; jwer...@chromium.org; jeffy.c...@rock-chips.com; > > Herrero, Gregory; huang...@rock-chips.com; > > rockchip-disc...@chromium.org; Greg Kroah- Hartman; > > linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org > > Subject: RE: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free > > bug > > > > > -Original Message- > > > From: John Youn [mailto:john.y...@synopsys.com] > > > Sent: Thursday, June 11, 2015 4:16 AM > > > To: Yunzhi Li; john.y...@synopsys.com > > > Cc: he...@sntech.de; c...@rock-chips.com; h...@rock-chips.com; yk@rock- > > > chips.com; gaura...@google.com; albe...@google.com; > > > w...@rock-chips.com; jwer...@chromium.org; > > > jeffy.c...@rock-chips.com; Herrero, Gregory; Kaukab, Yousaf; > > > huang...@rock-chips.com; rockchip-disc...@chromium.org; Greg > > > Kroah-Hartman; linux-usb@vger.kernel.org; > > > linux-ker...@vger.kernel.org > > > Subject: Re: [PATCH v1] usb: dwc2: gadget: fix a memory > > > use-after-free bug > > > > > > On 5/28/2015 10:22 PM, Yunzhi Li wrote: > > > > When s3c_hsotg_handle_unaligned_buf_complete() hs_req->req.buf > > > > already destroyed, in s3c_hsotg_unmap_dma(), it touches > > > > hs_req->req.dma again, so s3c_hsotg_unmap_dma() should be called > > > > before s3c_hsotg_handle_unaligned_buf_complete(). Otherwise, it > > > > will cause a bad_page BUG, when allocate this memory page next time. > > > > > > > > This bug led to the following crash: > > > > > > > > BUG: Bad page state in process swapper/0 pfn:2bdbc > > > > [ 26.820440] page:eed76780 count:0 mapcount:0 mapping: (null) > > index:0x0 > > > > [ 26.854710] page flags: 0x200(arch_1) > > > > [ 26.885836] page dumped because: PAGE_FLAGS_CHECK_AT_PREP flag > > set > > > > [ 26.919179] bad because of flags: > > > > [ 26.948917] page flags: 0x200(arch_1) > > > > [ 26.979100] Modules linked in: > > > > [ 27.008401] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W3.14.0 #17 > > > > [ 27.041816] [] (unwind_backtrace) from [] > > > (show_stack+0x20/0x24) > > > > [ 27.076108] [] (show_stack) from [] > > > (dump_stack+0x70/0x8c) > > > > [ 27.110246] [] (dump_stack) from [] > > > (bad_page+0xfc/0x12c) > > > > [ 27.143958] [] (bad_page) from [] > > > (get_page_from_freelist+0x3e4/0x50c) > > > > [ 27.179298] [] (get_page_from_freelist) from [] > > > (__alloc_pages_nodemask) > > > > [ 27.216296] [] (__alloc_pages_nodemask) from > [] > > > (__get_free_pages+0x20/) > > > > [ 27.252326] [] (__get_free_pages) from [] > > > (kmalloc_order_trace+0x34/0xa) > > > > [ 27.288295] [] (kmalloc_order_trace) from [] > > > (__kmalloc+0x40/0x1ac) > > > > [ 27.323751] [] (__kmalloc) from [] > > > (s3c_hsotg_ep_queue.isra.12+0x7c/0x1) > > > > [ 27.359937] [] (s3c_hsotg_ep_queue.isra.12) from > > [] > > > (s3c_hsotg_ep_queue) > > > > [ 27.397478] [] (s3c_hsotg_ep_queue_lock) from > [] > > > (rx_submit+0xfc/0x164) > > > > [ 27.433619] [] (rx_submit) from [] > > > (rx_complete+0x22c/0x230) > > > > [ 27.468872] [] (rx_complete) from [] > > > (s3c_hsotg_complete_request+0xfc/0) > > > > [ 27.506240] [] (s3c_hsotg_complete_request) from > > > [] (s3c_hsotg_handle_o) > > > > [
RE: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free bug
> -Original Message- > From: Kaukab, Yousaf > Sent: Tuesday, September 22, 2015 2:24 PM > To: John Youn; Yunzhi Li; Felipe Balbi > Cc: he...@sntech.de; c...@rock-chips.com; h...@rock-chips.com; yk@rock- > chips.com; gaura...@google.com; albe...@google.com; w...@rock-chips.com; > jwer...@chromium.org; jeffy.c...@rock-chips.com; Herrero, Gregory; > huang...@rock-chips.com; rockchip-disc...@chromium.org; Greg Kroah- > Hartman; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: RE: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free bug > > > -Original Message- > > From: John Youn [mailto:john.y...@synopsys.com] > > Sent: Thursday, June 11, 2015 4:16 AM > > To: Yunzhi Li; john.y...@synopsys.com > > Cc: he...@sntech.de; c...@rock-chips.com; h...@rock-chips.com; yk@rock- > > chips.com; gaura...@google.com; albe...@google.com; > > w...@rock-chips.com; jwer...@chromium.org; jeffy.c...@rock-chips.com; > > Herrero, Gregory; Kaukab, Yousaf; huang...@rock-chips.com; > > rockchip-disc...@chromium.org; Greg Kroah-Hartman; > > linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org > > Subject: Re: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free > > bug > > > > On 5/28/2015 10:22 PM, Yunzhi Li wrote: > > > When s3c_hsotg_handle_unaligned_buf_complete() hs_req->req.buf > > > already destroyed, in s3c_hsotg_unmap_dma(), it touches > > > hs_req->req.dma again, so s3c_hsotg_unmap_dma() should be called > > > before s3c_hsotg_handle_unaligned_buf_complete(). Otherwise, it will > > > cause a bad_page BUG, when allocate this memory page next time. > > > > > > This bug led to the following crash: > > > > > > BUG: Bad page state in process swapper/0 pfn:2bdbc > > > [ 26.820440] page:eed76780 count:0 mapcount:0 mapping: (null) > index:0x0 > > > [ 26.854710] page flags: 0x200(arch_1) > > > [ 26.885836] page dumped because: PAGE_FLAGS_CHECK_AT_PREP flag > set > > > [ 26.919179] bad because of flags: > > > [ 26.948917] page flags: 0x200(arch_1) > > > [ 26.979100] Modules linked in: > > > [ 27.008401] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W3.14.0 #17 > > > [ 27.041816] [] (unwind_backtrace) from [] > > (show_stack+0x20/0x24) > > > [ 27.076108] [] (show_stack) from [] > > (dump_stack+0x70/0x8c) > > > [ 27.110246] [] (dump_stack) from [] > > (bad_page+0xfc/0x12c) > > > [ 27.143958] [] (bad_page) from [] > > (get_page_from_freelist+0x3e4/0x50c) > > > [ 27.179298] [] (get_page_from_freelist) from [] > > (__alloc_pages_nodemask) > > > [ 27.216296] [] (__alloc_pages_nodemask) from [] > > (__get_free_pages+0x20/) > > > [ 27.252326] [] (__get_free_pages) from [] > > (kmalloc_order_trace+0x34/0xa) > > > [ 27.288295] [] (kmalloc_order_trace) from [] > > (__kmalloc+0x40/0x1ac) > > > [ 27.323751] [] (__kmalloc) from [] > > (s3c_hsotg_ep_queue.isra.12+0x7c/0x1) > > > [ 27.359937] [] (s3c_hsotg_ep_queue.isra.12) from > [] > > (s3c_hsotg_ep_queue) > > > [ 27.397478] [] (s3c_hsotg_ep_queue_lock) from [] > > (rx_submit+0xfc/0x164) > > > [ 27.433619] [] (rx_submit) from [] > > (rx_complete+0x22c/0x230) > > > [ 27.468872] [] (rx_complete) from [] > > (s3c_hsotg_complete_request+0xfc/0) > > > [ 27.506240] [] (s3c_hsotg_complete_request) from > > [] (s3c_hsotg_handle_o) > > > [ 27.545401] [] (s3c_hsotg_handle_outdone) from > [] > > (s3c_hsotg_epint+0x2c) > > > [ 27.583689] [] (s3c_hsotg_epint) from [] > > (s3c_hsotg_irq+0x1dc/0x4ac) > > > [ 27.621041] [] (s3c_hsotg_irq) from [] > > (handle_irq_event_percpu+0x70/0x) > > > [ 27.659066] [] (handle_irq_event_percpu) from [] > > (handle_irq_event+0x4c) > > > [ 27.697322] [] (handle_irq_event) from [] > > (handle_fasteoi_irq+0xc8/0x11) > > > [ 27.735451] [] (handle_fasteoi_irq) from [] > > (generic_handle_irq+0x30/0x) > > > [ 27.773918] [] (generic_handle_irq) from [] > > (__handle_domain_irq+0x84/0) > > > [ 27.812018] [] (__handle_domain_irq) from [] > > (gic_handle_irq+0x48/0x6c) > > > [ 27.849695] [] (gic_handle_irq) from [] > > (__irq_svc+0x40/0x50) > > > [ 27.886907] Exception stack(0xc0d01ee0 to 0xc0d01f28) > > > > > > Signed-off-by: Yunzhi Li > > > > > > --- > > > > > > drivers/usb/dwc2/gadget.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff
RE: [PATCH v2 14/32] usb: dwc2: host: wait 3ms for controller stabilization
> -Original Message- > From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com] > Sent: Tuesday, September 22, 2015 9:42 PM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > john.y...@synopsys.com; Herrero, Gregory > Cc: he...@sntech.de; diand...@chromium.org; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org; > david.a.co...@linux.intel.com > Subject: Re: [PATCH v2 14/32] usb: dwc2: host: wait 3ms for controller > stabilization > > On 09/22/2015 04:16 PM, Mian Yousaf Kaukab wrote: > > > From: Gregory Herrero > > > > Some high speed mass storage devices fail to enumerate with following > > error: > > > > Cannot enable port %i. Maybe the USB cable is bad? > > > > This happens only when the device is plugged while the controller is > > in hibernation state. After exiting hibernation, the controller > > detects the device as a low speed device and fail to enumerate it. > > > > Problem occurs only if HPRT0.PWR bit is programmed in a too short > > delay after exiting hibernation. Dumping hprt register in > > _dwc2_hcd_resume() directly after dwc2_exit_hibernation() shows that > > HPRT0.LNSTS (D+/D- level) becomes valid approximately 2ms after > > exiting hibernation. > > > Since dwc2_exit_hibernation() is called from atomic context, move the > > delay out of this function. > > Hm, I don't see Gregory moving anything in this patch... > Perhaps keep is more appropriate word. I'll replace move with keep if I end up doing another revision of this series. BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free bug
> -Original Message- > From: John Youn [mailto:john.y...@synopsys.com] > Sent: Thursday, June 11, 2015 4:16 AM > To: Yunzhi Li; john.y...@synopsys.com > Cc: he...@sntech.de; c...@rock-chips.com; h...@rock-chips.com; yk@rock- > chips.com; gaura...@google.com; albe...@google.com; w...@rock-chips.com; > jwer...@chromium.org; jeffy.c...@rock-chips.com; Herrero, Gregory; > Kaukab, Yousaf; huang...@rock-chips.com; rockchip-disc...@chromium.org; > Greg Kroah-Hartman; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free bug > > On 5/28/2015 10:22 PM, Yunzhi Li wrote: > > When s3c_hsotg_handle_unaligned_buf_complete() hs_req->req.buf already > > destroyed, in s3c_hsotg_unmap_dma(), it touches hs_req->req.dma again, > > so s3c_hsotg_unmap_dma() should be called before > > s3c_hsotg_handle_unaligned_buf_complete(). Otherwise, it will cause a > > bad_page BUG, when allocate this memory page next time. > > > > This bug led to the following crash: > > > > BUG: Bad page state in process swapper/0 pfn:2bdbc > > [ 26.820440] page:eed76780 count:0 mapcount:0 mapping: (null) index:0x0 > > [ 26.854710] page flags: 0x200(arch_1) > > [ 26.885836] page dumped because: PAGE_FLAGS_CHECK_AT_PREP flag set > > [ 26.919179] bad because of flags: > > [ 26.948917] page flags: 0x200(arch_1) > > [ 26.979100] Modules linked in: > > [ 27.008401] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W3.14.0 #17 > > [ 27.041816] [] (unwind_backtrace) from [] > (show_stack+0x20/0x24) > > [ 27.076108] [] (show_stack) from [] > (dump_stack+0x70/0x8c) > > [ 27.110246] [] (dump_stack) from [] > (bad_page+0xfc/0x12c) > > [ 27.143958] [] (bad_page) from [] > (get_page_from_freelist+0x3e4/0x50c) > > [ 27.179298] [] (get_page_from_freelist) from [] > (__alloc_pages_nodemask) > > [ 27.216296] [] (__alloc_pages_nodemask) from [] > (__get_free_pages+0x20/) > > [ 27.252326] [] (__get_free_pages) from [] > (kmalloc_order_trace+0x34/0xa) > > [ 27.288295] [] (kmalloc_order_trace) from [] > (__kmalloc+0x40/0x1ac) > > [ 27.323751] [] (__kmalloc) from [] > (s3c_hsotg_ep_queue.isra.12+0x7c/0x1) > > [ 27.359937] [] (s3c_hsotg_ep_queue.isra.12) from [] > (s3c_hsotg_ep_queue) > > [ 27.397478] [] (s3c_hsotg_ep_queue_lock) from [] > (rx_submit+0xfc/0x164) > > [ 27.433619] [] (rx_submit) from [] > (rx_complete+0x22c/0x230) > > [ 27.468872] [] (rx_complete) from [] > (s3c_hsotg_complete_request+0xfc/0) > > [ 27.506240] [] (s3c_hsotg_complete_request) from > [] (s3c_hsotg_handle_o) > > [ 27.545401] [] (s3c_hsotg_handle_outdone) from [] > (s3c_hsotg_epint+0x2c) > > [ 27.583689] [] (s3c_hsotg_epint) from [] > (s3c_hsotg_irq+0x1dc/0x4ac) > > [ 27.621041] [] (s3c_hsotg_irq) from [] > (handle_irq_event_percpu+0x70/0x) > > [ 27.659066] [] (handle_irq_event_percpu) from [] > (handle_irq_event+0x4c) > > [ 27.697322] [] (handle_irq_event) from [] > (handle_fasteoi_irq+0xc8/0x11) > > [ 27.735451] [] (handle_fasteoi_irq) from [] > (generic_handle_irq+0x30/0x) > > [ 27.773918] [] (generic_handle_irq) from [] > (__handle_domain_irq+0x84/0) > > [ 27.812018] [] (__handle_domain_irq) from [] > (gic_handle_irq+0x48/0x6c) > > [ 27.849695] [] (gic_handle_irq) from [] > (__irq_svc+0x40/0x50) > > [ 27.886907] Exception stack(0xc0d01ee0 to 0xc0d01f28) > > > > Signed-off-by: Yunzhi Li > > > > --- > > > > drivers/usb/dwc2/gadget.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > > index 6a30887..8070602 100644 > > --- a/drivers/usb/dwc2/gadget.c > > +++ b/drivers/usb/dwc2/gadget.c > > @@ -1389,14 +1389,14 @@ static void s3c_hsotg_complete_request(struct > dwc2_hsotg *hsotg, > > if (hs_req->req.status == -EINPROGRESS) > > hs_req->req.status = result; > > > > + if (using_dma(hsotg)) > > + s3c_hsotg_unmap_dma(hsotg, hs_ep, hs_req); > > + > > s3c_hsotg_handle_unaligned_buf_complete(hsotg, hs_ep, > hs_req); > > > > hs_ep->req = NULL; > > list_del_init(&hs_req->queue); > > > > - if (using_dma(hsotg)) > > - s3c_hsotg_unmap_dma(hsotg, hs_ep, hs_req); > > - > > /* > > * call the complete request with the locks off, just in case the > > * request tries to queue more work for this endpoint. > > > > > Acked-by: John Youn > Hi Felipe, This patch is still missing in testing/next. Can you just take this one or would you like it to be send again? > > John BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1 02/32] usb: dwc2: host: create a function to handle port_resume
> -Original Message-
> From: diand...@google.com [mailto:diand...@google.com] On Behalf Of
> Doug Anderson
> Sent: Thursday, September 17, 2015 9:42 PM
> To: Kaukab, Yousaf
> Cc: linux-usb@vger.kernel.org; Felipe Balbi; John Youn; Herrero, Gregory;
> Heiko
> Stübner; r.bald...@samsung.com; Dinh Nguyen; Zhangfei Gao;
> sergei.shtyl...@cogentembedded.com; david.a.co...@linux.intel.com
> Subject: Re: [PATCH v1 02/32] usb: dwc2: host: create a function to handle
> port_resume
>
> Hi,
>
> On Wed, Sep 9, 2015 at 3:20 AM, Mian Yousaf Kaukab
> wrote:
> > From: Gregory Herrero
> >
> > port resume sequence may be used in different places. Create a
> > function to handle it. Moreover, make hprt0 read-modify-write atomic.
> >
> > Signed-off-by: Gregory Herrero
> > Signed-off-by: Mian Yousaf Kaukab
> > Tested-by: Robert Baldyga
> > ---
> > drivers/usb/dwc2/hcd.c | 30 ++
> > 1 file changed, 22 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index
> > 007a3d5..0cef770 100644
> > --- a/drivers/usb/dwc2/hcd.c
> > +++ b/drivers/usb/dwc2/hcd.c
> > @@ -1484,6 +1484,27 @@ static void dwc2_port_suspend(struct dwc2_hsotg
> *hsotg, u16 windex)
> > }
> > }
> >
> > +/* Must NOT be called with interrupt disabled or spinlock held */
> > +static void dwc2_port_resume(struct dwc2_hsotg *hsotg) {
> > + unsigned long flags;
> > + u32 hprt0;
> > +
> > + spin_lock_irqsave(&hsotg->lock, flags);
> > + hprt0 = dwc2_read_hprt0(hsotg);
> > + hprt0 |= HPRT0_RES;
> > + hprt0 &= ~HPRT0_SUSP;
> > + writel(hprt0, hsotg->regs + HPRT0);
>
> Just from a completely "what changed" point of view, you have made a non-
> obvious behavior change here. Previously the code only masked out
> HPRT0_SUSP in the 2nd write, not the first. Was that a bug in the old code,
> or
> does it just not matter?
It doesn't matter. I will add this to the change log anyway.
HPRT0_SUSP is "read, write-set, and self-clear (R_WS_SC)" Application
can only set it and writing 0 has no effect. Hardware should clear it
when we set HPRT0_RES. So it doesn't matter if it is set in the first
write. But since we are clearing it, it's better to clear it for both
writes. So that there is no chance hardware can see this as another
assertion of this bit.
>
> > + spin_unlock_irqrestore(&hsotg->lock, flags);
> > +
> > + msleep(USB_RESUME_TIMEOUT);
> > +
> > + spin_lock_irqsave(&hsotg->lock, flags);
> > + hprt0 &= ~HPRT0_RES;
> > + writel(hprt0, hsotg->regs + HPRT0);
>
> Just curious: since you released and re-grabbed the lock, shouldn't you
> re-read
> HPRT0?
Agree we should. I will update the patch.
>
> Also: one thing that was added in my local tree here was to make things
> parallel to dwc2_port_suspend() and update "hsotg->lx_state"
> here. I'm not sure if it was added it because there as a real issue or if it
> just
> seemed better...
>
> > + spin_unlock_irqrestore(&hsotg->lock, flags); }
> > +
> > /* Handles hub class-specific requests */ static int
> > dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq,
> > u16 wvalue, u16 windex, char *buf, u16
> > wlength) @@ -1532,14 +1553,7 @@ static int dwc2_hcd_hub_control(struct
> dwc2_hsotg *hsotg, u16 typereq,
> > writel(0, hsotg->regs + PCGCTL);
> > usleep_range(2, 4);
>
> In my local tree we already had a dwc2_port_resume() function and it included
> the two lines above. I presume you didn't include them in your patch because
> future callers of dwc2_port_resume() don't want those lines. That's fine,
> but I
> figured I'd mention it...
dwc2_exit_hibernation() is taking care of it for the other caller. It's better
to
move this to dwc2_port_resume() to keep it balanced with dwc2_port_suspend().
I will update " usb: dwc2: host: enter hibernation during bus suspend" to add
the
core_params->hibernation check to this new part as well.
>
> >
> > - hprt0 = dwc2_read_hprt0(hsotg);
> > - hprt0 |= HPRT0_RES;
> > - writel(hprt0, hsotg->regs + HPRT0);
> > - hprt0 &= ~HPRT0_SUSP;
> > - msleep(USB_RESUME_TIMEOUT);
> > -
> > - hprt0 &= ~HPRT0_RES;
> > - writel(hprt0, hsotg->regs + HPRT0);
> > + dwc2_port_resume(hsotg);
> > break;
> >
> > case USB_PORT_FEAT_POWER:
>
> -Doug
BR,
Yousaf
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
RE: [PATCH v1 27/32] usb: dwc2: gadget: kill ep0 requests before reinitializing core
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: Friday, September 18, 2015 5:03 AM
> To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com;
> john.y...@synopsys.com
> Cc: Herrero, Gregory; he...@sntech.de; diand...@chromium.org;
> r.bald...@samsung.com; dingu...@opensource.altera.com;
> zhangfei@linaro.org; sergei.shtyl...@cogentembedded.com;
> david.a.co...@linux.intel.com
> Subject: Re: [PATCH v1 27/32] usb: dwc2: gadget: kill ep0 requests before
> reinitializing core
>
> On 9/9/2015 3:21 AM, Mian Yousaf Kaukab wrote:
> > Make sure there are no requests pending on ep0 before reinitializing
> > core. Otherwise, dwc2_hsotg_enqueue_setup will fail afterwards.
> >
> > Signed-off-by: Mian Yousaf Kaukab
> > Tested-by: Robert Baldyga
> > ---
> > drivers/usb/dwc2/gadget.c | 8 +++-
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> > index c7da6b7..a6a1a6a 100644
> > --- a/drivers/usb/dwc2/gadget.c
> > +++ b/drivers/usb/dwc2/gadget.c
> > @@ -2287,6 +2287,9 @@ void dwc2_hsotg_core_init_disconnected(struct
> > dwc2_hsotg *hsotg, {
> > u32 val;
> >
> > + /* Kill any ep0 requests as controller will be reinitialized */
> > + kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
> > +
>
> I am seeing this hang when I start off as a B-device, then plug in an A-cable
> with no device attached on the other end. Then unplug it.
>
> I haven't verified but it is probably due to the
> dwc2_hsotg_complete_request() (called from kill_all_requests) expecting lock
> to be held. But this is not locked when called from
> dwc2_conn_id_status_change().
You are spot on. dwc2_conn_id_status_change() is the only place where
dwc2_hsotg_core_init_disconnected() is called without locks. I will add
following
change to " usb: dwc2: gadget: kill ep0 requests before reinitializing core"
patch.
@@ -1386,7 +1387,9 @@ static void dwc2_conn_id_status_change(struct work_struct
*work)
hsotg->op_state = OTG_STATE_B_PERIPHERAL;
dwc2_core_init(hsotg, false, -1);
dwc2_enable_global_interrupts(hsotg);
+ spin_lock(&hsotg->lock);
dwc2_hsotg_core_init_disconnected(hsotg, false);
+ spin_unlock(&hsotg->lock);
dwc2_hsotg_core_connect(hsotg);
>
> John
BR,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1] media: uvcvideo: handle urb completion in a work queue
> -Original Message- > From: Alan Stern [mailto:st...@rowland.harvard.edu] > Sent: Wednesday, September 9, 2015 6:30 PM > To: Laurent Pinchart > Cc: Hans de Goede; Kaukab, Yousaf; linux-me...@vger.kernel.org; > mche...@osg.samsung.com; linux-usb@vger.kernel.org > Subject: Re: [PATCH v1] media: uvcvideo: handle urb completion in a work > queue > > On Wed, 9 Sep 2015, Laurent Pinchart wrote: > > > > > Instead of fixing the issue in the uvcvideo driver, would it then > > > > make more sense to fix it in the remaining hcd drivers ? > > > > > > Unfortunately that's not so easy. It involves some subtle changes > > > related to the way isochronous endpoints are handled. I wouldn't > > > know what to change in any of the HCDs, except the ones that I maintain. > > > > I'm not saying it would be easy, but I'm wondering whether it makes > > change to move individual USB device drivers to work queues when the > > long term goal is to use tasklets for URB completion anyway. > > I'm not sure that this is a long-term goal for every HCD. For instance, there > probably isn't much incentive to convert a driver if its host controllers can > only > run at low speed or full speed. > I can convert this patch to use tasklets instead and only schedule the tasklet if urb->complete is called in interrupt context. This way, hcd using tasklet or not, uvc driver behavior will be almost same. Only difference is that local interrupts will still be enabled when tasklet scheduled from uvc driver is executing the completion function. This patch can be dropped once all hcd's start using tasklets for the completion callback (if that ever happens). BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 00/32] usb: dwc2: various bug fixes
> -Original Message- > From: Robert Baldyga [mailto:r.bald...@samsung.com] > Sent: Monday, August 31, 2015 4:46 PM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > john.y...@synopsys.com > Cc: Herrero, Gregory; he...@sntech.de; diand...@chromium.org; > dingu...@opensource.altera.com; zhangfei@linaro.org > Subject: Re: [PATCH 00/32] usb: dwc2: various bug fixes > > On 08/28/2015 12:27 PM, Mian Yousaf Kaukab wrote: > > Hi, > > This series consists of various bug fixes for both host and gadget > > sides. All patches are verified on dwc2 v3.0a with dedicated fifos. > > It would be good to get some Tested-bys for other platforms. > > > > It is based on testing/next branch in Felipe's git and depends on [1]. > > > > Thank you, > > > > Best regards, > > Yousaf > > > > Works good on dwc2 v2.81a, dedicated fifos. > > [for gadget part] > Tested-by: Robert Baldyga Thanks! BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 05/32] usb: dwc2: host: update hcd and lx_state during start/stop callbacks
> -Original Message- > From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com] > Sent: Friday, August 28, 2015 11:50 PM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > john.y...@synopsys.com > Cc: Herrero, Gregory; he...@sntech.de; diand...@chromium.org; > r.bald...@samsung.com; dingu...@opensource.altera.com; > zhangfei@linaro.org > Subject: Re: [PATCH 05/32] usb: dwc2: host: update hcd and lx_state during > start/stop callbacks > > On 08/28/2015 01:27 PM, Mian Yousaf Kaukab wrote: > > > From: Gregory Herrero > > > During hcd initialization, hcd and lx states must be resetted to the > > Reset. And what is "lx"? s/lx/lx_state I will update both words and the description. > > > working state since controller is powered at this stage. > > > > Same logic applied for stop callback. > > > > Signed-off-by: Gregory Herrero > > Signed-off-by: Mian Yousaf Kaukab > > [...] > > MBR, Sergei > BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3] usb: dwc2: host: allocate qh before atomic enqueue
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: Wednesday, June 10, 2015 7:33 PM
> To: Kaukab, Yousaf; John Youn; linux-usb@vger.kernel.org; ba...@ti.com
> Cc: Herrero, Gregory; he...@sntech.de; Holmberg, Hans
> Subject: Re: [PATCH 1/3] usb: dwc2: host: allocate qh before atomic enqueue
>
> On 6/10/2015 6:27 AM, Kaukab, Yousaf wrote:
> >> -Original Message-
> >> From: John Youn [mailto:john.y...@synopsys.com]
> >> Sent: Wednesday, June 10, 2015 1:06 AM
> >> To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com;
> >> john.y...@synopsys.com
> >> Cc: Herrero, Gregory; he...@sntech.de; Holmberg, Hans
> >> Subject: Re: [PATCH 1/3] usb: dwc2: host: allocate qh before atomic
> >> enqueue
> >>
> >> On 6/5/2015 7:23 AM, Mian Yousaf Kaukab wrote:
> >>> To avoid sleep while atomic bugs, allocate qh before calling
> >>> dwc2_hcd_urb_enqueue. qh pointer can be used directly now instead of
> >>> passing ep->hcpriv as double pointer.
> >>>
> >>> Signed-off-by: Mian Yousaf Kaukab
> >>> ---
> >>> drivers/usb/dwc2/hcd.c | 31 ++
> >>> drivers/usb/dwc2/hcd.h | 5 -
> >>> drivers/usb/dwc2/hcd_queue.c | 45
> >>>
> >>> 3 files changed, 43 insertions(+), 38 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index
> >>> b10377c..80bce71 100644
> >>> --- a/drivers/usb/dwc2/hcd.c
> >>> +++ b/drivers/usb/dwc2/hcd.c
> >>> @@ -359,7 +359,7 @@ void dwc2_hcd_stop(struct dwc2_hsotg *hsotg)
> >>>
> >>> /* Caller must hold driver lock */
> >>> static int dwc2_hcd_urb_enqueue(struct dwc2_hsotg *hsotg,
> >>> - struct
> >> dwc2_hcd_urb *urb, void **ep_handle,
> >>> + struct
> >> dwc2_hcd_urb *urb, struct dwc2_qh *qh,
> >>> gfp_t mem_flags)
> >>> {
> >>> struct dwc2_qtd *qtd;
> >>> @@ -391,8 +391,7 @@ static int dwc2_hcd_urb_enqueue(struct
> >>> dwc2_hsotg
> >> *hsotg,
> >>> return -ENOMEM;
> >>>
> >>> dwc2_hcd_qtd_init(qtd, urb);
> >>> - retval = dwc2_hcd_qtd_add(hsotg, qtd, (struct dwc2_qh
> >> **)ep_handle,
> >>> - mem_flags);
> >>> + retval = dwc2_hcd_qtd_add(hsotg, qtd, qh);
> >>> if (retval) {
> >>> dev_err(hsotg->dev,
> >>> "DWC OTG HCD URB Enqueue
> >> failed adding QTD. Error status %d\n", @@
> >>> -2445,6 +2444,8 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd
> >>> *hcd,
> >> struct urb *urb,
> >>> u32 tflags = 0;
> >>> void *buf;
> >>> unsigned long flags;
> >>> + struct dwc2_qh *qh;
> >>> + bool qh_allocated = false;
> >>>
> >>> if (dbg_urb(urb)) {
> >>> dev_vdbg(hsotg->dev, "DWC OTG HCD URB
> >> Enqueue\n"); @@ -2523,13
> >>> +2524,24 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd,
> >>> +struct
> >> urb *urb,
> >>>
> >> urb->iso_frame_desc[i].length);
> >>>
> >>> urb->hcpriv = dwc2_urb;
> >>> + qh = (struct dwc2_qh *) ep->hcpriv;
> >>> + /* Create QH for the endpoint if it doesn't exist */
> >>> + if (!qh) {
> >>> + qh = dwc2_hcd_qh_create(hsotg, dwc2_urb,
> >> mem_flags);
> >>> + if (!qh) {
> >>> + retval = -ENOMEM;
> >>> + goto fail0;
> >>> + }
> >>> + ep->hcpriv = qh;
> >>> + qh_allocated = true;
> >>> + }
> >>>
> >>> spin_lock_irqsave(&hsotg->lock, flags);
> >>> retval = usb_hcd_link_urb_to_ep(hcd, urb);
> >>> if (retval)
> >>> goto fail1;
> >>>
> >>> - retval = dwc2_hcd_urb_enqueue(hsotg, dwc2_urb, &ep->hcpriv,
> >> mem_flags);
> >>> + retval = dwc2_hcd_urb_enqueue(hsotg, dwc2_urb, qh,
> >> mem_flags);
> >>> if (retval)
> >>> goto fail2;
> >>>
> >>> @@ -2549,6 +2561,17 @@ fail2:
> >>> fail1:
> >
RE: [PATCH 1/3] usb: dwc2: host: allocate qh before atomic enqueue
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: Wednesday, June 10, 2015 1:06 AM
> To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com;
> john.y...@synopsys.com
> Cc: Herrero, Gregory; he...@sntech.de; Holmberg, Hans
> Subject: Re: [PATCH 1/3] usb: dwc2: host: allocate qh before atomic enqueue
>
> On 6/5/2015 7:23 AM, Mian Yousaf Kaukab wrote:
> > To avoid sleep while atomic bugs, allocate qh before calling
> > dwc2_hcd_urb_enqueue. qh pointer can be used directly now instead of
> > passing ep->hcpriv as double pointer.
> >
> > Signed-off-by: Mian Yousaf Kaukab
> > ---
> > drivers/usb/dwc2/hcd.c | 31 ++
> > drivers/usb/dwc2/hcd.h | 5 -
> > drivers/usb/dwc2/hcd_queue.c | 45
> >
> > 3 files changed, 43 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index
> > b10377c..80bce71 100644
> > --- a/drivers/usb/dwc2/hcd.c
> > +++ b/drivers/usb/dwc2/hcd.c
> > @@ -359,7 +359,7 @@ void dwc2_hcd_stop(struct dwc2_hsotg *hsotg)
> >
> > /* Caller must hold driver lock */
> > static int dwc2_hcd_urb_enqueue(struct dwc2_hsotg *hsotg,
> > - struct
> dwc2_hcd_urb *urb, void **ep_handle,
> > + struct
> dwc2_hcd_urb *urb, struct dwc2_qh *qh,
> > gfp_t mem_flags)
> > {
> > struct dwc2_qtd *qtd;
> > @@ -391,8 +391,7 @@ static int dwc2_hcd_urb_enqueue(struct dwc2_hsotg
> *hsotg,
> > return -ENOMEM;
> >
> > dwc2_hcd_qtd_init(qtd, urb);
> > - retval = dwc2_hcd_qtd_add(hsotg, qtd, (struct dwc2_qh
> **)ep_handle,
> > - mem_flags);
> > + retval = dwc2_hcd_qtd_add(hsotg, qtd, qh);
> > if (retval) {
> > dev_err(hsotg->dev,
> > "DWC OTG HCD URB Enqueue
> failed adding QTD. Error status %d\n", @@
> > -2445,6 +2444,8 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd,
> struct urb *urb,
> > u32 tflags = 0;
> > void *buf;
> > unsigned long flags;
> > + struct dwc2_qh *qh;
> > + bool qh_allocated = false;
> >
> > if (dbg_urb(urb)) {
> > dev_vdbg(hsotg->dev, "DWC OTG HCD URB
> Enqueue\n"); @@ -2523,13
> > +2524,24 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct
> urb *urb,
> >
>urb->iso_frame_desc[i].length);
> >
> > urb->hcpriv = dwc2_urb;
> > + qh = (struct dwc2_qh *) ep->hcpriv;
> > + /* Create QH for the endpoint if it doesn't exist */
> > + if (!qh) {
> > + qh = dwc2_hcd_qh_create(hsotg, dwc2_urb,
> mem_flags);
> > + if (!qh) {
> > + retval = -ENOMEM;
> > + goto fail0;
> > + }
> > + ep->hcpriv = qh;
> > + qh_allocated = true;
> > + }
> >
> > spin_lock_irqsave(&hsotg->lock, flags);
> > retval = usb_hcd_link_urb_to_ep(hcd, urb);
> > if (retval)
> > goto fail1;
> >
> > - retval = dwc2_hcd_urb_enqueue(hsotg, dwc2_urb, &ep->hcpriv,
> mem_flags);
> > + retval = dwc2_hcd_urb_enqueue(hsotg, dwc2_urb, qh,
> mem_flags);
> > if (retval)
> > goto fail2;
> >
> > @@ -2549,6 +2561,17 @@ fail2:
> > fail1:
> > spin_unlock_irqrestore(&hsotg->lock, flags);
> > urb->hcpriv = NULL;
> > + if (qh_allocated) {
> > + struct dwc2_qtd *qtd2, *qtd2_tmp;
> > +
> > + ep->hcpriv = NULL;
> > + dwc2_hcd_qh_unlink(hsotg, qh);
> > + /* Free each QTD in the QH's QTD list */
> > + list_for_each_entry_safe(qtd2, qtd2_tmp, &qh-
> >qtd_list,
> > +
>qtd_list_entry)
> > +
> dwc2_hcd_qtd_unlink_and_free(hsotg, qtd2, qh);
> > + dwc2_hcd_qh_free(hsotg, qh);
> > + }
> > fail0:
> > kfree(dwc2_urb);
> >
> > diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h index
> > 7b5841c..fc10549 100644
> > --- a/drivers/usb/dwc2/hcd.h
> > +++ b/drivers/usb/dwc2/hcd.h
> > @@ -463,6 +463,9 @@ extern void dwc2_hcd_queue_transactions(struct
> > dwc2_hsotg *hsotg,
> > /* Schedule Queue Functions */
> > /* Implemented in hcd_queue.c */
> > extern void dwc2_hcd_init_usecs(struct dwc
RE: [PATCH v5 00/22] usb: third series of updates for dwc2 driver
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Monday, June 1, 2015 8:34 PM > To: John Youn > Cc: ba...@ti.com; Kaukab, Yousaf; Heiko Stuebner; linux-usb@vger.kernel.org; > Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org; > jwer...@chromium.org; sergei.shtyl...@cogentembedded.com > Subject: Re: [PATCH v5 00/22] usb: third series of updates for dwc2 driver > > On Mon, Jun 01, 2015 at 06:16:03PM +, John Youn wrote: > > On 5/26/2015 8:21 AM, Felipe Balbi wrote: > > > On Tue, May 26, 2015 at 03:18:32PM +, Kaukab, Yousaf wrote: > > >>> -Original Message- > > >>> From: Felipe Balbi [mailto:ba...@ti.com] > > >>> Sent: Tuesday, May 26, 2015 11:12 PM > > >>> To: Heiko Stuebner > > >>> Cc: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > > >>> john.y...@synopsys.com; Herrero, Gregory; r.bald...@samsung.com; > > >>> dingu...@opensource.altera.com; zhangfei@linaro.org; > > >>> jwer...@chromium.org; sergei.shtyl...@cogentembedded.com > > >>> Subject: Re: [PATCH v5 00/22] usb: third series of updates for > > >>> dwc2 driver > > >>> > > >>> On Sun, May 24, 2015 at 12:33:39AM +0200, Heiko Stuebner wrote: > > >>>> Hi, > > >>>> > > >>>> Am Mittwoch, 29. April 2015, 22:08:58 schrieb Mian Yousaf Kaukab: > > >>>>> Hi, > > >>>>> This patchset consists of some bug fixes and feature > > >>>>> enhancements for the dwc2 driver. All the patches are verified > > >>>>> on dwc2 v3.0a with dedicated fifos. Main focus of testing was with > dma enabled. > > >>>>> Although basic testing without dma was also done. > > >>>>> > > >>>>> This is based on testing/next branch in Felipe's git. > > >>>>> > > >>>>> Thank you, > > >>>>> > > >>>>> Best regards, > > >>>>> Yousaf > > >>>>> > > >>>>> History: > > >>>>> v5: > > >>>>> - Fixed comments from Felipe Balbi > > >>>>> v4: > > >>>>> - Fixed comment from Julius Werner > > >>>>> v3: > > >>>>> - Fixed comments from Julius Werner and Sergei Shtylyov > > >>>>> - Dropped "usb: dwc2: allow dwc2_pci to be a module even when > > >>>>> dwc2 is statically linked" due to > > >>>>> http://marc.info/?l=linux-usb&m=142661773703793&w=2 > > >>>>> - "usb: dwc2: host: ensure qtb exists before dereferencing it" is > > >>>>> added > > >>>>>to fix a NULL pointer dereferencing bug > > >>>>> v2: > > >>>>> - Fixed comments from John Youn and Julius Werner > > >>>>> - Fixed regression, found by John, when switching to gadget mode > > >>>>>after running host mode > > >>>>> - Added patch to add core parameter for enabling/disabling > > >>>>> hibernation > > >>>>> - Added patch to build dwc2_pci.ko independent from dwc2.ko > > >>>>> v1: > > >>>>> - Fixed comments from John Youn and Robert Baldyga > > >>>>> - Dropped all changes to pci.c due to > > >>>>>http://permalink.gmane.org/gmane.linux.usb.general/123411 > > >>>>> - Added patch to remove unnecessary EXPORT_SYMBOL_GPL calls > > >>>> > > >>>> With this series applied I get the warning below about a sleeping > > >>>> function, that is not present without it. This happens quite > > >>>> often (on boot, when going to suspend, etc). > > >>>> > > >>>> Other than that, usb still works and the dwc2 now also finally > > >>>> suspends :-D, so on a rk3288 > > >>>> > > >>>> Tested-by: Heiko Stuebner > > >>>> > > >>>> -- 8< -- > > >>>> [ 19.799200] BUG: sleeping function called from invalid context at > > >>> mm/slab.c:2863 > > >>> > > >>> Will I see a patch for fixing this ? > > >> > > >> I am currently on a business trip and can't look into this for at > > >> least a couple of weeks. > > > > > > John, since this is your driver, could you fix it up ? > > > > > > > Hi Felipe, > > > > I've been out of the office for the past 2 weeks and am catching up on > > stuff now. > > > > I'll take a look at it later this week if I don't hear anything from > > Yousaf. I have patches to fix this issue (and another one). I will send them out hopefully tomorrow. > > Cool, thanks a lot :-) > > -- > balbi BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free bug
> -Original Message- > From: Yunzhi Li [mailto:l...@rock-chips.com] > Sent: Friday, May 29, 2015 1:22 PM > To: johny...@synopsys.com > Cc: he...@sntech.de; c...@rock-chips.com; h...@rock-chips.com; yk@rock- > chips.com; gaura...@google.com; albe...@google.com; w...@rock-chips.com; > jwer...@chromium.org; jeffy.c...@rock-chips.com; Herrero, Gregory; > Kaukab, Yousaf; huang...@rock-chips.com; rockchip-disc...@chromium.org; > Yunzhi Li; Greg Kroah-Hartman; linux-usb@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free bug > > When s3c_hsotg_handle_unaligned_buf_complete() hs_req->req.buf already > destroyed, in s3c_hsotg_unmap_dma(), it touches hs_req->req.dma again, so > s3c_hsotg_unmap_dma() should be called before > s3c_hsotg_handle_unaligned_buf_complete(). Otherwise, it will cause a > bad_page BUG, when allocate this memory page next time. > > This bug led to the following crash: > > BUG: Bad page state in process swapper/0 pfn:2bdbc > [ 26.820440] page:eed76780 count:0 mapcount:0 mapping: (null) index:0x0 > [ 26.854710] page flags: 0x200(arch_1) > [ 26.885836] page dumped because: PAGE_FLAGS_CHECK_AT_PREP flag set > [ 26.919179] bad because of flags: > [ 26.948917] page flags: 0x200(arch_1) > [ 26.979100] Modules linked in: > [ 27.008401] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W3.14.0 #17 > [ 27.041816] [] (unwind_backtrace) from [] > (show_stack+0x20/0x24) > [ 27.076108] [] (show_stack) from [] > (dump_stack+0x70/0x8c) > [ 27.110246] [] (dump_stack) from [] > (bad_page+0xfc/0x12c) > [ 27.143958] [] (bad_page) from [] > (get_page_from_freelist+0x3e4/0x50c) > [ 27.179298] [] (get_page_from_freelist) from [] > (__alloc_pages_nodemask) > [ 27.216296] [] (__alloc_pages_nodemask) from [] > (__get_free_pages+0x20/) > [ 27.252326] [] (__get_free_pages) from [] > (kmalloc_order_trace+0x34/0xa) > [ 27.288295] [] (kmalloc_order_trace) from [] > (__kmalloc+0x40/0x1ac) > [ 27.323751] [] (__kmalloc) from [] > (s3c_hsotg_ep_queue.isra.12+0x7c/0x1) > [ 27.359937] [] (s3c_hsotg_ep_queue.isra.12) from [] > (s3c_hsotg_ep_queue) > [ 27.397478] [] (s3c_hsotg_ep_queue_lock) from [] > (rx_submit+0xfc/0x164) > [ 27.433619] [] (rx_submit) from [] > (rx_complete+0x22c/0x230) > [ 27.468872] [] (rx_complete) from [] > (s3c_hsotg_complete_request+0xfc/0) > [ 27.506240] [] (s3c_hsotg_complete_request) from [] > (s3c_hsotg_handle_o) > [ 27.545401] [] (s3c_hsotg_handle_outdone) from [] > (s3c_hsotg_epint+0x2c) > [ 27.583689] [] (s3c_hsotg_epint) from [] > (s3c_hsotg_irq+0x1dc/0x4ac) > [ 27.621041] [] (s3c_hsotg_irq) from [] > (handle_irq_event_percpu+0x70/0x) > [ 27.659066] [] (handle_irq_event_percpu) from [] > (handle_irq_event+0x4c) > [ 27.697322] [] (handle_irq_event) from [] > (handle_fasteoi_irq+0xc8/0x11) > [ 27.735451] [] (handle_fasteoi_irq) from [] > (generic_handle_irq+0x30/0x) > [ 27.773918] [] (generic_handle_irq) from [] > (__handle_domain_irq+0x84/0) > [ 27.812018] [] (__handle_domain_irq) from [] > (gic_handle_irq+0x48/0x6c) > [ 27.849695] [] (gic_handle_irq) from [] > (__irq_svc+0x40/0x50) > [ 27.886907] Exception stack(0xc0d01ee0 to 0xc0d01f28) > > Signed-off-by: Yunzhi Li > > --- > > drivers/usb/dwc2/gadget.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index > 6a30887..8070602 100644 > --- a/drivers/usb/dwc2/gadget.c > +++ b/drivers/usb/dwc2/gadget.c > @@ -1389,14 +1389,14 @@ static void s3c_hsotg_complete_request(struct > dwc2_hsotg *hsotg, > if (hs_req->req.status == -EINPROGRESS) > hs_req->req.status = result; > > + if (using_dma(hsotg)) > + s3c_hsotg_unmap_dma(hsotg, hs_ep, hs_req); > + > s3c_hsotg_handle_unaligned_buf_complete(hsotg, hs_ep, > hs_req); > > hs_ep->req = NULL; > list_del_init(&hs_req->queue); > > - if (using_dma(hsotg)) > - s3c_hsotg_unmap_dma(hsotg, hs_ep, hs_req); > - > /* >* call the complete request with the locks off, just in case the >* request tries to queue more work for this endpoint. Looks good. Reviewed-by: Mian Yousaf Kaukab BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 01/11] usb: phy: return error on failure
> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Wednesday, May 27, 2015 12:20 PM
> To: Kaukab, Yousaf
> Cc: ba...@ti.com; linux-usb@vger.kernel.org
> Subject: Re: [PATCH 01/11] usb: phy: return error on failure
>
> On Wed, May 27, 2015 at 12:31:28AM +, Kaukab, Yousaf wrote:
> > > -Original Message-
> > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > Sent: Tuesday, May 26, 2015 11:17 PM
> > > To: Kaukab, Yousaf
> > > Cc: linux-usb@vger.kernel.org; ba...@ti.com
> > > Subject: Re: [PATCH 01/11] usb: phy: return error on failure
> > >
> > > On Tue, May 05, 2015 at 04:14:35PM +0200, Mian Yousaf Kaukab wrote:
> > > > If the requested phy operation can't be done, return error instead
> > > > of success to let the caller know.
> > > >
> > > > Signed-off-by: Mian Yousaf Kaukab
> > > > ---
> > > > include/linux/usb/phy.h | 10 +-
> > > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> > > > index bc91b5d..d2b03db 100644
> > > > --- a/include/linux/usb/phy.h
> > > > +++ b/include/linux/usb/phy.h
> > > > @@ -262,7 +262,7 @@ usb_phy_set_power(struct usb_phy *x, unsigned
> > > > mA) {
> > > > if (x && x->set_power)
> > > > return x->set_power(x, mA);
> > > > - return 0;
> > > > + return -EOPNOTSUPP;
> > >
> > > the idea is that these operations are actually optional, so it's ok to
> > > return 0.
> >
> > Caller can ignore the error if its optional for it. There are others
>
> ok, I have to spell it out.
>
> It's not optional for the *caller*, it's optional for the PHY driver to
> implement
> it.
>
> > who have put the similar validity checks before usb phy api calls
> > because they want to return error in case the call fails. We can
> > remove such checks by returning the error here.
>
> no... what we're doing here is:
>
> if the phy driver implements this call, then let's call that callback and pass
> along any error it might return, otherwise we just return success because that
> means this phy doesn't really implement this call.
>
> This is done is many other subsystems where some function pointers might be
> optional to be implemented. Just think about the error handling. When we
> return 0:
>
> ret = usb_phy_set_power(x, 100);
> if (ret < 0)
> bail();
>
> /* continue */
>
> if we return EOPNOTSUPP:
>
> ret = usb_phy_set_power(x, 100);
> if (ret < 0 && ret != -EOPNOTSUPP)
> bail();
>
> /* continue */
Got it. You can drop this series for now. I will resend a subset to fix
IS_ERR_OR_NULL stuff.
>
> cheers
>
> --
> balbi
Thanks,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 00/22] usb: third series of updates for dwc2 driver
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Tuesday, May 26, 2015 11:19 PM > To: Kaukab, Yousaf > Cc: ba...@ti.com; Heiko Stuebner; linux-usb@vger.kernel.org; > john.y...@synopsys.com; Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org; > jwer...@chromium.org; sergei.shtyl...@cogentembedded.com > Subject: Re: [PATCH v5 00/22] usb: third series of updates for dwc2 driver > > On Tue, May 26, 2015 at 03:18:32PM +, Kaukab, Yousaf wrote: > > > -Original Message- > > > From: Felipe Balbi [mailto:ba...@ti.com] > > > Sent: Tuesday, May 26, 2015 11:12 PM > > > To: Heiko Stuebner > > > Cc: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > > > john.y...@synopsys.com; Herrero, Gregory; r.bald...@samsung.com; > > > dingu...@opensource.altera.com; zhangfei@linaro.org; > > > jwer...@chromium.org; sergei.shtyl...@cogentembedded.com > > > Subject: Re: [PATCH v5 00/22] usb: third series of updates for dwc2 > > > driver > > > > > > On Sun, May 24, 2015 at 12:33:39AM +0200, Heiko Stuebner wrote: > > > > Hi, > > > > > > > > Am Mittwoch, 29. April 2015, 22:08:58 schrieb Mian Yousaf Kaukab: > > > > > Hi, > > > > > This patchset consists of some bug fixes and feature > > > > > enhancements for the dwc2 driver. All the patches are verified > > > > > on dwc2 v3.0a with dedicated fifos. Main focus of testing was with dma > enabled. > > > > > Although basic testing without dma was also done. > > > > > > > > > > This is based on testing/next branch in Felipe's git. > > > > > > > > > > Thank you, > > > > > > > > > > Best regards, > > > > > Yousaf > > > > > > > > > > History: > > > > > v5: > > > > > - Fixed comments from Felipe Balbi > > > > > v4: > > > > > - Fixed comment from Julius Werner > > > > > v3: > > > > > - Fixed comments from Julius Werner and Sergei Shtylyov > > > > > - Dropped "usb: dwc2: allow dwc2_pci to be a module even when > > > > > dwc2 is statically linked" due to > > > > > http://marc.info/?l=linux-usb&m=142661773703793&w=2 > > > > > - "usb: dwc2: host: ensure qtb exists before dereferencing it" is > > > > > added > > > > >to fix a NULL pointer dereferencing bug > > > > > v2: > > > > > - Fixed comments from John Youn and Julius Werner > > > > > - Fixed regression, found by John, when switching to gadget mode > > > > >after running host mode > > > > > - Added patch to add core parameter for enabling/disabling > > > > > hibernation > > > > > - Added patch to build dwc2_pci.ko independent from dwc2.ko > > > > > v1: > > > > > - Fixed comments from John Youn and Robert Baldyga > > > > > - Dropped all changes to pci.c due to > > > > >http://permalink.gmane.org/gmane.linux.usb.general/123411 > > > > > - Added patch to remove unnecessary EXPORT_SYMBOL_GPL calls > > > > > > > > With this series applied I get the warning below about a sleeping > > > > function, that is not present without it. This happens quite often > > > > (on boot, when going to suspend, etc). > > > > > > > > Other than that, usb still works and the dwc2 now also finally > > > > suspends :-D, so on a rk3288 > > > > > > > > Tested-by: Heiko Stuebner > > > > > > > > -- 8< -- > > > > [ 19.799200] BUG: sleeping function called from invalid context at > > > mm/slab.c:2863 > > > > > > Will I see a patch for fixing this ? > > > > I am currently on a business trip and can't look into this for at > > least a couple of weeks. > > John, since this is your driver, could you fix it up ? Following is most likely the culprit patch. 33ad261 usb: dwc2: host: spinlock urb_enqueue I will try to get to it as soon as possible if no one beats me to it. > > -- > balbi BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 01/11] usb: phy: return error on failure
> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Tuesday, May 26, 2015 11:17 PM
> To: Kaukab, Yousaf
> Cc: linux-usb@vger.kernel.org; ba...@ti.com
> Subject: Re: [PATCH 01/11] usb: phy: return error on failure
>
> On Tue, May 05, 2015 at 04:14:35PM +0200, Mian Yousaf Kaukab wrote:
> > If the requested phy operation can't be done, return error instead of
> > success to let the caller know.
> >
> > Signed-off-by: Mian Yousaf Kaukab
> > ---
> > include/linux/usb/phy.h | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h index
> > bc91b5d..d2b03db 100644
> > --- a/include/linux/usb/phy.h
> > +++ b/include/linux/usb/phy.h
> > @@ -262,7 +262,7 @@ usb_phy_set_power(struct usb_phy *x, unsigned mA)
> > {
> > if (x && x->set_power)
> > return x->set_power(x, mA);
> > - return 0;
> > + return -EOPNOTSUPP;
>
> the idea is that these operations are actually optional, so it's ok to return
> 0.
Caller can ignore the error if its optional for it. There are others who have
put the similar validity checks before usb phy api calls because they want to
return error in case the call fails. We can remove such checks by returning the
error here.
>
> --
> Balbi
BR,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 00/22] usb: third series of updates for dwc2 driver
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Tuesday, May 26, 2015 11:12 PM > To: Heiko Stuebner > Cc: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > john.y...@synopsys.com; Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org; > jwer...@chromium.org; sergei.shtyl...@cogentembedded.com > Subject: Re: [PATCH v5 00/22] usb: third series of updates for dwc2 driver > > On Sun, May 24, 2015 at 12:33:39AM +0200, Heiko Stuebner wrote: > > Hi, > > > > Am Mittwoch, 29. April 2015, 22:08:58 schrieb Mian Yousaf Kaukab: > > > Hi, > > > This patchset consists of some bug fixes and feature enhancements > > > for the dwc2 driver. All the patches are verified on dwc2 v3.0a with > > > dedicated fifos. Main focus of testing was with dma enabled. > > > Although basic testing without dma was also done. > > > > > > This is based on testing/next branch in Felipe's git. > > > > > > Thank you, > > > > > > Best regards, > > > Yousaf > > > > > > History: > > > v5: > > > - Fixed comments from Felipe Balbi > > > v4: > > > - Fixed comment from Julius Werner > > > v3: > > > - Fixed comments from Julius Werner and Sergei Shtylyov > > > - Dropped "usb: dwc2: allow dwc2_pci to be a module even when dwc2 > > > is statically linked" due to > > > http://marc.info/?l=linux-usb&m=142661773703793&w=2 > > > - "usb: dwc2: host: ensure qtb exists before dereferencing it" is added > > >to fix a NULL pointer dereferencing bug > > > v2: > > > - Fixed comments from John Youn and Julius Werner > > > - Fixed regression, found by John, when switching to gadget mode > > >after running host mode > > > - Added patch to add core parameter for enabling/disabling > > > hibernation > > > - Added patch to build dwc2_pci.ko independent from dwc2.ko > > > v1: > > > - Fixed comments from John Youn and Robert Baldyga > > > - Dropped all changes to pci.c due to > > >http://permalink.gmane.org/gmane.linux.usb.general/123411 > > > - Added patch to remove unnecessary EXPORT_SYMBOL_GPL calls > > > > With this series applied I get the warning below about a sleeping > > function, that is not present without it. This happens quite often (on > > boot, when going to suspend, etc). > > > > Other than that, usb still works and the dwc2 now also finally > > suspends :-D, so on a rk3288 > > > > Tested-by: Heiko Stuebner > > > > -- 8< -- > > [ 19.799200] BUG: sleeping function called from invalid context at > mm/slab.c:2863 > > Will I see a patch for fixing this ? I am currently on a business trip and can't look into this for at least a couple of weeks. > > -- > balbi BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 14/22] usb: dwc2: host: register handle to the phy
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Tuesday, April 28, 2015 10:49 PM > To: Kaukab, Yousaf > Cc: ba...@ti.com; linux-usb@vger.kernel.org; john.y...@synopsys.com; > Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org; > jwer...@chromium.org; sergei.shtyl...@cogentembedded.com > Subject: Re: [PATCH v4 14/22] usb: dwc2: host: register handle to the phy > > On Tue, Apr 28, 2015 at 08:47:08PM +, Kaukab, Yousaf wrote: > > > -Original Message- > > > From: Felipe Balbi [mailto:ba...@ti.com] > > > Sent: Monday, April 27, 2015 10:43 PM > > > To: Kaukab, Yousaf > > > Cc: linux-usb@vger.kernel.org; ba...@ti.com; john.y...@synopsys.com; > > > Herrero, Gregory; r.bald...@samsung.com; > > > dingu...@opensource.altera.com; zhangfei@linaro.org; > > > jwer...@chromium.org; sergei.shtyl...@cogentembedded.com > > > Subject: Re: [PATCH v4 14/22] usb: dwc2: host: register handle to > > > the phy > > > > > > On Tue, Mar 24, 2015 at 10:01:02AM +0100, Mian Yousaf Kaukab wrote: > > > > If phy driver is present register hcd handle to it. > > > > > > > > Signed-off-by: Mian Yousaf Kaukab > > > > --- > > > > drivers/usb/dwc2/hcd.c | 6 ++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index > > > > 1cca5e0..919658e 100644 > > > > --- a/drivers/usb/dwc2/hcd.c > > > > +++ b/drivers/usb/dwc2/hcd.c > > > > @@ -2913,6 +2913,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, > > > > int > > > irq) > > > > /* Don't support SG list at this point */ > > > > hcd->self.sg_tablesize = 0; > > > > > > > > + if (!IS_ERR_OR_NULL(hsotg->uphy)) > > > > > > when is uphy NULL ? Aparently, only platform.c sets it to NULL in > > > case of error, I'd say we should just make NULL a valid PHY pointer, > > > then we can remove a bunch of these checks scattered around. > > > > Here we can't as we want to dereference uphy. > > However, I like the idea and I guess it entails a separate patch to > > fix where possible? > > yeah, I'll handle that, unless you want to. It should be pretty simple. I can do it if it can wait. > > -- > balbi BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 14/22] usb: dwc2: host: register handle to the phy
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Monday, April 27, 2015 10:43 PM > To: Kaukab, Yousaf > Cc: linux-usb@vger.kernel.org; ba...@ti.com; john.y...@synopsys.com; > Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org; > jwer...@chromium.org; sergei.shtyl...@cogentembedded.com > Subject: Re: [PATCH v4 14/22] usb: dwc2: host: register handle to the phy > > On Tue, Mar 24, 2015 at 10:01:02AM +0100, Mian Yousaf Kaukab wrote: > > If phy driver is present register hcd handle to it. > > > > Signed-off-by: Mian Yousaf Kaukab > > --- > > drivers/usb/dwc2/hcd.c | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index > > 1cca5e0..919658e 100644 > > --- a/drivers/usb/dwc2/hcd.c > > +++ b/drivers/usb/dwc2/hcd.c > > @@ -2913,6 +2913,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int > irq) > > /* Don't support SG list at this point */ > > hcd->self.sg_tablesize = 0; > > > > + if (!IS_ERR_OR_NULL(hsotg->uphy)) > > when is uphy NULL ? Aparently, only platform.c sets it to NULL in case of > error, > I'd say we should just make NULL a valid PHY pointer, then we can remove a > bunch of these checks scattered around. Here we can't as we want to dereference uphy. However, I like the idea and I guess it entails a separate patch to fix where possible? > > -- > balbi BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1 18/20] usb: dwc2: host: don't use dma_alloc_coherent with irqs disabled
> -Original Message-
> From: jwer...@google.com [mailto:jwer...@google.com] On Behalf Of Julius
> Werner
> Sent: Friday, March 20, 2015 7:23 PM
> To: Kaukab, Yousaf
> Cc: Julius Werner; linux-usb@vger.kernel.org; Felipe Balbi;
> john.y...@synopsys.com; Herrero, Gregory; r.bald...@samsung.com; Dinh
> Nguyen; zhangfei@linaro.org
> Subject: Re: [PATCH v1 18/20] usb: dwc2: host: don't use dma_alloc_coherent
> with irqs disabled
>
> >> > - qh->dw_align_buf = dma_alloc_coherent(hsotg->dev,
> >> > buf_size,
> >> > -
> >> > &qh->dw_align_buf_dma,
> >> > - GFP_ATOMIC);
> >> > + qh->dw_align_buf = kzalloc(buf_size, GFP_ATOMIC);
> >>
> >> Shouldn't this also set GFP_DMA? [...]
> >
> > No, it should be GFP_ATOMIC, as this can be reached from interrupt handler.
>
> Are the two mutually exclusive? I meant that I think this should be
> 'kzalloc(buf_size, GFP_DMA | GFP_ATOMIC)', because I wouldn't be sure that
> GFP_ATOMIC always returns DMA-able memory on systems that may have
> limitations there.
>
> >> > }
> >> > }
> >> >
> >> > + qh->dw_align_buf_dma = dma_map_single(hsotg->dev,
> >> > + qh->dw_align_buf, qh->dw_align_buf_size,
> >> > + DMA_TO_DEVICE);
> >>
> >> Documentation/DMA-API.txt says that you must always use the same
> >> arguments for matching dma_map_single() and dma_unmap_single()... so
> >> I think this and all the unmaps should use DMA_BIDIRECTIONAL.
> >
> > The mapping is wrong. It should consider endpoint direction. Something
> > like chan->ep_is_in ? DMA_FROM_DEVICE : DMA_TO_DEVICE
>
> Yeah, I think this should work as well.
>
> However, looking at this again I think there are more problems on unmapping.
> Since some of those calls are guarded by chan->ep_is_in, you will not unmap
> the memory out OUT transfers. DMA map/unmap calls must always be
> balanced.
>
> I think in all the cases where the code previously had something like
>
> if (chan->align_buf && chan->ep_is_in) {
> memcpy(...);
> ...;
> }
>
> you'll need to change it into
>
> if (chan->align_buf) {
> if (chan->ep_is_in) {
> memcpy(...);
> dma_unmap_single(..., DMA_FROM_DEVICE);
> ...
> } else {
> dma_unmap_single(..., DMA_TO_DEVICE);
> }
> }
>
> You might also want to double-check all abnormal error paths to ensure you're
> not leaking a DMA mapping. The previous code might not have been so careful
> (since for a really bad error you usually don't care about memcpy()ing the
> receive buffer back), but if you use DMA mappings you have to.
Error paths looks ok to me. Let me know if you have any specific case in mind.
I will update the patch for the rest of the comments.
BR,
Yousaf
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
RE: [PATCH v2 00/22] usb: third series of updates for dwc2 driver
> -Original Message- > From: John Youn [mailto:john.y...@synopsys.com] > Sent: Saturday, March 21, 2015 1:46 AM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > john.y...@synopsys.com > Cc: Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org; > jwer...@chromium.org > Subject: Re: [PATCH v2 00/22] usb: third series of updates for dwc2 driver > > On 3/20/2015 8:35 AM, Mian Yousaf Kaukab wrote: > > Hi, > > This patchset consists of some bug fixes and feature enhancements for > > the dwc2 driver. All the patches are verified on dwc2 v3.0a with > > dedicated fifos. Main focus of testing was with dma enabled. Although > > basic testing without dma was also done. > > > > This is based on testing/next branch in Felipe's git. > > > > Hi Yousaf, > > The last 3 patches don't apply against Felipe's testing/next. > > Also, you can drop the last patch as it is addressed by this: > http://marc.info/?l=linux-usb&m=142661773703793&w=2 OK. I will drop "usb: dwc2: allow dwc2_pci to be a module even when dwc2 is statically linked" > > John BR, Yousaf
RE: [PATCH v1 00/20] usb: third series of updates for dwc2 driver
> -Original Message- > From: John Youn [mailto:john.y...@synopsys.com] > Sent: Thursday, March 19, 2015 11:55 PM > To: Kaukab, Yousaf; John Youn; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org > Subject: Re: [PATCH v1 00/20] usb: third series of updates for dwc2 driver > > On 3/19/2015 8:53 AM, Kaukab, Yousaf wrote: > > > > > >> -Original Message- > >> From: John Youn [mailto:john.y...@synopsys.com] > >> Sent: Wednesday, March 18, 2015 8:14 PM > >> To: Kaukab, Yousaf; John Youn; linux-usb@vger.kernel.org; > >> ba...@ti.com > >> Cc: Herrero, Gregory; r.bald...@samsung.com; > >> dingu...@opensource.altera.com; zhangfei@linaro.org > >> Subject: Re: [PATCH v1 00/20] usb: third series of updates for dwc2 > >> driver > >> > >> On 3/18/2015 8:45 AM, Kaukab, Yousaf wrote: > >>> > >>> > >>>> -Original Message- > >>>> From: John Youn [mailto:john.y...@synopsys.com] > >>>> Sent: Wednesday, March 18, 2015 3:33 AM > >>>> To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > >>>> john.y...@synopsys.com > >>>> Cc: Herrero, Gregory; r.bald...@samsung.com; > >>>> dingu...@opensource.altera.com; zhangfei@linaro.org > >>>> Subject: Re: [PATCH v1 00/20] usb: third series of updates for dwc2 > >>>> driver > >>>> > >>>> On 3/17/2015 2:54 AM, Mian Yousaf Kaukab wrote: > >>>>> Hi, > >>>>> This patchset consists of some bug fixes and feature enhancements > >>>>> for the dwc2 driver. All the patches are verified on dwc2 v3.0a > >>>>> with dedicated fifos. Main focus of testing was with dma enabled. > >>>>> Although basic testing without dma was also done. > >>>>> > >>>>> This is based on testing/next branch in Felipe's git. > >>>>> > >>>>> Thank you, > >>>>> > >>>>> Best regards, > >>>>> Yousaf > >>>>> > >>>> > >>>> Hi Yousaf, > >>>> > >>>> Patch 15 and 16 introduce regressions in two systems that I tested with. > >>> > >>> :( sorry for that. I will try to fix this asap. > >>> > >>>> > >>>> After patch 15, the host fails to work at all. > >>> > >>> My understanding is that functionality can break within the patchset > >>> but it > >> should build at any point. If that's not correct, is merging patch 15 > >> and 16 OK for you? > >>> > >> > >> Ok that's fine. Just thought I'd point out where the breakage started. > >> > >> > >>>> > >>>> After patch 16, the host works but the device fails enumeration > >>>> whenever switching from an A to B cable, going from host to > >>>> peripheral. After this fails, if you unplug and replug the B cable > >>>> it will enumerate. But whenever you switch from A to B it always fails. > >>>> > >>>> Tested on a Synopsys (2.94a) and Altera (2.93a) system. > >>>> > >>>> I don't have any logs right now but I can get those tomorrow if you > >>>> need > >> them. > >>> > >>> It will be very helpful if you can provide log after > >> CONFUG_USB_DWC2_DEBUG enabled and also function trace of dwc2 > driver. > >>> > >> > >> Logs attached. First is device connection after driver load. Second > >> is after role switch. > >> > >> Seems to fail both times with similar error. > >> > >> Relevant snippet: > >> > >> [ 77.814073] dwc2 dwc2.1.auto: gintsts=44008020 gintmsk=d08c3cd4 > >> [ 77.814079] dwc2 dwc2.1.auto: ++Session Request Interrupt++ > >> [ 77.814091] dwc2 dwc2.1.auto: s3c_hsotg_irq: 04008020 > >> (d08c3cd4) retry 8 > >> [ 77.817097] dwc2 dwc2.1.auto: s3c_hsotg_irq: 04008420 0400 > >> (d08c3cd4) retry 8 > >> [ 77.817102] dwc2 dwc2.1.auto: GINTSTS_ErlySusp > >> [ 77.820189] dwc2 dwc2.1.auto: gintsts=04008820 gintmsk=d08c3cd4 > >> [ 77.820194] dwc2 dwc2.1.auto: USB SUSPEND > >> [ 77.820202] dwc2 dwc2.1.auto: DSTS=0x3 > >> [ 77.820204] dwc2 dwc2.1.auto: DSTS.Suspend Status=
RE: [PATCH v1 18/20] usb: dwc2: host: don't use dma_alloc_coherent with irqs disabled
> -Original Message-
> From: jwer...@google.com [mailto:jwer...@google.com] On Behalf Of Julius
> Werner
> Sent: Wednesday, March 18, 2015 11:43 PM
> To: Kaukab, Yousaf
> Cc: linux-usb@vger.kernel.org; Felipe Balbi; john.y...@synopsys.com; Herrero,
> Gregory; r.bald...@samsung.com; Dinh Nguyen; zhangfei@linaro.org
> Subject: Re: [PATCH v1 18/20] usb: dwc2: host: don't use dma_alloc_coherent
> with irqs disabled
>
> [once more, without Gmail being stupid]
>
> Nice! This also solves problems with exhausting coherent DMA memory when
> you plug too many devices in.
>
> On Tue, Mar 17, 2015 at 2:54 AM, Mian Yousaf Kaukab
> wrote:
> > From: Gregory Herrero
> >
> > Align buffer must be allocated using kmalloc since irqs are disabled.
> > Coherency is handled through dma_map_single which can be used with
> > irqs disabled.
> >
> > Signed-off-by: Gregory Herrero
> > ---
> > drivers/usb/dwc2/hcd.c | 7 ---
> > drivers/usb/dwc2/hcd_intr.c | 10 ++
> > drivers/usb/dwc2/hcd_queue.c | 7 ---
> > 3 files changed, 18 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index
> > fd2ad25..54f58c1 100644
> > --- a/drivers/usb/dwc2/hcd.c
> > +++ b/drivers/usb/dwc2/hcd.c
> > @@ -710,9 +710,7 @@ static int dwc2_hc_setup_align_buf(struct
> dwc2_hsotg *hsotg, struct dwc2_qh *qh,
> > /* 3072 = 3 max-size Isoc packets */
> > buf_size = 3072;
> >
> > - qh->dw_align_buf = dma_alloc_coherent(hsotg->dev, buf_size,
> > - &qh->dw_align_buf_dma,
> > - GFP_ATOMIC);
> > + qh->dw_align_buf = kzalloc(buf_size, GFP_ATOMIC);
>
> Shouldn't this also set GFP_DMA? [...]
No, it should be GFP_ATOMIC, as this can be reached from interrupt handler.
> [...] (And does it need to be kzalloc()? I think
> kmalloc() should be enough and might avoid another memory
> transfer.)
kmalloc() is better.
>
> > if (!qh->dw_align_buf)
> > return -ENOMEM;
> > qh->dw_align_buf_size = buf_size; @@ -737,6 +735,9 @@
> > static int dwc2_hc_setup_align_buf(struct dwc2_hsotg *hsotg, struct dwc2_qh
> *qh,
> > }
> > }
> >
> > + qh->dw_align_buf_dma = dma_map_single(hsotg->dev,
> > + qh->dw_align_buf, qh->dw_align_buf_size,
> > + DMA_TO_DEVICE);
>
> Documentation/DMA-API.txt says that you must always use the same
> arguments for matching dma_map_single() and dma_unmap_single()... so I
> think this and all the unmaps should use DMA_BIDIRECTIONAL.
The mapping is wrong. It should consider endpoint direction. Something like
chan->ep_is_in ? DMA_FROM_DEVICE : DMA_TO_DEVICE
>
> > +
> > chan->align_buf = qh->dw_align_buf_dma;
> > return 0;
> > }
> > diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
> > index 6927bba..22f1476 100644
> > --- a/drivers/usb/dwc2/hcd_intr.c
> > +++ b/drivers/usb/dwc2/hcd_intr.c
> > @@ -468,6 +468,8 @@ static int dwc2_update_urb_state(struct dwc2_hsotg
> *hsotg,
> > /* Non DWORD-aligned buffer case handling */
> > if (chan->align_buf && xfer_length && chan->ep_is_in) {
> > dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n",
> > __func__);
> > + dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
> > + chan->qh->dw_align_buf_size,
> > + DMA_FROM_DEVICE);
> > memcpy(urb->buf + urb->actual_length,
> > chan->qh->dw_align_buf,
> >xfer_length);
> > }
> > @@ -559,6 +561,8 @@ static enum dwc2_halt_status
> dwc2_update_isoc_urb_state(
> > chan->ep_is_in) {
> > dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n",
> > __func__);
> > + dma_unmap_single(hsotg->dev, chan->qh-
> >dw_align_buf_dma,
> > + chan->qh->dw_align_buf_size,
> > + DMA_FROM_DEVICE);
> > memcpy(urb->buf + frame_desc->offset +
> >qtd->isoc_split_offset,
> > chan->qh->dw_align_buf,
> >frame_desc->
RE: [PATCH v1 00/20] usb: third series of updates for dwc2 driver
> -Original Message- > From: John Youn [mailto:john.y...@synopsys.com] > Sent: Wednesday, March 18, 2015 8:14 PM > To: Kaukab, Yousaf; John Youn; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org > Subject: Re: [PATCH v1 00/20] usb: third series of updates for dwc2 driver > > On 3/18/2015 8:45 AM, Kaukab, Yousaf wrote: > > > > > >> -Original Message- > >> From: John Youn [mailto:john.y...@synopsys.com] > >> Sent: Wednesday, March 18, 2015 3:33 AM > >> To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > >> john.y...@synopsys.com > >> Cc: Herrero, Gregory; r.bald...@samsung.com; > >> dingu...@opensource.altera.com; zhangfei@linaro.org > >> Subject: Re: [PATCH v1 00/20] usb: third series of updates for dwc2 > >> driver > >> > >> On 3/17/2015 2:54 AM, Mian Yousaf Kaukab wrote: > >>> Hi, > >>> This patchset consists of some bug fixes and feature enhancements > >>> for the dwc2 driver. All the patches are verified on dwc2 v3.0a with > >>> dedicated fifos. Main focus of testing was with dma enabled. > >>> Although basic testing without dma was also done. > >>> > >>> This is based on testing/next branch in Felipe's git. > >>> > >>> Thank you, > >>> > >>> Best regards, > >>> Yousaf > >>> > >> > >> Hi Yousaf, > >> > >> Patch 15 and 16 introduce regressions in two systems that I tested with. > > > > :( sorry for that. I will try to fix this asap. > > > >> > >> After patch 15, the host fails to work at all. > > > > My understanding is that functionality can break within the patchset but it > should build at any point. If that's not correct, is merging patch 15 and 16 > OK > for you? > > > > Ok that's fine. Just thought I'd point out where the breakage started. > > > >> > >> After patch 16, the host works but the device fails enumeration > >> whenever switching from an A to B cable, going from host to > >> peripheral. After this fails, if you unplug and replug the B cable it > >> will enumerate. But whenever you switch from A to B it always fails. > >> > >> Tested on a Synopsys (2.94a) and Altera (2.93a) system. > >> > >> I don't have any logs right now but I can get those tomorrow if you need > them. > > > > It will be very helpful if you can provide log after > CONFUG_USB_DWC2_DEBUG enabled and also function trace of dwc2 driver. > > > > Logs attached. First is device connection after driver load. Second is after > role > switch. > > Seems to fail both times with similar error. > > Relevant snippet: > > [ 77.814073] dwc2 dwc2.1.auto: gintsts=44008020 gintmsk=d08c3cd4 > [ 77.814079] dwc2 dwc2.1.auto: ++Session Request Interrupt++ > [ 77.814091] dwc2 dwc2.1.auto: s3c_hsotg_irq: 04008020 > (d08c3cd4) retry 8 > [ 77.817097] dwc2 dwc2.1.auto: s3c_hsotg_irq: 04008420 0400 > (d08c3cd4) retry 8 > [ 77.817102] dwc2 dwc2.1.auto: GINTSTS_ErlySusp > [ 77.820189] dwc2 dwc2.1.auto: gintsts=04008820 gintmsk=d08c3cd4 > [ 77.820194] dwc2 dwc2.1.auto: USB SUSPEND > [ 77.820202] dwc2 dwc2.1.auto: DSTS=0x3 > [ 77.820204] dwc2 dwc2.1.auto: DSTS.Suspend Status=1 HWCFG4.Power > Optimize=0 > [ 77.820213] dwc2 dwc2.1.auto: s3c_hsotg_irq: 04008020 > (d08c3cd4) retry 8 > [ 78.074297] dwc2 dwc2.1.auto: s3c_hsotg_irq: 04009020 1000 > (d08c3cd4) retry 8 > [ 78.074305] dwc2 dwc2.1.auto: s3c_hsotg_irq: USBRst > [ 78.074310] dwc2 dwc2.1.auto: GNPTXSTS=00040400 > [ 78.074314] dwc2 dwc2.1.auto: complete: ep 8800d50ce618 ep0, req > 8800d524ae80, -104 => a042be00 > [ 78.074317] dwc2 dwc2.1.auto: s3c_hsotg_complete_setup: failed -104 > [ 78.074326] dwc2 dwc2.1.auto: FIFOs reset, timeout at 100 > [ 78.074343] dwc2 dwc2.1.auto: EP0: DIEPCTL0=0x8000, > DOEPCTL0=0x80008000 > [ 78.074347] dwc2 dwc2.1.auto: gsintmsk now 0xd08c3cc4 > [ 78.074351] dwc2 dwc2.1.auto: gsintmsk now 0xd08c3cd4 > [ 78.074361] dwc2 dwc2.1.auto: DCTL=0x > [ 78.074362] dwc2 dwc2.1.auto: s3c_hsotg_enqueue_setup: queueing setup > request > [ 78.074363] dwc2 dwc2.1.auto: ep0: req 8800d524ae80: > 8@8801199df798, noi=0, zero=0, snok=0 > [ 78.074364] dwc2 dwc2.1.auto: s3c_hsotg_ep_queue: don't submit request > while suspended > [ 78.074365] dwc2 dwc2.1.auto: s3c_hsotg_enqueue_setup: fai
RE: [PATCH v1 00/20] usb: third series of updates for dwc2 driver
> -Original Message- > From: John Youn [mailto:john.y...@synopsys.com] > Sent: Wednesday, March 18, 2015 3:33 AM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > john.y...@synopsys.com > Cc: Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com; zhangfei@linaro.org > Subject: Re: [PATCH v1 00/20] usb: third series of updates for dwc2 driver > > On 3/17/2015 2:54 AM, Mian Yousaf Kaukab wrote: > > Hi, > > This patchset consists of some bug fixes and feature enhancements for > > the dwc2 driver. All the patches are verified on dwc2 v3.0a with > > dedicated fifos. Main focus of testing was with dma enabled. Although > > basic testing without dma was also done. > > > > This is based on testing/next branch in Felipe's git. > > > > Thank you, > > > > Best regards, > > Yousaf > > > > Hi Yousaf, > > Patch 15 and 16 introduce regressions in two systems that I tested with. :( sorry for that. I will try to fix this asap. > > After patch 15, the host fails to work at all. My understanding is that functionality can break within the patchset but it should build at any point. If that's not correct, is merging patch 15 and 16 OK for you? > > After patch 16, the host works but the device fails enumeration whenever > switching from an A to B cable, going from host to peripheral. After this > fails, if > you unplug and replug the B cable it will enumerate. But whenever you switch > from A to B it always fails. > > Tested on a Synopsys (2.94a) and Altera (2.93a) system. > > I don't have any logs right now but I can get those tomorrow if you need them. It will be very helpful if you can provide log after CONFUG_USB_DWC2_DEBUG enabled and also function trace of dwc2 driver. > > John > And thank you for other comments. I have fixed all of them. I will send the next revision as soon as we find a fix for the regression you mentioned. BR, Yousaf
RE: [PATCH 00/19] usb: third series of updates for dwc2 driver
> -Original Message- > From: zhangfei [mailto:zhangfei@linaro.org] > Sent: Monday, March 16, 2015 3:26 AM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; > john.y...@synopsys.com > Cc: Herrero, Gregory; r.bald...@samsung.com; > dingu...@opensource.altera.com > Subject: Re: [PATCH 00/19] usb: third series of updates for dwc2 driver > > > > On 03/09/2015 11:04 PM, Mian Yousaf Kaukab wrote: > > Hi, > > This patchset consists of some bug fixes and feature enhancements for > > the dwc2 driver. All the patches are verified on dwc2 v3.0a with > > dedicated fifos. Main focus of testing was with dma enabled. Although > > basic testing without dma was also done. > > > > This is based on testing/next branch in Felipe's git. > > Thanks Yousaf > Also verified on hikey board, dma mode, though it is also dwc2 v3.0a. Thank you for verifying this patch-set. Can I put your Tested-by on this series? BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 01/19] usb: dwc2: move debugfs code to a separate file
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: Monday, March 16, 2015 6:31 PM
> To: Kaukab, Yousaf; 'ba...@ti.com'; John Youn
> Cc: linux-usb@vger.kernel.org; Herrero, Gregory; r.bald...@samsung.com;
> dingu...@opensource.altera.com; zhangfei@linaro.org
> Subject: Re: [PATCH 01/19] usb: dwc2: move debugfs code to a separate file
>
> On 3/16/2015 6:43 AM, Kaukab, Yousaf wrote:
> >
> >> -Original Message-
> >> From: Felipe Balbi [mailto:ba...@ti.com]
> >> Sent: Saturday, March 14, 2015 6:11 PM
> >> To: John Youn
> >> Cc: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; Herrero,
> >> Gregory; r.bald...@samsung.com; dingu...@opensource.altera.com;
> >> zhangfei@linaro.org
> >> Subject: Re: [PATCH 01/19] usb: dwc2: move debugfs code to a separate
> >> file
> >>
> >> On Mon, Mar 09, 2015 at 06:42:36PM -0700, John Youn wrote:
> >>> On 3/9/2015 8:04 AM, Mian Yousaf Kaukab wrote:
> >>>> +
> >>>> +int dwc2_debugfs_init(struct dwc2_hsotg *hsotg) {
> >>>> +int ret;
> >>>> +
> >>>> +hsotg->debug_root = debugfs_create_dir(dev_name(hsotg-
> >>> dev), NULL);
> >>>> +if (!hsotg->debug_root) {
> >>>> +ret = -ENOMEM;
> >>>> +goto err0;
> >>>> +}
> >>>> +
> >>>> +/* Add gadget debugfs nodes */
> >>>> +s3c_hsotg_create_debug(hsotg);
> >>>> +err0:
> >>>> +return ret;
> >>>> +}
> >>>
> >>> Need export for this function when dwc2-platform is configured as a
> module.
> >>
> >> the file is still part of the same binary, right ? EXPORT_SYMBOL*()
> >> are only needed when functions are exposed to other modules. Usually,
> >> EXPORT_SYMBOL*() in a driver (not in the framework) is an indication
> >> that something's wrong ;-)
> >
> > EXPORT_SYMBOL* are needed because the driver is built as three different
> modules dwc2.ko dwc2_platform.ko and dwc2_pci.ko.
> > As dwc2_pci is now exporting platform device as well, we can get rid
> > of all EXPORT_SYMBOLS* by having only two modules dwc2.ko and
> > dwc2_pci.ko. That is by including platform.o in dwc2.ko
> >
> > I will include the missing EXPORT_SYMBOLS* in this patch.
> >
> > I have another patch read to merge dwc2.ko and dwc2_platform.ko and
> remove all EXPORT_SYMBOLS*. If John agrees, then I can include this in the
> next revision of this patch-set.
>
> Yes that would be great if you could do that.
Ok I will include it in the next patch-set revision. Which, hopefully, I will
send out in a couple of day.
>
> Thanks,
> John
>
BR,
Yousaf
RE: [PATCH 02/19] usb: dwc2: debugfs: add support for complete register dump
> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Saturday, March 14, 2015 6:15 PM
> To: Kaukab, Yousaf
> Cc: linux-usb@vger.kernel.org; ba...@ti.com; john.y...@synopsys.com;
> Herrero, Gregory; r.bald...@samsung.com;
> dingu...@opensource.altera.com; zhangfei@linaro.org
> Subject: Re: [PATCH 02/19] usb: dwc2: debugfs: add support for complete
> register dump
>
> On Mon, Mar 09, 2015 at 04:04:42PM +0100, Mian Yousaf Kaukab wrote:
> > Dump all registers to take a complete snapshot of dwc2 state.
> > Code is inspired by dwc3/debugfs.c
> >
> > Signed-off-by: Mian Yousaf Kaukab
> > ---
> > drivers/usb/dwc2/core.h| 1 +
> > drivers/usb/dwc2/debugfs.c | 356
> > +
> > 2 files changed, 357 insertions(+)
> >
> > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index
> > 9e2bf4d..e60655f 100644
> > --- a/drivers/usb/dwc2/core.h
> > +++ b/drivers/usb/dwc2/core.h
> > @@ -613,6 +613,7 @@ struct dwc2_hsotg {
> > enum dwc2_lx_state lx_state;
> >
> > struct dentry *debug_root;
> > + struct debugfs_regset32 *regset;
> >
> > /* DWC OTG HW Release versions */
> > #define DWC2_CORE_REV_2_71a0x4f54271a
> > diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
> > index 6c3225c..b8b105e 100644
> > --- a/drivers/usb/dwc2/debugfs.c
> > +++ b/drivers/usb/dwc2/debugfs.c
> > @@ -391,9 +391,344 @@ static inline void s3c_hsotg_create_debug(struct
> > dwc2_hsotg *hsotg) {}
> >
> > /* s3c_hsotg_delete_debug is removed as cleanup in done in
> > dwc2_debugfs_exit */
> >
> > +#define dump_register(nm) \
> > +{ \
> > + .name = #nm, \
>
> sure you don't need __stringify() here ?
Yes, sure.
with __stringify() I get register addresses instead of names. That is something
like (0x000) = 0x04cd instead of GOTGCTL = 0x04cd.
>
> --
> balbi
BR,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 01/19] usb: dwc2: move debugfs code to a separate file
> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Saturday, March 14, 2015 6:11 PM
> To: John Youn
> Cc: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; Herrero,
> Gregory; r.bald...@samsung.com; dingu...@opensource.altera.com;
> zhangfei@linaro.org
> Subject: Re: [PATCH 01/19] usb: dwc2: move debugfs code to a separate file
>
> On Mon, Mar 09, 2015 at 06:42:36PM -0700, John Youn wrote:
> > On 3/9/2015 8:04 AM, Mian Yousaf Kaukab wrote:
> > > +
> > > +int dwc2_debugfs_init(struct dwc2_hsotg *hsotg) {
> > > + int ret;
> > > +
> > > + hsotg->debug_root = debugfs_create_dir(dev_name(hsotg-
> >dev), NULL);
> > > + if (!hsotg->debug_root) {
> > > + ret = -ENOMEM;
> > > + goto err0;
> > > + }
> > > +
> > > + /* Add gadget debugfs nodes */
> > > + s3c_hsotg_create_debug(hsotg);
> > > +err0:
> > > + return ret;
> > > +}
> >
> > Need export for this function when dwc2-platform is configured as a module.
>
> the file is still part of the same binary, right ? EXPORT_SYMBOL*() are only
> needed when functions are exposed to other modules. Usually,
> EXPORT_SYMBOL*() in a driver (not in the framework) is an indication that
> something's wrong ;-)
EXPORT_SYMBOL* are needed because the driver is built as three different
modules dwc2.ko dwc2_platform.ko and dwc2_pci.ko.
As dwc2_pci is now exporting platform device as well, we can get rid of all
EXPORT_SYMBOLS* by having only two modules dwc2.ko and dwc2_pci.ko. That is by
including platform.o in dwc2.ko
I will include the missing EXPORT_SYMBOLS* in this patch.
I have another patch read to merge dwc2.ko and dwc2_platform.ko and remove all
EXPORT_SYMBOLS*. If John agrees, then I can include this in the next revision
of this patch-set.
>
> --
> balbi
BR,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3] usb: dwc2: gadget reuse ahbcfg assigned from platform
> -Original Message- > From: John Youn [mailto:john.y...@synopsys.com] > Sent: Tuesday, February 24, 2015 3:27 AM > To: zhangfei; John Youn; ba...@ti.com; Kaukab, Yousaf > Cc: linux-usb@vger.kernel.org > Subject: Re: [PATCH v3] usb: dwc2: gadget reuse ahbcfg assigned from platform > > On 2/20/2015 7:26 PM, zhangfei wrote: > > Hi, John > > > > On 02/21/2015 08:35 AM, John Youn wrote: > >> On 2/15/2015 5:50 AM, Zhangfei Gao wrote: > >>> Reuse ahbcfg if assigned from platform > >>> > >>> Input from John: > >>> AHB_SINGLE, NOTI_ALL_DMA_WRIT, REM_MEM_SUPP, HBSTLEN, and > >>> INV_DESC_ENDIANNESS only apply in DMA mode and are ignored in slave > >>> mode operation. > >>> > >>> Signed-off-by: Zhangfei Gao > >>> --- > >>> drivers/usb/dwc2/gadget.c | 11 --- > >>> 1 file changed, 8 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > >>> index 15aa578..5726fbe 100644 > >>> --- a/drivers/usb/dwc2/gadget.c > >>> +++ b/drivers/usb/dwc2/gadget.c > >>> @@ -2314,14 +2314,19 @@ void s3c_hsotg_core_init_disconnected(struct > dwc2_hsotg *hsotg, > >>> GINTSTS_USBSUSP | GINTSTS_WKUPINT, > >>> hsotg->regs + GINTMSK); > >>> > >>> + if (hsotg->core_params && hsotg->core_params->ahbcfg != -1) > >>> + val = hsotg->core_params->ahbcfg & > ~GAHBCFG_CTRL_MASK; > >>> + else > >>> + val = 0; > >>> + > >>> if (using_dma(hsotg)) > >>> writel(GAHBCFG_GLBL_INTR_EN | > GAHBCFG_DMA_EN | > >>> -(GAHBCFG_HBSTLEN_INCR4 << > GAHBCFG_HBSTLEN_SHIFT), > >>> -hsotg->regs + GAHBCFG); > >>> +(val ? val : GAHBCFG_HBSTLEN_INCR4 << > >>> +GAHBCFG_HBSTLEN_SHIFT), hsotg->regs + > GAHBCFG); > >>> else > >>> writel(((hsotg->dedicated_fifos) ? > (GAHBCFG_NP_TXF_EMP_LVL | > >>> > GAHBCFG_P_TXF_EMP_LVL) : 0) | > >>> -GAHBCFG_GLBL_INTR_EN, > >>> +GAHBCFG_GLBL_INTR_EN | val, > >>> hsotg->regs + GAHBCFG); > >>> > >>> /* > >>> > >> > >> Hi Zhangfei, > >> > >> It looks like currently that the hsotg->core_params structure is not > >> designed > to be accessed by the gadget part (or this structure was overlooked during the > integration of these drivers). It is only initialized in the host mode and > this > should crash if host mode is not configured. > >> > > If dwc2_hcd_init only called in host mode, hsotg->core_params = NULL. > > So add judgement if (hsotg->core_params) here to prevent crash. > > Yes you're right. However core_params is documented as both a device and > host accessible member but it is only ever initialized and used in host mode. > I > think it should be fixed to always have valid and initialized values in device > mode as well. http://permalink.gmane.org/gmane.linux.usb.general/123272 should fix this. > > John > > BR, Yousaf
RE: [PATCH 1/4] usb: gadget: net2280: use ep_autoconfig compatible names in advance mode
Hi Felipe,
Can you take this series or would you like me to resend it?
BR,
Yousaf
> -Original Message-
> From: Kaukab, Yousaf
> Sent: Monday, February 2, 2015 10:55 AM
> To: linux-usb@vger.kernel.org; ba...@ti.com; ricardo.riba...@gmail.com;
> st...@rowland.harvard.edu
> Cc: Kaukab, Yousaf
> Subject: [PATCH 1/4] usb: gadget: net2280: use ep_autoconfig compatible
> names in advance mode
>
> Each struct usb_ep added for net2280 can be used in either direction.
> Whereas, each struct usb_ep for usb3380 has fixed direction. Use ep_autoconf
> compatible names so that endpoint with correct direction can be selected.
>
> Name sequence is due to the logic in usb_reinit_338x() in ne[] and
> ep_reg_addr[].
>
> Signed-off-by: Mian Yousaf Kaukab
> ---
> drivers/usb/gadget/udc/net2280.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/net2280.c
> b/drivers/usb/gadget/udc/net2280.c
> index d2c0bf6..b7024dc 100644
> --- a/drivers/usb/gadget/udc/net2280.c
> +++ b/drivers/usb/gadget/udc/net2280.c
> @@ -80,6 +80,13 @@ static const char *const ep_name[] = {
> "ep-e", "ep-f", "ep-g", "ep-h",
> };
>
> +/* Endpoint names for usb3380 advance mode */ static const char *const
> +ep_name_adv[] = {
> + ep0name,
> + "ep1in", "ep2out", "ep3in", "ep4out",
> + "ep1out", "ep2in", "ep3out", "ep4in",
> +};
> +
> /* mode 0 == ep-{a,b,c,d} 1K fifo each
> * mode 1 == ep-{a,b} 2K fifo each, ep-{c,d} unavailable
> * mode 2 == ep-a 2K fifo, ep-{b,c} 1K each, ep-d unavailable @@ -1977,7
> +1984,7 @@ static void usb_reinit_338x(struct net2280 *dev)
> for (i = 0; i < dev->n_ep; i++) {
> struct net2280_ep *ep = &dev->ep[i];
>
> - ep->ep.name = ep_name[i];
> + ep->ep.name = dev->enhanced_mode ?
> ep_name_adv[i] : ep_name[i];
> ep->dev = dev;
> ep->num = i;
>
> --
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH v1] usb: dwc2: reduce dwc2 driver probe time
> -Original Message- > From: John Youn [mailto:john.y...@synopsys.com] > Sent: Thursday, February 12, 2015 4:33 AM > To: Roy; john.y...@synopsys.com; Felipe Balbi > Cc: Yunzhi Li; jwer...@chromium.org; Herrero, Gregory; Kaukab, Yousaf; > r.bald...@samsung.com; Dinh Nguyen; Eddie Cai; Lin Huang; wulf; 杨凯; Tao > Huang; walkr...@126.com; Douglas Anderson; Greg Kroah-Hartman; linux- > u...@vger.kernel.org; LKML > Subject: Re: [RFC PATCH v1] usb: dwc2: reduce dwc2 driver probe time > > On 2/11/2015 3:42 AM, Roy wrote: > > Hi John Youn: > > > > Could you please give some suggestions from your point of view, > > about this probe time issue ? > > > > Thanks a lot. > > > > at 2015/2/11 2:23, Julius Werner wrote: > >>> @@ -2703,7 +2703,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg > *hsotg) > >>> gusbcfg = readl(hsotg->regs + GUSBCFG); > >>> gusbcfg &= ~GUSBCFG_FORCEHOSTMODE; > >>> writel(gusbcfg, hsotg->regs + GUSBCFG); > >>> - usleep_range(10, 15); > >>> + usleep_range(25000, 5); > >> The point of usleep_range() is to coalesce multiple timer interrupts > >> in idle systems for power efficiency. It's pretty pointless/harmful > >> during probe anyway and there's almost never a reason to make the > >> span larger than a few milliseconds. You should reduce this to > >> something reasonable (e.g. usleep_range(25000, 26000) or even > >> usleep_range(25000, 25000)) to save another chunk of time. Same > >> applies to other delays above. > > Databook does say 25ms. From what I could gather this has to do with the > debounce filter time on the IDDIG pin after the ForceHstMode/ForceDevMode > is programmed. There is no way to poll this. I think the change is acceptable, > even to lower the range as Julius suggested. > > >> > >>> do you know what's the upper boundary for AHB clock ? How fast can > >>> it be? It's not wise to change timers because "it works on my RK3288 > >>> board", you need to guarantee that this won't break anybody else. > >> But this code is already a loop that spins on the AHBIdle bit, right? > >> It should work correctly regardless of the delay. The only question > >> is whether the code could be more efficient with a longer sleep... > >> but since the general recommendation is to delay for ranges less than > >> 10us, and the AHB clock would need to be lower than 100KHz (the ones > >> I see are usually in the range of tens or hundreds of MHz) to take > >> longer than that, this seems reasonable to me. > > Agree with this. It shouldn't take nearly that long and you are polling > anyways. > > > As for the other change: > > > It seems that usleep_range() at boot time will pick the longest value > > in the range. In dwc2_core_reset() there is a very long delay takes > > 200ms, and this function run twice when probe, could any one tell me > > is this delay time resonable ? > > I'm not sure about this value or the reasoning/history behind it. It is not > in our > internal code. It looks like it is taking into account the delay for the > ForceHstMode/ForceDevMode programming. However, I think your change is > conservative and should be ok. Maybe Samsung engineers know about this? If the delay is due to ForceHstMode/ForceDevMode then it should be reduce to 25ms range. As done in dwc2_get_hwparams for example. > > John > > > BR, Yousaf
RE: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from platform
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: Wednesday, February 11, 2015 11:00 PM
> To: Zhangfei Gao; Kaukab, Yousaf
> Cc: ba...@ti.com; john.y...@synopsys.com; linux-usb@vger.kernel.org
> Subject: Re: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from
> platform
>
> On 2/6/2015 2:23 PM, John Youn wrote:
> > On 2/6/2015 6:02 AM, Zhangfei Gao wrote:
> >> On 6 February 2015 at 16:07, Kaukab, Yousaf
> wrote:
> >>>>>> GAHBCFG_HBSTLEN_INCR4 << diff --git a/drivers/usb/dwc2/gadget.c
> >>>>>> b/drivers/usb/dwc2/gadget.c index 15aa578..20085de 100644
> >>>>>> --- a/drivers/usb/dwc2/gadget.c
> >>>>>> +++ b/drivers/usb/dwc2/gadget.c
> >>>>>> @@ -2314,9 +2314,13 @@ void
> >>>>>> s3c_hsotg_core_init_disconnected(struct
> >>>>>> dwc2_hsotg *hsotg,
> >>>>>> GINTSTS_USBSUSP | GINTSTS_WKUPINT,
> >>>>>> hsotg->regs + GINTMSK);
> >>>>>>
> >>>>>> + if ((hsotg->core_params) && (hsotg->core_params->ahbcfg !=
> >>>>>> + -
> >>>>>> 1))
> >>>>>> + val = hsotg->core_params->ahbcfg &
> >>>>>> ~GAHBCFG_CTRL_MASK;
> >>>>>> + else
> >>>>>> + val = GAHBCFG_HBSTLEN_INCR4 <<
> >>>>>> GAHBCFG_HBSTLEN_SHIFT;
> >>>>>> +
> >>>>>> if (using_dma(hsotg))
> >>>>>> - writel(GAHBCFG_GLBL_INTR_EN |
> >>>>>> GAHBCFG_DMA_EN |
> >>>>>> -(GAHBCFG_HBSTLEN_INCR4 <<
> >>>>>> GAHBCFG_HBSTLEN_SHIFT),
> >>>>>> + writel(GAHBCFG_GLBL_INTR_EN |
> >>>>>> GAHBCFG_DMA_EN | val,
> >>>>>> hsotg->regs + GAHBCFG);
> >>>>>> else
> >>>>>> writel(((hsotg->dedicated_fifos) ?
> >>>>>> (GAHBCFG_NP_TXF_EMP_LVL |
> >>>>>
> >>>>> There are other bits in GAHBCFG that can be set from platform.
> >>>>> They will be
> >>>> preserved by your patch, as they are not part of GAHBCFG_CTRL_MASK,
> >>>> but only in case dma is enabled. Perhaps preserve them in non-dma case
> as well.
> >>>>
> >>>> Here may have issue if also set hsotg->core_params->ahbcfg for
> >>>> non-dma case, since GAHBCFG[4:1] may be set.
> >>>
> >>> You can mask off HBstLen in that case. However, I don't think setting
> >>> burst
> length will be an issue in non DMA case as DWC2 will not act as a bus master.
> John, can you please confirm if setting burst length will be an issue in
> non-dma
> case?
> >
> > I don't think it hurts to preserve those bits, but I will check.
> >
>
> I can confirm that AHB_SINGLE, NOTI_ALL_DMA_WRIT, REM_MEM_SUPP,
> HBSTLEN, and INV_DESC_ENDIANNESS only apply in DMA mode and are
> ignored in slave mode operation.
John, thank you! So, there is no harm in setting these in non-dma mode.
>
> John
>
>
BR,
Yousaf
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
RE: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from platform
> -Original Message-
> From: Zhangfei Gao [mailto:zhangfei@linaro.org]
> Sent: Thursday, February 5, 2015 3:22 AM
> To: Kaukab, Yousaf
> Cc: ba...@ti.com; john.y...@synopsys.com; linux-usb@vger.kernel.org
> Subject: Re: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from
> platform
>
> Hi Yousaf
>
> On 4 February 2015 at 17:41, Kaukab, Yousaf
> wrote:
> >> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c index
> >> d5197d4..8d388cc 100644
> >> --- a/drivers/usb/dwc2/core.c
> >> +++ b/drivers/usb/dwc2/core.c
> >> @@ -2563,7 +2563,7 @@ void dwc2_set_param_reload_ctl(struct
> >> dwc2_hsotg *hsotg, int val)
> >>
> >> void dwc2_set_param_ahbcfg(struct dwc2_hsotg *hsotg, int val) {
> >> - if (val != -1)
> >> + if (val)
> >> hsotg->core_params->ahbcfg = val;
> >> else
> >> hsotg->core_params->ahbcfg =
> >> GAHBCFG_HBSTLEN_INCR4 << diff --git a/drivers/usb/dwc2/gadget.c
> >> b/drivers/usb/dwc2/gadget.c index 15aa578..20085de 100644
> >> --- a/drivers/usb/dwc2/gadget.c
> >> +++ b/drivers/usb/dwc2/gadget.c
> >> @@ -2314,9 +2314,13 @@ void s3c_hsotg_core_init_disconnected(struct
> >> dwc2_hsotg *hsotg,
> >> GINTSTS_USBSUSP | GINTSTS_WKUPINT,
> >> hsotg->regs + GINTMSK);
> >>
> >> + if ((hsotg->core_params) && (hsotg->core_params->ahbcfg != -
> >> 1))
> >> + val = hsotg->core_params->ahbcfg &
> >> ~GAHBCFG_CTRL_MASK;
> >> + else
> >> + val = GAHBCFG_HBSTLEN_INCR4 <<
> >> GAHBCFG_HBSTLEN_SHIFT;
> >> +
> >> if (using_dma(hsotg))
> >> - writel(GAHBCFG_GLBL_INTR_EN |
> >> GAHBCFG_DMA_EN |
> >> -(GAHBCFG_HBSTLEN_INCR4 <<
> >> GAHBCFG_HBSTLEN_SHIFT),
> >> + writel(GAHBCFG_GLBL_INTR_EN |
> >> GAHBCFG_DMA_EN | val,
> >> hsotg->regs + GAHBCFG);
> >> else
> >> writel(((hsotg->dedicated_fifos) ?
> >> (GAHBCFG_NP_TXF_EMP_LVL |
> >
> > There are other bits in GAHBCFG that can be set from platform. They will be
> preserved by your patch, as they are not part of GAHBCFG_CTRL_MASK, but
> only in case dma is enabled. Perhaps preserve them in non-dma case as well.
>
> Here may have issue if also set hsotg->core_params->ahbcfg for non-dma case,
> since GAHBCFG[4:1] may be set.
You can mask off HBstLen in that case. However, I don't think setting burst
length will be an issue in non DMA case as DWC2 will not act as a bus master.
John, can you please confirm if setting burst length will be an issue in
non-dma case?
>
> Though from drivers/usb/dwc2/core.h we can not see @ahbcfg is specifically
> used for dma case, most case in drivers/usb/dwc2/platform.c use ahbcfg is set
> hbstlen, GAHBCFG[4:1].
> For example, our platform set GAHBCFG_HBSTLEN_INCR16.
>
> So I just assume @ahbcfg is used for dma case.
> What do you think.
While you are fixing it, why not fix it for other bits, for example AHBSingle,
InvDescEndianness etc., which are part of the same register and will be
overwritten at the same place.
>
> Thanks
BR,
Yousaf
RE: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from platform
> -Original Message-
> From: Zhangfei Gao [mailto:zhangfei@linaro.org]
> Sent: Wednesday, February 4, 2015 9:01 AM
> To: Kaukab, Yousaf; ba...@ti.com; john.y...@synopsys.com
> Cc: linux-usb@vger.kernel.org; Zhangfei Gao
> Subject: [PATCH 2/2] usb: dwc2: gadget reuse ahbcfg assigned from platform
>
> Gadget directly set GAHBCFG_HBSTLEN_INCR4, reuse ahbcfg if assigned from
> platform
>
> Signed-off-by: Zhangfei Gao
> ---
> drivers/usb/dwc2/core.c | 2 +-
> drivers/usb/dwc2/gadget.c | 8 ++--
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c index
> d5197d4..8d388cc 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -2563,7 +2563,7 @@ void dwc2_set_param_reload_ctl(struct dwc2_hsotg
> *hsotg, int val)
>
> void dwc2_set_param_ahbcfg(struct dwc2_hsotg *hsotg, int val) {
> - if (val != -1)
> + if (val)
> hsotg->core_params->ahbcfg = val;
> else
> hsotg->core_params->ahbcfg =
> GAHBCFG_HBSTLEN_INCR4 << diff --git a/drivers/usb/dwc2/gadget.c
> b/drivers/usb/dwc2/gadget.c index 15aa578..20085de 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2314,9 +2314,13 @@ void s3c_hsotg_core_init_disconnected(struct
> dwc2_hsotg *hsotg,
> GINTSTS_USBSUSP | GINTSTS_WKUPINT,
> hsotg->regs + GINTMSK);
>
> + if ((hsotg->core_params) && (hsotg->core_params->ahbcfg != -
> 1))
> + val = hsotg->core_params->ahbcfg &
> ~GAHBCFG_CTRL_MASK;
> + else
> + val = GAHBCFG_HBSTLEN_INCR4 <<
> GAHBCFG_HBSTLEN_SHIFT;
> +
> if (using_dma(hsotg))
> - writel(GAHBCFG_GLBL_INTR_EN |
> GAHBCFG_DMA_EN |
> -(GAHBCFG_HBSTLEN_INCR4 <<
> GAHBCFG_HBSTLEN_SHIFT),
> + writel(GAHBCFG_GLBL_INTR_EN |
> GAHBCFG_DMA_EN | val,
> hsotg->regs + GAHBCFG);
> else
> writel(((hsotg->dedicated_fifos) ?
> (GAHBCFG_NP_TXF_EMP_LVL |
There are other bits in GAHBCFG that can be set from platform. They will be
preserved by your patch, as they are not part of GAHBCFG_CTRL_MASK, but only in
case dma is enabled. Perhaps preserve them in non-dma case as well.
> --
> 1.9.1
BR,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH RESEND] usb: dwc2: Fix a bug in reading the endpoint directions from reg.
Hi Roshan,
> -Original Message-
> From: Roshan Pius [mailto:rp...@chromium.org]
> Sent: Monday, February 2, 2015 11:56 PM
> To: linux-usb@vger.kernel.org
> Cc: benc...@chromium.org; Kaukab, Yousaf; Roshan Pius
> Subject: [PATCH RESEND] usb: dwc2: Fix a bug in reading the endpoint
> directions from reg.
>
> According to the DWC2 datasheet, the HWCFG1 register stores the configured
> endpoint directions for endpoints 0-15 in bit positions 0-31.
> ==
> Endpoint Direction (EpDir)
> This 32-bit field uses two bits per endpoint to determine the endpoint
> direction.
> Endpoint
> Bits [31:30]: Endpoint 15 direction
> Bits [29:28]: Endpoint 14 direction
>
> Bits [3:2]: Endpoint 1 direction
> Bits[1:0]: Endpoint 0 direction (always BIDIR) ==
>
> The DWC2 driver is currently interpreting the contents of the register as
> directions for endpoints 1-15 which leads to an error in determining the
> configured endpoint directions in the core because the first 2 bits determine
> the direction of endpoint 0 and not 1.
>
> This is based on testing/next branch in Felipe's git.
>
> Signed-off-by: Roshan Pius
> ---
> drivers/usb/dwc2/gadget.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index
> 50ae096..706165c 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3167,7 +3167,7 @@ static int s3c_hsotg_hw_cfg(struct dwc2_hsotg
> *hsotg)
> hsotg->eps_out[0] = hsotg->eps_in[0];
>
> cfg = readl(hsotg->regs + GHWCFG1);
> - for (i = 1; i < hsotg->num_of_eps; i++, cfg >>= 2) {
> + for (i = 1, cfg >>= 2; i < hsotg->num_of_eps; i++, cfg >>= 2) {
Nice catch! I wouldn't have found it on my hardware as GHWCFG1 is always 0.
> ep_type = cfg & 3;
> /* Direction in or both */
> if (!(ep_type & 2)) {
> --
> 2.2.0.rc0.207.ga3a616c
BR,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1 01/13] usb: dwc2: host: register hcd handle to the phy
> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: Thursday, January 29, 2015 3:25 AM
> To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com;
> john.y...@synopsys.com
> Cc: Herrero, Gregory; r.bald...@samsung.com;
> dingu...@opensource.altera.com; sergei.shtyl...@cogentembedded.com
> Subject: RE: [PATCH v1 01/13] usb: dwc2: host: register hcd handle to the phy
>
> > From: Mian Yousaf Kaukab [mailto:yousaf.kau...@intel.com]
> > Sent: Wednesday, January 21, 2015 6:37 AM
> >
> > From: Gregory Herrero
> >
> > If phy driver is present, register hcd handle to it and let it take
> > care of calling usb_add_hcd. Otherwise, add hcd here.
> >
> > Moreover, save irq number so that it can be used to call usb_add_hcd.
> >
> > Signed-off-by: Gregory Herrero
> > ---
> > drivers/usb/dwc2/hcd.c | 35 ++-
> > 1 file changed, 30 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index
> > 755e16b..4375d4f 100644
> > --- a/drivers/usb/dwc2/hcd.c
> > +++ b/drivers/usb/dwc2/hcd.c
> > @@ -2779,6 +2779,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int
> > irq,
> > u32 hcfg;
> > int i, num_channels;
> > int retval;
> > + bool add_host = true;
> >
> > if (usb_disabled())
> > return -ENODEV;
> > @@ -2935,14 +2936,30 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg,
> > int irq,
> > /* Don't support SG list at this point */
> > hcd->self.sg_tablesize = 0;
> >
> > + /* Save irq number */
> > + hcd->irq = irq;
> > +
> > /*
> > * Finish generic HCD initialization and start the HCD. This
> function
> > * allocates the DMA buffer pool, registers the USB bus, requests
> the
> > * IRQ line, and calls hcd_start method.
> > +* If a phy driver is present, let it handle the hcd initialization.
> > */
> > - retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
> > - if (retval < 0)
> > - goto error3;
> > + if (!IS_ERR_OR_NULL(hsotg->uphy)) {
> > + retval = otg_set_host(hsotg->uphy->otg, &hcd-
> >self);
> > + if (retval) {
> > + if (retval != -ENOTSUPP)
> > + goto error3;
> > + } else {
> > + add_host = false;
> > + }
> > + }
> > +
> > + if (add_host) {
> > + retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
> > + if (retval)
> > + goto error3;
> > + }
> >
> > device_wakeup_enable(hcd->self.controller);
> >
> > @@ -2976,7 +2993,8 @@ EXPORT_SYMBOL_GPL(dwc2_hcd_init); void
> > dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {
> > struct usb_hcd *hcd;
> > -
> > + bool remove_host = true;
> > + int retval;
> > dev_dbg(hsotg->dev, "DWC OTG HCD REMOVE\n");
> >
> > hcd = dwc2_hsotg_to_hcd(hsotg);
> > @@ -2988,7 +3006,14 @@ void dwc2_hcd_remove(struct dwc2_hsotg
> > *hsotg)
> > return;
> > }
> >
> > - usb_remove_hcd(hcd);
> > + if (!IS_ERR_OR_NULL(hsotg->uphy)) {
> > + retval = otg_set_host(hsotg->uphy->otg, NULL);
> > + if (!retval)
> > + remove_host = false;
> > + }
> > +
> > + if (remove_host)
> > + usb_remove_hcd(hcd);
> > hsotg->priv = NULL;
> > dwc2_hcd_release(hsotg);
> > usb_put_hcd(hcd);
> > --
> > 1.9.1
>
> Hi Yousaf,
Hi John,
>
> This patch seems to break host-mode on the Altera platform.
>
> When it comes up as an A-Host, the HCD doesn't get loaded. When it comes up
> as a B-Peripheral, it gets the connector ID status change interrupt, but hangs
> the system.
Thank you testing this patchset. Can you point me to the phy driver used by
your platform?
I will take a look what's going on.
I can drop this patch for now. Are you ok with rest of the patches in this set?
If so, can you send your conditional ACK for them? May be there is still time
to get them queued for v3.20.
>
> Dinh,
>
> Have you tried this on your platform yet? Do you see the same thing?
>
> Thanks,
> John
>
>
BR,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1 00/13] usb: second series of updates for dwc2 driver
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Tuesday, January 27, 2015 4:31 PM > To: John Youn > Cc: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; Herrero, > Gregory; r.bald...@samsung.com; dingu...@opensource.altera.com; > sergei.shtyl...@cogentembedded.com > Subject: Re: [PATCH v1 00/13] usb: second series of updates for dwc2 driver > > On Thu, Jan 22, 2015 at 02:28:33AM +, John Youn wrote: > > > From: Mian Yousaf Kaukab [mailto:yousaf.kau...@intel.com] > > > Sent: Wednesday, January 21, 2015 6:37 AM > > > > > > Hi, > > > This patchset consists of some bug fixes, feature enhancements and > > > cosmetic changes for the dwc2 driver. All the patches are verified > > > on > > > dwc2 v3.0a with dedicated fifos. Main focus of testing was with dma > > > enabled. Although basic testing without dma was also done. > > > > > > This is based on testing/next branch in Felipe's git. > > > > > > Removed Paul Zimmerman from CC as his Synopsys address is not valid > > > any longer. > > > > > > Thank you, > > > > > > Best regards, > > > Yousaf > > > > > > History: > > > v1: > > > - Fixed comments from Sergei Shtylyov and Robert Baldyga > > > > > > Gregory Herrero (8): > > > usb: dwc2: host: register hcd handle to the phy > > > usb: dwc2: host: resume root hub on remote wakeup > > > usb: dwc2: gadget: fix clear halt feature handling > > > usb: dwc2: gadget: add TEST_MODE feature support > > > usb: dwc2: gadget: fix a typo in comment > > > usb: dwc2: gadget: add reset flag in init function > > > usb: dwc2: gadget: don't modify pullup status during reset > > > usb: dwc2: gadget: initialize controller in pullup callback > > > > > > Mian Yousaf Kaukab (5): > > > usb: dwc2: gadget: remove hardcoded if (0) and if (1) checks > > > usb: dwc2: gadget: add unaligned buffers support > > > usb: dwc2: gadget: fix debug message for zlp > > > usb: dwc2: gadget: fix phy interface configuration > > > usb: dwc2: gadget: replace constants with defines > > > > > > drivers/usb/dwc2/core.h | 12 +- > > > drivers/usb/dwc2/gadget.c | 359 > > > ++ > > > drivers/usb/dwc2/hcd.c| 43 -- > > > drivers/usb/dwc2/hw.h | 1 + > > > 4 files changed, 349 insertions(+), 66 deletions(-) > > > > > > -- > > > 1.9.1 > > > > > > Hi Yousaf, > > > > I'm working on testing this series now on our platform. Please give me > > a couple days as I am still getting up to speed. > > there's very little time to get this in v3.20 merge window, we might have to > delay to 3.21. I prefer that you test this properly then us pushing something > that > causes regressiosn :-s I would prefer if we still can get it in for v3.20. I have more patches planned for v3.21. This series has been verified on dwc2 v3.00a and also for the gadget parts Tested-by: Robert Baldyga Who has dwc2 v2.81a. > > -- > balbi BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 00/13] usb: second series of updates for dwc2 driver
Hi John, > -Original Message- > From: Kaukab, Yousaf > Sent: Thursday, January 15, 2015 6:09 PM > To: linux-usb@vger.kernel.org; ba...@ti.com > Cc: john.y...@synopsys.com; Herrero, Gregory; pa...@synopsys.com; > r.bald...@samsung.com; dingu...@opensource.altera.com; Kaukab, Yousaf > Subject: [PATCH 00/13] usb: second series of updates for dwc2 driver > > Hi, > This patchset consists of some bug fixes, feature enhancements and cosmetic > changes for the dwc2 driver. All the patches are verified on > dwc2 v3.0a with dedicated fifos. Main focus of testing was with dma enabled. > Although basic testing without dma was also done. > > This is based on testing/next branch in Felipe's git. > > Thank you, > > Best regards, > Yousaf > > Gregory Herrero (8): > usb: dwc2: host: register hcd handle to the phy > usb: dwc2: host: resume root hub on remote wakeup > usb: dwc2: gadget: fix clear halt feature handling > usb: dwc2: gadget: add TEST_MODE feature support > usb: dwc2: gadget: fix a typo in comment > usb: dwc2: gadget: add reset flag in init function > usb: dwc2: gadget: don't modify pullup status during reset > usb: dwc2: gadget: initialize controller in pullup callback > > Mian Yousaf Kaukab (5): > usb: dwc2: gadget: remove hardcoded if (0) and if (1) checks > usb: dwc2: gadget: add unaligned buffers support > usb: dwc2: gadget: fix debug message for zlp > usb: dwc2: gadget: fix phy interface configuration > usb: dwc2: gadget: replace constants with defines > > drivers/usb/dwc2/core.h | 12 +- > drivers/usb/dwc2/gadget.c | 358 > ++ > drivers/usb/dwc2/hcd.c| 43 -- > drivers/usb/dwc2/hw.h | 1 + > 4 files changed, 348 insertions(+), 66 deletions(-) > Did you get a chance to review this patchset? Can you please ACK it if you don't have any objections? BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 03/13] usb: dwc2: gadget: fix clear halt feature handling
> -Original Message- > From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com] > Sent: Thursday, January 15, 2015 7:21 PM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com > Cc: john.y...@synopsys.com; Herrero, Gregory; pa...@synopsys.com; > r.bald...@samsung.com; dingu...@opensource.altera.com > Subject: Re: [PATCH 03/13] usb: dwc2: gadget: fix clear halt feature handling > > Hello. Hi, Thank you for reviewing these patches! > > On 01/15/2015 08:09 PM, Mian Yousaf Kaukab wrote: > > > From: Gregory Herrero > > > When clearing HALT on an endpoint, in progress requests must be called > > with locks off. > > Requests must be *called*? s/request/request->complete > > > Request must be started only if there is not already a pending request > > on the endpoint caused by previous completion. > > > Signed-off-by: Gregory Herrero > > [...] > > WBR, Sergei BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 00/30] usb: updates for dwc2 gadget driver
Hi Robert, > -Original Message- > From: Robert Baldyga [mailto:r.bald...@samsung.com] > Sent: Monday, January 12, 2015 11:00 AM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; pa...@synopsys.com; > sergei.shtyl...@cogentembedded.com; dingu...@opensource.altera.com > Subject: Re: [PATCH v3 00/30] usb: updates for dwc2 gadget driver > > On 01/09/2015 01:38 PM, Mian Yousaf Kaukab wrote: > > Hi, > > This patchset consists of various bug fixes and feature enhancements > > for the > > dwc2 gadget driver. All the patches are verified on dwc2 v3.0a with > > dedicated fifos. Main focus of testing was with dma enabled. Although > > basic testing without dma was also done. > > > > It is based on testing/next branch in Felipe's git and > > > > Tested-by: Dinh Nguyen > > > > > I still see problem described here [1] but it seems to be unrelated to this > patch > series. Tested with USB functions present in mainline kernel it works well. > > Tested-by: Robert Baldyga Thank you for testing this patchset. Can you please provide the version of dwc2 ip on which you tested? > > [1] https://lkml.org/lkml/2014/12/22/185 > > Thanks, > Robert Baldyga BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 03/30] usb: dwc2: gadget: don't process XferCompl on setup packet
Hi, > -Original Message- > From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com] > Sent: Friday, January 2, 2015 6:09 PM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; pa...@synopsys.com; r.bald...@samsung.com > Subject: Re: [PATCH v2 03/30] usb: dwc2: gadget: don't process XferCompl on > setup packet > > Hello. > > On 1/2/2015 5:42 PM, Mian Yousaf Kaukab wrote: > > > Only process DOEPINT.XferCompl on data packet as DOEPINTn.SetUp can > > occur with or without DOEPINT.XferCompl. When DOEPINT.SetUp occurs > > with DOEPINT.XferCompl, only DOEPINT.SetUp needs to be handled. > > > Moreover, ignore DOEPINT.XferCompl when it occurs with > > DOEPINT.StupPktRcvd as driver needs to wait for DOEPINT.SetUp to > > continue. > > > Signed-off-by: Mian Yousaf Kaukab > > --- > > drivers/usb/dwc2/gadget.c | 4 > > drivers/usb/dwc2/hw.h | 1 + > > 2 files changed, 5 insertions(+) > > > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > > index 843f3ee..e190d68 100644 > > --- a/drivers/usb/dwc2/gadget.c > > +++ b/drivers/usb/dwc2/gadget.c > > @@ -1810,6 +1810,10 @@ static void s3c_hsotg_epint(struct dwc2_hsotg > *hsotg, unsigned int idx, > > dev_dbg(hsotg->dev, "%s: ep%d(%s) DxEPINT=0x%08x\n", > > __func__, idx, dir_in ? "in" : "out", ints); > > > > + /* Don't process XferCompl interrupt if it is a setup packet */ > > + if (ints & DXEPINT_SETUP || ints & DXEPINT_SETUP_RCVD) > > if (ints & (DXEPINT_SETUP | DXEPINT_SETUP_RCVD)) Sure. In a rare case, DXEPINT_SETUP_RCVD was also set for a non-control endpoint. So I will modify the check to following: if (idx == 0 && (ints & (DXEPINT_SETUP | DXEPINT_SETUP_RCVD))) > > [...] > > WBR, Sergei BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 00/30] usb: updates for dwc2 gadget driver
> -Original Message- > From: Dinh Nguyen [mailto:dingu...@opensource.altera.com] > Sent: Thursday, January 8, 2015 5:34 PM > To: Kaukab, Yousaf; 'Paul Zimmerman'; 'linux-usb@vger.kernel.org'; > 'ba...@ti.com' > Cc: Herrero, Gregory; 'sergei.shtyl...@cogentembedded.com'; > 'r.bald...@samsung.com' > Subject: Re: [PATCH v2 00/30] usb: updates for dwc2 gadget driver > > Hi Yousaf, > > On 01/08/2015 08:27 AM, Kaukab, Yousaf wrote: > > Hi, > > > >> -Original Message- > >> From: Kaukab, Yousaf > >> Sent: Wednesday, January 7, 2015 7:55 PM > >> To: Dinh Nguyen; Paul Zimmerman; linux-usb@vger.kernel.org; > >> ba...@ti.com > >> Cc: Herrero, Gregory; sergei.shtyl...@cogentembedded.com; > >> r.bald...@samsung.com > >> Subject: RE: [PATCH v2 00/30] usb: updates for dwc2 gadget driver > >> > >> Hi Dinh, > >> > >>> -Original Message- > >>> From: Dinh Nguyen [mailto:dingu...@opensource.altera.com] > >>> Sent: Wednesday, January 7, 2015 5:26 PM > >>> To: Kaukab, Yousaf; Paul Zimmerman; linux-usb@vger.kernel.org; > >>> ba...@ti.com > >>> Cc: Herrero, Gregory; sergei.shtyl...@cogentembedded.com; > >>> r.bald...@samsung.com > >>> Subject: Re: [PATCH v2 00/30] usb: updates for dwc2 gadget driver > >>> > >>> Hi Yousaf, > >>> > >>> > >>> I've also attached a text file of the log. > >>> > >>> > >>> > >>> root@socfpga_cyclone5:~# insmod g_mass_storage.ko > >>> file=/dev/mmcblk0p1 [ 144.084933] Number of LUNs=8 [ 144.087817] > >>> Mass Storage Function, > >> version: > >>> 2009/09/11 [ 144.092936] LUN: removable file: (no medium) [ > >>> 144.097228] Number of LUNs=1 [ 144.100217] LUN: file: > >>> /dev/mmcblk0p1 [ 144.103867] Number of LUNs=1 [ 144.106941] > g_mass_storage gadget: > >>> Mass Storage Gadget, version: > >>> 2009/09/11 > >>> [ 144.113874] g_mass_storage gadget: userspace failed to provide > >>> iSerialNumber [ 144.120922] g_mass_storage gadget: g_mass_storage > >>> ready > >>> > >>> [ 144.129387] dwc2 ffb4.usb: bound driver g_mass_storage > >>> > >>> root@socfpga_cyclone5:~# > >>> > >>> root@socfpga_cyclone5:~# [ 144.353610] dwc2 ffb4.usb: new > >>> device is high-speed [ 144.527612] dwc2 ffb4.usb: new device is > >>> high-speed > >>> > >>> [ 149.595802] [ cut here ] > >>> > >>> [ 149.600420] WARNING: CPU: 0 PID: 0 at > >>> drivers/usb/dwc2/gadget.c:1352 > >>> s3c_hsotg_rx_data+0xcc/0x144() > >>> [ 149.609423] Modules linked in: g_mass_storage usb_f_mass_storage > >>> libcomposite [ 149.616582] CPU: 0 PID: 0 Comm: swapper/0 Not > >>> tainted > >>> 3.19.0-rc1-00090-g4864f14 #14 > >>> [ 149.624202] Hardware name: Altera SOCFPGA > >>> > >>> [ 149.628222] [] (unwind_backtrace) from [] > >>> (show_stack+0x20/0x24) > >>> [ 149.635941] [] (show_stack) from [] > >>> (dump_stack+0x7c/0x98) > >>> [ 149.643147] [] (dump_stack) from [] > >>> (warn_slowpath_common+0x88/0xc4) > >>> [ 149.651208] [] (warn_slowpath_common) from [] > >>> (warn_slowpath_null+0x2c/0x34) > >>> [ 149.659961] [] (warn_slowpath_null) from [] > >>> (s3c_hsotg_rx_data+0xcc/0x144) > >>> [ 149.668542] [] (s3c_hsotg_rx_data) from [] > >>> (s3c_hsotg_irq+0x3e0/0x708) > >>> [ 149.676777] [] (s3c_hsotg_irq) from [] > >>> (handle_irq_event_percpu+0x68/0x204) > >>> [ 149.685442] [] (handle_irq_event_percpu) from > >>> [] > >>> (handle_irq_event+0x54/0x74) > >>> [ 149.694280] [] (handle_irq_event) from [] > >>> (handle_fasteoi_irq+0xb8/0x190) > >>> [ 149.702771] [] (handle_fasteoi_irq) from [] > >>> (generic_handle_irq+0x30/0x40) > >>> [ 149.711348] [] (generic_handle_irq) from [] > >>> (__handle_domain_irq+0x64/0xc4) > >>> [ 149.720012] [] (__handle_domain_irq) from [] > >>> (gic_handle_irq+0x30/0x6c) > >>> [ 149.728333] [] (gic_handle_irq) from [] > >>> (__irq_svc+0x40/0x54) > >>> [ 149.735781] Exception stack(0xc06fbf38 to 0xc06fbf80) [ > >>> 149.740811] > >> bf
RE: [PATCH v2 00/30] usb: updates for dwc2 gadget driver
Hi, > -Original Message- > From: Kaukab, Yousaf > Sent: Wednesday, January 7, 2015 7:55 PM > To: Dinh Nguyen; Paul Zimmerman; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; sergei.shtyl...@cogentembedded.com; > r.bald...@samsung.com > Subject: RE: [PATCH v2 00/30] usb: updates for dwc2 gadget driver > > Hi Dinh, > > > -Original Message- > > From: Dinh Nguyen [mailto:dingu...@opensource.altera.com] > > Sent: Wednesday, January 7, 2015 5:26 PM > > To: Kaukab, Yousaf; Paul Zimmerman; linux-usb@vger.kernel.org; > > ba...@ti.com > > Cc: Herrero, Gregory; sergei.shtyl...@cogentembedded.com; > > r.bald...@samsung.com > > Subject: Re: [PATCH v2 00/30] usb: updates for dwc2 gadget driver > > > > Hi Yousaf, > > > > > > I've also attached a text file of the log. > > > > > > > > root@socfpga_cyclone5:~# insmod g_mass_storage.ko file=/dev/mmcblk0p1 > > [ 144.084933] Number of LUNs=8 [ 144.087817] Mass Storage Function, > version: > > 2009/09/11 [ 144.092936] LUN: removable file: (no medium) [ > > 144.097228] Number of LUNs=1 [ 144.100217] LUN: file: /dev/mmcblk0p1 > > [ 144.103867] Number of LUNs=1 [ 144.106941] g_mass_storage gadget: > > Mass Storage Gadget, version: > > 2009/09/11 > > [ 144.113874] g_mass_storage gadget: userspace failed to provide > > iSerialNumber [ 144.120922] g_mass_storage gadget: g_mass_storage > > ready > > > > [ 144.129387] dwc2 ffb4.usb: bound driver g_mass_storage > > > > root@socfpga_cyclone5:~# > > > > root@socfpga_cyclone5:~# [ 144.353610] dwc2 ffb4.usb: new device > > is high-speed [ 144.527612] dwc2 ffb4.usb: new device is > > high-speed > > > > [ 149.595802] [ cut here ] > > > > [ 149.600420] WARNING: CPU: 0 PID: 0 at > > drivers/usb/dwc2/gadget.c:1352 > > s3c_hsotg_rx_data+0xcc/0x144() > > [ 149.609423] Modules linked in: g_mass_storage usb_f_mass_storage > > libcomposite [ 149.616582] CPU: 0 PID: 0 Comm: swapper/0 Not tainted > > 3.19.0-rc1-00090-g4864f14 #14 > > [ 149.624202] Hardware name: Altera SOCFPGA > > > > [ 149.628222] [] (unwind_backtrace) from [] > > (show_stack+0x20/0x24) > > [ 149.635941] [] (show_stack) from [] > > (dump_stack+0x7c/0x98) > > [ 149.643147] [] (dump_stack) from [] > > (warn_slowpath_common+0x88/0xc4) > > [ 149.651208] [] (warn_slowpath_common) from [] > > (warn_slowpath_null+0x2c/0x34) > > [ 149.659961] [] (warn_slowpath_null) from [] > > (s3c_hsotg_rx_data+0xcc/0x144) > > [ 149.668542] [] (s3c_hsotg_rx_data) from [] > > (s3c_hsotg_irq+0x3e0/0x708) > > [ 149.676777] [] (s3c_hsotg_irq) from [] > > (handle_irq_event_percpu+0x68/0x204) > > [ 149.685442] [] (handle_irq_event_percpu) from > > [] > > (handle_irq_event+0x54/0x74) > > [ 149.694280] [] (handle_irq_event) from [] > > (handle_fasteoi_irq+0xb8/0x190) > > [ 149.702771] [] (handle_fasteoi_irq) from [] > > (generic_handle_irq+0x30/0x40) > > [ 149.711348] [] (generic_handle_irq) from [] > > (__handle_domain_irq+0x64/0xc4) > > [ 149.720012] [] (__handle_domain_irq) from [] > > (gic_handle_irq+0x30/0x6c) > > [ 149.728333] [] (gic_handle_irq) from [] > > (__irq_svc+0x40/0x54) > > [ 149.735781] Exception stack(0xc06fbf38 to 0xc06fbf80) [ 149.740811] > bf20: > > > > [ 149.748953] bf40: c06fbf90 c001f600 c0702c60 c0733560 c0702498 > > c04cf08c > > c0732d73 413fc090 [ 149.757096] bf60: c0733560 c06fbf8c c06fbf90 > > c06fbf80 > > c000f934 > > c000f938 6013 > > [ 149.765243] [] (__irq_svc) from [] > > (arch_cpu_idle+0x40/0x4c) > > [ 149.772616] [] (arch_cpu_idle) from [] > > (cpu_startup_entry+0x144/0x22c) > > [ 149.780857] [] (cpu_startup_entry) from [] > > (rest_init+0x70/0x88) > > [ 149.788581] [] (rest_init) from [] > > (start_kernel+0x310/0x368) > > [ 149.796030] ---[ end trace 05bdb288fcc8864d ]--- [ 155.793924] random: > > nonblocking pool is initialized > > > > I don't see any debug messages. Did you enable CONFIG_USB_DWC2_DEBUG? > If so, then perhaps you need to set the log level in your console > (/proc/sys/kernel/printk)? > > What about Values of following registers GSNPSID, GHWCFG1-GHWCFG4? Is it > possible for you to print them as well? > Without the debug log it's difficult to conclude why you are seeing this behavior. One probable reason could be that your revision of the dwc2 core does not generate GRXSTS_PKTSTS_OUTDONE on setup packets.
RE: [PATCH v2 00/30] usb: updates for dwc2 gadget driver
Hi Dinh, > -Original Message- > From: Dinh Nguyen [mailto:dingu...@opensource.altera.com] > Sent: Wednesday, January 7, 2015 5:26 PM > To: Kaukab, Yousaf; Paul Zimmerman; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; sergei.shtyl...@cogentembedded.com; > r.bald...@samsung.com > Subject: Re: [PATCH v2 00/30] usb: updates for dwc2 gadget driver > > Hi Yousaf, > > > I've also attached a text file of the log. > > > > root@socfpga_cyclone5:~# insmod g_mass_storage.ko file=/dev/mmcblk0p1 [ > 144.084933] Number of LUNs=8 [ 144.087817] Mass Storage Function, version: > 2009/09/11 [ 144.092936] LUN: removable file: (no medium) [ 144.097228] > Number of LUNs=1 [ 144.100217] LUN: file: /dev/mmcblk0p1 [ 144.103867] > Number of LUNs=1 [ 144.106941] g_mass_storage gadget: Mass Storage > Gadget, version: > 2009/09/11 > [ 144.113874] g_mass_storage gadget: userspace failed to provide > iSerialNumber [ 144.120922] g_mass_storage gadget: g_mass_storage ready > > [ 144.129387] dwc2 ffb4.usb: bound driver g_mass_storage > > root@socfpga_cyclone5:~# > > root@socfpga_cyclone5:~# [ 144.353610] dwc2 ffb4.usb: new device is > high-speed [ 144.527612] dwc2 ffb4.usb: new device is high-speed > > [ 149.595802] [ cut here ] > > [ 149.600420] WARNING: CPU: 0 PID: 0 at drivers/usb/dwc2/gadget.c:1352 > s3c_hsotg_rx_data+0xcc/0x144() > [ 149.609423] Modules linked in: g_mass_storage usb_f_mass_storage > libcomposite [ 149.616582] CPU: 0 PID: 0 Comm: swapper/0 Not tainted > 3.19.0-rc1-00090-g4864f14 #14 > [ 149.624202] Hardware name: Altera SOCFPGA > > [ 149.628222] [] (unwind_backtrace) from [] > (show_stack+0x20/0x24) > [ 149.635941] [] (show_stack) from [] > (dump_stack+0x7c/0x98) > [ 149.643147] [] (dump_stack) from [] > (warn_slowpath_common+0x88/0xc4) > [ 149.651208] [] (warn_slowpath_common) from [] > (warn_slowpath_null+0x2c/0x34) > [ 149.659961] [] (warn_slowpath_null) from [] > (s3c_hsotg_rx_data+0xcc/0x144) > [ 149.668542] [] (s3c_hsotg_rx_data) from [] > (s3c_hsotg_irq+0x3e0/0x708) > [ 149.676777] [] (s3c_hsotg_irq) from [] > (handle_irq_event_percpu+0x68/0x204) > [ 149.685442] [] (handle_irq_event_percpu) from [] > (handle_irq_event+0x54/0x74) > [ 149.694280] [] (handle_irq_event) from [] > (handle_fasteoi_irq+0xb8/0x190) > [ 149.702771] [] (handle_fasteoi_irq) from [] > (generic_handle_irq+0x30/0x40) > [ 149.711348] [] (generic_handle_irq) from [] > (__handle_domain_irq+0x64/0xc4) > [ 149.720012] [] (__handle_domain_irq) from [] > (gic_handle_irq+0x30/0x6c) > [ 149.728333] [] (gic_handle_irq) from [] > (__irq_svc+0x40/0x54) > [ 149.735781] Exception stack(0xc06fbf38 to 0xc06fbf80) [ 149.740811] bf20: > > [ 149.748953] bf40: c06fbf90 c001f600 c0702c60 c0733560 c0702498 c04cf08c > c0732d73 413fc090 [ 149.757096] bf60: c0733560 c06fbf8c c06fbf90 c06fbf80 > c000f934 > c000f938 6013 > [ 149.765243] [] (__irq_svc) from [] > (arch_cpu_idle+0x40/0x4c) > [ 149.772616] [] (arch_cpu_idle) from [] > (cpu_startup_entry+0x144/0x22c) > [ 149.780857] [] (cpu_startup_entry) from [] > (rest_init+0x70/0x88) > [ 149.788581] [] (rest_init) from [] > (start_kernel+0x310/0x368) > [ 149.796030] ---[ end trace 05bdb288fcc8864d ]--- [ 155.793924] random: > nonblocking pool is initialized > I don't see any debug messages. Did you enable CONFIG_USB_DWC2_DEBUG? If so, then perhaps you need to set the log level in your console (/proc/sys/kernel/printk)? What about Values of following registers GSNPSID, GHWCFG1-GHWCFG4? Is it possible for you to print them as well? BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 00/30] usb: updates for dwc2 gadget driver
> -Original Message- > From: Dinh Nguyen [mailto:dingu...@opensource.altera.com] > Sent: Wednesday, January 7, 2015 4:39 PM > To: Kaukab, Yousaf; Paul Zimmerman; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; sergei.shtyl...@cogentembedded.com; > r.bald...@samsung.com > Subject: Re: [PATCH v2 00/30] usb: updates for dwc2 gadget driver > > On 01/07/2015 09:34 AM, Kaukab, Yousaf wrote: > > Hi, > > > >> -Original Message- > >> From: Dinh Nguyen [mailto:dingu...@opensource.altera.com] > >> Sent: Wednesday, January 7, 2015 1:01 AM > >> To: Paul Zimmerman; Kaukab, Yousaf; linux-usb@vger.kernel.org; > ba...@ti.com > >> Cc: Herrero, Gregory; sergei.shtyl...@cogentembedded.com; > >> r.bald...@samsung.com > >> Subject: Re: [PATCH v2 00/30] usb: updates for dwc2 gadget driver > >> > >> On 01/06/2015 05:47 PM, Dinh Nguyen wrote: > >>> On 01/05/2015 01:16 PM, Paul Zimmerman wrote: > >>>> Adding Dinh to CC. > >>>> > >>>> Robert, Dinh, I would like to have your tested-bys for this series, > >>>> please. > >>>> > >>> > >>> This patch series is producing this error on the SOCFPGA platform. > >>> I'll try to bisect to a specific patch. > >>> > >>> root@socfpga_cyclone5:~# [ 47.929743] dwc2 ffb4.usb: new device is > >>> low-speed > >>> [ 63.873596] dwc2 ffb4.usb: new device is low-speed > >>> [ 64.219220] dwc2 ffb4.usb: new device is low-speed > >>> [ 64.421495] dwc2 ffb4.usb: new device is low-speed > >>> [ 64.834505] dwc2 ffb4.usb: new device is high-speed > >>> [ 69.899695] [ cut here ] > >>> [ 69.904315] WARNING: CPU: 0 PID: 0 at drivers/usb/dwc2/gadget.c:1352 > >>> s3c_hsotg_rx_data+0xc0/0xd4() > >>> [ 69.913230] Modules linked in: g_mass_storage usb_f_mass_storage > >>> libcomposite > >>> [ 69.920385] CPU: 0 PID: 0 Comm: swapper/0 Not tainted > >>> 3.19.0-rc1-00090-g4864f14 #4 > >>> [ 69.927917] Hardware name: Altera SOCFPGA > >>> [ 69.931936] [] (unwind_backtrace) from [] > >>> (show_stack+0x10/0x14) > >>> [ 69.939654] [] (show_stack) from [] > >>> (dump_stack+0x74/0x90) > >>> [ 69.946857] [] (dump_stack) from [] > >>> (warn_slowpath_common+0x78/0xb4) > >>> [ 69.954916] [] (warn_slowpath_common) from [] > >>> (warn_slowpath_null+0x1c/0x24) > >>> [ 69.963669] [] (warn_slowpath_null) from [] > >>> (s3c_hsotg_rx_data+0xc0/0xd4) > >>> [ 69.972162] [] (s3c_hsotg_rx_data) from [] > >>> (s3c_hsotg_irq+0x454/0x520) > >>> [ 69.980394] [] (s3c_hsotg_irq) from [] > >>> (handle_irq_event_percpu+0x5c/0x1f4) > >>> [ 69.989056] [] (handle_irq_event_percpu) from [] > >>> (handle_irq_event+0x44/0x64) > >>> [ 69.997892] [] (handle_irq_event) from [] > >>> (handle_fasteoi_irq+0xa8/0x180) > >>> [ 70.006382] [] (handle_fasteoi_irq) from [] > >>> (generic_handle_irq+0x20/0x30) > >>> [ 70.014958] [] (generic_handle_irq) from [] > >>> (__handle_domain_irq+0x54/0xb4) > >>> [ 70.023619] [] (__handle_domain_irq) from [] > >>> (gic_handle_irq+0x20/0x5c) > >>> [ 70.031937] [] (gic_handle_irq) from [] > >>> (__irq_svc+0x40/0x54) > >>> [ 70.039384] Exception stack(0xc0635f60 to 0xc0635fa8) > >>> [ 70.044415] 5f60: c0635fb0 c001cee0 c066cca0 > >>> c063c498 c046908c c066c4b2 > >>> [ 70.052556] 5f80: c066cca0 0001 ef7fccc0 0100 > >>> c0635fa8 c000f0fc c000f100 > >>> [ 70.060694] 5fa0: 600f0013 > >>> [ 70.064174] [] (__irq_svc) from [] > >>> (arch_cpu_idle+0x30/0x3c) > >>> [ 70.071545] [] (arch_cpu_idle) from [] > >>> (cpu_startup_entry+0x138/0x224) > >>> [ 70.079783] [] (cpu_startup_entry) from [] > >>> (start_kernel+0x304/0x35c) > >>> [ 70.087921] ---[ end trace 66b76f6dcbbda3bf ]--- > >>> [ 79.911589] dwc2 ffb4.usb: new device is low-speed > >>> > >>> Dinh > >>> > >> > >> It looks like this patch is producing the above error. > >> > >> usb: dwc2: gadget: manage ep0 state in software > >> > >> Manage ep0 state in software to add handling of status OUT stage. > >>
RE: [PATCH v2 00/30] usb: updates for dwc2 gadget driver
Hi, > -Original Message- > From: Dinh Nguyen [mailto:dingu...@opensource.altera.com] > Sent: Wednesday, January 7, 2015 1:01 AM > To: Paul Zimmerman; Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; sergei.shtyl...@cogentembedded.com; > r.bald...@samsung.com > Subject: Re: [PATCH v2 00/30] usb: updates for dwc2 gadget driver > > On 01/06/2015 05:47 PM, Dinh Nguyen wrote: > > On 01/05/2015 01:16 PM, Paul Zimmerman wrote: > >> Adding Dinh to CC. > >> > >> Robert, Dinh, I would like to have your tested-bys for this series, please. > >> > > > > This patch series is producing this error on the SOCFPGA platform. > > I'll try to bisect to a specific patch. > > > > root@socfpga_cyclone5:~# [ 47.929743] dwc2 ffb4.usb: new device is > > low-speed > > [ 63.873596] dwc2 ffb4.usb: new device is low-speed > > [ 64.219220] dwc2 ffb4.usb: new device is low-speed > > [ 64.421495] dwc2 ffb4.usb: new device is low-speed > > [ 64.834505] dwc2 ffb4.usb: new device is high-speed > > [ 69.899695] [ cut here ] > > [ 69.904315] WARNING: CPU: 0 PID: 0 at drivers/usb/dwc2/gadget.c:1352 > > s3c_hsotg_rx_data+0xc0/0xd4() > > [ 69.913230] Modules linked in: g_mass_storage usb_f_mass_storage > > libcomposite > > [ 69.920385] CPU: 0 PID: 0 Comm: swapper/0 Not tainted > > 3.19.0-rc1-00090-g4864f14 #4 > > [ 69.927917] Hardware name: Altera SOCFPGA > > [ 69.931936] [] (unwind_backtrace) from [] > > (show_stack+0x10/0x14) > > [ 69.939654] [] (show_stack) from [] > > (dump_stack+0x74/0x90) > > [ 69.946857] [] (dump_stack) from [] > > (warn_slowpath_common+0x78/0xb4) > > [ 69.954916] [] (warn_slowpath_common) from [] > > (warn_slowpath_null+0x1c/0x24) > > [ 69.963669] [] (warn_slowpath_null) from [] > > (s3c_hsotg_rx_data+0xc0/0xd4) > > [ 69.972162] [] (s3c_hsotg_rx_data) from [] > > (s3c_hsotg_irq+0x454/0x520) > > [ 69.980394] [] (s3c_hsotg_irq) from [] > > (handle_irq_event_percpu+0x5c/0x1f4) > > [ 69.989056] [] (handle_irq_event_percpu) from [] > > (handle_irq_event+0x44/0x64) > > [ 69.997892] [] (handle_irq_event) from [] > > (handle_fasteoi_irq+0xa8/0x180) > > [ 70.006382] [] (handle_fasteoi_irq) from [] > > (generic_handle_irq+0x20/0x30) > > [ 70.014958] [] (generic_handle_irq) from [] > > (__handle_domain_irq+0x54/0xb4) > > [ 70.023619] [] (__handle_domain_irq) from [] > > (gic_handle_irq+0x20/0x5c) > > [ 70.031937] [] (gic_handle_irq) from [] > > (__irq_svc+0x40/0x54) > > [ 70.039384] Exception stack(0xc0635f60 to 0xc0635fa8) > > [ 70.044415] 5f60: c0635fb0 c001cee0 c066cca0 > > c063c498 c046908c c066c4b2 > > [ 70.052556] 5f80: c066cca0 0001 ef7fccc0 0100 > > c0635fa8 c000f0fc c000f100 > > [ 70.060694] 5fa0: 600f0013 > > [ 70.064174] [] (__irq_svc) from [] > > (arch_cpu_idle+0x30/0x3c) > > [ 70.071545] [] (arch_cpu_idle) from [] > > (cpu_startup_entry+0x138/0x224) > > [ 70.079783] [] (cpu_startup_entry) from [] > > (start_kernel+0x304/0x35c) > > [ 70.087921] ---[ end trace 66b76f6dcbbda3bf ]--- > > [ 79.911589] dwc2 ffb4.usb: new device is low-speed > > > > Dinh > > > > It looks like this patch is producing the above error. > > usb: dwc2: gadget: manage ep0 state in software > > Manage ep0 state in software to add handling of status OUT stage. > Just toggling hsotg->setup in s3c_hsotg_handle_outdone leaves it in > wrong state in 2-stage control transfers. > Moreover, don't call s3c_hsotg_handle_outdone both from SetupDone and > OutDone. Only use one of them, as for a setup packet we get both. > > Signed-off-by: Mian Yousaf Kaukab Thank you for testing this patchset. I have tested g_mass_storage without dma in my setup and it is working fine. As I don't see this problem, I need some help in figuring out what's going on. Would it be possible for you to provide me following? 1) Console output after enabling CONFIG_USB_DWC2_DEBUG 2) Function trace echo s3c_* > /sys/kernel/debug/tracing/set_ftrace_filter echo function > /sys/kernel/debug/tracing/current_tracer 3) Values of following registers GSNPSID, GHWCFG1-GHWCFG4 > > Dinh BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 17/29] usb: dwc2: gadget: manage ep0 state in software
Hi, > -Original Message- > From: Kaukab, Yousaf > Sent: Thursday, December 18, 2014 7:54 PM > To: linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; pa...@synopsys.com; Kaukab, Yousaf > Subject: [PATCH 17/29] usb: dwc2: gadget: manage ep0 state in software > > Manage ep0 state in software to add handling of status OUT stage. > Just toggling hsotg->setup in s3c_hsotg_handle_outdone leaves it in wrong > state in 2-stage control transfers. > Moreover, there is no need to handle SetupDone as requests can be complete > on XferCmpl of status stage. > > Signed-off-by: Mian Yousaf Kaukab > --- > drivers/usb/dwc2/core.h | 13 +++- > drivers/usb/dwc2/gadget.c | 151 > ++ > 2 files changed, 83 insertions(+), 81 deletions(-) [...] > @@ -1533,8 +1517,7 @@ static void s3c_hsotg_handle_rx(struct dwc2_hsotg > *hsotg) > "SetupDone (Frame=0x%08x, > DOPEPCTL=0x%08x)\n", > s3c_hsotg_read_frameno(hsotg), > readl(hsotg->regs + DOEPCTL(0))); > - > - s3c_hsotg_handle_outdone(hsotg, epnum, true); > + /* XferCmpl is used to complete the transfers */ > break; This comment is incorrect. XferCmpl should read OutDone. BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1 16/29] usb: dwc2: gadget: kill requests after disabling ep
Hi, > -Original Message- > From: Robert Baldyga [mailto:r.bald...@samsung.com] > Sent: Tuesday, December 23, 2014 9:40 AM > To: Mian Yousaf Kaukab; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; pa...@synopsys.com; > sergei.shtyl...@cogentembedded.com; Kaukab, Yousaf > Subject: Re: [PATCH v1 16/29] usb: dwc2: gadget: kill requests after disabling > ep > > Hi, > > On 12/21/2014 05:15 PM, Mian Yousaf Kaukab wrote: > > kill_all_requests() can flush the fifo. Call it after disabling the > > endpoint. Moreover, remove even the current IN request so that next IN > > request after s3c_hsotg_ep_enable can be properly handled. > > > > Signed-off-by: Mian Yousaf Kaukab > > --- > > drivers/usb/dwc2/gadget.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > > index 031f7edc..4ccf59b 100644 > > --- a/drivers/usb/dwc2/gadget.c > > +++ b/drivers/usb/dwc2/gadget.c > > @@ -2612,8 +2612,6 @@ static int s3c_hsotg_ep_disable(struct usb_ep *ep) > > epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index); > > > > spin_lock_irqsave(&hsotg->lock, flags); > > - /* terminate all requests with shutdown */ > > - kill_all_requests(hsotg, hs_ep, -ESHUTDOWN, false); > > > > hsotg->fifo_map &= ~(1<fifo_index); > > hs_ep->fifo_index = 0; > > @@ -2630,6 +2628,9 @@ static int s3c_hsotg_ep_disable(struct usb_ep *ep) > > /* disable endpoint interrupts */ > > s3c_hsotg_ctrl_epint(hsotg, hs_ep->index, hs_ep->dir_in, 0); > > > > + /* terminate all requests with shutdown */ > > + kill_all_requests(hsotg, hs_ep, -ESHUTDOWN, true); > > + > > spin_unlock_irqrestore(&hsotg->lock, flags); > > return 0; > > } > > > > After this change function kill_all_requests() is always called with second > parameter = 'true', so we don't need to have this parameter anymore. > > I have already sent patch making that change: > https://lkml.org/lkml/2014/12/16/135 I am OK with your patch. I also want to call kill_all_requests after disabling the ep so I can just rebase this change on top of your patch. Felipe, should I already rebase on top of Robert's patch and mark it as dependency in my patchset or should I wait for the rebase till you apply Robert's patch to your branch? BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1 03/29] usb: dwc2: gadget: process setup packet on transfer complete
Hi > -Original Message- > From: Mian Yousaf Kaukab [mailto:yousaf.kau...@gmail.com] > Sent: Sunday, December 21, 2014 5:15 PM > To: linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; pa...@synopsys.com; > sergei.shtyl...@cogentembedded.com; Kaukab, Yousaf > Subject: [PATCH v1 03/29] usb: dwc2: gadget: process setup packet on transfer > complete > > DOEPINTn.SetUp also indicates an OUT token for the data stage, so instead use > DOEPINTn.StupPktRcvd. Moreover, check DOEPINTn.StupPktRcvd on > DOEPINTn.XferComp as described in programming guide. > According to the programming guide, DIEPCTL.CNAK should not be set for the DATA stage unless DOEPINTn.SetUp is seen. If setup packet is processed on DOEPINTn.StupPktRcvd and s3c_hsotg_start_req() is called before a IN/NAK or Out/NAK is seen on the bus, there is a chance that DIEPCTL.CNAK rule is violated. As a result no interrupt might be seen for the DATA stage transfers. So it's better to process the setup packet on DOEPINTn.SetUp. Only change required is not to call s3c_hsotg_complete_in() or s3c_hsotg_handle_outdone in DOEPINTn.XferCompl handling when DOEPINTn.StupPktRcvd or DOEPINTn.SetUp is set. I will replace it with another patch to do this. BR, Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 07/29] usb: dwc2: gadget: add device tree property to enable dma
Hi,
> -Original Message-
> From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com]
> Sent: Friday, December 19, 2014 1:41 PM
> To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com
> Cc: Herrero, Gregory; pa...@synopsys.com
> Subject: Re: [PATCH 07/29] usb: dwc2: gadget: add device tree property to
> enable dma
>
> On 12/18/2014 9:53 PM, Mian Yousaf Kaukab wrote:
>
> > From: Gregory Herrero
>
> > * Add an of specific function to parse device node properties.
> > * Enable dma usage only if device tree property 'g_use_dma' is present.
>
> Hyphen is preferred to underscore in the device tree prop names.
>
> > Signed-off-by: Gregory Herrero
>
> [...]
> > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> > index 58699c3..eb58305 100644
> > --- a/drivers/usb/dwc2/gadget.c
> > +++ b/drivers/usb/dwc2/gadget.c
> [...]
> > @@ -3402,6 +3402,24 @@ static void s3c_hsotg_delete_debug(struct
> dwc2_hsotg *hsotg)
> > debugfs_remove(hsotg->debug_root);
> > }
> >
> > +#ifdef CONFIG_OF
> > +static int s3c_hsotg_of_probe(struct dwc2_hsotg *hsotg) {
> > + struct device_node *np = hsotg->dev->of_node;
> > +
> > + /* Enable dma if requested in device tree */
> > + if (of_find_property(np, "g_use_dma", NULL))
>
> Use of_propery_read_bool() please.
>
> > + hsotg->g_using_dma = true;
> > +
> > + return 0;
>
> The function always returns 0, why not make it *void* for now?
>
> [...]
> > @@ -3421,6 +3439,10 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg,
> int irq)
> > /* Set default UTMI width */
> > hsotg->phyif = GUSBCFG_PHYIF16;
> >
> > + ret = s3c_hsotg_of_probe(hsotg);
> > + if (ret)
> > + return ret;
>
> Dead code as s3c_hsotg_of_probe() always returns 0.
>
> [...]
>
> WBR, Sergei
I will fix all of them.
Thanks,
Yousaf
--
Intel Sweden AB
Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden
Registration Number: 556189-6027
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 05/29] usb: dwc2: gadget: write correct value in ahbcfg register
Hi, > -Original Message- > From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com] > Sent: Friday, December 19, 2014 1:36 PM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; pa...@synopsys.com > Subject: Re: [PATCH 05/29] usb: dwc2: gadget: write correct value in ahbcfg > register > > On 12/18/2014 9:53 PM, Mian Yousaf Kaukab wrote: > > > From: Gregory Herrero > > Felipe requires some change log in every patch. We thought the patch is simple enough to be described by the subject. If this is required, I will add some description to it. > > > Signed-off-by: Gregory Herrero > > [...] > > WBR, Sergei BR, Yousaf -- Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 04/29] usb: dwc2: gadget: don't embedded ep0 buffers
Hi, Thank you for reviewing these patches. > -Original Message- > From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com] > Sent: Friday, December 19, 2014 1:34 PM > To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; pa...@synopsys.com > Subject: Re: [PATCH 04/29] usb: dwc2: gadget: don't embedded ep0 buffers > > Hello. > > On 12/18/2014 9:53 PM, Mian Yousaf Kaukab wrote: > > s/embedded/embed/ in the subject. > > > When using DMA, data of the previous setup packet can be read back > > from cache because ep0 and ctrl buffers are embedded in struct s3c_hsotg. > > Allocate buffers instead of embeddeding them. > > s/embeddeding/embedding/. Ok. > > > Signed-off-by: Mian Yousaf Kaukab > > [...] > > WBR, Sergei BR, Yousaf -- Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 00/29] usb: updates for dwc2 gadget driver
Hi, Apologies for the email footer. It is automatically being appended by the smtp server. I need to talk to IT folks here to get this fixed. BR, Yousaf > -Original Message- > From: Kaukab, Yousaf > Sent: Thursday, December 18, 2014 7:53 PM > To: linux-usb@vger.kernel.org; ba...@ti.com > Cc: Herrero, Gregory; pa...@synopsys.com; Kaukab, Yousaf > Subject: [PATCH 00/29] usb: updates for dwc2 gadget driver > > Hi, > This patchset consists of various bug fixes and feature enhancements for the > dwc2 gadget driver. All the patches are verified on dwc2 v3.0a with dedicated > fifos. Main focus of testing was with dma enabled. Although basic testing > without dma was also done. > > It is based on linux-next tag next-20141218. > > Thank you, > > Best regards, > Yousaf > > Gregory Herrero (13): > usb: dwc2: gadget: register gadget handle to the phy > usb: dwc2: gadget: write correct value in ahbcfg register > usb: dwc2: gadget: don't erase gahbcfg register when enabling dma > usb: dwc2: gadget: add device tree property to enable dma > Documentation: dt-bindings: add dt binding info for dwc2 g_use_dma > usb: dwc2: gadget: configure fifos from device tree > Documentation: dt-bindings: add dt binding info for dwc2 fifo resizing > usb: dwc2: gadget: don't block after fifo flush timeout > usb: dwc2: gadget: add vbus_session support > usb: dwc2: gadget: reset fifo_map when initializing fifos > usb: dwc2: gadget: fix pullup handling > usb: dwc2: gadget: add vbus_draw support > usb: dwc2: gadget: force gadget initialization in dev mode > > Mian Yousaf Kaukab (16): > usb: dwc2: gadget: mask fifo empty irq with dma > usb: dwc2: gadget: process setup packet on transfer complete > usb: dwc2: gadget: don't embedded ep0 buffers > usb: dwc2: gadget: add bi-directional endpoint support > usb: dwc2: gadget: check interrupts for all endpoints > usb: dwc2: gadget: remove unused members from hsotg_req > usb: dwc2: gadget: fix debug loop limits > usb: dwc2: gadget: consider all tx fifos > usb: dwc2: gadget: kill requests after disabling ep > usb: dwc2: gadget: manage ep0 state in software > usb: dwc2: gadget: fix zero length packet transfers > usb: dwc2: gadget: dont warn if endpoint is not enabled > usb: dwc2: gadget: rename sent_zlp to send_zlp > usb: dwc2: gadget: pick smallest acceptable fifo > usb: dwc2: gadget: fix fifo allocation leak > usb: dwc2: gadget: report disconnection after reset > > Documentation/devicetree/bindings/usb/dwc2.txt | 4 + > drivers/usb/dwc2/core.h| 46 +- > drivers/usb/dwc2/gadget.c | 874 > - > drivers/usb/dwc2/hw.h | 1 + > 4 files changed, 602 insertions(+), 323 deletions(-) > > -- > 1.9.1 -- Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
