Re: rndis_host: rndis device enumeration
Bin Liu writes: > Hi, > > Sorry if I missed any previous discussion on this topic. > > The msdn link [1] says that > > The host does not continuously poll the USB Control endpoint for input > control messages. Upon placing a control message on its Control > endpoint, the device must return a notification on the Communication > Class interface's Interrupt IN endpoint, which is polled by the host > whenever the device can return control messages. > > Which means the following transfer sequence from the host: > > - SETUP xfer: SET_ENCAPSULATED_COMMAND > - INT xfer > - SETUP xfer: GET_ENCAPSULATED_RESPONSE > > But in rndis_command() in rndis_host.c, the INT xfer between > SET_ENCAPSULATED_COMMAND and GET_ENCAPSULATED_RESPONSE has a if > condition check, so the INT xfer is not sent for some (most) rndis > devices. > > Is there any reason why the INT xfer has this if condition? I don't know the previous discussions either, but I have always assumed this is just due to the usbnet restrictions at the time the code was written. You had to to the sync send + recv commands then, and the interrupt transfer was probably considered unnecessary. Until some devices actually requiring it started showing up and the flag was added. Guessing a lot here... Anyway, I believe this code could be rewritten to do the async commands now that Dan has added the ability to start the usbnet status URB at driver probe time. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [help] imx27 - isp1504 : unable to init transceiver, probably missing
Hi everyone, Sorry for the late answer ! Le 26/09/2013 17:59, Christoph Fritz a écrit : > On Wed, 2013-09-25 at 16:00 +0200, Eric Bénard wrote: >> Le Wed, 25 Sep 2013 12:17:40 +0200, >> Christoph Fritz a écrit : >> >>> On Tue, 2013-04-09 at 14:28 -0300, Fabio Estevam wrote: On Mon, Apr 8, 2013 at 9:09 PM, Fabio Estevam wrote: >> I know that I have to use the driver ULPI but with my configuration, I >> get these errors : >> " >> ehci-mxc: Freescale On-Chip EHCI Host driver >> mxc-ehci mxc-ehci.0: initializing i.MX USB Controller >> timeout polling for ULPI device >> mxc-ehci mxc-ehci.0: unable to init transceiver, probably missing Just tested mx31pdk on a 3.8.6 kernel and I got: ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver ehci-mxc: Freescale On-Chip EHCI Host driver mxc-ehci mxc-ehci.0: initializing i.MX USB Controller ULPI transceiver vendor/product ID 0x04cc/0x1504 Found NXP ISP1504 ULPI transceiver. ULPI integrity check: passed. mxc-ehci mxc-ehci.0: EHCI Host Controller mxc-ehci mxc-ehci.0: new USB bus registered, assigned bus number 1 mxc-ehci mxc-ehci.0: irq 53, io mem 0x43f88000 mxc-ehci mxc-ehci.0: USB 2.0 started, EHCI 1.00 hub 1-0:1.0: USB hub found hub 1-0:1.0: 1 port detected mxc-ehci mxc-ehci.2: initializing i.MX USB Controller timeout polling for ULPI device mxc-ehci mxc-ehci.2: unable to init transceiver, probably missing >>> >>> Any updates on this? For me, no updates about ISP1504. But we wants to change the ISP1504 by the SMSC 3340 to have a reset pin. So we are trying to detect this USB phy with an IMX27 too ! And unfortunately, we have the same problem with the ULPI detection (and tested with different kernel versions). We are having some help with the support to try to know why it is not detected. I will let you know when I have updates but right now, we are still searching why. But we have found that some pins DATA [1 and 6] stay high even after the reset [probably a problem with the SoC configuration ?]. I will let you know if we succeed to make it work, how and why we had these problems ! >>> >>> I'm facing the same kind of issue with an SMSC3340 phy connected to an >>> imx.27: After some minutes in power-off state, the first boot fails to >>> detect the ULPI device connected to USBOTG-Pins, no matter if host- or >>> device-mode is configured. >>> But the strange thing is that then, after a reboot or reset the phy gets >>> detected. You succeeded to get it detected with a reboot ? It is great but strange. >>> >> are you sure some pins on the ULPI interface don't change their level >> between the time where you release the PHY's reset and when the ULPI >> access occurs ? > > I'm pretty sure that there are, but not intended by software I could > control (bootloader+kernel). > > I got a pdf entitled "Using ISP1504 with i.MX27" which is a > Freescale-"Hardware-Bug-Tech-Note" from 2008. It's about the same > problem we are facing here in this thread. To quote their conclusion: > > - i.MX27 HS OTG core is software configurable for ULPI or Serial >transceiver interface > - Default setup at power-up is for Serial transceiver > - This has been found to cause problems with ISP1504 ULPI High- >Speed USB OTG transceiver >- Problems appear rarely and randomly during system power-up >- Final result is that sometimes ISP1504 enters suspend mode during > startup preventing proper initialization via ULPI bus > > Their software fix: > "As first attemp at fixing this issue, one may try to switch LINK to > ULPI mode before configuring MUX and pads for USB operation. Fear is > that this could cause unexpected transfer from PHY to LINK, somehow > preventing proper initialization" > > I'll try that here with the smsc3340-phy. Very interesting ! > > Please drop me a private mail if you want me to send the pdf to you. It would be great if you could send me this pdf ! :) > > Thanks > -- Christoph > Thanks, Best regards, -- Mylène JOSSERAND Navocap
[PATCH] USB: EHCI: tegra: drop clk_put for devm_clk_get in tegra_ehci_probe()
From: Wei Yongjun devm_clk_get() is used so there is no reason to explicitly call clk_put() in probe or remove functions. Signed-off-by: Wei Yongjun --- drivers/usb/host/ehci-tegra.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index 78fa76d..e6d8e26 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -388,7 +388,7 @@ static int tegra_ehci_probe(struct platform_device *pdev) err = clk_prepare_enable(tegra->clk); if (err) - goto cleanup_clk_get; + goto cleanup_hcd_create; tegra_periph_reset_assert(tegra->clk); udelay(1); @@ -465,8 +465,6 @@ cleanup_phy: usb_phy_shutdown(hcd->phy); cleanup_clk_en: clk_disable_unprepare(tegra->clk); -cleanup_clk_get: - clk_put(tegra->clk); cleanup_hcd_create: usb_put_hcd(hcd); return err; -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/51] DMA mask changes
On Thu, Sep 26, 2013 at 10:23:08PM +0200, Rafał Miłecki wrote: > 2013/9/19 Russell King - ARM Linux : > > This email is only being sent to the mailing lists in question, not to > > anyone personally. The list of individuals is far to great to do that. > > I'm hoping no mailing lists reject the patches based on the number of > > recipients. > > Huh, I think it was enough to send only 3 patches to the b43-dev@. Like: > [PATCH 01/51] DMA-API: provide a helper to set both DMA and coherent DMA masks > [PATCH 14/51] DMA-API: net: b43: (...) > [PATCH 15/51] DMA-API: net: b43legacy: (...) > ;) > > I believe Joe has some nice script for doing it that way. When fixing > some coding style / formatting, he sends only related patches to the > given ML. If I did that, then the mailing lists would not get the first patch, because almost none of the lists would be referred to by patch 1. Moreover, people complain when they don't have access to the full patch series - they assume patches are missing for some reason, and they ask for the rest of the series. -- 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: gadget: f_fs: fix error handling
This patch add missing error check in ffs_func_bind() function, after ffs_do_descs() funcion call for hs descriptors. Without this check it's possible that the module will try dereference incorrect pointer. Signed-off-by: Robert Baldyga --- drivers/usb/gadget/f_fs.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 1a66c5b..fe7d532 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -2264,7 +2264,10 @@ static int ffs_func_bind(struct usb_configuration *c, data->raw_descs + ret, (sizeof data->raw_descs) - ret, __ffs_func_bind_do_descs, func); + if (unlikely(ret < 0)) + goto error; } + /* * Now handle interface numbers allocation and interface and -- 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] USB: gadget: epautoconf: fix ep maxpacket check
This patch fixes validation of maxpacket value given in endpoint descriptor. Added check of maxpacket for bulk endpoints. Correct maxpacket value is: FULL-SPEED HIGH-SPEED BULK 64 512 INTERRUPT64 1024 ISOCHRONOUS 10231024 Signed-off-by: Robert Baldyga --- drivers/usb/gadget/epautoconf.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index a777f7b..35bde34 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -136,16 +136,26 @@ ep_matches ( * the usb spec fixes high speed bulk maxpacket at 512 bytes. */ max = 0x7ff & usb_endpoint_maxp(desc); + + if (ep->maxpacket < max) + return 0; + switch (type) { + case USB_ENDPOINT_XFER_BULK: + /* BULK: limit 512 high/super speed */ + if (max > 512) + return 0; + /* FALLTHROUGH */ + case USB_ENDPOINT_XFER_INT: - /* INT: limit 64 bytes full speed, 1024 high/super speed */ + /* BULK/INT: limit 64 bytes full speed */ if (!gadget_is_dualspeed(gadget) && max > 64) return 0; /* FALLTHROUGH */ case USB_ENDPOINT_XFER_ISOC: - /* ISO: limit 1023 bytes full speed, 1024 high/super speed */ - if (ep->maxpacket < max) + /* INT/ISO: limit 1023 bytes full speed, 1024 high/super speed */ + if (max > 1024) return 0; if (!gadget_is_dualspeed(gadget) && max > 1023) return 0; -- 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] USB: gadget: s3c-hsotg: fix maxpacket size
This patch changes ep maxpacket value from 512 to 1024, becouse it's needed to handle interupt and isochronous endpoints in high-speed mode. This change doesn't affect on driver functioning, becouse fifo size (3072) is still enough for the maximum transaction payload (3*1024 for high-speed high-bandwidtch endpoints). Signed-off-by: Robert Baldyga --- drivers/usb/gadget/s3c-hsotg.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c index 8a9e2c6..0d1dcbd 100644 --- a/drivers/usb/gadget/s3c-hsotg.c +++ b/drivers/usb/gadget/s3c-hsotg.c @@ -3146,7 +3146,7 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg, hs_ep->parent = hsotg; hs_ep->ep.name = hs_ep->name; - hs_ep->ep.maxpacket = epnum ? 512 : EP0_MPS_LIMIT; + hs_ep->ep.maxpacket = epnum ? 1024 : EP0_MPS_LIMIT; hs_ep->ep.ops = &s3c_hsotg_ep_ops; /* -- 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/1] USB: EHCI: tegra: Remove incorrect clk_put
tegra->clk is obtained using devm_clk_get(). Hence clk_put should not be used. Signed-off-by: Sachin Kamat Cc: Stephen Warren --- drivers/usb/host/ehci-tegra.c |4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index 78fa76d..e6d8e26 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -388,7 +388,7 @@ static int tegra_ehci_probe(struct platform_device *pdev) err = clk_prepare_enable(tegra->clk); if (err) - goto cleanup_clk_get; + goto cleanup_hcd_create; tegra_periph_reset_assert(tegra->clk); udelay(1); @@ -465,8 +465,6 @@ cleanup_phy: usb_phy_shutdown(hcd->phy); cleanup_clk_en: clk_disable_unprepare(tegra->clk); -cleanup_clk_get: - clk_put(tegra->clk); cleanup_hcd_create: usb_put_hcd(hcd); return err; -- 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] USB: gadget: f_fs: fix error handling
On Fri, Sep 27 2013, Robert Baldyga wrote: > This patch add missing error check in ffs_func_bind() function, after > ffs_do_descs() funcion call for hs descriptors. Without this check it's > possible that the module will try dereference incorrect pointer. > > Signed-off-by: Robert Baldyga Acked-by: Michal Nazarewicz > --- > drivers/usb/gadget/f_fs.c |3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c > index 1a66c5b..fe7d532 100644 > --- a/drivers/usb/gadget/f_fs.c > +++ b/drivers/usb/gadget/f_fs.c > @@ -2264,7 +2264,10 @@ static int ffs_func_bind(struct usb_configuration *c, > data->raw_descs + ret, > (sizeof data->raw_descs) - ret, > __ffs_func_bind_do_descs, func); > + if (unlikely(ret < 0)) > + goto error; > } > + This new line with only a tab in it is not needed here though. > > /* >* Now handle interface numbers allocation and interface and -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Re: [PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver
Manjunath, Manjunath Goudar writes: > Separate the TI OHCI Atmel host controller driver from ohci-hcd > host code so that it can be built as a separate driver module. > This work is part of enabling multi-platform kernels on ARM. This broke booting on atmel sama5 boards (and likely others with the same conversion)... [...] > +static int __init ohci_at91_init(void) > +{ > + if (usb_disabled()) > + return -ENODEV; > + > + pr_info("%s: " DRIVER_DESC "\n", hcd_name); > + ohci_init_driver(&ohci_at91_hc_driver, NULL); ohci_init_driver() doesn't have any sanity checks for NULL overrides, so it blindly dereferences and faults. Some of the other conversions have this same problem (at least omap3). Did anyone test this series on hardware? I'm not too familiar with OHCI, but something like the patch below is probably needed along with this series. Kevin >From a3b5cc90e74038a6449fbd25e0d720ea02884f30 Mon Sep 17 00:00:00 2001 From: Kevin Hilman Date: Fri, 27 Sep 2013 08:07:19 -0700 Subject: [PATCH] USB: OHCI: ohci_init_driver(): sanity check overrides Check for non-NULL overrides before dereferencing since platforms may pass in NULL overrides. Signed-off-by: Kevin Hilman --- drivers/usb/host/ohci-hcd.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 21d937a..8ada13f 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1161,10 +1161,12 @@ void ohci_init_driver(struct hc_driver *drv, /* Copy the generic table to drv and then apply the overrides */ *drv = ohci_hc_driver; - drv->product_desc = over->product_desc; - drv->hcd_priv_size += over->extra_priv_size; - if (over->reset) - drv->reset = over->reset; + if (over) { + drv->product_desc = over->product_desc; + drv->hcd_priv_size += over->extra_priv_size; + if (over->reset) + drv->reset = over->reset; + } } EXPORT_SYMBOL_GPL(ohci_init_driver); -- 1.8.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
'ci_hdrc.0: failed to start' when using gadgetfs
Hi, I am using mx53 (which uses the chipidea driver) and it works fine with g_ether. Now, I need to test gadgetfs and this is what I get: $ modprobe gadgetfs gadgetfs: USB Gadget filesystem, version 24 Aug 2004 $ mkdir /dev/gadget $ mount -t gadgetfs none /dev/gadget nop ci_hdrc.0: failed to start (null): -120 This error message comes from: ret = driver->bind(udc->gadget, driver); inside udc_bind_to_driver() from udc_core.c Any idea as to why driver->bind fails? I will start debugging it, but would appreciate any suggestions. Thanks, Fabio Estevan -- 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/3] usb: Disable USB 2.0 Link PM before device reset.
From: Sarah Sharp Before the USB core resets a device, we need to disable the L1 timeout for the roothub, if USB 2.0 Link PM is enabled. Otherwise the port may transition into L1 in between descriptor fetches, before we know if the USB device descriptors changed. LPM will be re-enabled after the full device descriptors are fetched, and we can confirm the device still supports USB 2.0 LPM after the reset. We don't need to wait for the USB device to exit L1 before resetting the device, since the xHCI roothub port diagrams show a transition to the Reset state from any of the Ux states (see Figure 34 in the 2012-08-14 xHCI specification update). Signed-off-by: Sarah Sharp --- drivers/usb/core/hub.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index e6b682c..cfb31a2 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -5086,6 +5086,12 @@ static int usb_reset_and_verify_device(struct usb_device *udev) } parent_hub = usb_hub_to_struct_hub(parent_hdev); + /* Disable USB2 hardware LPM. +* It will be re-enabled by the enumeration process. +*/ + if (udev->usb2_hw_lpm_enabled == 1) + usb_set_usb2_hardware_lpm(udev, 0); + bos = udev->bos; udev->bos = NULL; -- 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/3] USB 2.0 Link PM broken (updated series)
Add a patch to enable Link PM for hardwired devices and BESL capable devices as default. Otherwise this is the same series by Sarah that disables usb2 LPM as default for devices because usb3 devices may falsely claim usb2 LPM support when connected to a usb2 port. Previous series with better cover letter is found here: http://marc.info/?l=linux-usb&m=138005811328073&w=2 Mathias Nyman (1): xhci: Enable LPM support only for hardwired or BESL devices Sarah Sharp (2): usb: Don't enable USB 2.0 Link PM by default. usb: Disable USB 2.0 Link PM before device reset. drivers/usb/core/driver.c |3 + drivers/usb/core/hub.c |6 ++ drivers/usb/core/sysfs.c|1 + drivers/usb/host/xhci-mem.c | 10 --- drivers/usb/host/xhci.c | 163 ++- include/linux/usb.h |4 +- 6 files changed, 35 insertions(+), 152 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 3/3] xhci: Enable LPM support only for hardwired or BESL devices
Some usb3 devices falsely claim they support usb2 hardware Link PM when connected to a usb2 port. We only trust hardwired devices or devices with the later BESL LPM support to be LPM enabled as default. Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index b554eba..f9e5a0a 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -31,6 +31,7 @@ #include "xhci.h" #include "xhci-trace.h" +#include "../core/usb.h" #define DRIVER_AUTHOR "Sarah Sharp" #define DRIVER_DESC "'eXtensible' Host Controller (xHC) Driver" @@ -4141,6 +4142,7 @@ int xhci_update_device(struct usb_hcd *hcd, struct usb_device *udev) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); int portnum = udev->portnum - 1; + int ret, connect_type; if (xhci->hw_lpm_support == 1 && xhci_check_usb2_port_capability( @@ -4151,8 +4153,21 @@ int xhci_update_device(struct usb_hcd *hcd, struct usb_device *udev) if (xhci_check_usb2_port_capability(xhci, portnum, XHCI_BLC)) udev->usb2_hw_lpm_besl_capable = 1; - } + connect_type = usb_get_hub_port_connect_type(udev->parent, +udev->portnum); + /* Some usb3 devices falsely claim to be LPM capable when +* connected to a usb2 port. Only trust hardwired usb2 +* devices and devices with BESL support to actually work +*/ + if (udev->usb2_hw_lpm_besl_capable || + connect_type == USB_PORT_CONNECT_TYPE_HARD_WIRED) { + udev->usb2_hw_lpm_allowed = 1; + ret = xhci_set_usb2_hardware_lpm(hcd, udev, 1); + if (!ret) + udev->usb2_hw_lpm_enabled = 1; + } + } return 0; } -- 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/3] usb: Don't enable USB 2.0 Link PM by default.
From: Sarah Sharp How it's supposed to work: -- USB 2.0 Link PM is a lower power state that some newer USB 2.0 devices support. USB 3.0 devices certified by the USB-IF are required to support it if they are plugged into a USB 2.0 only port, or a USB 2.0 cable is used. USB 2.0 Link PM requires both a USB device and a host controller that supports USB 2.0 hardware-enabled LPM. USB 2.0 Link PM is designed to be enabled once by software, and the host hardware handles transitions to the L1 state automatically. The premise of USB 2.0 Link PM is to be able to put the device into a lower power link state when the bus is idle or the device NAKs USB IN transfers for a specified amount of time. ...but hardware is broken: -- It turns out many USB 3.0 devices claim to support USB 2.0 Link PM (by setting the LPM bit in their USB 2.0 BOS descriptor), but they don't actually implement it correctly. This manifests as the USB device refusing to respond to transfers when it is plugged into a USB 2.0 only port under the Haswell-ULT/Lynx Point LP xHCI host. These devices pass the xHCI driver's simple test to enable USB 2.0 Link PM, wait for the port to enter L1, and then bring it back into L0. They only start to break when L1 entry is interleaved with transfers. Some devices then fail to respond to the next control transfer (usually a Set Configuration). This results in devices never enumerating. Other mass storage devices (such as a later model Western Digital My Passport USB 3.0 hard drive) respond fine to going into L1 between control transfers. They ACK the entry, come out of L1 when the host needs to send a control transfer, and respond properly to those control transfers. However, when the first READ10 SCSI command is sent, the device NAKs the data phase while it's reading from the spinning disk. Eventually, the host requests to put the link into L1, and the device ACKs that request. Then it never responds to the data phase of the READ10 command. This results in not being able to read from the drive. Some mass storage devices (like the Corsair Survivor USB 3.0 flash drive) are well behaved. They ACK the entry into L1 during control transfers, and when SCSI commands start coming in, they NAK the requests to go into L1, because they need to be at full power. Not all USB 3.0 devices advertise USB 2.0 link PM support. My Point Grey USB 3.0 webcam advertises itself as a USB 2.1 device, but doesn't have a USB 2.0 BOS descriptor, so we don't enable USB 2.0 Link PM. I suspect that means the device isn't certified. What do we do about it? --- There's really no good way for the kernel to test these devices. Therefore, the kernel needs to disable USB 2.0 Link PM by default, and distros will have to enable it by writing 1 to the sysfs file /sys/bus/usb/devices/../power/usb2_hardware_lpm. Rip out the xHCI Link PM test, since it's not sufficient to detect these buggy devices, and don't automatically enable LPM after the device is addressed. This patch should be backported to kernels as old as 3.11, that contain the commit a558ccdcc71c7770c5e80c926a31cfe8a3892a09 "usb: xhci: add USB2 Link power management BESL support". Without this fix, some USB 3.0 devices will not enumerate or work properly under USB 2.0 ports on Haswell-ULT systems. Signed-off-by: Sarah Sharp Cc: sta...@vger.kernel.org --- drivers/usb/core/driver.c |3 + drivers/usb/core/sysfs.c|1 + drivers/usb/host/xhci-mem.c | 10 --- drivers/usb/host/xhci.c | 152 +++ include/linux/usb.h |4 +- 5 files changed, 16 insertions(+), 154 deletions(-) diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index f7841d4..689433c 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -1790,6 +1790,9 @@ int usb_set_usb2_hardware_lpm(struct usb_device *udev, int enable) struct usb_hcd *hcd = bus_to_hcd(udev->bus); int ret = -EPERM; + if (enable && !udev->usb2_hw_lpm_allowed) + return 0; + if (hcd->driver->set_usb2_hw_lpm) { ret = hcd->driver->set_usb2_hw_lpm(hcd, udev, enable); if (!ret) diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index 6d2c8ed..76bce3a 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -469,6 +469,7 @@ static ssize_t usb2_hardware_lpm_store(struct device *dev, ret = strtobool(buf, &value); + udev->usb2_hw_lpm_allowed = value; if (!ret) ret = usb_set_usb2_hardware_lpm(udev, value); diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 83bcd13..49b8bd0 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1693,9 +1693,7 @@ void xhci_free_command(struct xhci_hcd *xhci, void xhci_mem_cleanup(struct xhci_hcd *xhci) { struct pci_dev *pdev = to_pci_dev(xhc
Re: [PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver
On Fri, Sep 27, 2013 at 08:10:32AM -0700, Kevin Hilman wrote: > Manjunath, > > Manjunath Goudar writes: > > > Separate the TI OHCI Atmel host controller driver from ohci-hcd > > host code so that it can be built as a separate driver module. > > This work is part of enabling multi-platform kernels on ARM. > > This broke booting on atmel sama5 boards (and likely others with the > same conversion)... Crap. And it looks like Manjunath is now no longer working at Linaro, so these patches are "abandoned" :( Unless someone at Linaro steps up in the next few days to fix these, I'll have to go revert them all. Again. bleah. 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
Re: [PATCH] Memory mapping for USBFS
On Thu, Sep 26, 2013 at 2:13 AM, Greg KH wrote: > On Tue, Sep 24, 2013 at 09:12:39PM +0200, Markus Rechberger wrote: >> This patch adds memory mapping support to USBFS for isochronous and bulk >> data transfers, it allows to pre-allocate usb transfer buffers. > > Does libusb support this? > >> The CPU usage decreases 1-2% on my 1.3ghz U7300 notebook when >> transferring 20mbyte/sec, it should be more interesting to see those >> statistics on embedded systems where copying data is more expensive. > > Any chance you could test this on a system with a "smaller" processor to > see if that really is the case or not? > just tested it on an Acer Aspire One Netbook. Intel Atom N270 1.6ghz, but running at 800mhz. The CPU Usage went down from 24% to 14-18% (transferring 20Mbyte/sec) Markus -- 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: gadget: epautoconf: fix ep maxpacket check
On Fri, 27 Sep 2013, Robert Baldyga wrote: > This patch fixes validation of maxpacket value given in endpoint descriptor. > Added check of maxpacket for bulk endpoints. > Correct maxpacket value is: > > FULL-SPEED HIGH-SPEED > BULK 64 512 Don't forget SuperSpeed. It requires Bulk endpoints to have maxpacket = 1024. > INTERRUPT64 1024 > ISOCHRONOUS 10231024 > > Signed-off-by: Robert Baldyga > --- > drivers/usb/gadget/epautoconf.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c > index a777f7b..35bde34 100644 > --- a/drivers/usb/gadget/epautoconf.c > +++ b/drivers/usb/gadget/epautoconf.c > @@ -136,16 +136,26 @@ ep_matches ( >* the usb spec fixes high speed bulk maxpacket at 512 bytes. >*/ > max = 0x7ff & usb_endpoint_maxp(desc); > + > + if (ep->maxpacket < max) > + return 0; > + > switch (type) { > + case USB_ENDPOINT_XFER_BULK: > + /* BULK: limit 512 high/super speed */ 512 high, 1024 super. > + if (max > 512) > + return 0; > + /* FALLTHROUGH */ > + > case USB_ENDPOINT_XFER_INT: > - /* INT: limit 64 bytes full speed, 1024 high/super speed */ > + /* BULK/INT: limit 64 bytes full speed */ > if (!gadget_is_dualspeed(gadget) && max > 64) > return 0; > /* FALLTHROUGH */ > > case USB_ENDPOINT_XFER_ISOC: > - /* ISO: limit 1023 bytes full speed, 1024 high/super speed */ > - if (ep->maxpacket < max) > + /* INT/ISO: limit 1023 bytes full speed, 1024 high/super speed > */ The comment mentions INT/ISO. But this code can also run in the BULK case. > + if (max > 1024) > return 0; > if (!gadget_is_dualspeed(gadget) && max > 1023) > return 0; I suspect this code would become a lot clearer if all the FALLTHROUGH logic were eliminated and each case was handled on its own. 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: 'ci_hdrc.0: failed to start' when using gadgetfs
Dear Fabio Estevam, > Hi, > > I am using mx53 (which uses the chipidea driver) and it works fine with > g_ether. > > Now, I need to test gadgetfs and this is what I get: > > $ modprobe gadgetfs > gadgetfs: USB Gadget filesystem, version 24 Aug 2004 > > $ mkdir /dev/gadget > $ mount -t gadgetfs none /dev/gadget > nop ci_hdrc.0: failed to start (null): -120 > > This error message comes from: > > ret = driver->bind(udc->gadget, driver); > > inside udc_bind_to_driver() from udc_core.c > > Any idea as to why driver->bind fails? > > I will start debugging it, but would appreciate any suggestions. gadgetfs_probe() returns -EISNAM unconditionally. That's rather strange. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver
On Fri, 27 Sep 2013, Greg KH wrote: > On Fri, Sep 27, 2013 at 08:10:32AM -0700, Kevin Hilman wrote: > > Manjunath, > > > > Manjunath Goudar writes: > > > > > Separate the TI OHCI Atmel host controller driver from ohci-hcd > > > host code so that it can be built as a separate driver module. > > > This work is part of enabling multi-platform kernels on ARM. > > > > This broke booting on atmel sama5 boards (and likely others with the > > same conversion)... > > Crap. > > And it looks like Manjunath is now no longer working at Linaro, so these > patches are "abandoned" :( > > Unless someone at Linaro steps up in the next few days to fix these, > I'll have to go revert them all. Again. > > bleah. It's not so bad as all that. The patch Kevin wrote duplicates what ehci-hcd does. It's an obvious oversight; I need to work harder at keeping the two drivers in sync. 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 V5 5/9] USB: OHCI: make ohci-at91 a separate driver
On Fri, 27 Sep 2013, Kevin Hilman wrote: > Manjunath, > > Manjunath Goudar writes: > > > Separate the TI OHCI Atmel host controller driver from ohci-hcd > > host code so that it can be built as a separate driver module. > > This work is part of enabling multi-platform kernels on ARM. > > This broke booting on atmel sama5 boards (and likely others with the > same conversion)... > > [...] > > > +static int __init ohci_at91_init(void) > > +{ > > + if (usb_disabled()) > > + return -ENODEV; > > + > > + pr_info("%s: " DRIVER_DESC "\n", hcd_name); > > + ohci_init_driver(&ohci_at91_hc_driver, NULL); > > ohci_init_driver() doesn't have any sanity checks for NULL overrides, so > it blindly dereferences and faults. > > Some of the other conversions have this same problem (at least omap3). > Did anyone test this series on hardware? > > I'm not too familiar with OHCI, but something like the patch below is > probably needed along with this series. > > Kevin > > > From a3b5cc90e74038a6449fbd25e0d720ea02884f30 Mon Sep 17 00:00:00 2001 > From: Kevin Hilman > Date: Fri, 27 Sep 2013 08:07:19 -0700 > Subject: [PATCH] USB: OHCI: ohci_init_driver(): sanity check overrides > > Check for non-NULL overrides before dereferencing since platforms may > pass in NULL overrides. > > Signed-off-by: Kevin Hilman > --- > drivers/usb/host/ohci-hcd.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c > index 21d937a..8ada13f 100644 > --- a/drivers/usb/host/ohci-hcd.c > +++ b/drivers/usb/host/ohci-hcd.c > @@ -1161,10 +1161,12 @@ void ohci_init_driver(struct hc_driver *drv, > /* Copy the generic table to drv and then apply the overrides */ > *drv = ohci_hc_driver; > > - drv->product_desc = over->product_desc; > - drv->hcd_priv_size += over->extra_priv_size; > - if (over->reset) > - drv->reset = over->reset; > + if (over) { > + drv->product_desc = over->product_desc; > + drv->hcd_priv_size += over->extra_priv_size; > + if (over->reset) > + drv->reset = over->reset; > + } > } > EXPORT_SYMBOL_GPL(ohci_init_driver); Yes, this is exactly what ehci-hcd does and it clearly is necessary. Acked-by: 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 V5 5/9] USB: OHCI: make ohci-at91 a separate driver
On Fri, Sep 27, 2013 at 11:37:16AM -0400, Alan Stern wrote: > On Fri, 27 Sep 2013, Greg KH wrote: > > > On Fri, Sep 27, 2013 at 08:10:32AM -0700, Kevin Hilman wrote: > > > Manjunath, > > > > > > Manjunath Goudar writes: > > > > > > > Separate the TI OHCI Atmel host controller driver from ohci-hcd > > > > host code so that it can be built as a separate driver module. > > > > This work is part of enabling multi-platform kernels on ARM. > > > > > > This broke booting on atmel sama5 boards (and likely others with the > > > same conversion)... > > > > Crap. > > > > And it looks like Manjunath is now no longer working at Linaro, so these > > patches are "abandoned" :( > > > > Unless someone at Linaro steps up in the next few days to fix these, > > I'll have to go revert them all. Again. > > > > bleah. > > It's not so bad as all that. The patch Kevin wrote duplicates what > ehci-hcd does. It's an obvious oversight; I need to work harder at > keeping the two drivers in sync. Ah, ok, so Kevin's patch should resolve it all? That's good, I'll go queue it up. 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
Re: 'ci_hdrc.0: failed to start' when using gadgetfs
On Fri, 27 Sep 2013, Marek Vasut wrote: > Dear Fabio Estevam, > > > Hi, > > > > I am using mx53 (which uses the chipidea driver) and it works fine with > > g_ether. > > > > Now, I need to test gadgetfs and this is what I get: > > > > $ modprobe gadgetfs > > gadgetfs: USB Gadget filesystem, version 24 Aug 2004 > > > > $ mkdir /dev/gadget > > $ mount -t gadgetfs none /dev/gadget > > nop ci_hdrc.0: failed to start (null): -120 > > > > This error message comes from: > > > > ret = driver->bind(udc->gadget, driver); > > > > inside udc_bind_to_driver() from udc_core.c > > > > Any idea as to why driver->bind fails? > > > > I will start debugging it, but would appreciate any suggestions. > > gadgetfs_probe() returns -EISNAM unconditionally. That's rather strange. It is a temporary measure, used only when the file system is set up initially. The real bind routine is gadgetfs_bind(), which gets called when userspace configures the gadget. In short, this is how it is intended to work. It isn't a bug. 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: EHCI bus glue driver works for storage, fails for a WiFi module
On Fri, 27 Sep 2013, Arokux X wrote: > I have done something simple, but reliable now. I have stripped the > vendors driver, so that it had only ~230 LOC. This driver was still > fully functional and wifi module worked. Then I carefully implanted it > into the mainline kernel, making only minimal changes to it. > Afterwards I tried to > > ip link set wlan0 up > > and saw exactly same errors in kernel log as with my driver (lots of > detected XactErr len 0/0 retry). This suggest the problem might be > elsewhere, for example in the wifi driver. To eliminate this unknown > I've used the wifi driver from 3.10 backports (not the latest > mainline) in the vendors tree (it is based on 3.4). The wifi module > still worked. So again - the problem is elsewhere. This time however I > do not know what to look for. I hope you can help. I don't know. Did you run all these tests on the same computer? What happens if you back-port your glue driver to the vendor's kernel? > The ARM SoC I am working on is Allwinner A10 aka sun4i. It belongs to > sunxi family. Not very much was mainlined so far. Some people have > suggested the missing DMA bits can be what causes the problem. I have > discovered the usb stack uses DMA too, since I assign dma_mask. The > vendors DMA driver that is used for my SoC is here [0]. > > Do you think the absence of the DMA support is what causing the > problem? Can I somehow check it? If not, what else could be? I doubt that DMA is the problem. If it were, the driver probably wouldn't work at all. > Additionally I have gathered logs with usbmon. The working.mon.out [1] > is from the legacy 3.4 vendor's kernel. The not-working.mon.out [2] is > the output I get with my driver. Maybe these logs can give some clue > too. I'm not familiar with the USB protocol used for WiFi, so I can't interpret the logs. As far as I can see, they are essentially the same. This isn't surprising. The errors you are getting are hardware errors, not protocol errors. They could be caused by excessive noise in the USB data lines. Or there could be some sort of timing issue. > > The line where sunxi_ehci_init_module() assigns a string to > > sunxi_ehci_hc_driver.product_desc should be removed. > > I have found some discussion about this and now I understand why it > should be removed. Originally I was confused because OHCI overrides > allow you to specify product_desc. Now I assume it is a historical > leftover. Perhaps you want to unify this with EHCI drivers? If so, I > could submit a patch. Yes, ohci-hcd needs to be changed. I'll take care of it later on. > > What is the meaning of the "FIXME: Should there be two of those?" > > comment on line 215? Two of what? > > Two instances of the structure hc_driver. So rephrasing, is one > instance of the hc_driver enough to manage two EHCI controllers? One instance of hc_driver is enough to manage any number of controllers. It describes the driver, not the device -- one driver can work with any number of devices. > The new code with applied corrections can be found at [3], the whole > branch with dt bindings at [4]. Once the wifi problem is solved and > enough usb devices are tested I'll submit patches. > > Best regards, > Arokux > > [0] > https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/arch/arm/plat-sunxi/dma.c > [1] https://www.dropbox.com/s/lpvp4jhsm4ki2tv/working.mon.out > [2] https://www.dropbox.com/s/hdu0dfdowofx737/not-working.mon.out > [3] > https://github.com/arokux/linux/commit/c86e622367769173f8cf0e1af132d7ae9b3ee727 > [4] https://github.com/arokux/linux 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 10/27] mmc: vub300: Remove redundant suspend and resume callbacks
On Thu, 26 Sep 2013, Ulf Hansson wrote: > Suspend and resume of cards are handled by the protocol layer and > consequently the mmc_suspend|resume_host APIs are marked as deprecated. > > While moving away from using the deprecated APIs, there are nothing > left to be done for the suspend and resume callbacks, so remove them. > > Cc: Tony Olech > Cc: linux-usb@vger.kernel.org > Signed-off-by: Ulf Hansson > --- > drivers/mmc/host/vub300.c | 30 -- > 1 file changed, 30 deletions(-) > > diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c > index e9028ad..db99edc 100644 > --- a/drivers/mmc/host/vub300.c > +++ b/drivers/mmc/host/vub300.c > @@ -2389,34 +2389,6 @@ static void vub300_disconnect(struct usb_interface > *interface) > } > } > > -#ifdef CONFIG_PM > -static int vub300_suspend(struct usb_interface *intf, pm_message_t message) > -{ > - struct vub300_mmc_host *vub300 = usb_get_intfdata(intf); > - if (!vub300 || !vub300->mmc) { > - return 0; > - } else { > - struct mmc_host *mmc = vub300->mmc; > - mmc_suspend_host(mmc); > - return 0; > - } > -} > - > -static int vub300_resume(struct usb_interface *intf) > -{ > - struct vub300_mmc_host *vub300 = usb_get_intfdata(intf); > - if (!vub300 || !vub300->mmc) { > - return 0; > - } else { > - struct mmc_host *mmc = vub300->mmc; > - mmc_resume_host(mmc); > - return 0; > - } > -} > -#else > -#define vub300_suspend NULL > -#define vub300_resume NULL > -#endif > static int vub300_pre_reset(struct usb_interface *intf) > {/* NOT irq */ > struct vub300_mmc_host *vub300 = usb_get_intfdata(intf); > @@ -2437,8 +2409,6 @@ static struct usb_driver vub300_driver = { > .name = "vub300", > .probe = vub300_probe, > .disconnect = vub300_disconnect, > - .suspend = vub300_suspend, > - .resume = vub300_resume, > .pre_reset = vub300_pre_reset, > .post_reset = vub300_post_reset, > .id_table = vub300_table, You shouldn't do it this way. The USB core treats drivers differently depending on whether their suspend and resume callbacks are defined. If those method pointers are NULL, the driver will be treated as though it doesn't support power management at all. You should keep the pointers and the routines. The contents of the routines can be removed, leaving nothing but a "return 0;" line. 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: 'ci_hdrc.0: failed to start' when using gadgetfs
Hi Alan, On Fri, Sep 27, 2013 at 12:44 PM, Alan Stern wrote: > It is a temporary measure, used only when the file system is set up > initially. The real bind routine is gadgetfs_bind(), which gets called > when userspace configures the gadget. > > In short, this is how it is intended to work. It isn't a bug. Thanks for your explanation. Should we change this message to dev_dbg? --- a/drivers/usb/gadget/udc-core.c +++ b/drivers/usb/gadget/udc-core.c @@ -356,7 +356,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); return 0; err1: - dev_err(&udc->dev, "failed to start %s: %d\n", + dev_dbg(&udc->dev, "failed to start %s: %d\n", udc->driver->function, ret); udc->driver = NULL; udc->dev.driver = NULL; Thanks, Fabio Estevam -- 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: 'ci_hdrc.0: failed to start' when using gadgetfs
On Fri, 27 Sep 2013, Fabio Estevam wrote: > Hi Alan, > > On Fri, Sep 27, 2013 at 12:44 PM, Alan Stern > wrote: > > > It is a temporary measure, used only when the file system is set up > > initially. The real bind routine is gadgetfs_bind(), which gets called > > when userspace configures the gadget. > > > > In short, this is how it is intended to work. It isn't a bug. > > Thanks for your explanation. > > Should we change this message to dev_dbg? Or maybe just don't report EISNAM as an error at all. 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
[PATCH] gadget: udc-core: Ignore -EISNAM error from gadgetfs
From: Fabio Estevam When mounting a gadgetfs the following error message is seen: $ modprobe gadgetfs gadgetfs: USB Gadget filesystem, version 24 Aug 2004 $ mkdir /dev/gadget $ mount -t gadgetfs none /dev/gadget nop ci_hdrc.0: failed to start (null): -120 The error comes from gadgetfs_probe(), which returns -EISNAM (-120). As Alan Stern explains[1] this is the normal behavior: "It is a temporary measure, used only when the file system is set up initially. The real bind routine is gadgetfs_bind(), which gets called when userspace configures the gadget. In short, this is how it is intended to work. It isn't a bug." [1] http://marc.info/?l=linux-usb&m=138029668707075&w=2 So in order to prevent the error message, do not report EISNAM as an error. Signed-off-by: Fabio Estevam --- drivers/usb/gadget/udc-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c index 59891b1..0887e15 100644 --- a/drivers/usb/gadget/udc-core.c +++ b/drivers/usb/gadget/udc-core.c @@ -344,7 +344,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri udc->gadget->dev.driver = &driver->driver; ret = driver->bind(udc->gadget, driver); - if (ret) + if (ret && ret != -EISNAM) goto err1; ret = usb_gadget_udc_start(udc->gadget, driver); if (ret) { -- 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] USB: gadget: f_fs: fix error handling
Hello. On 09/27/2013 02:28 PM, Robert Baldyga wrote: This patch add missing error check in ffs_func_bind() function, after ffs_do_descs() funcion call for hs descriptors. Without this check it's s/funcion/function/. Perhaps it's worth expanding "hs" to "high speed" for clarity? possible that the module will try dereference incorrect pointer. Signed-off-by: Robert Baldyga --- drivers/usb/gadget/f_fs.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 1a66c5b..fe7d532 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -2264,7 +2264,10 @@ static int ffs_func_bind(struct usb_configuration *c, data->raw_descs + ret, (sizeof data->raw_descs) - ret, __ffs_func_bind_do_descs, func); + if (unlikely(ret < 0)) + goto error; } + Why add second empty line here? /* * Now handle interface numbers allocation and interface and 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: [PATCH 3/3] xhci: Enable LPM support only for hardwired or BESL devices
> +#include "../core/usb.h" You might want to move usb_get_hub_port_connect_type() to include/linux/usb.h instead of doing this. Also, I think you need to mark it EXPORT_SYMBOL_GPL in core/hub.c or you could run into trouble when both xhci-hcd and usbcore are compiled as modules. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode
> From: linux-usb-ow...@vger.kernel.org > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Manu Gautam > Sent: Thursday, September 26, 2013 12:08 AM > > On 9/26/2013 2:10 AM, Felipe Balbi wrote: > > > > On Tue, Sep 24, 2013 at 03:00:20PM +0530, Manu Gautam wrote: > >> Hi Felipe, > >> > >> I wanted to mention one point with respect to this patch: Below > >> changes in the functionfs.h to add ss_count (super speed descriptors > >> count) in desc_header (which is passed from userspace) make the driver > >> incompatible with existing userspace applications compiled against old > >> header file. Let me know if that is acceptable. We are using this > >> driver with Android for adbd (android debug bridge) and these changes > >> are required to support adb over Super Speed controllers e.g. DWC3 > >> along with changed in adbd to pass SS EP and companion descriptors. > > > > Good you mentioned, it saves me the trouble of reviewing this patch :-) > > > > It's not acceptable to break userspace ABI at all. If you want > > SuperSpeed support on function fs, we need to figure out a way to do so > > without breaking userspace. > > > > This might mean adding a separate userspace interface to be used with > > superspeed. While at that, we might want to add a few bytes of reserved, > > unused space in our structures for situations where we need to add more > > data into it, just to make it slightly future proof. > > > > Thanks for your reply. > As you suggested we can have a different interface for super speed > which would be optional to workaround ABI compatibility issue. > Let me know if below interface looks fine to you, I will then implement > accordingly: Just a suggestion: Instead of a new interface for SuperSpeed, why not just add the new fields to the end of the ffs_data struct? And have the functions that copy the struct to/from userspace check the 'len' value passed in, and only handle the SuperSpeed stuff if the length indicates it is new userspace? -- Paul -- 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] usb: wusbcore: transfer ID endianness fixes
Hi, This set of patches fixes a few places where the endiannes was not being converted for the dwTransferID field of transfer requests and transfer results. It also adds the xfer ID to some debug prints to aid in bus log analysis. Thomas Pugliese (2): fix endianess issues when using dwTransferID include the xfer_id in debug prints for transfers. drivers/usb/wusbcore/wa-xfer.c | 98 +--- 1 file changed, 52 insertions(+), 46 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: wusbcore: include the xfer_id in debug prints
Include the xfer_id in debug prints for transfers and transfer segments. This makes it much easier to correlate debug logs to USB analyzer logs. Signed-off-by: Thomas Pugliese --- drivers/usb/wusbcore/wa-xfer.c | 94 +--- 1 file changed, 49 insertions(+), 45 deletions(-) diff --git a/drivers/usb/wusbcore/wa-xfer.c b/drivers/usb/wusbcore/wa-xfer.c index 9464f20d..9b6501e 100644 --- a/drivers/usb/wusbcore/wa-xfer.c +++ b/drivers/usb/wusbcore/wa-xfer.c @@ -238,6 +238,31 @@ static void wa_xfer_completion(struct wa_xfer *xfer) } /* + * Initialize a transfer's ID + * + * We need to use a sequential number; if we use the pointer or the + * hash of the pointer, it can repeat over sequential transfers and + * then it will confuse the HWAwonder why in hell they put a 32 + * bit handle in there then. + */ +static void wa_xfer_id_init(struct wa_xfer *xfer) +{ + xfer->id = atomic_add_return(1, &xfer->wa->xfer_id_count); +} + +/* Return the xfer's ID. */ +static inline u32 wa_xfer_id(struct wa_xfer *xfer) +{ + return xfer->id; +} + +/* Return the xfer's ID in transport format (little endian). */ +static inline __le32 wa_xfer_id_le32(struct wa_xfer *xfer) +{ + return cpu_to_le32(xfer->id); +} + +/* * If transfer is done, wrap it up and return true * * xfer->lock has to be locked @@ -259,8 +284,9 @@ static unsigned __wa_xfer_is_done(struct wa_xfer *xfer) switch (seg->status) { case WA_SEG_DONE: if (found_short && seg->result > 0) { - dev_dbg(dev, "xfer %p#%u: bad short segments (%zu)\n", - xfer, cnt, seg->result); + dev_dbg(dev, "xfer %p ID %08X#%u: bad short segments (%zu)\n", + xfer, wa_xfer_id(xfer), cnt, + seg->result); urb->status = -EINVAL; goto out; } @@ -268,24 +294,26 @@ static unsigned __wa_xfer_is_done(struct wa_xfer *xfer) if (seg->result < xfer->seg_size && cnt != xfer->segs-1) found_short = 1; - dev_dbg(dev, "xfer %p#%u: DONE short %d " + dev_dbg(dev, "xfer %p ID %08X#%u: DONE short %d " "result %zu urb->actual_length %d\n", - xfer, seg->index, found_short, seg->result, - urb->actual_length); + xfer, wa_xfer_id(xfer), seg->index, found_short, + seg->result, urb->actual_length); break; case WA_SEG_ERROR: xfer->result = seg->result; - dev_dbg(dev, "xfer %p#%u: ERROR result %zu\n", - xfer, seg->index, seg->result); + dev_dbg(dev, "xfer %p ID %08X#%u: ERROR result %zu(0x%08X)\n", + xfer, wa_xfer_id(xfer), seg->index, seg->result, + seg->result); goto out; case WA_SEG_ABORTED: - dev_dbg(dev, "xfer %p#%u ABORTED: result %d\n", - xfer, seg->index, urb->status); + dev_dbg(dev, "xfer %p ID %08X#%u ABORTED: result %d\n", + xfer, wa_xfer_id(xfer), seg->index, + urb->status); xfer->result = urb->status; goto out; default: - dev_warn(dev, "xfer %p#%u: is_done bad state %d\n", -xfer, cnt, seg->status); + dev_warn(dev, "xfer %p ID %08X#%u: is_done bad state %d\n", +xfer, wa_xfer_id(xfer), cnt, seg->status); xfer->result = -EINVAL; goto out; } @@ -296,31 +324,6 @@ out: } /* - * Initialize a transfer's ID - * - * We need to use a sequential number; if we use the pointer or the - * hash of the pointer, it can repeat over sequential transfers and - * then it will confuse the HWAwonder why in hell they put a 32 - * bit handle in there then. - */ -static void wa_xfer_id_init(struct wa_xfer *xfer) -{ - xfer->id = atomic_add_return(1, &xfer->wa->xfer_id_count); -} - -/* Return the xfer's ID. */ -static inline u32 wa_xfer_id(struct wa_xfer *xfer) -{ - return xfer->id; -} - -/* Return the xfer's ID in transport format (little endian). */ -static inline __le32 wa_xfer_id_le32(struct wa_xfer *xfer) -{ - return cpu_to_le32(xfer->id); -} - -/* * Search for a transfer list ID on the HCD's URB list * * For 32 bit architectures, we use the pointer
[PATCH 1/2] usb: wusbcore: fix endianess issues when using dwTransferID
Add a new function to get the xfer ID in little endian format (wa_xfer_id_le32), and use it instead of wa_xfer_id where appropriate. Signed-off-by: Thomas Pugliese --- drivers/usb/wusbcore/wa-xfer.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/usb/wusbcore/wa-xfer.c b/drivers/usb/wusbcore/wa-xfer.c index 08bf21d..9464f20d 100644 --- a/drivers/usb/wusbcore/wa-xfer.c +++ b/drivers/usb/wusbcore/wa-xfer.c @@ -308,16 +308,18 @@ static void wa_xfer_id_init(struct wa_xfer *xfer) xfer->id = atomic_add_return(1, &xfer->wa->xfer_id_count); } -/* - * Return the xfer's ID associated with xfer - * - * Need to generate a - */ -static u32 wa_xfer_id(struct wa_xfer *xfer) +/* Return the xfer's ID. */ +static inline u32 wa_xfer_id(struct wa_xfer *xfer) { return xfer->id; } +/* Return the xfer's ID in transport format (little endian). */ +static inline __le32 wa_xfer_id_le32(struct wa_xfer *xfer) +{ + return cpu_to_le32(xfer->id); +} + /* * Search for a transfer list ID on the HCD's URB list * @@ -381,7 +383,7 @@ static void __wa_xfer_abort(struct wa_xfer *xfer) b->cmd.bLength = sizeof(b->cmd); b->cmd.bRequestType = WA_XFER_ABORT; b->cmd.wRPipe = rpipe->descr.wRPipeIndex; - b->cmd.dwTransferID = wa_xfer_id(xfer); + b->cmd.dwTransferID = wa_xfer_id_le32(xfer); usb_init_urb(&b->urb); usb_fill_bulk_urb(&b->urb, xfer->wa->usb_dev, @@ -477,7 +479,7 @@ static void __wa_xfer_setup_hdr0(struct wa_xfer *xfer, xfer_hdr0->bLength = xfer_hdr_size; xfer_hdr0->bRequestType = xfer_type; xfer_hdr0->wRPipe = rpipe->descr.wRPipeIndex; - xfer_hdr0->dwTransferID = wa_xfer_id(xfer); + xfer_hdr0->dwTransferID = wa_xfer_id_le32(xfer); xfer_hdr0->bTransferSegment = 0; switch (xfer_type) { case WA_XFER_TYPE_CTL: { @@ -1752,7 +1754,7 @@ static void wa_dti_cb(struct urb *urb) if (usb_status == WA_XFER_STATUS_NOT_FOUND) /* taken care of already */ break; - xfer_id = xfer_result->dwTransferID; + xfer_id = le32_to_cpu(xfer_result->dwTransferID); xfer = wa_xfer_get_by_id(wa, xfer_id); if (xfer == NULL) { /* FIXME: transaction might have been cancelled */ -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] USBNET: fix handling padding packet
On Mon, Sep 23, 2013 at 8:59 PM, Ming Lei wrote: > Commit 638c5115a7949(USBNET: support DMA SG) introduces DMA SG > if the usb host controller is capable of building packet from > discontinuous buffers, but missed handling padding packet when > building DMA SG. > > This patch attachs the pre-allocated padding packet at the > end of the sg list, so padding packet can be sent to device > if drivers require that. > > Reported-by: David Laight > Acked-by: Oliver Neukum > Signed-off-by: Ming Lei > --- > v1: > - fix bug in case of dev->can_dma_sg and !urb->num_sgs David, could you queue this patch in your tree? And it should fix regression caused by commit 638c5115a7949(USBNET: support DMA SG), since I found that without the padding the device can't send out packets successfully if their length is dividable by max packet size, and the problem can be observed with icmp sending: $ping -s 974 another_machine #from host with ax99179_178a attached 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 v12 0/7] PHY framework
On Fri, Sep 27, 2013 at 11:53:24AM +0530, Kishon Vijay Abraham I wrote: > Added a generic PHY framework that provides a set of APIs for the PHY drivers > to create/destroy a PHY and APIs for the PHY users to obtain a reference to > the PHY with or without using phandle. > > This framework will be of use only to devices that uses external PHY (PHY > functionality is not embedded within the controller). > > The intention of creating this framework is to bring the phy drivers spread > all over the Linux kernel to drivers/phy to increase code re-use and to > increase code maintainability. > > Comments to make PHY as bus wasn't done because PHY devices can be part of > other bus and making a same device attached to multiple bus leads to bad > design. > > If the PHY driver has to send notification on connect/disconnect, the PHY > driver should make use of the extcon framework. Using this susbsystem > to use extcon framwork will have to be analysed. > > You can find this patch series @ > git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git testing > > I'll create a new branch *next* once this patch series is finalized. All the > PHY driver development that depends on PHY framework can be based on this > branch. > > Did USB enumeration testing in panda and beagle after applying [1] (needed for > non-dt) All now applied to my usb-next branch. Thanks for redoing this many times and sticking with it. 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
Re: [PATCH] Memory mapping for USBFS
On Wed, Sep 25, 2013 at 3:12 AM, Markus Rechberger wrote: > This patch adds memory mapping support to USBFS for isochronous and bulk > data transfers, it allows to pre-allocate usb transfer buffers. > > The CPU usage decreases 1-2% on my 1.3ghz U7300 notebook when > transferring 20mbyte/sec, it should be more interesting to see those > statistics on embedded systems where copying data is more expensive. Given USB3 is becoming popular and throughput is increased much, zero copy should be charming. And another approach is to use direct I/O method(SG DMA to pages allocated to user space directly), which should be more flexible, and user don't need to use mmap/munmap, so should be easier to use. At least, wrt. usb mass storage test, both CPU utilization and throughput can be improved with direct I/O. 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] gadget: udc-core: Ignore -EISNAM error from gadgetfs
On Fri, 27 Sep 2013, Fabio Estevam wrote: > From: Fabio Estevam > > When mounting a gadgetfs the following error message is seen: > > $ modprobe gadgetfs > gadgetfs: USB Gadget filesystem, version 24 Aug 2004 > $ mkdir /dev/gadget > $ mount -t gadgetfs none /dev/gadget > nop ci_hdrc.0: failed to start (null): -120 > > The error comes from gadgetfs_probe(), which returns -EISNAM (-120). > > As Alan Stern explains[1] this is the normal behavior: > > "It is a temporary measure, used only when the file system is set up > initially. The real bind routine is gadgetfs_bind(), which gets called > when userspace configures the gadget. > > In short, this is how it is intended to work. It isn't a bug." > > [1] http://marc.info/?l=linux-usb&m=138029668707075&w=2 > > So in order to prevent the error message, do not report EISNAM as an error. > > Signed-off-by: Fabio Estevam > --- > drivers/usb/gadget/udc-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c > index 59891b1..0887e15 100644 > --- a/drivers/usb/gadget/udc-core.c > +++ b/drivers/usb/gadget/udc-core.c > @@ -344,7 +344,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct > usb_gadget_driver *dri > udc->gadget->dev.driver = &driver->driver; > > ret = driver->bind(udc->gadget, driver); > - if (ret) > + if (ret && ret != -EISNAM) > goto err1; > ret = usb_gadget_udc_start(udc->gadget, driver); > if (ret) { You still have to _treat_ -EISNAM as an error. Just don't _report_ it in the system log. 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] Memory mapping for USBFS
On Sat, 28 Sep 2013, Ming Lei wrote: > On Wed, Sep 25, 2013 at 3:12 AM, Markus Rechberger > wrote: > > This patch adds memory mapping support to USBFS for isochronous and bulk > > data transfers, it allows to pre-allocate usb transfer buffers. > > > > The CPU usage decreases 1-2% on my 1.3ghz U7300 notebook when > > transferring 20mbyte/sec, it should be more interesting to see those > > statistics on embedded systems where copying data is more expensive. > > Given USB3 is becoming popular and throughput is increased much, zero > copy should be charming. > > And another approach is to use direct I/O method(SG DMA to pages > allocated to user space directly), which should be more flexible, and > user don't need to use mmap/munmap, so should be easier to use. > > At least, wrt. usb mass storage test, both CPU utilization and throughput > can be improved with direct I/O. For zero-copy to work, on many systems the pages have to be allocated in the first 4 GB of physical memory. How can the userspace program make sure this will happen? 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] Memory mapping for USBFS
On Sat, Sep 28, 2013 at 10:29 AM, Alan Stern wrote: > On Sat, 28 Sep 2013, Ming Lei wrote: > >> On Wed, Sep 25, 2013 at 3:12 AM, Markus Rechberger >> wrote: >> > This patch adds memory mapping support to USBFS for isochronous and bulk >> > data transfers, it allows to pre-allocate usb transfer buffers. >> > >> > The CPU usage decreases 1-2% on my 1.3ghz U7300 notebook when >> > transferring 20mbyte/sec, it should be more interesting to see those >> > statistics on embedded systems where copying data is more expensive. >> >> Given USB3 is becoming popular and throughput is increased much, zero >> copy should be charming. >> >> And another approach is to use direct I/O method(SG DMA to pages >> allocated to user space directly), which should be more flexible, and >> user don't need to use mmap/munmap, so should be easier to use. >> >> At least, wrt. usb mass storage test, both CPU utilization and throughput >> can be improved with direct I/O. > > For zero-copy to work, on many systems the pages have to be allocated > in the first 4 GB of physical memory. How can the userspace program It depends if device can DMA to/from 4GB above physical memory. > make sure this will happen? That can't be guaranteed but we can handle it with page bounce, just like block device. Actually I observed both throughput and cpu utilization can be improved with the 4GB of DMA limit on either 32bit arch or 64bit arch, wrt. direct I/O over usb mass storage block device. 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
[PATCH v2] gadget: udc-core: Do not report -EISNAM error from gadgetfs
From: Fabio Estevam When mounting a gadgetfs the following error message is seen: $ modprobe gadgetfs gadgetfs: USB Gadget filesystem, version 24 Aug 2004 $ mkdir /dev/gadget $ mount -t gadgetfs none /dev/gadget nop ci_hdrc.0: failed to start (null): -120 The error comes from gadgetfs_probe(), which returns -EISNAM (-120). As Alan Stern explains[1], this is the normal behavior: "It is a temporary measure, used only when the file system is set up initially. The real bind routine is gadgetfs_bind(), which gets called when userspace configures the gadget. In short, this is how it is intended to work. It isn't a bug." [1] http://marc.info/?l=linux-usb&m=138029668707075&w=2 So in order to prevent the error message, do not report EISNAM as an error. Signed-off-by: Fabio Estevam --- Changes since v1: - Still treat EISNAM as error. Only discard printing the error message. drivers/usb/gadget/udc-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c index 59891b1..27768a7 100644 --- a/drivers/usb/gadget/udc-core.c +++ b/drivers/usb/gadget/udc-core.c @@ -356,7 +356,8 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); return 0; err1: - dev_err(&udc->dev, "failed to start %s: %d\n", + if (ret != -EISNAM) + dev_err(&udc->dev, "failed to start %s: %d\n", udc->driver->function, ret); udc->driver = NULL; udc->dev.driver = NULL; -- 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