Re: ehci frame index goes backwards?
On Wed, Oct 24, 2018 at 9:48 AM Daniel Goertzen wrote: > > I am developing a custom USB device that makes use of isochronous IN > transfers. It works fine from my laptop (xHCI) however on an embedded > target (vortex86dx3, EHCI) I am seeing mysterious EFBIG errors. After > adding lots of debug to ehci-sched I discovered that the EHCI register > "FRINDEX" which is supposed to increase monotonically, is actually > taking a jump backwards: It does increment monotonically, but not forever. There is a limit defined by the ehci spec. Frame Index. The value in this register increments at the end of each time frame (e.g. micro-frame). Bits [N:3] are used for the Frame List current index. This means that each location of the frame list is accessed 8 times (frames or micro-frames) before moving to the next index. The following illustrates values of N based on the value of the Frame List Size field in the USBCMD register. 10, 11 or 12 bits is the N size. This becomes the SOF number. It does wrap, maybe you are seeing a reading after a wrap? Regards, Steve > ehci_read_frame_index(ehci)=12318 > [ 25.576523] ehci-pci :00:0a.1: iso_stream_schedule > ehci_read_frame_index(ehci)=12292 <- (count goes down!?) > [ 25.576550] ehci-pci :00:0a.1: request c302d23e would overflow > (4104+288 >= 4096) > > The backwards jump screws up periodic scheduling and culminates in the > EFBIG error I've been seeing. I'm suspecting buggy EHCI hardware, but > my question is... could anything else cause this? > > Other misc background facts: > - Host is an Adlink CM1 PC104 module which contains a Vortex86 DX3. > USB device is an LPC4323 operating full-speed. > - I instigate the error by softbooting the system. EFBIG typically > happens within a minute or does not happen at all. > - I also get EXDEV errors from "URB was too late" in sitd_complete(). > I suspect this is related to the problem above but don't fully > understand the mechanism. > > Thanks, > Dan.
usbfs zerocopy broken on ARM/ARM64
Hi, I'm the author of a userspace library that makes use of the usbfs zerocopy-feature via libusb-1.0, and recently received a bug report that garbage data is received with bulk transfers on ARM-based systems. When I debugged the issue, I found out that the Kernel maps seemingly random memory to my transfer buffers, containing memory of other processes or even the Kernel itself. The code that does the mapping in drivers/usb/core/devio.c: (line 243 in v4.19-rc7) > if (remap_pfn_range(vma, vma->vm_start, > virt_to_phys(usbm->mem) >> PAGE_SHIFT, > size, vma->vm_page_prot) < 0) { With the following change the issue is fixed for ARM systems, but it breaks x86 systems: - virt_to_phys(usbm->mem) >> PAGE_SHIFT, + page_to_pfn(virt_to_page(dma_addr)), Both usbm->mem and dma_addr are returned by the previous call to usb_alloc_coherent(). Here's an example of the pointers generated by the two macros on an ARM64 system for the same buffer: virt_to_phys(usbm->mem) >> PAGE_SHIFT: 808693ce page_to_pfn(virt_to_page(dma_addr)): 9775a856 >From what I read so far I got the impression that the 'proper' way would be to use dma_mmap_coherent() with dma_addr instead of remap_pfn_range(), however, I did not get it to work. Can anyone help out? Best Regards, Steve Markgraf
Re: [PATCH v3] smsc95xx: Add comments to the registers definition
On 12 April 2017 at 10:24, Martin Wetterwald wrote: > This chip is used by a lot of embedded devices and also by the Raspberry > Pi 1, 2 & 3 which were created to promote the study of computer > sciences. Students wanting to learn kernel / network device driver > programming through those devices can only rely on the Linux kernel > driver source to make their own. > > This commit adds a lot of comments to the registers definition to expand > the register names. > > Cc: Steve Glendinning > Cc: Microchip Linux Driver Support > CC: David Miller > Signed-off-by: Martin Wetterwald Acked-by: Steve Glendinning -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/4] pegasus: Use heap buffers for all register access
On Mon, Feb 6, 2017 at 4:51 AM, Petko Manolov wrote: > On 17-02-06 09:28:22, Greg KH wrote: >> On Mon, Feb 06, 2017 at 10:14:44AM +0200, Petko Manolov wrote: >> > On 17-02-05 01:30:39, Greg KH wrote: >> > > On Sat, Feb 04, 2017 at 04:56:03PM +, Ben Hutchings wrote: >> > > > Allocating USB buffers on the stack is not portable, and no longer >> > > > works >> > > > on x86_64 (with VMAP_STACK enabled as per default). >> > > >> > > It's never worked on other platforms, so these should go to the stable >> > > releases please. >> > >> > As far as i know both drivers works fine on other platforms, though I only >> > tested it on arm and mipsel. ;) >> >> It all depends on the arm and mips platforms, the ones that can not DMA from >> stack memory are the ones that would always fail here (since the 2.2 kernel >> days). > > Seems like most modern SOCs have decent DMA controllers. > The real problem is not DMA exactly, it is cache coherency. X86 has a coherent cache and all the cpu cores watch DMA transfers and keep the cpu caches up to date. Most ARMs and MIPS processors have incoherent cache, so DMA can change memory without the CPU cache updates. CPU cache view of what is in memory can be different from what was DMAed in, this makes failures very hard to detect, reproduce and racy. So all DMA buffers should always be separate allocations from the stack AND not be embedded in structs. Memory allocations are always at least cache line aligned, so coherency is not a problem. Regards, Steve -- 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] cifs: Fix smbencrypt() to stop pointing a scatterlist at the stack
merged into cifs-2.6.git for-next On Tue, Dec 13, 2016 at 7:07 AM, Jeff Layton wrote: > On Mon, 2016-12-12 at 12:54 -0800, Andy Lutomirski wrote: >> smbencrypt() points a scatterlist to the stack, which is breaks if >> CONFIG_VMAP_STACK=y. >> >> Fix it by switching to crypto_cipher_encrypt_one(). The new code >> should be considerably faster as an added benefit. >> >> This code is nearly identical to some code that Eric Biggers >> suggested. >> >> Cc: sta...@vger.kernel.org # 4.9 only >> Reported-by: Eric Biggers >> Signed-off-by: Andy Lutomirski >> --- >> >> Compile-tested only. >> >> fs/cifs/smbencrypt.c | 40 >> 1 file changed, 8 insertions(+), 32 deletions(-) >> >> diff --git a/fs/cifs/smbencrypt.c b/fs/cifs/smbencrypt.c >> index 699b7868108f..c12bffefa3c9 100644 >> --- a/fs/cifs/smbencrypt.c >> +++ b/fs/cifs/smbencrypt.c >> @@ -23,7 +23,7 @@ >> Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. >> */ >> >> -#include >> +#include >> #include >> #include >> #include >> @@ -69,46 +69,22 @@ str_to_key(unsigned char *str, unsigned char *key) >> static int >> smbhash(unsigned char *out, const unsigned char *in, unsigned char *key) >> { >> - int rc; >> unsigned char key2[8]; >> - struct crypto_skcipher *tfm_des; >> - struct scatterlist sgin, sgout; >> - struct skcipher_request *req; >> + struct crypto_cipher *tfm_des; >> >> str_to_key(key, key2); >> >> - tfm_des = crypto_alloc_skcipher("ecb(des)", 0, CRYPTO_ALG_ASYNC); >> + tfm_des = crypto_alloc_cipher("des", 0, 0); >> if (IS_ERR(tfm_des)) { >> - rc = PTR_ERR(tfm_des); >> - cifs_dbg(VFS, "could not allocate des crypto API\n"); >> - goto smbhash_err; >> - } >> - >> - req = skcipher_request_alloc(tfm_des, GFP_KERNEL); >> - if (!req) { >> - rc = -ENOMEM; >> cifs_dbg(VFS, "could not allocate des crypto API\n"); >> - goto smbhash_free_skcipher; >> + return PTR_ERR(tfm_des); >> } >> >> - crypto_skcipher_setkey(tfm_des, key2, 8); >> - >> - sg_init_one(&sgin, in, 8); >> - sg_init_one(&sgout, out, 8); >> + crypto_cipher_setkey(tfm_des, key2, 8); >> + crypto_cipher_encrypt_one(tfm_des, out, in); >> + crypto_free_cipher(tfm_des); >> >> - skcipher_request_set_callback(req, 0, NULL, NULL); >> - skcipher_request_set_crypt(req, &sgin, &sgout, 8, NULL); >> - >> - rc = crypto_skcipher_encrypt(req); >> - if (rc) >> - cifs_dbg(VFS, "could not encrypt crypt key rc: %d\n", rc); >> - >> - skcipher_request_free(req); >> - >> -smbhash_free_skcipher: >> - crypto_free_skcipher(tfm_des); >> -smbhash_err: >> - return rc; >> + return 0; >> } >> >> static int > > Acked-by: Jeff Layton > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Thanks, Steve -- 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: TCPCI driver issue
Hi Guenter. I would be interested in seeing the test driver, either on the list or privately if you are not comfortable with a general release. Please try to clone https://chromium.googlesource.com/chromiumos/third_party/kernel and fetch refs/sandbox/groeck/typec-4.4-cros-090816 (sha a3fb79db5e901ce265176702c5769666ecb6c679). That should give you everything I have. It should also fix the problem you have seen. Thanks, I'll take a look. I'm currently working on support for the Analogix AN7688. Is that the HDMI to USB Type-C Bridge ? I would have thought this would be used in a Type-C dock ? It converts HDMI (native to the SoC) to DisplayPort rather than the other way around as you'd need in a dock. Regards, Steve -- 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: TCPCI driver issue
Hi Guenter. I think (hope) I did mention that the tcpci patch was compile tested only. Apologies if not. I'll try to get to it today and send a fix, though it will obviously only be a workaround (config data is platform specific). I also hope I can send a Reviewed-by: for Heikki's series today. Sorry for being late on that one. I did see in the patch header that it is a WIP, but didn't realize it was compile tested only. I understand I'm on the bleeding edge here. But we probable need to talk about how to extract the platform specific details needed for the config in any case (and also what exactly is needed) at one point. I think we can get all of them with device properties regardless of the system (ACPI/DT/whatever), so we would just need to agree on the what the properties are. Yes, we'll definitely need that. I've used platform data and DMI internally for x86, but that seems so out of date. I'll also see if it makes sense to attach the test driver I used to test the tcpm code. Note that we also have a driver for fusb302 as client of tcpm in the works, plus an extcon based driver connected to the EC on chromebooks (and ties directly into the Type-C infrastructure). I would be interested in seeing the test driver, either on the list or privately if you are not comfortable with a general release. I'm currently working on support for the Analogix AN7688. Regards, Steve -- 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: question on trust in chaoskey
Hi Keith, On Thu, May 19, 2016 at 9:23 PM, Keith Packard wrote: > Dave Tian writes: > >> I am personally in favor of a TPM-like solution, since we probably >> couldn’t/shouldn’t disable the firmware update anyway, >> and we really need a hardware root of trust (with a key embedded) in >> the device, like the TPM in the host. > > I don't think we need a true TPM in the hardware; the device is > read-only in normal operation with firmware upgrades requiring physical > presence. So, supply the private key with the firmware and then erase it > From the host once programmed. Once the programming jumper is removed, > only physical access would be sufficient to extract the private key. > > Here's more information about the hardware: > > http://altusmetrum.org/ChaosKey/ > > -- This is the first I have seen this gadget. The obvious use is with encryption. The obvious problem is someone substituting a false USB key for a genuine one. The classic "drop flash drives in a conference lobby", and someone will use it and infect windows. A more capitalistic attack for any system is someone offering a false USB "key" for 10% less than anyone else. Any computer that allows any USB device to be plugged in is at risk. That's why there is physical protection for servers. A clever attacker would provide a false USB key which is "almost" random. This would allow them to decrypt messages based on the false key, with nobody else knowing there was a vulnerability. An almost random number simplifies cracking. It is easy to exactly duplicate all the descriptors and functionality in a false device. It could be easily done with a PIC, Arduino, or $9 CHIP. Who could tell a key is false or genuine? The false device could do the same dance with public keys (or whatever secret handshake you setup). If a user cannot be sure a key is genuine, and a false key can leak information, I don't see the point of anyone using such a USB device. Regards, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Create an audit record of USB specific details
On Tuesday, April 05, 2016 07:02:48 PM Oliver Neukum wrote: > On Tue, 2016-04-05 at 18:40 +1000, Wade Mealing wrote: > > Consider the following scenario. Currently we have device drivers > > that emit text via a printk request which is eventually picked up by > > syslog like implementation (not the audit subsystem). > > We also have UEVENTs. The crucial question is why udevd feeding > back events to the audit subsystem is inferior to the kernel > itself generating audit events. If this was going to be done in user space, then we are talking about auditd growing the ability to monitor another netlink socket for events. The question that decides if this is feasible is whether or not UEVENTS are protected from loss if several occur in a short time before auditd can get around to reading them. The other issue that I'm curious about is if adding hardware can fail. Do the events coming out by UEVENTS have any sense of pass or fail? Or are they all implicitly successful? And then we get to the issue of whether or not UEVENTS can be filtered. If so, then we will also need to add auditing around the configuration of the filters to see if anything is impacting the audit trail. -Steve -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Create an audit record of USB specific details
On Monday, April 04, 2016 12:02:42 AM wmealing wrote: > I'm looking to create an audit trail for when devices are added or removed > from the system. > > The audit subsystem is a logging subsystem in kernel space that can be > used to create advanced filters on generated events. It has partnered > userspace utilities ausearch, auditd, aureport, auditctl which work > exclusively on audit records. > > These tools are able to set filters to "trigger" on specific in-kernel > events specified by privileged users. While the userspace tools can create > audit events these are not able to be handled intelligently > (decoded,filtered or ignored) as kernel generated audit events are. > > I have this working at the moment with the USB subsystem (as an example). > Its been suggested that I use systemd-udev however this means that the audit > tools (ausearch) will not be able to index these records. > > Here is an example of picking out the AUDIT_DEVICE record type for example. > > > # ausearch -l -i -ts today -m AUDIT_DEVICE > > > > type=AUDIT_DEVICE msg=audit(31/03/16 16:37:15.642:2) : action=add > > manufacturer=Linux 4.4.0-ktest ehci_hcd product=EHCI Host Controller > > serial=:00:06.7 major=189 minor=0 bus="usb" About this event's format...we can't have any spaces in the value side of the name=value fields unless its encoded as an untrusted string. You can replace spaces with an underscore or dash for readability. So, manufacturer and product would need this treatment. -Steve > Admittedly this is only the USB device type at the moment, but I'd like to > break this out into other bus types at some time in the future, gotta start > somewhere. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Create an audit record of USB specific details
On Monday, April 04, 2016 05:56:26 AM Greg KH wrote: > On Mon, Apr 04, 2016 at 12:02:42AM -0400, wmealing wrote: > > From: Wade Mealing > > > > Gday, > > > > I'm looking to create an audit trail for when devices are added or removed > > from the system. > > Then please do it in userspace, as I suggested before, that way you > catch all types of devices, not just USB ones. > > Also I don't think you realize that USB interfaces are what are bound to > drivers, not USB devices, so that is going to mess with any attempted > audit trails here. How are you going to distinguish between the 5 > different devices that just got plugged in that all have / as > vid/pid for them because they are "cheap" devices from China, yet do > totally different things because they are different _types_ of devices? This sounds like vid/pid should be captured in the event. > Again, do this in userspace please, that is where it belongs. There is one issue that may need some clarification. The audit system has to do everything possible to make sure that an event is captured and logged. Does the uevent netlink protocol ever drop events because the user space queue is full? If the uevent interface drops events, then its not audit quality in terms of doing everything possible to prevent the loss of a record. If this were to happen, how would user space find out when a uevent gets dropped? I may have to panic the machine if that happens depending on the configured policy. So, we need to know when it happens. If on the otherhand it doesn't ever drop events, then it might be usable. -Steve -- 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_midi: Fixed a bug when buflen was smaller than wMaxPacketSize
On Thu, Mar 10, 2016 at 1:23 AM, Felipe Ferreri Tonello wrote: >>> --- a/drivers/usb/gadget/function/f_midi.c >>> +++ b/drivers/usb/gadget/function/f_midi.c >>> @@ -366,7 +366,9 @@ static int f_midi_set_alt(struct usb_function *f, >>> unsigned intf, unsigned alt) >>> /* allocate a bunch of read buffers and queue them all at once. */ >>> for (i = 0; i < midi->qlen && err == 0; i++) { >>> struct usb_request *req = >>> - midi_alloc_ep_req(midi->out_ep, midi->buflen); >>> + midi_alloc_ep_req(midi->out_ep, >>> + max_t(unsigned, midi->buflen, >>> + bulk_out_desc.wMaxPacketSize)); >>> if (req == NULL) >>> return -ENOMEM; >>> >> Won't you get a buffer overrun if midi->buflen is less than wMaxPacketSize? >> > > No, because of the *max_t(unsigned, midi->buflen, > bulk_out_desc.wMaxPacketSize)*. > > Maybe that's not the most clear indentation but I had to break it in > order to avoid passing 80 columns. > Yes, I understood that code. It is a little confusing that gadgets use OUT endpoints to receive data. I think we are talking about different buffers. I meant that if you ever received data into your enlarged ep buffer, will you not copy it into your midi buffer? If the code checks, I guess you will truncate the received data? If not, it could be a midi buffer overrun. Regards, Steve -- 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_midi: Fixed a bug when buflen was smaller than wMaxPacketSize
On Wed, Mar 9, 2016 at 11:39 AM, Felipe F. Tonello wrote: > buflen by default (256) is smaller than wMaxPacketSize (512) in high-speed > devices. > > That caused the OUT endpoint to freeze if the host send any data packet of > length greater than 256 bytes. > > This is an example dump of what happended on that enpoint: > HOST: [DATA][Length=260][...] > DEVICE: [NAK] > HOST: [PING] > DEVICE: [NAK] > HOST: [PING] > DEVICE: [NAK] > ... > HOST: [PING] > DEVICE: [NAK] > > This patch fixes this problem by setting the minimum usb_request's buffer size > for the OUT endpoint as its wMaxPacketSize. > > Signed-off-by: Felipe F. Tonello > --- > drivers/usb/gadget/function/f_midi.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/function/f_midi.c > b/drivers/usb/gadget/function/f_midi.c > index 8475e3dc82d4..826ba641f29d 100644 > --- a/drivers/usb/gadget/function/f_midi.c > +++ b/drivers/usb/gadget/function/f_midi.c > @@ -366,7 +366,9 @@ static int f_midi_set_alt(struct usb_function *f, > unsigned intf, unsigned alt) > /* allocate a bunch of read buffers and queue them all at once. */ > for (i = 0; i < midi->qlen && err == 0; i++) { > struct usb_request *req = > - midi_alloc_ep_req(midi->out_ep, midi->buflen); > + midi_alloc_ep_req(midi->out_ep, > + max_t(unsigned, midi->buflen, > + bulk_out_desc.wMaxPacketSize)); > if (req == NULL) > return -ENOMEM; > Won't you get a buffer overrun if midi->buflen is less than wMaxPacketSize? Regards, Steve -- 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 BACKPORT] xhci:_don't_finish_a_TD_if_we_get_a_short_transfer_event_mid_TD
On Sun, 2016-02-14 at 14:15 -0800, Greg KH wrote: > On Wed, Dec 16, 2015 at 06:22:30AM -0800, Steve Bangert wrote: > > Hi, > > > > This original patch was posted and applied to the 4.3-rcX kernel > > and tagged for > > stable kernels. > > > > http://marc.info/?l=linux-usb&m=144463835322214&w=2 > > > > > > But didn't to make it into the 4.1 and 3.14 kernels as it failed > > to apply in it's original form. > > > > A backport from Ben Hutchings was posted during the 3.2.73-rc1 > > review cycle and will now apply. > > > > Please consider for the next stable kernel release cycle. > > Patch is corrupted and does not apply properly :( > You can disregard as the patch was reverted upstream. Steve -- 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 driver issue (ASMedia Controller using xhci_hcd)
Oliver Neukum writes: > > # echo -1 >/sys/module/usbcore/parameters/autosuspend > > Did you see any other device failing or is it specific > to your scanner? > > Regards > Oliver > All other devices I own have worked as expected: mouse, keyboard, storage (memory card, hard disk) and a TV decoder. -- 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 driver issue (ASMedia Controller using xhci_hcd)
In my case, my scanner problem has been fixed (so far) by tuning off autosuspend: # echo -1 >/sys/module/usbcore/parameters/autosuspend -- 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 driver issue (ASMedia Controller using xhci_hcd)
I've been trying to get my scanner working on my newer laptop (entire Linux installation, apart from the kernel (4.2.5) cloned from the old one) and I seem to be seeing the same issue, although I have no problem with USB storage or HID devices on the USB 3 port. Here's a clip from the debug with SANE_DEBUG_SANEI_USB & SANE_DEBUG_SANEI_LM983X set: [sanei_lm983x] sanei_lm983x_write: fd=1, reg=0x59, len=1, increment=0 [sanei_usb] sanei_usb_write_bulk: trying to write 5 bytes [sanei_usb] 000 00 59 00 01 66 .Y..f [sanei_usb] sanei_usb_write_bulk: sg : about to call usb_bulk_write [sanei_usb] sanei_usb_write_bulk: write failed: Resource temporarily unavailable [sanei_usb] sanei_usb_write_bulk: wanted 5 bytes, wrote 0 bytes [sanei_lm983x] sanei_lm983x_write: short write (0/5) [sanei_lm983x] sanei_lm983x_write: couldn't even send command That's after four successful reads and writes, and is at the same point every time. After this, all reads and writes fail and the sane interface hangs. If you try running it again, it fails straight away, and the USB plug has to be pulled and re-inserted before the first series of reads and writes work again. I'll carry on poking at this, but maybe someone who really understands what is going on will see the posts and think of something! -- 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 BACKPORT] xhci:_don't_finish_a_TD_if_we_get_a_short_transfer_event_mid_TD
Hi, This original patch was posted and applied to the 4.3-rcX kernel and tagged for stable kernels. http://marc.info/?l=linux-usb&m=144463835322214&w=2 But didn't to make it into the 4.1 and 3.14 kernels as it failed to apply in it's original form. A backport from Ben Hutchings was posted during the 3.2.73-rc1 review cycle and will now apply. Please consider for the next stable kernel release cycle. Steve Bangert From: Mathias Nyman commit e210c422b6fdd2dc123bedc588f399aefd8bf9de upstream. If the difference is big enough between the bytes asked and received in a bulk transfer we can get a short transfer event pointing to a TRB in the middle of the TD. We don't want to handle the TD yet as we will anyway receive a new event for the last TRB in the TD. Hold off from finishing the TD and removing it from the list until we receive an event for the last TRB in the TD Signed-off-by: Mathias Nyman Signed-off-by: Greg Kroah-Hartman [bwh: Backported to 3.2: adjust context] Signed-off-by: Ben Hutchings Reported-by: Steve Bangert Cc: # 4.1.x- Cc: # 3.12.x- --- drivers/usb/host/xhci-ring.c | 10 ++ 1 file changed, 10 insertions(+) --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2187,6 +2187,10 @@ static int process_bulk_intr_td(struct x EVENT_TRB_LEN(le32_to_cpu(event->transfer_len))); /* Fast path - was this the last TRB in the TD for this URB? */ if (event_trb == td->last_trb) { + if (td->urb_length_set && trb_comp_code == COMP_SHORT_TX) + return finish_td(xhci, td, event_trb, event, ep, + status, false); + if (EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) { td->urb->actual_length = td->urb->transfer_buffer_length - @@ -2238,6 +2242,12 @@ static int process_bulk_intr_td(struct x td->urb->actual_length += TRB_LEN(le32_to_cpu(cur_trb->generic.field[2])) - EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); + + if (trb_comp_code == COMP_SHORT_TX) { + xhci_dbg(xhci, "mid bulk/intr SP, wait for last TRB event\n"); + td->urb_length_set = true; + return 0; + } } return finish_td(xhci, td, event_trb, event, ep, status, false); -- -- 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: First kernel patch (optimization)
On Tue, Sep 15, 2015 at 12:53 PM, Eric Curtin wrote: > Signed-off-by: Eric Curtin > > diff --git a/tools/usb/usbip/src/usbip_detach.c > b/tools/usb/usbip/src/usbip_detach.c > index 05c6d15..9db9d21 100644 > --- a/tools/usb/usbip/src/usbip_detach.c > +++ b/tools/usb/usbip/src/usbip_detach.c > @@ -47,7 +47,9 @@ static int detach_port(char *port) > uint8_t portnum; > char path[PATH_MAX+1]; > > - for (unsigned int i = 0; i < strlen(port); i++) > + unsigned int port_len = strlen(port); > + > + for (unsigned int i = 0; i < port_len; i++) > if (!isdigit(port[i])) { > err("invalid port %s", port); > return -1; > > -- Hi Eric, This is fine, but what kind of wimpy compiler optimizer will not move the constant initializer out of the loop? I bet if you compare binary sizes/code it will be exactly the same, and you added some characters of code. Reorganizing code for readability is fine, but for compiler (in)efficiency seems like a bad idea. Regards, Steve -- 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: About zero-length packet design for EHCI
On Wed, Jul 1, 2015 at 11:00 PM, Peter Chen wrote: > If "one" is "multiple"? > The wLength at setup packet is 64 bytes, Maximum Packet Length at > dQH is 64 bytes, and the Total Bytes at dTD is 64 bytes too, does device > must prepare a zero-length packet? > > I would like to double confirm it since the iPhone (As host after HNP) > can't work well with it if I have a zero-length packet after 64 bytes packet > which I describe above, if without zero-length packet, the iPhone works well. > >> The host hardware will recognize when this happens, because the HCD >> will set the appropriate bits in the data-stage qTD. For example, with >> EHCI the HCD will set the Alternate Next qTD Pointer. Or with UHCI, >> the HCD will set the SPD (Short Packet Detect) bit. >> > > I see it in the code, but it is a null pointer (EHCI_LIST_END), does it > mean one qTD (with valid Next qTD Pointer) can handle one 64 byte packet > with or without zero-length packet well both? > > Any reasons why iPhone can't handle zero-length packet well? > >> But the HCD should never prepare a zero-length qTD for an IN transfer. >> Zero-length packets are the responsibility of the _sender_, and for IN >> transfers the sender is the peripheral, not the host. I can't speak about the details of the ehci driver. But for this issue we can speak about sender and receiver, Host and gadget both send and receive during transfers. If the protocol does not specify a transfer length, then the sender MUST send a zlt if the transfer is less than expected and a multiple (1*, 2*, 3*) of maxpacketsize. If the protocol does specify a transfer length, then exactly that length should be sent and received with NO ZLT. For example USB mass storage sends multiples of maxpacketsize but does not send ZLTs. If you do an analyzer trace between windows and your gadget, then you can see what those guys think the rules for your protocol are. Regards, Steve -- 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: 64 byte EP0 OUT data transfer issue on Chipidea highspeed dual role controller
On Mon, Jun 29, 2015 at 7:16 AM, Alan Stern wrote: > On Mon, 29 Jun 2015, Peter Chen wrote: > >> Just like Steve pointed, it should be a ZLT problem, do you have >> below patch in your tree, and the host may not send zlt, but you >> may queue an zero-length request, the f_hid does not set req->zero >> flag either. >> >> commit 953c66469735aed8d2ada639a72b150f01dae605 >> Author: Abbas Raza >> Date: Thu Jul 17 19:34:31 2014 +0800 >> >> usb: chipidea: udc: Disable auto ZLP generation on ep0 >> >> >> If it still has problem, send me apps if possible (with needed kernel >> patches). > > Note that a UDC should _never_ queue an extra zero-length packet for an > OUT transfer, no matter how req->zero is set. In other words, > req->zero is supposed to affect only IN transfers. > Yes UDC gadgets receive packets sent by the host on OUT transfers. If ZLT transfers are needed for the protocol, a short transfer is needed to end an OUT sequence and it must be sent by the sender, in this case the host. The UDC should detect receiving a zero length EP0 OUT and that signals the end of the 64 byte transfer from host to gadget. Regards, Steve -- 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: 64 byte EP0 OUT data transfer issue on Chipidea highspeed dual role controller
On Fri, Jun 26, 2015 at 5:01 AM, Jayan John wrote: > Thanks Alex. I appreciate you introducing me to Peter. Any help is > appreciated. > > On the host (Wandboard iMX6q) the test app opens /dev/hidraw0 and > write 64 bytes with report ID (1). The HID device has no Interrupt OUT > ep, therefore uses control endpoint ep0 for the 64 bytes transfer to > gadget (Wandboard iMX6q) using set_report. > > The setup phase is OK and the 64 bytes is written to gadget. However > the chipidea interrupt (ci_irq()) and resulting udc interrupt > (udc_irq()) is invoked. This indicates that the 64 bytes transaction > is not completed over ep0 from host to gadget. > > **this issue is reproducible for all data transfers that aligns on 64 bytes** > > ~jayan > Hi Jayan, This sounds like a Zero Length Transfer issue. This applies to any endpoint including EP0. A ZLT is needed to end any transfer IFF the length is not already known by the protocol. So if the hid URB requests say 1024 bytes and the gadget only has 64, it must first send the 64 byte maxpacket and then send a zero length packet. This way the host knows the transfer is complete. If there is no ZLT, the host hardware will keep requesting the rest of the message and the gadget which thinks it is done will keep NAKing Forever. If however the transfer is known to be exactly some multiple (1, 2 ...) of the maxpacketsize, no ZLT is needed since the host will ask for exactly that size. I believe for HID the packet size is determined by the report size in the HID descriptor. Good Luck, Steve -- 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 11/11] usbip: USB over WebSocket
If you are looking for a good C websockets library, gpl etc try: https://libwebsockets.org/trac/libwebsockets I used it in a project and was very pleased with how it worked and the support where it is hosted. Regards, Steve On Wed, Apr 22, 2015 at 11:52 PM, fx IWATA NOBUO wrote: > Hi Bjørn > >> This cannot possibly be necessary. Pointing to the toplevel "COPYING" >> file should be more than enough, if anything at all. >> >> And BTW, the same goes for the tools/usb/usbip/COPYING that seems to >> have snuck in somehow. I also wonder a bit about the AUTHORS file > both >> places. We have git and MAINTAINERS for such things, don't we? > > I will remove from v3. > >> > --- /dev/null >> > +++ b/tools/usb/usbip/websocket/configure.ac >> > @@ -0,0 +1,61 @@ >> > +dnl Process this file with autoconf to produce a configure script. >> > + >> > +AC_PREREQ(2.59) >> > +AC_INIT([usbip-utils], [1.1.1], [linux-usb@vger.kernel.org]) >> > +AC_DEFINE([USBIP_VERSION], [0x0111], [binary-coded decimal > version >> number]) >> >> This should either use the upper level definitions or have its own >> namespace. Defining the same symbols at different levels is a recipe >> for confusion. In fact, you are already out of sync with the >> definitions in tools/usb/usbip/configure.ac > > Sorry for my carelessness. > I will use own AC_INIT() in v3 and remove AC_DEFINE([USBIP_VERSION]) > from v3. > >> > Implementation of this patch depends on Poco C++ >> > (http://pocoproject.org/). >> >> Really? Is that OK? >> >> Yuck. Any reason this cannot be written in C? > > I couldn't find good C library which supports both server and client. > I intended other implementation may be stored under 'websocket' > directory next to 'poco'. > > Thank you for your comments, > > n.iwata > // -- 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] uas: Allow uas_use_uas_driver to return usb-storage flags
On Thu, 2015-04-16 at 14:17 +0200, Hans de Goede wrote: > uas_use_uas_driver may set some US_FL_foo flags during detection, currently > these are stored in a local variable and then throw away, but these may be > of interest to the caller, so add an extra parameter to (optionally) return > the detected flags, and use this in the uas driver. > > Signed-off-by: Hans de Goede Patched (all 3 of them) and compile tested on the current Fedora kernel (3.19.3-200), device is accessible and functioning without a kernel parameter Steve > --- > drivers/usb/storage/uas-detect.h | 6 +- > drivers/usb/storage/uas.c| 6 +++--- > drivers/usb/storage/usb.c| 2 +- > 3 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/storage/uas-detect.h > b/drivers/usb/storage/uas-detect.h > index 9893d69..63ae161 100644 > --- a/drivers/usb/storage/uas-detect.h > +++ b/drivers/usb/storage/uas-detect.h > @@ -51,7 +51,8 @@ static int uas_find_endpoints(struct usb_host_interface > *alt, > } > > static int uas_use_uas_driver(struct usb_interface *intf, > - const struct usb_device_id *id) > + const struct usb_device_id *id, > + unsigned long *flags_ret) > { > struct usb_host_endpoint *eps[4] = { }; > struct usb_device *udev = interface_to_usbdev(intf); > @@ -132,5 +133,8 @@ static int uas_use_uas_driver(struct usb_interface *intf, > return 0; > } > > + if (flags_ret) > + *flags_ret = flags; > + > return 1; > } > diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c > index 6cdabdc..c6109c1 100644 > --- a/drivers/usb/storage/uas.c > +++ b/drivers/usb/storage/uas.c > @@ -887,8 +887,9 @@ static int uas_probe(struct usb_interface *intf, const > struct usb_device_id *id) > struct Scsi_Host *shost = NULL; > struct uas_dev_info *devinfo; > struct usb_device *udev = interface_to_usbdev(intf); > + unsigned long dev_flags; > > - if (!uas_use_uas_driver(intf, id)) > + if (!uas_use_uas_driver(intf, id, &dev_flags)) > return -ENODEV; > > if (uas_switch_interface(udev, intf)) > @@ -910,8 +911,7 @@ static int uas_probe(struct usb_interface *intf, const > struct usb_device_id *id) > devinfo->udev = udev; > devinfo->resetting = 0; > devinfo->shutdown = 0; > - devinfo->flags = id->driver_info; > - usb_stor_adjust_quirks(udev, &devinfo->flags); > + devinfo->flags = dev_flags; > init_usb_anchor(&devinfo->cmd_urbs); > init_usb_anchor(&devinfo->sense_urbs); > init_usb_anchor(&devinfo->data_urbs); > diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c > index 5600c33..db6f6b5 100644 > --- a/drivers/usb/storage/usb.c > +++ b/drivers/usb/storage/usb.c > @@ -1080,7 +1080,7 @@ static int storage_probe(struct usb_interface *intf, > > /* If uas is enabled and this device can do uas then ignore it. */ > #if IS_ENABLED(CONFIG_USB_UAS) > - if (uas_use_uas_driver(intf, id)) > + if (uas_use_uas_driver(intf, id, NULL)) > return -ENXIO; > #endif > -- 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: Unable to access USB mass storage device with xhci. okay with ehci
On Thu, 2015-04-16 at 13:49 +0200, Hans de Goede wrote: > Hi, > > On 09-04-15 20:30, Steve Bangert wrote: > > On Thu, 2015-04-09 at 16:49 +0200, Hans de Goede wrote: > >> Hi, > >> > >> On 09-04-15 15:27, Steve Bangert wrote: > >>> On Wed, 2015-04-08 at 10:56 -0400, Alan Stern wrote: > >>>> On Wed, 8 Apr 2015, Steve Bangert wrote: > >>>> > >>>>> What i did was not correct, usb-storage.quirks=174c:55aa:u was added to > >>>>> /etc/default/grub file after GRUB_CMDLINE_LINUX="rhgb quiet", using > >>>>> the grub-configurator tool > >>>>> followed by a reboot and i now have a working usb storage device > >>>>> using xhci. Here's the usbmon trace and the dmesg log is attached. > >>>>> > >>>>> I also found a comment in the source code file (uas-detect.h see > >>>>> attached) that states that my device, the ASMedia bridge is not > >>>>> supported by the uas driver. > >>>> > >>>> The comment does not say that. It refers to a different model from > >>>> yours. You can tell because the comment says that the non-working > >>>> ASM1051 supports 32 streams, whereas your device supports 16. > >>>> > >>>> In fact, as far as I can tell, your device _does_ work with the uas > >>>> driver provided max_sectors isn't too high (i.e., <= 240, which is the > >>>> default value used by usb-storage). Unfortunately, the uas driver > >>>> doesn't provide any way to set max_sectors. > >>>> > >>>> You can test this by adding a line that says: > >>>> > >>>> .max_sectors = 240, > >>>> > >>>> anywhere inside the definition of uas_host_template in the uas.c source > >>>> file. > >>>> > >>>>> So my next question is there a way to bind usb-storage to this device > >>>>> and have a working device out of the box without the hassle of adding > >>>>> a module parameter? > >>>> > >>>> We should ask Hans, since he wrote the comment and code in uas-detect.h > >>>> and presumably has a better understanding of the situation. > >>>> > >>> > >>> uas.c was patched (see attached), the quirk line was temporarily removed > >> > >> Thanks for testing this. > >> > >>> and i now have access to the drive using xhci and uas. usbmon trace is > >>> attached. > >> > >> What exactly do you mean with "i now have access to the drive using xhci > >> and uas" ? > >> Do you mean that everything works correctly with xhci + uas when setting > >> .max_sectors = 240 ? > >> > >> Alan, any ideas on how to move forward with this, I do not want to limit > >> all > >> uas devices to .max_sectors = 240, I can whip up a patch to set > >> max_sectors = 240 > >> on the ASM1053 but not the ASM1153. > >> > >> Steve, can you send me the output of lsusb -vv for the device in question ? > > > > Hans, thanks for you assistance with this concern. Steve > > > > Bus 002 Device 002: ID 174c:55aa ASMedia Technology Inc. ASM1051 SATA > > 3Gb/s bridge > > Couldn't open device, some information will be missing > > Device Descriptor: > >bLength18 > >bDescriptorType 1 > >bcdUSB 3.00 > >bDeviceClass0 (Defined at Interface level) > >bDeviceSubClass 0 > >bDeviceProtocol 0 > >bMaxPacketSize0 9 > >idVendor 0x174c ASMedia Technology Inc. > >idProduct 0x55aa ASM1051 SATA 3Gb/s bridge > > > > Thanks, I've been running some tests with my own asmedia devices > as I was hoping that max_sectors = 240 would maybe also fix the > problems with the early asm1051 devices, but no such luck. > > I've prepared a patch sets which adds a US_FL_MAX_SECTORS_240 flag and > sets that on asm105x devices like yours. > > I'll Cc you when I submit the patchset upstream, please give it a test > run. > > Regards, > > Hans I'll do that. Thank you for your efforts Hans And to Alan Stern for his excellent support. Steve > -- > 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: Unable to access USB mass storage device with xhci. okay with ehci
On Thu, 2015-04-09 at 11:07 -0400, Alan Stern wrote: > On Thu, 9 Apr 2015, Hans de Goede wrote: > > > > uas.c was patched (see attached), the quirk line was temporarily removed > > > > Thanks for testing this. > > > > > and i now have access to the drive using xhci and uas. usbmon trace is > > > attached. > > > > What exactly do you mean with "i now have access to the drive using xhci > > and uas" ? > > Do you mean that everything works correctly with xhci + uas when setting > > .max_sectors = 240 ? > > Yes, that's what Steve meant. Without the .max_sectors = 240 > restriction, the device froze up when the kernel tried to perform a > transfer that was too large (around 190 KB). > > > Alan, any ideas on how to move forward with this, I do not want to limit all > > uas devices to .max_sectors = 240, I can whip up a patch to set max_sectors > > = 240 > > on the ASM1053 but not the ASM1153. > > Some sort of quirk definitely seems like the right thing to do. Given > the peculiarities of this device family, the quirk probably has to be > implemented in code (as opposed to a static table à la > unusual_devs.h). > Still, I can imagine the uas-detect.h code setting the > US_FL_MAX_SECTORS_64 flags bit and then the uas driver calling > blk_queue_max_hw_sectors() the way we do in > scsiglue.c:slave_configure(). Alan, would this approach create/implement dynamic max sector values that might optimize speed/performance of the drive(s) transfer speeds? The reason i ask is that the performance of the drive seems slow and there were intermittent hangs during transfers based on my limited use of the drive so far. Steve > > 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: Unable to access USB mass storage device with xhci. okay with ehci
On Thu, 2015-04-09 at 16:49 +0200, Hans de Goede wrote: > Hi, > > On 09-04-15 15:27, Steve Bangert wrote: > > On Wed, 2015-04-08 at 10:56 -0400, Alan Stern wrote: > >> On Wed, 8 Apr 2015, Steve Bangert wrote: > >> > >>> What i did was not correct, usb-storage.quirks=174c:55aa:u was added to > >>> /etc/default/grub file after GRUB_CMDLINE_LINUX="rhgb quiet", using > >>> the grub-configurator tool > >>> followed by a reboot and i now have a working usb storage device > >>> using xhci. Here's the usbmon trace and the dmesg log is attached. > >>> > >>> I also found a comment in the source code file (uas-detect.h see > >>> attached) that states that my device, the ASMedia bridge is not > >>> supported by the uas driver. > >> > >> The comment does not say that. It refers to a different model from > >> yours. You can tell because the comment says that the non-working > >> ASM1051 supports 32 streams, whereas your device supports 16. > >> > >> In fact, as far as I can tell, your device _does_ work with the uas > >> driver provided max_sectors isn't too high (i.e., <= 240, which is the > >> default value used by usb-storage). Unfortunately, the uas driver > >> doesn't provide any way to set max_sectors. > >> > >> You can test this by adding a line that says: > >> > >>.max_sectors = 240, > >> > >> anywhere inside the definition of uas_host_template in the uas.c source > >> file. > >> > >>> So my next question is there a way to bind usb-storage to this device > >>> and have a working device out of the box without the hassle of adding > >>> a module parameter? > >> > >> We should ask Hans, since he wrote the comment and code in uas-detect.h > >> and presumably has a better understanding of the situation. > >> > > > > uas.c was patched (see attached), the quirk line was temporarily removed > > Thanks for testing this. > > > and i now have access to the drive using xhci and uas. usbmon trace is > > attached. > > What exactly do you mean with "i now have access to the drive using xhci and > uas" ? > Do you mean that everything works correctly with xhci + uas when setting > .max_sectors = 240 ? > > Alan, any ideas on how to move forward with this, I do not want to limit all > uas devices to .max_sectors = 240, I can whip up a patch to set max_sectors = > 240 > on the ASM1053 but not the ASM1153. > > Steve, can you send me the output of lsusb -vv for the device in question ? Hans, thanks for you assistance with this concern. Steve Bus 002 Device 002: ID 174c:55aa ASMedia Technology Inc. ASM1051 SATA 3Gb/s bridge Couldn't open device, some information will be missing Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x174c ASMedia Technology Inc. idProduct 0x55aa ASM1051 SATA 3Gb/s bridge bcdDevice1.00 iManufacturer 2 iProduct3 iSerial 1 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 121 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower 36mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk-Only iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 15 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x02 EP 2 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval
Re: Unable to access USB mass storage device with xhci. okay with ehci
On Sun, 2015-04-05 at 10:31 -0400, Alan Stern wrote: > On Sun, 5 Apr 2015, Steve Bangert wrote: > > > On Sat, 2015-04-04 at 15:48 -0400, Alan Stern wrote: > > > > > > > > Is that really all? Was there a "UAS is blacklisted for this device" > > > line in the kernel log? It looks like the usb-storage driver didn't do > > > anything at all with the device. > > > > Yes that's it. Using xhci at super speed, blacklisting uas is ignored > > and the uas module apparently loaded anyway. > > Blacklisting isn't enough. I don't know why it didn't prevent the > module from being loaded, but it doesn't matter. Not only do you have > to tell the system _not_ to load the uas driver; you also have to tell > the system that it _should_ use the usb-storage driver. Otherwise > usb-storage will see that the device is capable of using UAS and will > avoid binding to it. > > To do this, you need to set the usb-storage "quirks" module parameter > to "174c:55aa:u". The 'u' quirk means to ignore the possibility of > using UAS. Here's dmesg using the kernel command line you provided: I can provide the full dmesg output/text if you wish in a follow up email. [ 65.322218] Bluetooth: RFCOMM socket layer initialized [ 65.322282] Bluetooth: RFCOMM ver 1.11 [ 95.963139] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 3 bytes untransferred [ 95.971138] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 1 bytes untransferred [ 100.225570] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 3 bytes untransferred [ 100.233605] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 1 bytes untransferred [ 104.405138] xhci_hcd :00:14.0: Waiting for status stage event [ 104.445247] xhci_hcd :00:14.0: Waiting for status stage event [ 104.466125] xhci_hcd :00:14.0: Waiting for status stage event [ 104.496071] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 3 bytes untransferred [ 104.504069] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 1 bytes untransferred [ 104.507745] xhci_hcd :00:14.0: Waiting for status stage event [ 104.588452] xhci_hcd :00:14.0: Waiting for status stage event [ 104.609339] xhci_hcd :00:14.0: Waiting for status stage event [ 104.630124] xhci_hcd :00:14.0: Waiting for status stage event [ 104.689963] xhci_hcd :00:14.0: Waiting for status stage event [ 104.710953] xhci_hcd :00:14.0: Waiting for status stage event [ 104.731570] xhci_hcd :00:14.0: Waiting for status stage event [ 104.752552] xhci_hcd :00:14.0: Waiting for status stage event [ 104.773191] xhci_hcd :00:14.0: Waiting for status stage event [ 104.794124] xhci_hcd :00:14.0: Waiting for status stage event [ 104.835897] xhci_hcd :00:14.0: Waiting for status stage event [ 104.855476] xhci_hcd :00:14.0: Waiting for status stage event [ 104.876310] xhci_hcd :00:14.0: Waiting for status stage event [ 108.758537] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 3 bytes untransferred [ 108.766533] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 1 bytes untransferred [ 113.029004] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 3 bytes untransferred [ 113.036993] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 1 bytes untransferred [ 117.299461] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 3 bytes untransferred [ 117.307460] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 1 bytes untransferred [ 118.954569] xhci_hcd :00:14.0: Port Status Change Event for port 2 [ 118.954583] xhci_hcd :00:14.0: handle_port_status: starting port polling. [ 118.955821] xhci_hcd :00:14.0: get port status, actual port 1 status = 0x202a0 [ 118.955833] xhci_hcd :00:14.0: Get port status returned 0x10100 [ 118.955878] xhci_hcd :00:14.0: clear port connect change, actual port 1 status = 0x2a0 [ 118.955903] xhci_hcd :00:14.0: get port status, actual port 1 status = 0x2a0 [ 118.955910] xhci_hcd :00:14.0: Get port status returned 0x100 [ 118.981968] xhci_hcd :00:14.0: get port status, actual port 1 status = 0x2a0 [ 118.981976] xhci_hcd :00:14.0: Get port status returned 0x100 [ 119.007965] xhci_hcd :00:14.0: get port status, actual port 1 status = 0x2a0 [ 119.007973] xhci_hcd :00:14.0: Get port status returned 0x100 [ 119.033950] xhci_hcd :00:14.0: get port status, actual port 1 status = 0x2a0 [ 119.033957] xhci_hcd :00:14.0: Get port status returned 0x100 [ 119.059935] xhci_hcd :00:14.0: get port status, actual port 1 status = 0x2a0 [ 119.059942] xhci_hcd :00:14.0: Get port status returned 0x100 [ 119.103881] xhci_hcd :00:14.0: xhci_hub_status_data: stopping port polling. [ 121.569913] xhci_hcd :00:14.0: ep 0x81 - asked for 5 bytes, 3 bytes untransferre
Re: Unable to access USB mass storage device with xhci. okay with ehci
the first > problem in the xHCI trace shows up when we try to transfer more than > 122880 bytes. A 196608-byte transfer has to be aborted after 30 > seconds, and only 65536 bytes were received. (Note that 122880-byte > transfers succeeded in both the EHCI and xHCI-with-a-USB-2-cable > traces.) > > Perhaps this drive needs some sort of max_sectors restriction. > > Alan Stern Okay, is there a patch i can try? Thanks for your help with this. Steve -- 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
Unable to access USB mass storage device with xhci. okay with ehci
Alan or Mathias, Machine is a 64 bit Dell Inspiron with Intel chips running Fedora 3.19.3 kernel Disabling the uas driver is no help. Any help is appreciated. Steve lspci -v 00:14.0 USB controller: Intel Corporation 8 Series/C220 Series Chipset Family USB xHCI (rev 05) (prog-if 30 [XHCI]) Subsystem: Dell Device 0622 Flags: bus master, medium devsel, latency 0, IRQ 30 Memory at f7e0 (64-bit, non-prefetchable) [size=64K] Capabilities: [70] Power Management version 2 Capabilities: [80] MSI: Enable+ Count=1/8 Maskable- 64bit+ Kernel driver in use: xhci_hcd lsusb -v Bus 002 Device 002: ID 174c:55aa ASMedia Technology Inc. ASM1051 SATA 3Gb/s bridge Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x174c ASMedia Technology Inc. idProduct 0x55aa ASM1051 SATA 3Gb/s bridge bcdDevice1.00 iManufacturer 2 Plugable iProduct3 USB3-SATA-UASP1 iSerial 1 123456789014 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 121 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower 36mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk-Only iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 15 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x02 EP 2 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 15 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 1 bNumEndpoints 4 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 98 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 15 MaxStreams 16 Data-in pipe (0x03) Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x02 EP 2 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 15 MaxStreams 16 Data-out pipe (0x04) Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 15 MaxStreams 16 Status pipe (0x02) Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x04 EP 4 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 0 Command pipe (0x01) Binary Object Store Descriptor: bLength 5 bDescriptorType15 wTotalLength 22
Re: XHCI, "brain-dead scanner", and microframe rounding
Mathias Nyman writes: Many changes were done in 3.18 and 3.19-rc release regarding to how we handle halted and stopped endpoints, Can you test if that fixes your issues? preferably with a 3.19-rc kernel Here's the result of 3.19-rc4: 2015-01-20T15:26:41.242455+01:00 xrated kernel: [ 37.979496] usb 6-10.3.1: new high-speed USB device number 11 using xhci_hcd 2015-01-20T15:26:41.329458+01:00 xrated kernel: [ 38.066734] usb 6-10.3.1: New USB device found, idVendor=04b8, idProduct=0119 2015-01-20T15:26:41.329477+01:00 xrated kernel: [ 38.066742] usb 6-10.3.1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 2015-01-20T15:26:41.329480+01:00 xrated kernel: [ 38.066746] usb 6-10.3.1: Product: EPSON Scanner 2015-01-20T15:26:41.329481+01:00 xrated kernel: [ 38.066749] usb 6-10.3.1: Manufacturer: EPSON 2015-01-20T15:26:41.329483+01:00 xrated kernel: [ 38.067029] usb 6-10.3.1: ep 0x81 - rounding interval to 128 microframes, ep desc says 255 microframes 2015-01-20T15:26:41.329484+01:00 xrated kernel: [ 38.067035] usb 6-10.3.1: ep 0x2 - rounding interval to 128 microframes, ep desc says 255 microframes Plugging the scanner to a USB2 port works flawlessly. Pete -- 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 I have the same experience steve -- 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, "brain-dead scanner", and microframe rounding
Any progress on this one? Quite a bunch of people, that try to use a scanner on a xhci port gets bitten by this issue. Given, that modern haswell mainboards tend to only support xhci ports anymore, this is a real showstopper and kind of regression from users POV. https://bugzilla.novell.com/show_bug.cgi?id=856794 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1102797 Would be nice to cc me on answers in order to relay results to other suffering people. To me it looked like the issue was fixes for several people by updating the system, and I didn't follow up on this issue that much. I'm stating to abandon the microframe interval theory and think this is related to endpoint halting or toggle bit status. checking the usbmon output it looks like scanner code regularly does a clear_feature[endpoint halt]. Many changes were done in 3.18 and 3.19-rc release regarding to how we handle halted and stopped endpoints, Can you test if that fixes your issues? preferably with a 3.19-rc kernel If that still doesn't do it then you might be one hitting this issue in xchi.c: /* * We might need to implement the config ep cmd in xhci 4.8.1 note: * The Reset Endpoint Command may only be issued to endpoints in the * Halted state. If software wishes reset the Data Toggle or Sequence * Number of an endpoint that isn't in the Halted state, then software * may issue a Configure Endpoint Command with the Drop and Add bits set * for the target endpoint. that is in the Stopped state. */ Which is not implemented yet -Mathias I am running 3.18 and have a brain dead Fujitsu scanner it doesn't work. i mistakenly submitted a no subject post about 4 wks ago I have enough knowledge to up load and test the fixes but not the knowledge to write the code. I filed a bug on bugs.launchpad.net on kernel 3.18 04c5:11fc [System76 gazp9b] Excuting scanimage -L finds Fujitsu 6110 Scanner but if repeated does not with url https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1402335 -steve -- 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
[no subject]
1) Summary pasted summary: scanimage -L finds fitjitsu 04c5:11fc but if repeated does not find it + 04c5:11fc [System76 gazp9b] Excuting scanimage -L finds Fujitsu 6110 + Scanner but if repeated does not 2) full description: i plug a fitjitsu 04c5:11fc into a usb2.0 plug lsusb shows the equipment command line simple-scan, scanimage -L or sane-find_scanner does not find it 3) Keywords 4) kernel version 3.18-vivid 5) syslog: Dec 14 12:53:48 skc kernel: [ 219.200113] usb 1-2: new high-speed USB device number 5 using xhci_hcd Dec 14 12:53:48 skc kernel: [ 219.390380] usb 1-2: New USB device found, idVendor=04c5, idProduct=11fc Dec 14 12:53:48 skc kernel: [ 219.390384] usb 1-2: New USB device strings: Mfr=0, Product=0, SerialNumber=0 Dec 14 12:53:48 skc kernel: [ 219.390563] usb 1-2: ep 0x81 - rounding interval to 128 microframes, ep desc says 255 microframes Dec 14 12:53:48 skc kernel: [ 219.390567] usb 1-2: ep 0x2 - rounding interval to 128 microframes, ep desc says 255 microframes Dec 14 12:53:48 skc colord: Device added: sysfs-04c5-11fc 6) scanimage -L simple-scan sane-find-scanner do not find any scanner 7) lsb_release -rd Description:Ubuntu 14.04.1 LTS Release:14.04 7.1) ver_linux If some fields are empty or look unusual you may have an old version. Compare to the current minimal requirements in Documentation/Changes. Linux skc 3.18.0-031800-generic #201412071935 SMP Mon Dec 8 00:36:34 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Gnu C 4.8 Gnu make 3.81 binutils 2.24 util-linux 2.20.1 mount support module-init-tools 15 e2fsprogs 1.42.9 pcmciautils018 PPP2.4.5 Linux C Library2.19 Dynamic linker (ldd) 2.19 Procps 3.3.9 Net-tools 1.60 Kbd1.15.5 Sh-utils 8.21 wireless-tools 30 Modules Loaded xt_conntrack ipt_REJECT nf_reject_ipv4 ip6table_filter ip6_tables ebtable_nat ebtables xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 ctr iptable_nat ccm nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack xt_tcpudp bridge stp llc iptable_filter ip_tables x_tables rfcomm bnep arc4 intel_rapl x86_pkg_temp_thermal intel_powerclamp iwlmvm coretemp mac80211 kvm_intel kvm uvcvideo snd_hda_codec_via videobuf2_vmalloc snd_hda_codec_generic snd_hda_codec_hdmi crct10dif_pclmul videobuf2_memops iwlwifi videobuf2_core crc32_pclmul v4l2_common ghash_clmulni_intel videodev aesni_intel media aes_x86_64 cfg80211 snd_hda_intel lrw btusb gf128mul joydev glue_helper bluetooth ablk_helper snd_hda_controller serio_raw cryptd rtsx_pci_ms snd_hda_codec memstick lpc_ich snd_hwdep mei_me i915 wmi mei snd_pcm tpm_infineon mac_hid snd_seq_midi snd_seq_midi_event snd_rawmidi video snd_seq drm_kms_helper snd_se q_device drm snd_timer snd i2c_algo_bit soundcore ie31200_edac parport_pc edac_core ppdev lp parport rtsx_pci_sdmmc psmouse r8169 ahci libahci mii rtsx_pci 7,2) /proc/cpuinfo processor: 0 vendor_id: GenuineIntel cpu family: 6 model: 60 model name: Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz stepping: 3 microcode: 0x17 cpu MHz: 2515.917 cache size: 6144 KB physical id: 0 siblings: 8 core id: 0 cpu cores: 4 apicid: 0 initial apicid: 0 fpu: yes fpu_exception: yes cpuid level: 13 wp: yes flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm ida arat epb pln pts dtherm tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid xsaveopt bugs: bogomips: 4988.89 clflush size: 64 cache_alignment: 64 address sizes: 39 bits physical, 48 bits virtual power management: processor: 1 vendor_id: GenuineIntel cpu family: 6 model: 60 model name: Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz stepping: 3 microcode: 0x17 cpu MHz: 2500.000 cache size: 6144 KB physical id: 0 siblings: 8 core id: 1 cpu cores: 4 apicid: 2 initial apicid: 2 fpu: yes fpu_exception: yes cpuid level: 13 wp: yes flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 movbe popcnt tsc_deadline_timer aes xsa
Re: [PATCH] usb: core: allow zero packet flag for interrupt urbs
On Thu, Jul 17, 2014 at 12:32 PM, Alan Stern wrote: > On Thu, 17 Jul 2014, Steve Calfee wrote: > >> Hi Alan, >> >> I did some testing with multi interrupt transfers some time ago. You >> can get allocated a guaranteed 3x1024 time slot per uframe for an >> interval of your choice on usb 2.0. So zlps are needed to send exactly >> 1024 or 2048 bytes in that time slot. > > That's true, but misleading. Use of zero-length packets is relatively > independent of the time slot divisions. zlps are necessary to mark the > end of a multi-packet transfer; they aren't necessary if all you want > to do is send less than 3072 bytes during a single 3x1024 time slot. > > Consider also that the URB_ZERO_PACKET flag will give you at most one > zlp. That won't be good enough if you want to send exactly 1024 bytes > in the time slot; you would need two zlps. Besides, why would you want > to limit the amount of data that gets sent during the time slot? If > you have more than 1024 or 2048 bytes, why not send all of it? > > (For that matter, what happens if you want to send 2000 bytes during > the time slot? The first packet will contain 1024 bytes, the second > will contain the remaining 976, and there won't be a zlp even if > URB_ZERO_PACKET is set. Instead, the third packet will contain > whatever data has been queued next. I was a little surprised to see > that section 4.10.3 in the EHCI spec _doesn't_ say the controller must > leave the Exercute Transaction state when a short packet is > encountered -- even though not doing so would violate the USB-2.0 > spec.) > > Conversely, if you have only 1024 bytes to send, you don't need to do > anything special. Just send them. There won't be any zlp during the > time slot, but if no data is queued then nothing will be sent after > those 1024 bytes. > Hi Alan, It has been a few years since I was doing this, but here is my understanding. If a device descriptor says x bytes should be reserved (1 to 3072), the host will allocate that much bandwidth. If the sender sends some number of bytes between 1 and 3072 (must be less than or equal to x allocation), everything is ok the receiver will know when the sender is done. But (as with bulk xfers) if exactly 1024 or 2048 bytes is sent, the receiver will not know the xfer is done. So in those cases, just as in bulk, the sender needs to send a ZLP. The receiver will request up to 3 int xfers (depending on x>2048 or x >1024), and will stop issuing requests (on a host sender) on a short transfer or a full xfer. Timeslots are guaranteed reservations, but they are not automatically always used. Unused bandwidth in a uframe (after reserved uses what it wants) will be used as needed by bulk or control transfers. Regards, Steve -- 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: core: allow zero packet flag for interrupt urbs
On Thu, Jul 17, 2014 at 7:55 AM, Alan Stern wrote: > > I can't say this is actually wrong, but have you ever encountered a > situation where this would be needed? How often does anyone need to do > a multi-packet transfer over an interrupt endpoint? > Hi Alan, I did some testing with multi interrupt transfers some time ago. You can get allocated a guaranteed 3x1024 time slot per uframe for an interval of your choice on usb 2.0. So zlps are needed to send exactly 1024 or 2048 bytes in that time slot. Also, interrupt transfers are crc checked and resent (as long as uframe time is still available). So you get guaranteed timing like iso sends, and guaranteed data like bulk sends. This could be valuable for time sensitive apps with known data capacities. Microsoft only started supporting multi-interrupt transfers in Windows Vista and beyond. Winxp refuses to die, so probably no one has built an actual hardware device for consumers that uses multi-interrupt xfers up to now. Regards, Steve -- 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: disable VBUS?
You are right, the device will get 5 volts. However it is still part of the spec that 2ms without a SOF will force the device into suspend mode, so it will use less than 2ma. So depending on your definition of function, there will be power, but no activity. If it uses more than 2ma, from the bus, problems will ensue. Steve On Mon, Jul 7, 2014 at 6:00 PM, Grant wrote: >> it isn't possible to disable Vbus on a USB host port and still use the >> port. To do what you want, you would have to physically cut the Vbus >> wire in the USB cable and splice the device side of that wire to the >> external power supply. > > > I think I'll try that. Can anyone confirm that it should work? If I > connect a 5V external power supply to the VBUS wire in a USB cable, > the connected USB device should still function? > > - Grant > -- > 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: Buffer I/O error after s2ram with usb storage persistance
On Fri, Apr 18, 2014 at 17:55 +0200, Matthieu CASTET wrote: > Le Fri, 18 Apr 2014 10:42:14 -0400, > Alan Stern a écrit : > > > > Since the SCSI layer thinks the media was changed, it has no choice but > > to fail the read request. > ok. > > I suppose the scsi layer can't ignore "Not-ready to ready change" event > at resume time ? I think the current behaviour might be the safer option. Scenarios: The USB key is really a phone or camera in mass-storage mode, and the filesystem is automounted when the PC disconnects. The key was put in to another machine, and then put back in to the original before resuming from s2ram. For example, suspending a laptop, forgetting that the key is mounted, and using it to transfer files from the desktop. However, I haven't tested either of these scenarios. Steve -- 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] xhci: Allocate the td array and urb_priv together.
On Wed, Dec 18, 2013 at 3:24 AM, David Laight wrote: > > This saves a kzalloc() call on every transfer and some memory > indirections. > > The only possible downside is for isochronous tranfers with 64 td > when the allocate is 8+4096 bytes (on 64bit systems) so requires > an additional page. > > Signed-off-by: David Laight > --- > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 6e0d886..0969f74 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1248,12 +1248,11 @@ static int xhci_check_maxpacket(struct xhci_hcd > *xhci, unsigned int slot_id, > int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) > { > struct xhci_hcd *xhci = hcd_to_xhci(hcd); > - struct xhci_td *buffer; > unsigned long flags; > int ret = 0; > unsigned int slot_id, ep_index; > struct urb_priv *urb_priv; > - int size, i; > + int size; > > if (!urb || xhci_check_args(hcd, urb->dev, urb->ep, > true, true, __func__) <= 0) > @@ -1275,21 +1274,10 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb > *urb, gfp_t mem_flags) > size = 1; > > urb_priv = kzalloc(sizeof(struct urb_priv) + > - size * sizeof(struct xhci_td *), mem_flags); > + size * sizeof(struct xhci_td), mem_flags); > if (!urb_priv) > return -ENOMEM; > > - buffer = kzalloc(size * sizeof(struct xhci_td), mem_flags); > - if (!buffer) { Hi David, Are you sure this will not cause weird bugs on non-cache coherent systems? You are smashing two memory allocations into one. This does not insure that the memory in each allocation is cache line aligned does it? Arm and Mips systems don't like having two i/o actions or cpu and i/o activity on data sharing a cache line. Regards, Steve -- 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/9] ARM: fix ohci-pxa27x build error with OF enabled
On Mon, Dec 09, 2013 at 02:53 +0400, Sergei Ianovich wrote: > --->8--- > $ make > CC [M] drivers/usb/host/ohci-pxa27x.o > drivers/usb/host/ohci-pxa27x.c: In function ‘ohci_pxa_of_init’: > drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function > ‘dma_coerce_mask_and_coherent’ [-Werror=implicit-function-declaration] > drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function > ‘DMA_BIT_MASK’ [-Werror=implicit-function-declaration] > --->8--- > > Signed-off-by: Sergei Ianovich > CC: Russell King - ARM Linux > > # Changes to be committed: > --- > drivers/usb/host/ohci-pxa27x.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c > index e89ac4d..97983fd 100644 > --- a/drivers/usb/host/ohci-pxa27x.c > +++ b/drivers/usb/host/ohci-pxa27x.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > #include Hi Sergei, There's already a different patch for this in the linux-next/master and gregkh/usb/usb-linus trees, but not in the linux-next/stable or gregkh/usb/usb-next trees. It adds that include 3 lines higher up, to keep the includes in alphabetical order. commit 9876388edfa553960815110acae4544359b385b5 Author: Daniel Mack AuthorDate: Fri Nov 15 14:19:02 2013 +0100 Commit: Greg Kroah-Hartman CommitDate: Wed Dec 4 16:57:46 2013 -0800 Cheers, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND] dma: cppi41: only allocate descriptor memory once
On Wed, Dec 04, 2013 at 11:21 +0100, Daniel Mack wrote: > While at it, remove the intermediate variable mem_decs (I guess it was > only there to make the code comply to the 80-chars CodingSytle rule). purge_descs() still has a (now unused) mem_decs intermediate variable. Regards, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] ohci-pci: devices not detected after hibernate
On Tue, Aug 20, 2013 at 03:43:28PM -0400, Alan Stern wrote: > On Tue, 20 Aug 2013, Steve Cotton wrote: > > > Once my PC has been hibernated and resumed, some devices plugged > > in to the on-motherboard ports stop working, and don't show up in > > lsusb. The pattern seems to be that ports with USB 1.1 devices > > are affected, but ports with USB 2.0 devices are not. > > My god, I can't believe this got left out! The lines in the patch > below should have been part of that c1117afb85 commit all along. It's > amazing that nobody has complained before this. > > Does this patch fix the problem? > > Alan Stern > > > > > Index: usb-3.11/drivers/usb/host/ohci-pci.c > === > --- usb-3.11.orig/drivers/usb/host/ohci-pci.c > +++ usb-3.11/drivers/usb/host/ohci-pci.c > @@ -304,6 +304,11 @@ static int __init ohci_pci_init(void) > pr_info("%s: " DRIVER_DESC "\n", hcd_name); > > ohci_init_driver(&ohci_pci_hc_driver, &pci_overrides); > + > + /* Entries for the PCI suspend/resume callbacks are special */ > + ohci_pci_hc_driver.pci_suspend = ohci_suspend; > + ohci_pci_hc_driver.pci_resume = ohci_resume; > + > return pci_register_driver(&ohci_pci_driver); > } > module_init(ohci_pci_init); Thanks Alan, that patch fixes it. Cheers, Steve -- 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
[BUG] ohci-pci: devices not detected after hibernate
Last working release: 3.10 Bisected earliest broken: c1117afb85 Broken in: 3.11-rc1, 3.11-rc6, bd479f2933 (the current usb-next) Once my PC has been hibernated and resumed, some devices plugged in to the on-motherboard ports stop working, and don't show up in lsusb. The pattern seems to be that ports with USB 1.1 devices are affected, but ports with USB 2.0 devices are not. It's an x86-amd64 desktop, with an MSI motherboard based on the AMD 970 chipset. Ports in USB 1.1 mode use the ohci-pci driver. I've bisected the problem to c1117afb85, included in v3.11-rc1. c1117afb85 USB: OHCI: make ohci-pci a separate driver If the PC is booted without a resume image, everything works. It's running Debian unstable with a local kernel build, and to hibernate I'm using the "pm-hibernate" script from pm-utils. For a sanity check of my local builds, I've seen the same behaviour in the Debian archive's build of 3.11-rc4. Using an optical mouse as my main test device: The mouse doesn't show up in lsusb, and the light (optical mouse) is off. It doesn't sent any input to X11. Plugging the mouse in to a different USB port on the motherboard will show brief activity in usbmon, but again it doesn't work, doesn't show in lsusb and doesn't light up. Once the mouse has been plugged in to a USB port, no device will be detected in that physical port again until the PC is rebooted. Nothing will show in dmesg or usbmon when another device is plugged or unplugged (even with CONFIG_USB_DEBUG). USB 2.0 devices still work (but not in ports that a 1.1 device has previously been plugged in to). An external USB 2.0 hub does still show up after hibernation, and the same USB 1.1 mouse works if plugged in to this hub. Attached are logs from bd479f2933 with CONFIG_USB_DEBUG. There's no physical movement of plugs in this log, just hibernating and resuming with the same devices plugged in. The keyboard is PS/2, not USB. Affected devices in the log: 046d:c03d Logitech, Inc. M-BT96a Pilot Optical Mouse 04b8:0101 Seiko Epson Corp. GT-7000U [Perfection 636] 046d:c216 Logitech, Inc. Dual Action Gamepad Unaffected devices in the log: 0424:2514 Standard Microsystems Corp. USB 2.0 Hub 04cb:01c1 Fuji Photo Film Co., Ltd FinePix F31fd (PTP) 04e8:681c Samsung Electronics Co., Ltd Galaxy Portal/Spica/S 04e8:6860 Samsung Electronics Co., Ltd GT-I9100 Phone In the lsusb output, the USB 1.1 root hubs' power settings change (diff of before and after hibernation): Hub Descriptor: bLength 9 bDescriptorType 41 nNbrPorts 5 - wHubCharacteristic 0x0012 -No power switching (usb 1.0) + wHubCharacteristic 0x0011 +Per-port power switching No overcurrent protection bPwrOn2PwrGood2 * 2 milli seconds bHubContrCurrent 0 milli Ampere DeviceRemovable0x00 PortPwrCtrlMask0xff Hub Port Status: - Port 1: .0100 power - Port 2: .0100 power - Port 3: .0303 lowspeed power enable connect - Port 4: .0100 power - Port 5: .0100 power + Port 1: . + Port 2: . + Port 3: . + Port 4: . + Port 5: . Device Status: 0x0001 Self Powered usb_next_bd479f2__no_x11.tar.bz2 Description: Binary data
[PATCH v2] usb: serial: Add Rigblaster Advantage to device table
The Rigblaster Advantage is an amateur radio interface sold by West Mountain Radio. It contains a cp210x serial interface but the device ID is not in the driver. Signed-off-by: Steve Conklin --- drivers/usb/serial/cp210x.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index edc0f0d..78721ba 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -85,6 +85,7 @@ static const struct usb_device_id id_table[] = { { USB_DEVICE(0x10C4, 0x813F) }, /* Tams Master Easy Control */ { USB_DEVICE(0x10C4, 0x814A) }, /* West Mountain Radio RIGblaster P&P */ { USB_DEVICE(0x10C4, 0x814B) }, /* West Mountain Radio RIGtalk */ + { USB_DEVICE(0x2405, 0x0003) }, /* West Mountain Radio RIGblaster Advantage */ { USB_DEVICE(0x10C4, 0x8156) }, /* B&G H3000 link cable */ { USB_DEVICE(0x10C4, 0x815E) }, /* Helicomm IP-Link 1220-DVM */ { USB_DEVICE(0x10C4, 0x815F) }, /* Timewave HamLinkUSB */ -- 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: serial: Add Rigblaster Advantage to device table
The Rigblaster Advantage is an amateur radio interface sold by West Mountain Radio. It contains a cp210x serial interface but the device ID is not in the driver. Signed-off-by: Steve Conklin --- drivers/usb/serial/cp210x.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index edc0f0d..78721ba 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -85,6 +85,7 @@ static const struct usb_device_id id_table[] = { { USB_DEVICE(0x10C4, 0x813F) }, /* Tams Master Easy Control */ { USB_DEVICE(0x10C4, 0x814A) }, /* West Mountain Radio RIGblaster P&P */ { USB_DEVICE(0x10C4, 0x814B) }, /* West Mountain Radio RIGtalk */ + { USB_DEVICE(0x2405, 0x0003) }, /* West Mountain Radio RIGblaster Advantage */ { USB_DEVICE(0x10C4, 0x8156) }, /* B&G H3000 link cable */ { USB_DEVICE(0x10C4, 0x815E) }, /* Helicomm IP-Link 1220-DVM */ { USB_DEVICE(0x10C4, 0x815F) }, /* Timewave HamLinkUSB */ -- 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: Device configuration fails on superspeed, succeeds on highspeed
On Mon, Dec 17, 2012 at 11:08 AM, Steve Calfee wrote: > On Mon, Dec 17, 2012 at 7:33 AM, Alan Stern wrote: >> On Sun, 16 Dec 2012, Vincent Pelletier wrote: >> >>> Le dimanche 16 décembre 2012 20:46:38, Vincent Pelletier a écrit : >>> > I checked the specs, and the warnings about wMaxPacketSize seem >>> > justified (although it's unclear to me wether wMaxPacketSize is >>> > restricted to exactly 512B in HS, or only upper-bound at 512B). >>> >>> Another reply to myself: >>> Tested USB device uses a Cypress FX2LP: >>> >>> http://www.summitsoftconsulting.com/picts/iti_pcb_top.jpg >>> >>> I checked the FX2LP specs, and endpoint 1 in both directions is hardwired to >>> wMaxPacketSize of 64. I believe the FX2LP is quite widespread (judging by >>> the >>> availability of a GPL toolchain for it and two CLI tools to send its >>> firmware), so those warnings might cause redundant bug reports. >> >> It does seem odd that people would buy and use these chips even >> though they are explicitly in violation of the USB specification. A >> device containing one of these things could never pass the USB-CV >> verification test. >> >> It may be possible for xhci-hcd to work around the bug (by internally >> changing wMaxPacketSize to 512). >> > > It does not need to be worked around. The fx2 docs specifically say > ep1 cannot be used as a bulk endpoint in HS mode. Vincent, RTFM, > Cypress has good, downloadable chip docs. Ep1 can either be a legal > int/iso endpoint or just not used - all specified in the > device/interface descriptors. Nothing in the USB spec requires all > device endpoints to be used, or that interfaces have sequentially > numbered endpoints. > > Regards, Steve Hi, Vincent, After reading the first message in the thread I realized that you are not writing the fx2 device, but are trying to use a device you purchased. Sorry. The fx2 is not at fault, the device has a bad, invalid endpoint descriptor for ep1. I doubt if they are using the ep, just defining it. Perhaps you could contact the manufacturer? Regards, Steve -- 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: Device configuration fails on superspeed, succeeds on highspeed
On Mon, Dec 17, 2012 at 7:33 AM, Alan Stern wrote: > On Sun, 16 Dec 2012, Vincent Pelletier wrote: > >> Le dimanche 16 décembre 2012 20:46:38, Vincent Pelletier a écrit : >> > I checked the specs, and the warnings about wMaxPacketSize seem >> > justified (although it's unclear to me wether wMaxPacketSize is >> > restricted to exactly 512B in HS, or only upper-bound at 512B). >> >> Another reply to myself: >> Tested USB device uses a Cypress FX2LP: >> >> http://www.summitsoftconsulting.com/picts/iti_pcb_top.jpg >> >> I checked the FX2LP specs, and endpoint 1 in both directions is hardwired to >> wMaxPacketSize of 64. I believe the FX2LP is quite widespread (judging by the >> availability of a GPL toolchain for it and two CLI tools to send its >> firmware), so those warnings might cause redundant bug reports. > > It does seem odd that people would buy and use these chips even > though they are explicitly in violation of the USB specification. A > device containing one of these things could never pass the USB-CV > verification test. > > It may be possible for xhci-hcd to work around the bug (by internally > changing wMaxPacketSize to 512). > It does not need to be worked around. The fx2 docs specifically say ep1 cannot be used as a bulk endpoint in HS mode. Vincent, RTFM, Cypress has good, downloadable chip docs. Ep1 can either be a legal int/iso endpoint or just not used - all specified in the device/interface descriptors. Nothing in the USB spec requires all device endpoints to be used, or that interfaces have sequentially numbered endpoints. Regards, Steve -- 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: [PATCHv2][RFC] smsc95xx: enable dynamic autosuspend
On 11 December 2012 16:27, Joe Perches wrote: > On Tue, 2012-12-11 at 15:26 +0000, Steve Glendinning wrote: > []> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c > [] >> + ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val); >> + if (ret < 0) { >> + netdev_warn(dev->net, "Error reading RX_FIFO_INF"); > > Please remember terminating newlines. Oops, I've fixed this now in my tree. > If you are always going to warn bad reads or writes, > it'd be good to change smsc95xx_read_reg_nopm > and write to emit the message. > > It saves ~3% of code, 1K > $ size drivers/net/usb/smsc95xx.o* >textdata bss dec hex filename > 2706427246072 358608c14 drivers/net/usb/smsc95xx.o.new > 2792127246256 369019025 drivers/net/usb/smsc95xx.o.old > > Signed-off-by: Joe Perches Thanks for this Joe, actually as the register access functions already print a warning with the register number I don't think we need to log this at the calling site at all. I'll rip out all the warnings that don't add any significant new information (i.e. *why* the call may have failed, or what its failure means). -- Steve Glendinning -- 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: [PATCHv2][RFC] smsc95xx: enable dynamic autosuspend
On 11 December 2012 16:13, Ming Lei wrote: > On Tue, Dec 11, 2012 at 11:26 PM, Steve Glendinning > wrote: >> + >> + if (on) >> + usb_autopm_get_interface_no_resume(dev->intf); >> + else >> + usb_autopm_put_interface_no_suspend(dev->intf); > > The above line should be > > usb_autopm_put_interface(dev->intf); Thanks Ming, I've updated this. > IMO, it is better to keep smsc95xx_info.manage_power as NULL > for devices without FEATURE_AUTOSUSPEND, so that fewer code > and follow the current .mange_power usage(see its comment). > > Currently, if the function pointer of manage_power is set, it means that > the device supports USB autosuspend, but your trick will make the > assumption not true for some smsc devices. Oliver? -- Steve Glendinning -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb bus monitoring
Hi Tilman, Another option is to find and use fx2lib. It includes fx1 based stuff and should give you some ideas on how to at least make something work. It uses the open source SDCC compiler, so if you are using something else some of the syntax is different. Regards, Steve On Tue, Dec 11, 2012 at 2:58 PM, Tilman wrote: > Peter Stuge writes: > > >> Tilman wrote: > >> What device is that? Is it a Cypress FX2 or an SiLabs C8051? > ezusb (Cypress AN2131) > >> > The urb eventually times out. Via debug FS I can see that the urb is >> > indeed submitted. I suspect the firmware to be responsible for this. >> >> That's quite likely. > .. on the other hand, I do not see an obvious mistake when intializing the USB > core of the AN2131 :-) > >> > I would like to exclude the linux test driver owever, >> >> That doesn't make a lot of sense - if you take the driver away then >> there is nothing that will talk to your device, so there will be no >> traffic. > I did not mean to take the driver away -- I would like to exclude it as error > source, i.e. I understand that debugfs taps the communication between the > device > driver and the queue of the usb subsystem. It does not give me certainty that > the urb ever made it to the usb wire. > > Tilman > > -- > 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
[PATCHv2][RFC] smsc95xx: enable dynamic autosuspend
This patch enables USB dynamic autosuspend for LAN9500A. This saves very little power in itself, but it allows power saving in upstream hubs/hosts. The earlier devices in this family (LAN9500/9512/9514) do not support this feature. Signed-off-by: Steve Glendinning --- drivers/net/usb/smsc95xx.c | 159 +++- 1 file changed, 158 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 9b73670..bdd51fd 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -55,6 +55,13 @@ #define FEATURE_PHY_NLP_CROSSOVER (0x02) #define FEATURE_AUTOSUSPEND(0x04) +#define SUSPEND_SUSPEND0 (0x01) +#define SUSPEND_SUSPEND1 (0x02) +#define SUSPEND_SUSPEND2 (0x04) +#define SUSPEND_SUSPEND3 (0x08) +#define SUSPEND_ALLMODES (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \ +SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3) + struct smsc95xx_priv { u32 mac_cr; u32 hash_hi; @@ -62,6 +69,7 @@ struct smsc95xx_priv { u32 wolopts; spinlock_t mac_cr_lock; u8 features; + u8 suspend_flags; }; static bool turbo_mode = true; @@ -1341,6 +1349,8 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev) if (ret < 0) netdev_warn(dev->net, "Error reading PM_CTRL\n"); + pdata->suspend_flags |= SUSPEND_SUSPEND0; + return ret; } @@ -1393,11 +1403,14 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev) if (ret < 0) netdev_warn(dev->net, "Error writing PM_CTRL\n"); + pdata->suspend_flags |= SUSPEND_SUSPEND1; + return ret; } static int smsc95xx_enter_suspend2(struct usbnet *dev) { + struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); u32 val; int ret; @@ -1414,9 +1427,105 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev) if (ret < 0) netdev_warn(dev->net, "Error writing PM_CTRL\n"); + pdata->suspend_flags |= SUSPEND_SUSPEND2; + return ret; } +static int smsc95xx_enter_suspend3(struct usbnet *dev) +{ + struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); + u32 val; + int ret; + + ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val); + if (ret < 0) { + netdev_warn(dev->net, "Error reading RX_FIFO_INF"); + return ret; + } + + if (val & 0x) { + netdev_info(dev->net, "rx fifo not empty in autosuspend"); + return -EBUSY; + } + + ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val); + if (ret < 0) { + netdev_warn(dev->net, "Error reading PM_CTRL"); + return ret; + } + + val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_); + val |= PM_CTL_SUS_MODE_3 | PM_CTL_RES_CLR_WKP_STS; + + ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val); + if (ret < 0) { + netdev_warn(dev->net, "Error writing PM_CTRL"); + return ret; + } + + /* clear wol status */ + val &= ~PM_CTL_WUPS_; + val |= PM_CTL_WUPS_WOL_; + + ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val); + if (ret < 0) { + netdev_warn(dev->net, "Error writing PM_CTRL"); + return ret; + } + + pdata->suspend_flags |= SUSPEND_SUSPEND3; + + return 0; +} + +static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up) +{ + struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); + int ret; + + if (!netif_running(dev->net)) { + /* interface is ifconfig down so fully power down hw */ + netdev_dbg(dev->net, "autosuspend entering SUSPEND2"); + return smsc95xx_enter_suspend2(dev); + } + + if (!link_up) { + /* link is down so enter EDPD mode, but only if device can +* reliably resume from it. This check should be redundant +* as current FEATURE_AUTOSUSPEND parts also support +* FEATURE_PHY_NLP_CROSSOVER but it's included for clarity */ + if (!(pdata->features & FEATURE_PHY_NLP_CROSSOVER)) { + netdev_warn(dev->net, "EDPD not supported"); + return -EBUSY; + } + + netdev_dbg(dev->net, "autosuspend entering SUSPEND1"); + + /* enable PHY wakeup events for if cable is attached */ + ret = smsc95xx_enable_phy_wakeup_interrupts(dev, + PHY_INT_MASK_ANEG_COMP_); +
Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)
On 11 December 2012 12:53, Ming Lei wrote: > On Tue, Dec 11, 2012 at 6:27 PM, Oliver Neukum wrote: >> So they can autosuspend if the interface is up and no cable is plugged >> in? > > From the open datasheet, that is the suspend 1 mode, which is supported > by all LAN95xx devices. Steve, correct me if I am wrong. All parts support SUSPEND1, but some parts can't 100% reliably wake on ENERGYON - some link partners will wake them but others won't. The driver already detects parts that work reliably with all link partners and sets the FEATURE_PHY_NLP_CROSSOVER feature flag. I didn't block these devices from configuring WOL, because they do work in *some* cases and the user is explicitly requesting to wake the system so we try to do that (and sometimes succeed). >>> I suggest to introduce link-off triggered runtime suspend for these >>> usbnet devices(non-LAN9500A device, devices which don't support >>> USB auto-suspend), and I have posted one patch set before[1]. >>> If no one objects that, I'd like to post them again with some fix and >>> update for checking link after link_reset(). >> >> If you can get rid of a periodic work this would be great. > > For the LAN95xx devices, the periodic work isn't needed because > they may generate remote wakeup when link change is detected. As above, some parts will do this but some will not. I think we should only consider sleeping the part if we're sure it'll wake up when a cable is connected! -- Steve Glendinning -- 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][RFC] smsc95xx: enable dynamic autosuspend (RFC)
On 10 December 2012 12:09, Oliver Neukum wrote: > So this is a problem with remote wakeup on older hardware? Exactly, the older hardware revisions can't reliably do it. >> Unfortunately we don't know if the connected device supports >> this feature until we query its ID register at runtime. >> >> Suggestions on how best to indicate this capability at runtime >> instead of compile-time would be appreciated, so we don't have >> to repeatedly fail if accidentally enabled. Or maybe this is >> actually the best way? > > If this is a problem with remote wakeup, you should up the > pm counter (usb_autopm_get_noresume()) in .manage_power > That was the reason I implemented this is a callback and not as > a helper in usbnet. Thanks, so something like this should do the job? static int smsc95xx_manage_power(struct usbnet *dev, int on) { struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); dev->intf->needs_remote_wakeup = on; if (pdata->features & FEATURE_AUTOSUSPEND) return 0; /* this chip revision doesn't support autosuspend */ netdev_info(dev->net, "hardware doesn't support USB autosuspend\n"); if (on) usb_autopm_get_interface_no_resume(dev->intf); else usb_autopm_put_interface_no_suspend(dev->intf); return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)
This is a work in-progress patch. It's not yet complete but I thought I'd share it for comments, feedback and testing. This patch enables dynamic autosuspend for all devices supported by the driver, but it will only actually work on LAN9500A (as this has a new SUSPEND3 mode for this purpose). Unfortunately we don't know if the connected device supports this feature until we query its ID register at runtime. On unsupported devices (LAN9500/9512/9514) this patch claims to support the feature but if enabled it will always return failure to the autosuspend call (and fill up the kernel log with a message every 2s). Suggestions on how best to indicate this capability at runtime instead of compile-time would be appreciated, so we don't have to repeatedly fail if accidentally enabled. Or maybe this is actually the best way? We should also be able to identify devices supporting autosuspend from the USB VID/PID if this would help. UPDATE: reposting this to a wider audience due to lack of feedback last time round Signed-off-by: Steve Glendinning --- drivers/net/usb/smsc95xx.c | 136 +++- 1 file changed, 135 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 9b73670..d9c6674 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -55,6 +55,13 @@ #define FEATURE_PHY_NLP_CROSSOVER (0x02) #define FEATURE_AUTOSUSPEND(0x04) +#define SUSPEND_SUSPEND0 (0x01) +#define SUSPEND_SUSPEND1 (0x02) +#define SUSPEND_SUSPEND2 (0x04) +#define SUSPEND_SUSPEND3 (0x08) +#define SUSPEND_ALLMODES (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \ +SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3) + struct smsc95xx_priv { u32 mac_cr; u32 hash_hi; @@ -62,6 +69,7 @@ struct smsc95xx_priv { u32 wolopts; spinlock_t mac_cr_lock; u8 features; + u8 suspend_flags; }; static bool turbo_mode = true; @@ -1341,6 +1349,8 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev) if (ret < 0) netdev_warn(dev->net, "Error reading PM_CTRL\n"); + pdata->suspend_flags |= SUSPEND_SUSPEND0; + return ret; } @@ -1393,11 +1403,14 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev) if (ret < 0) netdev_warn(dev->net, "Error writing PM_CTRL\n"); + pdata->suspend_flags |= SUSPEND_SUSPEND1; + return ret; } static int smsc95xx_enter_suspend2(struct usbnet *dev) { + struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); u32 val; int ret; @@ -1414,9 +1427,96 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev) if (ret < 0) netdev_warn(dev->net, "Error writing PM_CTRL\n"); + pdata->suspend_flags |= SUSPEND_SUSPEND2; + return ret; } +static int smsc95xx_enter_suspend3(struct usbnet *dev) +{ + struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); + u32 val; + int ret; + + ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val); + if (ret < 0) { + netdev_warn(dev->net, "Error reading RX_FIFO_INF"); + return ret; + } + + if (val & 0x) { + netdev_info(dev->net, "rx fifo not empty in autosuspend"); + return -EBUSY; + } + + ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val); + if (ret < 0) { + netdev_warn(dev->net, "Error reading PM_CTRL"); + return ret; + } + + val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_); + val |= PM_CTL_SUS_MODE_3 | PM_CTL_RES_CLR_WKP_STS; + + ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val); + if (ret < 0) { + netdev_warn(dev->net, "Error writing PM_CTRL"); + return ret; + } + + /* clear wol status */ + val &= ~PM_CTL_WUPS_; + val |= PM_CTL_WUPS_WOL_; + + ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val); + if (ret < 0) { + netdev_warn(dev->net, "Error writing PM_CTRL"); + return ret; + } + + pdata->suspend_flags |= SUSPEND_SUSPEND3; + + return 0; +} + +static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up) +{ + int ret; + + if (!netif_running(dev->net)) { + /* interface is ifconfig down so fully power down hw */ + netdev_dbg(dev->net, "autosuspend entering SUSPEND2"); + return smsc95xx_enter_suspend2(dev); + } + + if (!link_up) { + /* link is down so enter EDPD mode */ +
Re: [PATCH 1/2] smsc75xx: refactor entering suspend modes
>> + >> + ret = device_set_wakeup_enable(&net->dev, pdata->wolopts); > > You are touching the network device here. That should have been the USB > device. Try something like > > ret = device_set_wakeup_enable(&dev->udev->dev, pdata->wolopts); Perfect, this works exactly as expected now. Patch included in resubmitted patchset, thanks Bjorn! -- 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] smsc75xx: refactor entering suspend modes
> Looking at the different ethernet drivers, the normal way do do this > seems to be something like this in their .set_wol implementation: > > device_set_wakeup_enable(&adapter->pdev->dev, adapter->wol); > > > where "adapter" is a netdev_priv private struct, "pdev" is a pci device > and "wol" is an u32. I don't see any problem doing the same for USB > network devices implementing ethtool "set_wol". Ahh, good spot. I've implemented this (patch below for reference) and it still doesn't work: $ cat /sys/bus/usb/devices/2-1.2/power/wakeup disabled $ sudo ethtool -s eth2 wol p [ 1607.237767] smsc75xx 2-1.2:1.0 eth2: set_wol before device_can_wakeup=1 device_may_wakeup=0 [ 1607.237772] smsc75xx 2-1.2:1.0 eth2: set_wol after device_can_wakeup=1 device_may_wakeup=1 $ cat /sys/bus/usb/devices/2-1.2/power/wakeup disabled Huh?! My debugging printk statements tell me I've succesfully set it, but then the sysfs entry is disabled when I read it. Is something else setting this back? My testing patch for reference (note this is against my tree with a few to-be submitted patches so won't apply cleanly!): diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c index d8fa649..4c17849 100644 --- a/drivers/net/usb/smsc75xx.c +++ b/drivers/net/usb/smsc75xx.c @@ -61,7 +61,6 @@ #define SUSPEND_SUSPEND1 (0x02) #define SUSPEND_SUSPEND2 (0x04) #define SUSPEND_SUSPEND3 (0x08) -#define SUSPEND_REMOTEWAKE (0x10) #define SUSPEND_ALLMODES (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \ SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3) @@ -172,26 +171,6 @@ static int __must_check smsc75xx_write_reg(struct usbnet *dev, u32 index, return __smsc75xx_write_reg(dev, index, data, 0); } -static int smsc75xx_set_feature(struct usbnet *dev, u32 feature) -{ - if (WARN_ON_ONCE(!dev)) - return -EINVAL; - - return usbnet_write_cmd_nopm(dev, USB_REQ_SET_FEATURE, - USB_DIR_OUT | USB_RECIP_DEVICE, - feature, 0, NULL, 0); -} - -static int smsc75xx_clear_feature(struct usbnet *dev, u32 feature) -{ - if (WARN_ON_ONCE(!dev)) - return -EINVAL; - - return usbnet_write_cmd_nopm(dev, USB_REQ_CLEAR_FEATURE, - USB_DIR_OUT | USB_RECIP_DEVICE, - feature, 0, NULL, 0); -} - /* Loop until the read is completed with timeout * called with phy_mutex held */ static __must_check int __smsc75xx_phy_wait_not_busy(struct usbnet *dev, @@ -674,8 +653,19 @@ static int smsc75xx_ethtool_set_wol(struct net_device *net, { struct usbnet *dev = netdev_priv(net); struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]); + int ret; + + netdev_info(dev->net, "set_wol before device_can_wakeup=%d device_may_wakeup=%d\n", + device_can_wakeup(&net->dev), device_may_wakeup(&net->dev)); pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE; + + ret = device_set_wakeup_enable(&net->dev, pdata->wolopts); + check_warn_return(ret, "device_set_wakeup_enable error %d\n", ret); + + netdev_info(dev->net, "set_wol after device_can_wakeup=%d device_may_wakeup=%d\n", + device_can_wakeup(&net->dev), device_may_wakeup(&net->dev)); + return 0; } @@ -1197,12 +1187,17 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf) /* Init all registers */ ret = smsc75xx_reset(dev); + check_warn_return(ret, "smsc75xx_reset error %d\n", ret); dev->net->netdev_ops = &smsc75xx_netdev_ops; dev->net->ethtool_ops = &smsc75xx_ethtool_ops; dev->net->flags |= IFF_MULTICAST; dev->net->hard_header_len += SMSC75XX_TX_OVERHEAD; dev->hard_mtu = dev->net->mtu + dev->net->hard_header_len; + + ret = device_init_wakeup(&dev->net->dev, 1); + check_warn_return(ret, "device_init_wakeup error %d\n", ret); + return 0; } @@ -1262,9 +1257,7 @@ static int smsc75xx_enter_suspend0(struct usbnet *dev) ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val); check_warn_return(ret, "Error writing PMT_CTL\n"); - smsc75xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP); - - pdata->suspend_flags |= SUSPEND_SUSPEND0 | SUSPEND_REMOTEWAKE; + pdata->suspend_flags |= SUSPEND_SUSPEND0; return 0; } @@ -1291,9 +1284,7 @@ static int smsc75xx_enter_suspend1(struct usbnet *dev) ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val); check_warn_return(ret, "Error writing PMT_CTL\n"); - smsc75xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP); - - pdata->suspend_flags |= SUSPEND_SUSPEND1 | SUSPEND_REMOTEWAKE; + pdata->suspend_flags |= SUSPEND_SUSPEND1; return 0; } @@ -1348,9 +1339,7 @@ static int smsc75xx_enter_suspend3(struct usbnet *dev) ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val); check_warn_return(ret, "Error writing PMT_CTL\n"); - smsc75xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP); - - pdata->suspend_flags |= SUSPEND_SUSPEND3 | SUSPEND_REMOTEWAKE; + pdata->suspend_flags |= SUSPEND_SUSPEND3; return 0; } @@ -1650,11 +1639,6 @@ static int smsc75xx_resume(struct usb_interface *intf) /* do this first to ensure it's cleared even in error case */ pdata->suspend_flags = 0; - if (suspend_flags & SUSPEND_REMOTEWAKE) { - ret = smsc75xx_clear_feature
Re: [PATCH 1/2] smsc75xx: refactor entering suspend modes
Hi Bjorn, On 28 November 2012 09:31, Bjørn Mork wrote: > > Remote wakeup will not be enabled on system suspend unless the user (or > a userspace program on the users behalf) has requested it. If a user types "ethtool -s eth2 wol p" they *are* explicitly requesting the ethernet device to bring the system out of suspend, so I think the ethernet driver should set the feature automatically. from drivers/base/power/wakeup.c: * By default, most devices should leave wakeup disabled. The exceptions are * devices that everyone expects to be wakeup sources: keyboards, power buttons, * possibly network interfaces, etc. Steve -- 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] smsc75xx: refactor entering suspend modes
Hi Alan, >> udev->do_remote_wakeup is set in choose_wakeup() in >> drivers/usb/core/driver.c. AFAICS it is always set as long as >> device_may_wakeup(&udev->dev) is true. > > That's right. But is device_may_wakeup(&udev->dev) true? > > By default it wouldn't be. The normal way to set it is for the user or > a program to do: > > echo enabled >/sys/bus/usb/devices/.../power/wakeup > > Of course, a driver could disregard the user's choice and set the flag > by itself. If I set that from userspace the system is able to resume, but I can't work out how to successfully set this from the driver. I believe the driver should be overriding this as if the user has asked for the device to wake on lan they're expecting this to resume the system. I've tried placing various combinations of device_set_wakeup_capable and device_set_wakeup_enable in different places (bind, suspend), but it still doesn't allow the device to resume from suspend. How should I do this? -- 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] smsc75xx: refactor entering suspend modes
Hi Bjorn, On 27 November 2012 17:21, Steve Glendinning wrote: > Hi Bjorn, > >>> + smsc75xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP); >> >> As mentioned in another comment to the smsc95xx driver: This is weird. >> Do you really need to do that? >> >> This is an USB interface driver. The USB device is handled by the >> generic "usb" driver, which will do the right thing. See >> drivers/usb/generic.c and drivers/usb/core/hub.c > > Thanks, I've tested removing all these calls from the driver and > wakeup functionality seems to still work. > > I'll resubmit my smsc75xx enhancement patchset with this change once > I've done some more testing. Further testing shows that removing these calls stop wakeup from system suspend working (although don't appear to impact runtime autosuspend). Have I missed a flag or somewhere that causes udev->do_remote_wakeup to be set in the code you posted? -- 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] smsc75xx: refactor entering suspend modes
Hi Bjorn, >> + smsc75xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP); > > As mentioned in another comment to the smsc95xx driver: This is weird. > Do you really need to do that? > > This is an USB interface driver. The USB device is handled by the > generic "usb" driver, which will do the right thing. See > drivers/usb/generic.c and drivers/usb/core/hub.c Thanks, I've tested removing all these calls from the driver and wakeup functionality seems to still work. I'll resubmit my smsc75xx enhancement patchset with this change once I've done some more testing. David: please discard this patch. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] usbnet: smsc95xx: fix memory leak in smsc95xx_suspend
> BTW, I just saw the smsc95xx datasheet and the vendor's driver > source code, and found the chip supports runtime PM well > (remote wakeup on 'good packet' or link change), so do you > plan to implement that? Yes, I do plan to implement this. Note that this feature is only supported on some product variants, for example LAN9500A, so it won't benefit all smsc95xx users. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] usbnet: smsc95xx: fix memory leak in smsc95xx_suspend
On 6 November 2012 01:45, Ming Lei wrote: > This patch fixes memory leak in smsc95xx_suspend. Good spot, thanks! Note that check_warn_return just above your kfree can cause early return in the error case, which would still leak filter_mask, so we might want to explicitly expand that instance of the helper macro. > Also, it isn't necessary to bother mm to allocate 8bytes/16byte, > and we can use stack variable safely. Acked-By: Steve Glendinning -- 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 089/128] USB: at91_udc uses generic GPIO calls; minor cleanup
Hi Guys, I have checked this patch on a board that inverts the signal via a transistor: I.E the UcDragon YL9200 The patch appears to be functioning correctly. The reason they did this was to allow greater driving potential via a transistor, rather than stressing the Atmel chip directly. Unfortunately.. the transistor should have been 'NPN' rather than the 'PNP' they used. But we do at least now have a system to allow manufacturing production variation . Thanks again goes to David B., for incorporating this change, so rapidly. Steve On Feb 2, 2008, at 7:18 AM, Greg Kroah-Hartman wrote: From: David Brownell <[EMAIL PROTECTED]> Various small at91_udc cleanups: - Use generic GPIO calls, not older platform-specific ones - Use gpio_request()/gpio_free() - Use VERBOSE_DEBUG convention, not older VERBOSE - Fix sparse complaint about parameter type (changed to gfp_t) - Add missing newline to some rarely-seen debug messages - Fix some old cleanup bugs on probe() fault paths Also add a mechanism whereby rm9200 gpios can drive the D+ pullup through an inverting transistor, based on a patch from Steve Birtles. Most UDC drivers supporting a GPIO based pullup should probably have such an option, but testing it requries such a board in hand! Signed-off-by: David Brownell <[EMAIL PROTECTED]> Cc: Steve Birtles <[EMAIL PROTECTED]> Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]> --- drivers/usb/gadget/at91_udc.c | 79 ++ ++- drivers/usb/gadget/at91_udc.h |2 +- include/asm-arm/arch-at91/board.h |3 +- 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/ at91_udc.c index 305db36..a83e8b7 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -21,8 +21,7 @@ * Boston, MA 02111-1307, USA. */ -#undef DEBUG -#undef VERBOSE +#undef VERBOSE_DEBUG #undef PACKET_TRACE #include @@ -46,8 +45,8 @@ #include #include #include +#include -#include #include #include #include @@ -580,7 +579,7 @@ static int at91_ep_disable (struct usb_ep * _ep) */ static struct usb_request * -at91_ep_alloc_request(struct usb_ep *_ep, unsigned int gfp_flags) +at91_ep_alloc_request(struct usb_ep *_ep, gfp_t gfp_flags) { struct at91_request *req; @@ -881,6 +880,8 @@ static void clk_off(struct at91_udc *udc) */ static void pullup(struct at91_udc *udc, int is_on) { + int active = !udc->board.pullup_active_low; + if (!udc->enabled || !udc->vbus) is_on = 0; DBG("%sactive\n", is_on ? "" : "in"); @@ -890,7 +891,7 @@ static void pullup(struct at91_udc *udc, int is_on) at91_udp_write(udc, AT91_UDP_ICR, AT91_UDP_RXRSM); at91_udp_write(udc, AT91_UDP_TXVC, 0); if (cpu_is_at91rm9200()) - at91_set_gpio_value(udc->board.pullup_pin, 1); + gpio_set_value(udc->board.pullup_pin, active); else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) { u32 txvc = at91_udp_read(udc, AT91_UDP_TXVC); @@ -908,7 +909,7 @@ static void pullup(struct at91_udc *udc, int is_on) at91_udp_write(udc, AT91_UDP_IDR, AT91_UDP_RXRSM); at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS); if (cpu_is_at91rm9200()) - at91_set_gpio_value(udc->board.pullup_pin, 0); + gpio_set_value(udc->board.pullup_pin, !active); else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) { u32 txvc = at91_udp_read(udc, AT91_UDP_TXVC); @@ -1551,7 +1552,7 @@ static irqreturn_t at91_vbus_irq(int irq, void *_udc) /* vbus needs at least brief debouncing */ udelay(10); - value = at91_get_gpio_value(udc->board.vbus_pin); + value = gpio_get_value(udc->board.vbus_pin); if (value != udc->vbus) at91_vbus_session(&udc->gadget, value); @@ -1647,12 +1648,12 @@ static int __init at91udc_probe(struct platform_device *pdev) } if (pdev->num_resources != 2) { - DBG("invalid num_resources"); + DBG("invalid num_resources\n"); return -ENODEV; } if ((pdev->resource[0].flags != IORESOURCE_MEM) || (pdev->resource[1].flags != IORESOURCE_IRQ)) { - DBG("invalid resource type"); + DBG("invalid resource type\n"); return -ENODEV; } @@ -1674,10 +1675,26 @@ static int __init at91udc_probe(struct platform_device *pdev) udc->pdev = pdev; udc->enabled = 0; + /* rm9200 needs manual D+ pullup; off by default
Re: [patch 2.6.24-rc6-git] usb: at91_udc uses generic GPIO calls; minor cleanup
David, Just like to report that the patch appears works , including the inversion on the D+pullup. if the inversion is set to ".pullup_active_low=0" then the gadgets don't show up on the remote host . By setting it ".pullup_active_low=1" they show up on the host. (which is correct for the YL-9200) Ethernet Gadget: Removable Media: No Detachable Drive: Yes BSD Name: en2 Version: 2.13 Bus Power (mA): 500 Speed:Up to 12 Mb/sec Manufacturer: Linux 2.6.24-rc6-g4c885b4d-dirty/at91_udc OS9 Drivers: No Product ID: 0xa4a1 Vendor ID:0x0525 Which would indicate it's working (at least for my reversed logic YL-9200 board.) Steve On Jan 6, 2008, at 5:21 AM, David Brownell wrote: From: David Brownell <[EMAIL PROTECTED]> Various small at91_udc cleanups: - Use generic GPIO calls, not older platform-specific ones - Use gpio_request()/gpio_free() - Use VERBOSE_DEBUG convention, not older VERBOSE - Fix sparse complaint about parameter type (changed to gfp_t) - Add missing newline to some rarely-seen debug messages - Fix some old cleanup bugs on probe() fault paths Also add a mechanism whereby rm9200 gpios can drive the D+ pullup through an inverting transistor, based on a patch from Steve Birtles. Most UDC drivers supporting a GPIO based pullup should probably have such an option, but testing it requries such a board in hand! Signed-off-by: David Brownell <[EMAIL PROTECTED]> --- Updated with better fixes for those cleanup bugs, and to include the support for active-low rm9200 VBUS pullup. drivers/usb/gadget/at91_udc.c | 79 ++ ++-- drivers/usb/gadget/at91_udc.h |2 include/asm-arm/arch-at91/board.h |3 - 3 files changed, 63 insertions(+), 21 deletions(-) --- at91.orig/drivers/usb/gadget/at91_udc.c 2008-01-05 12:48:16.0 -0800 +++ at91/drivers/usb/gadget/at91_udc.c 2008-01-05 13:07:59.0 -0800 @@ -21,8 +21,7 @@ * Boston, MA 02111-1307, USA. */ -#undef DEBUG -#undef VERBOSE +#undef VERBOSE_DEBUG #undef PACKET_TRACE #include @@ -46,8 +45,8 @@ #include #include #include +#include -#include #include #include #include @@ -580,7 +579,7 @@ static int at91_ep_disable (struct usb_e */ static struct usb_request * -at91_ep_alloc_request(struct usb_ep *_ep, unsigned int gfp_flags) +at91_ep_alloc_request(struct usb_ep *_ep, gfp_t gfp_flags) { struct at91_request *req; @@ -881,6 +880,8 @@ static void clk_off(struct at91_udc *udc */ static void pullup(struct at91_udc *udc, int is_on) { + int active = !udc->board.pullup_active_low; + if (!udc->enabled || !udc->vbus) is_on = 0; DBG("%sactive\n", is_on ? "" : "in"); @@ -890,7 +891,7 @@ static void pullup(struct at91_udc *udc, at91_udp_write(udc, AT91_UDP_ICR, AT91_UDP_RXRSM); at91_udp_write(udc, AT91_UDP_TXVC, 0); if (cpu_is_at91rm9200()) - at91_set_gpio_value(udc->board.pullup_pin, 1); + gpio_set_value(udc->board.pullup_pin, active); else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) { u32 txvc = at91_udp_read(udc, AT91_UDP_TXVC); @@ -908,7 +909,7 @@ static void pullup(struct at91_udc *udc, at91_udp_write(udc, AT91_UDP_IDR, AT91_UDP_RXRSM); at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS); if (cpu_is_at91rm9200()) - at91_set_gpio_value(udc->board.pullup_pin, 0); + gpio_set_value(udc->board.pullup_pin, !active); else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) { u32 txvc = at91_udp_read(udc, AT91_UDP_TXVC); @@ -1551,7 +1552,7 @@ static irqreturn_t at91_vbus_irq(int irq /* vbus needs at least brief debouncing */ udelay(10); - value = at91_get_gpio_value(udc->board.vbus_pin); + value = gpio_get_value(udc->board.vbus_pin); if (value != udc->vbus) at91_vbus_session(&udc->gadget, value); @@ -1645,12 +1646,12 @@ static int __init at91udc_probe(struct p } if (pdev->num_resources != 2) { - DBG("invalid num_resources"); + DBG("invalid num_resources\n"); return -ENODEV; } if ((pdev->resource[0].flags != IORESOURCE_MEM) || (pdev->resource[1].flags != IORESOURCE_IRQ)) { - DBG("invalid resource type"); + DBG("invalid resource type\n"); return -ENODEV; } @@ -1672,10 +1673,26 @@ static int __init at91udc_probe(struct p udc->pdev = pdev; udc->enabled = 0; + /* rm9200 needs m
Re: [patch 2.6.24-rc6-git] usb: at91_udc uses generic GPIO calls; minor cleanup
That's great David, I've just move forward to rc6 ,GOD what a fiasco that was. I'm showing a kernel panic in the Amtel_spi code (i thought we finished with that damned thing), which I have disabled for now. But let me apply this patch and see how it goes. Steve On Jan 6, 2008, at 5:21 AM, David Brownell wrote: From: David Brownell <[EMAIL PROTECTED]> Various small at91_udc cleanups: - Use generic GPIO calls, not older platform-specific ones - Use gpio_request()/gpio_free() - Use VERBOSE_DEBUG convention, not older VERBOSE - Fix sparse complaint about parameter type (changed to gfp_t) - Add missing newline to some rarely-seen debug messages - Fix some old cleanup bugs on probe() fault paths Also add a mechanism whereby rm9200 gpios can drive the D+ pullup through an inverting transistor, based on a patch from Steve Birtles. Most UDC drivers supporting a GPIO based pullup should probably have such an option, but testing it requries such a board in hand! Signed-off-by: David Brownell <[EMAIL PROTECTED]> --- Updated with better fixes for those cleanup bugs, and to include the support for active-low rm9200 VBUS pullup. drivers/usb/gadget/at91_udc.c | 79 ++ ++-- drivers/usb/gadget/at91_udc.h |2 include/asm-arm/arch-at91/board.h |3 - 3 files changed, 63 insertions(+), 21 deletions(-) --- at91.orig/drivers/usb/gadget/at91_udc.c 2008-01-05 12:48:16.0 -0800 +++ at91/drivers/usb/gadget/at91_udc.c 2008-01-05 13:07:59.0 -0800 @@ -21,8 +21,7 @@ * Boston, MA 02111-1307, USA. */ -#undef DEBUG -#undef VERBOSE +#undef VERBOSE_DEBUG #undef PACKET_TRACE #include @@ -46,8 +45,8 @@ #include #include #include +#include -#include #include #include #include @@ -580,7 +579,7 @@ static int at91_ep_disable (struct usb_e */ static struct usb_request * -at91_ep_alloc_request(struct usb_ep *_ep, unsigned int gfp_flags) +at91_ep_alloc_request(struct usb_ep *_ep, gfp_t gfp_flags) { struct at91_request *req; @@ -881,6 +880,8 @@ static void clk_off(struct at91_udc *udc */ static void pullup(struct at91_udc *udc, int is_on) { + int active = !udc->board.pullup_active_low; + if (!udc->enabled || !udc->vbus) is_on = 0; DBG("%sactive\n", is_on ? "" : "in"); @@ -890,7 +891,7 @@ static void pullup(struct at91_udc *udc, at91_udp_write(udc, AT91_UDP_ICR, AT91_UDP_RXRSM); at91_udp_write(udc, AT91_UDP_TXVC, 0); if (cpu_is_at91rm9200()) - at91_set_gpio_value(udc->board.pullup_pin, 1); + gpio_set_value(udc->board.pullup_pin, active); else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) { u32 txvc = at91_udp_read(udc, AT91_UDP_TXVC); @@ -908,7 +909,7 @@ static void pullup(struct at91_udc *udc, at91_udp_write(udc, AT91_UDP_IDR, AT91_UDP_RXRSM); at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS); if (cpu_is_at91rm9200()) - at91_set_gpio_value(udc->board.pullup_pin, 0); + gpio_set_value(udc->board.pullup_pin, !active); else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) { u32 txvc = at91_udp_read(udc, AT91_UDP_TXVC); @@ -1551,7 +1552,7 @@ static irqreturn_t at91_vbus_irq(int irq /* vbus needs at least brief debouncing */ udelay(10); - value = at91_get_gpio_value(udc->board.vbus_pin); + value = gpio_get_value(udc->board.vbus_pin); if (value != udc->vbus) at91_vbus_session(&udc->gadget, value); @@ -1645,12 +1646,12 @@ static int __init at91udc_probe(struct p } if (pdev->num_resources != 2) { - DBG("invalid num_resources"); + DBG("invalid num_resources\n"); return -ENODEV; } if ((pdev->resource[0].flags != IORESOURCE_MEM) || (pdev->resource[1].flags != IORESOURCE_IRQ)) { - DBG("invalid resource type"); + DBG("invalid resource type\n"); return -ENODEV; } @@ -1672,10 +1673,26 @@ static int __init at91udc_probe(struct p udc->pdev = pdev; udc->enabled = 0; + /* rm9200 needs manual D+ pullup; off by default */ + if (cpu_is_at91rm9200()) { + if (udc->board.pullup_pin <= 0) { + DBG("no D+ pullup?\n"); + retval = -ENODEV; + goto fail0; + } + retval = gpio_request(udc->board.pullup_pin, "udc_pullup"); + if (retval) { + DBG("D+ pullup is bu
Re: [patch 1/1] at91_udc modification for systems that require active low on i/o pin to take D+ high
Hi David, On Jan 2, 2008, at 1:36 AM, David Brownell wrote: The YL-9200 board switches a transistor to provide pullup Presumably driving through a pulldown, so that when the board resets it doesn't wrongly present itself as ready to respond to a USB host. I'm just the fire brigade called after the kiddies have been playing with matches. ;-) Actually it's floating, PC4 directly drives a PNP transistor through a 4.7k, one side of the PNP is connected to the 3v3 rail the other side through a limiting resistor to the D+ ( Remember a few weeks back where there was a discussion about the, the USART and how I should let the USARTS init then re-write my changes to the registers in the Board file , rather than change the existing USART code when not all pins were implemented) And I pointed out how dangerous that might be, well here's an example. This modification adds a field to allow the state of the pin level to be specified Here's a replacement patch, addressing the critical issues I had mentioned to you before: (a) not breaking every existing at91rm9200 board, and (b) applying atop a previous patch [1]. a.)That's why I added the code to initially set the flag to 1, but I cannot see why my patch would break existing boards. b.). Yes sorry about that it won't happen again , I was in the process of merging forward. Just a couple of questions, 1. The way the patch is now: intactive = !udc->board.pullup_inverted; will that not force all existing code bases to implement a patch to add 'pullup_inverted' to their board code because an initial state of active is not set , in any existing board code ? (which is what i specifically tried to avoid) Also the name "pullup_inverted" suggests a negative situation I.E pullup_inverted = 0; (do not invert the signal) pullup_inverted = 1; (do invert the signal) Whereas the reverse is actually true. 2. What happends if the user passes in a value that is not 1 or 0 , will the call gpio_set_value , automatically mask it off? When replacing this I happened to notice two small glitches in the cleanup code of that previous patch, so I may instead just merge this into an updated version of that patch. That would be great and I would really appreciate it. Steve - Dave [1] http://marc.info/?l=linux-usb&m=119878579910739&w=2 === CUT HERE Add a mechanism whereby AT91 boards can drive the D+ pullup through an inverter. Based on a patch from Steve Birtles. Signed-off-by: David Brownell <[EMAIL PROTECTED]> --- drivers/usb/gadget/at91_udc.c | 13 ++--- include/asm-arm/arch-at91/board.h |3 ++- 2 files changed, 12 insertions(+), 4 deletions(-) --- at91.orig/drivers/usb/gadget/at91_udc.c 2008-01-01 08:28:36.0 -0800 +++ at91/drivers/usb/gadget/at91_udc.c 2008-01-01 08:42:46.0 -0800 @@ -880,6 +880,8 @@ static void clk_off(struct at91_udc *udc */ static void pullup(struct at91_udc *udc, int is_on) { + int active = !udc->board.pullup_inverted; + if (!udc->enabled || !udc->vbus) is_on = 0; DBG("%sactive\n", is_on ? "" : "in"); @@ -889,7 +891,7 @@ static void pullup(struct at91_udc *udc, at91_udp_write(udc, AT91_UDP_ICR, AT91_UDP_RXRSM); at91_udp_write(udc, AT91_UDP_TXVC, 0); if (cpu_is_at91rm9200()) - gpio_set_value(udc->board.pullup_pin, 1); + gpio_set_value(udc->board.pullup_pin, active); else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) { u32 txvc = at91_udp_read(udc, AT91_UDP_TXVC); @@ -907,7 +909,7 @@ static void pullup(struct at91_udc *udc, at91_udp_write(udc, AT91_UDP_IDR, AT91_UDP_RXRSM); at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS); if (cpu_is_at91rm9200()) - gpio_set_value(udc->board.pullup_pin, 0); + gpio_set_value(udc->board.pullup_pin, !active); else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) { u32 txvc = at91_udp_read(udc, AT91_UDP_TXVC); @@ -1683,7 +1685,8 @@ static int __init at91udc_probe(struct p DBG("D+ pullup is busy\n"); goto fail0; } - gpio_direction_output(udc->board.pullup_pin, 0); + gpio_direction_output(udc->board.pullup_pin, + udc->board.pullup_inverted); } udc->udp_baseaddr = ioremap(res->start, res->end - res->start + 1); @@ -1766,6 +1769,7 @@ fail0a: if (cpu_is_at91rm9200()) gpio_free(udc->board.pullup_pin); fail0: + iounmap(udc->udp_baseaddr); release_mem_r
[patch 1/1] at91_udc modification for systems that require active low on i/o pin to take D+ high
Hi, Patch as attachment to prevent mailer messing format. The YL-9200 board switches a transistor to provide pullup, as a result the usb code is incorrect in that it assumes all boards use 'active' high on a pin to pull up the USB state. This modification adds a field to allow the state of the pin level to be specified Signed-Off-By: Steve Birtles <[EMAIL PROTECTED]> usb.patch Description: Binary data
Re: Issue with USB thumb drives on gumstix port
Matt, You hit it on the first try! Everything works if I manually load sd_mod. Should sd_mod be loading automatically? Or should I add it to /etc/modules? Currently I'm only autoloading ohci-hcd and everything else USB needs seems to be pulled in as needed. Thanks! Steve On Sat, 2007-12-22 at 00:05 -0800, Matthew Dharm wrote: > Is sd_mod getting loaded? What happens if you load it manually? > > Matt > > On Fri, Dec 21, 2007 at 11:51:42PM -0800, Steve Sakoman wrote: > > I'm working on an OpenEmbedded port for gumstix modules (PXA270 based) > > and running into an issue with USB storage devices. > > > > I am using a 2.6.21 kernel and udev-115. > > > > Other devices seem to work fine: USB keyboards, Bluetooth dongles, etc. > > > > However when I insert a USB storage device the mount process doesn't > > quite complete: > > > > <6>usb 1-2.3: new full speed USB device using pxa27x-ohci and address 6 > > <6>usb 1-2.3: configuration #1 chosen from 1 choice > > <6>scsi2 : SCSI emulation for USB Mass Storage devices > > <7>usb-storage: device found at 6 > > <7>usb-storage: waiting for device to settle before scanning > > <5>scsi 2:0:0:0: Direct-Access USB 2.0 Flash Drive 0.00 PQ: 0 > > ANSI: 2 > > <7>usb-storage: device scan complete > > > > And nothing further--the expected sda device is never created. > > > > This is new territory for me. Any advice on where I should begin > > looking? Is this a udev thing? > > > > Thanks! > > > > Steve > > > > > > - > > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > > the body of a message to [EMAIL PROTECTED] > > 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Issue with USB thumb drives on gumstix port
I'm working on an OpenEmbedded port for gumstix modules (PXA270 based) and running into an issue with USB storage devices. I am using a 2.6.21 kernel and udev-115. Other devices seem to work fine: USB keyboards, Bluetooth dongles, etc. However when I insert a USB storage device the mount process doesn't quite complete: <6>usb 1-2.3: new full speed USB device using pxa27x-ohci and address 6 <6>usb 1-2.3: configuration #1 chosen from 1 choice <6>scsi2 : SCSI emulation for USB Mass Storage devices <7>usb-storage: device found at 6 <7>usb-storage: waiting for device to settle before scanning <5>scsi 2:0:0:0: Direct-Access USB 2.0 Flash Drive 0.00 PQ: 0 ANSI: 2 <7>usb-storage: device scan complete And nothing further--the expected sda device is never created. This is new territory for me. Any advice on where I should begin looking? Is this a udev thing? Thanks! Steve - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html