Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
On 05/22/15 02:21, Laura Abbott wrote: On 05/21/2015 08:26 AM, Alan Stern wrote: On Thu, 21 May 2015, Marcel Holtmann wrote: Hi Alan, Then avoiding the failed firmware is no solution, indeed. If it's a new probe, it should be never executed during resume. Can you expand this comment? What's wrong with probing during resume? The USB stack does carry out probes during resume under certain circumstances. A driver lacking a reset_resume callback is one of those circumstances. in case the platform kills the power to the USB lines, we can never do anything about this. I do not want to hack around this in the driver. What are the cases where we should implement reset_resume and would it really help here. Since the btusb.ko driver implements suspend/resume support, would reset_resume ever be called? One of those cases is exactly what you have been talking about: when the platform kills power to the USB lines during suspend. The driver's reset_resume routine will be called during resume, as opposed to the probe routine being called. Therefore the driver will be able to tell that this is not a new device instance. The other cases are less likely to occur: a device is unable to resume normally and requires a reset before it will start working again, or something else goes wrong along those lines. However I get the feeling someone needs to go back and see if the device is the same one and just gets probed again or if it is a new one from the USB host stack perspective. That can be done easily enough by enabling usbcore debugging before carrying out the system suspend: echo 'module usbcore =p' >/debug/dynamic_debug/control The debugging information in the kernel log will tell just what happened. Playing around in my test setup as a baseline [ 41.991035] usb usb1-port11: not reset yet, waiting 50ms [ 42.092902] usb 1-11: reset full-speed USB device number 4 using xhci_hcd [ 42.143575] usb usb1-port11: not reset yet, waiting 50ms [ 42.257822] btusb 1-11:1.0: no reset_resume for driver btusb? [ 42.257823] btusb 1-11:1.1: no reset_resume for driver btusb? [ 42.257825] btusb 1-11:1.0: forced unbind [ 42.258305] kworker/dying (826) used greatest stack depth: 10680 bytes left [ 42.331342] usb 1-9.2: reset full-speed USB device number 7 using xhci_hcd [ 42.416631] usb 1-9.2: ep0 maxpacket = 8 [ 42.681288] usb 1-9.1: reset low-speed USB device number 5 using xhci_hcd [ 42.968138] usb 1-9.1: ep 0x81 - rounding interval to 64 microframes, ep desc says 80 microframes [ 42.968157] usb 1-9.1: ep 0x82 - rounding interval to 64 microframes, ep desc says 80 microframes [ 43.036290] usb 1-9.4: reset high-speed USB device number 8 using xhci_hcd [ 43.123126] hub 1-9.4:1.0: hub_reset_resume [ 43.123581] hub 1-9.4:1.0: enabling power on all ports [ 43.224853] PM: resume of devices complete after 2456.587 msecs [ 43.225038] btusb 1-11:1.0: usb_probe_interface [ 43.225040] btusb 1-11:1.0: usb_probe_interface - got id [ 43.225802] [ cut here ] [ 43.225807] WARNING: CPU: 7 PID: 2844 at drivers/base/firmware_class.c:1118 _request_firmware+0x5ee/0x890() so it is trying to call the reset resume. If I try a 'dummy reset resume' diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index a7bdac0..cda8137 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -3401,6 +3401,7 @@ static struct usb_driver btusb_driver = { #ifdef CONFIG_PM .suspend = btusb_suspend, .resume = btusb_resume, + .reset_resume = btusb_resume, #endif .id_table = btusb_table, .supports_autosuspend = 1, I no longer see the warning which means that probe is no longer being called. Marcel, does implementing a proper reset_resume callback seem like the right approach or do you need more information? Hi, Laura I believe that some devices supported by btusb would need to do a request_firmware() in the reset_resume() callback and thus end up with the same issue. btusb could store the firmware obtained during the probe in it driver private structure and use that in reset_resume() callback, but it means the memory for the firmware blobs will not be released until the driver is unloaded. Regards, Arend Thanks, Laura -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
On 05/22/15 09:37, Arend van Spriel wrote: On 05/22/15 02:21, Laura Abbott wrote: On 05/21/2015 08:26 AM, Alan Stern wrote: On Thu, 21 May 2015, Marcel Holtmann wrote: Hi Alan, Then avoiding the failed firmware is no solution, indeed. If it's a new probe, it should be never executed during resume. Can you expand this comment? What's wrong with probing during resume? The USB stack does carry out probes during resume under certain circumstances. A driver lacking a reset_resume callback is one of those circumstances. in case the platform kills the power to the USB lines, we can never do anything about this. I do not want to hack around this in the driver. What are the cases where we should implement reset_resume and would it really help here. Since the btusb.ko driver implements suspend/resume support, would reset_resume ever be called? One of those cases is exactly what you have been talking about: when the platform kills power to the USB lines during suspend. The driver's reset_resume routine will be called during resume, as opposed to the probe routine being called. Therefore the driver will be able to tell that this is not a new device instance. The other cases are less likely to occur: a device is unable to resume normally and requires a reset before it will start working again, or something else goes wrong along those lines. However I get the feeling someone needs to go back and see if the device is the same one and just gets probed again or if it is a new one from the USB host stack perspective. That can be done easily enough by enabling usbcore debugging before carrying out the system suspend: echo 'module usbcore =p' >/debug/dynamic_debug/control The debugging information in the kernel log will tell just what happened. Playing around in my test setup as a baseline [ 41.991035] usb usb1-port11: not reset yet, waiting 50ms [ 42.092902] usb 1-11: reset full-speed USB device number 4 using xhci_hcd [ 42.143575] usb usb1-port11: not reset yet, waiting 50ms [ 42.257822] btusb 1-11:1.0: no reset_resume for driver btusb? [ 42.257823] btusb 1-11:1.1: no reset_resume for driver btusb? [ 42.257825] btusb 1-11:1.0: forced unbind [ 42.258305] kworker/dying (826) used greatest stack depth: 10680 bytes left [ 42.331342] usb 1-9.2: reset full-speed USB device number 7 using xhci_hcd [ 42.416631] usb 1-9.2: ep0 maxpacket = 8 [ 42.681288] usb 1-9.1: reset low-speed USB device number 5 using xhci_hcd [ 42.968138] usb 1-9.1: ep 0x81 - rounding interval to 64 microframes, ep desc says 80 microframes [ 42.968157] usb 1-9.1: ep 0x82 - rounding interval to 64 microframes, ep desc says 80 microframes [ 43.036290] usb 1-9.4: reset high-speed USB device number 8 using xhci_hcd [ 43.123126] hub 1-9.4:1.0: hub_reset_resume [ 43.123581] hub 1-9.4:1.0: enabling power on all ports [ 43.224853] PM: resume of devices complete after 2456.587 msecs [ 43.225038] btusb 1-11:1.0: usb_probe_interface [ 43.225040] btusb 1-11:1.0: usb_probe_interface - got id [ 43.225802] [ cut here ] [ 43.225807] WARNING: CPU: 7 PID: 2844 at drivers/base/firmware_class.c:1118 _request_firmware+0x5ee/0x890() so it is trying to call the reset resume. If I try a 'dummy reset resume' diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index a7bdac0..cda8137 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -3401,6 +3401,7 @@ static struct usb_driver btusb_driver = { #ifdef CONFIG_PM .suspend = btusb_suspend, .resume = btusb_resume, + .reset_resume = btusb_resume, #endif .id_table = btusb_table, .supports_autosuspend = 1, I no longer see the warning which means that probe is no longer being called. Marcel, does implementing a proper reset_resume callback seem like the right approach or do you need more information? Hi, Laura I believe that some devices supported by btusb would need to do a request_firmware() in the reset_resume() callback and thus end up with the same issue. btusb could store the firmware obtained during the probe in it driver private structure and use that in reset_resume() callback, but it means the memory for the firmware blobs will not be released until the driver is unloaded. Same is true if caching is done in firmware_loader so it may not be a big deal. Regards, Arend Regards, Arend Thanks, Laura -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On 05/22/2015 02:46 PM, Lu, Baolu wrote: On 05/22/2015 11:11 AM, David Cohen wrote: On Thu, May 21, 2015 at 08:09:54PM -0700, David Cohen wrote: Hi, On Fri, May 22, 2015 at 10:07:05AM +0800, Lu Baolu wrote: Many drivers and modules depend on ULPI bus registeration to register ULPI interfaces and drivers. It's more appropriate to register ULPI bus in subsys_initcall instead of module_init. Kernel panic has been reported with some kind of kernel config. Even though I agree subsys_initcall is better to register ulpi bus, it's still no excuse to have kernel panic. What about ULPI bus being compiled as module? No kernel panic if ULPI is built as a module. IMHO this is avoiding the proper kernel panic fix which should be failing gracefully (or defer probe) from tusb1210 driver. Maybe I need to express myself better :) IMHO we should not consider work done with this patch only. It's still incomplete. I am with you on that we should know the real problem. I could go ahead with further debugging. Do you have any suggestions about which direction should I go? I forgot to mention that the panic was found in an Android environment. The kernel version is v4.1-rc3. Br, David Thank you, -Baolu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- 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
AM3517 usb host issue
I am trying to get the full-speed USB host working on an custom AM3517 device with the 3.18.12 kernel. The hardware works (a 2.6.37 kernel has been used for testing). Does anyone have any experience of 3.18 (or similarly recent kernel on an AM3517 system) or have any pointers as where to start debugging? The ti-linux-3.14.y does not have any patches that aren't applied to the usb on 3.18.13. The cpu port 1 is connected by a TI TUSB1106 usb transceiver that is directly connected to a full-speed hub (TI USB2046) hub so the OHCI driver is the only one in use. Note, the ohci-omap3 is loaded as a module as this is how their user application expects to be able to shut down usb when it does not need it. The device tree configuration for the usb host: > &usbhshost { > status = "okay";/* just in case it is started disabled */ > > port1-mode = "ohci-phy-6pin-dpdm"; > }; > > &usbhsohci { > status = "okay"; > }; > > &usbhsehci { > status = "disabled";/* no ehci on board */ > }; The usb from the logs is as follows. Some extra debugging has been added to verify the device-tree settings: > [0.00] AM3517 ES1.1 (l2cache sgx neon) > > > [0.869706] usbcore: registered new interface driver usbfs > > [0.874270] usbcore: registered new interface driver hub > > [0.878592] usbcore: registered new device driver usb > > [1.223199] usbhs_tll 48062000.usbhstll: starting TI HSUSB TLL Controller > > [1.273000] usbhs_omap 48064000.usbhshost: ports 0 > > [1.278291] usbhs_omap 48064000.usbhshost: port 0: ohci-phy-6pin-dpdm > > [1.284476] usbhs_omap 48064000.usbhshost: port0-mode: ohci-phy-6pin-dpdm > ->5 > [1.288689] usbhs_tll 48062000.usbhstll: omap_tll_init() > > [1.293628] usbhs_omap 48064000.usbhshost: usbhs_runtime_resume > > [1.298434] usbhs_omap 48064000.usbhshost: sysconfig 0x1009 > > [1.302730] usbhs_tll 48062000.usbhstll: omap_tll_enable() > > [1.307668] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend > > [1.310142] stopping usb controller > > [1.419910] usbhs_tll 48062000.usbhstll: omap_tll_disable() > > [1.423547] usbhs_omap 48064000.usbhshost: 3 ports > > [1.429065] usbhs_omap 48064000.usbhshost: starting TI HSUSB Controller > > [1.433831] usbhs_omap 48064000.usbhshost: usbhs_runtime_resume > > [1.438625] usbhs_omap 48064000.usbhshost: sysconfig 0x1009 > > [1.442921] usbhs_tll 48062000.usbhstll: omap_tll_enable() > > [1.448548] usbhs_omap 48064000.usbhshost: omap_usbhs_rev1_hostconfig => > > [1.455034] usbhs_omap 48064000.usbhshost: UHH setup done, > uhh_hostconfig=80d > [1.459918] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend > > [1.462337] stopping usb controller > > [1.569905] usbhs_tll 48062000.usbhstll: omap_tll_disable() > > [1.575408] usbhs_omap 48064000.usbhshost: populating usb sub nodes > > > [ 77.609168] usbhs_omap 48064000.usbhshost: usbhs_runtime_resume > > [ 77.613927] usbhs_omap 48064000.usbhshost: sysconfig 0x1009 > > [ 77.618374] usbhs_tll 48062000.usbhstll: omap_tll_enable() > > [ 77.802694] usb usb1: New USB device found, idVendor=1d6b, idProduct=0001 > > [ 77.816003] usb usb1: New USB device strings: Mfr=3, Product=2, > SerialNumber1 > [ 77.827391] usb usb1: Product: OHCI Host Controller > > [ 77.838674] usb usb1: Manufacturer: Linux 3.18.13-00203-ga3c52be-dirty > ohci_d > [ 77.849913] usb usb1: SerialNumber: 48064400.ohci > -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- 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: Bogus traffic since 3.19.3 -> 4.0.2 with Ericsson F5521gw WWAN
Florian Bruhin writes: > Hi, > > (this is the first time I'm reporting a but against the kernel - so > please bear with me, and if I'm doing anything wrong or anything is > missing, please let me know!) > > I have a Thinkpad x220t with an Ericsson F5521gw WWAN interface, > running Archlinux. > > Since updating my kernel from 3.19.3 to 4.0.2 I see the following > behaviour, which I can also reproduce with 4.1.0 mainline: > > The network interface (wwan0) has traffic spikes of some gigabytes per > second in ifconfig or conky - right now, 10 minutes after booting, I > see this: > > RX packets 4083 bytes 2277043 (2.1 MiB) > RX errors 0 dropped 0 overruns 0 frame 0 > TX packets 3680 bytes 12025908915562 (10.9 TiB) > TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0 Yes, I am able to reproduce this problem. It is an effect of the feable attempts to make it count the actual transmitted data instead of the variable padding. Which obviously was a failure, resulting in counters being more off than ever. I'll try to figure out how this happened. Bjørn -- 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
USB Host Controller no PCD interrupt and CCS and CSC update on the device disconnect
Hi, We have one USB 2.0 Controller on an ARM SoC, with internal PHY confirming to UTMI. The PHY would detect unexpected disconnect (amplitude of the differential envelop still < 625 mV) and assert the HostDisconnect signal to the Controller to indicate a disconnection event. When the HostDisconnect signal is asserted, the Controller will first do some internal clean work before it update CCS and CSC in PORTSCx and reports a Port Change Detect interrupt. We want to improve the situation by masking the HostDisconnect signal from PHY (take the cost into consideration), but it will cause no CCS and CSC update in PORTSCx, and no PCD interrupt. Thus the true disconnection only can be determined by an XactErr. Once the driver determine that it's a true disconnect, the HostDisconnect signal can be unmasked and the Controller will detect this disconnect. We are worried about the workaroud, since there may be many corner cases we have not taken into consideration, such as the race condition on connect and disconnect. So have you encountered this situation before? Or is there any suggestion on this workaround? Thanks! Regards Rong -- 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 6/6] usb: chipidea: add work-around for Marvell HSIC PHY startup
On Thu, May 21, 2015 at 09:54:20AM +, Peter Chen wrote: > > > > > On Wed, May 20, 2015 at 10:13 PM, Peter Chen > > wrote: > > > On Tue, May 19, 2015 at 09:10:05PM -0500, Rob Herring wrote: > > >> The Marvell 28nm HSIC PHY requires the port to be forced to HS mode > > >> after the port power is applied. This is done using the test mode in > > >> the PORTSC register. > > >> > > >> As HSIC is always HS, this work-around should be safe to do with all > > >> HSIC PHYs. If not, a flag can also be added. > > > > > > I think a flag is needed, not sure all vendors can work well with that. > > > > Only i.MX6Sx uses HSIC in mainline. Is that something you can test? It > > would be > > better to not add flags unless they are really needed. > > Otherwise you end up with dozens of flags like SDHCI drivers have. > > > > I will have a test for this, and show you the result later. > Since imx6sx HSIC support is not supported at mainline, I tested this sequence at internal branch, it does not affect imx6's hsic function, I am ok with this patch. Tested-by: Peter Chen > Peter > ��칻�&�~�&���+-��ݶ��w��˛���m�b��n��ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?&�)ߢf -- Best Regards, Peter Chen -- 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 0/5] USB: atmel: rework clock handling
Le 17/03/2015 17:15, Boris Brezillon a écrit : > Hello, > > This series reworks clock handling in atmel USB host drivers, and while > doing so fixes a regression introduced by 3440ef1 (ARM: at91/dt: fix USB > high-speed clock to select UTMI). > > Best Regards, > > Boris > > Boris Brezillon (5): > USB: ehci-atmel: rework clk handling > USB: host: ohci-at91: remove useless uclk clock Now that these ones are in Linus' tree... ...these patches vvv > USB: atmel: update DT bindings documentation > ARM: at91/dt: remove useless uhpck clock references from ehci > defintions > ARM: at91/dt: remove useless usb clock ^ can be queued in at91-4.2-dt. They will go through the arm-soc route. And BTW: Acked-by: Nicolas Ferre Thanks Boris, bye. > .../devicetree/bindings/usb/atmel-usb.txt | 25 ++ > arch/arm/boot/dts/at91rm9200.dtsi | 4 +-- > arch/arm/boot/dts/at91sam9260.dtsi | 4 +-- > arch/arm/boot/dts/at91sam9261.dtsi | 4 +-- > arch/arm/boot/dts/at91sam9263.dtsi | 4 +-- > arch/arm/boot/dts/at91sam9g45.dtsi | 8 +++--- > arch/arm/boot/dts/at91sam9n12.dtsi | 5 ++-- > arch/arm/boot/dts/at91sam9x5.dtsi | 8 +++--- > arch/arm/boot/dts/sama5d3.dtsi | 9 +++ > arch/arm/boot/dts/sama5d4.dtsi | 9 +++ > drivers/usb/host/ehci-atmel.c | 30 > +++--- > drivers/usb/host/ohci-at91.c | 18 +++-- > 12 files changed, 63 insertions(+), 65 deletions(-) > -- Nicolas Ferre -- 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 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
> >>>Many drivers and modules depend on ULPI bus registeration to > >>>register ULPI interfaces and drivers. It's more appropriate > >>>to register ULPI bus in subsys_initcall instead of module_init. > >>> > >>>Kernel panic has been reported with some kind of kernel config. > >>Even though I agree subsys_initcall is better to register ulpi bus, it's > >>still no excuse to have kernel panic. What about ULPI bus being compiled > >>as module? > > No kernel panic if ULPI is built as a module. > > >>IMHO this is avoiding the proper kernel panic fix which should be > >>failing gracefully (or defer probe) from tusb1210 driver. > >Maybe I need to express myself better :) > >IMHO we should not consider work done with this patch only. It's still > >incomplete. > > I am with you on that we should know the real problem. > > I could go ahead with further debugging. Do you have any suggestions > about which direction should I go? This patch does not address all the cases where the panic may occur, like the case where the bus itself fails register, while Sasha's patch does. For the panic, we'll use Sasha's patch. I though we were clear on this. This patch addresses an issue with the load order. Even with the panic fixed, we still may end up loading the drivers depending on the bus, i.e. ulpi phy drivers or the ulpi interface providers, before the bus. That is a different issue, but we need to fix it as well. To fix the panic, you can already pick Sasha's patch titled: "usb: ulpi: don't register drivers if bus doesn't exist" Baolu, please fix the commit message to explain this patch is fixing the problem with the load order. Thanks, -- heikki -- 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] usb: ulpi: don't register drivers if bus doesn't exist
> > >>diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c > > >>index 0e6f968..0b0a5e7 100644 > > >>--- a/drivers/usb/common/ulpi.c > > >>+++ b/drivers/usb/common/ulpi.c > > >>@@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) > > >> if (!drv->probe) > > >> return -EINVAL; > > >>+ /* Was the bus registered successfully? */ > > >>+ if (!ulpi_bus.p) > > >>+ return -ENODEV; > > I think we need to warn in this case. How about: > > if (unlikely(WARN_ON(!ulpi_bus.p))) > return -ENODEV; I think we should also return -EAGAIN here. Thanks, -- heikki -- 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] usb: ulpi: don't register drivers if bus doesn't exist
On Fri, May 22, 2015 at 01:16:38PM +0300, Heikki Krogerus wrote: > > > >>diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c > > > >>index 0e6f968..0b0a5e7 100644 > > > >>--- a/drivers/usb/common/ulpi.c > > > >>+++ b/drivers/usb/common/ulpi.c > > > >>@@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) > > > >>if (!drv->probe) > > > >>return -EINVAL; > > > >>+ /* Was the bus registered successfully? */ > > > >>+ if (!ulpi_bus.p) > > > >>+ return -ENODEV; > > > > I think we need to warn in this case. How about: > > > > if (unlikely(WARN_ON(!ulpi_bus.p))) > > return -ENODEV; > > I think we should also return -EAGAIN here. The same check needs to be added to ulpi_register_interface() as well. Cheers, -- heikki -- 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: [BUG] USB 3.0 device "lost for good" when in SS.inactive or Compliance Mode during hub_port_reset()
On Thu, 21 May 2015, Alan Stern wrote: > I suspect you have not divided up all the actions in the correct way. > Look at everything in hub_port_finish_reset and think carefully about > where each one belongs. I had taken a very "conservative" approach to only minimally alter the behavior specifically for the fix I needed, since I understand too little of the code to know what I might break if I made bolder changes. Now following "bold" approach, I went all the way and dissolved hub_port_finish_reset() completely, moving all of its actions over to hub_port_reset(). Moving the clearing of the port change bits over to hub_port_wait_reset() seemed impractical to me, since that has many return statements in it, and the clearing must be done in every case, so it's simpler to put this in hub_port_reset() after hub_port_wait_reset() has returned. While at it, I also changed the error handling for the initial "double check" and removed an unneeded advance declaration of hub_port_reset(). The resulting unified (!:-)) diff patch versus 3.19.8 is: diff -u -r a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c --- a/drivers/usb/core/hub.c2015-05-21 00:11:25.0 +0200 +++ b/drivers/usb/core/hub.c2015-05-22 08:15:37.199743870 +0200 @@ -2616,9 +2616,6 @@ return USE_NEW_SCHEME(retry); } -static int hub_port_reset(struct usb_hub *hub, int port1, - struct usb_device *udev, unsigned int delay, bool warm); - /* Is a USB 3.0 port in the Inactive or Compliance Mode state? * Port worm reset is required to recover */ @@ -2706,44 +2703,6 @@ return 0; } -static void hub_port_finish_reset(struct usb_hub *hub, int port1, - struct usb_device *udev, int *status) -{ - switch (*status) { - case 0: - /* TRSTRCY = 10 ms; plus some extra */ - msleep(10 + 40); - if (udev) { - struct usb_hcd *hcd = bus_to_hcd(udev->bus); - - update_devnum(udev, 0); - /* The xHC may think the device is already reset, -* so ignore the status. -*/ - if (hcd->driver->reset_device) - hcd->driver->reset_device(hcd, udev); - } - /* FALL THROUGH */ - case -ENOTCONN: - case -ENODEV: - usb_clear_port_feature(hub->hdev, - port1, USB_PORT_FEAT_C_RESET); - if (hub_is_superspeed(hub->hdev)) { - usb_clear_port_feature(hub->hdev, port1, - USB_PORT_FEAT_C_BH_PORT_RESET); - usb_clear_port_feature(hub->hdev, port1, - USB_PORT_FEAT_C_PORT_LINK_STATE); - usb_clear_port_feature(hub->hdev, port1, - USB_PORT_FEAT_C_CONNECTION); - } - if (udev) - usb_set_device_state(udev, *status - ? USB_STATE_NOTATTACHED - : USB_STATE_DEFAULT); - break; - } -} - /* Handle port reset and port warm(BH) reset (for USB3 protocol ports) */ static int hub_port_reset(struct usb_hub *hub, int port1, struct usb_device *udev, unsigned int delay, bool warm) @@ -2767,17 +2726,15 @@ * If the caller hasn't explicitly requested a warm reset, * double check and see if one is needed. */ - status = hub_port_status(hub, port1, - &portstatus, &portchange); - if (status < 0) - goto done; - - if (hub_port_warm_reset_required(hub, port1, portstatus)) - warm = true; + if (hub_port_status(hub, port1, &portstatus, &portchange) == 0) { + if (hub_port_warm_reset_required(hub, port1, portstatus)) + warm = true; + } } clear_bit(port1, hub->warm_reset_bits); /* Reset the port */ + status = 0; for (i = 0; i < PORT_RESET_TRIES; i++) { status = set_port_feature(hub->hdev, port1, (warm ? USB_PORT_FEAT_BH_PORT_RESET : @@ -2795,11 +2752,22 @@ dev_dbg(hub->intfdev, "port_wait_reset: err = %d\n", status); + + /* clear wPortChange bits which may have been set */ + usb_clear_port_feature(hub->hdev, + port1, USB_PORT_FEAT_C_RESET); + if (hub_is_superspeed(hub->hdev)) { + usb_clear_port_feat
[PATCH] usb: renesas_usbhs: avoid uninitialized variable use
After the renesas_usbhs driver is enabled in ARM multi_v7_defconfig, we now get a new warning: renesas_usbhs/mod.c: In function 'usbhs_interrupt': renesas_usbhs/mod.c:246:7: warning: 'intenb1' may be used uninitialized in this function [-Wmaybe-uninitialized] gcc correctly points to a problem here, for the case that the device is in host mode, we use the intenb1 variable without having assigned it first. The state->intsts1 has a similar problem, but gcc cannot know that. This avoids the problem by initializing both sides of the comparison to zero when we don't read them from the respective registers. Signed-off-by: Arnd Bergmann Fixes: 88a25e02f3 ("usb: renesas_usbhs: Add access control for INTSTS1 and INTENB1 register") diff --git a/drivers/usb/renesas_usbhs/mod.c b/drivers/usb/renesas_usbhs/mod.c index e5ce6e6d4f51..d4be5d594896 100644 --- a/drivers/usb/renesas_usbhs/mod.c +++ b/drivers/usb/renesas_usbhs/mod.c @@ -223,6 +223,8 @@ static int usbhs_status_get_each_irq(struct usbhs_priv *priv, if (usbhs_mod_is_host(priv)) { state->intsts1 = usbhs_read(priv, INTSTS1); intenb1 = usbhs_read(priv, INTENB1); + } else { + state->intsts1 = intenb1 = 0; } /* mask */ -- 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: usb: gadget: f_fs: O_NONBLOCK waits MAX_SCHEDULE_TIMEOUT
> On Wed, Apr 01, 2015 at 06:29:05PM +0100, Baxter, Jim wrote: >>> >>> FunctionFS is very specific, because read/write operations are directly >>> translated into USB requests, which are asynchronous, so you cannot use >>> O_NONBLOCK. >>> >>> If you need non-blocking API you can use Asynchronous I/O (AIO). You can >>> find some examples in kernel sources (tools/usb/ffs-aio-example/). >>> >>> Br, >>> Robert Baldyga >>> >> >> Thank you, that sounds like the best approach. >> In this case I think perhaps the long wait without any data is an >> problem with the imx6 Chipidea USB controller. > > What's the possible problem? Sorry for the delay in replying, I have been getting some more details with a USB Analyser. The scenario is that the NCM device is enumerating so we see the messages to: SetAddress (1) GetDescriptor (Device) GetDescriptor (StringN) GetDescriptor (Configuration) SetConfiguration (1) GetDescriptor (String iInterface) GetDescriptor (String iInterface) At this point the NCM host sends Writes to the F_FS EP0 but for some reason the host device does not respond and only issues SOF packets for hours. This happens occasionally and is fixed by turning the device off and on again. Unless I am mistaken from a NCM gadget point of view the attached device is working correctly and there is no way to know it has failed, is that correct? > >> >> I guess it should suspend and drop the connections if there is no >> traffic for more than 10ms? >> > > If the Device side NAK host's IN/OUT token continually, the pipe will > not be stopped, the host will send token continually until the application > cancel this request. > -- 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
[PATCH net,stable-4.0] cdc_ncm: Fix tx_bytes statistics
The tx_curr_frame_payload field is u32. When we try to calculate a small negative delta based on it, we end up with a positive integer close to 2^32 instead. So the tx_bytes pointer increases by about 2^32 for every transmitted frame. Fix by calculating the delta as a signed long. Cc: Ben Hutchings Reported-by: Florian Bruhin Fixes: 7a1e890e2168 ("usbnet: Fix tx_bytes statistic running backward in cdc_ncm") Signed-off-by: Bjørn Mork --- drivers/net/usb/cdc_ncm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index c3e4da9e79ca..8067b8fbb0ee 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -1182,7 +1182,7 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign) * payload data instead. */ usbnet_set_skb_tx_stats(skb_out, n, - ctx->tx_curr_frame_payload - skb_out->len); + (long)ctx->tx_curr_frame_payload - skb_out->len); return skb_out; -- 2.1.4 -- 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
[PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
Phy drivers and the ulpi interface providers depend on the registeration of the ulpi bus. Ulpi registers the bus in module_init(). This could result in a load order issue, i.e. ulpi phy drivers or the ulpi interface providers loading before the bus registeration. This patch fixes this load order issue by putting ulpi_init in subsys_initcall(). Reported-by: Zhuo Qiuxu Signed-off-by: Lu Baolu --- drivers/usb/common/ulpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c index 0e6f968..01c0c04 100644 --- a/drivers/usb/common/ulpi.c +++ b/drivers/usb/common/ulpi.c @@ -242,7 +242,7 @@ static int __init ulpi_init(void) { return bus_register(&ulpi_bus); } -module_init(ulpi_init); +subsys_initcall(ulpi_init); static void __exit ulpi_exit(void) { -- 2.1.4 -- 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: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
On Thu, 2015-05-21 at 22:46 +0200, Arend van Spriel wrote: > On 05/21/15 19:32, Takashi Iwai wrote: > > Well, if the probe requires the access to a user-space file, it can't > > be done during resume. That's the very problem we're seeing now. > > The firmware loader can't help much alone if it's a new device > > object. > > So you are saying each device driver should come up with some retry > mechanism. Would make more sense to come up with something like that > behind the scenes in the firmware loader so all device drivers can rely > on one and the same solution. There is already a notifier for this. I don't see why the firmware layer couldn't retrigger a match for all unbound devices, just like we do when a new driver is loaded. Regards Oliver -- 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] usb: renesas_usbhs: avoid uninitialized variable use
Hi Arnd, > Sent: Friday, May 22, 2015 8:07 PM > > After the renesas_usbhs driver is enabled in ARM multi_v7_defconfig, > we now get a new warning: > > renesas_usbhs/mod.c: In function 'usbhs_interrupt': > renesas_usbhs/mod.c:246:7: warning: 'intenb1' may be used uninitialized in > this function [-Wmaybe-uninitialized] > > gcc correctly points to a problem here, for the case that the > device is in host mode, we use the intenb1 variable without > having assigned it first. The state->intsts1 has a similar > problem, but gcc cannot know that. > > This avoids the problem by initializing both sides of the > comparison to zero when we don't read them from the respective > registers. > > Signed-off-by: Arnd Bergmann > Fixes: 88a25e02f3 ("usb: renesas_usbhs: Add access control for INTSTS1 and > INTENB1 register") Thank you very much for the patch! Acked-by: Yoshihiro Shimoda (I'm not sure why a toolchain I used (Linaro GCC 2014.11) doesn't show this warning...) Best regards, Yoshihiro Shimoda > diff --git a/drivers/usb/renesas_usbhs/mod.c b/drivers/usb/renesas_usbhs/mod.c > index e5ce6e6d4f51..d4be5d594896 100644 > --- a/drivers/usb/renesas_usbhs/mod.c > +++ b/drivers/usb/renesas_usbhs/mod.c > @@ -223,6 +223,8 @@ static int usbhs_status_get_each_irq(struct usbhs_priv > *priv, > if (usbhs_mod_is_host(priv)) { > state->intsts1 = usbhs_read(priv, INTSTS1); > intenb1 = usbhs_read(priv, INTENB1); > + } else { > + state->intsts1 = intenb1 = 0; > } > > /* mask */ > > -- > 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 -- 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 v8 1/9] xhci: Set shared HCD's hcd_priv in xhci_gen_setup
Hi On 19.05.2015 21:39, Andrew Bresticker wrote: > Hi Mathias, > > On Mon, May 4, 2015 at 10:36 AM, Andrew Bresticker > wrote: >> xhci_gen_setup() sets the hcd_priv field for the primary HCD, but not >> for the shared HCD, requiring xHCI host-controller drivers to set it >> between usb_create_shared_hcd() and usb_add_hcd(). There's no reason >> xhci_gen_setup() can't set the shared HCD's hcd_priv as well, so move >> that bit out of the host-controller drivers and into xhci_gen_setup(). >> >> Signed-off-by: Andrew Bresticker >> Reviewed-by: Felipe Balbi >> Cc: Mathias Nyman >> Cc: Greg Kroah-Hartman > > Any chance you could pick this up for 4.2? It's a dependency for the > xhci-tegra driver which looks like it's going to slip to 4.3 now. > I got some internal tasks I need(ed) to attend, and I'm a bit late with the 4.2 patches. I'll try to sort it out next week. A got some patches from Roger Quadros where he changed how struct xhci_hcd is allocated to get xhci working with OTG code. That code will conflict with patch 1/9, so patch 1/9 will probably be dropped, but that shouldn't affect the rest of your patches. -Mathias -- 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: AM3517 usb host issue
Hi, On Fri, May 22, 2015 at 11:04:33AM +0300, Ben Dooks wrote: > I am trying to get the full-speed USB host working on an custom AM3517 > device with the 3.18.12 kernel. The hardware works (a 2.6.37 kernel has > been used for testing). > > Does anyone have any experience of 3.18 (or similarly recent kernel on > an AM3517 system) or have any pointers as where to start debugging? The > ti-linux-3.14.y does not have any patches that aren't applied to the > usb on 3.18.13. > > The cpu port 1 is connected by a TI TUSB1106 usb transceiver that is > directly connected to a full-speed hub (TI USB2046) hub so the OHCI > driver is the only one in use. > > Note, the ohci-omap3 is loaded as a module as this is how their user > application expects to be able to shut down usb when it does not need > it. > > The device tree configuration for the usb host: and what exactly doesn't work ? That old OHCI driver hasn't been touched in years, it's no surprise that it stopped working :-( Anyway, what exactly doesn't work ? No device enumerates ? Do you get any IRQs by plugging a new device in ? > > &usbhshost { > > status = "okay";/* just in case it is started disabled */ > > > > port1-mode = "ohci-phy-6pin-dpdm"; > > }; > > > > &usbhsohci { > > status = "okay"; > > }; > > > > &usbhsehci { > > status = "disabled";/* no ehci on board */ > > }; > > > The usb from the logs is as follows. Some extra debugging has been > added to verify the device-tree settings: > > > [0.00] AM3517 ES1.1 (l2cache sgx neon) > > > > > > [0.869706] usbcore: registered new interface driver usbfs > > > > [0.874270] usbcore: registered new interface driver hub > > > > [0.878592] usbcore: registered new device driver usb > > > > [1.223199] usbhs_tll 48062000.usbhstll: starting TI HSUSB TLL > > Controller > > [1.273000] usbhs_omap 48064000.usbhshost: ports 0 > > > > [1.278291] usbhs_omap 48064000.usbhshost: port 0: ohci-phy-6pin-dpdm > > > > [1.284476] usbhs_omap 48064000.usbhshost: port0-mode: > > ohci-phy-6pin-dpdm ->5 > > [1.288689] usbhs_tll 48062000.usbhstll: omap_tll_init() > > > > [1.293628] usbhs_omap 48064000.usbhshost: usbhs_runtime_resume > > > > [1.298434] usbhs_omap 48064000.usbhshost: sysconfig 0x1009 > > > > [1.302730] usbhs_tll 48062000.usbhstll: omap_tll_enable() > > > > [1.307668] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend > > > > [1.310142] stopping usb controller > > > > [1.419910] usbhs_tll 48062000.usbhstll: omap_tll_disable() > > > > [1.423547] usbhs_omap 48064000.usbhshost: 3 ports > > > > [1.429065] usbhs_omap 48064000.usbhshost: starting TI HSUSB Controller > > > > [1.433831] usbhs_omap 48064000.usbhshost: usbhs_runtime_resume > > > > [1.438625] usbhs_omap 48064000.usbhshost: sysconfig 0x1009 > > > > [1.442921] usbhs_tll 48062000.usbhstll: omap_tll_enable() > > > > [1.448548] usbhs_omap 48064000.usbhshost: omap_usbhs_rev1_hostconfig => > > > > [1.455034] usbhs_omap 48064000.usbhshost: UHH setup done, > > uhh_hostconfig=80d > > [1.459918] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend > > > > [1.462337] stopping usb controller > > > > [1.569905] usbhs_tll 48062000.usbhstll: omap_tll_disable() > > > > [1.575408] usbhs_omap 48064000.usbhshost: populating usb sub nodes > > > > > > [ 77.609168] usbhs_omap 48064000.usbhshost: usbhs_runtime_resume > > > > [ 77.613927] usbhs_omap 48064000.usbhshost: sysconfig 0x1009 > > > > [ 77.618374] usbhs_tll 48062000.usbhstll: omap_tll_enable() > > > > [ 77.802694] usb usb1: New USB device found, idVendor=1d6b, > > idProduct=0001 > > [ 77.816003] usb usb1: New USB device strings: Mfr=3, Product=2, > > SerialNumber1 > > [ 77.827391] usb usb1: Product: OHCI Host Controller > > > > [ 77.838674] usb usb1: Manufacturer: Linux 3.18.13-00203-ga3c52be-dirty > > ohci_d > > [ 77.849913] usb usb1: SerialNumber: 48064400.ohci > > OK, so this is roothub, what happens when a device is plugged to the other end ? Is VBUS charged ? We didn't even enumerate TUSB2046, did you look at its datasheet (http://www.ti.com/lit/ds/symlink/tusb2046b.pdf) ? What is the state of RESETn pin ? Perhaps that's tied to a GPIO and the old TI kernel toggles that ? Anything interesting from usbmon ? cheers -- balbi signature.asc Description: Digital signature
Re: USB Host Controller no PCD interrupt and CCS and CSC update on the device disconnect
On Fri, 22 May 2015, Rong Wang wrote: > Hi, > > We have one USB 2.0 Controller on an ARM SoC, with internal PHY > confirming to UTMI. > The PHY would detect unexpected disconnect (amplitude of the > differential envelop still < 625 mV) > and assert the HostDisconnect signal to the Controller to indicate a > disconnection event. > When the HostDisconnect signal is asserted, the Controller will first > do some internal clean work > before it update CCS and CSC in PORTSCx and reports a Port Change > Detect interrupt. > We want to improve the situation Why do you want to improve the situation? What's wrong with the way it is now? Detecting a disconnect while the differential amplitude is < 625 mv is perfectly legal. The USB-2.0 spec requires: "Signals with differential amplitudes <= 525 mV must never activate the Disconnection Envelope Detector" (section 7.1.7.3). > by masking the HostDisconnect signal > from PHY (take the > cost into consideration), but it will cause no CCS and CSC update in > PORTSCx, and no PCD interrupt. Thus the true disconnection only can be > determined by an XactErr. Once the driver > determine that it's a true disconnect, the HostDisconnect signal can > be unmasked and the > Controller will detect this disconnect. > We are worried about the workaroud, since there may be many corner > cases we have not taken into consideration, such as the race condition > on connect and disconnect. It sounds like you are going to make the situation worse, not improve it. > So have you encountered this situation before? Or is there any > suggestion on this workaround? I don't see any reason to make any changes. Alan Stern -- 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] usb: ulpi: don't register drivers if bus doesn't exist
On 05/22/2015 06:16 AM, Heikki Krogerus wrote: > diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c > > > >>index 0e6f968..0b0a5e7 100644 > > > >>--- a/drivers/usb/common/ulpi.c > > > >>+++ b/drivers/usb/common/ulpi.c > > > >>@@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver > > > >>*drv) > > > >>if (!drv->probe) > > > >>return -EINVAL; > > > >>+ /* Was the bus registered successfully? */ > > > >>+ if (!ulpi_bus.p) > > > >>+ return -ENODEV; >> > >> > I think we need to warn in this case. How about: >> > >> > if (unlikely(WARN_ON(!ulpi_bus.p))) No, please, the bus just doesn't exist - there's nothing wrong with that and there's no reason to trigger an alarm for the user. Nothing out of the ordinary happened here and the return value should be enough to tell the user what's up. This will cause a perma-WARN for folks who have that bus built in but don't actually have it on their system. >> > return -ENODEV; > I think we should also return -EAGAIN here. EAGAIN? For when a bus doesn't exist? How would a user retrying fix the issue? Thanks, Sasha -- 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 1/2] USB: io_ti: Add heartbeat to keep idle Edgeport ports from disconnecting
On Fri, May 15, 2015 at 12:09:53AM -0500, Peter E. Berger wrote: > From: "Peter E. Berger" > > When using newer Edgeport devices such as the EP/416, idle ports are > automatically bounced (disconnected and then reconnected) approximately > every 60 seconds. This breaks programs (e.g: minicom) where idle periods > are common, normal and expected. > > I confirmed with the manufacturer (Digi International) that Edgeports now > ship from the factory with firmware that expects periodic "heartbeat" > queries from the driver to keep ports alive. This patch implements > heartbeat support using the mechanism Digi suggested (requesting an > I2C descriptor address every 15 seconds) that appears effective on > Edgeports running the newer firmware (that require it) and benign on > Edgeport devices running older firmware. > > Signed-off-by: Peter E. Berger First of all, thanks for the patch. You should always run your patches through checkpatch before submitting, it would have let you know that there are some white space issues below (I will not comment on them further). You say you've tested this with older firmware, but I'd still prefer if we only enable this when actually needed. Just set a flag during probe if the firmware requires this heartbeat and only start it if needed. Just to clarify, is this hearbeat needed also when no port is open? > --- > drivers/usb/serial/io_ti.c | 68 > ++ > 1 file changed, 68 insertions(+) > > diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c > index ddbb8fe..1db929b 100644 > --- a/drivers/usb/serial/io_ti.c > +++ b/drivers/usb/serial/io_ti.c > @@ -101,6 +101,7 @@ struct edgeport_serial { > struct mutex es_lock; > int num_ports_open; > struct usb_serial *serial; > + struct delayed_work heartbeat_task; Rename heartbeat_work > }; > > > @@ -209,6 +210,8 @@ static void edge_send(struct usb_serial_port *port, > struct tty_struct *tty); > static int edge_create_sysfs_attrs(struct usb_serial_port *port); > static int edge_remove_sysfs_attrs(struct usb_serial_port *port); > > +/* Newer Edgeport firmware disconnects ports after periods of inactivity */ You should expand this comment a bit (e.g. explain what EP_HEARTBEAT_SECS is). > +#define EP_HEARTBEAT_SECS 15 Could this interval be increased from somewhat (you mention 60 seconds above)? > static int ti_vread_sync(struct usb_device *dev, __u8 request, > __u16 value, __u16 index, u8 *data, int size) > @@ -2373,11 +2376,33 @@ static void edge_break(struct tty_struct *tty, int > break_state) > __func__, status); > } > > +static void heartbeat(struct work_struct *taskp) Rename edge_heartbeat_work. > +{ > + struct edgeport_serial *serial; > + struct ti_i2c_desc rom_desc; This one cannot be allocated om the stack as it is eventually used for DMA. Use kmalloc. > + > + serial = container_of(taskp, struct edgeport_serial, > + heartbeat_task.work); > + > + dev_dbg(&serial->serial->dev->dev, "%s - serial:%s", > + __func__, serial->serial->dev->serial); Just remove this one. > + > + /* Descriptor address request is enough to reset the firmware timer */ > + if (!get_descriptor_addr(serial, I2C_DESC_TYPE_ION, &rom_desc)) > + dev_warn(&serial->serial->dev->dev, > + "%s - Incomplete heartbeat.", __func__); Use serial->serial->interface->dev for all messages throughout (won't mention again). Add braces to to if-block for readability. > + > + schedule_delayed_work(&serial->heartbeat_task, HZ * EP_HEARTBEAT_SECS); Use EP_HEARTBEAT_SECS * HZ throughout. > + > +} > + > static int edge_startup(struct usb_serial *serial) > { > struct edgeport_serial *edge_serial; > int status; > > + dev_dbg(&serial->dev->dev, "%s: serial:%s\n", __func__, > + serial->dev->serial); > /* create our private serial structure */ > edge_serial = kzalloc(sizeof(struct edgeport_serial), GFP_KERNEL); > if (!edge_serial) > @@ -2393,6 +2418,10 @@ static int edge_startup(struct usb_serial *serial) > return status; > } > > + INIT_DELAYED_WORK(&edge_serial->heartbeat_task, heartbeat); > + schedule_delayed_work(&edge_serial->heartbeat_task, > + HZ * EP_HEARTBEAT_SECS); > + > return 0; > } > > @@ -2458,8 +2487,11 @@ err: > static int edge_port_remove(struct usb_serial_port *port) > { > struct edgeport_port *edge_port; > + struct edgeport_serial *edge_serial; > edge_port = usb_get_serial_port_data(port); > + edge_serial = edge_port->edge_serial; > + cancel_delayed_work_sync(&edge_serial->heartbeat_task); This should be done in edge_remove (you have one heartbeat work per interface, not per port). > edge_remove_sysfs_attrs(port); > kfree(edge_port); > > @@
[PATCH 1/2] usb: gadget: g_ffs: Fix counting of missing_functions
Returning non-zero value from ready callback makes ffs instance return error from writing strings and enter FFS_CLOSING state. This means that this this function is not truly ready and close callback will not be called. This commit fix ffs_ready_callback() to undo all side effects of this function in case of error. Signed-off-by: Krzysztof Opasiak --- drivers/usb/gadget/legacy/g_ffs.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/legacy/g_ffs.c b/drivers/usb/gadget/legacy/g_ffs.c index b01b88e..9dde887 100644 --- a/drivers/usb/gadget/legacy/g_ffs.c +++ b/drivers/usb/gadget/legacy/g_ffs.c @@ -304,8 +304,10 @@ static int functionfs_ready_callback(struct ffs_data *ffs) gfs_registered = true; ret = usb_composite_probe(&gfs_driver); - if (unlikely(ret < 0)) + if (unlikely(ret < 0)) { + ++missing_funcs; gfs_registered = false; + } return ret; } -- 1.7.9.5 -- 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
[PATCH 2/2] usb: gadget: ffs: fix: Always call ffs_closed() in ffs_data_clear()
Originally FFS_FL_CALL_CLOSED_CALLBACK flag has been used to indicate if we should call ffs_closed_callback(). Commit 4b187fceec3c ("usb: gadget: FunctionFS: add devices management code") changed its semantic to indicate if we should call ffs_closed() function which does a little bit more. This situation leads to: [ 122.362269] [ cut here ] [ 122.362287] WARNING: CPU: 2 PID: 2384 at drivers/usb/gadget/function/f_fs.c:3417 ffs_ep0_write+0x730/0x810 [usb_f_fs]() [ 122.362292] Modules linked in: [ 122.362555] CPU: 2 PID: 2384 Comm: adbd Tainted: GW 4.1.0-0.rc4.git0.1.1.fc22.i686 #1 [ 122.362561] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/25/2014 [ 122.362567] c0d1f947 415badfa d1029e64 c0a86e54 d1029e94 c045b937 [ 122.362584] c0c37f94 0002 0950 f9b313d4 0d59 f9b2ebf0 f9b2ebf0 fff0 [ 122.362600] 0003 deb53d00 d1029ea4 c045ba42 0009 d1029f08 f9b2ebf0 [ 122.362617] Call Trace: [ 122.362633] [] dump_stack+0x41/0x52 [ 122.362645] [] warn_slowpath_common+0x87/0xc0 [ 122.362658] [] ? ffs_ep0_write+0x730/0x810 [usb_f_fs] [ 122.362668] [] ? ffs_ep0_write+0x730/0x810 [usb_f_fs] [ 122.362678] [] warn_slowpath_null+0x22/0x30 [ 122.362689] [] ffs_ep0_write+0x730/0x810 [usb_f_fs] [ 122.362702] [] ? ffs_ep0_read+0x380/0x380 [usb_f_fs] [ 122.362712] [] __vfs_write+0x2f/0x100 [ 122.362722] [] ? __sb_start_write+0x52/0x110 [ 122.362731] [] vfs_write+0x94/0x1b0 [ 122.362740] [] ? mutex_lock+0x10/0x30 [ 122.362749] [] SyS_write+0x51/0xb0 [ 122.362759] [] sysenter_do_call+0x12/0x12 [ 122.362766] ---[ end trace 0673d3467cecf8db ]--- in some cases (reproduction path below). This commit get back semantic of that flag and ensures that ffs_closed() is called always when needed but ffs_closed_callback() is called only if this flag is set. Reproduction path: Compile kernel without any UDC driver or bound some gadget to existing one and then: $ modprobe g_ffs $ mount none -t functionfs mount_point $ ffs-example mount_point This will fail with -ENODEV as there is no udc. $ ffs-example mount_point This will fail with -EBUSY because ffs_data has not been properly cleaned up. Signed-off-by: Krzysztof Opasiak --- drivers/usb/gadget/function/f_fs.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 6bdb570..71f68c4 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -315,7 +315,6 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf, return ret; } - set_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags); return len; } break; @@ -1463,8 +1462,7 @@ static void ffs_data_clear(struct ffs_data *ffs) { ENTER(); - if (test_and_clear_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags)) - ffs_closed(ffs); + ffs_closed(ffs); BUG_ON(ffs->gadget); @@ -3422,9 +3420,13 @@ static int ffs_ready(struct ffs_data *ffs) ffs_obj->desc_ready = true; ffs_obj->ffs_data = ffs; - if (ffs_obj->ffs_ready_callback) + if (ffs_obj->ffs_ready_callback) { ret = ffs_obj->ffs_ready_callback(ffs); + if (ret) + goto done; + } + set_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags); done: ffs_dev_unlock(); return ret; @@ -3443,7 +3445,8 @@ static void ffs_closed(struct ffs_data *ffs) ffs_obj->desc_ready = false; - if (ffs_obj->ffs_closed_callback) + if (test_and_clear_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags) && + ffs_obj->ffs_closed_callback) ffs_obj->ffs_closed_callback(ffs); if (!ffs_obj->opts || ffs_obj->opts->no_configfs -- 1.7.9.5 -- 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: Running ADB on a "stock" distribution (g_ffs)
On 05/21/2015 06:26 PM, Bastien Nocera wrote: On Thu, 2015-05-21 at 11:08 +0200, Krzysztof Opasiak wrote: On 05/21/2015 10:39 AM, Bastien Nocera wrote: On Thu, 2015-05-21 at 10:09 +0200, Krzysztof Opasiak wrote: Could you specify exactly the model? Onda v975w If android is running fine on it you may check android kernel config for this device and check which udc is enabled. No kernel sources from this Chinese vendor. But it looks like the USB_DWC3 config is the one to enable. I see that this tablet is running windows by default so just flash it with stock image and check what device manager says about udc;) The Baytrail UDC driver is supposed to be a DWC3 PCI device, but it doesn't show up in lspci at all. Given that I managed to make an NFC chip show up that I didn't know was in the machine, I guess it's possibly hidden in the firmware somewhere. In the meantime, shouldn't the ffs-test and adbd have failed with ENOTSUPP instead of EBUSY if no hardware support was available? As far as I understand the code it should fail with -ENODEV and debug message: pr_debug("couldn't find an available UDC\n"); I get that stack trace when writing is attempted: [ 122.362269] [ cut here ] [ 122.362287] WARNING: CPU: 2 PID: 2384 at drivers/usb/gadget/function/f_fs.c:3417 ffs_ep0_write+0x730/0x810 [usb_f_fs]() [ 122.362292] Modules linked in: g_ffs usb_f_fs libcomposite udc_core bnep bluetooth fuse hid_logitech_hidpp uvcvideo videobuf2_vmalloc snd_usb_audio videobuf2_core videobuf2_memops v4l2_common snd_usbmidi_lib videodev snd_hwdep snd_rawmidi media hid_multitouch hid_logitech_dj nf_conntrack_netbios_ns nf_conntrack_broadcast ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw vfat pn544_mei fat mei_phy pn544 intel_rapl hci intel_soc_dts_thermal coretemp nfc gpio_keys snd_soc_sst_baytrail_pcm [ 122.362412] kvm_intel iTCO_wdt snd_soc_sst_ipc iTCO_vendor_support snd_soc_sst_dsp kvm snd_soc_sst_byt_rt5640_mach crc32_pclmul crc32c_intel snd_intel_sst_acpi snd_intel_sst_core snd_soc_sst_mfld_platform snd_soc_rt5640 snd_soc_rl6231 snd_soc_core snd_compress snd_pcm_dmaengine tpm_crb snd_seq tpm dw_dmac int3402_thermal dw_dmac_core snd_seq_device int3400_thermal kxcjk_1013 soc_button_array snd_pcm processor_thermal_device int3403_thermal acpi_thermal_rel int340x_thermal_zone mei_txe industrialio_triggered_buffer goodix mei snd_timer lpc_ich ac i2c_hid kfifo_buf acpi_pad dell_smo8800 iosf_mbi industrialio snd soundcore pwm_lpss_platform rfkill_gpio battery regmap_i2c pwm_lpss i2c_designware_platform rfkill snd_soc_sst_acpi i2c_designware_core nfsd auth_rpcgss nfs_acl lockd grace sunrpc dm9601 usbnet [ 122.362529] mii i915 mmc_block i2c_algo_bit drm_kms_helper drm video sdhci_acpi sdhci mmc_core [ 122.362555] CPU: 2 PID: 2384 Comm: adbd Tainted: GW 4.1.0-0.rc4.git0.1.1.fc22.i686 #1 [ 122.362561] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/25/2014 [ 122.362567] c0d1f947 415badfa d1029e64 c0a86e54 d1029e94 c045b937 [ 122.362584] c0c37f94 0002 0950 f9b313d4 0d59 f9b2ebf0 f9b2ebf0 fff0 [ 122.362600] 0003 deb53d00 d1029ea4 c045ba42 0009 d1029f08 f9b2ebf0 [ 122.362617] Call Trace: [ 122.362633] [] dump_stack+0x41/0x52 [ 122.362645] [] warn_slowpath_common+0x87/0xc0 [ 122.362658] [] ? ffs_ep0_write+0x730/0x810 [usb_f_fs] [ 122.362668] [] ? ffs_ep0_write+0x730/0x810 [usb_f_fs] [ 122.362678] [] warn_slowpath_null+0x22/0x30 [ 122.362689] [] ffs_ep0_write+0x730/0x810 [usb_f_fs] [ 122.362702] [] ? ffs_ep0_read+0x380/0x380 [usb_f_fs] [ 122.362712] [] __vfs_write+0x2f/0x100 [ 122.362722] [] ? __sb_start_write+0x52/0x110 [ 122.362731] [] vfs_write+0x94/0x1b0 [ 122.362740] [] ? mutex_lock+0x10/0x30 [ 122.362749] [] SyS_write+0x51/0xb0 [ 122.362759] [] sysenter_do_call+0x12/0x12 [ 122.362766] ---[ end trace 0673d3467cecf8db ]--- I have managed to reproduce your situation when I compiled kernel without any UDC driver. This stack trace is related to a bug in ffs. I have prepared patches to fix this[1]. Unfortunately those patches will not make ffs working in your case. They will only make your adb failing with -ENODEV instead of -EBUSY as it should. Your basic problem is that you don't have any UDC in your system. Add one and you are ready to go;) Have you tried loading other gadget modules? No, I didn't try. If not, maybe try to load g_ether and check what dmesg says. I didn't have g_ether built, so tried with g_serial: # modprobe g_serial modprobe: ERROR: could not insert 'g_serial': No su
Re: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
Hi, On Fri, May 22, 2015 at 07:29:15PM +0800, Lu Baolu wrote: > Phy drivers and the ulpi interface providers depend on the > registeration of the ulpi bus. Ulpi registers the bus in > module_init(). This could result in a load order issue, i.e. It's still not an issue :( I'd say "unnecessary probe delays". But of cource it's Felipe's call :) Description looks better now. BR, David > ulpi phy drivers or the ulpi interface providers loading > before the bus registeration. > > This patch fixes this load order issue by putting ulpi_init > in subsys_initcall(). > > Reported-by: Zhuo Qiuxu > Signed-off-by: Lu Baolu > --- > drivers/usb/common/ulpi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c > index 0e6f968..01c0c04 100644 > --- a/drivers/usb/common/ulpi.c > +++ b/drivers/usb/common/ulpi.c > @@ -242,7 +242,7 @@ static int __init ulpi_init(void) > { > return bus_register(&ulpi_bus); > } > -module_init(ulpi_init); > +subsys_initcall(ulpi_init); > > static void __exit ulpi_exit(void) > { > -- > 2.1.4 > -- 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 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On Fri, May 22, 2015 at 03:50:47PM +0800, Lu, Baolu wrote: > > > On 05/22/2015 02:46 PM, Lu, Baolu wrote: > > > > > >On 05/22/2015 11:11 AM, David Cohen wrote: > >>On Thu, May 21, 2015 at 08:09:54PM -0700, David Cohen wrote: > >>>Hi, > >>> > >>>On Fri, May 22, 2015 at 10:07:05AM +0800, Lu Baolu wrote: > Many drivers and modules depend on ULPI bus registeration to > register ULPI interfaces and drivers. It's more appropriate > to register ULPI bus in subsys_initcall instead of module_init. > > Kernel panic has been reported with some kind of kernel config. > >>>Even though I agree subsys_initcall is better to register ulpi bus, > >>>it's > >>>still no excuse to have kernel panic. What about ULPI bus being > >>>compiled > >>>as module? > > > >No kernel panic if ULPI is built as a module. > > > >>>IMHO this is avoiding the proper kernel panic fix which should be > >>>failing gracefully (or defer probe) from tusb1210 driver. > >>Maybe I need to express myself better :) > >>IMHO we should not consider work done with this patch only. It's still > >>incomplete. > > > >I am with you on that we should know the real problem. > > > >I could go ahead with further debugging. Do you have any suggestions > >about which direction should I go? > > I forgot to mention that the panic was found in an Android environment. > The kernel version is v4.1-rc3. FWIW: The problem with Android environment is the amount of off-tree patches you may have over there. For upstream tasks, I'd suggest use a clean tree + patches you want to test. Usually yocto looks more friendly to test under this environment. Br, David > > > > >> > >>Br, David > > > >Thank you, > >-Baolu > > > >>-- > >>To unsubscribe from this list: send the line "unsubscribe linux-kernel" > >>in > >>the body of a message to majord...@vger.kernel.org > >>More majordomo info at http://vger.kernel.org/majordomo-info.html > >>Please read the FAQ at http://www.tux.org/lkml/ > >> > >> > > > >-- > >To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > >the body of a message to majord...@vger.kernel.org > >More majordomo info at http://vger.kernel.org/majordomo-info.html > >Please read the FAQ at http://www.tux.org/lkml/ > > > -- 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 2/2] USB: io_ti: Fix Edgeport firmware download code
On Fri, May 15, 2015 at 12:09:54AM -0500, Peter E. Berger wrote: > From: "Peter E. Berger" > > The io_ti driver fails to download firmware to Edgeport devices such > as the EP/416, even when the on-disk firmware image > (/lib/firmware/edgeport/down3.bin) is more current than the version > on the EP/416. The current download code is broken in a few ways. > Notably it mis-uses global variables OperationalMajorVersion and > OperationalMinorVersion (reading their values before they've been properly > initialized and subsequently initializing them multiple times without > synchronization) and using an insufficient timeout in ti_vsend_sync() > when doing UMPC_COPY_DNLD_TO_I2C operations. With this patch, firmware > downloads work as expected: if the on disk firmware version is newer > than that on the device, it will be downloaded. Thanks for fixing this. We should probably consider backporting some of this to stable as well. But if you're fixing several independent issues, these should go in separate patches if possible. > Signed-off-by: Peter E. Berger > --- > drivers/usb/serial/io_ti.c | 72 > ++ > 1 file changed, 53 insertions(+), 19 deletions(-) > > diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c > index 1db929b..7e40fbb 100644 > --- a/drivers/usb/serial/io_ti.c > +++ b/drivers/usb/serial/io_ti.c > @@ -188,10 +188,6 @@ static const struct usb_device_id id_table_combined[] = { > > MODULE_DEVICE_TABLE(usb, id_table_combined); > > -static unsigned char OperationalMajorVersion; > -static unsigned char OperationalMinorVersion; > -static unsigned short OperationalBuildNumber; > - > static int closing_wait = EDGE_CLOSING_WAIT; > static bool ignore_cpu_rev; > static int default_uart_mode;/* RS232 */ > @@ -213,6 +209,25 @@ static int edge_remove_sysfs_attrs(struct > usb_serial_port *port); > /* Newer Edgeport firmware disconnects ports after periods of inactivity */ > #define EP_HEARTBEAT_SECS 15 > > +int wrap_request_firmware(const struct firmware **fw, struct device *dev, > + __u8 *MajorVersion, __u8 *MinorVersion, __u16 *BuildNumber) Is it really necessary to wrap the request call? Looks like what you really want is a edge_fw_get_version() helper. And I'm not sure even that is really needed to fix the bug at hand (we want to keep fixes minimal). I know this driver wasn't a good example, but don't use CamelCase. Also drop __ from the types here and elsewhere. > +{ > + const char *fw_name = "edgeport/down3.bin"; > + int err; > + > + err = request_firmware(fw, fw_name, dev); > + if (err) { > + dev_dbg(dev, "%s - Failed to load image \"%s\" err %d\n", > +__func__, fw_name, err); > + } else { Return err here and skip the else clause. > + /* Save Download Version Number */ > + *MajorVersion = (*fw)->data[0]; > + *MinorVersion = (*fw)->data[1]; > + *BuildNumber = le16_to_cpup((__le16 *)&(*fw)->data[2]); Use get_unaligned_le16(). > + } > + return err; return 0 > +} > + > static int ti_vread_sync(struct usb_device *dev, __u8 request, > __u16 value, __u16 index, u8 *data, int size) > { > @@ -235,10 +250,13 @@ static int ti_vsend_sync(struct usb_device *dev, __u8 > request, > __u16 value, __u16 index, u8 *data, int size) > { > int status; > + int timeout = 1000; /* Timeout in msecs */ > > + if (request == UMPC_COPY_DNLD_TO_I2C) /* Downloads take longer */ > + timeout = 1; > status = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request, > (USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT), > - value, index, data, size, 1000); > + value, index, data, size, timeout); > if (status < 0) > return status; > if (status != size) { > @@ -762,7 +780,9 @@ static int build_i2c_fw_hdr(__u8 *header, struct device > *dev) > struct ti_i2c_image_header *img_header; > struct ti_i2c_firmware_rec *firmware_rec; > const struct firmware *fw; > - const char *fw_name = "edgeport/down3.bin"; > + __u8 OperationalMajorVersion = 0; > + __u8 OperationalMinorVersion = 0; > + __u16 OperationalBuildNumber = 0; CamelCase could be ok here in order to minimise diff. Could be fixed in a follow up patch. [...] > /* This routine is entered by both the BOOT mode and the Download mode >* We can determine which code is running by the reading the config > @@ -1046,6 +1065,19 @@ static int download_fw(struct edgeport_serial *serial) > return status; > } > > + /* Get version numbers for on-disk fw */ > + if (wrap_request_firmware(&fw, dev, > + &OperationalMajorVe
Re: [PATCH net,stable-4.0] cdc_ncm: Fix tx_bytes statistics
On Fri, 2015-05-22 at 13:15 +0200, Bjørn Mork wrote: > The tx_curr_frame_payload field is u32. When we try to calculate a > small negative delta based on it, we end up with a positive integer > close to 2^32 instead. So the tx_bytes pointer increases by about > 2^32 for every transmitted frame. > > Fix by calculating the delta as a signed long. Told you I hadn't tested it. Ben. > Cc: Ben Hutchings > Reported-by: Florian Bruhin > Fixes: 7a1e890e2168 ("usbnet: Fix tx_bytes statistic running backward in > cdc_ncm") > Signed-off-by: Bjørn Mork > --- > drivers/net/usb/cdc_ncm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c > index c3e4da9e79ca..8067b8fbb0ee 100644 > --- a/drivers/net/usb/cdc_ncm.c > +++ b/drivers/net/usb/cdc_ncm.c > @@ -1182,7 +1182,7 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct > sk_buff *skb, __le32 sign) >* payload data instead. >*/ > usbnet_set_skb_tx_stats(skb_out, n, > - ctx->tx_curr_frame_payload - skb_out->len); > + (long)ctx->tx_curr_frame_payload - > skb_out->len); > > return skb_out; > -- 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: [BUG] USB 3.0 device "lost for good" when in SS.inactive or Compliance Mode during hub_port_reset()
On Fri, 22 May 2015, Robert Schlabbach wrote: > On Thu, 21 May 2015, Alan Stern wrote: > > > I suspect you have not divided up all the actions in the correct way. > > Look at everything in hub_port_finish_reset and think carefully about > > where each one belongs. > > I had taken a very "conservative" approach to only minimally alter the > behavior specifically for the fix I needed, since I understand too little > of the code to know what I might break if I made bolder changes. > > Now following "bold" approach, I went all the way and dissolved > hub_port_finish_reset() completely, moving all of its actions over to > hub_port_reset(). Moving the clearing of the port change bits over to > hub_port_wait_reset() seemed impractical to me, since that has many > return statements in it, and the clearing must be done in every case, so > it's simpler to put this in hub_port_reset() after hub_port_wait_reset() > has returned. > > While at it, I also changed the error handling for the initial "double > check" and removed an unneeded advance declaration of hub_port_reset(). Okay. > The resulting unified (!:-)) diff patch versus 3.19.8 is: > > > diff -u -r a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > --- a/drivers/usb/core/hub.c 2015-05-21 00:11:25.0 +0200 > +++ b/drivers/usb/core/hub.c 2015-05-22 08:15:37.199743870 +0200 > @@ -2616,9 +2616,6 @@ > return USE_NEW_SCHEME(retry); > } > > -static int hub_port_reset(struct usb_hub *hub, int port1, > - struct usb_device *udev, unsigned int delay, bool warm); > - > /* Is a USB 3.0 port in the Inactive or Compliance Mode state? > * Port worm reset is required to recover > */ > @@ -2706,44 +2703,6 @@ > return 0; > } > > -static void hub_port_finish_reset(struct usb_hub *hub, int port1, > - struct usb_device *udev, int *status) > -{ > - switch (*status) { > - case 0: > - /* TRSTRCY = 10 ms; plus some extra */ > - msleep(10 + 40); > - if (udev) { > - struct usb_hcd *hcd = bus_to_hcd(udev->bus); > - > - update_devnum(udev, 0); > - /* The xHC may think the device is already reset, > - * so ignore the status. > - */ > - if (hcd->driver->reset_device) > - hcd->driver->reset_device(hcd, udev); > - } > - /* FALL THROUGH */ > - case -ENOTCONN: > - case -ENODEV: > - usb_clear_port_feature(hub->hdev, > - port1, USB_PORT_FEAT_C_RESET); > - if (hub_is_superspeed(hub->hdev)) { > - usb_clear_port_feature(hub->hdev, port1, > - USB_PORT_FEAT_C_BH_PORT_RESET); > - usb_clear_port_feature(hub->hdev, port1, > - USB_PORT_FEAT_C_PORT_LINK_STATE); > - usb_clear_port_feature(hub->hdev, port1, > - USB_PORT_FEAT_C_CONNECTION); > - } > - if (udev) > - usb_set_device_state(udev, *status > - ? USB_STATE_NOTATTACHED > - : USB_STATE_DEFAULT); > - break; > - } > -} > - > /* Handle port reset and port warm(BH) reset (for USB3 protocol ports) */ > static int hub_port_reset(struct usb_hub *hub, int port1, > struct usb_device *udev, unsigned int delay, bool warm) > @@ -2767,17 +2726,15 @@ >* If the caller hasn't explicitly requested a warm reset, >* double check and see if one is needed. >*/ > - status = hub_port_status(hub, port1, > - &portstatus, &portchange); > - if (status < 0) > - goto done; So you removed this goto. Fair enough; it's probably not useful. > - > - if (hub_port_warm_reset_required(hub, port1, portstatus)) > - warm = true; > + if (hub_port_status(hub, port1, &portstatus, &portchange) == 0) > { > + if (hub_port_warm_reset_required(hub, port1, > portstatus)) > + warm = true; > + } > } > clear_bit(port1, hub->warm_reset_bits); > > /* Reset the port */ > + status = 0; I don't think this line is necessary, because status gets overwritten two lines below. > for (i = 0; i < PORT_RESET_TRIES; i++) { > status = set_port_feature(hub->hdev, port1, (warm ? > USB_PORT_FEAT_BH_PORT_RESET : > @@ -2795,11 +2752,22 @@ > dev_dbg(hub->intfdev, > "port_wait_reset: err = %d\n", > status); > + > +
Re: [PATCH net,stable-4.0] cdc_ncm: Fix tx_bytes statistics
From: Bjørn Mork Date: Fri, 22 May 2015 13:15:22 +0200 > The tx_curr_frame_payload field is u32. When we try to calculate a > small negative delta based on it, we end up with a positive integer > close to 2^32 instead. So the tx_bytes pointer increases by about > 2^32 for every transmitted frame. > > Fix by calculating the delta as a signed long. > > Cc: Ben Hutchings > Reported-by: Florian Bruhin > Fixes: 7a1e890e2168 ("usbnet: Fix tx_bytes statistic running backward in > cdc_ncm") > Signed-off-by: Bjørn Mork Applied and queued up for -stable. -- 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 net,stable-4.0] cdc_ncm: Fix tx_bytes statistics
On 22 May 2015 19:22:47 CEST, Ben Hutchings wrote: >On Fri, 2015-05-22 at 13:15 +0200, Bjørn Mork wrote: >> The tx_curr_frame_payload field is u32. When we try to calculate a >> small negative delta based on it, we end up with a positive integer >> close to 2^32 instead. So the tx_bytes pointer increases by about >> 2^32 for every transmitted frame. >> >> Fix by calculating the delta as a signed long. > >Told you I hadn't tested it. Yes, I know. This was all my bad. Sorry. Didn't mean to imply that you were to blame here. On the contrary. Thanks again for showing how to solve this. Bjørn -- 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 4/5] phy: add Marvell HSIC 28nm PHY
On Thu, May 21, 2015 at 7:51 AM, Kishon Vijay Abraham I wrote: > On Thursday 21 May 2015 06:15 PM, Kishon Vijay Abraham I wrote: >> >> Hi, >> >> On Thursday 14 May 2015 04:18 AM, Rob Herring wrote: >>> >>> Add PHY driver for the Marvell HSIC 28nm PHY. This PHY is found in >>> PXA1928 >>> SOC. [...] >>> + writel(readl(base + PHY_28NM_HSIC_CTRL) & >>> ~PHY_28NM_HSIC_S2H_HSIC_EN, >>> + base + PHY_28NM_HSIC_CTRL); >>> + >>> + clk_disable_unprepare(mv_phy->clk); >>> + return 0; >>> +} >>> + >>> +static const struct phy_ops hsic_ops = { >>> + .init = mv_hsic_phy_init, >>> + .power_on = mv_hsic_phy_power_on, >>> + .power_off = mv_hsic_phy_power_off, >> >> >> exit callback is missing? Shouldn't we turn off the PLLs in exit callback? I really don't understand the division of the ops functions. It seems backwards to me. Don't you need to power on the phy before you can initialize it? Or init is supposed to be s/w init of some kind. AFAICT, all the drivers just call init and power_on back to back. > > Also add the .owner member since this driver can be used as module. Strange. Generally an ops struct just has ops. Rob -- 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: AM3517 usb host issue
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 22/05/15 16:50, Felipe Balbi wrote: > Hi, > > On Fri, May 22, 2015 at 11:04:33AM +0300, Ben Dooks wrote: >> I am trying to get the full-speed USB host working on an custom >> AM3517 device with the 3.18.12 kernel. The hardware works (a >> 2.6.37 kernel has been used for testing). >> >> Does anyone have any experience of 3.18 (or similarly recent >> kernel on an AM3517 system) or have any pointers as where to >> start debugging? The ti-linux-3.14.y does not have any patches >> that aren't applied to the usb on 3.18.13. >> >> The cpu port 1 is connected by a TI TUSB1106 usb transceiver that >> is directly connected to a full-speed hub (TI USB2046) hub so the >> OHCI driver is the only one in use. >> >> Note, the ohci-omap3 is loaded as a module as this is how their >> user application expects to be able to shut down usb when it does >> not need it. >> >> The device tree configuration for the usb host: > > and what exactly doesn't work ? That old OHCI driver hasn't been > touched in years, it's no surprise that it stopped working :-( > > Anyway, what exactly doesn't work ? No device enumerates ? Do you > get any IRQs by plugging a new device in ? I will check the interrupts when I get back into the office. There is only one device (the hub) which isn't enumerating and is hard-wired on the board. >>> &usbhshost { status = "okay"; /* just in case it is started >>> disabled */ >>> >>> port1-mode = "ohci-phy-6pin-dpdm"; }; >>> >>> &usbhsohci { status = "okay"; }; >>> >>> &usbhsehci { status = "disabled"; /* no ehci on board */ }; >> >> >> The usb from the logs is as follows. Some extra debugging has >> been added to verify the device-tree settings: >> >>> [0.00] AM3517 ES1.1 (l2cache sgx neon) >>> >>> >>> [0.869706] usbcore: registered new interface driver usbfs >>> [0.874270] usbcore: registered new interface driver hub >>> [0.878592] usbcore: registered new device driver usb >>> [1.223199] usbhs_tll 48062000.usbhstll: starting TI HSUSB >>> TLL Controller [1.273000] usbhs_omap 48064000.usbhshost: >>> ports 0 [1.278291] usbhs_omap 48064000.usbhshost: port 0: >>> ohci-phy-6pin-dpdm [1.284476] usbhs_omap >>> 48064000.usbhshost: port0-mode: ohci-phy-6pin-dpdm ->5 [ >>> 1.288689] usbhs_tll 48062000.usbhstll: omap_tll_init() >>> [1.293628] usbhs_omap 48064000.usbhshost: >>> usbhs_runtime_resume [1.298434] usbhs_omap >>> 48064000.usbhshost: sysconfig 0x1009 [1.302730] >>> usbhs_tll 48062000.usbhstll: omap_tll_enable() >>> [1.307668] usbhs_omap 48064000.usbhshost: >>> usbhs_runtime_suspend [1.310142] stopping usb controller >>> [1.419910] usbhs_tll 48062000.usbhstll: omap_tll_disable() >>> [1.423547] usbhs_omap 48064000.usbhshost: 3 ports >>> [1.429065] usbhs_omap 48064000.usbhshost: starting TI >>> HSUSB Controller [1.433831] usbhs_omap 48064000.usbhshost: >>> usbhs_runtime_resume [1.438625] usbhs_omap >>> 48064000.usbhshost: sysconfig 0x1009 [1.442921] >>> usbhs_tll 48062000.usbhstll: omap_tll_enable() >>> [1.448548] usbhs_omap 48064000.usbhshost: >>> omap_usbhs_rev1_hostconfig => [1.455034] usbhs_omap >>> 48064000.usbhshost: UHH setup done, uhh_hostconfig=80d [ >>> 1.459918] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend >>> [1.462337] stopping usb controller >>> [1.569905] usbhs_tll 48062000.usbhstll: omap_tll_disable() >>> [1.575408] usbhs_omap 48064000.usbhshost: populating usb >>> sub nodes >>> >>> [ 77.609168] usbhs_omap 48064000.usbhshost: >>> usbhs_runtime_resume [ 77.613927] usbhs_omap >>> 48064000.usbhshost: sysconfig 0x1009 [ 77.618374] >>> usbhs_tll 48062000.usbhstll: omap_tll_enable() >>> [ 77.802694] usb usb1: New USB device found, idVendor=1d6b, >>> idProduct=0001 [ 77.816003] usb usb1: New USB device strings: >>> Mfr=3, Product=2, SerialNumber1 [ 77.827391] usb usb1: >>> Product: OHCI Host Controller [ 77.838674] usb usb1: >>> Manufacturer: Linux 3.18.13-00203-ga3c52be-dirty ohci_d [ >>> 77.849913] usb usb1: SerialNumber: 48064400.ohci >>> > > OK, so this is roothub, what happens when a device is plugged to > the other end ? Is VBUS charged ? We didn't even enumerate > TUSB2046, did you look at its datasheet > (http://www.ti.com/lit/ds/symlink/tusb2046b.pdf) ? What is the > state of RESETn pin ? Perhaps that's tied to a GPIO and the old TI > kernel toggles that ? Anything interesting from usbmon ? I've gone through the schematics and verified the hub's reset line is connected where we think it is (will try and check with a 'scope on monday that it is being taken high, the boot-loader starts it low). The root has just the one device connected, and unfortunately the hw designer decided to hard-wire the D+ pull-up to 3.3V instead of the transceiver's pull pin. Not tried usbmon yet. However if the hub is not being detected then probably won't show anything either. I am going t
seagate SRD00f2 expansion 4Tb desktop drive does not work on USB 3.0
This drive works on USB 2.0 but not USB 3.0. I saw a procedure on here to use this statement in the modprobe.d directory: "options usb-storage quirks=Vendor_ID:Product_ID:u" That did not work. I get the same response that I did before when plugging it into a USB 3.0 port. May 22 18:16:45 (none) kernel: [ 1354.449411] usb 3-3.1: new high-speed USB device number 11 using xhci_hcd May 22 18:16:45 (none) kernel: [ 1354.449501] usb 3-3.1: Device not responding to set address. May 22 18:16:45 (none) kernel: [ 1354.653379] usb 3-3.1: Device not responding to set address. May 22 18:16:45 (none) kernel: [ 1354.857295] usb 3-3.1: device not accepting address 11, error -71 Here is my kernel version: 3.13.0-52-generic #86-Ubuntu SMP I get basically the same response when I run kernel vmlinuz-4.0.1-1.el7.elrepo.x86_64 with Centos. I am not running any mod called uas and can't load it because it does not exist. After plugging it into a USB 2.0 port i get: "Bus 003 Device 013: ID 0bc2:3312 Seagate RSS LLC " Bus 003 Device 013: ID 0bc2:3312 Seagate RSS LLC Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.10 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x0bc2 Seagate RSS LLC idProduct 0x3312 bcdDevice7.39 iManufacturer 1 Seagate iProduct2 Expansion Desk iSerial 3 NA4MZS4N bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 85 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower2mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk-Only iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x8b EP 11 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x0a EP 10 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 1 bNumEndpoints 4 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 98 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x08 EP 8 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Command pipe (0x01) Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x89 EP 9 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Status pipe (0x02) Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x0a EP 10 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Data-out pipe (0x04) Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x8b EP 11 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Data-in pipe (0x03) Binary Object Store Descriptor: bLength 5 bDescriptorType15 wTotalLength 22 bNumDevi
Re: AM3517 usb host issue
Hi, On Fri, May 22, 2015 at 11:13:22PM +0300, Ben Dooks wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA256 > > On 22/05/15 16:50, Felipe Balbi wrote: > > Hi, > > > > On Fri, May 22, 2015 at 11:04:33AM +0300, Ben Dooks wrote: > >> I am trying to get the full-speed USB host working on an custom > >> AM3517 device with the 3.18.12 kernel. The hardware works (a > >> 2.6.37 kernel has been used for testing). > >> > >> Does anyone have any experience of 3.18 (or similarly recent > >> kernel on an AM3517 system) or have any pointers as where to > >> start debugging? The ti-linux-3.14.y does not have any patches > >> that aren't applied to the usb on 3.18.13. > >> > >> The cpu port 1 is connected by a TI TUSB1106 usb transceiver that > >> is directly connected to a full-speed hub (TI USB2046) hub so the > >> OHCI driver is the only one in use. > >> > >> Note, the ohci-omap3 is loaded as a module as this is how their > >> user application expects to be able to shut down usb when it does > >> not need it. > >> > >> The device tree configuration for the usb host: > > > > and what exactly doesn't work ? That old OHCI driver hasn't been > > touched in years, it's no surprise that it stopped working :-( > > > > Anyway, what exactly doesn't work ? No device enumerates ? Do you > > get any IRQs by plugging a new device in ? > > I will check the interrupts when I get back into the office. > > There is only one device (the hub) which isn't enumerating and is > hard-wired on the board. alright, that makes things a little more difficult :-) > >>> &usbhshost { status = "okay"; /* just in case it is started > >>> disabled */ > >>> > >>> port1-mode = "ohci-phy-6pin-dpdm"; }; > >>> > >>> &usbhsohci { status = "okay"; }; > >>> > >>> &usbhsehci { status = "disabled"; /* no ehci on board */ }; > >> > >> > >> The usb from the logs is as follows. Some extra debugging has > >> been added to verify the device-tree settings: > >> > >>> [0.00] AM3517 ES1.1 (l2cache sgx neon) > >>> > >>> > >>> [0.869706] usbcore: registered new interface driver usbfs > >>> [0.874270] usbcore: registered new interface driver hub > >>> [0.878592] usbcore: registered new device driver usb > >>> [1.223199] usbhs_tll 48062000.usbhstll: starting TI HSUSB > >>> TLL Controller [1.273000] usbhs_omap 48064000.usbhshost: > >>> ports 0 [1.278291] usbhs_omap 48064000.usbhshost: port 0: > >>> ohci-phy-6pin-dpdm [1.284476] usbhs_omap > >>> 48064000.usbhshost: port0-mode: ohci-phy-6pin-dpdm ->5 [ > >>> 1.288689] usbhs_tll 48062000.usbhstll: omap_tll_init() > >>> [1.293628] usbhs_omap 48064000.usbhshost: > >>> usbhs_runtime_resume [1.298434] usbhs_omap > >>> 48064000.usbhshost: sysconfig 0x1009 [1.302730] > >>> usbhs_tll 48062000.usbhstll: omap_tll_enable() > >>> [1.307668] usbhs_omap 48064000.usbhshost: > >>> usbhs_runtime_suspend [1.310142] stopping usb controller > >>> [1.419910] usbhs_tll 48062000.usbhstll: omap_tll_disable() > >>> [1.423547] usbhs_omap 48064000.usbhshost: 3 ports > >>> [1.429065] usbhs_omap 48064000.usbhshost: starting TI > >>> HSUSB Controller [1.433831] usbhs_omap 48064000.usbhshost: > >>> usbhs_runtime_resume [1.438625] usbhs_omap > >>> 48064000.usbhshost: sysconfig 0x1009 [1.442921] > >>> usbhs_tll 48062000.usbhstll: omap_tll_enable() > >>> [1.448548] usbhs_omap 48064000.usbhshost: > >>> omap_usbhs_rev1_hostconfig => [1.455034] usbhs_omap > >>> 48064000.usbhshost: UHH setup done, uhh_hostconfig=80d [ > >>> 1.459918] usbhs_omap 48064000.usbhshost: usbhs_runtime_suspend > >>> [1.462337] stopping usb controller > >>> [1.569905] usbhs_tll 48062000.usbhstll: omap_tll_disable() > >>> [1.575408] usbhs_omap 48064000.usbhshost: populating usb > >>> sub nodes > >>> > >>> [ 77.609168] usbhs_omap 48064000.usbhshost: > >>> usbhs_runtime_resume [ 77.613927] usbhs_omap > >>> 48064000.usbhshost: sysconfig 0x1009 [ 77.618374] > >>> usbhs_tll 48062000.usbhstll: omap_tll_enable() > >>> [ 77.802694] usb usb1: New USB device found, idVendor=1d6b, > >>> idProduct=0001 [ 77.816003] usb usb1: New USB device strings: > >>> Mfr=3, Product=2, SerialNumber1 [ 77.827391] usb usb1: > >>> Product: OHCI Host Controller [ 77.838674] usb usb1: > >>> Manufacturer: Linux 3.18.13-00203-ga3c52be-dirty ohci_d [ > >>> 77.849913] usb usb1: SerialNumber: 48064400.ohci > >>> > > > > OK, so this is roothub, what happens when a device is plugged to > > the other end ? Is VBUS charged ? We didn't even enumerate > > TUSB2046, did you look at its datasheet > > (http://www.ti.com/lit/ds/symlink/tusb2046b.pdf) ? What is the > > state of RESETn pin ? Perhaps that's tied to a GPIO and the old TI > > kernel toggles that ? Anything interesting from usbmon ? > > I've gone through the schematics and verified the hub's reset line > is connected where we think it is (will try and check with a 'scope > on monday that it is being
[PATCH 2/2] usb: phy: tahvo: Pass the IRQF_ONESHOT flag
From: Fabio Estevam Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests") threaded IRQs without a primary handler need to be requested with IRQF_ONESHOT, otherwise the request will fail. So pass the IRQF_ONESHOT flag in this case. The semantic patch that makes this change is available in scripts/coccinelle/misc/irqf_oneshot.cocci. Signed-off-by: Fabio Estevam --- drivers/usb/phy/phy-tahvo.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c index 845f658..2b28443 100644 --- a/drivers/usb/phy/phy-tahvo.c +++ b/drivers/usb/phy/phy-tahvo.c @@ -401,7 +401,8 @@ static int tahvo_usb_probe(struct platform_device *pdev) dev_set_drvdata(&pdev->dev, tu); tu->irq = platform_get_irq(pdev, 0); - ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt, 0, + ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt, + IRQF_ONESHOT, "tahvo-vbus", tu); if (ret) { dev_err(&pdev->dev, "could not register tahvo-vbus irq: %d\n", -- 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
[PATCH 1/2] usb: phy: ab8500-usb: Pass the IRQF_ONESHOT flag
From: Fabio Estevam Since commit 1c6c69525b40 ("genirq: Reject bogus threaded irq requests") threaded IRQs without a primary handler need to be requested with IRQF_ONESHOT, otherwise the request will fail. So pass the IRQF_ONESHOT flag in this case. The semantic patch that makes this change is available in scripts/coccinelle/misc/irqf_oneshot.cocci. Signed-off-by: Fabio Estevam --- drivers/usb/phy/phy-ab8500-usb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/phy/phy-ab8500-usb.c b/drivers/usb/phy/phy-ab8500-usb.c index f063b2b..0c912d3 100644 --- a/drivers/usb/phy/phy-ab8500-usb.c +++ b/drivers/usb/phy/phy-ab8500-usb.c @@ -1179,7 +1179,7 @@ static int ab8500_usb_irq_setup(struct platform_device *pdev, } err = devm_request_threaded_irq(&pdev->dev, irq, NULL, ab8500_usb_link_status_irq, - IRQF_NO_SUSPEND | IRQF_SHARED, + IRQF_NO_SUSPEND | IRQF_SHARED | IRQF_ONESHOT, "usb-link-status", ab); if (err < 0) { dev_err(ab->dev, "request_irq failed for link status irq\n"); @@ -1195,7 +1195,7 @@ static int ab8500_usb_irq_setup(struct platform_device *pdev, } err = devm_request_threaded_irq(&pdev->dev, irq, NULL, ab8500_usb_disconnect_irq, - IRQF_NO_SUSPEND | IRQF_SHARED, + IRQF_NO_SUSPEND | IRQF_SHARED | IRQF_ONESHOT, "usb-id-fall", ab); if (err < 0) { dev_err(ab->dev, "request_irq failed for ID fall irq\n"); @@ -1211,7 +1211,7 @@ static int ab8500_usb_irq_setup(struct platform_device *pdev, } err = devm_request_threaded_irq(&pdev->dev, irq, NULL, ab8500_usb_disconnect_irq, - IRQF_NO_SUSPEND | IRQF_SHARED, + IRQF_NO_SUSPEND | IRQF_SHARED | IRQF_ONESHOT, "usb-vbus-fall", ab); if (err < 0) { dev_err(ab->dev, "request_irq failed for Vbus fall irq\n"); -- 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