Re: [PATCH] USB: serial: move the simple drivers into usb-serial-simple.c
On Mon, Aug 05, 2013 at 09:36:23AM -0500, Dan Williams wrote: On Mon, 2013-08-05 at 19:41 +0800, Greg Kroah-Hartman wrote: +/* All of the above structures mushed into two lists */ +static struct usb_serial_driver * const serial_drivers[] = { + zio_device, + funsoft_device, + flashloader_device, + vivopay_device, + moto_device, Maybe moto_modem_device to preserve the module name? I don't actually care either way, but using only moto_device here means driver.name will now be different for these devices. Good point, I've now changed it, thanks for the review. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] usb: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
At former design, both ci_hdrc_imx and usbmisc_imx are individual module, ci_hdrc_imx is glue layer for imx usb driver. usbmisc_imx handles non-core registers which has different register layout for imx SoC serials, it usually supplies interface for wakeup setting, PHY setting, etc. usbmisc_imx uses symbols from ci_hdrc_imx, So, when we combile both of drivers as loadable module, usbmisc is needed to be unload first. However at ci_hdrc_imx, it needs to call usbmisc_imx's interface at its removal function once we enable wakeup/runtime pm function, so keeping two drivers can't work well at loadable module use case. To fix loadable module use case, we need to build usbmisc_imx into ci_hdrc_imx. The usbmisc_imx still handles misc setting for controllers, and will be included at ci_hdrc_imx. There is a short discussion for it: http://marc.info/?l=linux-usbm=136861599423172w=2 Besides, we update dts and binding doc for at this commit Signed-off-by: Peter Chen peter.c...@freescale.com --- .../devicetree/bindings/usb/ci13xxx-imx.txt| 10 +- .../devicetree/bindings/usb/usbmisc-imx.txt| 14 -- arch/arm/boot/dts/imx23.dtsi |1 + arch/arm/boot/dts/imx25.dtsi | 14 +- arch/arm/boot/dts/imx28.dtsi |2 + arch/arm/boot/dts/imx51.dtsi | 13 + arch/arm/boot/dts/imx53.dtsi | 13 + arch/arm/boot/dts/imx6qdl.dtsi | 20 +- drivers/usb/chipidea/Makefile |2 +- drivers/usb/chipidea/ci_hdrc_imx.c | 125 ++- drivers/usb/chipidea/ci_hdrc_imx.h | 32 ++-- drivers/usb/chipidea/usbmisc_imx.c | 237 +--- 12 files changed, 192 insertions(+), 291 deletions(-) delete mode 100644 Documentation/devicetree/bindings/usb/usbmisc-imx.txt diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt index b4b5b79..13720a2 100644 --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt @@ -13,8 +13,8 @@ Recommended properies: Optional properties: - fsl,usbphy: phandler of usb phy that connects to the only one port -- fsl,usbmisc: phandler of non-core register device, with one argument - that indicate usb controller index +- ci,noncore: phandler of non-core register device, the user must enable +syscon driver to use it - vbus-supply: regulator for vbus - disable-over-current: disable over current detect - external-vbus-divider: enables off-chip resistor divider for Vbus @@ -25,7 +25,11 @@ usb@02184000 { /* USB OTG */ reg = 0x02184000 0x200; interrupts = 0 43 0x04; fsl,usbphy = usbphy1; - fsl,usbmisc = usbmisc 0; + ci,noncore = noncore; disable-over-current; external-vbus-divider; }; +noncore: usb-non-core@02184800 { +compatible = fsl,imx-usb-non-core, syscon; +reg = 0x02184800 0x200; +}; diff --git a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt b/Documentation/devicetree/bindings/usb/usbmisc-imx.txt deleted file mode 100644 index 97ce94e..000 --- a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt +++ /dev/null @@ -1,14 +0,0 @@ -* Freescale i.MX non-core registers - -Required properties: -- #index-cells: Cells used to descibe usb controller index. Should be 1 -- compatible: Should be one of below: - fsl,imx6q-usbmisc for imx6q -- reg: Should contain registers location and length - -Examples: -usbmisc@02184800 { - #index-cells = 1; - compatible = fsl,imx6q-usbmisc; - reg = 0x02184800 0x200; -}; diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi index 587ceef..a0d8471 100644 --- a/arch/arm/boot/dts/imx23.dtsi +++ b/arch/arm/boot/dts/imx23.dtsi @@ -20,6 +20,7 @@ gpio2 = gpio2; serial0 = auart0; serial1 = auart1; + usb0 = usb0; }; cpus { diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi index 1f72862..7bb94ce 100644 --- a/arch/arm/boot/dts/imx25.dtsi +++ b/arch/arm/boot/dts/imx25.dtsi @@ -461,7 +461,7 @@ interrupts = 37; clocks = clks 9, clks 70, clks 8; clock-names = ipg, ahb, per; - fsl,usbmisc = usbmisc 0; + ci,noncore = noncore; status = disabled; }; @@ -471,17 +471,13 @@ interrupts = 35; clocks = clks 9, clks 70, clks 8; clock-names = ipg, ahb, per; - fsl,usbmisc = usbmisc 1; + ci,noncore = noncore; status = disabled;
Re: [PATCH 2/2] ARM: OMAP: rx51: change musb mode to OTG
Hi Aaro, On 06.08.2013 19:06, Aaro Koskinen wrote: Peripheral-only mode got broken in v3.11-rc1 because of unknown reasons. Given that you have a board that is affected, could you investigate a little maybe? I would have done it a long time ago, but I don't have access to such hardware unfortunately. Not saying that your patch shouldn't go in, but at some point, we need a sane explanation why this has happened. Thanks, Daniel -- 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: VM Kernel bug with CONFIG_DEBUG_VM
On Wed, Aug 7, 2013 at 1:41 PM, Christoffer Dall christoffer.d...@linaro.org wrote: Hi all, I've found that booting a recent kernel (and as far back as 3.9 at least) I get a kernel bug during boot with VM debugging enabled. I haven't had time to dig further into it, but wanted to check if this is a known issue? Remember Catalin introduced the flush_dcache_page() to fix cache incoherency problem when PIO reading from USB mass storage device to mapped page, so the flush should not need for pages not in page cache. Care to test below patch to see if the problem can be fixed? diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c index 2facee5..9d47c71 100644 --- a/drivers/usb/host/isp1760-hcd.c +++ b/drivers/usb/host/isp1760-hcd.c @@ -707,8 +707,12 @@ __acquires(priv-lock) void *ptr; for (ptr = urb-transfer_buffer; ptr urb-transfer_buffer + urb-transfer_buffer_length; -ptr += PAGE_SIZE) - flush_dcache_page(virt_to_page(ptr)); +ptr += PAGE_SIZE) { + struct page *page = virt_to_page(ptr); + if (PageSlab(page)) + break; + flush_dcache_page(page); + } } /* complete() can reenter this HCD */ Thanks, -- Ming Lei -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 01:34:37PM +0800, Peter Chen wrote: At former design, both ci_hdrc_imx and usbmisc_imx are individual module, ci_hdrc_imx is glue layer for imx usb driver. usbmisc_imx handles non-core registers which has different register layout for imx SoC serials, it usually supplies interface for wakeup setting, PHY setting, etc. usbmisc_imx uses symbols from ci_hdrc_imx, So, when we combile both of drivers as loadable module, usbmisc is needed to be unload first. However at ci_hdrc_imx, it needs to call usbmisc_imx's interface at its removal function once we enable wakeup/runtime pm function, so keeping two drivers can't work well at loadable module use case. To fix loadable module use case, we need to build usbmisc_imx into ci_hdrc_imx. The usbmisc_imx still handles misc setting for controllers, and will be included at ci_hdrc_imx. There is a short discussion for it: http://marc.info/?l=linux-usbm=136861599423172w=2 Besides, we update dts and binding doc for at this commit This patch does far too many things at once. - Convert imx-usb-misc from a driver into something which is called directly - add aliases - change devicetree bindings (which causes pain and it's not explained why this is necessary at all) - converts the misc stuff to regmap Please split this up so that it can be reviewed. #include ci.h #include ci_hdrc_imx.h +#include usbmisc_imx.c Don't include C files. - ret); - memset(usbdev, 0, sizeof(*usbdev)); + ret = of_alias_get_id(np, usb); + if (ret 0) { + dev_err(dev, failed to get alias id, errno %d\n, ret); return ret; } - usbdev-index = args.args[0]; - of_node_put(args.np); if (of_find_property(np, disable-over-current, NULL)) - usbdev-disable_oc = 1; + data-disable_oc = 1; if (of_find_property(np, external-vbus-divider, NULL)) - usbdev-evdo = 1; + data-evdo = 1; + + /* mx23 and mx28 doesn't have non core registers */ + if (data-misc_data (!strcmp(data-misc_data-name, imx23-usb) || + !strcmp(data-misc_data-name, imx28-usb))) + return 0; If a USB node does not have a usbmisc (or noncore) property then don't initialize it. No need for string compares. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 10:15:22AM +0200, Sascha Hauer wrote: On Wed, Aug 07, 2013 at 01:34:37PM +0800, Peter Chen wrote: At former design, both ci_hdrc_imx and usbmisc_imx are individual module, ci_hdrc_imx is glue layer for imx usb driver. usbmisc_imx handles non-core registers which has different register layout for imx SoC serials, it usually supplies interface for wakeup setting, PHY setting, etc. usbmisc_imx uses symbols from ci_hdrc_imx, So, when we combile both of drivers as loadable module, usbmisc is needed to be unload first. However at ci_hdrc_imx, it needs to call usbmisc_imx's interface at its removal function once we enable wakeup/runtime pm function, so keeping two drivers can't work well at loadable module use case. To fix loadable module use case, we need to build usbmisc_imx into ci_hdrc_imx. The usbmisc_imx still handles misc setting for controllers, and will be included at ci_hdrc_imx. There is a short discussion for it: http://marc.info/?l=linux-usbm=136861599423172w=2 Besides, we update dts and binding doc for at this commit This patch does far too many things at once. - Convert imx-usb-misc from a driver into something which is called directly - add aliases - change devicetree bindings (which causes pain and it's not explained why this is necessary at all) Yes, I need to split doc from implementation. - converts the misc stuff to regmap Please split this up so that it can be reviewed. The changes are all related to convert usbmisc_imx from a driver to something which can be called directly. - usbmisc has #index-cells to indicate controller number, now, it is not a driver, we use aliases at controller driver to instead of it. - When usbmisc_imx is a device, the register maps only one time. But ci_hdrc_imx will be several devices, the non-core register will be mapped several times, we have to use syscon to visit one register region among several devices. #include ci.h #include ci_hdrc_imx.h +#include usbmisc_imx.c Don't include C files. Yes, it is not a good practice, but I want to keep ci_hdrc_imx.c as clean as possible. The SoC related implementation will be more and more in the future after we add more USB functions. Do you have any good suggestions how to organize general imx stuff and SoC specifc? Put all the things at controller driver is another solution. - ret); - memset(usbdev, 0, sizeof(*usbdev)); + ret = of_alias_get_id(np, usb); + if (ret 0) { + dev_err(dev, failed to get alias id, errno %d\n, ret); return ret; } - usbdev-index = args.args[0]; - of_node_put(args.np); if (of_find_property(np, disable-over-current, NULL)) - usbdev-disable_oc = 1; + data-disable_oc = 1; if (of_find_property(np, external-vbus-divider, NULL)) - usbdev-evdo = 1; + data-evdo = 1; + + /* mx23 and mx28 doesn't have non core registers */ + if (data-misc_data (!strcmp(data-misc_data-name, imx23-usb) || + !strcmp(data-misc_data-name, imx28-usb))) + return 0; If a USB node does not have a usbmisc (or noncore) property then don't initialize it. No need for string compares. OK, will let the code be simple, I can't add device_id for mxs either. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 03:00:05PM +0800, Peter Chen wrote: On Wed, Aug 07, 2013 at 10:15:22AM +0200, Sascha Hauer wrote: On Wed, Aug 07, 2013 at 01:34:37PM +0800, Peter Chen wrote: At former design, both ci_hdrc_imx and usbmisc_imx are individual module, ci_hdrc_imx is glue layer for imx usb driver. usbmisc_imx handles non-core registers which has different register layout for imx SoC serials, it usually supplies interface for wakeup setting, PHY setting, etc. usbmisc_imx uses symbols from ci_hdrc_imx, So, when we combile both of drivers as loadable module, usbmisc is needed to be unload first. However at ci_hdrc_imx, it needs to call usbmisc_imx's interface at its removal function once we enable wakeup/runtime pm function, so keeping two drivers can't work well at loadable module use case. To fix loadable module use case, we need to build usbmisc_imx into ci_hdrc_imx. The usbmisc_imx still handles misc setting for controllers, and will be included at ci_hdrc_imx. There is a short discussion for it: http://marc.info/?l=linux-usbm=136861599423172w=2 Besides, we update dts and binding doc for at this commit This patch does far too many things at once. - Convert imx-usb-misc from a driver into something which is called directly - add aliases - change devicetree bindings (which causes pain and it's not explained why this is necessary at all) Yes, I need to split doc from implementation. No, you don't. You just have to explain *why* the bindings need to be chaged. - converts the misc stuff to regmap Please split this up so that it can be reviewed. The changes are all related to convert usbmisc_imx from a driver to something which can be called directly. - usbmisc has #index-cells to indicate controller number, now, it is not a driver, we use aliases at controller driver to instead of it. Why? - When usbmisc_imx is a device, the register maps only one time. But ci_hdrc_imx will be several devices, the non-core register will be mapped several times, we have to use syscon to visit one register region among several devices. Converting the imx-usb misc driver to regmap is fine, but I see no reason to not make this a separate patch. This would make this much easier to read. #include ci.h #include ci_hdrc_imx.h +#include usbmisc_imx.c Don't include C files. Yes, it is not a good practice, but I want to keep ci_hdrc_imx.c as clean as possible. The SoC related implementation will be more and more in the future after we add more USB functions. Just because you want to have the binary code in a single module doesn't mean the source code has to be in a single C file. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 01:34:37PM +0800, Peter Chen wrote: - - reg = usbmisc-base + MX25_USB_PHY_CTRL_OFFSET; - - if (usbdev-evdo) { - spin_lock_irqsave(usbmisc-lock, flags); - val = readl(reg); - writel(val | MX25_BM_EXTERNAL_VBUS_DIVIDER, reg); - spin_unlock_irqrestore(usbmisc-lock, flags); + if (data-evdo) { + spin_lock_irqsave(data-lock, flags); + regmap_read(data-non_core_base_addr, + MX25_USB_PHY_CTRL_OFFSET, val); + regmap_write(data-non_core_base_addr, + MX25_USB_PHY_CTRL_OFFSET, + val | MX25_BM_EXTERNAL_VBUS_DIVIDER); + spin_unlock_irqrestore(data-lock, flags); Have a look at regmap_update_bits. You won't need this spinlock anymore. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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: dwc3-pci: Ensure system sleep PM ops are defined only when used
On Tue, Aug 06, 2013 at 10:35:52PM -0300, Fabio Estevam wrote: On Tue, Aug 6, 2013 at 12:49 PM, Mark Brown broo...@kernel.org wrote: From: Andy Green andy.gr...@linaro.org You might have CONFIG_PM, but you might not have CONFIG_SUSPEND, in which case these are unused. Signed-off-by: Andy Green andy.gr...@linaro.org Signed-off-by: Mark Brown broo...@linaro.org What about doing this instead? Makes sense to me - Andy? signature.asc Description: Digital signature
Re: ar5523 Gigaset USB Adapter 108 issue
Just in case, i tested ar5523 based adapter on two different xhci controllers. No visible problems. Am 07.08.2013 05:39, schrieb Sarah Sharp: Please recompile your kernel with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on, and email dmesg from when you first plug in the device, to when it fails. Thanks, Sarah Sharp On Thu, Aug 01, 2013 at 07:41:27PM +0200, Yannik Völker wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, I was told to message you about this issue: I own an Gigaset USB Adapter 108 which uses the ar5523 chipset, support for that should have been added in 3.8, i am using uname -a Linux Hostname 3.10.3-1-ARCH #1 SMP PREEMPT Fri Jul 26 11:26:59 CEST 2013 x86_64 GNU/Linux right now. It is not the firmwares fault, its not even getting touched at all. The device is not broken either, i have tested it on another computer running 32 bit windows. Its not a broken USB port as i have tired several ones. When plugging it in the following lines appear in dmesg: [12482.680674] usb 8-6: USB disconnect, device number 7 [12486.112103] usb 8-6: new high-speed USB device number 8 using ehci-pci [12488.235438] usb 8-6: timeout waiting for command 01 reply [12488.235444] usb 8-6: could not initialize adapter [12488.235643] usb 8-6: RX USB error -2. [12488.235652] usb 8-6: error -1 when submitting rx urb [12488.235748] ar5523: probe of 8-6:1.0 failed with error -110 I was told that -110 means not enough power which seems unlikely as this is an Desktop computer, and I had it running on this exact computer using ndiswrapper and an 32 bit kernel. The specifications do not state extremly high Values either (Webarchive as the original website is gone) http://web.archive.org/web/20100808072407/http://gigaset.com/hq/en/product/GIGASETUSBADAPTER108.html?tab=data Power Consumption during operation: TX: 475mA, RX:290mA, Power Consumption during standby: 330mA So I thought that this might be a bug in the usb subsystem If there is any more information I can provide: just tell me. - -- -- Regards, Oleksij -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 11:08:45AM +0200, Sascha Hauer wrote: - Convert imx-usb-misc from a driver into something which is called directly - add aliases - change devicetree bindings (which causes pain and it's not explained why this is necessary at all) Yes, I need to split doc from implementation. No, you don't. You just have to explain *why* the bindings need to be chaged. OK, I will - converts the misc stuff to regmap Please split this up so that it can be reviewed. The changes are all related to convert usbmisc_imx from a driver to something which can be called directly. - usbmisc has #index-cells to indicate controller number, now, it is not a driver, we use aliases at controller driver to instead of it. Why? We need to know controller number, like pdev-id in the past. The registers at non core register is messy, the specific bit is for specific controller. - When usbmisc_imx is a device, the register maps only one time. But ci_hdrc_imx will be several devices, the non-core register will be mapped several times, we have to use syscon to visit one register region among several devices. Converting the imx-usb misc driver to regmap is fine, but I see no reason to not make this a separate patch. This would make this much easier to read. You mean delete reg entry at usbmisc dts, and using noncore phandle at usbmisc_imx.c as the first patch, then, convert driver as separate file. Yes, it is not a good practice, but I want to keep ci_hdrc_imx.c as clean as possible. The SoC related implementation will be more and more in the future after we add more USB functions. Just because you want to have the binary code in a single module doesn't mean the source code has to be in a single C file. OK, I will do. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 11:11:10AM +0200, Sascha Hauer wrote: On Wed, Aug 07, 2013 at 01:34:37PM +0800, Peter Chen wrote: - - reg = usbmisc-base + MX25_USB_PHY_CTRL_OFFSET; - - if (usbdev-evdo) { - spin_lock_irqsave(usbmisc-lock, flags); - val = readl(reg); - writel(val | MX25_BM_EXTERNAL_VBUS_DIVIDER, reg); - spin_unlock_irqrestore(usbmisc-lock, flags); + if (data-evdo) { + spin_lock_irqsave(data-lock, flags); + regmap_read(data-non_core_base_addr, + MX25_USB_PHY_CTRL_OFFSET, val); + regmap_write(data-non_core_base_addr, + MX25_USB_PHY_CTRL_OFFSET, + val | MX25_BM_EXTERNAL_VBUS_DIVIDER); + spin_unlock_irqrestore(data-lock, flags); Have a look at regmap_update_bits. You won't need this spinlock anymore. Thanks. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc3-pci: Ensure system sleep PM ops are defined only when used
On 7 August 2013 17:34, Mark Brown broo...@kernel.org wrote: On Tue, Aug 06, 2013 at 10:35:52PM -0300, Fabio Estevam wrote: On Tue, Aug 6, 2013 at 12:49 PM, Mark Brown broo...@kernel.org wrote: From: Andy Green andy.gr...@linaro.org You might have CONFIG_PM, but you might not have CONFIG_SUSPEND, in which case these are unused. Signed-off-by: Andy Green andy.gr...@linaro.org Signed-off-by: Mark Brown broo...@linaro.org What about doing this instead? Makes sense to me - Andy? Sure, it seems a much more complete solution. -Andy -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 03:54:06PM +0800, Peter Chen wrote: On Wed, Aug 07, 2013 at 11:08:45AM +0200, Sascha Hauer wrote: - Convert imx-usb-misc from a driver into something which is called directly - add aliases - change devicetree bindings (which causes pain and it's not explained why this is necessary at all) Yes, I need to split doc from implementation. No, you don't. You just have to explain *why* the bindings need to be chaged. OK, I will - converts the misc stuff to regmap Please split this up so that it can be reviewed. The changes are all related to convert usbmisc_imx from a driver to something which can be called directly. - usbmisc has #index-cells to indicate controller number, now, it is not a driver, we use aliases at controller driver to instead of it. Why? We need to know controller number, like pdev-id in the past. The registers at non core register is messy, the specific bit is for specific controller. The controller number is present in the old binding: fsl,usbmisc = usbmisc 1; ^^^ Merging the usbmisc stuff into the imx driver shouldn't be a reason to change this binding. Please get used to the fact that dt bindings cannot be changed easily just because the newer binding looks nicer. They define an ABI and breaking this ABI causes pain. - When usbmisc_imx is a device, the register maps only one time. But ci_hdrc_imx will be several devices, the non-core register will be mapped several times, we have to use syscon to visit one register region among several devices. Converting the imx-usb misc driver to regmap is fine, but I see no reason to not make this a separate patch. This would make this much easier to read. You mean delete reg entry at usbmisc dts, and using noncore phandle at usbmisc_imx.c as the first patch, then, convert driver as separate file. I mean: create a patch which converts the usbmisc driver to regmap and base your other stuff on this patch. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 01:34:37PM +0800, Peter Chen wrote: diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt index b4b5b79..13720a2 100644 --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt @@ -13,8 +13,8 @@ Recommended properies: Optional properties: - fsl,usbphy: phandler of usb phy that connects to the only one port -- fsl,usbmisc: phandler of non-core register device, with one argument - that indicate usb controller index +- ci,noncore: phandler of non-core register device, the user must enable +syscon driver to use it Don't add Linux specifics to the binding documentation. The bindings are supposed to be OS agnostic. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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: [alsa-devel] Linux 3.10.4 and Logitech HD Webcam C525/C310
Hi Alan, Thank you very much for quick response and the patch. The patch fixed the mic on both cameras. I have also added the following patch (see https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=36691e1be6ec551eef4a5225f126a281f8c051c2) for avoiding the kernel warning. --- linux-3.10.5-orig/sound/usb/mixer.c 2013-08-04 12:51:49.0 +0400 +++ linux-3.10.5/sound/usb/mixer.c 2013-08-06 21:12:42.274197287 +0400 @@ -888,6 +888,7 @@ case USB_ID(0x046d, 0x081b): /* HD Webcam c310 */ case USB_ID(0x046d, 0x081d): /* HD Webcam c510 */ case USB_ID(0x046d, 0x0825): /* HD Webcam c270 */ + case USB_ID(0x046d, 0x0826): /* HD Webcam c525 */ case USB_ID(0x046d, 0x0991): /* Most audio usb devices lie about volume resolution. * Most Logitech webcams have res = 384. -- Maksim Boyko -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 12:05:07PM +0200, Sascha Hauer wrote: We need to know controller number, like pdev-id in the past. The registers at non core register is messy, the specific bit is for specific controller. The controller number is present in the old binding: fsl,usbmisc = usbmisc 1; ^^^ Merging the usbmisc stuff into the imx driver shouldn't be a reason to change this binding. Please get used to the fact that dt bindings cannot be changed easily just because the newer binding looks nicer. They define an ABI and breaking this ABI causes pain. But current situation is the device node of usbmisc will not be needed. The usbmisc will not be a driver. We only need usbmisc to supply kinds of SoC specific implementations, as well as filling the content of the data at struct of_device_id (imx-usb's). - When usbmisc_imx is a device, the register maps only one time. But ci_hdrc_imx will be several devices, the non-core register will be mapped several times, we have to use syscon to visit one register region among several devices. Converting the imx-usb misc driver to regmap is fine, but I see no reason to not make this a separate patch. This would make this much easier to read. You mean delete reg entry at usbmisc dts, and using noncore phandle at usbmisc_imx.c as the first patch, then, convert driver as separate file. I mean: create a patch which converts the usbmisc driver to regmap and base your other stuff on this patch. This is what I mean. We can't request one region twice, so we need to delete devm_ioremap_resource at probe first. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 12:06:42PM +0200, Sascha Hauer wrote: On Wed, Aug 07, 2013 at 01:34:37PM +0800, Peter Chen wrote: diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt index b4b5b79..13720a2 100644 --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt @@ -13,8 +13,8 @@ Recommended properies: Optional properties: - fsl,usbphy: phandler of usb phy that connects to the only one port -- fsl,usbmisc: phandler of non-core register device, with one argument - that indicate usb controller index +- ci,noncore: phandler of non-core register device, the user must enable +syscon driver to use it Don't add Linux specifics to the binding documentation. The bindings are supposed to be OS agnostic. OK, I will change 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
[PATCH] dwc3: dwc3-pci: Use SIMPLE_DEV_PM_OPS
From: Fabio Estevam fabio.este...@freescale.com Using SIMPLE_DEV_PM_OPS can make the code simpler and cleaner. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- drivers/usb/dwc3/dwc3-pci.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c index 5d746e5..b922315 100644 --- a/drivers/usb/dwc3/dwc3-pci.c +++ b/drivers/usb/dwc3/dwc3-pci.c @@ -191,7 +191,7 @@ static DEFINE_PCI_DEVICE_TABLE(dwc3_pci_id_table) = { }; MODULE_DEVICE_TABLE(pci, dwc3_pci_id_table); -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP static int dwc3_pci_suspend(struct device *dev) { struct pci_dev *pci = to_pci_dev(dev); @@ -216,15 +216,10 @@ static int dwc3_pci_resume(struct device *dev) return 0; } +#endif /* CONFIG_PM_SLEEP */ -static const struct dev_pm_ops dwc3_pci_dev_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(dwc3_pci_suspend, dwc3_pci_resume) -}; - -#define DEV_PM_OPS (dwc3_pci_dev_pm_ops) -#else -#define DEV_PM_OPS NULL -#endif /* CONFIG_PM */ +static SIMPLE_DEV_PM_OPS(dwc3_pci_dev_pm_ops, dwc3_pci_suspend, +dwc3_pci_resume); static struct pci_driver dwc3_pci_driver = { .name = dwc3-pci, @@ -232,7 +227,7 @@ static struct pci_driver dwc3_pci_driver = { .probe = dwc3_pci_probe, .remove = dwc3_pci_remove, .driver = { - .pm = DEV_PM_OPS, + .pm = dwc3_pci_dev_pm_ops, }, }; -- 1.8.1.2 -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 05:24:45PM +0800, Peter Chen wrote: On Wed, Aug 07, 2013 at 12:05:07PM +0200, Sascha Hauer wrote: We need to know controller number, like pdev-id in the past. The registers at non core register is messy, the specific bit is for specific controller. The controller number is present in the old binding: fsl,usbmisc = usbmisc 1; ^^^ Merging the usbmisc stuff into the imx driver shouldn't be a reason to change this binding. Please get used to the fact that dt bindings cannot be changed easily just because the newer binding looks nicer. They define an ABI and breaking this ABI causes pain. But current situation is the device node of usbmisc will not be needed. If it's really not needed then you could remove it. The usbmisc will not be a driver. Not true. It's still a driver, just one that happens to be not compilable as module: syscon We only need usbmisc to supply kinds of SoC specific implementations, as well as filling the content of the data at struct of_device_id (imx-usb's). Yes, that's the purpose of usbmisc and ever has been. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
Hi Peter, On Wed, Aug 07, 2013 at 05:24:45PM +0800, Peter Chen wrote: On Wed, Aug 07, 2013 at 12:05:07PM +0200, Sascha Hauer wrote: We need to know controller number, like pdev-id in the past. The registers at non core register is messy, the specific bit is for specific controller. The controller number is present in the old binding: fsl,usbmisc = usbmisc 1; ^^^ Merging the usbmisc stuff into the imx driver shouldn't be a reason to change this binding. Please get used to the fact that dt bindings cannot be changed easily just because the newer binding looks nicer. They define an ABI and breaking this ABI causes pain. But current situation is the device node of usbmisc will not be needed. The usbmisc will not be a driver. We only need usbmisc to supply kinds of SoC specific implementations, as well as filling the content of the data at struct of_device_id (imx-usb's). - When usbmisc_imx is a device, the register maps only one time. But ci_hdrc_imx will be several devices, the non-core register will be mapped several times, we have to use syscon to visit one register region among several devices. Converting the imx-usb misc driver to regmap is fine, but I see no reason to not make this a separate patch. This would make this much easier to read. You mean delete reg entry at usbmisc dts, and using noncore phandle at usbmisc_imx.c as the first patch, then, convert driver as separate file. I mean: create a patch which converts the usbmisc driver to regmap and base your other stuff on this patch. This is what I mean. We can't request one region twice, so we need to delete devm_ioremap_resource at probe first. Please try to reply with your answers in direct context to the questions. Otherwise this thread will not end and probably produce more confusion than answers. What we need is a clearly seperated series of independent changes; Just like Sascha already mentioned: On Wed, Aug 07, 2013 at 10:15:22AM +0200, Sascha Hauer wrote: This patch does far too many things at once. - Convert imx-usb-misc from a driver into something which is called directly - add aliases - change devicetree bindings (which causes pain and it's not explained why this is necessary at all) - converts the misc stuff to regmap Please split this up so that it can be reviewed. Your work needs the clean seperation before we can discuss the changes more in detail. Please try that first! Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c
On Wed, Aug 07, 2013 at 02:28:20PM +0200, Michael Grzeschik wrote: Hi Peter, On Wed, Aug 07, 2013 at 05:24:45PM +0800, Peter Chen wrote: On Wed, Aug 07, 2013 at 12:05:07PM +0200, Sascha Hauer wrote: We need to know controller number, like pdev-id in the past. The registers at non core register is messy, the specific bit is for specific controller. The controller number is present in the old binding: fsl,usbmisc = usbmisc 1; ^^^ Merging the usbmisc stuff into the imx driver shouldn't be a reason to change this binding. Please get used to the fact that dt bindings cannot be changed easily just because the newer binding looks nicer. They define an ABI and breaking this ABI causes pain. But current situation is the device node of usbmisc will not be needed. The usbmisc will not be a driver. We only need usbmisc to supply kinds of SoC specific implementations, as well as filling the content of the data at struct of_device_id (imx-usb's). - When usbmisc_imx is a device, the register maps only one time. But ci_hdrc_imx will be several devices, the non-core register will be mapped several times, we have to use syscon to visit one register region among several devices. Converting the imx-usb misc driver to regmap is fine, but I see no reason to not make this a separate patch. This would make this much easier to read. You mean delete reg entry at usbmisc dts, and using noncore phandle at usbmisc_imx.c as the first patch, then, convert driver as separate file. I mean: create a patch which converts the usbmisc driver to regmap and base your other stuff on this patch. This is what I mean. We can't request one region twice, so we need to delete devm_ioremap_resource at probe first. Please try to reply with your answers in direct context to the questions. Otherwise this thread will not end and probably produce more confusion than answers. What we need is a clearly seperated series of independent changes; Just like Sascha already mentioned: On Wed, Aug 07, 2013 at 10:15:22AM +0200, Sascha Hauer wrote: This patch does far too many things at once. - Convert imx-usb-misc from a driver into something which is called directly - add aliases - change devicetree bindings (which causes pain and it's not explained why this is necessary at all) - converts the misc stuff to regmap Please split this up so that it can be reviewed. Your work needs the clean seperation before we can discuss the changes more in detail. Please try that first! Thanks, Michael. I will do that. But discussion is needed, it can let us understand the problem deeply. For you, what the real problem, for me, how to do it properly. -- 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: [alsa-devel] [PATCH] 6fire: fix DMA issues with URB transfer_buffer usage
At Tue, 06 Aug 2013 14:53:24 +0300, Jussi Kivilinna wrote: Patch fixes 6fire not to use stack as URB transfer_buffer. URB buffers need to be DMA-able, which stack is not. Furthermore, transfer_buffer should not be allocated as part of larger device structure because DMA coherency issues and patch fixes this issue too. Patch is only compile tested. The changes look OK, but I'd like to let it checked with a real hardware before putting to stable kernel. Torsten, could you check this? thanks, Takashi Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- sound/usb/6fire/comm.c | 38 +- sound/usb/6fire/comm.h |2 +- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c index 9e6e3ff..23452ee 100644 --- a/sound/usb/6fire/comm.c +++ b/sound/usb/6fire/comm.c @@ -110,19 +110,37 @@ static int usb6fire_comm_send_buffer(u8 *buffer, struct usb_device *dev) static int usb6fire_comm_write8(struct comm_runtime *rt, u8 request, u8 reg, u8 value) { - u8 buffer[13]; /* 13: maximum length of message */ + u8 *buffer; + int ret; + + /* 13: maximum length of message */ + buffer = kmalloc(13, GFP_KERNEL); + if (!buffer) + return -ENOMEM; usb6fire_comm_init_buffer(buffer, 0x00, request, reg, value, 0x00); - return usb6fire_comm_send_buffer(buffer, rt-chip-dev); + ret = usb6fire_comm_send_buffer(buffer, rt-chip-dev); + + kfree(buffer); + return ret; } static int usb6fire_comm_write16(struct comm_runtime *rt, u8 request, u8 reg, u8 vl, u8 vh) { - u8 buffer[13]; /* 13: maximum length of message */ + u8 *buffer; + int ret; + + /* 13: maximum length of message */ + buffer = kmalloc(13, GFP_KERNEL); + if (!buffer) + return -ENOMEM; usb6fire_comm_init_buffer(buffer, 0x00, request, reg, vl, vh); - return usb6fire_comm_send_buffer(buffer, rt-chip-dev); + ret = usb6fire_comm_send_buffer(buffer, rt-chip-dev); + + kfree(buffer); + return ret; } int usb6fire_comm_init(struct sfire_chip *chip) @@ -135,6 +153,12 @@ int usb6fire_comm_init(struct sfire_chip *chip) if (!rt) return -ENOMEM; + rt-receiver_buffer = kzalloc(COMM_RECEIVER_BUFSIZE, GFP_KERNEL); + if (!rt-receiver_buffer) { + kfree(rt); + return -ENOMEM; + } + urb = rt-receiver; rt-serial = 1; rt-chip = chip; @@ -153,6 +177,7 @@ int usb6fire_comm_init(struct sfire_chip *chip) urb-interval = 1; ret = usb_submit_urb(urb, GFP_KERNEL); if (ret 0) { + kfree(rt-receiver_buffer); kfree(rt); snd_printk(KERN_ERR PREFIX cannot create comm data receiver.); return ret; @@ -171,6 +196,9 @@ void usb6fire_comm_abort(struct sfire_chip *chip) void usb6fire_comm_destroy(struct sfire_chip *chip) { - kfree(chip-comm); + struct comm_runtime *rt = chip-comm; + + kfree(rt-receiver_buffer); + kfree(rt); chip-comm = NULL; } diff --git a/sound/usb/6fire/comm.h b/sound/usb/6fire/comm.h index 6a0840b..780d5ed 100644 --- a/sound/usb/6fire/comm.h +++ b/sound/usb/6fire/comm.h @@ -24,7 +24,7 @@ struct comm_runtime { struct sfire_chip *chip; struct urb receiver; - u8 receiver_buffer[COMM_RECEIVER_BUFSIZE]; + u8 *receiver_buffer; u8 serial; /* urb serial */ ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- 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: phy: Cleanup error code in **_usb_get_phy_**() APIs
**_usb_get_phy_**() APIs should generate equivalent error codes in case of failure in getting phy. There's no need of returning NULL pointer, like in devm_usb_get_phy() or devm_usb_get_phy_dev(), since it's nowhere handled. Earlier series of patches starting: 9ee1c7f usb: host: ohci-exynos: fix PHY error handling, fixed PHY error handling. Also adding error handling in usb_phy_**() APIs. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/phy/phy.c | 18 ++ include/linux/usb/phy.h | 42 ++ 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index a9984c7..86f9905 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -25,7 +25,7 @@ static DEFINE_SPINLOCK(phy_lock); static struct usb_phy *__usb_find_phy(struct list_head *list, enum usb_phy_type type) { - struct usb_phy *phy = NULL; + struct usb_phy *phy; list_for_each_entry(phy, list, head) { if (phy-type != type) @@ -40,7 +40,7 @@ static struct usb_phy *__usb_find_phy(struct list_head *list, static struct usb_phy *__usb_find_phy_dev(struct device *dev, struct list_head *list, u8 index) { - struct usb_phy_bind *phy_bind = NULL; + struct usb_phy_bind *phy_bind; list_for_each_entry(phy_bind, list, list) { if (!(strcmp(phy_bind-dev_name, dev_name(dev))) @@ -94,11 +94,11 @@ static int devm_usb_phy_match(struct device *dev, void *res, void *match_data) */ struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type) { - struct usb_phy **ptr, *phy; + struct usb_phy *phy = ERR_PTR(-ENOMEM), **ptr; ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL); if (!ptr) - return NULL; + goto err0; phy = usb_get_phy(type); if (!IS_ERR(phy)) { @@ -107,6 +107,7 @@ struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type) } else devres_free(ptr); +err0: return phy; } EXPORT_SYMBOL_GPL(devm_usb_get_phy); @@ -123,7 +124,7 @@ EXPORT_SYMBOL_GPL(devm_usb_get_phy); */ struct usb_phy *usb_get_phy(enum usb_phy_type type) { - struct usb_phy *phy = NULL; + struct usb_phy *phy; unsigned long flags; spin_lock_irqsave(phy_lock, flags); @@ -221,7 +222,7 @@ EXPORT_SYMBOL_GPL(devm_usb_get_phy_by_phandle); */ struct usb_phy *usb_get_phy_dev(struct device *dev, u8 index) { - struct usb_phy *phy = NULL; + struct usb_phy *phy; unsigned long flags; spin_lock_irqsave(phy_lock, flags); @@ -254,11 +255,11 @@ EXPORT_SYMBOL_GPL(usb_get_phy_dev); */ struct usb_phy *devm_usb_get_phy_dev(struct device *dev, u8 index) { - struct usb_phy **ptr, *phy; + struct usb_phy *phy = ERR_PTR(-ENOMEM), **ptr; ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL); if (!ptr) - return NULL; + goto err0; phy = usb_get_phy_dev(dev, index); if (!IS_ERR(phy)) { @@ -267,6 +268,7 @@ struct usb_phy *devm_usb_get_phy_dev(struct device *dev, u8 index) } else devres_free(ptr); +err0: return phy; } EXPORT_SYMBOL_GPL(devm_usb_get_phy_dev); diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h index 4403680..9aa6aae 100644 --- a/include/linux/usb/phy.h +++ b/include/linux/usb/phy.h @@ -142,7 +142,7 @@ extern void usb_remove_phy(struct usb_phy *); /* helpers for direct access thru low-level io interface */ static inline int usb_phy_io_read(struct usb_phy *x, u32 reg) { - if (x-io_ops x-io_ops-read) + if (!IS_ERR(x) x-io_ops x-io_ops-read) return x-io_ops-read(x, reg); return -EINVAL; @@ -150,7 +150,7 @@ static inline int usb_phy_io_read(struct usb_phy *x, u32 reg) static inline int usb_phy_io_write(struct usb_phy *x, u32 val, u32 reg) { - if (x-io_ops x-io_ops-write) + if (!IS_ERR(x) x-io_ops x-io_ops-write) return x-io_ops-write(x, val, reg); return -EINVAL; @@ -159,7 +159,7 @@ static inline int usb_phy_io_write(struct usb_phy *x, u32 val, u32 reg) static inline int usb_phy_init(struct usb_phy *x) { - if (x-init) + if (!IS_ERR(x) x-init) return x-init(x); return 0; @@ -168,26 +168,27 @@ usb_phy_init(struct usb_phy *x) static inline void usb_phy_shutdown(struct usb_phy *x) { - if (x-shutdown) + if (!IS_ERR(x) x-shutdown) x-shutdown(x); } static inline int usb_phy_vbus_on(struct usb_phy *x) { - if (!x-set_vbus) - return 0; + if (!IS_ERR(x) x-set_vbus) + return x-set_vbus(x, true); - return x-set_vbus(x, true); + return 0; } static inline int usb_phy_vbus_off(struct usb_phy *x) { -
[PATCH 1/2] net/usb: catc: allocate URB setup_packet as separate buffer
URB setup packet must not be allocated as part of larger structure because DMA coherence issues. Patch changes catc to allocate ctrl_dr member as separate buffer. Patch is only compile tested. Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- drivers/net/usb/catc.c | 40 +--- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/drivers/net/usb/catc.c b/drivers/net/usb/catc.c index 8d5cac2..a7d3c1b 100644 --- a/drivers/net/usb/catc.c +++ b/drivers/net/usb/catc.c @@ -173,7 +173,7 @@ struct catc { u8 rx_buf[RX_MAX_BURST * (PKT_SZ + 2)]; u8 irq_buf[2]; u8 ctrl_buf[64]; - struct usb_ctrlrequest ctrl_dr; + struct usb_ctrlrequest *ctrl_dr; struct timer_list timer; u8 stats_buf[8]; @@ -485,7 +485,7 @@ static void catc_ctrl_run(struct catc *catc) struct ctrl_queue *q = catc-ctrl_queue + catc-ctrl_tail; struct usb_device *usbdev = catc-usbdev; struct urb *urb = catc-ctrl_urb; - struct usb_ctrlrequest *dr = catc-ctrl_dr; + struct usb_ctrlrequest *dr = catc-ctrl_dr; int status; dr-bRequest = q-request; @@ -779,7 +779,7 @@ static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id struct net_device *netdev; struct catc *catc; u8 broadcast[6]; - int i, pktsz; + int i, pktsz, err; if (usb_set_interface(usbdev, intf-altsetting-desc.bInterfaceNumber, 1)) { @@ -793,6 +793,12 @@ static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id catc = netdev_priv(netdev); + catc-ctrl_dr = kzalloc(sizeof(*catc-ctrl_dr), GFP_KERNEL); + if (!catc-ctrl_dr) { + err = -ENOMEM; + goto err_free_dev; + } + netdev-netdev_ops = catc_netdev_ops; netdev-watchdog_timeo = TX_TIMEOUT; SET_ETHTOOL_OPS(netdev, ops); @@ -814,12 +820,8 @@ static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id if ((!catc-ctrl_urb) || (!catc-tx_urb) || (!catc-rx_urb) || (!catc-irq_urb)) { dev_err(intf-dev, No free urbs available.\n); - usb_free_urb(catc-ctrl_urb); - usb_free_urb(catc-tx_urb); - usb_free_urb(catc-rx_urb); - usb_free_urb(catc-irq_urb); - free_netdev(netdev); - return -ENOMEM; + err = -ENOMEM; + goto err_free; } /* The F5U011 has the same vendor/product as the netmate but a device version of 0x130 */ @@ -918,14 +920,21 @@ static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id SET_NETDEV_DEV(netdev, intf-dev); if (register_netdev(netdev) != 0) { usb_set_intfdata(intf, NULL); - usb_free_urb(catc-ctrl_urb); - usb_free_urb(catc-tx_urb); - usb_free_urb(catc-rx_urb); - usb_free_urb(catc-irq_urb); - free_netdev(netdev); - return -EIO; + err = -EIO; + goto err_free; } return 0; + +err_free: + usb_free_urb(catc-ctrl_urb); + usb_free_urb(catc-tx_urb); + usb_free_urb(catc-rx_urb); + usb_free_urb(catc-irq_urb); + kfree(catc-ctrl_dr); +err_free_dev: + free_netdev(netdev); + + return err; } static void catc_disconnect(struct usb_interface *intf) @@ -939,6 +948,7 @@ static void catc_disconnect(struct usb_interface *intf) usb_free_urb(catc-tx_urb); usb_free_urb(catc-rx_urb); usb_free_urb(catc-irq_urb); + kfree(catc-ctrl_dr); free_netdev(catc-netdev); } } -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] net/usb: catc: allocate URB transfer_buffers as separate buffers
URB transfer_buffer must not be allocated as part of larger structure because DMA coherence issues. Patch changes catc to allocate tx_buf, rx_buf, irq_buf and ctrl_buf members as separate buffers. Patch is only compile tested. Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- drivers/net/usb/catc.c | 30 +++--- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/drivers/net/usb/catc.c b/drivers/net/usb/catc.c index a7d3c1b..26039d6 100644 --- a/drivers/net/usb/catc.c +++ b/drivers/net/usb/catc.c @@ -169,10 +169,10 @@ struct catc { unsigned int ctrl_head, ctrl_tail; spinlock_t tx_lock, ctrl_lock; - u8 tx_buf[2][TX_MAX_BURST * (PKT_SZ + 2)]; - u8 rx_buf[RX_MAX_BURST * (PKT_SZ + 2)]; - u8 irq_buf[2]; - u8 ctrl_buf[64]; + u8 *tx_buf[2]; + u8 *rx_buf; + u8 *irq_buf; + u8 *ctrl_buf; struct usb_ctrlrequest *ctrl_dr; struct timer_list timer; @@ -794,9 +794,15 @@ static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id catc = netdev_priv(netdev); catc-ctrl_dr = kzalloc(sizeof(*catc-ctrl_dr), GFP_KERNEL); - if (!catc-ctrl_dr) { + catc-tx_buf[0] = kzalloc(TX_MAX_BURST * (PKT_SZ + 2), GFP_KERNEL); + catc-tx_buf[1] = kzalloc(TX_MAX_BURST * (PKT_SZ + 2), GFP_KERNEL); + catc-rx_buf = kzalloc(RX_MAX_BURST * (PKT_SZ + 2), GFP_KERNEL); + catc-irq_buf = kzalloc(2, GFP_KERNEL); + catc-ctrl_buf = kzalloc(64, GFP_KERNEL); + if (!catc-ctrl_dr || !catc-ctrl_buf || !catc-tx_buf[0] || + !catc-tx_buf[1] || !catc-rx_buf || !catc-irq_buf) { err = -ENOMEM; - goto err_free_dev; + goto err_free_buf; } netdev-netdev_ops = catc_netdev_ops; @@ -930,8 +936,13 @@ err_free: usb_free_urb(catc-tx_urb); usb_free_urb(catc-rx_urb); usb_free_urb(catc-irq_urb); +err_free_buf: + kfree(catc-ctrl_buf); + kfree(catc-irq_buf); + kfree(catc-rx_buf); + kfree(catc-tx_buf[1]); + kfree(catc-tx_buf[0]); kfree(catc-ctrl_dr); -err_free_dev: free_netdev(netdev); return err; @@ -948,6 +959,11 @@ static void catc_disconnect(struct usb_interface *intf) usb_free_urb(catc-tx_urb); usb_free_urb(catc-rx_urb); usb_free_urb(catc-irq_urb); + kfree(catc-ctrl_buf); + kfree(catc-irq_buf); + kfree(catc-rx_buf); + kfree(catc-tx_buf[1]); + kfree(catc-tx_buf[0]); kfree(catc-ctrl_dr); free_netdev(catc-netdev); } -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] net/usb: hso: allocate serial_state_notification as separate buffer
URB transfer_buffers must not be allocated as part of larger structure because DMA coherence issues. Patch changes hso to allocate serial_state_notification member of 'struct hso_tiocmget' as separate buffer. Patch is only compile tested. Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- drivers/net/usb/hso.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index fc082c0..21ffa6f 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -211,7 +211,7 @@ struct hso_tiocmget { intintr_completed; struct usb_endpoint_descriptor *endp; struct urb *urb; - struct hso_serial_state_notification serial_state_notification; + struct hso_serial_state_notification *serial_state_notification; u16prev_UART_state_bitmap; struct uart_icount icount; }; @@ -1465,7 +1465,7 @@ static int tiocmget_submit_urb(struct hso_serial *serial, usb_rcvintpipe(usb, tiocmget-endp- bEndpointAddress 0x7F), -tiocmget-serial_state_notification, +tiocmget-serial_state_notification, sizeof(struct hso_serial_state_notification), tiocmget_intr_callback, serial, tiocmget-endp-bInterval); @@ -1499,7 +1499,7 @@ static void tiocmget_intr_callback(struct urb *urb) if (!tiocmget) return; usb = serial-parent-usb; - serial_state_notification = tiocmget-serial_state_notification; + serial_state_notification = tiocmget-serial_state_notification; if (serial_state_notification-bmRequestType != BM_REQUEST_TYPE || serial_state_notification-bNotification != B_NOTIFICATION || le16_to_cpu(serial_state_notification-wValue) != W_VALUE || @@ -2608,6 +2608,7 @@ static void hso_free_tiomget(struct hso_serial *serial) usb_free_urb(tiocmget-urb); tiocmget-urb = NULL; serial-tiocmget = NULL; + kfree(tiocmget-serial_state_notification); kfree(tiocmget); } } @@ -2664,6 +2665,13 @@ static struct hso_device *hso_create_bulk_serial_device( */ if (serial-tiocmget) { tiocmget = serial-tiocmget; + + tiocmget-serial_state_notification = kzalloc( + sizeof(struct hso_serial_state_notification), + GFP_KERNEL); + if (!tiocmget-serial_state_notification) + goto tiocmget_fail; + tiocmget-urb = usb_alloc_urb(0, GFP_KERNEL); if (tiocmget-urb) { mutex_init(tiocmget-mutex); @@ -2672,8 +2680,10 @@ static struct hso_device *hso_create_bulk_serial_device( interface, USB_ENDPOINT_XFER_INT, USB_DIR_IN); - } else + } else { +tiocmget_fail: hso_free_tiomget(serial); + } } } else -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] net/usb: hso: allocate URB setup_packets as separate buffers
URB setup packets must not be allocated as part of larger structure because DMA coherence issues. Patch changes hso to allocate ctrl_req_[tr]x members of 'struct hso_serial' as separate buffers. Patch is only compile tested. Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- drivers/net/usb/hso.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index cba1d46..fc082c0 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -237,8 +237,8 @@ struct hso_serial { u16 tx_data_length; /* should contain allocated length */ u16 tx_data_count; u16 tx_buffer_count; - struct usb_ctrlrequest ctrl_req_tx; - struct usb_ctrlrequest ctrl_req_rx; + struct usb_ctrlrequest *ctrl_req_tx; + struct usb_ctrlrequest *ctrl_req_rx; struct usb_endpoint_descriptor *in_endp; struct usb_endpoint_descriptor *out_endp; @@ -1847,7 +1847,7 @@ static int hso_mux_serial_read(struct hso_serial *serial) USB_CDC_GET_ENCAPSULATED_RESPONSE, serial-parent-port_spec HSO_PORT_MASK, serial-rx_urb[0], - serial-ctrl_req_rx, + serial-ctrl_req_rx, serial-rx_data[0], serial-rx_data_length); } @@ -1916,7 +1916,7 @@ static int hso_mux_serial_write_data(struct hso_serial *serial) USB_CDC_SEND_ENCAPSULATED_COMMAND, serial-parent-port_spec HSO_PORT_MASK, serial-tx_urb, - serial-ctrl_req_tx, + serial-ctrl_req_tx, serial-tx_data, serial-tx_data_count); } @@ -2264,6 +2264,10 @@ static void hso_serial_common_free(struct hso_serial *serial) usb_free_urb(serial-tx_urb); kfree(serial-tx_data); tty_port_destroy(serial-port); + + /* free control URB setup packet buffers */ + kfree(serial-ctrl_req_tx); + kfree(serial-ctrl_req_rx); } static int hso_serial_common_create(struct hso_serial *serial, int num_urbs, @@ -2292,6 +2296,12 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs, spin_lock_init(serial-serial_lock); serial-num_rx_urbs = num_urbs; + /* allocate control URB setup packet buffers */ + serial-ctrl_req_tx = kzalloc(sizeof(*serial-ctrl_req_tx), GFP_KERNEL); + serial-ctrl_req_rx = kzalloc(sizeof(*serial-ctrl_req_rx), GFP_KERNEL); + if (!serial-ctrl_req_tx || !serial-ctrl_req_rx) + goto exit; + /* RX, allocate urb and initialize */ /* prepare our RX buffer */ -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] net/usb: pegasus: allocate URB transfer_buffers as separate buffers
URB transfer_buffers must not be allocated as part of larger structure because DMA coherence issues. Patch changes pegasus to allocate intr_buff and tx_buff members of 'struct pegasus' as separate buffers. Patch is only compile tested. Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- drivers/net/usb/pegasus.c | 11 ++- drivers/net/usb/pegasus.h |6 -- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c index 3bce862..b734702 100644 --- a/drivers/net/usb/pegasus.c +++ b/drivers/net/usb/pegasus.c @@ -854,7 +854,7 @@ static int pegasus_open(struct net_device *net) usb_fill_int_urb(pegasus-intr_urb, pegasus-usb, usb_rcvintpipe(pegasus-usb, 3), -pegasus-intr_buff, sizeof(pegasus-intr_buff), +pegasus-intr_buff, PEGASUS_INTR_BUFLEN, intr_callback, pegasus, pegasus-intr_interval); if ((res = usb_submit_urb(pegasus-intr_urb, GFP_KERNEL))) { if (res == -ENODEV) @@ -1162,6 +1162,11 @@ static int pegasus_probe(struct usb_interface *intf, pegasus = netdev_priv(net); pegasus-dev_index = dev_index; + pegasus-intr_buff = kzalloc(PEGASUS_INTR_BUFLEN, GFP_KERNEL); + pegasus-tx_buff = kzalloc(PEGASUS_MTU, GFP_KERNEL); + if (!pegasus-intr_buff || !pegasus-tx_buff) + goto out1; + res = alloc_urbs(pegasus); if (res 0) { dev_err(intf-dev, can't allocate %s\n, urbs); @@ -1223,6 +1228,8 @@ out3: out2: free_all_urbs(pegasus); out1: + kfree(pegasus-tx_buff); + kfree(pegasus-intr_buff); free_netdev(net); out: pegasus_dec_workqueue(); @@ -1248,6 +1255,8 @@ static void pegasus_disconnect(struct usb_interface *intf) dev_kfree_skb(pegasus-rx_skb); pegasus-rx_skb = NULL; } + kfree(pegasus-tx_buff); + kfree(pegasus-intr_buff); free_netdev(pegasus-net); pegasus_dec_workqueue(); } diff --git a/drivers/net/usb/pegasus.h b/drivers/net/usb/pegasus.h index d156462..6469aed 100644 --- a/drivers/net/usb/pegasus.h +++ b/drivers/net/usb/pegasus.h @@ -55,6 +55,8 @@ #definePEGASUS_REQ_SET_REGS0xf1 #definePEGASUS_REQ_SET_REG PEGASUS_REQ_SET_REGS +#define PEGASUS_INTR_BUFLEN8 + enum pegasus_registers { EthCtrl0 = 0, EthCtrl1 = 1, @@ -96,8 +98,8 @@ typedef struct pegasus { struct urb *rx_urb, *tx_urb, *intr_urb; struct sk_buff *rx_skb; int chip; - unsigned char intr_buff[8]; - __u8tx_buff[PEGASUS_MTU]; + unsigned char *intr_buff; + __u8*tx_buff; __u8eth_regs[4]; __u8phy; __u8gpio_res; -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] net/usb: pegasus: do not use stack for URB buffers
Currently pegasus passes stack memory (which must not be used for DMA transfers) as URB buffers with get_registers(), set_registers() and set_register(). Apply fix inside these functions instead of changing every call site. Stack usage was introduced by recent commit 323b34963d11 drivers: net: usb: pegasus: fix control urb submission. Patch is only compile tested. Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- drivers/net/usb/pegasus.c | 26 +++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c index 03e8a15..3bce862 100644 --- a/drivers/net/usb/pegasus.c +++ b/drivers/net/usb/pegasus.c @@ -127,39 +127,59 @@ static void async_ctrl_callback(struct urb *urb) static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data) { int ret; + void *buf; + + buf = kmalloc(size, GFP_KERNEL); + if (!buf) + return -ENOMEM; ret = usb_control_msg(pegasus-usb, usb_rcvctrlpipe(pegasus-usb, 0), PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0, - indx, data, size, 1000); + indx, buf, size, 1000); if (ret 0) netif_dbg(pegasus, drv, pegasus-net, %s returned %d\n, __func__, ret); + else + memcpy(data, buf, ret); /* ret == bytes read (max == size) */ + kfree(buf); return ret; } static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data) { int ret; + void *buf; + + buf = kmemdup(data, size, GFP_KERNEL); + if (!buf) + return -ENOMEM; ret = usb_control_msg(pegasus-usb, usb_sndctrlpipe(pegasus-usb, 0), PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0, - indx, data, size, 100); + indx, buf, size, 100); if (ret 0) netif_dbg(pegasus, drv, pegasus-net, %s returned %d\n, __func__, ret); + kfree(buf); return ret; } static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data) { int ret; + void *buf; + + buf = kmemdup(data, 1, GFP_KERNEL); + if (!buf) + return -ENOMEM; ret = usb_control_msg(pegasus-usb, usb_sndctrlpipe(pegasus-usb, 0), PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data, - indx, data, 1, 1000); + indx, buf, 1, 1000); if (ret 0) netif_dbg(pegasus, drv, pegasus-net, %s returned %d\n, __func__, ret); + kfree(buf); return ret; } -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net/usb: rtl8150: allocate URB transfer_buffer and setup_packet separately
rtl8150 allocates URB transfer_buffer and setup_packet as part of same structure 'struct async_req'. This can cause same cacheline to be DMA-mapped twice with same URB. This can lead to memory corruption on some systems. Patch make allocation of these buffers separate. Use of 'struct async_req' was introduced by recent commit 4d12997a9b drivers: net: usb: rtl8150: concurrent URB bugfix. Patch is only compile tested. Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- drivers/net/usb/rtl8150.c | 48 +++-- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c index 6cbdac6..c353bfd 100644 --- a/drivers/net/usb/rtl8150.c +++ b/drivers/net/usb/rtl8150.c @@ -142,11 +142,6 @@ struct rtl8150 { typedef struct rtl8150 rtl8150_t; -struct async_req { - struct usb_ctrlrequest dr; - u16 rx_creg; -}; - static const char driver_name [] = rtl8150; /* @@ -170,12 +165,12 @@ static int set_registers(rtl8150_t * dev, u16 indx, u16 size, void *data) static void async_set_reg_cb(struct urb *urb) { - struct async_req *req = (struct async_req *)urb-context; int status = urb-status; if (status 0) dev_dbg(urb-dev-dev, %s failed with %d, __func__, status); - kfree(req); + kfree(urb-setup_packet); /* dr */ + kfree(urb-transfer_buffer); /* rx_creg */ usb_free_urb(urb); } @@ -183,25 +178,27 @@ static int async_set_registers(rtl8150_t *dev, u16 indx, u16 size, u16 reg) { int res = -ENOMEM; struct urb *async_urb; - struct async_req *req; + struct usb_ctrlrequest *dr; + u16 *rx_creg; + + dr = kmalloc(sizeof(*dr), GFP_ATOMIC); + rx_creg = kmalloc(sizeof(*rx_creg), GFP_ATOMIC); + if (!dr || !rx_creg) + goto err; - req = kmalloc(sizeof(struct async_req), GFP_ATOMIC); - if (req == NULL) - return res; async_urb = usb_alloc_urb(0, GFP_ATOMIC); - if (async_urb == NULL) { - kfree(req); - return res; - } - req-rx_creg = cpu_to_le16(reg); - req-dr.bRequestType = RTL8150_REQT_WRITE; - req-dr.bRequest = RTL8150_REQ_SET_REGS; - req-dr.wIndex = 0; - req-dr.wValue = cpu_to_le16(indx); - req-dr.wLength = cpu_to_le16(size); + if (async_urb == NULL) + goto err; + + *rx_creg = cpu_to_le16(reg); + dr-bRequestType = RTL8150_REQT_WRITE; + dr-bRequest = RTL8150_REQ_SET_REGS; + dr-wIndex = 0; + dr-wValue = cpu_to_le16(indx); + dr-wLength = cpu_to_le16(size); usb_fill_control_urb(async_urb, dev-udev, -usb_sndctrlpipe(dev-udev, 0), (void *)req-dr, -req-rx_creg, size, async_set_reg_cb, req); +usb_sndctrlpipe(dev-udev, 0), (void *)dr, +rx_creg, size, async_set_reg_cb, NULL); res = usb_submit_urb(async_urb, GFP_ATOMIC); if (res) { if (res == -ENODEV) @@ -209,6 +206,11 @@ static int async_set_registers(rtl8150_t *dev, u16 indx, u16 size, u16 reg) dev_err(dev-udev-dev, %s failed with %d\n, __func__, res); } return res; + +err: + kfree(dr); + kfree(rx_creg); + return res; } static int read_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 * reg) -- 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: [alsa-devel] [PATCH] 6fire: fix DMA issues with URB transfer_buffer usage
At Wed, 7 Aug 2013 15:59:07 +0200, Torsten Schenk wrote: On Wed, 07 Aug 2013 14:43:43 +0200 Takashi Iwai ti...@suse.de wrote: At Tue, 06 Aug 2013 14:53:24 +0300, Jussi Kivilinna wrote: Patch fixes 6fire not to use stack as URB transfer_buffer. URB buffers need to be DMA-able, which stack is not. Furthermore, transfer_buffer should not be allocated as part of larger device structure because DMA coherency issues and patch fixes this issue too. Thanks for the information. There is another section where this applies in midi.c/midi.h. I can post a patch later. Yes, please. Patch is only compile tested. The changes look OK, but I'd like to let it checked with a real hardware before putting to stable kernel. Torsten, could you check this? The patch works nicely. OK, I queued the patch to sound git tree now. I'm just wondering if instead of calling kmalloc() every time it'd be better to put a global sender_buffer and a mutex into the comm_runtime struct and lock it every time a writeX is performed. This also would have the advantage that message blocks would never overlap which currently is possible. I can prepare a patch on top of this one and send it later, if you agree. Yes, it'd be much better. thanks, Takashi Thanks, Torsten thanks, Takashi Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- sound/usb/6fire/comm.c | 38 +- sound/usb/6fire/comm.h |2 +- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c index 9e6e3ff..23452ee 100644 --- a/sound/usb/6fire/comm.c +++ b/sound/usb/6fire/comm.c @@ -110,19 +110,37 @@ static int usb6fire_comm_send_buffer(u8 *buffer, struct usb_device *dev) static int usb6fire_comm_write8 (struct comm_runtime *rt, u8 request, u8 reg, u8 value) { - u8 buffer[13]; /* 13: maximum length of message */ + u8 *buffer; + int ret; + + /* 13: maximum length of message */ + buffer = kmalloc(13, GFP_KERNEL); + if (!buffer) + return -ENOMEM; usb6fire_comm_init_buffer(buffer, 0x00, request, reg, value, 0x00); - return usb6fire_comm_send_buffer(buffer, rt-chip-dev); + ret = usb6fire_comm_send_buffer(buffer, rt-chip-dev); + + kfree(buffer); + return ret; } static int usb6fire_comm_write16(struct comm_runtime *rt, u8 request, u8 reg, u8 vl, u8 vh) { - u8 buffer[13]; /* 13: maximum length of message */ + u8 *buffer; + int ret; + + /* 13: maximum length of message */ + buffer = kmalloc(13, GFP_KERNEL); + if (!buffer) + return -ENOMEM; usb6fire_comm_init_buffer(buffer, 0x00, request, reg, vl, vh); - return usb6fire_comm_send_buffer(buffer, rt-chip-dev); + ret = usb6fire_comm_send_buffer(buffer, rt-chip-dev); + + kfree(buffer); + return ret; } int usb6fire_comm_init(struct sfire_chip *chip) @@ -135,6 +153,12 @@ int usb6fire_comm_init(struct sfire_chip *chip) if (!rt) return -ENOMEM; + rt-receiver_buffer = kzalloc(COMM_RECEIVER_BUFSIZE, GFP_KERNEL); + if (!rt-receiver_buffer) { + kfree(rt); + return -ENOMEM; + } + urb = rt-receiver; rt-serial = 1; rt-chip = chip; @@ -153,6 +177,7 @@ int usb6fire_comm_init(struct sfire_chip *chip) urb-interval = 1; ret = usb_submit_urb(urb, GFP_KERNEL); if (ret 0) { + kfree(rt-receiver_buffer); kfree(rt); snd_printk(KERN_ERR PREFIX cannot create comm data receiver.); return ret; @@ -171,6 +196,9 @@ void usb6fire_comm_abort(struct sfire_chip *chip) void usb6fire_comm_destroy(struct sfire_chip *chip) { - kfree(chip-comm); + struct comm_runtime *rt = chip-comm; + + kfree(rt-receiver_buffer); + kfree(rt); chip-comm = NULL; } diff --git a/sound/usb/6fire/comm.h b/sound/usb/6fire/comm.h index 6a0840b..780d5ed 100644 --- a/sound/usb/6fire/comm.h +++ b/sound/usb/6fire/comm.h @@ -24,7 +24,7 @@ struct comm_runtime { struct sfire_chip *chip; struct urb receiver; - u8 receiver_buffer[COMM_RECEIVER_BUFSIZE]; + u8 *receiver_buffer; u8 serial; /* urb serial */ ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- 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: [alsa-devel] [PATCH] 6fire: fix DMA issues with URB transfer_buffer usage
On Wed, 07 Aug 2013 14:43:43 +0200 Takashi Iwai ti...@suse.de wrote: At Tue, 06 Aug 2013 14:53:24 +0300, Jussi Kivilinna wrote: Patch fixes 6fire not to use stack as URB transfer_buffer. URB buffers need to be DMA-able, which stack is not. Furthermore, transfer_buffer should not be allocated as part of larger device structure because DMA coherency issues and patch fixes this issue too. Thanks for the information. There is another section where this applies in midi.c/midi.h. I can post a patch later. Patch is only compile tested. The changes look OK, but I'd like to let it checked with a real hardware before putting to stable kernel. Torsten, could you check this? The patch works nicely. I'm just wondering if instead of calling kmalloc() every time it'd be better to put a global sender_buffer and a mutex into the comm_runtime struct and lock it every time a writeX is performed. This also would have the advantage that message blocks would never overlap which currently is possible. I can prepare a patch on top of this one and send it later, if you agree. Thanks, Torsten thanks, Takashi Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- sound/usb/6fire/comm.c | 38 +- sound/usb/6fire/comm.h |2 +- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c index 9e6e3ff..23452ee 100644 --- a/sound/usb/6fire/comm.c +++ b/sound/usb/6fire/comm.c @@ -110,19 +110,37 @@ static int usb6fire_comm_send_buffer(u8 *buffer, struct usb_device *dev) static int usb6fire_comm_write8 (struct comm_runtime *rt, u8 request, u8 reg, u8 value) { - u8 buffer[13]; /* 13: maximum length of message */ + u8 *buffer; + int ret; + + /* 13: maximum length of message */ + buffer = kmalloc(13, GFP_KERNEL); + if (!buffer) + return -ENOMEM; usb6fire_comm_init_buffer(buffer, 0x00, request, reg, value, 0x00); - return usb6fire_comm_send_buffer(buffer, rt-chip-dev); + ret = usb6fire_comm_send_buffer(buffer, rt-chip-dev); + + kfree(buffer); + return ret; } static int usb6fire_comm_write16(struct comm_runtime *rt, u8 request, u8 reg, u8 vl, u8 vh) { - u8 buffer[13]; /* 13: maximum length of message */ + u8 *buffer; + int ret; + + /* 13: maximum length of message */ + buffer = kmalloc(13, GFP_KERNEL); + if (!buffer) + return -ENOMEM; usb6fire_comm_init_buffer(buffer, 0x00, request, reg, vl, vh); - return usb6fire_comm_send_buffer(buffer, rt-chip-dev); + ret = usb6fire_comm_send_buffer(buffer, rt-chip-dev); + + kfree(buffer); + return ret; } int usb6fire_comm_init(struct sfire_chip *chip) @@ -135,6 +153,12 @@ int usb6fire_comm_init(struct sfire_chip *chip) if (!rt) return -ENOMEM; + rt-receiver_buffer = kzalloc(COMM_RECEIVER_BUFSIZE, GFP_KERNEL); + if (!rt-receiver_buffer) { + kfree(rt); + return -ENOMEM; + } + urb = rt-receiver; rt-serial = 1; rt-chip = chip; @@ -153,6 +177,7 @@ int usb6fire_comm_init(struct sfire_chip *chip) urb-interval = 1; ret = usb_submit_urb(urb, GFP_KERNEL); if (ret 0) { + kfree(rt-receiver_buffer); kfree(rt); snd_printk(KERN_ERR PREFIX cannot create comm data receiver.); return ret; @@ -171,6 +196,9 @@ void usb6fire_comm_abort(struct sfire_chip *chip) void usb6fire_comm_destroy(struct sfire_chip *chip) { - kfree(chip-comm); + struct comm_runtime *rt = chip-comm; + + kfree(rt-receiver_buffer); + kfree(rt); chip-comm = NULL; } diff --git a/sound/usb/6fire/comm.h b/sound/usb/6fire/comm.h index 6a0840b..780d5ed 100644 --- a/sound/usb/6fire/comm.h +++ b/sound/usb/6fire/comm.h @@ -24,7 +24,7 @@ struct comm_runtime { struct sfire_chip *chip; struct urb receiver; - u8 receiver_buffer[COMM_RECEIVER_BUFSIZE]; + u8 *receiver_buffer; u8 serial; /* urb serial */ ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- 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: [alsa-devel] [PATCH] 6fire: fix DMA issues with URB transfer_buffer usage
On 07.08.2013 16:59, Torsten Schenk wrote: On Wed, 07 Aug 2013 14:43:43 +0200 Takashi Iwai ti...@suse.de wrote: At Tue, 06 Aug 2013 14:53:24 +0300, Jussi Kivilinna wrote: Patch fixes 6fire not to use stack as URB transfer_buffer. URB buffers need to be DMA-able, which stack is not. Furthermore, transfer_buffer should not be allocated as part of larger device structure because DMA coherency issues and patch fixes this issue too. Thanks for the information. There is another section where this applies in midi.c/midi.h. I can post a patch later. Appearently in pcm.c/pcm.h too: http://lxr.free-electrons.com/source/sound/usb/6fire/pcm.c#L571 http://lxr.free-electrons.com/source/sound/usb/6fire/pcm.h#L35 -Jussi Patch is only compile tested. The changes look OK, but I'd like to let it checked with a real hardware before putting to stable kernel. Torsten, could you check this? The patch works nicely. I'm just wondering if instead of calling kmalloc() every time it'd be better to put a global sender_buffer and a mutex into the comm_runtime struct and lock it every time a writeX is performed. This also would have the advantage that message blocks would never overlap which currently is possible. I can prepare a patch on top of this one and send it later, if you agree. Thanks, Torsten -- 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: EHCI: accept very late isochronous URBs
Since commits 4005ad4390bf (EHCI: implement new semantics for URB_ISO_ASAP) and c75c5ab575af (ALSA: USB: adjust for changed 3.8 USB API) became widely distributed, people have been experiencing problems with audio transfers. The slightest underrun causes complete failure, requiring the audio stream to be restarted. It turns out that the current isochronous API doesn't handle underruns in the best way. The ALSA developers would much rather have transfers that are submitted too late be accepted and complete in the normal fashion, rather than being refused outright. This patch implements the requested approach. When an isochronous URB submission is so late that all its scheduled slots have already expired, a debugging message will be printed in the log and the URB will be accepted as usual. Assuming it was submitted by a completion handler (which is normally the case), it will complete shortly thereafter with all the usb_iso_packet_descriptor status fields marked -EXDEV. This fixes (for ehci-hcd) https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1191603 It should be applied to all kernels that include commit 4005ad4390bf. Signed-off-by: Alan Stern st...@rowland.harvard.edu Tested-by: Maksim Boyko maksbo...@yandex.ru CC: Clemens Ladisch clem...@ladisch.de CC: sta...@vger.kernel.org --- [as1700] drivers/usb/host/ehci-sched.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) Index: usb-3.11/drivers/usb/host/ehci-sched.c === --- usb-3.11.orig/drivers/usb/host/ehci-sched.c +++ usb-3.11/drivers/usb/host/ehci-sched.c @@ -1391,21 +1391,20 @@ iso_stream_schedule ( /* Behind the scheduling threshold? */ if (unlikely(start next)) { + unsigned now2 = (now - base) (mod - 1); /* USB_ISO_ASAP: Round up to the first available slot */ if (urb-transfer_flags URB_ISO_ASAP) start += (next - start + period - 1) -period; /* -* Not ASAP: Use the next slot in the stream. If -* the entire URB falls before the threshold, fail. +* Not ASAP: Use the next slot in the stream, +* no matter what. */ - else if (start + span - period next) { - ehci_dbg(ehci, iso urb late %p (%u+%u %u)\n, + else if (start + span - period now2) { + ehci_dbg(ehci, iso underrun %p (%u+%u %u)\n, urb, start + base, - span - period, next + base); - status = -EXDEV; - goto fail; + span - period, now2 + base); } } -- 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] net/usb: hso: allocate URB setup_packets as separate buffers
On Wed, 2013-08-07 at 16:29 +0300, Jussi Kivilinna wrote: URB setup packets must not be allocated as part of larger structure because DMA coherence issues. Patch changes hso to allocate ctrl_req_[tr]x members of 'struct hso_serial' as separate buffers. Patch is only compile tested. Tested-by: Dan Williams d...@redhat.com Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- drivers/net/usb/hso.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index cba1d46..fc082c0 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -237,8 +237,8 @@ struct hso_serial { u16 tx_data_length; /* should contain allocated length */ u16 tx_data_count; u16 tx_buffer_count; - struct usb_ctrlrequest ctrl_req_tx; - struct usb_ctrlrequest ctrl_req_rx; + struct usb_ctrlrequest *ctrl_req_tx; + struct usb_ctrlrequest *ctrl_req_rx; struct usb_endpoint_descriptor *in_endp; struct usb_endpoint_descriptor *out_endp; @@ -1847,7 +1847,7 @@ static int hso_mux_serial_read(struct hso_serial *serial) USB_CDC_GET_ENCAPSULATED_RESPONSE, serial-parent-port_spec HSO_PORT_MASK, serial-rx_urb[0], - serial-ctrl_req_rx, + serial-ctrl_req_rx, serial-rx_data[0], serial-rx_data_length); } @@ -1916,7 +1916,7 @@ static int hso_mux_serial_write_data(struct hso_serial *serial) USB_CDC_SEND_ENCAPSULATED_COMMAND, serial-parent-port_spec HSO_PORT_MASK, serial-tx_urb, - serial-ctrl_req_tx, + serial-ctrl_req_tx, serial-tx_data, serial-tx_data_count); } @@ -2264,6 +2264,10 @@ static void hso_serial_common_free(struct hso_serial *serial) usb_free_urb(serial-tx_urb); kfree(serial-tx_data); tty_port_destroy(serial-port); + + /* free control URB setup packet buffers */ + kfree(serial-ctrl_req_tx); + kfree(serial-ctrl_req_rx); } static int hso_serial_common_create(struct hso_serial *serial, int num_urbs, @@ -2292,6 +2296,12 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs, spin_lock_init(serial-serial_lock); serial-num_rx_urbs = num_urbs; + /* allocate control URB setup packet buffers */ + serial-ctrl_req_tx = kzalloc(sizeof(*serial-ctrl_req_tx), GFP_KERNEL); + serial-ctrl_req_rx = kzalloc(sizeof(*serial-ctrl_req_rx), GFP_KERNEL); + if (!serial-ctrl_req_tx || !serial-ctrl_req_rx) + goto exit; + /* RX, allocate urb and initialize */ /* prepare our RX buffer */ -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] net/usb: hso: allocate serial_state_notification as separate buffer
On Wed, 2013-08-07 at 16:29 +0300, Jussi Kivilinna wrote: URB transfer_buffers must not be allocated as part of larger structure because DMA coherence issues. Patch changes hso to allocate serial_state_notification member of 'struct hso_tiocmget' as separate buffer. Patch is only compile tested. Tested-by: Dan Williams d...@redhat.com Cc: sta...@vger.kernel.org Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi --- drivers/net/usb/hso.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index fc082c0..21ffa6f 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -211,7 +211,7 @@ struct hso_tiocmget { intintr_completed; struct usb_endpoint_descriptor *endp; struct urb *urb; - struct hso_serial_state_notification serial_state_notification; + struct hso_serial_state_notification *serial_state_notification; u16prev_UART_state_bitmap; struct uart_icount icount; }; @@ -1465,7 +1465,7 @@ static int tiocmget_submit_urb(struct hso_serial *serial, usb_rcvintpipe(usb, tiocmget-endp- bEndpointAddress 0x7F), - tiocmget-serial_state_notification, + tiocmget-serial_state_notification, sizeof(struct hso_serial_state_notification), tiocmget_intr_callback, serial, tiocmget-endp-bInterval); @@ -1499,7 +1499,7 @@ static void tiocmget_intr_callback(struct urb *urb) if (!tiocmget) return; usb = serial-parent-usb; - serial_state_notification = tiocmget-serial_state_notification; + serial_state_notification = tiocmget-serial_state_notification; if (serial_state_notification-bmRequestType != BM_REQUEST_TYPE || serial_state_notification-bNotification != B_NOTIFICATION || le16_to_cpu(serial_state_notification-wValue) != W_VALUE || @@ -2608,6 +2608,7 @@ static void hso_free_tiomget(struct hso_serial *serial) usb_free_urb(tiocmget-urb); tiocmget-urb = NULL; serial-tiocmget = NULL; + kfree(tiocmget-serial_state_notification); kfree(tiocmget); } } @@ -2664,6 +2665,13 @@ static struct hso_device *hso_create_bulk_serial_device( */ if (serial-tiocmget) { tiocmget = serial-tiocmget; + + tiocmget-serial_state_notification = kzalloc( + sizeof(struct hso_serial_state_notification), + GFP_KERNEL); + if (!tiocmget-serial_state_notification) + goto tiocmget_fail; + tiocmget-urb = usb_alloc_urb(0, GFP_KERNEL); if (tiocmget-urb) { mutex_init(tiocmget-mutex); @@ -2672,8 +2680,10 @@ static struct hso_device *hso_create_bulk_serial_device( interface, USB_ENDPOINT_XFER_INT, USB_DIR_IN); - } else + } else { +tiocmget_fail: hso_free_tiomget(serial); + } } } else -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xHCI ISR be registered twice
Hi Felipe, This patch brings up an interesting point: will a dwc3 PCI host use the xhci platform driver instead of the xhci PCI driver? If so, that seems less than ideal. Won't it not use the standard xHCI PCI suspend and resume functions if it's registered as a platform device? Also, it seems registering it that way means XHCI_BROKEN_MSI is set unconditionally. That leads to the xhci driver not enabling MSI or MSI-X for the PCI host, which will impact performance. Hi Yu, Thanks for taking this bug down. I have some process-oriented issues that need to be addressed, and some comments that need to be addressed in the next version of your patch. First, please use this email address to send me patches: Sarah Sharp sarah.a.sh...@linux.intel.com I use the linux.intel.com email address to handle all patches and traffic to the Linux mailing lists. My intel.com email address is for Intel internal communications only. In order for us to apply patches, you need to send them inline in the email, without your introduction. The subject line must be the subject line of the patch. Basically, you need to use either `git send-email` or `git format-patch` and mutt to send the patch. Please see this tutorial for more information on sending proper patches: http://kernelnewbies.org/OPWfirstpatch#head-2043a643d048c341b2a52044fd72d852aac87fef If you still need to introduce a patch, you can put this scissors line between your introduction and the patch description: 8-8 However, in general, your patch description should contain all the information necessary for us to decide whether we need to apply the patch. If the bug was only hit with a specific configuration, that needs to be included in the patch description, so that distros know whether they need to apply the patch. Comments on the patch below. On Tue, Aug 06, 2013 at 11:08:04PM -0700, Wang, Yu Y wrote: Hi Balbi, Sarah, I found that when CONFIG_PCI is set, and xHCI driver register as platform device driver. Then xhci ISR will be register twice. The first time is in usb_add_hcd, the second time is in xhci_run (xhci_try_enable_msi). This issue should be reproduce when dwc3 driver registered as PCI device driver. And CONFIG_USB_DWC3_DUAL_ROLE is set. This information should all be in your patch description. It's important to document how to reproduce the bug you're fixing in the patch that fixes it. Fixed patch please help review: Hopefully when you use `git send-email` or mutt you won't get these extra newlines. Don't send patches through outlook, it completely mangles them. You'll need to set up esmtp on a Linux system in order to be able to send mail to the Intel exchange servers with `git send-email` or mutt. From 8b437ac59be296cd7c0fa189344f3b30281fb3f1 Mon Sep 17 00:00:00 2001 From: Wang, Yu yu.y.w...@intel.com Date: Wed, 7 Aug 2013 13:26:30 +0800 Subject: [PATCH 5/6] xhci: xHCI ISR be registers twice. This is one xHCI driver BUG. If CONFIG_PCI be set and xHCI driver registers as platform driver. Then xHCI ISR will be registers twice, the first time is during usb_add_hcd. The second time is in xhci_run. You'll need a newline between your description and the Signed-off-by line. I can't tell if you currently have one because of the mangled patch. Signed-off-by: Wang, Yu yu.y.w...@intel.com Change-Id: Ibf86bca8f099586a0f379ee843b95c4d93b88d67 --- drivers/usb/host/xhci.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index d8f640b..b43f722 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -372,6 +372,10 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd) } legacy_irq: + /* The leacy irq was already registered in USB core */ + if (hcd-irq) + return 0; + Let's take a look at the function you're patching: static int xhci_try_enable_msi(struct usb_hcd *hcd) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)-self.controller); int ret; /* * Some Fresco Logic host controllers advertise MSI, but fail to * generate interrupts. Don't even try to enable MSI. */ if (xhci-quirks XHCI_BROKEN_MSI) goto legacy_irq; /* unregister the legacy interrupt */ if (hcd-irq) free_irq(hcd-irq, hcd); hcd-irq = 0; ret = xhci_setup_msix(xhci); if (ret) /* fall back to msi*/ ret = xhci_setup_msi(xhci); if (!ret) /* hcd-irq is 0, we have MSI */ return 0; if (!pdev-irq) { xhci_err(xhci, No msi-x/msi found and no IRQ in BIOS\n); return -EINVAL; } legacy_irq: /*
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On Tue, Aug 06, 2013 at 07:00:17PM +0100, Julius Werner wrote: This patch simplifies the way the phy-samsung-usb code finds the correct power management register to enable PHY clock gating. Previously, the code would calculate the register address from a device tree supplied base address and add an offset based on the PHY type. Since every PHY has its own device tree entry and needs only one register, we can just encode the address itself in the device tree and remove the diffentiation in the code. The bitmask needed to specify the bit within that register stays in place, allowing support for platforms like s3c64xx that use different bits within the same register. This breaks compatibility, both for an old kernel and a new dt and a new kernel with an old dt. Is anyone using these bindings? Signed-off-by: Julius Werner jwer...@chromium.org --- .../devicetree/bindings/usb/samsung-usbphy.txt | 26 +- arch/arm/boot/dts/exynos5250.dtsi | 4 ++-- drivers/usb/phy/phy-samsung-usb.c | 18 --- drivers/usb/phy/phy-samsung-usb.h | 23 +-- drivers/usb/phy/phy-samsung-usb2.c | 11 - drivers/usb/phy/phy-samsung-usb3.c | 2 +- 6 files changed, 22 insertions(+), 62 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt index 33fd354..1cf9b68 100644 --- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt @@ -34,14 +34,7 @@ Optional properties: - The child node 'usbphy-sys' to the node 'usbphy' is for the system controller interface for usb-phy. It should provide the following information required by usb-phy controller to control phy. - - reg : base physical address of PHY_CONTROL registers. - The size of this register is the total sum of size of all PHY_CONTROL - registers that the SoC has. For example, the size will be - '0x4' in case we have only one PHY_CONTROL register (e.g. - OTHERS register in S3C64XX or USB_PHY_CONTROL register in S5PV210) - and, '0x8' in case we have two PHY_CONTROL registers (e.g. - USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers in exynos4x). - and so on. + - reg : address of PHY_CONTROL register for this PHY. Example: - Exynos4210 @@ -57,8 +50,8 @@ Example: clock-names = xusbxti, otg; usbphy-sys { - /* USB device and host PHY_CONTROL registers */ - reg = 0x10020704 0x8; + /* USB device PHY_CONTROL register */ + reg = 0x10020704 0x4; }; }; Why are we describing fewer registers now? Are they described elsewhere? The dt should describe the device, not only the portion of it Linux wants to use right now. Thanks, Mark. -- 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] 6fire: fix URB transfer buffer for midi output
[Cc'ed to linux-usb ML] At Wed, 7 Aug 2013 16:51:49 +0200, Torsten Schenk wrote: Patch fixes URB transfer buffer allocation for midi output to be DMA-able. Is this really needed? That is, can't a transfer buffer be at middle of kmalloc'ed space, but must be always the head of the kmalloc'ed space? Takashi Signed-off-by: Torsten Schenk torsten.sch...@zoho.com --- diff -Nur a/sound/usb/6fire/midi.c b/sound/usb/6fire/midi.c --- a/sound/usb/6fire/midi.c 2013-08-07 16:32:10.579639391 +0200 +++ b/sound/usb/6fire/midi.c 2013-08-07 16:32:31.363378104 +0200 @@ -19,6 +19,10 @@ #include chip.h #include comm.h +enum { + MIDI_BUFSIZE = 64 +}; + static void usb6fire_midi_out_handler(struct urb *urb) { struct midi_runtime *rt = urb-context; @@ -156,6 +160,12 @@ if (!rt) return -ENOMEM; + rt-out_buffer = kzalloc(MIDI_BUFSIZE, GFP_KERNEL); + if (!rt-out_buffer) { + kfree(rt); + return -ENOMEM; + } + rt-chip = chip; rt-in_received = usb6fire_midi_in_received; rt-out_buffer[0] = 0x80; /* 'send midi' command */ @@ -169,6 +179,7 @@ ret = snd_rawmidi_new(chip-card, 6FireUSB, 0, 1, 1, rt-instance); if (ret 0) { + kfree(rt-out_buffer); kfree(rt); snd_printk(KERN_ERR PREFIX unable to create midi.\n); return ret; @@ -197,6 +208,9 @@ void usb6fire_midi_destroy(struct sfire_chip *chip) { - kfree(chip-midi); + struct midi_runtime *rt = chip-midi; + + kfree(rt-out_buffer); + kfree(rt); chip-midi = NULL; } diff -Nur a/sound/usb/6fire/midi.h b/sound/usb/6fire/midi.h --- a/sound/usb/6fire/midi.h 2013-08-07 16:32:10.579639391 +0200 +++ b/sound/usb/6fire/midi.h 2013-08-07 16:32:31.363378104 +0200 @@ -16,10 +16,6 @@ #include common.h -enum { - MIDI_BUFSIZE = 64 -}; - struct midi_runtime { struct sfire_chip *chip; struct snd_rawmidi *instance; @@ -32,7 +28,7 @@ struct snd_rawmidi_substream *out; struct urb out_urb; u8 out_serial; /* serial number of out packet */ - u8 out_buffer[MIDI_BUFSIZE]; + u8 *out_buffer; int buffer_offset; void (*in_received)(struct midi_runtime *rt, u8 *data, int length); -- 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] alauda: do not use stack for URB transfer_buffers
On Wed, 7 August 2013 08:50:53 +0300, Jussi Kivilinna wrote: On 06.08.2013 19:49, Jörn Engel wrote: On Tue, 6 August 2013 15:03:29 +0300, Jussi Kivilinna wrote: Patch fixes alauda not to use stack as URB transfer_buffer. URB buffers need to be DMA-able, which stack is not. Patch is only compile tested. I have tested the driver back when I wrote it. Not sure why it worked then, maybe the chip in my notebook back then didn't care about alignment or I just got lucky with the memory allocations. You might not see problems with your hardware/architecture. Documentation/DMA-API-HOWTO.txt discusses about What memory is DMA'able?. Makes sense. I've read your patch over and it also makes sense. Acked-by: Joern Engel jo...@logfs.org Jörn -- It's just what we asked for, but not what we want! -- anonymous -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
This breaks compatibility, both for an old kernel and a new dt and a new kernel with an old dt. Is anyone using these bindings? They only affect Samsung SoCs and have only been upstream for half a year, so I doubt it's heavily used. Why are we describing fewer registers now? Are they described elsewhere? The dt should describe the device, not only the portion of it Linux wants to use right now. This only ever described a small section of the huge set of PMU registers anyway. Before it described up to three registers controlling different PHYs (using hardcoded offsets in the code to later find the right one)... with my patch every PHY's DT entry only describes the one register concerning itself, which makes more sense in my opinion. It will also prevent the register descriptions in different DT entries from overlapping. -- 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: ar5523 Gigaset USB Adapter 108 issue
On Wed, Aug 07, 2013 at 11:35:35AM +0200, Oleksij Rempel wrote: Just in case, i tested ar5523 based adapter on two different xhci controllers. No visible problems. Ah, ok, I had missed the fact that Yannik was using the device under EHCI. I'm not sure why he emailed me instead of Alan Stern, since he's the EHCI maintainer. Yannik, you don't need to recompile your kernel in that case. Alan, can you debug this issue with Yannik since it doesn't seem to be an xHCI issue? Sarah Sharp Am 07.08.2013 05:39, schrieb Sarah Sharp: Please recompile your kernel with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on, and email dmesg from when you first plug in the device, to when it fails. Thanks, Sarah Sharp On Thu, Aug 01, 2013 at 07:41:27PM +0200, Yannik Völker wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, I was told to message you about this issue: I own an Gigaset USB Adapter 108 which uses the ar5523 chipset, support for that should have been added in 3.8, i am using uname -a Linux Hostname 3.10.3-1-ARCH #1 SMP PREEMPT Fri Jul 26 11:26:59 CEST 2013 x86_64 GNU/Linux right now. It is not the firmwares fault, its not even getting touched at all. The device is not broken either, i have tested it on another computer running 32 bit windows. Its not a broken USB port as i have tired several ones. When plugging it in the following lines appear in dmesg: [12482.680674] usb 8-6: USB disconnect, device number 7 [12486.112103] usb 8-6: new high-speed USB device number 8 using ehci-pci [12488.235438] usb 8-6: timeout waiting for command 01 reply [12488.235444] usb 8-6: could not initialize adapter [12488.235643] usb 8-6: RX USB error -2. [12488.235652] usb 8-6: error -1 when submitting rx urb [12488.235748] ar5523: probe of 8-6:1.0 failed with error -110 I was told that -110 means not enough power which seems unlikely as this is an Desktop computer, and I had it running on this exact computer using ndiswrapper and an 32 bit kernel. The specifications do not state extremly high Values either (Webarchive as the original website is gone) http://web.archive.org/web/20100808072407/http://gigaset.com/hq/en/product/GIGASETUSBADAPTER108.html?tab=data Power Consumption during operation: TX: 475mA, RX:290mA, Power Consumption during standby: 330mA So I thought that this might be a bug in the usb subsystem If there is any more information I can provide: just tell me. - -- -- Regards, Oleksij -- 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] 6fire: fix URB transfer buffer for midi output
On Wed, 7 Aug 2013, Takashi Iwai wrote: [Cc'ed to linux-usb ML] At Wed, 7 Aug 2013 16:51:49 +0200, Torsten Schenk wrote: Patch fixes URB transfer buffer allocation for midi output to be DMA-able. Is this really needed? That is, can't a transfer buffer be at middle of kmalloc'ed space, but must be always the head of the kmalloc'ed space? A buffer _can_ be in the middle of a kmalloc'ed space, but the CPU must not access any of the fields around it that might occupy the same cache line while the buffer is being used for DMA. In general, it's safest not to put any other data in the same kmalloc'ed region with a DMA buffer. @@ -32,7 +28,7 @@ struct snd_rawmidi_substream *out; struct urb out_urb; u8 out_serial; /* serial number of out packet */ - u8 out_buffer[MIDI_BUFSIZE]; + u8 *out_buffer; int buffer_offset; In this case, the CPU would access out_urb while out_buffer was in use. 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: phy: Cleanup error code in **_usb_get_phy_**() APIs
@@ -94,11 +94,11 @@ static int devm_usb_phy_match(struct device *dev, void *res, void *match_data) */ struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type) { - struct usb_phy **ptr, *phy; + struct usb_phy *phy = ERR_PTR(-ENOMEM), **ptr; This looks a little roundabout, don't you think? Why don't you just directly have 'return ERR_PTR(-ENOMEM)' down there where you put 'goto err0'? ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL); if (!ptr) - return NULL; + goto err0; phy = usb_get_phy(type); if (!IS_ERR(phy)) { @@ -107,6 +107,7 @@ struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type) } else devres_free(ptr); +err0: return phy; } EXPORT_SYMBOL_GPL(devm_usb_get_phy); struct usb_phy *devm_usb_get_phy_dev(struct device *dev, u8 index) { - struct usb_phy **ptr, *phy; + struct usb_phy *phy = ERR_PTR(-ENOMEM), **ptr; Same here ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL); if (!ptr) - return NULL; + goto err0; phy = usb_get_phy_dev(dev, index); if (!IS_ERR(phy)) { @@ -267,6 +268,7 @@ struct usb_phy *devm_usb_get_phy_dev(struct device *dev, u8 index) } else devres_free(ptr); +err0: return phy; } EXPORT_SYMBOL_GPL(devm_usb_get_phy_dev); @@ -142,7 +142,7 @@ extern void usb_remove_phy(struct usb_phy *); /* helpers for direct access thru low-level io interface */ static inline int usb_phy_io_read(struct usb_phy *x, u32 reg) { - if (x-io_ops x-io_ops-read) + if (!IS_ERR(x) x-io_ops x-io_ops-read) I liked the ones where we had IS_ERR_OR_NULL() here (and in all the ones below)... you sometimes have to handle PHYs in platform-independent code where you don't want to worry about if this platform actually has a PHY driver there or not. Any reason you changed that? return x-io_ops-read(x, reg); return -EINVAL; @@ -150,7 +150,7 @@ static inline int usb_phy_io_read(struct usb_phy *x, u32 reg) static inline int usb_phy_io_write(struct usb_phy *x, u32 val, u32 reg) { - if (x-io_ops x-io_ops-write) + if (!IS_ERR(x) x-io_ops x-io_ops-write) return x-io_ops-write(x, val, reg); return -EINVAL; @@ -159,7 +159,7 @@ static inline int usb_phy_io_write(struct usb_phy *x, u32 val, u32 reg) static inline int usb_phy_init(struct usb_phy *x) { - if (x-init) + if (!IS_ERR(x) x-init) return x-init(x); return 0; @@ -168,26 +168,27 @@ usb_phy_init(struct usb_phy *x) static inline void usb_phy_shutdown(struct usb_phy *x) { - if (x-shutdown) + if (!IS_ERR(x) x-shutdown) x-shutdown(x); } static inline int usb_phy_vbus_on(struct usb_phy *x) { - if (!x-set_vbus) - return 0; + if (!IS_ERR(x) x-set_vbus) + return x-set_vbus(x, true); - return x-set_vbus(x, true); + return 0; } static inline int usb_phy_vbus_off(struct usb_phy *x) { - if (!x-set_vbus) - return 0; + if (!IS_ERR(x) x-set_vbus) + return x-set_vbus(x, false); + + return 0; - return x-set_vbus(x, false); } /* for usb host and peripheral controller drivers */ @@ -249,8 +250,9 @@ static inline int usb_bind_phy(const char *dev_name, u8 index, static inline int usb_phy_set_power(struct usb_phy *x, unsigned mA) { - if (x x-set_power) + if (!IS_ERR(x) x-set_power) return x-set_power(x, mA); + return 0; } @@ -258,28 +260,28 @@ usb_phy_set_power(struct usb_phy *x, unsigned mA) static inline int usb_phy_set_suspend(struct usb_phy *x, int suspend) { - if (x-set_suspend != NULL) + if (!IS_ERR(x) x-set_suspend != NULL) return x-set_suspend(x, suspend); - else - return 0; + + return 0; } static inline int usb_phy_notify_connect(struct usb_phy *x, enum usb_device_speed speed) { - if (x-notify_connect) + if (!IS_ERR(x) x-notify_connect) return x-notify_connect(x, speed); - else - return 0; + + return 0; } static inline int usb_phy_notify_disconnect(struct usb_phy *x, enum usb_device_speed speed) { - if (x-notify_disconnect) + if (!IS_ERR(x) x-notify_disconnect) return x-notify_disconnect(x, speed); - else - return 0; + + return 0; } Otherwise looks good to me. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On 08/07/2013 07:06 PM, Julius Werner wrote: This breaks compatibility, both for an old kernel and a new dt and a new kernel with an old dt. Is anyone using these bindings? They only affect Samsung SoCs and have only been upstream for half a year, so I doubt it's heavily used. It probably wouldn't be of much concern, but I can't tell for sure. Why are we describing fewer registers now? Are they described elsewhere? The dt should describe the device, not only the portion of it Linux wants to use right now. This only ever described a small section of the huge set of PMU registers anyway. The PMU registers are already scattered across various drivers, like Power Domain, various PHYs (USB/HSIC/MIPI CSI-2/DSI, ADC, ...), Reset and more. And it happens currently there is no central driver for the Power Management Unit, that would, for example expose the regmap interface that the interested drivers could use. The downside of this would be that each, e.g. USB PHY driver would need to know SoC specific offsets to its registers in the PMU. Before device tree started to be used those PMU registers were controlled by respective drivers, mostly through platform_data callbacks. As they could be considered parts of given device. Before it described up to three registers controlling different PHYs (using hardcoded offsets in the code to later find the right one)... with my patch every PHY's DT entry only describes the one register concerning itself, which makes more sense in my opinion. It will also prevent the register descriptions in different DT entries from overlapping. Yes, the patch looks sensible. Since currently each USB PHY has its own device tree node it looks wrong to have the reg property in the usbphy-sys subnodes defining register region for all possible PHYs. And we indeed and up with multiple reg properties pointing to same register region. However the biggest drawback is breaking backwards compatibility. I'm not sure if those changes are worth it, especially that all those USB PHY drivers are supposed to be rewritten to use the generic PHY API. Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] lsusb: Reports devices that support BESL on USB2.0
This patch adds USB2-LPM-Errata for USB2.0 Binary Object Store Descriptor that support Best Effort Latency Tolerance (BELT), with the Baseline BESL value, and Deep BESL values. Additionally, it identifies if a Binary Object Store Descriptor is pre-errata HIRD Link Power Management. Signed-off-by: Alexandra Yates alexandra.ya...@intel.com --- lsusb.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lsusb.c b/lsusb.c index 4306697..5e92297 100644 --- a/lsusb.c +++ b/lsusb.c @@ -3639,9 +3639,18 @@ static void dump_usb2_device_capability_desc(unsigned char *buf) buf[0], buf[1], buf[2], wide); if (!(wide 0x02)) printf( (Missing must-be-set LPM bit!)\n); - else - printf( Link Power Management (LPM) + else if (!(wide 0x04)) + printf( HIRD Link Power Management (LPM) +Supported\n); + else { + printf( BESL Link Power Management (LPM) Supported\n); + if (wide 0x08) + printf(BESL value%5u us \n, wide 0xf00); + if (wide 0x10) + printf(Deep BESL value%5u us \n, + wide 0xf000); + } } static void dump_ss_device_capability_desc(unsigned char *buf) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] lsusb LPM BESL support
*** BLURB HERE *** Alexandra Yates (2): lsusb: Reports if USB2.0 port is on L1 state lsusb: Reports devices that support BESL on USB2.0 lsusb.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] lsusb: Reports if USB2.0 port is on L1 state
From: Alexandra Yates alexandra.ya...@linux.intel.com This patch reports the low power state L1 for ports with attahced USB2 devices. The output will be part of the roothub port status as follows: ayates@magd:~$ sudo lsusb -v -s 001:001 Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub ... Hub Descriptor: bLength 11 bDescriptorType 41 nNbrPorts 9 wHubCharacteristic 0x000a No power switching (usb 1.0) Per-port overcurrent protection TT think time 8 FS bits bPwrOn2PwrGood 10 * 2 milli seconds bHubContrCurrent 0 milli Ampere DeviceRemovable0x68 0x02 PortPwrCtrlMask0xff 0xff Hub Port Status: Port 1: .0103 power enable Port 2: .0523 highspeed power L1 enable Port 3: .0100 power Port 4: .0100 power Port 5: .0503 highspeed power enable Port 6: .0100 power Port 7: .0100 power Port 8: .0100 power Port 9: .0100 power Device Status: 0x0001 In this example port id: .0523 is in L1 status. This output is collected from testing the change on a HSWULT platform. Signed-off-by: Alexandra Yates alexandra.ya...@linux.intel.com --- lsusb.c |1 + 1 file changed, 1 insertion(+) diff --git a/lsusb.c b/lsusb.c index 470146c..4306697 100644 --- a/lsusb.c +++ b/lsusb.c @@ -3317,6 +3317,7 @@ static void do_hub(libusb_device_handle *fd, unsigned tt_type, unsigned speed) (status[1] 0x04) ? highspeed : , (status[1] 0x02) ? lowspeed : , (status[1] 0x01) ? power : , + (status[0] 0x20) ? L1 : , (status[0] 0x10) ? RESET : , (status[0] 0x08) ? oc : , (status[0] 0x04) ? suspend : , -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] lsusb LPM BESL support
On Wed, Aug 07, 2013 at 12:12:26PM -0700, Alexandra Yates wrote: *** BLURB HERE *** What is supposed to go here? -- 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: misc: Fix swapped properties in usb3503 DT parsing
From: Mark Brown broo...@linaro.org The intn and connect GPIO properties are swapped in the code which will cause failures at runtime if these are connected, fix the code. There are currently no in-tree users of this device to check or update. Signed-off-by: Mark Brown broo...@linaro.org --- drivers/usb/misc/usb3503.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c index cbb6b78..4e3a2d2 100644 --- a/drivers/usb/misc/usb3503.c +++ b/drivers/usb/misc/usb3503.c @@ -215,10 +215,10 @@ static int usb3503_probe(struct i2c_client *i2c, const struct i2c_device_id *id) } } - hub-gpio_intn = of_get_named_gpio(np, connect-gpios, 0); + hub-gpio_intn = of_get_named_gpio(np, intn-gpios, 0); if (hub-gpio_intn == -EPROBE_DEFER) return -EPROBE_DEFER; - hub-gpio_connect = of_get_named_gpio(np, intn-gpios, 0); + hub-gpio_connect = of_get_named_gpio(np, connect-gpios, 0); if (hub-gpio_connect == -EPROBE_DEFER) return -EPROBE_DEFER; hub-gpio_reset = of_get_named_gpio(np, reset-gpios, 0); -- 1.8.4.rc1 -- 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] Fix invalid volume resolution for Logitech HD Webcam C525
From: Maksim Boyko maksim.a.bo...@gmail.com Add the volume control quirk for avoiding the kernel warning for Logitech HD Webcam C525. The similar patch was previously reported for Logitech HD Webcam C310 (see 36691e1be6ec551eef4a5225f126a281f8c051c2). --- linux-3.10.5-orig/sound/usb/mixer.c 2013-08-04 12:51:49.0 +0400 +++ linux-3.10.5/sound/usb/mixer.c 2013-08-06 21:12:42.274197287 +0400 @@ -888,6 +888,7 @@ case USB_ID(0x046d, 0x081b): /* HD Webcam c310 */ case USB_ID(0x046d, 0x081d): /* HD Webcam c510 */ case USB_ID(0x046d, 0x0825): /* HD Webcam c270 */ + case USB_ID(0x046d, 0x0826): /* HD Webcam c525 */ case USB_ID(0x046d, 0x0991): /* Most audio usb devices lie about volume resolution. * Most Logitech webcams have res = 384. -- 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] Fix invalid volume resolution for Logitech HD Webcam C525
On Wed, Aug 07, 2013 at 11:24:46PM +0400, Бойко Максим wrote: From: Maksim Boyko maksim.a.bo...@gmail.com Add the volume control quirk for avoiding the kernel warning for Logitech HD Webcam C525. The similar patch was previously reported for Logitech HD Webcam C310 (see 36691e1be6ec551eef4a5225f126a281f8c051c2). --- linux-3.10.5-orig/sound/usb/mixer.c 2013-08-04 12:51:49.0 +0400 +++ linux-3.10.5/sound/usb/mixer.c2013-08-06 21:12:42.274197287 +0400 @@ -888,6 +888,7 @@ case USB_ID(0x046d, 0x081b): /* HD Webcam c310 */ case USB_ID(0x046d, 0x081d): /* HD Webcam c510 */ case USB_ID(0x046d, 0x0825): /* HD Webcam c270 */ + case USB_ID(0x046d, 0x0826): /* HD Webcam c525 */ case USB_ID(0x046d, 0x0991): /* Most audio usb devices lie about volume resolution. * Most Logitech webcams have res = 384. formletter This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read Documentation/stable_kernel_rules.txt for how to do this properly. /formletter -- 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] Fix invalid volume resolution for Logitech HD Webcam C525
Hello. On 08/07/2013 11:24 PM, Бойко Максим wrote: From: Maksim Boyko maksim.a.bo...@gmail.com Add the volume control quirk for avoiding the kernel warning for Logitech HD Webcam C525. The similar patch was previously reported for Logitech HD Webcam C310 (see 36691e1be6ec551eef4a5225f126a281f8c051c2). Please also specif that commit's summary line in parens. And wrap your changelog lines at 80 columns at least. --- linux-3.10.5-orig/sound/usb/mixer.c 2013-08-04 12:51:49.0 +0400 +++ linux-3.10.5/sound/usb/mixer.c 2013-08-06 21:12:42.274197287 +0400 @@ -888,6 +888,7 @@ case USB_ID(0x046d, 0x081b): /* HD Webcam c310 */ case USB_ID(0x046d, 0x081d): /* HD Webcam c510 */ case USB_ID(0x046d, 0x0825): /* HD Webcam c270 */ + case USB_ID(0x046d, 0x0826): /* HD Webcam c525 */ case USB_ID(0x046d, 0x0991): /* Most audio usb devices lie about volume resolution. * Most Logitech webcams have res = 384. Hm, the file has bad indentation (your added line is fine unless it's out of sync with the surrounding ones). WBR, 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: VM Kernel bug with CONFIG_DEBUG_VM
On Wed, Aug 07, 2013 at 03:50:42PM +0800, Ming Lei wrote: On Wed, Aug 7, 2013 at 1:41 PM, Christoffer Dall christoffer.d...@linaro.org wrote: Hi all, I've found that booting a recent kernel (and as far back as 3.9 at least) I get a kernel bug during boot with VM debugging enabled. I haven't had time to dig further into it, but wanted to check if this is a known issue? Remember Catalin introduced the flush_dcache_page() to fix cache incoherency problem when PIO reading from USB mass storage device to mapped page, so the flush should not need for pages not in page cache. Care to test below patch to see if the problem can be fixed? diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c index 2facee5..9d47c71 100644 --- a/drivers/usb/host/isp1760-hcd.c +++ b/drivers/usb/host/isp1760-hcd.c @@ -707,8 +707,12 @@ __acquires(priv-lock) void *ptr; for (ptr = urb-transfer_buffer; ptr urb-transfer_buffer + urb-transfer_buffer_length; - ptr += PAGE_SIZE) - flush_dcache_page(virt_to_page(ptr)); + ptr += PAGE_SIZE) { + struct page *page = virt_to_page(ptr); + if (PageSlab(page)) + break; + flush_dcache_page(page); + } } /* complete() can reenter this HCD */ Yep, that fixes it. Thanks!! -Christoffer -- 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
Investment
ATTENTION. Let me use the liberty of this medium to inform you that my principal is interested in investing his bond as a silent business partner in your company. He would like to invest in private sector projects with an established company in any project(s) which are already in the market and have market value or new company requiring the injection of huge funds, provided there are lots of opportunities available, taking into proper consideration the Return on Investment (ROI) based on a ten (10) year strategic plan. Kindly indicate your interest in my client’s proposition by furnishing me with your Bio data, business/personal contact details and any other information/detail that may help in the actualization of the impending investment portfolio. Thanks, Milos -- 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: misc: usb3503: Convert to devm_ APIs
From: Mark Brown broo...@linaro.org Saves us a bit of code. Signed-off-by: Mark Brown broo...@linaro.org --- This depends on my previous patch usb: misc: Fix swapped properties in usb3503 DT parsing. drivers/usb/misc/usb3503.c | 42 +++--- 1 file changed, 7 insertions(+), 35 deletions(-) diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c index 4e3a2d2..41b4228 100644 --- a/drivers/usb/misc/usb3503.c +++ b/drivers/usb/misc/usb3503.c @@ -187,7 +187,7 @@ static int usb3503_probe(struct i2c_client *i2c, const struct i2c_device_id *id) const u32 *property; int len; - hub = kzalloc(sizeof(struct usb3503), GFP_KERNEL); + hub = devm_kzalloc(i2c-dev, sizeof(struct usb3503), GFP_KERNEL); if (!hub) { dev_err(i2c-dev, private data alloc fail\n); return err; @@ -229,35 +229,35 @@ static int usb3503_probe(struct i2c_client *i2c, const struct i2c_device_id *id) } if (gpio_is_valid(hub-gpio_intn)) { - err = gpio_request_one(hub-gpio_intn, + err = devm_gpio_request_one(i2c-dev, hub-gpio_intn, GPIOF_OUT_INIT_HIGH, usb3503 intn); if (err) { dev_err(i2c-dev, unable to request GPIO %d as connect pin (%d)\n, hub-gpio_intn, err); - goto err_out; + return err; } } if (gpio_is_valid(hub-gpio_connect)) { - err = gpio_request_one(hub-gpio_connect, + err = devm_gpio_request_one(i2c-dev, hub-gpio_connect, GPIOF_OUT_INIT_HIGH, usb3503 connect); if (err) { dev_err(i2c-dev, unable to request GPIO %d as connect pin (%d)\n, hub-gpio_connect, err); - goto err_gpio_connect; + return err; } } if (gpio_is_valid(hub-gpio_reset)) { - err = gpio_request_one(hub-gpio_reset, + err = devm_gpio_request_one(i2c-dev, hub-gpio_reset, GPIOF_OUT_INIT_LOW, usb3503 reset); if (err) { dev_err(i2c-dev, unable to request GPIO %d as reset pin (%d)\n, hub-gpio_reset, err); - goto err_gpio_reset; + return err; } } @@ -267,33 +267,6 @@ static int usb3503_probe(struct i2c_client *i2c, const struct i2c_device_id *id) (hub-mode == USB3503_MODE_HUB) ? hub : standby); return 0; - -err_gpio_reset: - if (gpio_is_valid(hub-gpio_connect)) - gpio_free(hub-gpio_connect); -err_gpio_connect: - if (gpio_is_valid(hub-gpio_intn)) - gpio_free(hub-gpio_intn); -err_out: - kfree(hub); - - return err; -} - -static int usb3503_remove(struct i2c_client *i2c) -{ - struct usb3503 *hub = i2c_get_clientdata(i2c); - - if (gpio_is_valid(hub-gpio_intn)) - gpio_free(hub-gpio_intn); - if (gpio_is_valid(hub-gpio_connect)) - gpio_free(hub-gpio_connect); - if (gpio_is_valid(hub-gpio_reset)) - gpio_free(hub-gpio_reset); - - kfree(hub); - - return 0; } static const struct i2c_device_id usb3503_id[] = { @@ -316,7 +289,6 @@ static struct i2c_driver usb3503_driver = { .of_match_table = of_match_ptr(usb3503_of_match), }, .probe = usb3503_probe, - .remove = usb3503_remove, .id_table = usb3503_id, }; -- 1.8.4.rc1 -- 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/hcd: Log error code if reset() fails
From: Mark Brown broo...@linaro.org If someone provided meaningful error codes from reset() we should tell the user what they were. Signed-off-by: Mark Brown broo...@linaro.org --- drivers/usb/core/hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index dc1346f..c7fdf10 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2580,7 +2580,7 @@ int usb_add_hcd(struct usb_hcd *hcd, * should already have been reset (and boot firmware kicked off etc). */ if (hcd-driver-reset (retval = hcd-driver-reset(hcd)) 0) { - dev_err(hcd-self.controller, can't setup\n); + dev_err(hcd-self.controller, can't setup: %d\n, retval); goto err_hcd_driver_setup; } hcd-rh_pollable = 1; -- 1.8.4.rc1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/2] lsusb LPM BESL support
Sorry about that. Here is the blurb: The two patches enable lsusb with LPM for USB2.0 Errata. They present the user with link status, BESL and DBESL values. Thank you, Alexandra. -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Wednesday, August 07, 2013 12:28 PM To: Yates, Alexandra Cc: linux-usb@vger.kernel.org Subject: Re: [PATCH 0/2] lsusb LPM BESL support On Wed, Aug 07, 2013 at 12:12:26PM -0700, Alexandra Yates wrote: *** BLURB HERE *** What is supposed to go here? -- 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 v2 0/2] lsusb LPM support USB2.0
The two patches enable lsusb with LPM for USB2.0 Errata. They present the user with link status, BESL, and DBESL values. Alexandra Yates (2): lsusb: Reports if USB2.0 port is on L1 state lsusb: Reports devices that support BESL on USB2.0 COPYING | 41 +++--- NEWS | 39 ++ configure.ac |2 +- lsusb-t.c|2 +- lsusb.c | 18 ++- usb-devices |2 +- usb.ids | 417 +- 7 files changed, 432 insertions(+), 89 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] lsusb: Reports devices that support BESL on USB2.0
This patch adds USB2-LPM-Errata for USB2.0 Binary Object Store Descriptor that support Best Effort Latency Tolerance (BELT), with the Baseline BESL value, and Deep BESL values. Additionally, it identifies if a Binary Object Store Descriptor is pre-errata HIRD Link Power Management. Signed-off-by: Alexandra Yates alexandra.ya...@intel.com --- lsusb.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lsusb.c b/lsusb.c index 4306697..5e92297 100644 --- a/lsusb.c +++ b/lsusb.c @@ -3639,9 +3639,18 @@ static void dump_usb2_device_capability_desc(unsigned char *buf) buf[0], buf[1], buf[2], wide); if (!(wide 0x02)) printf( (Missing must-be-set LPM bit!)\n); - else - printf( Link Power Management (LPM) + else if (!(wide 0x04)) + printf( HIRD Link Power Management (LPM) +Supported\n); + else { + printf( BESL Link Power Management (LPM) Supported\n); + if (wide 0x08) + printf(BESL value%5u us \n, wide 0xf00); + if (wide 0x10) + printf(Deep BESL value%5u us \n, + wide 0xf000); + } } static void dump_ss_device_capability_desc(unsigned char *buf) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] lsusb: Reports if USB2.0 port is on L1 state
From: Alexandra Yates alexandra.ya...@linux.intel.com This patch reports the low power state L1 for ports with attahced USB2 devices. The output will be part of the roothub port status as follows: ayates@magd:~$ sudo lsusb -v -s 001:001 Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub ... Hub Descriptor: bLength 11 bDescriptorType 41 nNbrPorts 9 wHubCharacteristic 0x000a No power switching (usb 1.0) Per-port overcurrent protection TT think time 8 FS bits bPwrOn2PwrGood 10 * 2 milli seconds bHubContrCurrent 0 milli Ampere DeviceRemovable0x68 0x02 PortPwrCtrlMask0xff 0xff Hub Port Status: Port 1: .0103 power enable Port 2: .0523 highspeed power L1 enable Port 3: .0100 power Port 4: .0100 power Port 5: .0503 highspeed power enable Port 6: .0100 power Port 7: .0100 power Port 8: .0100 power Port 9: .0100 power Device Status: 0x0001 In this example port id: .0523 is in L1 status. This output is collected from testing the change on a HSWULT platform. Signed-off-by: Alexandra Yates alexandra.ya...@linux.intel.com --- lsusb.c |1 + 1 file changed, 1 insertion(+) diff --git a/lsusb.c b/lsusb.c index 470146c..4306697 100644 --- a/lsusb.c +++ b/lsusb.c @@ -3317,6 +3317,7 @@ static void do_hub(libusb_device_handle *fd, unsigned tt_type, unsigned speed) (status[1] 0x04) ? highspeed : , (status[1] 0x02) ? lowspeed : , (status[1] 0x01) ? power : , + (status[0] 0x20) ? L1 : , (status[0] 0x10) ? RESET : , (status[0] 0x08) ? oc : , (status[0] 0x04) ? suspend : , -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
USB Gadget:how to test udc with gadget gmidi?
Hi, I am going to test my udc driver with gadget gmidi. And I haven't used it before so I googled how to use it, unfortunately,there seems nothing talk about it. Has anyone else used it or know any info that makes it work? Thanks. Best regards Tong Li -- 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: xHCI ISR be registered twice
Hi Felipe, This patch brings up an interesting point: will a dwc3 PCI host use the xhci platform driver instead of the xhci PCI driver? If so, that seems less than ideal. Won't it not use the standard xHCI PCI suspend and resume functions if it's registered as a platform device? Also, it seems registering it that way means XHCI_BROKEN_MSI is set unconditionally. That leads to the xhci driver not enabling MSI or MSI-X for the PCI host, which will impact performance. [Yu:] The upstream dwc3 driver haven't enable power management for the host mode until now. Actually, this is another big changes. In Intel intern tree, I have to wrote another separate file to re-implemented the PM callback to support platform device design. Maybe we need consider to add one new file(hcd-plat.c) which is familiar with hcd-pci.c? Hi Yu, Thanks for taking this bug down. I have some process-oriented issues that need to be addressed, and some comments that need to be addressed in the next version of your patch. [Yu:] Ok. Thanks for your review. Sorry, I am newcomer of the community. I will setup environment followed the guide and re-submit one new patch. First, please use this email address to send me patches: Sarah Sharp sarah.a.sh...@linux.intel.com I use the linux.intel.com email address to handle all patches and traffic to the Linux mailing lists. My intel.com email address is for Intel internal communications only. In order for us to apply patches, you need to send them inline in the email, without your introduction. The subject line must be the subject line of the patch. Basically, you need to use either `git send-email` or `git format-patch` and mutt to send the patch. Please see this tutorial for more information on sending proper patches: http://kernelnewbies.org/OPWfirstpatch#head-2043a643d048c341b2a52044fd 72d852aac87fef If you still need to introduce a patch, you can put this scissors line between your introduction and the patch description: 8-8 However, in general, your patch description should contain all the information necessary for us to decide whether we need to apply the patch. If the bug was only hit with a specific configuration, that needs to be included in the patch description, so that distros know whether they need to apply the patch. [Yu:] Get it. I will provide more details in the comment. Comments on the patch below. On Tue, Aug 06, 2013 at 11:08:04PM -0700, Wang, Yu Y wrote: Hi Balbi, Sarah, I found that when CONFIG_PCI is set, and xHCI driver register as platform device driver. Then xhci ISR will be register twice. The first time is in usb_add_hcd, the second time is in xhci_run (xhci_try_enable_msi). This issue should be reproduce when dwc3 driver registered as PCI device driver. And CONFIG_USB_DWC3_DUAL_ROLE is set. This information should all be in your patch description. It's important to document how to reproduce the bug you're fixing in the patch that fixes it. Fixed patch please help review: Hopefully when you use `git send-email` or mutt you won't get these extra newlines. Don't send patches through outlook, it completely mangles them. You'll need to set up esmtp on a Linux system in order to be able to send mail to the Intel exchange servers with `git send-email` or mutt. From 8b437ac59be296cd7c0fa189344f3b30281fb3f1 Mon Sep 17 00:00:00 2001 From: Wang, Yu yu.y.w...@intel.com Date: Wed, 7 Aug 2013 13:26:30 +0800 Subject: [PATCH 5/6] xhci: xHCI ISR be registers twice. This is one xHCI driver BUG. If CONFIG_PCI be set and xHCI driver registers as platform driver. Then xHCI ISR will be registers twice, the first time is during usb_add_hcd. The second time is in xhci_run. You'll need a newline between your description and the Signed-off-by line. I can't tell if you currently have one because of the mangled patch. Signed-off-by: Wang, Yu yu.y.w...@intel.com Change-Id: Ibf86bca8f099586a0f379ee843b95c4d93b88d67 --- drivers/usb/host/xhci.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index d8f640b..b43f722 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -372,6 +372,10 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd) } legacy_irq: + /* The leacy irq was already registered in USB core */ + if (hcd-irq) + return 0; + Let's take a look at the function you're patching: static int xhci_try_enable_msi(struct usb_hcd *hcd) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)-self.controller); int ret; /* * Some Fresco Logic host controllers advertise MSI, but