Re: [Bug 103461] i.MX USB runtime power management breaks the boot
On Tue, Aug 25, 2015 at 1:07 PM, Vincent Stehlé vincent.ste...@freescale.com wrote: Hi, I have encountered a bug related to the USB runtime pm in a specific boot configuration. The issue happens on an i.MX6 SabreSD platform with recent kernel versions (e.g. v4.2-rc8) and imx_v6_v7_defconfig, when booting u-boot by USB and the kernel by ethernet. I have bisected to a specific commit. More details and logs related to this bug are captured in the kernel bugzilla there : https://bugzilla.kernel.org/show_bug.cgi?id=103461 Thank you for your help and comments. Adding Peter Chen on Cc. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usbnet: Get EVENT_NO_RUNTIME_PM bit before it is cleared
On Mon, 2015-08-24 at 23:13 +0300, Eugene Shatokhin wrote: It is needed to check EVENT_NO_RUNTIME_PM bit of dev-flags in usbnet_stop(), but its value should be read before it is cleared when dev-flags is set to 0. The problem was spotted and the fix was provided by Oliver Neukum oneu...@suse.de. Signed-off-by: Eugene Shatokhin eugene.shatok...@rosalab.ru Acked-by: Oliver Neukum oneu...@suse.com -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usbnet: Get EVENT_NO_RUNTIME_PM bit before it is cleared
Oliver Neukum oneu...@suse.com writes: On Mon, 2015-08-24 at 23:13 +0300, Eugene Shatokhin wrote: It is needed to check EVENT_NO_RUNTIME_PM bit of dev-flags in usbnet_stop(), but its value should be read before it is cleared when dev-flags is set to 0. Can we agree that this at least is good and should go upstream and into stable? I definitely agree. 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
Fwd: [Bug 103461] i.MX USB runtime power management breaks the boot
Hi, I have encountered a bug related to the USB runtime pm in a specific boot configuration. The issue happens on an i.MX6 SabreSD platform with recent kernel versions (e.g. v4.2-rc8) and imx_v6_v7_defconfig, when booting u-boot by USB and the kernel by ethernet. I have bisected to a specific commit. More details and logs related to this bug are captured in the kernel bugzilla there : https://bugzilla.kernel.org/show_bug.cgi?id=103461 Thank you for your help and comments. Best regards, V. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usbnet: Get EVENT_NO_RUNTIME_PM bit before it is cleared
From: Eugene Shatokhin eugene.shatok...@rosalab.ru Date: Mon, 24 Aug 2015 23:13:42 +0300 It is needed to check EVENT_NO_RUNTIME_PM bit of dev-flags in usbnet_stop(), but its value should be read before it is cleared when dev-flags is set to 0. The problem was spotted and the fix was provided by Oliver Neukum oneu...@suse.de. Signed-off-by: Eugene Shatokhin eugene.shatok...@rosalab.ru 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 2/2] usbnet: Fix a race between usbnet_stop() and the BH
From: Eugene Shatokhin eugene.shatok...@rosalab.ru Date: Mon, 24 Aug 2015 23:13:43 +0300 The race may happen when a device (e.g. YOTA 4G LTE Modem) is unplugged while the system is downloading a large file from the Net. Hardware breakpoints and Kprobes with delays were used to confirm that the race does actually happen. The race is on skb_queue ('next' pointer) between usbnet_stop() and rx_complete(), which, in turn, calls usbnet_bh(). Here is a part of the call stack with the code where the changes to the queue happen. The line numbers are for the kernel 4.1.0: ... It looks like this patch needs more discussion/work. -- 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 103461] i.MX USB runtime power management breaks the boot
On Tue, Aug 25, 2015 at 02:37:45PM -0300, Fabio Estevam wrote: On Tue, Aug 25, 2015 at 1:07 PM, Vincent Stehlé vincent.ste...@freescale.com wrote: Hi, I have encountered a bug related to the USB runtime pm in a specific boot configuration. The issue happens on an i.MX6 SabreSD platform with recent kernel versions (e.g. v4.2-rc8) and imx_v6_v7_defconfig, when booting u-boot by USB and the kernel by ethernet. I have bisected to a specific commit. More details and logs related to this bug are captured in the kernel bugzilla there : https://bugzilla.kernel.org/show_bug.cgi?id=103461 Thank you for your help and comments. Adding Peter Chen on Cc. Vincent, would you supply related u-boot and pc tools for me? Then, I can reproduce it. This issue should be due to access register without clock, normal kernel boot can't reproduce it. -- 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: USB suspend resume issue on Vybrid (Chipidea IP/MXS PHY)
On 15-08-25 10:37:27, Peter Chen wrote: On Mon, Aug 24, 2015 at 09:40:04PM +0530, Sanchayan Maity wrote: Hello, I am working on Freescale Vybrid which is a Cortex A5 based SoC, with a Chipidea based USB controller and a Sigmatel Phy or some- thing if my memory serves me right. We are currently in the process of implementing suspend resume and fixing related issues. After resume the USB PHY does not come up and the USB subsytem prints [129.570097] usb 1-1: USB disconnect, device number 2 which comes from the core code in hub.c. I am using the 4.1.5 kernel with some of our own patches especially with regards to suspend resume being present only in our own tree. From what I can see, the USB USBPHYx_PWDn register which has bits related to power down, all stay in their default 1 state which denotes power down, after resume. Now this in spite of the fact that the code seems [2] to write 0 to the register on resume. However, doing a quick check with devmem2 shows the register to have the default values of 1 denoting power down. So write seems to have no effect at all. The PHY's clock may not be resumed correct, without normal PHY's clock, the writing to PHY's register has no effect. Check related PLL is on/enable, and output to USB PHY correct. Already checked that. The PLL registers related to USB seem to have the correct values after resume. Sorry forgot to mention this. UTMI clock is valid and PLL3 PLL7 used for USBs are shown locked by the ANADIG registers. Instead of the code at lines 392[1] and 394[2] if I do return mxs_phy_hw_init(mxs_phy); on resume, the USBPHYx_PWDn seems to have the correct value of all bits as zero. However of course, the USB PHY does not come up. The stmp block reset in mxs_phy_hw_init seems to make the write work. It seems the reset can work, and resume can not reset for PHY. Seems so. There is an errata for Vybrid at [3] in VYBRID_2N02G going as e4535: USB: USB suspend and resume flow clarifications. Not sure if I understand the explanation, however the following workaround which the errata mentions: To place the USB PHY in low power suspend mode, the following sequence should be performed in an atomic operation. (interrupts should be disabled during these three steps) 1. Set the PORTSC1.PHCD bit 2. Set all bits in USBPHY_PWD register 3. Set the USBPHY_CTRL.CLKGATE bit These sequence of steps seem to be correctly followed in the suspend code [4] of Chipidea IP AFAICT. I am not that well versed with USB subsystem code having only worked on it once before for fixing non working USB client on Vybrid [5]. Have tried messing with different register bits in the USB PHY and USB miscellaneous register but with no results. Peter, Felipe do you have any ideas perhaps? From what I can see this seems to be USB PHY issue. Yes, it is PHY's suspend/resume issue. The current code follows the errata you refer. Also Peter I wanted to ask you, the following bits MXS_PHY_ABNORMAL_IN_SUSPEND and MXS_PHY_SENDING_SOF_TOO_FAST where are they being used? I can see the MXS_PHY_NEED_IP_FIX being used but not the others. Perhaps I am missing something? There are un-upstream patch which add .notify_suspend/.nofify_resume after bus suspend/resume, in these two PHY's APIs, it will use these two flags. You can find these two APIs at freescale's released bsp. Ok. Found it in their 3.14 series and also your old v5 patchset. However .notify_suspend/.notify_resume seems to have been removed. From what I understand these would have run after the complete suspend/resume perhaps? I added the same in mxs_phy_suspend accordingly for lack of those notify calls, however that does not help. - Sanchayan. -- 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 suspend resume issue on Vybrid (Chipidea IP/MXS PHY)
On 15-08-24 11:22:17, Felipe Balbi wrote: Hi, On Mon, Aug 24, 2015 at 09:40:04PM +0530, Sanchayan Maity wrote: Hello, I am working on Freescale Vybrid which is a Cortex A5 based SoC, with a Chipidea based USB controller and a Sigmatel Phy or some- thing if my memory serves me right. We are currently in the process of implementing suspend resume and fixing related issues. After resume the USB PHY does not come up and the USB subsytem prints [129.570097] usb 1-1: USB disconnect, device number 2 which comes from the core code in hub.c. I am using the 4.1.5 kernel with some of our own patches especially with regards to suspend resume being present only in our own tree. I think you need to send that code if you want to get any help, but in any case, check if by the time you call usb_phy_suspend(phy, 0) (from CI's resume), mxs -resume() method has already been called. Thats in order ci_controller_suspend mxs_phy_suspend Checked with both printks and initcall_debug. - Sanchayan. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] phy: exynos-usb2: add vbus regulator support
On 25.08.2015 14:47, Marek Szyprowski wrote: Hello, On 2015-08-21 14:44, Kishon Vijay Abraham I wrote: On Friday 21 August 2015 06:08 PM, Marek Szyprowski wrote: Exynos USB2 PHY has separate power supply, which is usually provided by VBUS regulator. This patch adds support for it. VBUS regulator is optional, to keep compatibility with boards, which have VBUS provided from some always-on power source. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- .../devicetree/bindings/phy/samsung-phy.txt| 3 +++ drivers/phy/phy-samsung-usb2.c | 25 -- drivers/phy/phy-samsung-usb2.h | 2 ++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt index 60c6f2a633e0..0289d3b07853 100644 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt @@ -44,6 +44,9 @@ Required properties: - the ref clock is used to get the rate of the clock provided to the PHY module +Optional properties: +- vbus-supply: power-supply phandle for vbus power source how about using phy-supply? I wanted to make it a bit more descriptive (vbus-supply is rather self explaining name) and keep it in line with Exynos usb3-drd phy, which already supports vbus-supply. If you think that this is a bad idea, a will use phy-supply then. This is actually supply for VBUS, not for the phy. Using phy-supply would work fine and reduce the size of code... but would be rather a hacky-use of phy and it could be misleading. I don't have strong feeling about this, both ideas have its advantages. If I had to choose than I would like to use vbus-supply because of its correctness with real-world (this is a VBUS after all). Best regards, Krzysztof -- 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] usb: otg: don't set a_alt_hnp_support feature for OTG 2.0 device
Since a_alt_hnp_support is obsolete in OTG 2.0, HNP capable host should send this set feature request only if the otg device is connecting to a non-HNP port and it's compliant with OTG 1.x revision. This is done by checking its otg descriptor length, OTG 2.0 uses usb_otg20_descriptor which has different length than OTG 1.x using usb_otg_descriptor. Signed-off-by: Li Jun jun...@freescale.com --- drivers/usb/core/hub.c | 45 + 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 917c27c..47dca78 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2254,23 +2254,36 @@ static int usb_enumerate_device_otg(struct usb_device *udev) ? : non-); /* enable HNP before suspend, it's simpler */ - if (port1 == bus-otg_port) + if (port1 == bus-otg_port) { bus-b_hnp_enable = 1; - err = usb_control_msg(udev, - usb_sndctrlpipe(udev, 0), - USB_REQ_SET_FEATURE, 0, - bus-b_hnp_enable - ? USB_DEVICE_B_HNP_ENABLE - : USB_DEVICE_A_ALT_HNP_SUPPORT, - 0, NULL, 0, USB_CTRL_SET_TIMEOUT); - if (err 0) { - /* OTG MESSAGE: report errors here, -* customize to match your product. -*/ - dev_info(udev-dev, - can't set HNP mode: %d\n, - err); - bus-b_hnp_enable = 0; + err = usb_control_msg(udev, + usb_sndctrlpipe(udev, 0), + USB_REQ_SET_FEATURE, 0, + USB_DEVICE_B_HNP_ENABLE, + 0, NULL, 0, + USB_CTRL_SET_TIMEOUT); + if (err 0) { + /* OTG MESSAGE: report errors +* here, customize to match your +* product. +*/ + dev_info(udev-dev, + can't set HNP mode: %d\n, + err); + bus-b_hnp_enable = 0; + } + } else if (desc-bLength == sizeof + (struct usb_otg_descriptor)) { + err = usb_control_msg(udev, + usb_sndctrlpipe(udev, 0), + USB_REQ_SET_FEATURE, 0, + USB_DEVICE_A_ALT_HNP_SUPPORT, + 0, NULL, 0, + USB_CTRL_SET_TIMEOUT); + if (err 0) + dev_info(udev-dev, + set a_alt_hnp_support failed: %d\n, + err); } } } -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 4.2 kernel trace when hot unplug a mounted USB/SATA/MMC devices with ext2/ext3/ext4 file system
On Mon, Aug 24, 2015 at 02:20:07PM -0700, Duc Dang wrote: For more information. We tested kernel at commit 5f80f62ada ext4: remove useless condition in if statement. (right before your commit) and still saw the issue. df3305156f989339529b3d6744b898d498fb1f7b [media] v4l: xilinx: Add Xilinx Video IP core 08439fec266c3cc5702953b4f54bdf5649357de0 ext4: remove block_device_ejected 5f80f62adae2a2920781a847805d34b36b323f7d ext4: remove useless condition in if statement. c9bca8b33118573da9b7ac2ea21947a8e4d287dd [media] v4l: of: Add v4l2_of_parse_link() function Further more, the issue does not happen with 3.19-rc7 but happens with 4.00-rc1 Thanks for the report. Does it happen with commit aad653 block: discard bdi_unregister() in favour of bdi_destroy(), either cherry picked or the whole tree at that version? -- 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
[RESEND PATCH 3/7] usb: phy: isp1301: Export I2C module alias information
The I2C core always reports the MODALIAS uevent as i2c:client name regardless if the driver was matched using the I2C id_table or the of_match_table. So the driver needs to export the I2C table and this be built into the module or udev won't have the necessary information to auto load the correct module when the device is added. Signed-off-by: Javier Martinez Canillas jav...@osg.samsung.com --- drivers/usb/phy/phy-isp1301.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c index 8a55b37d1a02..db68156568e6 100644 --- a/drivers/usb/phy/phy-isp1301.c +++ b/drivers/usb/phy/phy-isp1301.c @@ -31,6 +31,7 @@ static const struct i2c_device_id isp1301_id[] = { { isp1301, 0 }, { } }; +MODULE_DEVICE_TABLE(i2c, isp1301_id); static struct i2c_client *isp1301_i2c_client; -- 2.4.3 -- 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
[RESEND PATCH 0/7] Export I2C and OF module aliases in missing drivers
Hello, This is a resend of the patches that were not picked from the series [PATCH 00/27] Export I2C and OF module aliases in missing drivers [0] posted about a month ago. The patches have no dependencies and can be picked individually by the relevant maintainer. I preferred to resend instead of sending a naked ping for each patch that I got no answer. Best regards, Javier Javier Martinez Canillas (7): i2c: core: Export I2C module alias information in dummy driver backlight: tosa: Export I2C module alias information usb: phy: isp1301: Export I2C module alias information ALSA: ppc: keywest: Export I2C module alias information extcon: Export OF module alias information in missing drivers leds: Export OF module alias information in missing drivers regulator: isl9305: Export OF module alias information drivers/extcon/extcon-rt8973a.c | 1 + drivers/extcon/extcon-sm5502.c| 1 + drivers/i2c/i2c-core.c| 1 + drivers/leds/leds-pca963x.c | 1 + drivers/leds/leds-tca6507.c | 1 + drivers/regulator/isl9305.c | 1 + drivers/usb/phy/phy-isp1301.c | 1 + drivers/video/backlight/tosa_bl.c | 1 + sound/ppc/keywest.c | 1 + 9 files changed, 9 insertions(+) -- 2.4.3 -- 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 v10 4/7] usb: interface authorization: Introduces the USB interface authorization
On 08/25/2015 08:20 PM, Stefan Koch wrote: Am Dienstag, den 25.08.2015, 11:58 +0200 schrieb Krzysztof Opasiak: On 08/24/2015 11:09 PM, Stefan Koch wrote: The kernel supports the device authorization because of wireless USB. These is usable for wired USB devices, too. These new interface authorization allows to enable or disable individual interfaces instead a whole device. If a deauthorized interface will be authorized so the driver probing must be triggered manually by writing INTERFACE to /sys/bus/usb/drivers_probe Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/message.c | 38 ++ drivers/usb/core/usb.h | 2 ++ 2 files changed, 40 insertions(+) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 3d25d89..c090f50 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1555,6 +1555,44 @@ static void usb_release_interface(struct device *dev) kfree(intf); } +/* + * usb_deauthorize_interface - deauthorize an USB interface + * + * @intf: USB interface structure + */ +void usb_deauthorize_interface(struct usb_interface *intf) +{ + struct device *dev = intf-dev; + + device_lock(dev-parent); + + if (intf-authorized) { + device_lock(dev); + intf-authorized = 0; + device_unlock(dev); + + usb_forced_unbind_intf(intf); + } + + device_unlock(dev-parent); +} + +/* + * usb_authorize_interface - authorize an USB interface + * + * @intf: USB interface structure + */ +void usb_authorize_interface(struct usb_interface *intf) +{ + struct device *dev = intf-dev; + + if (!intf-authorized) { + device_lock(dev); + intf-authorized = 1; /* authorize interface */ + device_unlock(dev); + } +} + Why usb_deauthorize_interface() takes both dev-parent and dev locks and usb_authorize_interface() takes only dev lock? Because usb_forced_unbind_intf() needs the devices lock... In the auth case, we change only a flag. Yeah true. Forgot that usb_device is parent for all its interfaces. Thank you for clarification, -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- 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: [TESTPATCH] xhci: custom debug patch for command mismatch
On Tue, 25 Aug 2015 14:51:56 +0300, Mathias Nyman mathias.ny...@linux.intel.com wrote: Add extra command ring, event ring and command queue debugging to get more info on command mismatch error triggered by Vincent Pelletier on BayTrail Thanks, applied, built and loaded on target. will blurt out around 500 lines when hitting a command mismatch I'm not sure how I'll get this (I have a lot to learn about kernel debugging). FWIW, I gave a try at unloading and reloading just xhci_pci, and the error did not pop up after more than 30 cycles. I wonder if this means BIOS legacy mode would be messing with the wrong knob, or there would be a timing issue in bios-to-kernel transition. For reference, below are the command I used while tail -f /var/log/messages was running in another terminal, and the output of one iteration. sleep is just here to let the block layer time to scan for partitions. # for INDEX in $(seq 30); do logger TRY $INDEX ; rmmod xhci_pci; modprobe xhci_pci; sleep 4; done Aug 25 21:00:17 boke vincent: TRY 30 Aug 25 21:00:17 boke kernel: [12189.241047] xhci_hcd :00:14.0: remove, state 4 Aug 25 21:00:17 boke kernel: [12189.241070] usb usb2: USB disconnect, device number 1 Aug 25 21:00:17 boke kernel: [12189.241077] usb 2-1: USB disconnect, device number 2 Aug 25 21:00:17 boke kernel: [12189.242468] xhci_hcd :00:14.0: USB bus 2 deregistered Aug 25 21:00:17 boke kernel: [12189.242489] xhci_hcd :00:14.0: remove, state 1 Aug 25 21:00:17 boke kernel: [12189.242507] usb usb1: USB disconnect, device number 1 Aug 25 21:00:17 boke kernel: [12189.242513] usb 1-1: USB disconnect, device number 2 Aug 25 21:00:17 boke kernel: [12189.243109] usb 1-4: USB disconnect, device number 3 Aug 25 21:00:17 boke kernel: [12189.292936] xhci_hcd :00:14.0: USB bus 1 deregistered Aug 25 21:00:17 boke kernel: [12189.317727] xhci_hcd :00:14.0: xHCI Host Controller Aug 25 21:00:17 boke kernel: [12189.317744] xhci_hcd :00:14.0: new USB bus registered, assigned bus number 1 Aug 25 21:00:17 boke kernel: [12189.396920] xhci_hcd :00:14.0: hcc params 0x200071e1 hci version 0x100 quirks 0x9810 Aug 25 21:00:17 boke kernel: [12189.403037] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002 Aug 25 21:00:17 boke kernel: [12189.403043] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 Aug 25 21:00:17 boke kernel: [12189.403046] usb usb1: Product: xHCI Host Controller Aug 25 21:00:17 boke kernel: [12189.403048] usb usb1: Manufacturer: Linux 4.1.4+ xhci-hcd Aug 25 21:00:17 boke kernel: [12189.403051] usb usb1: SerialNumber: :00:14.0 Aug 25 21:00:17 boke kernel: [12189.403494] hub 1-0:1.0: USB hub found Aug 25 21:00:17 boke kernel: [12189.403513] hub 1-0:1.0: 6 ports detected Aug 25 21:00:17 boke kernel: [12189.415715] xhci_hcd :00:14.0: xHCI Host Controller Aug 25 21:00:17 boke kernel: [12189.415734] xhci_hcd :00:14.0: new USB bus registered, assigned bus number 2 Aug 25 21:00:17 boke kernel: [12189.415857] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003 Aug 25 21:00:17 boke kernel: [12189.415861] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1 Aug 25 21:00:17 boke kernel: [12189.415864] usb usb2: Product: xHCI Host Controller Aug 25 21:00:17 boke kernel: [12189.415866] usb usb2: Manufacturer: Linux 4.1.4+ xhci-hcd Aug 25 21:00:17 boke kernel: [12189.415869] usb usb2: SerialNumber: :00:14.0 Aug 25 21:00:17 boke kernel: [12189.416154] hub 2-0:1.0: USB hub found Aug 25 21:00:17 boke kernel: [12189.416169] hub 2-0:1.0: 1 port detected Aug 25 21:00:18 boke kernel: [12189.715846] usb 1-1: new high-speed USB device number 2 using xhci_hcd Aug 25 21:00:18 boke kernel: [12189.727977] usb 2-1: new SuperSpeed USB device number 2 using xhci_hcd Aug 25 21:00:18 boke kernel: [12189.746396] usb 2-1: New USB device found, idVendor=05e3, idProduct=0617 Aug 25 21:00:18 boke kernel: [12189.746404] usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 Aug 25 21:00:18 boke kernel: [12189.746410] usb 2-1: Product: USB3.0 Hub Aug 25 21:00:18 boke kernel: [12189.746415] usb 2-1: Manufacturer: GenesysLogic Aug 25 21:00:18 boke kernel: [12189.748478] hub 2-1:1.0: USB hub found Aug 25 21:00:18 boke kernel: [12189.748749] hub 2-1:1.0: 4 ports detected Aug 25 21:00:18 boke kernel: [12189.853414] usb 1-1: New USB device found, idVendor=05e3, idProduct=0610 Aug 25 21:00:18 boke kernel: [12189.853424] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 Aug 25 21:00:18 boke kernel: [12189.853430] usb 1-1: Product: USB2.0 Hub Aug 25 21:00:18 boke kernel: [12189.853435] usb 1-1: Manufacturer: GenesysLogic Aug 25 21:00:18 boke kernel: [12189.854340] hub 1-1:1.0: USB hub found Aug 25 21:00:18 boke kernel: [12189.854757] hub 1-1:1.0: 4 ports detected Aug 25 21:00:18 boke kernel: [12190.024031] usb 1-4: new high-speed USB device number 3 using xhci_hcd Aug 25 21:00:18 boke kernel: [12190.153360] usb 1-4: New
Re: [PATCH v10 4/7] usb: interface authorization: Introduces the USB interface authorization
Am Dienstag, den 25.08.2015, 11:58 +0200 schrieb Krzysztof Opasiak: On 08/24/2015 11:09 PM, Stefan Koch wrote: The kernel supports the device authorization because of wireless USB. These is usable for wired USB devices, too. These new interface authorization allows to enable or disable individual interfaces instead a whole device. If a deauthorized interface will be authorized so the driver probing must be triggered manually by writing INTERFACE to /sys/bus/usb/drivers_probe Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/message.c | 38 ++ drivers/usb/core/usb.h | 2 ++ 2 files changed, 40 insertions(+) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 3d25d89..c090f50 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1555,6 +1555,44 @@ static void usb_release_interface(struct device *dev) kfree(intf); } +/* + * usb_deauthorize_interface - deauthorize an USB interface + * + * @intf: USB interface structure + */ +void usb_deauthorize_interface(struct usb_interface *intf) +{ + struct device *dev = intf-dev; + + device_lock(dev-parent); + + if (intf-authorized) { + device_lock(dev); + intf-authorized = 0; + device_unlock(dev); + + usb_forced_unbind_intf(intf); + } + + device_unlock(dev-parent); +} + +/* + * usb_authorize_interface - authorize an USB interface + * + * @intf: USB interface structure + */ +void usb_authorize_interface(struct usb_interface *intf) +{ + struct device *dev = intf-dev; + + if (!intf-authorized) { + device_lock(dev); + intf-authorized = 1; /* authorize interface */ + device_unlock(dev); + } +} + Why usb_deauthorize_interface() takes both dev-parent and dev locks and usb_authorize_interface() takes only dev lock? Because usb_forced_unbind_intf() needs the devices lock... In the auth case, we change only a flag. -- 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 v11 3/7] usb: interface authorization: Control interface probing and claiming
Driver probings and interface claims get rejected if an interface is not authorized. Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/driver.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 818369a..9908e74 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -295,6 +295,10 @@ static int usb_probe_interface(struct device *dev) if (udev-authorized == 0) { dev_err(intf-dev, Device is not authorized for usage\n); return error; + } else if (intf-authorized == 0) { + dev_err(intf-dev, Interface %d is not authorized for usage\n, + intf-altsetting-desc.bInterfaceNumber); + return error; } id = usb_match_dynamic_id(intf, driver); @@ -507,6 +511,10 @@ int usb_driver_claim_interface(struct usb_driver *driver, if (dev-driver) return -EBUSY; + /* reject claim if interface is not authorized */ + if (!iface-authorized) + return -ENODEV; + udev = interface_to_usbdev(iface); dev-driver = driver-drvwrap.driver; -- 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 v11 5/7] usb: interface authorization: SysFS part of USB interface authorization
This introduces an attribute for each interface to authorize (1) or deauthorize (0) it: /sys/bus/usb/devices/INTERFACE/authorized Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/sysfs.c | 36 1 file changed, 36 insertions(+) diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index d269738..3ddaada 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -926,6 +926,41 @@ static ssize_t supports_autosuspend_show(struct device *dev, } static DEVICE_ATTR_RO(supports_autosuspend); +/* + * interface_authorized_show - show authorization status of an USB interface + * 1 is authorized, 0 is deauthorized + */ +static ssize_t interface_authorized_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct usb_interface *intf = to_usb_interface(dev); + + return sprintf(buf, %u\n, intf-authorized); +} + +/* + * interface_authorized_store - authorize or deauthorize an USB interface + */ +static ssize_t interface_authorized_store(struct device *dev, + struct device_attribute *attr, const char *buf, size_t count) +{ + struct usb_interface *intf = to_usb_interface(dev); + bool val; + + if (strtobool(buf, val) != 0) + return -EINVAL; + + if (val) + usb_authorize_interface(intf); + else + usb_deauthorize_interface(intf); + + return count; +} +static struct device_attribute dev_attr_interface_authorized = + __ATTR(authorized, S_IRUGO | S_IWUSR, + interface_authorized_show, interface_authorized_store); + static struct attribute *intf_attrs[] = { dev_attr_bInterfaceNumber.attr, dev_attr_bAlternateSetting.attr, @@ -935,6 +970,7 @@ static struct attribute *intf_attrs[] = { dev_attr_bInterfaceProtocol.attr, dev_attr_modalias.attr, dev_attr_supports_autosuspend.attr, + dev_attr_interface_authorized.attr, NULL, }; static struct attribute_group intf_attr_grp = { -- 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 v11 2/7] usb: interface authorization: Introduces the default interface authorization
Interfaces are allowed per default. This can disabled or enabled (again) by writing 0 or 1 to /sys/bus/usb/devices/usbX/interface_authorized_default Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/hcd.c | 47 ++ drivers/usb/core/message.c | 1 + include/linux/usb/hcd.h| 9 + 3 files changed, 57 insertions(+) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index cbcd092..84b5923 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -882,9 +882,53 @@ static ssize_t authorized_default_store(struct device *dev, } static DEVICE_ATTR_RW(authorized_default); +/* + * interface_authorized_default_show - show default authorization status + * for USB interfaces + * + * note: interface_authorized_default is the default value + * for initializing the authorized attribute of interfaces + */ +static ssize_t interface_authorized_default_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct usb_device *usb_dev = to_usb_device(dev); + struct usb_hcd *hcd = bus_to_hcd(usb_dev-bus); + + return sprintf(buf, %u\n, !!HCD_INTF_AUTHORIZED(hcd)); +} + +/* + * interface_authorized_default_store - store default authorization status + * for USB interfaces + * + * note: interface_authorized_default is the default value + * for initializing the authorized attribute of interfaces + */ +static ssize_t interface_authorized_default_store(struct device *dev, + struct device_attribute *attr, const char *buf, size_t count) +{ + struct usb_device *usb_dev = to_usb_device(dev); + struct usb_hcd *hcd = bus_to_hcd(usb_dev-bus); + int rc = count; + bool val; + + if (strtobool(buf, val) != 0) + return -EINVAL; + + if (val) + set_bit(HCD_FLAG_INTF_AUTHORIZED, hcd-flags); + else + clear_bit(HCD_FLAG_INTF_AUTHORIZED, hcd-flags); + + return rc; +} +static DEVICE_ATTR_RW(interface_authorized_default); + /* Group all the USB bus attributes */ static struct attribute *usb_bus_attrs[] = { dev_attr_authorized_default.attr, + dev_attr_interface_authorized_default.attr, NULL, }; @@ -2682,6 +2726,9 @@ int usb_add_hcd(struct usb_hcd *hcd, hcd-authorized_default = authorized_default; set_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags); + /* per default all interfaces are authorized */ + set_bit(HCD_FLAG_INTF_AUTHORIZED, hcd-flags); + /* HC is in reset state, but accessible. Now do the one-time init, * bottom up so that hcds can customize the root hubs before hub_wq * starts talking to them. (Note, bus id is assigned early too.) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index f368d20..3d25d89 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1807,6 +1807,7 @@ free_interfaces: intfc = cp-intf_cache[i]; intf-altsetting = intfc-altsetting; intf-num_altsetting = intfc-num_altsetting; + intf-authorized = !!HCD_INTF_AUTHORIZED(hcd); kref_get(intfc-ref); alt = usb_altnum_to_altsetting(intf, 0); diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index c9aa779..e56c6b2 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -120,6 +120,7 @@ struct usb_hcd { #define HCD_FLAG_WAKEUP_PENDING4 /* root hub is resuming? */ #define HCD_FLAG_RH_RUNNING5 /* root hub is running? */ #define HCD_FLAG_DEAD 6 /* controller has died? */ +#define HCD_FLAG_INTF_AUTHORIZED 7 /* authorize interfaces? */ /* The flags can be tested using these macros; they are likely to * be slightly faster than test_bit(). @@ -131,6 +132,14 @@ struct usb_hcd { #define HCD_RH_RUNNING(hcd)((hcd)-flags (1U HCD_FLAG_RH_RUNNING)) #define HCD_DEAD(hcd) ((hcd)-flags (1U HCD_FLAG_DEAD)) + /* +* Specifies if interfaces are authorized by default +* or they require explicit user space authorization; this bit is +* settable through /sys/class/usb_host/X/interface_authorized_default +*/ +#define HCD_INTF_AUTHORIZED(hcd) \ + ((hcd)-flags (1U HCD_FLAG_INTF_AUTHORIZED)) + /* Flags that get set only during HCD registration or removal. */ unsignedrh_registered:1;/* is root hub registered? */ unsignedrh_pollable:1; /* may we poll the root hub? */ -- 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 v11 6/7] usb: interface authorization: Documentation part
This part adds the documentation for the interface authorization. Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- Documentation/ABI/testing/sysfs-bus-usb | 20 Documentation/usb/authorization.txt | 31 +++ 2 files changed, 51 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb index e5cc763..b5690d3 100644 --- a/Documentation/ABI/testing/sysfs-bus-usb +++ b/Documentation/ABI/testing/sysfs-bus-usb @@ -1,3 +1,23 @@ +What: /sys/bus/usb/devices/INTERFACE/authorized +Date: August 2015 +Description: + This allows to authorize (1) or deauthorize (0) + individual interfaces instead a whole device + in contrast to the device authorization. + If a deauthorized interface will be authorized + so the driver probing must be triggered manually + by writing INTERFACE to /sys/bus/usb/drivers_probe + This allows to avoid side-effects with drivers + that need multiple interfaces. + A deauthorized interface cannot be probed or claimed. + +What: /sys/bus/usb/devices/usbX/interface_authorized_default +Date: August 2015 +Description: + This is used as value that determines if interfaces + would be authorized by default. + The value can be 1 or 0. It's by default 1. + What: /sys/bus/usb/device/.../authorized Date: July 2008 KernelVersion: 2.6.26 diff --git a/Documentation/usb/authorization.txt b/Documentation/usb/authorization.txt index c069b68..c7e985f 100644 --- a/Documentation/usb/authorization.txt +++ b/Documentation/usb/authorization.txt @@ -90,3 +90,34 @@ etc, but you get the idea. Anybody with access to a device gadget kit can fake descriptors and device info. Don't trust that. You are welcome. + +Interface authorization +--- +There is a similar approach to allow or deny specific USB interfaces. +That allows to block only a subset of an USB device. + +Authorize an interface: +$ echo 1 /sys/bus/usb/devices/INTERFACE/authorized + +Deauthorize an interface: +$ echo 0 /sys/bus/usb/devices/INTERFACE/authorized + +The default value for new interfaces +on a particular USB bus can be changed, too. + +Allow interfaces per default: +$ echo 1 /sys/bus/usb/devices/usbX/interface_authorized_default + +Deny interfaces per default: +$ echo 0 /sys/bus/usb/devices/usbX/interface_authorized_default + +Per default the interface_authorized_default bit is 1. +So all interfaces would authorized per default. + +Note: +If a deauthorized interface will be authorized so the driver probing must +be triggered manually by writing INTERFACE to /sys/bus/usb/drivers_probe + +For drivers that need multiple interfaces all needed interfaces should be +authroized first. After that the drivers should be probed. +This avoids side effects. -- 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 v11 7/7] usb: interface authorization: Use a flag for the default device authorization
With this patch a flag instead of a variable is used for the default device authorization. Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/hcd.c | 31 +-- drivers/usb/core/usb.c | 2 +- include/linux/usb/hcd.h | 16 +--- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 84b5923..a567647 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -854,10 +854,10 @@ static ssize_t authorized_default_show(struct device *dev, { struct usb_device *rh_usb_dev = to_usb_device(dev); struct usb_bus *usb_bus = rh_usb_dev-bus; - struct usb_hcd *usb_hcd; + struct usb_hcd *hcd; - usb_hcd = bus_to_hcd(usb_bus); - return snprintf(buf, PAGE_SIZE, %u\n, usb_hcd-authorized_default); + hcd = bus_to_hcd(usb_bus); + return snprintf(buf, PAGE_SIZE, %u\n, !!HCD_DEV_AUTHORIZED(hcd)); } static ssize_t authorized_default_store(struct device *dev, @@ -868,12 +868,16 @@ static ssize_t authorized_default_store(struct device *dev, unsigned val; struct usb_device *rh_usb_dev = to_usb_device(dev); struct usb_bus *usb_bus = rh_usb_dev-bus; - struct usb_hcd *usb_hcd; + struct usb_hcd *hcd; - usb_hcd = bus_to_hcd(usb_bus); + hcd = bus_to_hcd(usb_bus); result = sscanf(buf, %u\n, val); if (result == 1) { - usb_hcd-authorized_default = val ? 1 : 0; + if (val) + set_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags); + else + clear_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags); + result = size; } else { result = -EINVAL; @@ -2720,10 +2724,17 @@ int usb_add_hcd(struct usb_hcd *hcd, dev_info(hcd-self.controller, %s\n, hcd-product_desc); /* Keep old behaviour if authorized_default is not in [0, 1]. */ - if (authorized_default 0 || authorized_default 1) - hcd-authorized_default = hcd-wireless ? 0 : 1; - else - hcd-authorized_default = authorized_default; + if (authorized_default 0 || authorized_default 1) { + if (hcd-wireless) + clear_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags); + else + set_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags); + } else { + if (authorized_default) + set_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags); + else + clear_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags); + } set_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags); /* per default all interfaces are authorized */ diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 8d5b2f4..f8bbd0b 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -510,7 +510,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, if (root_hub) /* Root hub always ok [and always wired] */ dev-authorized = 1; else { - dev-authorized = usb_hcd-authorized_default; + dev-authorized = !!HCD_DEV_AUTHORIZED(usb_hcd); dev-wusb = usb_bus_is_wusb(bus) ? 1 : 0; } return dev; diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index e56c6b2..09a51a4 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -58,12 +58,6 @@ * * Since struct usb_bus is so thin, you can't share much code in it. * This framework is a layer over that, and should be more sharable. - * - * @authorized_default: Specifies if new devices are authorized to - * connect by default or they require explicit - * user space authorization; this bit is settable - * through /sys/class/usb_host/X/authorized_default. - * For the rest is RO, so we don't lock to r/w it. */ /*-*/ @@ -121,6 +115,7 @@ struct usb_hcd { #define HCD_FLAG_RH_RUNNING5 /* root hub is running? */ #define HCD_FLAG_DEAD 6 /* controller has died? */ #define HCD_FLAG_INTF_AUTHORIZED 7 /* authorize interfaces? */ +#define HCD_FLAG_DEV_AUTHORIZED8 /* authorize devices? */ /* The flags can be tested using these macros; they are likely to * be slightly faster than test_bit(). @@ -140,6 +135,14 @@ struct usb_hcd { #define HCD_INTF_AUTHORIZED(hcd) \ ((hcd)-flags (1U HCD_FLAG_INTF_AUTHORIZED)) + /* +* Specifies if devices are authorized by default +* or they require explicit user space authorization; this bit is +* settable through /sys/class/usb_host/X/authorized_default +*/ +#define HCD_DEV_AUTHORIZED(hcd) \ + ((hcd)-flags (1U
[PATCH v11 1/7] usb: interface authorization: Declare authorized attribute
The attribute authorized shows the authorization state for an interface. Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- include/linux/usb.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index 447fe29..3deccab 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -178,6 +178,7 @@ struct usb_interface { unsigned needs_altsetting0:1; /* switch to altsetting 0 is pending */ unsigned needs_binding:1; /* needs delayed unbind/rebind */ unsigned resetting_device:1;/* true: bandwidth alloc after reset */ + unsigned authorized:1; /* used for interface authorization */ struct device dev; /* interface specific device info */ struct device *usb_dev; -- 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 v11 0/7] usb: interface authorization
This patch introduces an interface authorization for USB devices. The kernel supports a device authorization because of wireless USB. But the new interface authorization allows to authorize or deauthorize individual interfaces instead authorization or deauthorize a whole device. Therefore the authorized attribute is introduced for each interface. Each patch depends on all patches with a lesser number. v5 was acked-by Alan Stern: http://comments.gmane.org/gmane.linux.usb.general/127144 http://permalink.gmane.org/gmane.linux.usb.general/127151 Changes since v10: - Implemented spell suggestions from Krzysztof Opasiak Changes since v9: - Implemented suggestions from Greg K-H and Sergei Shtylyov Changes since v8: - Implemented suggestions from Greg K-H (number and doc issue) Changes since v7: - Implemented suggestions from Alan Stern and Sergei Shtylyov Changes since v6: - Implemented suggestions from Alan Stern and Sergei Shtylyov Changes since v5: - Implemented suggestions from Greg K-H - Changed device authorization to save the default bit in the same flag as the interface authorization does this (recommended by Alan Stern http://permalink.gmane.org/gmane.linux.usb.general/127086) Stefan Koch (7): usb: interface authorization: Declare authorized attribute usb: interface authorization: Introduces the default interface authorization usb: interface authorization: Control interface probing and claiming usb: interface authorization: Introduces the USB interface authorization usb: interface authorization: SysFS part of USB interface authorization usb: interface authorization: Documentation part usb: interface authorization: Use a flag for the default device authorization Documentation/ABI/testing/sysfs-bus-usb | 20 + Documentation/usb/authorization.txt | 31 + drivers/usb/core/driver.c | 8 drivers/usb/core/hcd.c | 78 - drivers/usb/core/message.c | 39 + drivers/usb/core/sysfs.c| 36 +++ drivers/usb/core/usb.c | 2 +- drivers/usb/core/usb.h | 2 + include/linux/usb.h | 1 + include/linux/usb/hcd.h | 25 --- 10 files changed, 224 insertions(+), 18 deletions(-) -- 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 v11 4/7] usb: interface authorization: Introduces the USB interface authorization
The kernel supports the device authorization because of wireless USB. These is usable for wired USB devices, too. These new interface authorization allows to enable or disable individual interfaces instead a whole device. If a deauthorized interface will be authorized so the driver probing must be triggered manually by writing INTERFACE to /sys/bus/usb/drivers_probe Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/message.c | 38 ++ drivers/usb/core/usb.h | 2 ++ 2 files changed, 40 insertions(+) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 3d25d89..c090f50 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1555,6 +1555,44 @@ static void usb_release_interface(struct device *dev) kfree(intf); } +/* + * usb_deauthorize_interface - deauthorize an USB interface + * + * @intf: USB interface structure + */ +void usb_deauthorize_interface(struct usb_interface *intf) +{ + struct device *dev = intf-dev; + + device_lock(dev-parent); + + if (intf-authorized) { + device_lock(dev); + intf-authorized = 0; + device_unlock(dev); + + usb_forced_unbind_intf(intf); + } + + device_unlock(dev-parent); +} + +/* + * usb_authorize_interface - authorize an USB interface + * + * @intf: USB interface structure + */ +void usb_authorize_interface(struct usb_interface *intf) +{ + struct device *dev = intf-dev; + + if (!intf-authorized) { + device_lock(dev); + intf-authorized = 1; /* authorize interface */ + device_unlock(dev); + } +} + static int usb_if_uevent(struct device *dev, struct kobj_uevent_env *env) { struct usb_device *usb_dev; diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index 457255a..05b5e17 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -27,6 +27,8 @@ extern void usb_release_interface_cache(struct kref *ref); extern void usb_disable_device(struct usb_device *dev, int skip_ep0); extern int usb_deauthorize_device(struct usb_device *); extern int usb_authorize_device(struct usb_device *); +extern void usb_deauthorize_interface(struct usb_interface *); +extern void usb_authorize_interface(struct usb_interface *); extern void usb_detect_quirks(struct usb_device *udev); extern void usb_detect_interface_quirks(struct usb_device *udev); extern int usb_remove_device(struct usb_device *udev); -- 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 v10 5/7] usb: interface authorization: SysFS part of USB interface authorization
On 08/24/2015 11:09 PM, Stefan Koch wrote: This introduces an attribute for each interface to authorize (1) or deauthorize (0) it: /sys/bus/usb/devices/INTERFACE/authorized Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/sysfs.c | 36 1 file changed, 36 insertions(+) diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index d269738..3ddaada 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -926,6 +926,41 @@ static ssize_t supports_autosuspend_show(struct device *dev, } static DEVICE_ATTR_RO(supports_autosuspend); +/* + * interface_authorized_show - show authorization status of an USB interface + * 1 is authorized, 0 is deauthorized + */ +static ssize_t interface_authorized_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct usb_interface *intf = to_usb_interface(dev); + + return sprintf(buf, %u\n, intf-authorized); +} + +/* + * interface_authorized_store - authorize or deauthorize an USB interface + */ +static ssize_t interface_authorized_store(struct device *dev, + struct device_attribute *attr, const char *buf, size_t count) +{ + struct usb_interface *intf = to_usb_interface(dev); + bool val; + + if (strtobool(buf, val) != 0) + return -EINVAL; + + if (val) + usb_authorize_interface(intf); + else + usb_deauthorize_interface(intf); + + return count; +} shouldn't we return error if user tries to authorize interface which is already authorized an likewise when he is trying to deauthorize interface which is not autorized? This is a little bit phylosophical if: cat authorized authorized should fail or not. -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- 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 v10 5/7] usb: interface authorization: SysFS part of USB interface authorization
On Tue, 2015-08-25 at 12:03 +0200, Krzysztof Opasiak wrote: shouldn't we return error if user tries to authorize interface which is already authorized an likewise when he is trying to deauthorize interface which is not autorized? That would be awkward if the user and udev try to do this at the same time. 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 v10 3/7] usb: interface authorization: Control interface probing and claiming
On 08/24/2015 11:09 PM, Stefan Koch wrote: Driver probings and interface claims get rejected if an interface is not authorized. Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/driver.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 818369a..d542d43 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -295,6 +295,10 @@ static int usb_probe_interface(struct device *dev) if (udev-authorized == 0) { dev_err(intf-dev, Device is not authorized for usage\n); return error; + } else if (intf-authorized == 0) { + dev_err(intf-dev, Interface %d is not authorized for usage\n, + intf-altsetting-desc.bInterfaceNumber); + return error; } id = usb_match_dynamic_id(intf, driver); @@ -507,6 +511,10 @@ int usb_driver_claim_interface(struct usb_driver *driver, if (dev-driver) return -EBUSY; + /* reject claim if not iterface is not authorized */ s/not iterface/interface Best regards, -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- 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 v10 4/7] usb: interface authorization: Introduces the USB interface authorization
On 08/24/2015 11:09 PM, Stefan Koch wrote: The kernel supports the device authorization because of wireless USB. These is usable for wired USB devices, too. These new interface authorization allows to enable or disable individual interfaces instead a whole device. If a deauthorized interface will be authorized so the driver probing must be triggered manually by writing INTERFACE to /sys/bus/usb/drivers_probe Signed-off-by: Stefan Koch stefan.koc...@gmail.com --- drivers/usb/core/message.c | 38 ++ drivers/usb/core/usb.h | 2 ++ 2 files changed, 40 insertions(+) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 3d25d89..c090f50 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1555,6 +1555,44 @@ static void usb_release_interface(struct device *dev) kfree(intf); } +/* + * usb_deauthorize_interface - deauthorize an USB interface + * + * @intf: USB interface structure + */ +void usb_deauthorize_interface(struct usb_interface *intf) +{ + struct device *dev = intf-dev; + + device_lock(dev-parent); + + if (intf-authorized) { + device_lock(dev); + intf-authorized = 0; + device_unlock(dev); + + usb_forced_unbind_intf(intf); + } + + device_unlock(dev-parent); +} + +/* + * usb_authorize_interface - authorize an USB interface + * + * @intf: USB interface structure + */ +void usb_authorize_interface(struct usb_interface *intf) +{ + struct device *dev = intf-dev; + + if (!intf-authorized) { + device_lock(dev); + intf-authorized = 1; /* authorize interface */ + device_unlock(dev); + } +} + Why usb_deauthorize_interface() takes both dev-parent and dev locks and usb_authorize_interface() takes only dev lock? -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
On 8/18/2015 8:45 AM, Scott Branden wrote: From: Roman Bacik rba...@broadcom.com USB OTG driver in isochronous mode has to set the parity of the receiving microframe. The parity is set to even by default. This causes problems for an audio gadget, if the host starts transmitting on odd microframes. This fix uses Incomplete Periodic Transfer interrupt to toggle between even and odd parity until the Transfer Complete interrupt is received. Signed-off-by: Roman Bacik rba...@broadcom.com Reviewed-by: Abhinav Ratna ara...@broadcom.com Reviewed-by: Srinath Mannam srinath.man...@broadcom.com Reviewed-by: Scott Branden sbran...@broadcom.com Signed-off-by: Scott Branden sbran...@broadcom.com --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 48 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 0ed87620..954d1cd 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -150,6 +150,7 @@ struct s3c_hsotg_ep { unsigned intperiodic:1; unsigned intisochronous:1; unsigned intsend_zlp:1; + unsigned intparity_set:1; charname[10]; }; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4d47b7c..28e4393 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -1954,6 +1954,8 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx, ints = ~DXEPINT_XFERCOMPL; if (ints DXEPINT_XFERCOMPL) { + if (hs_ep-isochronous !hs_ep-parity_set) + hs_ep-parity_set = 1; if (hs_ep-isochronous hs_ep-interval == 1) { if (ctrl DXEPCTL_EOFRNUM) ctrl |= DXEPCTL_SETEVENFR; @@ -2316,7 +2318,8 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST | GINTSTS_RESETDET | GINTSTS_ENUMDONE | GINTSTS_OTGINT | GINTSTS_USBSUSP | - GINTSTS_WKUPINT, + GINTSTS_WKUPINT | + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT, hsotg-regs + GINTMSK); if (using_dma(hsotg)) @@ -2581,6 +2584,48 @@ irq_retry: s3c_hsotg_dump(hsotg); } + if (gintsts GINTSTS_INCOMPL_SOIN) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg-dev, %s: GINTSTS_INCOMPL_SOIN\n, __func__); + for (idx = 1; idx MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg-eps_in[idx]; + if (hs_ep-isochronous !hs_ep-parity_set) { + u32 epctl_reg = DIEPCTL(idx); + u32 ctrl = readl(hsotg-regs + epctl_reg); + + if (ctrl DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg-regs + epctl_reg); + } + } + writel(GINTSTS_INCOMPL_SOIN, hsotg-regs + GINTSTS); + } + + if (gintsts GINTSTS_INCOMPL_SOOUT) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg-dev, %s: GINTSTS_INCOMPL_SOOUT\n, __func__); + for (idx = 1; idx MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg-eps_out[idx]; + if (hs_ep-isochronous !hs_ep-parity_set) { + u32 epctl_reg = DOEPCTL(idx); + u32 ctrl = readl(hsotg-regs + epctl_reg); + + if (ctrl DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg-regs + epctl_reg); + } + } + writel(GINTSTS_INCOMPL_SOOUT, hsotg-regs + GINTSTS); + } + /* * if we've had fifo events, we should try and go around the * loop again to see if there's any point in returning yet. @@ -2667,6 +2712,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep, hs_ep-periodic = 0; hs_ep-halted = 0; hs_ep-interval = desc-bInterval; + hs_ep-parity_set = 0; I'm not quite sure what the parity_set flag does in this patch. Shouldn't you be able to just toggle the even/odd frame when you get the interrupt? John -- 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
RE: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
-Original Message- From: John Youn [mailto:john.y...@synopsys.com] Sent: August-25-15 2:52 PM To: Scott Branden; John Youn; Greg Kroah-Hartman; linux- u...@vger.kernel.org Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list; Roman Bacik Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode On 8/18/2015 8:45 AM, Scott Branden wrote: From: Roman Bacik rba...@broadcom.com USB OTG driver in isochronous mode has to set the parity of the receiving microframe. The parity is set to even by default. This causes problems for an audio gadget, if the host starts transmitting on odd microframes. This fix uses Incomplete Periodic Transfer interrupt to toggle between even and odd parity until the Transfer Complete interrupt is received. Signed-off-by: Roman Bacik rba...@broadcom.com Reviewed-by: Abhinav Ratna ara...@broadcom.com Reviewed-by: Srinath Mannam srinath.man...@broadcom.com Reviewed-by: Scott Branden sbran...@broadcom.com Signed-off-by: Scott Branden sbran...@broadcom.com --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 48 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 0ed87620..954d1cd 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -150,6 +150,7 @@ struct s3c_hsotg_ep { unsigned intperiodic:1; unsigned intisochronous:1; unsigned intsend_zlp:1; + unsigned intparity_set:1; charname[10]; }; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4d47b7c..28e4393 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -1954,6 +1954,8 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx, ints = ~DXEPINT_XFERCOMPL; if (ints DXEPINT_XFERCOMPL) { + if (hs_ep-isochronous !hs_ep-parity_set) + hs_ep-parity_set = 1; if (hs_ep-isochronous hs_ep-interval == 1) { if (ctrl DXEPCTL_EOFRNUM) ctrl |= DXEPCTL_SETEVENFR; @@ -2316,7 +2318,8 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST | GINTSTS_RESETDET | GINTSTS_ENUMDONE | GINTSTS_OTGINT | GINTSTS_USBSUSP | - GINTSTS_WKUPINT, + GINTSTS_WKUPINT | + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT, hsotg-regs + GINTMSK); if (using_dma(hsotg)) @@ -2581,6 +2584,48 @@ irq_retry: s3c_hsotg_dump(hsotg); } + if (gintsts GINTSTS_INCOMPL_SOIN) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg-dev, %s: GINTSTS_INCOMPL_SOIN\n, __func__); + for (idx = 1; idx MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg-eps_in[idx]; + if (hs_ep-isochronous !hs_ep-parity_set) { + u32 epctl_reg = DIEPCTL(idx); + u32 ctrl = readl(hsotg-regs + epctl_reg); + + if (ctrl DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg-regs + epctl_reg); + } + } + writel(GINTSTS_INCOMPL_SOIN, hsotg-regs + GINTSTS); + } + + if (gintsts GINTSTS_INCOMPL_SOOUT) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg-dev, %s: GINTSTS_INCOMPL_SOOUT\n, __func__); + for (idx = 1; idx MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg-eps_out[idx]; + if (hs_ep-isochronous !hs_ep-parity_set) { + u32 epctl_reg = DOEPCTL(idx); + u32 ctrl = readl(hsotg-regs + epctl_reg); + + if (ctrl DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg-regs + epctl_reg); + } + } + writel(GINTSTS_INCOMPL_SOOUT, hsotg-regs + GINTSTS); + } + /* * if we've had fifo events, we should try and go around the * loop again to see if there's any point in returning yet. @@ -2667,6 +2712,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep, hs_ep-periodic = 0; hs_ep-halted = 0; hs_ep-interval = desc-bInterval; + hs_ep-parity_set = 0; I'm not quite sure
[PATCH v2] usb: message: remove redundant declaration
Fix the Sparse warning: message.c:1390:21: warning: symbol 'i' shadows an earlier one message.c:1294:13: originally declared here Signed-off-by: Kris Borer kbo...@gmail.com --- drivers/usb/core/message.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index f368d20..5f07e4e 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1387,8 +1387,6 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) * new altsetting. */ if (manual) { - int i; - for (i = 0; i alt-desc.bNumEndpoints; i++) { epaddr = alt-endpoint[i].desc.bEndpointAddress; pipe = __create_pipe(dev, -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
On Tue, Aug 25, 2015 at 10:00:17PM +, Roman Bacik wrote: -Original Message- From: John Youn [mailto:john.y...@synopsys.com] Sent: August-25-15 2:52 PM To: Scott Branden; John Youn; Greg Kroah-Hartman; linux- u...@vger.kernel.org Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list; Roman Bacik Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode On 8/18/2015 8:45 AM, Scott Branden wrote: From: Roman Bacik rba...@broadcom.com USB OTG driver in isochronous mode has to set the parity of the receiving microframe. The parity is set to even by default. This causes problems for an audio gadget, if the host starts transmitting on odd microframes. This fix uses Incomplete Periodic Transfer interrupt to toggle between even and odd parity until the Transfer Complete interrupt is received. Signed-off-by: Roman Bacik rba...@broadcom.com Reviewed-by: Abhinav Ratna ara...@broadcom.com Reviewed-by: Srinath Mannam srinath.man...@broadcom.com Reviewed-by: Scott Branden sbran...@broadcom.com Signed-off-by: Scott Branden sbran...@broadcom.com --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 48 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 0ed87620..954d1cd 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -150,6 +150,7 @@ struct s3c_hsotg_ep { unsigned intperiodic:1; unsigned intisochronous:1; unsigned intsend_zlp:1; + unsigned intparity_set:1; charname[10]; }; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4d47b7c..28e4393 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -1954,6 +1954,8 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx, ints = ~DXEPINT_XFERCOMPL; if (ints DXEPINT_XFERCOMPL) { + if (hs_ep-isochronous !hs_ep-parity_set) + hs_ep-parity_set = 1; it shouldn't be a problem to set the flag which was already set, so this could be simplified to: hs_ep-has_correct_parity = !!hs_ep0isochronous; if (hs_ep-isochronous hs_ep-interval == 1) { if (ctrl DXEPCTL_EOFRNUM) ctrl |= DXEPCTL_SETEVENFR; @@ -2316,7 +2318,8 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST | GINTSTS_RESETDET | GINTSTS_ENUMDONE | GINTSTS_OTGINT | GINTSTS_USBSUSP | - GINTSTS_WKUPINT, + GINTSTS_WKUPINT | + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT, why the two extra bits ? What are they doing ? hsotg-regs + GINTMSK); if (using_dma(hsotg)) @@ -2581,6 +2584,48 @@ irq_retry: s3c_hsotg_dump(hsotg); } + if (gintsts GINTSTS_INCOMPL_SOIN) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg-dev, %s: GINTSTS_INCOMPL_SOIN\n, __func__); + for (idx = 1; idx MAX_EPS_CHANNELS; idx++) { u32 epctl_reg; u32 ctrl; + hs_ep = hsotg-eps_in[idx]; you can decrease some indentation here: if (!hs_ep-isochronous) continue; if (hs_ep-has_correct_parity) continue; epctl_reg = DIEPCTL(idx); ctrl = readl(hsotg-regs + epctl_reg); if (ctrl DXEPCTL_EOFRNUM) ctrl |= DXEPCTL_SETEVENFR; else ctrl |= DXEPCTL_SETODDFR; writel(ctrl, hsotg-regs + epctl_reg); ditto to the other loop below snip I'm not quite sure what the parity_set flag does in this patch. Shouldn't you be able to just toggle the even/odd frame when you get the interrupt? John When Transfer Complete interrupt is received, we have the correct parity. Therefore we set the flag and we stop toggling. The parity_set flag indicates whether we have the correct parity set. then how about calling it has_correct_parity instead ? -- balbi signature.asc Description: Digital signature
[PATCH 1/5] usb: common: of_usb_get_maximum_speed to usb_get_maximum_speed
By using the unified device property interface, the function can be made available for all platforms and not just the ones using DT. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com --- drivers/usb/chipidea/core.c | 2 +- drivers/usb/common/common.c | 44 ++-- drivers/usb/dwc3/core.c | 3 ++- include/linux/usb/ch9.h | 11 ++- include/linux/usb/of.h | 6 -- 5 files changed, 31 insertions(+), 35 deletions(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 3feebf7..ce71532 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -648,7 +648,7 @@ static int ci_get_platdata(struct device *dev, return ret; } - if (of_usb_get_maximum_speed(dev-of_node) == USB_SPEED_FULL) + if (usb_get_maximum_speed(dev) == USB_SPEED_FULL) platdata-flags |= CI_HDRC_FORCE_FULLSPEED; platdata-itc_setting = 1; diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c index 9e39286..b25a111 100644 --- a/drivers/usb/common/common.c +++ b/drivers/usb/common/common.c @@ -60,6 +60,24 @@ const char *usb_speed_string(enum usb_device_speed speed) } EXPORT_SYMBOL_GPL(usb_speed_string); +enum usb_device_speed usb_get_maximum_speed(struct device *dev) +{ + const char *maximum_speed; + int err; + int i; + + err = device_property_read_string(dev, maximum-speed, maximum_speed); + if (err 0) + return USB_SPEED_UNKNOWN; + + for (i = 0; i ARRAY_SIZE(speed_names); i++) + if (strcmp(maximum_speed, speed_names[i]) == 0) + return i; + + return USB_SPEED_UNKNOWN; +} +EXPORT_SYMBOL_GPL(usb_get_maximum_speed); + const char *usb_state_string(enum usb_device_state state) { static const char *const names[] = { @@ -114,32 +132,6 @@ enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np) EXPORT_SYMBOL_GPL(of_usb_get_dr_mode); /** - * of_usb_get_maximum_speed - Get maximum requested speed for a given USB - * controller. - * @np: Pointer to the given device_node - * - * The function gets the maximum speed string from property maximum-speed, - * and returns the corresponding enum usb_device_speed. - */ -enum usb_device_speed of_usb_get_maximum_speed(struct device_node *np) -{ - const char *maximum_speed; - int err; - int i; - - err = of_property_read_string(np, maximum-speed, maximum_speed); - if (err 0) - return USB_SPEED_UNKNOWN; - - for (i = 0; i ARRAY_SIZE(speed_names); i++) - if (strcmp(maximum_speed, speed_names[i]) == 0) - return i; - - return USB_SPEED_UNKNOWN; -} -EXPORT_SYMBOL_GPL(of_usb_get_maximum_speed); - -/** * of_usb_host_tpl_support - to get if Targeted Peripheral List is supported * for given targeted hosts (non-PC hosts) * @np: Pointer to the given device_node diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 064123e..9694799 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -842,8 +842,9 @@ static int dwc3_probe(struct platform_device *pdev) */ hird_threshold = 12; + dwc-maximum_speed = usb_get_maximum_speed(dev); + if (node) { - dwc-maximum_speed = of_usb_get_maximum_speed(node); dwc-has_lpm_erratum = of_property_read_bool(node, snps,has-lpm-erratum); of_property_read_u8(node, snps,lpm-nyet-threshold, diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h index 27603bc..6cc96bb 100644 --- a/include/linux/usb/ch9.h +++ b/include/linux/usb/ch9.h @@ -32,9 +32,9 @@ #ifndef __LINUX_USB_CH9_H #define __LINUX_USB_CH9_H +#include linux/device.h #include uapi/linux/usb/ch9.h - /** * usb_speed_string() - Returns human readable-name of the speed. * @speed: The speed to return human-readable name for. If it's not @@ -43,6 +43,15 @@ */ extern const char *usb_speed_string(enum usb_device_speed speed); +/** + * usb_get_maximum_speed - Get maximum requested speed for a given USB + * controller. + * @dev: Pointer to the given USB controller device + * + * The function gets the maximum speed string from property maximum-speed, + * and returns the corresponding enum usb_device_speed. + */ +extern enum usb_device_speed usb_get_maximum_speed(struct device *dev); /** * usb_state_string - Returns human readable name for the state. diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h index 8c5a818..ff23fea 100644 --- a/include/linux/usb/of.h +++ b/include/linux/usb/of.h @@ -13,7 +13,6 @@ #if IS_ENABLED(CONFIG_OF) enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np); -enum usb_device_speed of_usb_get_maximum_speed(struct device_node *np); bool of_usb_host_tpl_support(struct device_node *np); int of_usb_update_otg_caps(struct device_node *np,
[PATCH 0/5] usb: common and dwc3: converting to unified device property
Hi, While converting dwc3 to the unified device property interface, I noticed that there is really nothing preventing of_usb_get_dr_mode and of_usb_get_maximum_speed from being converted as well. Hope that's OK. The only special case was dwc3-st.c, where the dr_mode is requested before the platform device has been populated. I changed it so that the dr_mode property is requested after the platform device is populated in a separate patch. Heikki Krogerus (5): usb: common: of_usb_get_maximum_speed to usb_get_maximum_speed usb: dwc3: st: prepare the driver for generic usb_get_dr_mode function usb: common: of_usb_get_dr_mode to usb_get_dr_mode usb: dwc3: core: convert to unified device property interface usb: dwc3: pci: passing forward the ACPI companion drivers/usb/chipidea/core.c | 4 +-- drivers/usb/common/common.c | 59 +++-- drivers/usb/dwc2/platform.c | 2 +- drivers/usb/dwc3/core.c | 50 +- drivers/usb/dwc3/dwc3-pci.c | 1 + drivers/usb/dwc3/dwc3-st.c | 12 +++-- drivers/usb/musb/musb_dsps.c| 2 +- drivers/usb/musb/sunxi.c| 2 +- drivers/usb/phy/phy-msm-usb.c | 2 +- drivers/usb/phy/phy-tegra-usb.c | 2 +- include/linux/usb/ch9.h | 11 +++- include/linux/usb/of.h | 12 - include/linux/usb/otg.h | 9 +++ 13 files changed, 84 insertions(+), 84 deletions(-) -- 2.5.0 -- 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 3/5] usb: common: of_usb_get_dr_mode to usb_get_dr_mode
By using the unified device property interface, the function can be made available for all platforms and not just the ones using DT. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com --- drivers/usb/chipidea/core.c | 2 +- drivers/usb/common/common.c | 15 --- drivers/usb/dwc2/platform.c | 2 +- drivers/usb/dwc3/core.c | 2 +- drivers/usb/dwc3/dwc3-st.c | 2 +- drivers/usb/musb/musb_dsps.c| 2 +- drivers/usb/musb/sunxi.c| 2 +- drivers/usb/phy/phy-msm-usb.c | 2 +- drivers/usb/phy/phy-tegra-usb.c | 2 +- include/linux/usb/of.h | 6 -- include/linux/usb/otg.h | 9 + 11 files changed, 21 insertions(+), 25 deletions(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index ce71532..bf25997 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -611,7 +611,7 @@ static int ci_get_platdata(struct device *dev, platdata-phy_mode = of_usb_get_phy_mode(dev-of_node); if (!platdata-dr_mode) - platdata-dr_mode = of_usb_get_dr_mode(dev-of_node); + platdata-dr_mode = usb_get_dr_mode(dev); if (platdata-dr_mode == USB_DR_MODE_UNKNOWN) platdata-dr_mode = USB_DR_MODE_OTG; diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c index b25a111..673d530 100644 --- a/drivers/usb/common/common.c +++ b/drivers/usb/common/common.c @@ -99,7 +99,6 @@ const char *usb_state_string(enum usb_device_state state) } EXPORT_SYMBOL_GPL(usb_state_string); -#ifdef CONFIG_OF static const char *const usb_dr_modes[] = { [USB_DR_MODE_UNKNOWN] = , [USB_DR_MODE_HOST] = host, @@ -107,19 +106,12 @@ static const char *const usb_dr_modes[] = { [USB_DR_MODE_OTG] = otg, }; -/** - * of_usb_get_dr_mode - Get dual role mode for given device_node - * @np:Pointer to the given device_node - * - * The function gets phy interface string from property 'dr_mode', - * and returns the correspondig enum usb_dr_mode - */ -enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np) +enum usb_dr_mode usb_get_dr_mode(struct device *dev) { const char *dr_mode; int err, i; - err = of_property_read_string(np, dr_mode, dr_mode); + err = device_property_read_string(dev, dr_mode, dr_mode); if (err 0) return USB_DR_MODE_UNKNOWN; @@ -129,8 +121,9 @@ enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np) return USB_DR_MODE_UNKNOWN; } -EXPORT_SYMBOL_GPL(of_usb_get_dr_mode); +EXPORT_SYMBOL_GPL(usb_get_dr_mode); +#ifdef CONFIG_OF /** * of_usb_host_tpl_support - to get if Targeted Peripheral List is supported * for given targeted hosts (non-PC hosts) diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index 9093530..73bb75d 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -220,7 +220,7 @@ static int dwc2_driver_probe(struct platform_device *dev) dev_dbg(dev-dev, mapped PA %08lx to VA %p\n, (unsigned long)res-start, hsotg-regs); - hsotg-dr_mode = of_usb_get_dr_mode(dev-dev.of_node); + hsotg-dr_mode = usb_get_dr_mode(dev-dev); /* * Attempt to find a generic PHY, then look for an old style diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 9694799..8a76b39 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -843,6 +843,7 @@ static int dwc3_probe(struct platform_device *pdev) hird_threshold = 12; dwc-maximum_speed = usb_get_maximum_speed(dev); + dwc-dr_mode = usb_get_dr_mode(dev); if (node) { dwc-has_lpm_erratum = of_property_read_bool(node, @@ -858,7 +859,6 @@ static int dwc3_probe(struct platform_device *pdev) dwc-needs_fifo_resize = of_property_read_bool(node, tx-fifo-resize); - dwc-dr_mode = of_usb_get_dr_mode(node); dwc-disable_scramble_quirk = of_property_read_bool(node, snps,disable_scramble_quirk); diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c index 02d47d5..5c0adb9 100644 --- a/drivers/usb/dwc3/dwc3-st.c +++ b/drivers/usb/dwc3/dwc3-st.c @@ -268,7 +268,7 @@ static int st_dwc3_probe(struct platform_device *pdev) goto undo_softreset; } - dwc3_data-dr_mode = of_usb_get_dr_mode(child_pdev-dev.of_node); + dwc3_data-dr_mode = usb_get_dr_mode(child_pdev-dev); /* * Configure the USB port as device or host according to the static diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index a0cfead..2f9b636 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -663,7 +663,7 @@ static int get_musb_port_mode(struct device *dev) { enum usb_dr_mode mode; - mode
[PATCH 4/5] usb: dwc3: core: convert to unified device property interface
No functional affect on existing platforms, but the driver is now ready to extract the properties also from ACPI tables as well as from DT. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com --- drivers/usb/dwc3/core.c | 45 ++--- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 8a76b39..f6a12b0 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -773,7 +773,6 @@ static int dwc3_probe(struct platform_device *pdev) { struct device *dev = pdev-dev; struct dwc3_platform_data *pdata = dev_get_platdata(dev); - struct device_node *node = dev-of_node; struct resource *res; struct dwc3 *dwc; u8 lpm_nyet_threshold; @@ -845,49 +844,49 @@ static int dwc3_probe(struct platform_device *pdev) dwc-maximum_speed = usb_get_maximum_speed(dev); dwc-dr_mode = usb_get_dr_mode(dev); - if (node) { - dwc-has_lpm_erratum = of_property_read_bool(node, + dwc-has_lpm_erratum = device_property_read_bool(dev, snps,has-lpm-erratum); - of_property_read_u8(node, snps,lpm-nyet-threshold, + device_property_read_u8(dev, snps,lpm-nyet-threshold, lpm_nyet_threshold); - dwc-is_utmi_l1_suspend = of_property_read_bool(node, + dwc-is_utmi_l1_suspend = device_property_read_bool(dev, snps,is-utmi-l1-suspend); - of_property_read_u8(node, snps,hird-threshold, + device_property_read_u8(dev, snps,hird-threshold, hird_threshold); - dwc-usb3_lpm_capable = of_property_read_bool(node, + dwc-usb3_lpm_capable = device_property_read_bool(dev, snps,usb3_lpm_capable); - dwc-needs_fifo_resize = of_property_read_bool(node, + dwc-needs_fifo_resize = device_property_read_bool(dev, tx-fifo-resize); - dwc-disable_scramble_quirk = of_property_read_bool(node, + dwc-disable_scramble_quirk = device_property_read_bool(dev, snps,disable_scramble_quirk); - dwc-u2exit_lfps_quirk = of_property_read_bool(node, + dwc-u2exit_lfps_quirk = device_property_read_bool(dev, snps,u2exit_lfps_quirk); - dwc-u2ss_inp3_quirk = of_property_read_bool(node, + dwc-u2ss_inp3_quirk = device_property_read_bool(dev, snps,u2ss_inp3_quirk); - dwc-req_p1p2p3_quirk = of_property_read_bool(node, + dwc-req_p1p2p3_quirk = device_property_read_bool(dev, snps,req_p1p2p3_quirk); - dwc-del_p1p2p3_quirk = of_property_read_bool(node, + dwc-del_p1p2p3_quirk = device_property_read_bool(dev, snps,del_p1p2p3_quirk); - dwc-del_phy_power_chg_quirk = of_property_read_bool(node, + dwc-del_phy_power_chg_quirk = device_property_read_bool(dev, snps,del_phy_power_chg_quirk); - dwc-lfps_filter_quirk = of_property_read_bool(node, + dwc-lfps_filter_quirk = device_property_read_bool(dev, snps,lfps_filter_quirk); - dwc-rx_detect_poll_quirk = of_property_read_bool(node, + dwc-rx_detect_poll_quirk = device_property_read_bool(dev, snps,rx_detect_poll_quirk); - dwc-dis_u3_susphy_quirk = of_property_read_bool(node, + dwc-dis_u3_susphy_quirk = device_property_read_bool(dev, snps,dis_u3_susphy_quirk); - dwc-dis_u2_susphy_quirk = of_property_read_bool(node, + dwc-dis_u2_susphy_quirk = device_property_read_bool(dev, snps,dis_u2_susphy_quirk); - dwc-tx_de_emphasis_quirk = of_property_read_bool(node, + dwc-tx_de_emphasis_quirk = device_property_read_bool(dev, snps,tx_de_emphasis_quirk); - of_property_read_u8(node, snps,tx_de_emphasis, + device_property_read_u8(dev, snps,tx_de_emphasis, tx_de_emphasis); - of_property_read_string(node, snps,hsphy_interface, - dwc-hsphy_interface); - } else if (pdata) { + device_property_read_string(dev, snps,hsphy_interface, + dwc-hsphy_interface); + + if (pdata) { dwc-maximum_speed = pdata-maximum_speed; dwc-has_lpm_erratum = pdata-has_lpm_erratum; if (pdata-lpm_nyet_threshold) -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the
[PATCH 5/5] usb: dwc3: pci: passing forward the ACPI companion
Sharing the ACPI companion with dwc3 core so it has access to the properties defined for DWC3 in ACPI tables. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com --- drivers/usb/dwc3/dwc3-pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c index f626179..89eb364 100644 --- a/drivers/usb/dwc3/dwc3-pci.c +++ b/drivers/usb/dwc3/dwc3-pci.c @@ -154,6 +154,7 @@ static int dwc3_pci_probe(struct pci_dev *pci, goto err; dwc3-dev.parent = dev; + ACPI_COMPANION_SET(dwc3-dev, ACPI_COMPANION(dev)); ret = platform_device_add(dwc3); if (ret) { -- 2.5.0 -- 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
[TESTPATCH] xhci: custom debug patch for command mismatch
Add extra command ring, event ring and command queue debugging to get more info on command mismatch error triggered by Vincent Pelletier on BayTrail will blurt out around 500 lines when hitting a command mismatch Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com --- drivers/usb/host/xhci-dbg.c | 6 +++--- drivers/usb/host/xhci-ring.c | 16 drivers/usb/host/xhci.c | 4 ++-- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c index a915031..f4ec5db 100644 --- a/drivers/usb/host/xhci-dbg.c +++ b/drivers/usb/host/xhci-dbg.c @@ -330,7 +330,7 @@ void xhci_debug_segment(struct xhci_hcd *xhci, struct xhci_segment *seg) for (i = 0; i TRBS_PER_SEGMENT; ++i) { trb = seg-trbs[i]; - xhci_dbg(xhci, @%016llx %08x %08x %08x %08x\n, addr, + xhci_err(xhci, @%016llx %08x %08x %08x %08x\n, addr, lower_32_bits(le64_to_cpu(trb-link.segment_ptr)), upper_32_bits(le64_to_cpu(trb-link.segment_ptr)), le32_to_cpu(trb-link.intr_target), @@ -426,9 +426,9 @@ void xhci_dbg_cmd_ptrs(struct xhci_hcd *xhci) u64 val; val = xhci_read_64(xhci, xhci-op_regs-cmd_ring); - xhci_dbg(xhci, // xHC command ring deq ptr low bits + flags = @%08x\n, + xhci_err(xhci, // xHC command ring deq ptr low bits + flags = @%08x\n, lower_32_bits(val)); - xhci_dbg(xhci, // xHC command ring deq ptr high bits = @%08x\n, + xhci_err(xhci, // xHC command ring deq ptr high bits = @%08x\n, upper_32_bits(val)); } diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 32f4d56..4db77a6 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1295,19 +1295,35 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, /* Is the command ring deq ptr out of sync with the deq seg ptr? */ if (cmd_dequeue_dma == 0) { xhci-error_bitmask |= 1 4; + xhci_err(xhci, Cmd ring deq ptr and seg out of sync\n); return; } /* Does the DMA address match our internal dequeue pointer address? */ if (cmd_dma != (u64) cmd_dequeue_dma) { xhci-error_bitmask |= 1 5; + xhci_err(xhci, Cmd DMA address mismatch %llx, %llx\n, +cmd_dma, (u64)cmd_dequeue_dma); return; } cmd = list_entry(xhci-cmd_list.next, struct xhci_command, cmd_list); if (cmd-command_trb != xhci-cmd_ring-dequeue) { + struct xhci_command *cur_cmd, *tmp_cmd; xhci_err(xhci, Command completion event does not match command\n); + xhci_err(xhci, cmd-command_trb: %p, cmd_ring_dequeue: %p\n, +cmd-command_trb, xhci-cmd_ring-dequeue); + + xhci_err(xhci, * Command list * (Current cmd trb= %p)\n, +xhci-current_cmd-command_trb); + list_for_each_entry_safe(cur_cmd, tmp_cmd, xhci-cmd_list, cmd_list) + xhci_err(xhci, list cmd trb: %p\n, cur_cmd-command_trb); + + xhci_err(xhci, * Command ring * (dq= %p)\n, xhci-cmd_ring-dequeue); + xhci_debug_ring(xhci, xhci-cmd_ring); + xhci_err(xhci, * Event ring * (dq= %p\n)\n, xhci-event_ring-dequeue); + xhci_debug_ring(xhci, xhci-event_ring); return; } diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index a9a96b8..8192d14 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -605,14 +605,14 @@ int xhci_run(struct usb_hcd *hcd) return ret; xhci_dbg(xhci, Command ring memory map follows:\n); - xhci_debug_ring(xhci, xhci-cmd_ring); +/* xhci_debug_ring(xhci, xhci-cmd_ring); */ xhci_dbg_ring_ptrs(xhci, xhci-cmd_ring); xhci_dbg_cmd_ptrs(xhci); xhci_dbg(xhci, ERST memory map follows:\n); xhci_dbg_erst(xhci, xhci-erst); xhci_dbg(xhci, Event ring:\n); - xhci_debug_ring(xhci, xhci-event_ring); +/* xhci_debug_ring(xhci, xhci-event_ring); */ xhci_dbg_ring_ptrs(xhci, xhci-event_ring); temp_64 = xhci_read_64(xhci, xhci-ir_set-erst_dequeue); temp_64 = ~ERST_PTR_MASK; -- 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 2/5] usb: dwc3: st: prepare the driver for generic usb_get_dr_mode function
of_usb_get_dr_mode will be converted into more generic usb_get_dr_mode function that will take struct device instead of struct device_node as it's parameter. To make the conversion possible later, waiting for the platform device for dwc3 to be populated before calling of_usb_get_dr_mode. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com CC: Giuseppe Cavallaro peppe.cavall...@st.com CC: Peter Griffin peter.grif...@linaro.org --- drivers/usb/dwc3/dwc3-st.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c index de4d52f..02d47d5 100644 --- a/drivers/usb/dwc3/dwc3-st.c +++ b/drivers/usb/dwc3/dwc3-st.c @@ -195,6 +195,7 @@ static int st_dwc3_probe(struct platform_device *pdev) struct resource *res; struct device *dev = pdev-dev; struct device_node *node = dev-of_node, *child; + struct platform_device *child_pdev; struct regmap *regmap; int ret; @@ -253,8 +254,6 @@ static int st_dwc3_probe(struct platform_device *pdev) goto undo_softreset; } - dwc3_data-dr_mode = of_usb_get_dr_mode(child); - /* Allocate and initialize the core */ ret = of_platform_populate(node, NULL, NULL, dev); if (ret) { @@ -262,6 +261,15 @@ static int st_dwc3_probe(struct platform_device *pdev) goto undo_softreset; } + child_pdev = of_find_device_by_node(child); + if (!child_pdev) { + dev_err(dev, failed to find dwc3 core device\n); + ret = -ENODEV; + goto undo_softreset; + } + + dwc3_data-dr_mode = of_usb_get_dr_mode(child_pdev-dev.of_node); + /* * Configure the USB port as device or host according to the static * configuration passed from DT. -- 2.5.0 -- 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: [TESTPATCH] xhci: custom debug patch for command mismatch
On Tue, 25 Aug 2015 21:09:20 +0200, Vincent Pelletier plr.vinc...@gmail.com wrote: I'm not sure how I'll get this (I have a lot to learn about kernel debugging). Answering to myself: log_buf_len kernel parameter. I sent you my dmesg off-list (335KB raw, 46KB gzipped). FWIW, I gave a try at unloading and reloading just xhci_pci, and the error did not pop up after more than 30 cycles. And of course, plugging a screen screen after requesting reboot, I caught a glimpse of many messages on TTY not appearing in the log file. It was too shot for me to tell if they were ones added in your patch though. Command completion event does not match command does not even appear there. Regards, -- Vincent Pelletier -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
-Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: August-25-15 3:36 PM To: Roman Bacik Cc: John Youn; Scott Branden; Greg Kroah-Hartman; linux- u...@vger.kernel.org; linux-ker...@vger.kernel.org; bcm-kernel-feedback- list Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode On Tue, Aug 25, 2015 at 10:00:17PM +, Roman Bacik wrote: -Original Message- From: John Youn [mailto:john.y...@synopsys.com] Sent: August-25-15 2:52 PM To: Scott Branden; John Youn; Greg Kroah-Hartman; linux- u...@vger.kernel.org Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list; Roman Bacik Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode On 8/18/2015 8:45 AM, Scott Branden wrote: From: Roman Bacik rba...@broadcom.com USB OTG driver in isochronous mode has to set the parity of the receiving microframe. The parity is set to even by default. This causes problems for an audio gadget, if the host starts transmitting on odd microframes. This fix uses Incomplete Periodic Transfer interrupt to toggle between even and odd parity until the Transfer Complete interrupt is received. Signed-off-by: Roman Bacik rba...@broadcom.com Reviewed-by: Abhinav Ratna ara...@broadcom.com Reviewed-by: Srinath Mannam srinath.man...@broadcom.com Reviewed-by: Scott Branden sbran...@broadcom.com Signed-off-by: Scott Branden sbran...@broadcom.com --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 48 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 0ed87620..954d1cd 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -150,6 +150,7 @@ struct s3c_hsotg_ep { unsigned intperiodic:1; unsigned intisochronous:1; unsigned intsend_zlp:1; + unsigned intparity_set:1; charname[10]; }; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4d47b7c..28e4393 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -1954,6 +1954,8 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx, ints = ~DXEPINT_XFERCOMPL; if (ints DXEPINT_XFERCOMPL) { + if (hs_ep-isochronous !hs_ep-parity_set) + hs_ep-parity_set = 1; it shouldn't be a problem to set the flag which was already set, so this could be simplified to: hs_ep-has_correct_parity = !!hs_ep0isochronous; It can be simplified to: hs_ep-has_correct_parity = 1; I just thought that the original shows better what we are trying to do. I do not mind to simplify it and remove the condition. if (hs_ep-isochronous hs_ep-interval == 1) { if (ctrl DXEPCTL_EOFRNUM) ctrl |= DXEPCTL_SETEVENFR; @@ -2316,7 +2318,8 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST | GINTSTS_RESETDET | GINTSTS_ENUMDONE | GINTSTS_OTGINT | GINTSTS_USBSUSP | - GINTSTS_WKUPINT, + GINTSTS_WKUPINT | + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT, why the two extra bits ? What are they doing ? This fix uses Incomplete Periodic Transfer interrupt (GINTSTS_INCOMPL) to toggle between even and odd parity until the Transfer Complete interrupt is received. We also need to set correct parity on both IN and OUT endpoints. hsotg-regs + GINTMSK); if (using_dma(hsotg)) @@ -2581,6 +2584,48 @@ irq_retry: s3c_hsotg_dump(hsotg); } + if (gintsts GINTSTS_INCOMPL_SOIN) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg-dev, %s: GINTSTS_INCOMPL_SOIN\n, __func__); + for (idx = 1; idx MAX_EPS_CHANNELS; idx++) { u32 epctl_reg; u32 ctrl; + hs_ep = hsotg-eps_in[idx]; you can decrease some indentation here: if (!hs_ep-isochronous) continue; if (hs_ep-has_correct_parity) continue; epctl_reg = DIEPCTL(idx); ctrl = readl(hsotg-regs + epctl_reg); if (ctrl DXEPCTL_EOFRNUM) ctrl |= DXEPCTL_SETEVENFR;
Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M
On Tue, 25 Aug 2015, Roland Weber wrote: Hi Alan, 638139eb95d2d241781330a321e88c8dafe46078 is the first bad commit Are you certain you found the right one? Yes. I kept all the kernels during bisecting and double-checked today. That was the second-to-last one, and it freezes. The last one was parent commit (37ebb54915dc) and it works. I'm including the output of git bisect log below. I also used the opportunity to collect more system information and directly compare the last working and the first freezing version. Note that the device numbering has changed since then. In the information below, the devices that cause the freeze are on the USB bus 001. Furthermore, the code changed by that commit doesn't run when you do lsusb -v. It runs only when the USB stack first starts up or when a new host controller is registered. OK, now things are getting interesting. Might the code also run during shutdown? I mentioned in my bug report that the system freezes on shutdown. Not exactly. You wrote: [1.] One line summary of the problem: lsusb -v as root freezes the kernel on Acer ES1-111M, which seems to imply that the system freezes when you run lsusb. At least, that's how I interpreted it. At the end, you also wrote During shutdown of the OS, the kernel also freezes. That's not entirely clear -- how can the kernel freeze when you run lsusb -v and then freeze again during shutdown? Do you mean that the bad kernel freezes during shutdown even if you don't run lsusb? Anyway, the changed code does not run during shutdown. I can confirm now that this is also caused by the above commit. Furthermore, there are significant differences during startup. The last good version prints a bunch of messages like these: (see further below for full dmesg output) [ 13.883381] usb usb1-port1: Cannot enable. Maybe the USB cable is bad? [ 13.941952] ehci-pci :00:1d.0: port 1 reset error -110 [ 14.814627] ehci-pci :00:1d.0: port 1 reset error -110 [ 15.631041] ehci-pci :00:1d.0: port 1 reset error -110 [ 16.448006] ehci-pci :00:1d.0: port 1 reset error -110 [ 17.264644] ehci-pci :00:1d.0: port 1 reset error -110 [ 17.874644] usb usb1-port1: Cannot enable. Maybe the USB cable is bad? [ 17.874677] usb usb1-port1: unable to enumerate USB device So that is the bus in question. I have verified that it is not connected to the external USB ports of the machine. So far, I haven't missed any of the internal devices that I know of either. So I presume that this USB bus is simply unused. Maybe the manufacturer decided to just leave it disconnected, instead of properly grounding the pins? I doubt it, in view of your later finding... With the commit that causes the freeze, these messages no longer appear. Instead, the kernel finds an additional device that the previous version did not. It's an USB hub, connected to bus 001 (last line): /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/1p, 5000M /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/6p, 480M |__ Port 2: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M |__ Port 1: Dev 4, If 0, Class=Wireless, Driver=, 12M |__ Port 1: Dev 4, If 1, Class=Wireless, Driver=, 12M |__ Port 4: Dev 3, If 0, Class=Video, Driver=uvcvideo, 480M |__ Port 4: Dev 3, If 1, Class=Video, Driver=uvcvideo, 480M /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/8p, 480M |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M This is very bizarre. The code changes are minimal; they should not affect anything like detection of devices. The Cannot enable error comes directly from the hardware. If you want, you could try an even finer bisection. The commit you identified adds a mutex_lock and a mutex_unlock, and it also changes an alloc_ordered_workqueue to alloc_workqueue. You could leave the mutex stuff out and just include the alloc_workqueue change, or vice versa. Could it be that lsusb -v triggers a lazy initialization of that hub? Not lazy, but it would cause the probe sequence to occur again. This time it might succeed -- you should be able to tell from the dmesg log. And that the shutdown sequence does the same? No. 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: message: remove redundant declaration
Hello. On 08/25/2015 05:30 AM, Kris Borer wrote: Fix the Sparse warning: message.c:1390:21: warning: symbol 'i' shadows an earlier one message.c:1294:13: originally declared here Signed-off-by: Kris Borer kbo...@gmail.com --- drivers/usb/core/message.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index f368d20..e83648d 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1387,7 +1387,6 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) * new altsetting. */ if (manual) { - int i; Why not remove (now unneeded) empty line? for (i = 0; i alt-desc.bNumEndpoints; i++) { epaddr = alt-endpoint[i].desc.bEndpointAddress; MBR, Sergei -- 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] usbnet: Fix two races between usbnet_stop() and the BH
On Mon, 2015-08-24 at 15:29 +0200, Bjørn Mork wrote: Eugene Shatokhin eugene.shatok...@rosalab.ru writes: 19.08.2015 15:31, Bjørn Mork пишет: Eugene Shatokhin eugene.shatok...@rosalab.ru writes: Stopping the tasklet rescheduling etc depends only on netif_running(), which will be false when usbnet_stop is called. There is no need to touch dev-flags for this to happen. That was one of the first ideas we discussed here. Unfortunately, it is probably not so simple. Setting dev-flags to 0 makes some delayed operations do nothing and, among other things, not to reschedule usbnet_bh(). Yes, but I believe that is merely a side effect. You should never need to clear multiple flags to get the desired behaviour. Why? Is there any reason you cannot have a TX and an RX halt at the same time? As you can see in drivers/net/usb/usbnet.c, usbnet_bh() can be called as a tasklet function and as a timer function in a number of situations (look for the usage of dev-bh and dev-delay there). netif_running() is indeed false when usbnet_stop() runs, usbnet_stop() also disables Tx. This seems to be enough for many cases where usbnet_bh() is scheduled, but I am not so sure about the remaining ones, namely: 1. A work function, usbnet_deferred_kevent(), may reschedule usbnet_bh(). Looks like the workqueue is only stopped in usbnet_disconnect(), so a work item might be processed while usbnet_stop() works. Setting dev-flags to 0 makes the work function do nothing, by the way. See also the comment in usbnet_stop() about this. Yes, this is the main reason the flags are collectively cleared. We could do them all with clear_bit(). Ugly though. A work item may be placed to this workqueue in a number of ways, by both usbnet module and the mini-drivers. It is not too easy to track all these situations. That's an understatement :) Yes. So FLAG_AVOID_UNLINK_URBS should probably be removed and replaced calls to usbnet_status_start() and usbnet_status_stop(). This will require testing on some of the devices with the original firmware problem however. And there you point out the main problem. In any case: I do not think this flag should be considered when trying to make usbnet_stop behaviour saner. It's only purpose is to deliberately break usbnet_stop by not actually stopping. Yes. 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] usbnet: Fix two races between usbnet_stop() and the BH
On Mon, 2015-08-24 at 14:21 -0400, Alan Stern wrote: In theory, an architecture could implement atomic bit operations using a spinlock to insure atomicity. I don't know if any architectures do this, but if they do then the scenario above could arise. Now that I see this in writing, I realize it's not possible after all. clear_bit() et al. will work with a single unsigned long, which doesn't leave any place for spinlocks or other mechanisms. I was thinking of atomic_t. Refuting yourself you are making the assumption that the lock has to be inside the data structure. That is not true. 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 1/2] usbnet: Get EVENT_NO_RUNTIME_PM bit before it is cleared
On Mon, 2015-08-24 at 23:13 +0300, Eugene Shatokhin wrote: It is needed to check EVENT_NO_RUNTIME_PM bit of dev-flags in usbnet_stop(), but its value should be read before it is cleared when dev-flags is set to 0. Can we agree that this at least is good and should go upstream and into stable? 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 1/1] usb: dwc2: gadget: parity fix in isochronous mode
On 8/25/2015 3:00 PM, Roman Bacik wrote: -Original Message- From: John Youn [mailto:john.y...@synopsys.com] Sent: August-25-15 2:52 PM To: Scott Branden; John Youn; Greg Kroah-Hartman; linux- u...@vger.kernel.org Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list; Roman Bacik Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode On 8/18/2015 8:45 AM, Scott Branden wrote: From: Roman Bacik rba...@broadcom.com USB OTG driver in isochronous mode has to set the parity of the receiving microframe. The parity is set to even by default. This causes problems for an audio gadget, if the host starts transmitting on odd microframes. This fix uses Incomplete Periodic Transfer interrupt to toggle between even and odd parity until the Transfer Complete interrupt is received. Signed-off-by: Roman Bacik rba...@broadcom.com Reviewed-by: Abhinav Ratna ara...@broadcom.com Reviewed-by: Srinath Mannam srinath.man...@broadcom.com Reviewed-by: Scott Branden sbran...@broadcom.com Signed-off-by: Scott Branden sbran...@broadcom.com --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 48 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 0ed87620..954d1cd 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -150,6 +150,7 @@ struct s3c_hsotg_ep { unsigned intperiodic:1; unsigned intisochronous:1; unsigned intsend_zlp:1; + unsigned intparity_set:1; charname[10]; }; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4d47b7c..28e4393 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -1954,6 +1954,8 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx, ints = ~DXEPINT_XFERCOMPL; if (ints DXEPINT_XFERCOMPL) { + if (hs_ep-isochronous !hs_ep-parity_set) + hs_ep-parity_set = 1; if (hs_ep-isochronous hs_ep-interval == 1) { if (ctrl DXEPCTL_EOFRNUM) ctrl |= DXEPCTL_SETEVENFR; @@ -2316,7 +2318,8 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST | GINTSTS_RESETDET | GINTSTS_ENUMDONE | GINTSTS_OTGINT | GINTSTS_USBSUSP | - GINTSTS_WKUPINT, + GINTSTS_WKUPINT | + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT, hsotg-regs + GINTMSK); if (using_dma(hsotg)) @@ -2581,6 +2584,48 @@ irq_retry: s3c_hsotg_dump(hsotg); } + if (gintsts GINTSTS_INCOMPL_SOIN) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg-dev, %s: GINTSTS_INCOMPL_SOIN\n, __func__); + for (idx = 1; idx MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg-eps_in[idx]; + if (hs_ep-isochronous !hs_ep-parity_set) { + u32 epctl_reg = DIEPCTL(idx); + u32 ctrl = readl(hsotg-regs + epctl_reg); + + if (ctrl DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg-regs + epctl_reg); + } + } + writel(GINTSTS_INCOMPL_SOIN, hsotg-regs + GINTSTS); + } + + if (gintsts GINTSTS_INCOMPL_SOOUT) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg-dev, %s: GINTSTS_INCOMPL_SOOUT\n, __func__); + for (idx = 1; idx MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg-eps_out[idx]; + if (hs_ep-isochronous !hs_ep-parity_set) { + u32 epctl_reg = DOEPCTL(idx); + u32 ctrl = readl(hsotg-regs + epctl_reg); + + if (ctrl DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg-regs + epctl_reg); + } + } + writel(GINTSTS_INCOMPL_SOOUT, hsotg-regs + GINTSTS); + } + /* * if we've had fifo events, we should try and go around the * loop again to see if there's any point in returning yet. @@ -2667,6 +2712,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep, hs_ep-periodic = 0; hs_ep-halted = 0; hs_ep-interval = desc-bInterval; + hs_ep-parity_set = 0; I'm not quite sure what the parity_set flag does in this patch. Shouldn't you be able to