Re: FW: xhci ASMedia lockups - a theory and a patch

2014-01-30 Thread renevant
It looks like the issue with being unable to get the device to work at all is 
limited to the Asmedia controller.

I plugged a VL800-Q8 based pcie card in and got 117MB/s when sending and 
receiving with scp. This is with kernel 3.12.9


I guess it's possible that the ax88179 never worked with the integrated 
Asmedia 1042 controller on my motherboard with any kernel, but I can say i'm 
seeing the issue back to 3.11.0

With the VIA controller I guess i'm good to test stability, though in the past 
i've used it with kvm and vfio but for some reason haven't been able to pass 
it through to a virtual machine with later kernels and then use the ax88179.


Regards,

Will Trives
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread David Laight
From: Sarah Sharp
  Current issue is when plugging in the ax88179 there is lag when bringing the
  interface up and a bunch of kernel messages:
 
 With which kernel?

I saw similar issues testing some patches yesterday.
Both with the ax179_178a and smsx95xx cards (connected to xhci).
My kernel claims to be 3.13.0-dsl+ but it is lying since it was
updated from linus's tree earlier this week.
I'd not seen any similar delays until the last week or so.

The xhci controller I'm using is the Intel 'Panther Point' 8086:1e31
rev 4 (also says 8086:1e2d and 8086:1e26).

David



--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread David Laight
From: Sarah Sharp
 On Tue, Jan 28, 2014 at 11:30:51PM -0500, Mark Lord wrote:
  On 14-01-28 03:30 PM, Sarah Sharp wrote:
  ..
   Can you please pull this branch, which contains a 3.13 kernel with
   David's patch reverted, and test whether your USB ethernet device works
   or fails?
 
  Fails.  dmesg log attached.
 
 It's funny, because there's certainly data transferred over endpoint
 0x82, even though there were link TRBs in the middle of transfers.  Did
 the untransferred messages stop when the device stopped working, or
 did they continue?

What I saw was that the USB transfers continued, but the ethernet
transmits stopped.
This rather changes the packets generated by TCP at both ends.
The effect can be seen in the timestamps (etc) in the USB trace.
There are still tx and rx packets, after a while they'll only
happen on ever-increasing timeouts.

That was the point where I wrote the NOP patch just to see if
it made a difference. I didn't really expect one!

I think that the LINK trb splits a 1k usb message in two.
This well and truly confuses the ethernet part of the ax88179_178a
hardware to the point where it doesn't even reset itself on the
next sub 1k message.

It might be that other targets (eg the smsx95xx) is more resilient
and only loses the single packet - which won't be immediately obvious.

David


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v2 1/1] USB: EHCI: wait more than 3ms until the device enters full-speed idle

2014-01-30 Thread Peter Chen
 
  
   What happens if you start putting a different PHY on the board, one
   that takes longer to enter low-power mode?
 
  A little difficult to change a PHY on the board.
 
  Just like I said above, it depends on when the hardware enables wake on
  disconnect, full-speed idle, full-speed idle + PHY enters low power,
  or only just PHY enters low power?
 
 The code should work correctly no matter when the hardware enables
 wake-on-disconnect.
 
   So far I have not seen any complaints about this happening from any
   user except you.  I just tried doing the experiment on my own
 computer
   (enable wakeup for the root hub, plug in a high-speed device, and
   suspend the computer).  It worked correctly.
 
  Have you enabled wakeup on your high speed device?
 
 No.
 
  This problem has not occurred for wakeup enabled device or old kernel
  (before you enable global suspend), since there is a 10ms delay at
  usb_port_suspend.
 
 I just tried the test again on a different computer.  This time I was
 running 3.13-rc8 (before was 3.12.something).  In both cases, the
 device does not support wakeup (it is a flash drive).
 
   Also, there already is a 5-ms sleep just below the code you changed.
   It depends on ehci-has_tdi_phy_lpm.  Is that flag set for your
 system?
   If it isn't, you could simply remove the test for has_tdi_phy_lpm.
   That should have the same effect as your patch.
  
 
  It is just for a specific platform which has tdi phy. This case may
  be generic, in a word, do we need to make sure the bus enters full-
 speed
  idle after ehci_bus_suspend has finished?
 
 You mean _before_ ehci_bus_suspend has finished.
 

Yes

 As far as I can see, it doesn't matter.  The important thing is whether
 the bus enters full-speed idle before we enable wake-on-disconnect.
 
   If we have not guaranteed
  it, platform code needs to make sure the bus will not change before
  the wakeup logic takes effect, of cos, these kinds of platform have
  no hardware logic to make sure above.
 
 So ehci-hcd has to take care of it.
 

Any changes I need to do for my current patch? I will work on it according to 
your
suggestion after China New year holiday (1 week later)

Peter

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] tools: usb: aio example applications

2014-01-30 Thread Robert Baldyga
This patch adds two example applications showing usage of Asynchronous I/O API
of FunctionFS. First one (aio_simple) is simple example of bidirectional data
transfer. Second one (aio_multibuff) shows multi-buffer data transfer, which
may to be used in high performance applications.

Both examples contains userspace applications for device and for host.
It needs libaio library on the device, and libusb library on host.

Signed-off-by: Robert Baldyga r.bald...@samsung.com
---

Hello,

This is third version of patch adding examples of use AIO API of FunctionFS.
From last version of this patch, I have fixed few bugs and style problems.

Changelog:

v3:
- get rid of global variables
- add missing error handling
- fix some style problems

v2: http://www.spinics.net/lists/linux-usb/msg101650.html
- cleanup code
- a lot of small fixes

v1: http://www.spinics.net/lists/linux-usb/msg101614.html

 tools/usb/aio_multibuff/device_app/aio_multibuff.c |  322 
 tools/usb/aio_multibuff/host_app/Makefile  |   13 +
 tools/usb/aio_multibuff/host_app/test.c|  142 +
 tools/usb/aio_simple/device_app/aio_simple.c   |  296 ++
 tools/usb/aio_simple/host_app/Makefile |   13 +
 tools/usb/aio_simple/host_app/test.c   |  145 +
 6 files changed, 931 insertions(+)
 create mode 100644 tools/usb/aio_multibuff/device_app/aio_multibuff.c
 create mode 100644 tools/usb/aio_multibuff/host_app/Makefile
 create mode 100644 tools/usb/aio_multibuff/host_app/test.c
 create mode 100644 tools/usb/aio_simple/device_app/aio_simple.c
 create mode 100644 tools/usb/aio_simple/host_app/Makefile
 create mode 100644 tools/usb/aio_simple/host_app/test.c

diff --git a/tools/usb/aio_multibuff/device_app/aio_multibuff.c 
b/tools/usb/aio_multibuff/device_app/aio_multibuff.c
new file mode 100644
index 000..0c5bf82
--- /dev/null
+++ b/tools/usb/aio_multibuff/device_app/aio_multibuff.c
@@ -0,0 +1,322 @@
+#define _BSD_SOURCE /* for endian.h */
+
+#include endian.h
+#include errno.h
+#include fcntl.h
+#include stdarg.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include sys/ioctl.h
+#include sys/stat.h
+#include sys/types.h
+#include sys/poll.h
+#include unistd.h
+#include stdbool.h
+#include libaio.h
+
+#include linux/usb/functionfs.h
+
+/ Descriptors and Strings ***/
+
+static const struct {
+   struct usb_functionfs_descs_head header;
+   struct {
+   struct usb_interface_descriptor intf;
+   struct usb_endpoint_descriptor_no_audio bulk_sink;
+   struct usb_endpoint_descriptor_no_audio bulk_source;
+   } __attribute__ ((__packed__)) fs_descs, hs_descs;
+} __attribute__ ((__packed__)) descriptors = {
+   .header = {
+   .magic = htole32(FUNCTIONFS_DESCRIPTORS_MAGIC),
+   .length = htole32(sizeof(descriptors)),
+   .fs_count = 3,
+   .hs_count = 3,
+   },
+   .fs_descs = {
+   .intf = {
+   .bLength = sizeof(descriptors.fs_descs.intf),
+   .bDescriptorType = USB_DT_INTERFACE,
+   .bNumEndpoints = 2,
+   .bInterfaceClass = USB_CLASS_VENDOR_SPEC,
+   .iInterface = 1,
+   },
+   .bulk_sink = {
+   .bLength = sizeof(descriptors.fs_descs.bulk_sink),
+   .bDescriptorType = USB_DT_ENDPOINT,
+   .bEndpointAddress = 1 | USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_BULK,
+   },
+   .bulk_source = {
+   .bLength = sizeof(descriptors.fs_descs.bulk_source),
+   .bDescriptorType = USB_DT_ENDPOINT,
+   .bEndpointAddress = 2 | USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_BULK,
+   },
+   },
+   .hs_descs = {
+   .intf = {
+   .bLength = sizeof(descriptors.hs_descs.intf),
+   .bDescriptorType = USB_DT_INTERFACE,
+   .bNumEndpoints = 2,
+   .bInterfaceClass = USB_CLASS_VENDOR_SPEC,
+   .iInterface = 1,
+   },
+   .bulk_sink = {
+   .bLength = sizeof(descriptors.hs_descs.bulk_sink),
+   .bDescriptorType = USB_DT_ENDPOINT,
+   .bEndpointAddress = 1 | USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_BULK,
+   },
+   .bulk_source = {
+   .bLength = sizeof(descriptors.hs_descs.bulk_source),
+   .bDescriptorType = USB_DT_ENDPOINT,
+   .bEndpointAddress = 2 | USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_BULK,
+   },
+   },
+};
+
+#define 

RE: [PATCH v3] tools: usb: aio example applications

2014-01-30 Thread David Laight
From: Robert Baldyga
This patch adds two example applications showing usage of Asynchronous I/O API
 of FunctionFS. First one (aio_simple) is simple example of bidirectional data
 transfer. Second one (aio_multibuff) shows multi-buffer data transfer, which
 may to be used in high performance applications.
 
 Both examples contains userspace applications for device and for host.
 It needs libaio library on the device, and libusb library on host.
...

 +int main()

You've still got KR function definitions...
I also suspect that most coding standard want structure definitions
and #defines at the top of the file.

David


--
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 driver-core-linus] kernfs: make kernfs_deactivate() honor KERNFS_LOCKDEP flag

2014-01-30 Thread Jiri Kosina
On Wed, 29 Jan 2014, Tejun Heo wrote:

 kernfs_deactivate() forgot to check whether KERNFS_LOCKDEP is set
 before performing lockdep annotations and ends up feeding
 uninitialized lockdep_map to lockdep triggering warning like the
 following on USB stick hotunplug.
 
  usb 1-2: USB disconnect, device number 2
  INFO: trying to register non-static key.
  the code is fine but needs lockdep annotation.
  turning off the locking correctness validator.
  CPU: 1 PID: 62 Comm: khubd Not tainted 3.13.0-work+ #82
  Hardware name: empty empty/S3992, BIOS 080011  10/26/2007
   880065ca7f60 88013a4ffa08 81cfb6bd 0002
   88013a4ffac8 810f8530 88013a4fc710 0002
   8801 82a3db50 0001 88013a4fc710
  Call Trace:
   [81cfb6bd] dump_stack+0x4e/0x7a
   [810f8530] __lock_acquire+0x1910/0x1e70
   [810f931a] lock_acquire+0x9a/0x1d0
   [8127c75e] kernfs_deactivate+0xee/0x130
   [8127d4c8] kernfs_addrm_finish+0x38/0x60
   [8127d701] kernfs_remove_by_name_ns+0x51/0xa0
   [8127b4f1] remove_files.isra.1+0x41/0x80
   [8127b7e7] sysfs_remove_group+0x47/0xa0
   [8127b873] sysfs_remove_groups+0x33/0x50
   [8177d66d] device_remove_attrs+0x4d/0x80
   [8177e25e] device_del+0x12e/0x1d0
   [819722c2] usb_disconnect+0x122/0x1a0
   [819749b5] hub_thread+0x3c5/0x1290
   [810c6a6d] kthread+0xed/0x110
   [81d0a56c] ret_from_fork+0x7c/0xb0
 
 Fix it by making kernfs_deactivate() perform lockdep annotations only
 if KERNFS_LOCKDEP is set.
 
 Signed-off-by: Tejun Heo t...@kernel.org
 Reported-by: Fabio Estevam feste...@gmail.com
 Reported-by: Alan Stern st...@rowland.harvard.edu

I reported it here:

https://lkml.org/lkml/2014/1/22/217

The patch fixes it for me as well.

Reported-and-tested-by: Jiri Kosina jkos...@suse.cz

Thanks,

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread renevant
When using the ax88179 connected via the via based card the whole system gets 
brought down after a while i got this my system log.

I'm going to take a break and see if I can narrow anything more down tomorrow. 
This log is in reverse because of the wonderful way journalctl works.

I suppose I could try using iommu=pt as a kernel boot parameter but that 
doesn't sound like a safe thing to do.


 Jan 30 21:04:38 athas kernel: [c11281a0] xhci_msi_irq [xhci_hcd]
Jan 30 21:04:38 athas kernel: handlers:
Jan 30 21:04:38 athas kernel:  [a74f9ee6] 
system_call_fastpath+0x1a/0x1f
Jan 30 21:04:38 athas kernel:  [a715ef6a] ? 
SyS_epoll_ctl+0x4fa/0xb00
Jan 30 21:04:38 athas kernel:  EOI  [a715f181] ? 
SyS_epoll_ctl+0x711/0xb00
Jan 30 21:04:38 athas kernel:  [a74f982a] common_interrupt+0x6a/0x6a
Jan 30 21:04:38 athas kernel:  [a700445a] do_IRQ+0x4a/0xf0
Jan 30 21:04:38 athas kernel:  [a7004679] handle_irq+0x19/0x30
Jan 30 21:04:38 athas kernel:  [a708200f] handle_edge_irq+0x6f/0x120
Jan 30 21:04:38 athas kernel:  [a707f8b1] handle_irq_event+0x31/0x50
Jan 30 21:04:38 athas kernel:  [a707f802] 
handle_irq_event_percpu+0xc2/0x140
Jan 30 21:04:38 athas kernel:  [a7081b00] note_interrupt+0xe0/0x1e0
Jan 30 21:04:38 athas kernel:  [a708176d] __report_bad_irq+0x2d/0xc0
Jan 30 21:04:38 athas kernel:  IRQ  [a74efd3f] 
dump_stack+0x45/0x56
Jan 30 21:04:38 athas kernel: Call Trace:
Jan 30 21:04:38 athas kernel:   88043edc3ed8 
a7081b00 
Jan 30 21:04:38 athas kernel:  88043edc3e98 a708176d 
880425031b00 004d
Jan 30 21:04:38 athas kernel:  880425031b84 88043edc3e70 
a74efd3f 880425031b00
Jan 30 21:04:38 athas kernel: Hardware name: To be filled by O.E.M. To be 
filled by O.E.M./M5A99FX PRO R2.0, BIOS 2201 11/22/2013
Jan 30 21:04:38 athas kernel: CPU: 7 PID: 1160 Comm: Chrome_IOThread Not 
tainted 3.13.0+ #13
Jan 30 21:04:38 athas kernel: irq event 77: bogus return value ff94
Jan 30 21:04:38 athas kernel: xhci_hcd :02:00.0: Host not halted after 
16000 microseconds.
Jan 30 21:04:38 athas kernel: AMD-Vi: Event logged [IO_PAGE_FAULT 
device=02:00.0 domain=0x0019 address=0x002b2000 flags=0x]
Jan 30 21:04:38 athas kernel: xhci_hcd :02:00.0: WARNING: Host System 
Error


Regards,

Will Trives
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread renevant
via vl800 pcie card 
kernel parameter iommu=pt
ethtool -K xxx sg off
ifconfig xxx mtu 4060 up

stable so far, it's way past the point that it usually crashes.

i'll do proper testing tomorrow


iommu=pt   bah !

Regards,

Will Trives

On Thursday 30 January 2014 21:46:27 renev...@internode.on.net wrote:
 When using the ax88179 connected via the via based card the whole system
 gets brought down after a while i got this my system log.
 
 I'm going to take a break and see if I can narrow anything more down
 tomorrow. This log is in reverse because of the wonderful way journalctl
 works.
 
 I suppose I could try using iommu=pt as a kernel boot parameter but that
 doesn't sound like a safe thing to do.
 
 
  Jan 30 21:04:38 athas kernel: [c11281a0] xhci_msi_irq [xhci_hcd]
 Jan 30 21:04:38 athas kernel: handlers:
 Jan 30 21:04:38 athas kernel:  [a74f9ee6]
 system_call_fastpath+0x1a/0x1f
 Jan 30 21:04:38 athas kernel:  [a715ef6a] ?
 SyS_epoll_ctl+0x4fa/0xb00
 Jan 30 21:04:38 athas kernel:  EOI  [a715f181] ?
 SyS_epoll_ctl+0x711/0xb00
 Jan 30 21:04:38 athas kernel:  [a74f982a]
 common_interrupt+0x6a/0x6a Jan 30 21:04:38 athas kernel: 
 [a700445a] do_IRQ+0x4a/0xf0 Jan 30 21:04:38 athas kernel: 
 [a7004679] handle_irq+0x19/0x30 Jan 30 21:04:38 athas kernel: 
 [a708200f] handle_edge_irq+0x6f/0x120 Jan 30 21:04:38 athas
 kernel:  [a707f8b1] handle_irq_event+0x31/0x50 Jan 30 21:04:38
 athas kernel:  [a707f802]
 handle_irq_event_percpu+0xc2/0x140
 Jan 30 21:04:38 athas kernel:  [a7081b00]
 note_interrupt+0xe0/0x1e0 Jan 30 21:04:38 athas kernel: 
 [a708176d] __report_bad_irq+0x2d/0xc0 Jan 30 21:04:38 athas
 kernel:  IRQ  [a74efd3f]
 dump_stack+0x45/0x56
 Jan 30 21:04:38 athas kernel: Call Trace:
 Jan 30 21:04:38 athas kernel:   88043edc3ed8
 a7081b00 
 Jan 30 21:04:38 athas kernel:  88043edc3e98 a708176d
 880425031b00 004d
 Jan 30 21:04:38 athas kernel:  880425031b84 88043edc3e70
 a74efd3f 880425031b00
 Jan 30 21:04:38 athas kernel: Hardware name: To be filled by O.E.M. To be
 filled by O.E.M./M5A99FX PRO R2.0, BIOS 2201 11/22/2013
 Jan 30 21:04:38 athas kernel: CPU: 7 PID: 1160 Comm: Chrome_IOThread Not
 tainted 3.13.0+ #13
 Jan 30 21:04:38 athas kernel: irq event 77: bogus return value ff94
 Jan 30 21:04:38 athas kernel: xhci_hcd :02:00.0: Host not halted after
 16000 microseconds.
 Jan 30 21:04:38 athas kernel: AMD-Vi: Event logged [IO_PAGE_FAULT
 device=02:00.0 domain=0x0019 address=0x002b2000 flags=0x]
 Jan 30 21:04:38 athas kernel: xhci_hcd :02:00.0: WARNING: Host System
 Error
 
 
 Regards,
 
 Will Trives

--
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 UAS test device

2014-01-30 Thread Greg KH
On Wed, Jan 29, 2014 at 01:23:26PM -0800, Sarah Sharp wrote:
 On Wed, Jan 29, 2014 at 11:38:19AM -0500, Alan Stern wrote:
  On Wed, 29 Jan 2014, Oliver Neukum wrote:
  
   Hi,
   
   can you recommend a test device for UAS?
  
  I have never encountered any.  You can always try the UAS gadget over 
  dummy-hcd, but that's not the same as real hardware.
 
 Pluggable UAS device:
 http://www.amazon.com/Plugable-Lay-Flat-Docking-Station-ASM1053E/dp/B00APP6694/

Ah, thanks, I've now ordered it.
--
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


[RFCv2 05/10] xhci: use command structure for xhci_queue_new_dequeue_state()

2014-01-30 Thread Mathias Nyman
Prepare for the global command ring by using command structures
internally in functions calling xhci_queue_new_dequeue_state()

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci-ring.c | 3 +++
 drivers/usb/host/xhci.c  | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index da83a844..df5b0f8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -855,11 +855,14 @@ remove_finished_td:
 
/* If necessary, queue a Set Transfer Ring Dequeue Pointer command */
if (deq_state.new_deq_ptr  deq_state.new_deq_seg) {
+   struct xhci_command *command;
+   command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
xhci_queue_new_dequeue_state(xhci,
slot_id, ep_index,
ep-stopped_td-urb-stream_id,
deq_state);
xhci_ring_cmd_db(xhci);
+   kfree(command);
} else {
/* Otherwise ring the doorbell(s) to restart queued transfers */
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d4684d0..0c3f0bc 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2869,10 +2869,16 @@ void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci,
 * issue a configure endpoint command later.
 */
if (!(xhci-quirks  XHCI_RESET_EP_QUIRK)) {
+   struct xhci_command *command;
+   /* Can't sleep if we're called from cleanup_halted_endpoint() */
+   command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
+   if (!command)
+   return;
xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
Queueing new dequeue state);
xhci_queue_new_dequeue_state(xhci, udev-slot_id,
ep_index, ep-stopped_stream, deq_state);
+   kfree(command);
} else {
/* Better hope no one uses the input context between now and the
 * reset endpoint completion!
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFCv2 06/10] xhci: use command structures for xhci_queue_reset_ep()

2014-01-30 Thread Mathias Nyman
Prepare for the global command ring by using command structures
internally in functions calling xhci_queue_reset_ep()

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci-ring.c | 5 +
 drivers/usb/host/xhci.c  | 6 ++
 2 files changed, 11 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index df5b0f8..bf50d28 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1913,6 +1913,10 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd 
*xhci,
struct xhci_td *td, union xhci_trb *event_trb)
 {
struct xhci_virt_ep *ep = xhci-devs[slot_id]-eps[ep_index];
+   struct xhci_command *command;
+   command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
+   if (!command)
+   return;
ep-ep_state |= EP_HALTED;
ep-stopped_td = td;
ep-stopped_trb = event_trb;
@@ -1926,6 +1930,7 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd 
*xhci,
ep-stopped_stream = 0;
 
xhci_ring_cmd_db(xhci);
+   kfree(command);
 }
 
 /* Check if an error has halted the endpoint ring.  The class driver will
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 0c3f0bc..37a1a7e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2908,6 +2908,7 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
unsigned long flags;
int ret;
struct xhci_virt_ep *virt_ep;
+   struct xhci_command *command;
 
xhci = hcd_to_xhci(hcd);
udev = (struct usb_device *) ep-hcpriv;
@@ -2930,6 +2931,10 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
return;
}
 
+   command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
+   if (!command)
+   return;
+
xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
Queueing reset endpoint command);
spin_lock_irqsave(xhci-lock, flags);
@@ -2948,6 +2953,7 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
virt_ep-stopped_trb = NULL;
virt_ep-stopped_stream = 0;
spin_unlock_irqrestore(xhci-lock, flags);
+   kfree(command);
 
if (ret)
xhci_warn(xhci, FIXME allocate a new ring segment\n);
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFCv2 10/10] xhci: rework command timeout and cancellation,

2014-01-30 Thread Mathias Nyman
Use one timer to control command timeout.

start/kick the timer every time a command is completed and a
new command is waiting, or a new command is added to a empty list.

If the timer runs out, then tag the current command as aborted, and
start the xhci command abortion process.

Previously each function that submitted a command had its own timer.
If that command timed out, a new command structure for
the command was created and it was put on a cancel_cmd_list list,
then a pci write to abort the command ring was issued.

when the ring was aborted, it checked if the current command
was the one to be cancelled, later when the ring was stopped the
driver got ownership of the TRBs in the command ring,
compared then to the TRBs in the cancel_cmd_list,
and turned them into No-ops.

Now, instead, at timeout we tag the status of the command in the
command queue to be aborted, and start the ring abortion.
Ring abortion stops the command ring and gives control of the commands to us.
All the aborted commands are now turned into No-ops.

This allows us to remove the entire cancel_cmd_list code.

The functions waiting for a command to finish no longer have their own timeouts.
They will wait either until the command completes normally,
or until the whole command abortion is done.

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci-hub.c  |  11 +-
 drivers/usb/host/xhci-mem.c  |  15 +-
 drivers/usb/host/xhci-ring.c | 336 +--
 drivers/usb/host/xhci.c  |  79 --
 drivers/usb/host/xhci.h  |   8 +-
 5 files changed, 139 insertions(+), 310 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 1079cc6..5df5fcf 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -271,7 +271,6 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
struct xhci_virt_device *virt_dev;
struct xhci_command *cmd;
unsigned long flags;
-   int timeleft;
int ret;
int i;
 
@@ -304,12 +303,10 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
spin_unlock_irqrestore(xhci-lock, flags);
 
/* Wait for last stop endpoint command to finish */
-   timeleft = wait_for_completion_interruptible_timeout(
-   cmd-completion,
-   XHCI_CMD_DEFAULT_TIMEOUT);
-   if (timeleft = 0) {
-   xhci_warn(xhci, %s while waiting for stop endpoint command\n,
-   timeleft == 0 ? Timeout : Signal);
+   wait_for_completion(cmd-completion);
+
+   if (cmd-status == COMP_CMD_ABORT || cmd-status == COMP_CMD_STOP) {
+   xhci_warn(xhci, Timeout while waiting for stop endpoint 
command\n);
ret = -ETIME;
}
xhci_free_command(xhci, cmd);
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 80b9c11..d27178b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1692,7 +1692,6 @@ void xhci_free_command(struct xhci_hcd *xhci,
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)-self.controller);
-   struct xhci_cd  *cur_cd, *next_cd;
struct xhci_command *cur_cmd, *next_cmd;
int size;
int i, j, num_ports;
@@ -1712,15 +1711,13 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
if (xhci-lpm_command)
xhci_free_command(xhci, xhci-lpm_command);
xhci-cmd_ring_reserved_trbs = 0;
+
+   del_timer_sync(xhci-cmd_timer);
+
if (xhci-cmd_ring)
xhci_ring_free(xhci, xhci-cmd_ring);
xhci-cmd_ring = NULL;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, Freed command ring);
-   list_for_each_entry_safe(cur_cd, next_cd,
-   xhci-cancel_cmd_list, cancel_cmd_list) {
-   list_del(cur_cd-cancel_cmd_list);
-   kfree(cur_cd);
-   }
 
list_for_each_entry_safe(cur_cmd, next_cmd,
xhci-cmd_list, cmd_list) {
@@ -2228,7 +2225,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
u32 page_size, temp;
int i;
 
-   INIT_LIST_HEAD(xhci-cancel_cmd_list);
INIT_LIST_HEAD(xhci-cmd_list);
 
page_size = xhci_readl(xhci, xhci-op_regs-page_size);
@@ -2414,6 +2410,11 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
Wrote ERST address to ir_set 0.);
xhci_print_ir_set(xhci, 0);
 
+   /* init command timeout timer */
+   init_timer(xhci-cmd_timer);
+   xhci-cmd_timer.data = (unsigned long) xhci;
+   xhci-cmd_timer.function = xhci_handle_command_timeout;
+
/*
 * XXX: Might need to set the Interrupter Moderation Register to
 * something other than the default (~1ms minimum between interrupts).
diff --git a/drivers/usb/host/xhci-ring.c 

[RFCv2 07/10] xhci: Use command structured when queuing commands

2014-01-30 Thread Mathias Nyman
Require each queued command to have a command structure.
We store the command trb pointer in the structure when queuing it,
making the find_next_enqueue() function obsolete.

Don't free the command strucures right away after sending the commands,
We will free the commands when we receive a command handled event
in a later patch.

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci-hub.c  |  8 ++--
 drivers/usb/host/xhci-ring.c | 94 ++--
 drivers/usb/host/xhci.c  | 47 +++---
 drivers/usb/host/xhci.h  | 31 ---
 4 files changed, 91 insertions(+), 89 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 68bc1c6..4342ec3 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -293,14 +293,14 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
spin_unlock_irqrestore(xhci-lock, flags);
xhci_free_command(xhci, cmd);
return -ENOMEM;
+
}
-   xhci_queue_stop_endpoint(xhci, slot_id, i, suspend);
-   kfree(command);
+   xhci_queue_stop_endpoint(xhci, command, slot_id, i,
+suspend);
}
}
-   cmd-command_trb = xhci_find_next_enqueue(xhci-cmd_ring);
list_add_tail(cmd-cmd_list, virt_dev-cmd_list);
-   xhci_queue_stop_endpoint(xhci, slot_id, 0, suspend);
+   xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(xhci-lock, flags);
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index bf50d28..6f6018c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -123,16 +123,6 @@ static int enqueue_is_link_trb(struct xhci_ring *ring)
return TRB_TYPE_LINK_LE32(link-control);
 }
 
-union xhci_trb *xhci_find_next_enqueue(struct xhci_ring *ring)
-{
-   /* Enqueue pointer can be left pointing to the link TRB,
-* we must handle that
-*/
-   if (TRB_TYPE_LINK_LE32(ring-enqueue-link.control))
-   return ring-enq_seg-next-trbs;
-   return ring-enqueue;
-}
-
 /* Updates trb to point to the next TRB in the ring, and updates seg if the 
next
  * TRB is in a new segment.  This does not skip over link TRBs, and it does not
  * effect the ring dequeue or enqueue pointers.
@@ -680,12 +670,14 @@ static void td_to_noop(struct xhci_hcd *xhci, struct 
xhci_ring *ep_ring,
}
 }
 
-static int queue_set_tr_deq(struct xhci_hcd *xhci, int slot_id,
+static int queue_set_tr_deq(struct xhci_hcd *xhci,
+   struct xhci_command *cmd, int slot_id,
unsigned int ep_index, unsigned int stream_id,
struct xhci_segment *deq_seg,
union xhci_trb *deq_ptr, u32 cycle_state);
 
 void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci,
+   struct xhci_command *cmd,
unsigned int slot_id, unsigned int ep_index,
unsigned int stream_id,
struct xhci_dequeue_state *deq_state)
@@ -700,7 +692,7 @@ void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci,
deq_state-new_deq_ptr,
(unsigned long 
long)xhci_trb_virt_to_dma(deq_state-new_deq_seg, deq_state-new_deq_ptr),
deq_state-new_cycle_state);
-   queue_set_tr_deq(xhci, slot_id, ep_index, stream_id,
+   queue_set_tr_deq(xhci, cmd, slot_id, ep_index, stream_id,
deq_state-new_deq_seg,
deq_state-new_deq_ptr,
(u32) deq_state-new_cycle_state);
@@ -857,12 +849,11 @@ remove_finished_td:
if (deq_state.new_deq_ptr  deq_state.new_deq_seg) {
struct xhci_command *command;
command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
-   xhci_queue_new_dequeue_state(xhci,
+   xhci_queue_new_dequeue_state(xhci, command,
slot_id, ep_index,
ep-stopped_td-urb-stream_id,
deq_state);
xhci_ring_cmd_db(xhci);
-   kfree(command);
} else {
/* Otherwise ring the doorbell(s) to restart queued transfers */
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
@@ -1187,11 +1178,10 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd 
*xhci, int slot_id,
command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
Queueing configure endpoint command);
-   xhci_queue_configure_endpoint(xhci,
+   

[RFCv2 09/10] xhci: Use completion and status in global command queue

2014-01-30 Thread Mathias Nyman
Remove the per-device command list and handle_cmd_in_cmd_wait_list()
and use the completion and status variables found in the
command structure in the global command list.

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci-hub.c  | 11 --
 drivers/usb/host/xhci-mem.c  |  1 -
 drivers/usb/host/xhci-ring.c | 79 
 drivers/usb/host/xhci.c  | 20 ++-
 drivers/usb/host/xhci.h  |  3 --
 5 files changed, 17 insertions(+), 97 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 4342ec3..1079cc6 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -299,7 +299,6 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
 suspend);
}
}
-   list_add_tail(cmd-cmd_list, virt_dev-cmd_list);
xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(xhci-lock, flags);
@@ -311,18 +310,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
if (timeleft = 0) {
xhci_warn(xhci, %s while waiting for stop endpoint command\n,
timeleft == 0 ? Timeout : Signal);
-   spin_lock_irqsave(xhci-lock, flags);
-   /* The timeout might have raced with the event ring handler, so
-* only delete from the list if the item isn't poisoned.
-*/
-   if (cmd-cmd_list.next != LIST_POISON1)
-   list_del(cmd-cmd_list);
-   spin_unlock_irqrestore(xhci-lock, flags);
ret = -ETIME;
-   goto command_cleanup;
}
-
-command_cleanup:
xhci_free_command(xhci, cmd);
return ret;
 }
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 7f8e4c3..80b9c11 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -919,7 +919,6 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int 
slot_id,
dev-num_rings_cached = 0;
 
init_completion(dev-cmd_completion);
-   INIT_LIST_HEAD(dev-cmd_list);
dev-udev = udev;
 
/* Point to output device context in dcbaa. */
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5ccf642..6b61787 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -69,10 +69,6 @@
 #include xhci.h
 #include xhci-trace.h
 
-static int handle_cmd_in_cmd_wait_list(struct xhci_hcd *xhci,
-   struct xhci_virt_device *virt_dev,
-   struct xhci_event_cmd *event);
-
 /*
  * Returns zero if the TRB isn't in this segment, otherwise it returns the DMA
  * address of the TRB.
@@ -761,7 +757,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
union xhci_trb *trb, struct xhci_event_cmd *event)
 {
unsigned int ep_index;
-   struct xhci_virt_device *virt_dev;
struct xhci_ring *ep_ring;
struct xhci_virt_ep *ep;
struct list_head *entry;
@@ -771,11 +766,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
struct xhci_dequeue_state deq_state;
 
if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb-generic.field[3] {
-   virt_dev = xhci-devs[slot_id];
-   if (virt_dev)
-   handle_cmd_in_cmd_wait_list(xhci, virt_dev,
-   event);
-   else
+   if (!xhci-devs[slot_id])
xhci_warn(xhci, Stop endpoint command 
completion for disabled slot %u\n,
slot_id);
@@ -1203,29 +1194,6 @@ static void xhci_complete_cmd_in_cmd_wait_list(struct 
xhci_hcd *xhci,
 }
 
 
-/* Check to see if a command in the device's command queue matches this one.
- * Signal the completion or free the command, and return 1.  Return 0 if the
- * completed command isn't at the head of the command list.
- */
-static int handle_cmd_in_cmd_wait_list(struct xhci_hcd *xhci,
-   struct xhci_virt_device *virt_dev,
-   struct xhci_event_cmd *event)
-{
-   struct xhci_command *command;
-
-   if (list_empty(virt_dev-cmd_list))
-   return 0;
-
-   command = list_entry(virt_dev-cmd_list.next,
-   struct xhci_command, cmd_list);
-   if (xhci-cmd_ring-dequeue != command-command_trb)
-   return 0;
-
-   xhci_complete_cmd_in_cmd_wait_list(xhci, command,
-   GET_COMP_CODE(le32_to_cpu(event-status)));
-   return 1;
-}
-
 /*
  * Finding the command trb need to be cancelled and modifying it to
  * NO OP command. And if the command is in device's command wait
@@ -1377,7 +1345,6 @@ static void xhci_handle_cmd_enable_slot(struct 

[RFCv2 08/10] xhci: Add a global command queue

2014-01-30 Thread Mathias Nyman
Create a list to store command structures, add a strucure to it every time
a command is submitted, and remove it from the list once we get a
command completion event matching the command.

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci-mem.c  |  8 
 drivers/usb/host/xhci-ring.c | 13 -
 drivers/usb/host/xhci.h  |  1 +
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 49b8bd0..7f8e4c3 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1694,6 +1694,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)-self.controller);
struct xhci_cd  *cur_cd, *next_cd;
+   struct xhci_command *cur_cmd, *next_cmd;
int size;
int i, j, num_ports;
 
@@ -1722,6 +1723,12 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
kfree(cur_cd);
}
 
+   list_for_each_entry_safe(cur_cmd, next_cmd,
+   xhci-cmd_list, cmd_list) {
+   list_del(cur_cmd-cmd_list);
+   kfree(cur_cmd);
+   }
+
for (i = 1; i  MAX_HC_SLOTS; ++i)
xhci_free_virt_device(xhci, i);
 
@@ -2223,6 +2230,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
int i;
 
INIT_LIST_HEAD(xhci-cancel_cmd_list);
+   INIT_LIST_HEAD(xhci-cmd_list);
 
page_size = xhci_readl(xhci, xhci-op_regs-page_size);
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6f6018c..5ccf642 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1502,6 +1502,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
dma_addr_t cmd_dequeue_dma;
u32 cmd_comp_code;
union xhci_trb *cmd_trb;
+   struct xhci_command *cmd;
u32 cmd_type;
 
cmd_dma = le64_to_cpu(event-cmd_trb);
@@ -1519,6 +1520,13 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
return;
}
 
+   cmd = list_entry(xhci-cmd_list.next, struct xhci_command, cmd_list);
+
+   if (cmd-command_trb != xhci-cmd_ring-dequeue) {
+   xhci_err(xhci,
+Command completion event does not match command\n);
+   return;
+   }
trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event);
 
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event-status));
@@ -1588,6 +1596,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci-error_bitmask |= 1  6;
break;
}
+
+   list_del(cmd-cmd_list);
+
inc_deq(xhci, xhci-cmd_ring);
 }
 
@@ -3989,7 +4000,7 @@ static int queue_command(struct xhci_hcd *xhci, struct 
xhci_command *cmd,
return ret;
}
cmd-command_trb = xhci-cmd_ring-enqueue;
-
+   list_add_tail(cmd-cmd_list, xhci-cmd_list);
queue_trb(xhci, xhci-cmd_ring, false, field1, field2, field3,
field4 | xhci-cmd_ring-cycle_state);
return 0;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d02b73d..71aed35 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1475,6 +1475,7 @@ struct xhci_hcd {
 #define CMD_RING_STATE_ABORTED (1  1)
 #define CMD_RING_STATE_STOPPED (1  2)
struct list_headcancel_cmd_list;
+   struct list_headcmd_list;
unsigned intcmd_ring_reserved_trbs;
struct xhci_ring*event_ring;
struct xhci_ersterst;
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFCv2 00/10] xhci: re-work command queue management

2014-01-30 Thread Mathias Nyman
Only changes since v1 are fixing smatch warnings and errors.
  patch 01/10 
Check for null return from alloc_command, release lock in error path and
don't dereference possible null pointer in error path.

  patch 04/10
release lock in xhci_stop_dev() error path.

This is the second attempt to re-work and solve the issues in xhci command
queue management that Sarah has descibed earlier:

Right now, the command management in the xHCI driver is rather ad-hock.  
Different parts of the driver all submit commands, including interrupt 
handling routines, functions called from the USB core (with or without the
bus bandwidth mutex held).
Some times they need to wait for the command to complete, and sometimes 
they just issue the command and don't care about the result of the command.

The places that wait on a command all time the command for five seconds,
and then attempt to cancel the command.  
Unfortunately, that means if several commands are issued at once, and one of
them times out, all the commands timeout, even though the host hasn't gotten
a chance to service them yet.

This is apparent with some devices that take a long time to respond to the 
Set Address command during device enumeration (when the device is plugged in).
If a driver for a different device attempts to change alternate interface
settings at the same time (causing a Configure Endpoint command to be issued),
both commands timeout.

Instead of having each command timeout after five seconds, the driver should
wait indefinitely in an uninterruptible sleep on the command completion.  
A global command queue manager should time whatever command is currently
running, and cancel that command after five seconds.

If the commands were in a list, like TDs currently are, it may be easier to keep
track of where the command ring dequeue pointer is, and avoid racing with 
events.
We may need to have parts of the driver that issue commands without waiting on
them still put the commands in the command list.

The Implementation:
---

First step is to create a list of the commands submitted to the command queue.
To accomplish this each command is required to be submitted with a properly
filled command structure containing completion, status variable and a pointer to
the command TRB that will be used.

The first 7 patches are all about creating these command structures and
submitting them when we queue commands.
The command structures are allocated on the fly, the commands that are submitted
in interrupt context are allocated with GFP_ATOMIC.

Next, the global command queue is introduced. Commands are added to the queue
when trb's are queued, and remove when the commad completes. 
Also switch to use the status variable and completion in the command struct.

A new timer handles command timeout, the timer is kicked every time when a 
command finishes and there's a new command waiting in the queue, or when a new
command is submitted to an _empty_ command queue.
Timer is deleted when the the last command on the queue finishes (empty queue)

The old cancel_cmd_list is removed. 
When the timer expires we simply tag the current command as ABORTED and start
the ring abortion process. Functions waiting for an aborted command to finish 
are
called after the command abortion is completed.

Mathias Nyman (10):
  xhci: Use command structures when calling xhci_configure_endpoint
  xhci: use a command structure internally in xhci_address_device()
  xhci: use command structures for xhci_queue_slot_control()
  xhci: use command structures for xhci_queue_stop_endpoint()
  xhci: use command structure for xhci_queue_new_dequeue_state()
  xhci: use command structures for xhci_queue_reset_ep()
  xhci: Use command structured when queuing commands
  xhci: Add a global command queue
  xhci: Use completion and status in global command queue
  xhci: rework command timeout and cancellation,

 drivers/usb/host/xhci-hub.c  |  43 ++--
 drivers/usb/host/xhci-mem.c  |  22 +-
 drivers/usb/host/xhci-ring.c | 532 ++-
 drivers/usb/host/xhci.c  | 266 --
 drivers/usb/host/xhci.h  |  43 ++--
 5 files changed, 376 insertions(+), 530 deletions(-)

-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFCv2 01/10] xhci: Use command structures when calling xhci_configure_endpoint

2014-01-30 Thread Mathias Nyman
To create a global command queue we need to fill a command structure
for each entry on the command ring.

We start by requiring xhci_configure_endpoint() to be called with
a proper command structure. Functions xhci_check_maxpacket and 
xhci_check_bandwith
that called xhci_configure_endpoint without a command strucure are fixed.

A special case where an endpoint needs to be configured after reset, done in 
interrupt
context is also changed to create a command strucure. (this command
structure is not used yet, but will be later when we requre all queued commands
to have a command strucure)

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci-ring.c | 10 ++---
 drivers/usb/host/xhci.c  | 97 
 2 files changed, 56 insertions(+), 51 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1e2f3f4..da83a844 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1180,12 +1180,15 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd 
*xhci, int slot_id,
 * because the HW can't handle two commands being queued in a row.
 */
if (xhci-quirks  XHCI_RESET_EP_QUIRK) {
+   struct xhci_command *command;
+   command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
Queueing configure endpoint command);
xhci_queue_configure_endpoint(xhci,
xhci-devs[slot_id]-in_ctx-dma, slot_id,
false);
xhci_ring_cmd_db(xhci);
+   kfree(command);
} else {
/* Clear our internal halted state and restart the ring(s) */
xhci-devs[slot_id]-eps[ep_index].ep_state = ~EP_HALTED;
@@ -1439,7 +1442,7 @@ static void xhci_handle_cmd_config_ep(struct xhci_hcd 
*xhci, int slot_id,
add_flags - SLOT_FLAG == drop_flags) {
ep_state = virt_dev-eps[ep_index].ep_state;
if (!(ep_state  EP_HALTED))
-   goto bandwidth_change;
+   return;
xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
Completed config ep cmd - 
last ep index = %d, state = %d,
@@ -1449,11 +1452,6 @@ static void xhci_handle_cmd_config_ep(struct xhci_hcd 
*xhci, int slot_id,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
return;
}
-bandwidth_change:
-   xhci_dbg_trace(xhci,  trace_xhci_dbg_context_change,
-   Completed config ep cmd);
-   virt_dev-cmd_status = cmd_comp_code;
-   complete(virt_dev-cmd_completion);
return;
 }
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4265b48..a40485e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1179,10 +1179,10 @@ static int xhci_configure_endpoint(struct xhci_hcd 
*xhci,
 static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id,
unsigned int ep_index, struct urb *urb)
 {
-   struct xhci_container_ctx *in_ctx;
struct xhci_container_ctx *out_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
struct xhci_ep_ctx *ep_ctx;
+   struct xhci_command *command;
int max_packet_size;
int hw_max_packet_size;
int ret = 0;
@@ -1207,18 +1207,24 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, 
unsigned int slot_id,
/* FIXME: This won't work if a non-default control endpoint
 * changes max packet sizes.
 */
-   in_ctx = xhci-devs[slot_id]-in_ctx;
-   ctrl_ctx = xhci_get_input_control_ctx(xhci, in_ctx);
+
+   command = xhci_alloc_command(xhci, false, true, GFP_KERNEL);
+   if (!command)
+   return -ENOMEM;
+
+   command-in_ctx = xhci-devs[slot_id]-in_ctx;
+   ctrl_ctx = xhci_get_input_control_ctx(xhci, command-in_ctx);
if (!ctrl_ctx) {
xhci_warn(xhci, %s: Could not get input context, bad 
type.\n,
__func__);
-   return -ENOMEM;
+   ret = -ENOMEM;
+   goto command_cleanup;
}
/* Set up the modified control endpoint 0 */
xhci_endpoint_copy(xhci, xhci-devs[slot_id]-in_ctx,
xhci-devs[slot_id]-out_ctx, ep_index);
 
-   ep_ctx = xhci_get_ep_ctx(xhci, in_ctx, ep_index);
+   ep_ctx = xhci_get_ep_ctx(xhci, command-in_ctx, ep_index);
ep_ctx-ep_info2 = cpu_to_le32(~MAX_PACKET_MASK);
ep_ctx-ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size));
 
@@ -1226,17 

[RFCv2 02/10] xhci: use a command structure internally in xhci_address_device()

2014-01-30 Thread Mathias Nyman
Preparing for the global command queue by using command strucure
in xhci_address_device

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci.c | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a40485e..5bf0f94 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3734,7 +3734,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct 
usb_device *udev)
struct xhci_slot_ctx *slot_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
u64 temp_64;
-   union xhci_trb *cmd_trb;
+   struct xhci_command *command;
 
if (!udev-slot_id) {
xhci_dbg_trace(xhci, trace_xhci_dbg_address,
@@ -3755,11 +3755,19 @@ int xhci_address_device(struct usb_hcd *hcd, struct 
usb_device *udev)
return -EINVAL;
}
 
+   command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+   if (!command)
+   return -ENOMEM;
+
+   command-in_ctx = virt_dev-in_ctx;
+   command-completion = xhci-addr_dev;
+
slot_ctx = xhci_get_slot_ctx(xhci, virt_dev-in_ctx);
ctrl_ctx = xhci_get_input_control_ctx(xhci, virt_dev-in_ctx);
if (!ctrl_ctx) {
xhci_warn(xhci, %s: Could not get input context, bad type.\n,
__func__);
+   kfree(command);
return -EINVAL;
}
/*
@@ -3781,20 +3789,22 @@ int xhci_address_device(struct usb_hcd *hcd, struct 
usb_device *udev)
slot_ctx-dev_info  27);
 
spin_lock_irqsave(xhci-lock, flags);
-   cmd_trb = xhci_find_next_enqueue(xhci-cmd_ring);
+   command-command_trb = xhci_find_next_enqueue(xhci-cmd_ring);
ret = xhci_queue_address_device(xhci, virt_dev-in_ctx-dma,
udev-slot_id);
if (ret) {
spin_unlock_irqrestore(xhci-lock, flags);
xhci_dbg_trace(xhci, trace_xhci_dbg_address,
FIXME: allocate a command ring segment);
+   kfree(command);
return ret;
}
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(xhci-lock, flags);
 
/* ctrl tx can take up to 5 sec; XXX: need more time for xHC? */
-   timeleft = wait_for_completion_interruptible_timeout(xhci-addr_dev,
+   timeleft = wait_for_completion_interruptible_timeout(
+   command-completion,
XHCI_CMD_DEFAULT_TIMEOUT);
/* FIXME: From section 4.3.4: Software shall be responsible for timing
 * the SetAddress() recovery interval required by USB and aborting the
@@ -3804,7 +3814,8 @@ int xhci_address_device(struct usb_hcd *hcd, struct 
usb_device *udev)
xhci_warn(xhci, %s while waiting for address device command\n,
timeleft == 0 ? Timeout : Signal);
/* cancel the address device command */
-   ret = xhci_cancel_cmd(xhci, NULL, cmd_trb);
+   ret = xhci_cancel_cmd(xhci, NULL, command-command_trb);
+   kfree(command);
if (ret  0)
return ret;
return -ETIME;
@@ -3840,6 +3851,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct 
usb_device *udev)
break;
}
if (ret) {
+   kfree(command);
return ret;
}
temp_64 = xhci_read_64(xhci, xhci-op_regs-dcbaa_ptr);
@@ -3874,7 +3886,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct 
usb_device *udev)
xhci_dbg_trace(xhci, trace_xhci_dbg_address,
   Internal device address = %d,
   le32_to_cpu(slot_ctx-dev_state)  DEV_ADDR_MASK);
-
+   kfree(command);
return 0;
 }
 
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFCv2 03/10] xhci: use command structures for xhci_queue_slot_control()

2014-01-30 Thread Mathias Nyman
Preparing for the global command queue by changing functions calling
xhci_queue_slot_control() to internally use command structures

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5bf0f94..4ce8c85 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3564,6 +3564,11 @@ void xhci_free_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
unsigned long flags;
u32 state;
int i, ret;
+   struct xhci_command *command;
+
+   command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+   if (!command)
+   return;
 
 #ifndef CONFIG_USB_DEFAULT_PERSIST
/*
@@ -3579,8 +3584,10 @@ void xhci_free_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
/* If the host is halted due to driver unload, we still need to free the
 * device.
 */
-   if (ret = 0  ret != -ENODEV)
+   if (ret = 0  ret != -ENODEV) {
+   kfree(command);
return;
+   }
 
virt_dev = xhci-devs[udev-slot_id];
 
@@ -3597,16 +3604,20 @@ void xhci_free_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
(xhci-xhc_state  XHCI_STATE_HALTED)) {
xhci_free_virt_device(xhci, udev-slot_id);
spin_unlock_irqrestore(xhci-lock, flags);
+   kfree(command);
return;
}
 
if (xhci_queue_slot_control(xhci, TRB_DISABLE_SLOT, udev-slot_id)) {
spin_unlock_irqrestore(xhci-lock, flags);
xhci_dbg(xhci, FIXME: allocate a command ring segment\n);
+   kfree(command);
return;
}
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(xhci-lock, flags);
+   kfree(command);
+
/*
 * Event command completion handler will free any data structures
 * associated with the slot.  XXX Can free sleep?
@@ -3646,31 +3657,41 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
unsigned long flags;
int timeleft;
int ret;
-   union xhci_trb *cmd_trb;
+   struct xhci_command *command;
+
+   command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+   if (!command)
+   return 0;
 
spin_lock_irqsave(xhci-lock, flags);
-   cmd_trb = xhci_find_next_enqueue(xhci-cmd_ring);
+   command-command_trb = xhci_find_next_enqueue(xhci-cmd_ring);
+   command-completion = xhci-addr_dev;
ret = xhci_queue_slot_control(xhci, TRB_ENABLE_SLOT, 0);
if (ret) {
spin_unlock_irqrestore(xhci-lock, flags);
xhci_dbg(xhci, FIXME: allocate a command ring segment\n);
+   kfree(command);
return 0;
}
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(xhci-lock, flags);
 
/* XXX: how much time for xHC slot assignment? */
-   timeleft = wait_for_completion_interruptible_timeout(xhci-addr_dev,
+   timeleft = wait_for_completion_interruptible_timeout(
+   command-completion,
XHCI_CMD_DEFAULT_TIMEOUT);
if (timeleft = 0) {
xhci_warn(xhci, %s while waiting for a slot\n,
timeleft == 0 ? Timeout : Signal);
/* cancel the enable slot request */
-   return xhci_cancel_cmd(xhci, NULL, cmd_trb);
+   ret = xhci_cancel_cmd(xhci, NULL, command-command_trb);
+   kfree(command);
+   return ret;
}
 
if (!xhci-slot_id) {
xhci_err(xhci, Error while assigning device slot ID\n);
+   kfree(command);
return 0;
}
 
@@ -3705,6 +3726,8 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device 
*udev)
pm_runtime_get_noresume(hcd-self.controller);
 #endif
 
+
+   kfree(command);
/* Is this a LS or FS device under a HS hub? */
/* Hub or peripherial? */
return 1;
@@ -3715,6 +3738,7 @@ disable_slot:
if (!xhci_queue_slot_control(xhci, TRB_DISABLE_SLOT, udev-slot_id))
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(xhci-lock, flags);
+   kfree(command);
return 0;
 }
 
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFCv2 04/10] xhci: use command structures for xhci_queue_stop_endpoint()

2014-01-30 Thread Mathias Nyman
Prepare for the global command ring by using command structures
internally in functions calling xhci_queue_stop_endpoint()

Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
 drivers/usb/host/xhci-hub.c | 15 +--
 drivers/usb/host/xhci.c |  3 +++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 805f234..68bc1c6 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -20,7 +20,8 @@
  * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
-#include linux/gfp.h
+
+#include linux/slab.h
 #include asm/unaligned.h
 
 #include xhci.h
@@ -284,8 +285,18 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
 
spin_lock_irqsave(xhci-lock, flags);
for (i = LAST_EP_INDEX; i  0; i--) {
-   if (virt_dev-eps[i].ring  virt_dev-eps[i].ring-dequeue)
+   if (virt_dev-eps[i].ring  virt_dev-eps[i].ring-dequeue) {
+   struct xhci_command *command;
+   command = xhci_alloc_command(xhci, false, false,
+GFP_NOIO);
+   if (!command) {
+   spin_unlock_irqrestore(xhci-lock, flags);
+   xhci_free_command(xhci, cmd);
+   return -ENOMEM;
+   }
xhci_queue_stop_endpoint(xhci, slot_id, i, suspend);
+   kfree(command);
+   }
}
cmd-command_trb = xhci_find_next_enqueue(xhci-cmd_ring);
list_add_tail(cmd-cmd_list, virt_dev-cmd_list);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4ce8c85..d4684d0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1466,6 +1466,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
unsigned int ep_index;
struct xhci_ring *ep_ring;
struct xhci_virt_ep *ep;
+   struct xhci_command *command;
 
xhci = hcd_to_xhci(hcd);
spin_lock_irqsave(xhci-lock, flags);
@@ -1535,6 +1536,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
 * the first cancellation to be handled.
 */
if (!(ep-ep_state  EP_HALT_PENDING)) {
+   command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
ep-ep_state |= EP_HALT_PENDING;
ep-stop_cmds_pending++;
ep-stop_cmd_timer.expires = jiffies +
@@ -1542,6 +1544,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
add_timer(ep-stop_cmd_timer);
xhci_queue_stop_endpoint(xhci, urb-dev-slot_id, ep_index, 0);
xhci_ring_cmd_db(xhci);
+   kfree(command);
}
 done:
spin_unlock_irqrestore(xhci-lock, flags);
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] tools: usb: aio example applications

2014-01-30 Thread Michal Nazarewicz
On Thu, Jan 30 2014, Robert Baldyga wrote:
 diff --git a/tools/usb/aio_multibuff/device_app/aio_multibuff.c 
 b/tools/usb/aio_multibuff/device_app/aio_multibuff.c

 +static void display_event(struct usb_functionfs_event *event)
 +{
 + static const char *const names[] = {
 + [FUNCTIONFS_BIND] = BIND,
 + [FUNCTIONFS_UNBIND] = UNBIND,
 + [FUNCTIONFS_ENABLE] = ENABLE,
 + [FUNCTIONFS_DISABLE] = DISABLE,
 + [FUNCTIONFS_SETUP] = SETUP,
 + [FUNCTIONFS_SUSPEND] = SUSPEND,
 + [FUNCTIONFS_RESUME] = RESUME,
 + };
 + switch (event-type) {
 + case FUNCTIONFS_BIND:
 + case FUNCTIONFS_UNBIND:
 + case FUNCTIONFS_ENABLE:
 + case FUNCTIONFS_DISABLE:
 + case FUNCTIONFS_SETUP:
 + case FUNCTIONFS_SUSPEND:
 + case FUNCTIONFS_RESUME:
 + printf(Event %s\n, names[event-type]);
 + default:
 + break;

The default case serves no purpose here.

 + }
 +}

 +struct io_buffer {
 + struct iocb **iocb;

I'm wondering whether it would be easier to just declare it as:

+   struct iocb **iocb;

and then allocate the structures as a single flat array rather than
array of pointers to the structures.  I can see why you might prefer not
to do that with “buf”, but struct iocb should not be that big, and
having one indirection less should simplify the code.

 + unsigned char **buf;
 + int cnt;
 + int len;

Make those unsigned.  Add:

+   unsigned requested;

 +};
 +
 +void init_bufs(struct io_buffer *iobuf, int n, int len)

Make those unsigned.

 +{
 + int i;

unsigned

 + iobuf-buf = malloc(n*sizeof(*iobuf-buf));
 + iobuf-iocb = malloc(n*sizeof(*iobuf-iocb));
 + iobuf-cnt = n;
 + iobuf-len = len;
 + for (i = 0; i  n; ++i) {
 + iobuf-buf[i] = malloc(len*sizeof(**iobuf-buf));
 + iobuf-iocb[i] = malloc(sizeof(**iobuf-iocb));
 + }

+   iobuf-requested = 0;

 +}
 +
 +void delete_bufs(struct io_buffer *iobuf)
 +{
 + int i;
 + for (i = 0; i  iobuf-cnt; ++i) {
 + free(iobuf-buf[i]);
 + free(iobuf-iocb[i]);
 + }
 + free(iobuf-buf);
 + free(iobuf-iocb);
 +}
 +
 +#define BUF_LEN  8192
 +#define BUFS_MAX 128
 +#define AIO_MAX  (BUFS_MAX*2)
 +
 +int main(int argc, char *argv[])
 +{
 + int i, ret;
 + char *ep_path;
 +
 + int ep0, ep1;
 +
 + io_context_t ctx;
 +
 + struct io_buffer iobuf1, iobuf2;

+   struct io_buffer iobuf[2];

But to be honest, why are there two of those anyway?  Each have a bunch
of buffers so why multiply number of buffers even more?

 + int requested1 = 0, requested2 = 0;
 + int actual;
 + bool ready;
 +
 + if (argc != 2) {
 + printf(ffs directory not specified!\n);
 + return 1;
 + }
 +
 + ep_path = malloc(strlen(argv[1]) + 4 /* /ep# */ + 1 /* '\0' */);
 + if (!ep_path) {
 + perror(malloc);
 + return 1;
 + }
 +
 + /* open endpoint files */
 + sprintf(ep_path, %s/ep0, argv[1]);
 + ep0 = open(ep_path, O_RDWR);
 + if (ep0  0) {
 + perror(unable to open ep0);
 + return 1;
 + }
 + if (write(ep0, descriptors, sizeof(descriptors))  0) {
 + perror(unable do write descriptors);
 + return 1;
 + }
 + if (write(ep0, strings, sizeof(strings))  0) {
 + perror(unable to write strings);
 + return 1;
 + }
 + sprintf(ep_path, %s/ep1, argv[1]);
 + ep1 = open(ep_path, O_RDWR);
 + if (ep1  0) {
 + perror(unable to open ep1);
 + return 1;
 + }
 +
 + free(ep_path);
 +
 + memset(ctx, 0, sizeof(ctx));
 + /* setup aio context to handle up to AIO_MAX requests */
 + if (io_setup(AIO_MAX, ctx)  0) {
 + perror(unable to setup aio);
 + return 1;
 + }
 +
 + init_bufs(iobuf1, BUFS_MAX, BUF_LEN);
 + init_bufs(iobuf2, BUFS_MAX, BUF_LEN);

+   for (i = 0; i  sizeof iobuf / sizeof *iobuf; ++i)
+   init_bufs(iobuf + i, BUFS_MAX, BUF_LEN);

 +
 + while (1) {
 + handle_ep0(ep0, ready);
 + /* we are waiting for function ENABLE */
 + if (!ready)
 + continue;
 + /*
 +  * when we're preparing new data to submit,
 +  * second buffer being transmitted
 +  */
 + if (!requested1) { /* if all req's from iocb1 completed */
 + actual = 2;
 + for (i = 0; i  iobuf1.cnt; ++i) /* prepare requests */
 + io_prep_pwrite(iobuf1.iocb[i], ep1,
 +iobuf1.buf[i], iobuf1.len, 0);
 + /* submit table of requests */
 + ret = io_submit(ctx, iobuf1.cnt, iobuf1.iocb);
 + if (ret = 0) {
 +

RE: [RFCv2 00/10] xhci: re-work command queue management

2014-01-30 Thread David Laight
From: Mathias Nyman
 Only changes since v1 are fixing smatch warnings and errors.
   patch 01/10
 Check for null return from alloc_command, release lock in error path and
 don't dereference possible null pointer in error path.
 
   patch 04/10
 release lock in xhci_stop_dev() error path.
 
 This is the second attempt to re-work and solve the issues in xhci command
 queue management that Sarah has descibed earlier:
 
 Right now, the command management in the xHCI driver is rather ad-hock.
 Different parts of the driver all submit commands, including interrupt
 handling routines, functions called from the USB core (with or without the
 bus bandwidth mutex held).
 Some times they need to wait for the command to complete, and sometimes
 they just issue the command and don't care about the result of the command.
 
 The places that wait on a command all time the command for five seconds,
 and then attempt to cancel the command.
 Unfortunately, that means if several commands are issued at once, and one of
 them times out, all the commands timeout, even though the host hasn't gotten
 a chance to service them yet.
 
 This is apparent with some devices that take a long time to respond to the
 Set Address command during device enumeration (when the device is plugged in).
 If a driver for a different device attempts to change alternate interface
 settings at the same time (causing a Configure Endpoint command to be issued),
 both commands timeout.
 
 Instead of having each command timeout after five seconds, the driver should
 wait indefinitely in an uninterruptible sleep on the command completion.
 A global command queue manager should time whatever command is currently
 running, and cancel that command after five seconds.
 
 If the commands were in a list, like TDs currently are, it may be easier to 
 keep
 track of where the command ring dequeue pointer is, and avoid racing with 
 events.
 We may need to have parts of the driver that issue commands without waiting on
 them still put the commands in the command list.
 
 The Implementation:
 ---
 
 First step is to create a list of the commands submitted to the command queue.
 To accomplish this each command is required to be submitted with a properly
 filled command structure containing completion, status variable and a pointer 
 to
 the command TRB that will be used.

I think it would be much simpler to allocate a parallel array to the actual
hardware command ring that contains the additional information for the request
(instead of allocating it pre-request).
This would immediately solve any problems allocating the memory from interrupt
context and failing to free in correctly in all the code paths.

A similar solution could be used for the transfer rings thus removing the
need to the 'td' list - which there are reports of it failing to find transfers
and the code paths for aborting isoch transfers are badly broken.

Adding another list that will have its own set of bugs seems retrograde top me.

David




--
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 regression since xhci: replace xhci_write_64() with writeq() - devices not detected

2014-01-30 Thread Xenia Ragiadakou

On 30/01/2014 11:21 πμ, Rafał Miłecki wrote:

2014-01-29 Xenia Ragiadakou burzalod...@gmail.com:

Rafał is it possible to send all the bad output with xhci debugging on?
Since your config file shows that dynamic debugging is on, that would be
easy to do by adding dyndbg='module xhci_hcd +p' or xhci_hcd.dyndbg=+p boot
option to your linux command line.
Also, I am interested in the lspci -v output related to your usb3
controllers.

Sure. I booted without any USB device connected and then with
04d8:00df connected to the USB 3.0 port.

I also attach full lspci output (with -nnv) just in case.



Thanks a lot! The following logs show that definitely something does not 
go well when you perform 64bit mmio transfers:


[snip]
[2.599818] xhci_hcd :04:00.0: // Setting command ring address to 
0x20
[2.649882] xhci_hcd :04:00.0: // xHC command ring deq ptr low 
bits + flags = @
[2.649884] xhci_hcd :04:00.0: // xHC command ring deq ptr high 
bits = @

[snip]
[2.850199] xhci_hcd :04:00.0: ERST deq = 64'hfff0
[2.850202] xhci_hcd :04:00.0: // Set the interrupt modulation 
register

[2.850210] xhci_hcd :04:00.0: // Enable interrupts, cmd = 0x4.
[2.850215] xhci_hcd :04:00.0: // Enabling event ring interrupter 
c900056f1020 by writing 0x2 to irq_pending

[2.850221] xhci_hcd :04:00.0:   c900056f1020: ir_set[0]
[2.850223] xhci_hcd :04:00.0:   c900056f1020: ir_set.pending 
= 0x2
[2.850227] xhci_hcd :04:00.0:   c900056f1024: ir_set.control 
= 0xa0
[2.850232] xhci_hcd :04:00.0:   c900056f1028: 
ir_set.erst_size = 0x1
[2.900268] xhci_hcd :04:00.0:   c900056f1030: 
ir_set.erst_base = @
[2.950340] xhci_hcd :04:00.0:   c900056f1038: 
ir_set.erst_dequeue = @

[snip]

And since after reverting the writeq patch, everything works then for 
sure writeq does not seem to work as I expected. Probably the 64bit 
transaction on the bus is not atomic, but David seems to understand 
better what could be the reason for that problem.


Another thing is whether readq works correctly. According to xhci specs, 
the bits of the command ring control register when read should return 
always 0 (except from the bit 3).
So, i wonder why readq returns 0x (xHC command ring deq 
ptr low bits + flags = @, xHC command ring deq ptr high bits = 
@).


Still I cannot understand though why ordered split transfers are 
necessary for 32bit capable usb3.0 host controllers. Maybe the ordered 
32bit transfers its a requirement by some usb3.0 host controllers in 
order to read/write 64bit fields, regardless their addressing capability.


best regards,
ksenia
--
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] tools: usb: aio example applications

2014-01-30 Thread Peter Stuge
Robert Baldyga wrote:
 v3:
..
 +++ b/tools/usb/aio_multibuff/host_app/Makefile
 @@ -0,0 +1,13 @@
 +CC = gcc
 +LIBUSB_CFLAGS = $(shell pkg-config --cflags libusb-1.0)
 +LIBUSB_LIBS = $(shell pkg-config --libs libusb-1.0)
 +WARNINGS = -Wall -Wextra
 +CFLAGS = $(LIBUSB_CFLAGS) $(WARNINGS)
 +LDFLAGS = $(LIBUSB_LIBS)
 +
 +all: test
 +%: %.c
 + $(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS)
 +
 +clean:
 + $(RM) test

Nice!


 +++ b/tools/usb/aio_multibuff/host_app/test.c
..
 + cnt = libusb_get_device_list(state-ctx, list);
 + if (cnt  0) {
 + printf(no devices found\n);
 + goto error1;
 + }
..
 +error1:
 + libusb_free_device_list(list, 1);
 + libusb_exit(state-ctx);
 + return 1;
 +}

The above tries to free uninitialized memory in the error path.


 + for (i = 0; i  cnt; ++i) {
 + libusb_device *dev = list[i];
 + struct libusb_device_descriptor desc;
 + if (libusb_get_device_descriptor(dev, desc)) {
 + printf(unable to get device descriptor\n);
 + goto error1;
 + }
 + if (desc.idVendor == VENDOR  desc.idProduct == PRODUCT) {
 + state-found = dev;
 + break;
 + }
 + }
 +
 + if (state-found) {
...
 + } else {
 + printf(no devices found\n);
 + goto error1;
 + }

A matter of taste, but I would reverse the if () condition to avoid
the extra indent level for when a device has been found.

I find that makes it more clear what part of the code handles errors
and what part is the expected common case.


A few other things in the same code:

 + if (state-found) {
 + printf(found device\n);
 +
 + printf(open device: );
 + if (libusb_open(state-found, state-handle)) {
 + printf(ERROR\n);
 + goto error1;
 + }
 + printf(DONE\n);
 +
 + if (libusb_kernel_driver_active(state-handle, 0)) {
 + printf(device busy.. detaching\n);
 + if (libusb_detach_kernel_driver(state-handle, 0)) {
 + printf(unable do deatch device\n);

Typo deatch


 + goto error2;
 + }
 + state-attached = 1;
 + } else
 + printf(device free from kernel\n);

This isn't completely accurate, in two ways. First, it's only the
interface and not the entire device which is claimed/detached.
Second, it could be either another userspace program (via the
usbfs kernel driver) or it could be a kernel driver which has
claimed the interface.

libusb_detach_kernel_driver() makes it so that no kernel driver
(usbfs or other) is attached to the interface.

libusb_claim_interface() then makes the usbfs kernel driver attach
the interface.


 +
 + if (libusb_claim_interface(state-handle, 0)) {
 + printf(failed to claim interface\n);
 + goto error3;
 + }
 + } else {

I still recommend the open/claim/detach logic to be:

libusb_open()
if (libusb_claim_interface() != LIBUSB_SUCCESS) {
  ret = libusb_detach_kernel_driver();
  if (ret != LIBUSB_SUCCESS) {
printf(unable to detach kernel driver: %s\n, libusb_error_name(ret));
goto ..
  }
  ret = libusb_claim_interface();
  if ( != LIBUSB_SUCCESS) {
printf(cannot claim interface: %s\n, libusb_error_name(ret));
goto ..
  }
}

If you prefer to be more conservative then don't use libusb_error_name()
which was only introduced in late 2011 and released in 2012.


//Peter
--
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 regression since xhci: replace xhci_write_64() with writeq() - devices not detected

2014-01-30 Thread David Laight
From: Xenia Ragiadakou
 On 30/01/2014 11:21 πμ, Rafał Miłecki wrote:
  2014-01-29 Xenia Ragiadakou burzalod...@gmail.com:
  Rafał is it possible to send all the bad output with xhci debugging on?
  Since your config file shows that dynamic debugging is on, that would be
  easy to do by adding dyndbg='module xhci_hcd +p' or xhci_hcd.dyndbg=+p boot
  option to your linux command line.
  Also, I am interested in the lspci -v output related to your usb3
  controllers.
  Sure. I booted without any USB device connected and then with
  04d8:00df connected to the USB 3.0 port.
 
  I also attach full lspci output (with -nnv) just in case.
 
 
 Thanks a lot! The following logs show that definitely something does not
 go well when you perform 64bit mmio transfers:
 
 [snip]
 [2.599818] xhci_hcd :04:00.0: // Setting command ring address to
 0x20
 [2.649882] xhci_hcd :04:00.0: // xHC command ring deq ptr low
 bits + flags = @
 [2.649884] xhci_hcd :04:00.0: // xHC command ring deq ptr high
 bits = @
 [snip]
 [2.850199] xhci_hcd :04:00.0: ERST deq = 64'hfff0
 [2.850202] xhci_hcd :04:00.0: // Set the interrupt modulation
 register
 [2.850210] xhci_hcd :04:00.0: // Enable interrupts, cmd = 0x4.
 [2.850215] xhci_hcd :04:00.0: // Enabling event ring interrupter
 c900056f1020 by writing 0x2 to irq_pending
 [2.850221] xhci_hcd :04:00.0:   c900056f1020: ir_set[0]
 [2.850223] xhci_hcd :04:00.0:   c900056f1020: ir_set.pending
 = 0x2
 [2.850227] xhci_hcd :04:00.0:   c900056f1024: ir_set.control
 = 0xa0
 [2.850232] xhci_hcd :04:00.0:   c900056f1028:
 ir_set.erst_size = 0x1
 [2.900268] xhci_hcd :04:00.0:   c900056f1030:
 ir_set.erst_base = @
 [2.950340] xhci_hcd :04:00.0:   c900056f1038:
 ir_set.erst_dequeue = @
 [snip]
 
 And since after reverting the writeq patch, everything works then for
 sure writeq does not seem to work as I expected. Probably the 64bit
 transaction on the bus is not atomic, but David seems to understand
 better what could be the reason for that problem.

I can only guess where the hardware guys f*cked it up:-)

 Another thing is whether readq works correctly. According to xhci specs,
 the bits of the command ring control register when read should return
 always 0 (except from the bit 3).
 So, i wonder why readq returns 0x (xHC command ring deq
 ptr low bits + flags = @, xHC command ring deq ptr high bits =
 @).

If 64bit writes are broken, I'd expect the same to be true for the reads.
You should be able to read back a value with both 64bit and two 32bit
reads to see if 64bit requests work.
 
 Still I cannot understand though why ordered split transfers are
 necessary for 32bit capable usb3.0 host controllers. Maybe the ordered
 32bit transfers its a requirement by some usb3.0 host controllers in
 order to read/write 64bit fields, regardless their addressing capability.

I've got a bit of experience of PCIe from getting a small ppc to 'talk'
efficiently to an altera fpga. I learnt more that I wanted to about PCIe!

My suspicion is that the xhci logic was designed with a 32bit interface
(for its own registers, not its dma engine) that is connected to a PCIe
slave.
However PCIe is inherently a 64bit 'bus' (it isn't a bus at all really).
If the host does a 64bit read/write the PCI logic generates a single 64bit
read/write into the xhci registers, and something would have to convert
that into two 32bit requests. I think that logic is sometimes missing.
It might even just be broken because the guys testing the hardware didn't
try it!
A 32bit transfer is just a 64bit one with only 4 (or the 8) byte enables
asserted - this can be handled by a simple multiplexor.

The fact that readq() seems to be returning 0x (rather than
either 32bit word or some obvious combination of both words) might
imply that it is just a bug that has got documented so that they
can sell the silicon.

David



[PATCH RFC 0/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread David Laight
I've marked this as RFC because I'm not sure whether this should
go through the usb tree or netdev.

I'm also not 100% sure that the code that generates a urb should
'know' the entire aligment rules of the host controller.
However this was already done when the flag was added to indicate
that 'unconstrained' fragmentation was supported by xhci.

This patch fixes problems with long scatter-gather lists on xhci bulk
endpoints by removing the limit on the number of entries and requesting
that the code that generates urb mark those that might have misaligned
fragments - which need special treatment.

The xhci driver cannot give a simple limit to the number of fragments
in a non-aligned transfer because it has to further split the fragments
on 64k address boundaries.

The only code (I can find) that currently generates non-aligned tranfers
is in usbnet.

It is important that all these patches be applied together.
More specifically the change xhci-ring.c must be applied AFTER the
change to usbnet.c.
Failure to do so will break some configurations including those using
the ax88179_178a usb ethernet on hosts with the Intel Panther Point
chipset (which I think is the newest one).

David Laight (1):
  usb: Tell xhci when usb data might be misaligned

 drivers/net/usb/usbnet.c |  1 +
 drivers/usb/host/xhci-ring.c | 12 
 drivers/usb/host/xhci.c  |  8 ++--
 include/linux/usb.h  |  1 +
 4 files changed, 16 insertions(+), 6 deletions(-)

-- 
1.8.1.2



--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread David Laight
Some xhci (USB3) controllers have a constraint on the offset within a
bulk transfer of the end of the transfer ring.

The xhci documentation (v1.00, but not the earlier versions) states that
the offset (from the beginning of the transfer) at end of the transfer
ring must be a multiple of the burst size (this is actually 16k for USB3
since the controller is configured for 16 message bursts).
However the effect is probably that the transfer is split at the ring end,
so the target will see correct messages provided the data is 1k aligned.

This mostly affects scatter-gather transfer requests, but can potentially
affect other requests as they must be split on 64k address boundaries.
(It might even affect non-bulk transfers.)

The only known current source of such misaligned transfers is the
ax88179_178a ethernet driver. The hardware stops transmitting ethernet
frames when the host controller (presumably) spilts a 1k message.

Not all host controllers behave this way.
The Intel Panther Point used on recent motherboards is affected.

A fix has been applied to the xhci driver (and backported), however this
has a side effect of limiting the number of fragments that can be sent.
(It works by putting all the buffer fragments in one ring segment.)

The SCSI system generates more fragments than was originally thought, and
code using libusb can generate arbitrarily long transfers that usually
get split into 8k fragments.

We've had reports of 4MB libusb requests failing. A 16MB request would
require 256 fragments (because of the requirement to not cross a 64k
address boundary) so could not be fitted into the 255 ring slots regarless
of the number and alignment of any fragments.

In fact libusb always uses 8k fragments. Anything over 1M can't be
split with the current limit of 128 fragments and is sent unfragmented.
This leads to kmalloc() failures.

This all means that the xhci driver needs to accept unlimited numbers
of 'aligned' fragments and only restrict the number of misaligned ones.

None of the other USB controllers allow buffer fragments that cross
USB message boundaries (512 bytes for USB2), so almost all the code
uses aligned buffers. Potentially these might cross 64k boundaries
at unaligned offsets, but I suspect that really doesn't happen.

So rather than change all the code that generates urbs, this patch
modifies the only code that generates misaligned transfares to tell
the host controller that the buffer might have alignment issues.

The patch:
- Adds the flag URB_UNCONSTRAINED_XFER to urb-transfer_flags.
  This reuses the value of URB_ASYNC_UNLINK (removed in 2005).
- Sets the flag in usbnet.c for all transmit requests.
  Since the buffer offsets aren't aligned an unfragmented message might
  need splitting on a 64k boundary.
- Pass the transfer_flags down to prepare_ring() and only check for
  the end of ring segments (filling with NOPs) if the flag is set.
- Remove the advertised restriction on the number fragments xhci supports.

This doesn't actually define what a 'constrained' transfer is - but
that wasn't defined when no_sg_constraint was added to struct usb_bus.
Possibly there should also be separate limits of the number of 'constrained'
and 'unconstrained' scatter-gather lists. But and the moment the former
is (more or less) required to be infinite, and the limit of the latter
won't be reached by any code that sets the flag.

Signed-off-by: David Laight david.lai...@aculab.com
---
 drivers/net/usb/usbnet.c |  1 +
 drivers/usb/host/xhci-ring.c | 12 
 drivers/usb/host/xhci.c  |  8 ++--
 include/linux/usb.h  |  1 +
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 4671da7..504be5b 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1303,6 +1303,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (build_dma_sg(skb, urb)  0)
goto drop;
}
+   urb-transfer_flags |= URB_UNCONSTRAINED_XFER;
length = urb-transfer_buffer_length;
 
/* don't assume the hardware handles USB_ZERO_PACKET
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a0b248c..5860874 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2932,7 +2932,8 @@ static void queue_trb(struct xhci_hcd *xhci, struct 
xhci_ring *ring,
  * FIXME allocate segments if the ring is full.
  */
 static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
-   u32 ep_state, unsigned int num_trbs, gfp_t mem_flags)
+   u32 ep_state, unsigned int num_trbs, gfp_t mem_flags,
+   unsigned int transfer_flags)
 {
unsigned int num_trbs_needed;
 
@@ -2980,6 +2981,9 @@ static int prepare_ring(struct xhci_hcd *xhci, struct 
xhci_ring *ep_ring,
 * Simplest solution is to fill the trb before the
 * LINK with nop 

some questions about bandwidth calculation

2014-01-30 Thread vichy
Hi all:
I have some questions about bandwidth calculation
1. why tt time need to include one maxp bus time ?
qh-tt_usecs = NS_TO_US (think_time +
usb_calc_bus_time (urb-dev-speed,
is_input, 0, max_packet (maxp)));

2. in tt_available, below is used to check whether tt time is bigger than 125us
if (125  usecs) {
int ufs = (usecs / 125);
int i;
for (i = uframe; i  (uframe + ufs)  i  8; i++)
if (0  tt_usecs[i]) {
ehci_vdbg(ehci,
multi-uframe xfer can't fit 
in frame %d uframe %d\n,
frame, i);
return 0;
}
}
is it possible tt time bigger than 1 uframe?
3. below is the fomula to calculate bus time
Full-speed (Input)
Non-Isochronous Transfer (Handshake Included)
= 9107 + (83.54 * Floor(3.167 + BitStuffTime(Data_bc))) + Host_Delay
what is 9107 and 3.167 used for?
(9102 is not equal to FS bit time * FS protocol overhead)

Thanks for your help in advance,
--
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 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread David Laight
From: Peter Stuge 
...
  code using libusb can generate arbitrarily long transfers that usually
  get split into 8k fragments.
 
 libusb splits transfers into 16k urbs, or doesn't with newer code
 when both kernel and libusb support scatter-gather.
 
  In fact libusb always uses 8k fragments.
 
 Hm? Worst-case libusb-1.0 submits 16k urbs. libusb-0.1 I'm unsure
 about, but could check.
...
 Where's the 8k coming from?

My memory, I meant 16k :-(

David



--
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: FW: xhci ASMedia lockups - a theory and a patch

2014-01-30 Thread David Laight
From: Sarah Sharp
 On Wed, Jan 29, 2014 at 03:48:27PM +, David Laight wrote:
  I've decided to forward this to the list.
  If you read further down he says that my patch makes the operation
  of the ax88179_178a stable - once the system has recognised it properly.
 
 I don't understand why your patch helped.  This person has a 0.96 host,
 which should not require the TD fragment rules.

Unless that is the documentation catching up with reality?

 On Wed, Jan 29, 2014 at 04:21:00PM +1100, renev...@internode.on.net wrote:
  I am someone who has been struggling to get an ax88179 net adapter working
  reliably.
 
  I have an integrated Asmedia 1042 xhci controller that is reportedly version
  0.96 on an ASUS M5A99FX PRO R2.0, BIOS 2201 motherboard based on the AMD 
  990FX
  chipset.
 
 The only thing you have in common here is that you've both got an
 ax88179 device.  I wonder if Mark Lord has one as well.

The ax88179 device is definitely part of the problem.
It really doesn't like bulk transfer requests that are the wrong
length for their embedded header.

It is likely that other hardware recovers.

I think there are a few separate bugs lurking here.

I've found an AMD motherboard with the ASMedia chipset.
I might install a usable linux on it and do some tests.

David



--
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: 174c:5106 1 TB External USB 3.0 Drive Fails to Automount through USB 3.0 dock with XHCI Enabled

2014-01-30 Thread Jay S

Hi Sarah,

I haven't tried using a USB 2.0 cable to attach the dock/drive, but I 
have disabled XHCI in my BIOS and the dock and 1 TB drive work fine at 
USB 2.0 speeds using EHCI. According to the manufacturer, the dock is 
designed for drives up to 4 TB.


On 01/29/2014 06:33 PM, Sarah Sharp wrote:

On Wed, Jan 22, 2014 at 08:51:31AM -0800, Jay S wrote:

Actually, the USB3 device I'm using, a Unitek Y-1072 HDD dock, works
fine with a WD 500 GB drive using XHCI, so it seems like it's
something to do with the larger drive.

I have also tried with a Rosewill RX35-AT-SU3 external enclosure and
a Rosewill RX-DU300 HDD dock. All failed with the 1 TB drive.
Transcend 16 GB flash drives work fine with XHCI, too.

As noted in my problem description, I RMA'd the 1 TB drive and the
replacement drive fails with the same error, though it too works
with USB2 via EHCI.

Have you tried connecting the drive with a USB 2.0 cable to the xHCI
port?  That would be good to rule out issues with xHCI vs EHCI.  Perhaps
the dock simply isn't designed to work with the bigger drive at USB 3.0
speeds?

Sarah Sharp


--
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 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Peter Stuge
David Laight wrote:
  Where's the 8k coming from?
 
 My memory, I meant 16k :-(

No problem. But what about that alignment? It seems that userspace
needs to start caring about alignment with xhci, right?


//Peter
--
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] commit ec513b1 from Linus triggers ASMedia quirk

2014-01-30 Thread walt
On 01/29/2014 05:15 PM, Sarah Sharp wrote:
 
 On Wed, Jan 29, 2014 at 04:16:49PM -0800, walt wrote:
 Hi Sarah and David.  I'm back with more ASMedia problems caused by this:

 commit ec513b16c480c6cdda1e3d597e611eafca05227b
 Merge: bcee634 2fc5a7d
 Author: Linus Torvalds torva...@linux-foundation.org
 Date:   Mon Jan 20 16:13:02 2014 -0800
 Merge tag 'usb-3.14-rc1' of 
 git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
 
 Pull USB updates from Greg KH:
  Here's the big USB pull request for 3.14-rc1
 
   Lots of little things all over the place, and the usual USB gadget
   updates, and XHCI fixes (some for an issue reported by a lot of
   people).  USB PHY updates as well as chipidea updates and fixes.

 When booting this kernel (version mentioned above) the ASMedia adapter is
 recognized by the kernel, but the external disk in my USB3 disk docking
 station is not discovered or recognized in any way.

 Try reverting commit 7dd09a1af2c7150269350aaa567a11b06e831003 xhci:
 replace xhci_write_64() with writeq()  That caused problems with
 another xHCI host controller that can only handle 32-bit DMA addresses.

I'm happy to say that fixed it, thanks.
--
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 regression since xhci: replace xhci_write_64() with writeq() - devices not detected

2014-01-30 Thread Xenia Ragiadakou

On 30/01/2014 12:00 πμ, Sarah Sharp wrote:

On Wed, Jan 29, 2014 at 12:50:04PM +0200, Xenia Ragiadakou wrote:

On 29/01/2014 12:08 μμ, Rafał Miłecki wrote:

I've enabled some debugging in xhci-dbg.c, does it help?
xhci_hcd :04:00.0: xHCI capability registers at c90004e6:
xhci_hcd :04:00.0: CAPLENGTH AND HCIVERSION 0x960020:
xhci_hcd :04:00.0: CAPLENGTH: 0x20
xhci_hcd :04:00.0: HCIVERSION: 0x96
xhci_hcd :04:00.0: HCSPARAMS 1: 0x4000820
xhci_hcd :04:00.0:   Max device slots: 32
xhci_hcd :04:00.0:   Max interrupters: 8
xhci_hcd :04:00.0:   Max ports: 4
xhci_hcd :04:00.0: HCSPARAMS 2: 0x17f1
xhci_hcd :04:00.0:   Isoc scheduling threshold: 1
xhci_hcd :04:00.0:   Maximum allowed segments in event ring: 15
xhci_hcd :04:00.0: HCSPARAMS 3 0x0:
xhci_hcd :04:00.0:   Worst case U1 device exit latency: 0
xhci_hcd :04:00.0:   Worst case U2 device exit latency: 0
xhci_hcd :04:00.0: HCC PARAMS 0x200f180:
xhci_hcd :04:00.0:   HC generates 32 bit addresses
xhci_hcd :04:00.0:   FIXME: more HCCPARAMS debugging
xhci_hcd :04:00.0: RTSOFF 0x1000:


Hi Rafał,

Like Bjørn already pointed out, I think too the problem is that the
USB3.0 host controller does not support 64 bit addressing (this can
be seen from the first bit of HCC PARAMS that is 0) but the patch
does not take it into account and blindly tries to perform 64bit
write accesses just because your system is 64bit. My mistake. I will
try to find a solution for that and send a patch when I ll return
home.

I think the solution should be to just revert the writeq patch, and
leave the xhci_write64 in place.  We can always optimize that function
later to do a writeq if the host supports 64-bit writes, but we'll have
to analyze whether the performance impact of doing so makes sense.

Sarah Sharp


I think another solution could be to undefine writeq just before 
including asm-generic/io-64-nonatomic-lo-hi.h header file, so that the 
readq gets defined as two 32bit writes in low-high order.
If you think that that could be a solution, tell me and I can send a 
patch to Rafał to test it when he has time.


regards,
ksenia
--
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 regression since xhci: replace xhci_write_64() with writeq() - devices not detected

2014-01-30 Thread Xenia Ragiadakou

On 30/01/2014 07:42 μμ, Xenia Ragiadakou wrote:

On 30/01/2014 12:00 πμ, Sarah Sharp wrote:

On Wed, Jan 29, 2014 at 12:50:04PM +0200, Xenia Ragiadakou wrote:

On 29/01/2014 12:08 μμ, Rafał Miłecki wrote:

I've enabled some debugging in xhci-dbg.c, does it help?
xhci_hcd :04:00.0: xHCI capability registers at c90004e6:
xhci_hcd :04:00.0: CAPLENGTH AND HCIVERSION 0x960020:
xhci_hcd :04:00.0: CAPLENGTH: 0x20
xhci_hcd :04:00.0: HCIVERSION: 0x96
xhci_hcd :04:00.0: HCSPARAMS 1: 0x4000820
xhci_hcd :04:00.0:   Max device slots: 32
xhci_hcd :04:00.0:   Max interrupters: 8
xhci_hcd :04:00.0:   Max ports: 4
xhci_hcd :04:00.0: HCSPARAMS 2: 0x17f1
xhci_hcd :04:00.0:   Isoc scheduling threshold: 1
xhci_hcd :04:00.0:   Maximum allowed segments in event ring: 15
xhci_hcd :04:00.0: HCSPARAMS 3 0x0:
xhci_hcd :04:00.0:   Worst case U1 device exit latency: 0
xhci_hcd :04:00.0:   Worst case U2 device exit latency: 0
xhci_hcd :04:00.0: HCC PARAMS 0x200f180:
xhci_hcd :04:00.0:   HC generates 32 bit addresses
xhci_hcd :04:00.0:   FIXME: more HCCPARAMS debugging
xhci_hcd :04:00.0: RTSOFF 0x1000:


Hi Rafał,

Like Bjørn already pointed out, I think too the problem is that the
USB3.0 host controller does not support 64 bit addressing (this can
be seen from the first bit of HCC PARAMS that is 0) but the patch
does not take it into account and blindly tries to perform 64bit
write accesses just because your system is 64bit. My mistake. I will
try to find a solution for that and send a patch when I ll return
home.

I think the solution should be to just revert the writeq patch, and
leave the xhci_write64 in place.  We can always optimize that function
later to do a writeq if the host supports 64-bit writes, but we'll have
to analyze whether the performance impact of doing so makes sense.

Sarah Sharp


I think another solution could be to undefine writeq just before 
including asm-generic/io-64-nonatomic-lo-hi.h header file, so that 
the readq gets defined as two 32bit writes in low-high order.
If you think that that could be a solution, tell me and I can send a 
patch to Rafał to test it when he has time.


regards,
ksenia


Well, actually a simple undef won't work here because 
asm-generic/io-64-nonatomic-lo-hi.h includes linux/io.h which defines 
writeq for 64bit systems. So the revert would be necessary i guess :(


ksenia
--
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


Explanation required for usb_kill_urb and usb_poision_urb functions

2014-01-30 Thread Kumar Gaurav

Hi All,

I have some questions regarding below two methods in 
drivers/usr/core/urb.c source code.


Methods:
void usb_kill_urb(struct urb *urb)
void usb_poison_urb(struct urb *urb)

In both functions we have used `wait_event` method but haven't checked 
for the condition `atomic_read(urb-use_count) == 0` before going to sleep.
Should we add the check before putting the calling process to sleep or 
avoiding check is intended (please explain if yes).


Regards
Kumar Gaurav
--
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: some questions about bandwidth calculation

2014-01-30 Thread Alan Stern
On Fri, 31 Jan 2014, vichy wrote:

 Hi all:
 I have some questions about bandwidth calculation
 1. why tt time need to include one maxp bus time ?
 qh-tt_usecs = NS_TO_US (think_time +
 usb_calc_bus_time (urb-dev-speed,
 is_input, 0, max_packet (maxp)));

Because tt_usecs is the time required to send a maximum-size packet.  
So of course you have to include the maxp bus time.

 2. in tt_available, below is used to check whether tt time is bigger than 
 125us
 if (125  usecs) {
 int ufs = (usecs / 125);
 int i;
 for (i = uframe; i  (uframe + ufs)  i  8; i++)
 if (0  tt_usecs[i]) {
 ehci_vdbg(ehci,
 multi-uframe xfer can't fit 
 in frame %d uframe %d\n,
 frame, i);
 return 0;
 }
 }
 is it possible tt time bigger than 1 uframe?

Yes.  Any isochronous transfer that is longer than 188 bytes will
require more than one uframe.

 3. below is the fomula to calculate bus time
 Full-speed (Input)
 Non-Isochronous Transfer (Handshake Included)
 = 9107 + (83.54 * Floor(3.167 + BitStuffTime(Data_bc))) + Host_Delay
 what is 9107 and 3.167 used for?
 (9102 is not equal to FS bit time * FS protocol overhead)

9107 is the overhead.  It includes things like inter-packet delays, the
IN or OUT token packet, the ACK packet, and so on.

The 3.167 is the time required for the DATAx PID byte at the start of
the data packet and the CRC bytes at the end.

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: Explanation required for usb_kill_urb and usb_poision_urb functions

2014-01-30 Thread Alan Stern
On Thu, 30 Jan 2014, Kumar Gaurav wrote:

 Hi All,
 
 I have some questions regarding below two methods in 
 drivers/usr/core/urb.c source code.
 
 Methods:
 void usb_kill_urb(struct urb *urb)
 void usb_poison_urb(struct urb *urb)
 
 In both functions we have used `wait_event` method but haven't checked 
 for the condition `atomic_read(urb-use_count) == 0` before going to sleep.
 Should we add the check before putting the calling process to sleep or 
 avoiding check is intended (please explain if yes).

We don't need to check, because wait_event checks the condition before 
it goes to sleep.

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: Explanation required for usb_kill_urb and usb_poision_urb functions

2014-01-30 Thread Kumar Gaurav

On Thursday 30 January 2014 11:46 PM, Alan Stern wrote:

On Thu, 30 Jan 2014, Kumar Gaurav wrote:


Hi All,

I have some questions regarding below two methods in
drivers/usr/core/urb.c source code.

Methods:
void usb_kill_urb(struct urb *urb)
void usb_poison_urb(struct urb *urb)

In both functions we have used `wait_event` method but haven't checked
for the condition `atomic_read(urb-use_count) == 0` before going to sleep.
Should we add the check before putting the calling process to sleep or
avoiding check is intended (please explain if yes).

We don't need to check, because wait_event checks the condition before
it goes to sleep.

Alan Stern


Okay Now I got it.
Thanks
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread Sarah Sharp
On Thu, Jan 30, 2014 at 09:44:53AM +, David Laight wrote:
 From: Sarah Sharp
   Current issue is when plugging in the ax88179 there is lag when bringing 
   the
   interface up and a bunch of kernel messages:

   [   79.481028] IPv6: ADDRCONF(NETDEV_UP): enp7s0u1: link is not ready
   [   82.210721] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 1
   [   82.338947] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 1
   [   82.467148] ax88179_178a 2-1:1.0 enp7s0u1: kevent 4 may have been 
   dropped
   [   82.470028] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 1
   [   82.595364] ax88179_178a 2-1:1.0 enp7s0u1: kevent 4 may have been 
   dropped
   [   82.598312] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 1
   [   82.723580] ax88179_178a 2-1:1.0 enp7s0u1: kevent 4 may have been 
   dropped
   [   82.726487] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 1
   [   82.851796] ax88179_178a 2-1:1.0 enp7s0u1: kevent 4 may have been 
   dropped
   [   82.854642] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 1

  
  With which kernel?
 
 I saw similar issues testing some patches yesterday.
 Both with the ax179_178a and smsx95xx cards (connected to xhci).
 My kernel claims to be 3.13.0-dsl+ but it is lying since it was
 updated from linus's tree earlier this week.
 I'd not seen any similar delays until the last week or so.

I see a report that these types of errors have been seen since the 3.11
kernel:

http://marc.info/?l=linux-usbm=139105963723745w=2

Since the USB ethernet scatter-gather support wasn't added until the
3.12 kernel, it's unlikely that the xHCI TD fragment issue is actually
the root cause.

The interesting piece of information in that report is that when the USB
3.0 device falls back to USB 2.0 speeds under xHCI, it works.

 The xhci controller I'm using is the Intel 'Panther Point' 8086:1e31
 rev 4 (also says 8086:1e2d and 8086:1e26).

That's the host I have as well.  I've been able to find an ASIX device
with the right chipset, so I'm going to poke around at what's happening
at the USB 3.0 bus level.

Sarah Sharp
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread Mark Lord
On 14-01-30 01:44 PM, Sarah Sharp wrote:
..
 Since the USB ethernet scatter-gather support wasn't added until the
 3.12 kernel, it's unlikely that the xHCI TD fragment issue is actually
 the root cause.
 
 The interesting piece of information in that report is that when the USB
 3.0 device falls back to USB 2.0 speeds under xHCI, it works.

Could be due to something related to the max URB length:

USB 2.0 High-Speed: max URB length 512.
USB 3.0 Super-Speed: max URB length 1024.

Cheers
-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread Paul Zimmerman
 From: linux-usb-ow...@vger.kernel.org 
 [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Sarah Sharp
 Sent: Thursday, January 30, 2014 10:44 AM
 To: David Laight
 Cc: renev...@internode.on.net; linux-usb@vger.kernel.org; Mark Lord; Greg 
 Kroah-Hartman;
 net...@vger.kernel.org
 Subject: Re: Help testing for USB ethernet/xHCI regression
 
 On Thu, Jan 30, 2014 at 09:44:53AM +, David Laight wrote:
  From: Sarah Sharp
Current issue is when plugging in the ax88179 there is lag when 
bringing the
interface up and a bunch of kernel messages:
 
[   79.481028] IPv6: ADDRCONF(NETDEV_UP): enp7s0u1: link is not ready
[   82.210721] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 
1
[   82.338947] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 
1
[   82.467148] ax88179_178a 2-1:1.0 enp7s0u1: kevent 4 may have been 
dropped
[   82.470028] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 
1
[   82.595364] ax88179_178a 2-1:1.0 enp7s0u1: kevent 4 may have been 
dropped
[   82.598312] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 
1
[   82.723580] ax88179_178a 2-1:1.0 enp7s0u1: kevent 4 may have been 
dropped
[   82.726487] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 
1
[   82.851796] ax88179_178a 2-1:1.0 enp7s0u1: kevent 4 may have been 
dropped
[   82.854642] ax88179_178a 2-1:1.0 enp7s0u1: ax88179 - Link status is: 
1
 
  
   With which kernel?
 
  I saw similar issues testing some patches yesterday.
  Both with the ax179_178a and smsx95xx cards (connected to xhci).
  My kernel claims to be 3.13.0-dsl+ but it is lying since it was
  updated from linus's tree earlier this week.
  I'd not seen any similar delays until the last week or so.
 
 I see a report that these types of errors have been seen since the 3.11
 kernel:
 
 http://marc.info/?l=linux-usbm=139105963723745w=2
 
 Since the USB ethernet scatter-gather support wasn't added until the
 3.12 kernel, it's unlikely that the xHCI TD fragment issue is actually
 the root cause.
 
 The interesting piece of information in that report is that when the USB
 3.0 device falls back to USB 2.0 speeds under xHCI, it works.
 
  The xhci controller I'm using is the Intel 'Panther Point' 8086:1e31
  rev 4 (also says 8086:1e2d and 8086:1e26).
 
 That's the host I have as well.  I've been able to find an ASIX device
 with the right chipset, so I'm going to poke around at what's happening
 at the USB 3.0 bus level.

Hi Sarah,

Can you give a pointer to where we could buy one of these devices? I
would like to test this with our (Synopsys) xHCI controller as well.

-- 
Paul

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread Mark Lord
On 14-01-30 02:54 PM, Paul Zimmerman wrote:

 Can you give a pointer to where we could buy one of these devices? I
 would like to test this with our (Synopsys) xHCI controller as well.
 

newegg.com, item N82E16817659005

-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Sarah Sharp
On Thu, Jan 30, 2014 at 05:35:08PM +0100, Peter Stuge wrote:
 David Laight wrote:
   Where's the 8k coming from?
  
  My memory, I meant 16k :-(
 
 No problem. But what about that alignment? It seems that userspace
 needs to start caring about alignment with xhci, right?

We need to step back and reassess the larger picture here, instead of
trying to fire-fight all the regressions caused by the link TRB commit
(35773dac5f86 usb: xhci: Link TRB must not occur within a USB payload
burst).

We shouldn't need to make userspace start to worry about alignment at
all.  libusb worked in the past, before the link TRB fix went in.  We
*cannot* break userspace USB drivers.  The breakage needs to be fixed in
the USB core or the xHCI driver.

Commit 35773dac5f86 was meant to be a short-term bandaid fix, but it's
already caused at least four different regressions.  Some we've fixed,
some have proposed solutions that David has sent.

The storage layer is getting borked because it submits scatter-gather
lists longer than what will fit on a segment, and now libusb has the
same issue.  One xHCI host controller stopped responding to commands,
and reverting the bandaid fix helped.  The implications of this change
just keep coming in, and I'm not comfortable wall-papering over the
issues.

On the flip side, it seems that the only devices that have been helped
by the bandaid fix patch are USB ethernet devices using the ax88179_178a
driver.  (Mark Lord still needs to confirm which device he uses.)  I
have not seen any other reports that other USB ethernet chipsets were
broken in 3.12 by the USB networking layer adding scatter-gather
support.

It should not matter what alignment or length of scatter-gather list the
upper layers pass the xHCI driver, it should just work.  I want to do
this fix right, by changing the fundamental way we queue TRBs to the
rings to fit the TD rules.  We should break each TD into fragment-sized
chunks, and add a link TRB in the middle of a segment where necessary.

Let's do this fix the right way, instead of wall papering over the
issue.  Here's what we should do:

1. Disable scatter-gather for the ax88179_178a driver when it's under an
   xHCI host.

2. Revert the following commits:
   f2d9b991c549 xhci: Set scatter-gather limit to avoid failed block writes.
   d6c9ea9069af xhci: Avoid infinite loop when sg urb requires too many trbs
   35773dac5f86 usb: xhci: Link TRB must not occur within a USB payload burst

3. Dan and Mathias can work together to come up with an overall plan to
   change the xHCI driver architecture to be fully compliant with the TD
   fragment rules.  That can be done over the next few kernel releases.

The end result is that we don't destabilize storage or break userspace
USB drivers, we don't break people's xHCI host controllers,
the ax88179_178a USB ethernet devices still work under xHCI (a bit with
worse performance), and other USB ethernet devices still get the
performance improvement introduced in 3.12.

Sarah Sharp
--
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 regression since xhci: replace xhci_write_64() with writeq() - devices not detected

2014-01-30 Thread Sarah Sharp
On Thu, Jan 30, 2014 at 07:55:20PM +0200, Xenia Ragiadakou wrote:
 Well, actually a simple undef won't work here because
 asm-generic/io-64-nonatomic-lo-hi.h includes linux/io.h which
 defines writeq for 64bit systems. So the revert would be necessary i
 guess :(

It's fine, the hosts are just buggy, so don't worry too much about it.
I think I'll also revert the readq patch, just in case it causes issues
with other xHCI hosts as well.

Sarah Sharp
--
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: FW: xhci ASMedia lockups - a theory and a patch

2014-01-30 Thread renevant
An update, iommu=pt made no difference in the end the system still came down 
eventually.

So i've started from scratch.

Cloned linus' tree

merged for-usb-linus-3.14 from the xhci tree

applied the latest patch I saw from david on the list: 

http://www.spinics.net/lists/linux-usb/msg101747.html


verdict: still get kevent 4 errors when plugging the ax88179_178a into the 
asmedia controller however the device appears to be working without crashing 
the system when plugged into the VIA VL800

Transfer speed has been impacted when transmitting a file via scp, speed has 
reduced from 117MB/s to 30-50MB/s. Recieving a file via scp still functions 
above 100MB/s.

I'm seeing this spammed in my dmesg:

[ 2281.855031] [ cut here ]
[ 2281.855070] WARNING: CPU: 2 PID: 0 at drivers/usb/core/urb.c:476 
usb_submit_urb+0x27c/0x580 [usbcore]()
[ 2281.855075] usb 8-1: BOGUS urb flags, 10 -- 0
[ 2281.855077] Modules linked in: fuse usb_storage ax88179_178a usbnet mii 
ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 
xt_conntrack nf_conntrack ipt_REJECT xt_CHECKSUM iptable_mangle xt_tcpudp 
ip6table_filter ip6_tables iptable_filter ip_tables ebtable_nat ebtables 
x_tables af_packet bridge stp llc vhost_net vhost macvtap it87 
vfio_iommu_type1 macvlan hwmon_vid vfio_pci vfio btrfs hid_generic raid6_pq 
usbhid hid libcrc32c xor kvm_amd kvm radeon crct10dif_pclmul crc32_pclmul 
fbcon ttm ghash_clmulni_intel aesni_intel snd_hda_codec_realtek bitblit 
softcursor snd_hda_codec_generic snd_hda_codec_hdmi aes_x86_64 lrw font 
tileblit drm_kms_helper mxm_wmi drm agpgart snd_hda_intel cfbfillrect 
snd_hda_codec cfbimgblt cfbcopyarea snd_hwdep i2c_algo_bit gf128mul 
glue_helper ablk_helper
[ 2281.855151]  cryptd xhci_hcd snd_pcm psmouse fb sp5100_tco snd_timer 
edac_core edac_mce_amd ohci_pci ohci_hcd ehci_pci ehci_hcd usbcore fbdev 
serio_raw i2c_piix4 snd soundcore pcspkr usb_common k10temp fam15h_power 
tpm_infineon tpm_tis wmi mac_hid microcode ipv6 autofs4 unix
[ 2281.855189] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW3.13.0+ #1
[ 2281.855195] Hardware name: To be filled by O.E.M. To be filled by 
O.E.M./M5A99FX PRO R2.0, BIOS 2201 11/22/2013
[ 2281.855198]  0009 88043ec834d8 8f4f0805 
88043ec83520
[ 2281.855204]  88043ec83510 8f041883 8801886159c0 
880423b04000
[ 2281.855208]  0002 0010 0020 
88043ec83570
[ 2281.855213] Call Trace:
[ 2281.855216]  IRQ  [8f4f0805] dump_stack+0x45/0x56
[ 2281.855231]  [8f041883] warn_slowpath_common+0x73/0x90
[ 2281.855236]  [8f0418e7] warn_slowpath_fmt+0x47/0x50
[ 2281.855256]  [c0a9e548] ? kvm_vcpu_kick+0x78/0x90 [kvm]
[ 2281.855279]  [c010df7c] usb_submit_urb+0x27c/0x580 [usbcore]
[ 2281.855287]  [8f336f07] ? __pm_runtime_resume+0x57/0x70
[ 2281.855294]  [c11f0393] usbnet_start_xmit+0x143/0x5f0 [usbnet]
[ 2281.855303]  [8f44a78b] dev_hard_start_xmit+0x2cb/0x4c0
[ 2281.855314]  [c1141180] ? deliver_clone+0x50/0x50 [bridge]
[ 2281.855321]  [8f467b00] sch_direct_xmit+0xe0/0x1c0
[ 2281.855326]  [8f44ab57] __dev_queue_xmit+0x1d7/0x470
[ 2281.855330]  [8f44adfb] dev_queue_xmit+0xb/0x10
[ 2281.855339]  [c11411f6] br_dev_queue_push_xmit+0x76/0xc0 [bridge]
[ 2281.855348]  [c114140d] br_forward_finish+0x1d/0x60 [bridge]
[ 2281.855357]  [c1141499] __br_deliver+0x49/0x100 [bridge]
[ 2281.855366]  [c11417bb] br_deliver+0x5b/0x60 [bridge]
[ 2281.855374]  [c113f1dc] br_dev_xmit+0x12c/0x260 [bridge]
[ 2281.855380]  [8f44a78b] dev_hard_start_xmit+0x2cb/0x4c0
[ 2281.855385]  [8f44ac76] __dev_queue_xmit+0x2f6/0x470
[ 2281.855389]  [8f44adfb] dev_queue_xmit+0xb/0x10
[ 2281.855395]  [8f480ea9] ip_finish_output+0x329/0x400
[ 2281.855399]  [8f4822a3] ip_output+0x53/0x90
[ 2281.855404]  [8f481a20] ip_local_out+0x20/0x30
[ 2281.855408]  [8f481d68] ip_queue_xmit+0x138/0x3e0
[ 2281.855414]  [8f4981a1] tcp_transmit_skb+0x451/0x8c0
[ 2281.855420]  [8f49adfc] tcp_send_ack+0xcc/0x110
[ 2281.855425]  [8f48e609] __tcp_ack_snd_check+0x59/0x90
[ 2281.855430]  [8f494f29] tcp_rcv_established+0x1f9/0x640
[ 2281.855434]  [8f49e985] tcp_v4_do_rcv+0x1b5/0x4c0
[ 2281.855439]  [8f4a0c84] tcp_v4_rcv+0x774/0x790
[ 2281.855444]  [8f47c830] ? ip_rcv_finish+0x340/0x340
[ 2281.855449]  [8f47633c] ? nf_hook_slow+0x6c/0x130
[ 2281.855454]  [8f47c830] ? ip_rcv_finish+0x340/0x340
[ 2281.855459]  [8f47c8d0] ip_local_deliver_finish+0xa0/0x200
[ 2281.855464]  [8f47cbb3] ip_local_deliver+0x43/0x80
[ 2281.855469]  [8f47c568] ip_rcv_finish+0x78/0x340
[ 2281.855474]  [8f47ce84] ip_rcv+0x294/0x3d0
[ 2281.855479]  [8f44912e] __netif_receive_skb_core+0x5ee/0x7d0
[ 2281.855484]  [8f449323] 

Re: [PATCH RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Mark Lord
On 14-01-30 04:18 PM, Sarah Sharp wrote:

 Let's do this fix the right way, instead of wall papering over the
 issue.  Here's what we should do:
 
 1. Disable scatter-gather for the ax88179_178a driver when it's under an
xHCI host.
 
 2. Revert the following commits:
f2d9b991c549 xhci: Set scatter-gather limit to avoid failed block writes.
d6c9ea9069af xhci: Avoid infinite loop when sg urb requires too many trbs
35773dac5f86 usb: xhci: Link TRB must not occur within a USB payload burst
 
 3. Dan and Mathias can work together to come up with an overall plan to
change the xHCI driver architecture to be fully compliant with the TD
fragment rules.  That can be done over the next few kernel releases.
 
 The end result is that we don't destabilize storage or break userspace
 USB drivers, we don't break people's xHCI host controllers,
 the ax88179_178a USB ethernet devices still work under xHCI (a bit with
 worse performance), and other USB ethernet devices still get the
 performance improvement introduced in 3.12.


Performance before 3.12/3.13 was not all that bad either.
My ax88179 dongle (yes, that one, using ax88179_178a.ko)
manages very close to 1gbit/sec throughput even without SG,
and without a huge cpu tax either.

SG done Right will make it better eventually.  I can wait.

Cheers
-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread Sarah Sharp
On Thu, Jan 30, 2014 at 03:00:47PM -0500, Mark Lord wrote:
 On 14-01-30 02:54 PM, Paul Zimmerman wrote:
 
  Can you give a pointer to where we could buy one of these devices? I
  would like to test this with our (Synopsys) xHCI controller as well.
  
 
 newegg.com, item N82E16817659005

Ah, so it's also the ASIX AX88179 that's causing you issues.

Mark and David, can you pull the 3.13-td-changes-reverted branch again,
and see if the latest patch fixes your issue?  It disables scatter
gather for the ax88179_178a device, but only when it's operating at USB
3.0 speeds.

Please test with it plugged into a USB 3.0 port, and when it's plugged
in behind a USB 2.0 hub.

Sarah Sharp
--
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 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Mark Lord
Sarah, on a related note:

Is there a parameter or knob of some kind to tell the XHCI driver
to treat a specific port as USB2 (480mbit/sec max) rather than USB3 ?

The Dell XPS-13 Ultrabooks all suffer from some kind of flaw, whereby the left 
side
USB3 port is unreliable at SuperSpeed; the right side port works flawlessly.
The MS-Windows driver has a workaround of some sort, but we don't.

Cheers
-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Alan Stern
On Thu, 30 Jan 2014, Sarah Sharp wrote:

 It should not matter what alignment or length of scatter-gather list the
 upper layers pass the xHCI driver, it should just work.  I want to do
 this fix right, by changing the fundamental way we queue TRBs to the
 rings to fit the TD rules.  We should break each TD into fragment-sized
 chunks, and add a link TRB in the middle of a segment where necessary.

That's a good plan.  However _some_ restriction will turn out to be
necessary.

For example, what will you do if a driver submits an SG list containing
300 elements, each 3 bytes long?  That's too many to fit in a single
ring segment, but it's smaller than a TD fragment -- it's even smaller
than maxpacket -- so there's no place to split it.  (Not that I think
drivers _will_ submit requests like this; this is just to demonstrate
the point.)

It ought to be acceptable to require, for example, that an SG URB 
contain no more than (say) 100 elements that are smaller than 512 
bytes.

ehci-hcd gets along okay with the restriction that each SG element 
except the last has to be a multiple of the maxpacket size.  xhci-hcd 
can relax this quite a lot, but not all the way.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Mark Lord
On 14-01-30 04:43 PM, Alan Stern wrote:
 On Thu, 30 Jan 2014, Sarah Sharp wrote:
 
 It should not matter what alignment or length of scatter-gather list the
 upper layers pass the xHCI driver, it should just work.  I want to do
 this fix right, by changing the fundamental way we queue TRBs to the
 rings to fit the TD rules.  We should break each TD into fragment-sized
 chunks, and add a link TRB in the middle of a segment where necessary.
 
 That's a good plan.  However _some_ restriction will turn out to be
 necessary.
 
 For example, what will you do if a driver submits an SG list containing
 300 elements, each 3 bytes long?

Allocate a contiguous (bounce) buffer and copy the fragments to/from it?

-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Bjørn Mork
FWIW, the plan looks fine to me.  Just adding a couple of hints to
simplify the implementation.

Sarah Sharp sarah.a.sh...@linux.intel.com writes:

 Let's do this fix the right way, instead of wall papering over the
 issue.  Here's what we should do:

 1. Disable scatter-gather for the ax88179_178a driver when it's under an
xHCI host.

No need to make this conditional.  SG is only enabled in the
ax88179_178a driver if udev-bus-no_sg_constraint is true, so it
applies only to xHCI hosts in the first place.

 2. Revert the following commits:
f2d9b991c549 xhci: Set scatter-gather limit to avoid failed block writes.
d6c9ea9069af xhci: Avoid infinite loop when sg urb requires too many trbs
35773dac5f86 usb: xhci: Link TRB must not occur within a USB payload burst

 3. Dan and Mathias can work together to come up with an overall plan to
change the xHCI driver architecture to be fully compliant with the TD
fragment rules.  That can be done over the next few kernel releases.

 The end result is that we don't destabilize storage or break userspace
 USB drivers, we don't break people's xHCI host controllers,
 the ax88179_178a USB ethernet devices still work under xHCI (a bit with
 worse performance), and other USB ethernet devices still get the
 performance improvement introduced in 3.12.

No other usbnet drivers has enabled SG...  Which is why you have only
seen this problem with the ax88179_178a devices.  So there is no
performance improvement to keep.



Bjørn
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Sarah Sharp
On Thu, Jan 30, 2014 at 04:43:54PM -0500, Alan Stern wrote:
 On Thu, 30 Jan 2014, Sarah Sharp wrote:
 
  It should not matter what alignment or length of scatter-gather list the
  upper layers pass the xHCI driver, it should just work.  I want to do
  this fix right, by changing the fundamental way we queue TRBs to the
  rings to fit the TD rules.  We should break each TD into fragment-sized
  chunks, and add a link TRB in the middle of a segment where necessary.
 
 That's a good plan.  However _some_ restriction will turn out to be
 necessary.
 
 For example, what will you do if a driver submits an SG list containing
 300 elements, each 3 bytes long?  That's too many to fit in a single
 ring segment, but it's smaller than a TD fragment -- it's even smaller
 than maxpacket -- so there's no place to split it.  (Not that I think
 drivers _will_ submit requests like this; this is just to demonstrate
 the point.)
 
 It ought to be acceptable to require, for example, that an SG URB 
 contain no more than (say) 100 elements that are smaller than 512 
 bytes.

At that point, the xHCI driver or USB core should probably use a bounce
buffer.  It feels like we should attempt to push down scatter-gather
lists as far down in the stack as possible, so the upper layers don't
have to care what alignment, length, or random 64KB boundary splits we
need.

 ehci-hcd gets along okay with the restriction that each SG element 
 except the last has to be a multiple of the maxpacket size.  xhci-hcd 
 can relax this quite a lot, but not all the way.

What does the EHCI driver do when it receives a SG list from the USB
networking layer that violates this restriction?

Sarah Sharp
--
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 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Bjørn Mork
Sarah Sharp sarah.a.sh...@linux.intel.com writes:
 On Thu, Jan 30, 2014 at 04:43:54PM -0500, Alan Stern wrote:

 ehci-hcd gets along okay with the restriction that each SG element 
 except the last has to be a multiple of the maxpacket size.  xhci-hcd 
 can relax this quite a lot, but not all the way.

 What does the EHCI driver do when it receives a SG list from the USB
 networking layer that violates this restriction?

The USB networking layer won't use SG with the EHCI driver.

Commit bcc48f1a7a0d4 introduced no_sg_constraint so that usbnet could
enable SG only for host controllers with no such restrictions. I.e.
currently for xHCI only.


Bjørn
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Alan Stern
On Thu, 30 Jan 2014, Sarah Sharp wrote:

  That's a good plan.  However _some_ restriction will turn out to be
  necessary.
  
  For example, what will you do if a driver submits an SG list containing
  300 elements, each 3 bytes long?  That's too many to fit in a single
  ring segment, but it's smaller than a TD fragment -- it's even smaller
  than maxpacket -- so there's no place to split it.  (Not that I think
  drivers _will_ submit requests like this; this is just to demonstrate
  the point.)
  
  It ought to be acceptable to require, for example, that an SG URB 
  contain no more than (say) 100 elements that are smaller than 512 
  bytes.
 
 At that point, the xHCI driver or USB core should probably use a bounce
 buffer.  It feels like we should attempt to push down scatter-gather
 lists as far down in the stack as possible, so the upper layers don't
 have to care what alignment, length, or random 64KB boundary splits we
 need.

Okay.  That should be doable, if awkward.

  ehci-hcd gets along okay with the restriction that each SG element 
  except the last has to be a multiple of the maxpacket size.  xhci-hcd 
  can relax this quite a lot, but not all the way.
 
 What does the EHCI driver do when it receives a SG list from the USB
 networking layer that violates this restriction?

It never receives such lists.  usb_submit_urb() returns -EINVAL before 
the request gets sent to ehci-hcd.

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: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread Mark Lord
On 14-01-30 04:41 PM, Sarah Sharp wrote:

 Mark and David, can you pull the 3.13-td-changes-reverted branch again,
 and see if the latest patch fixes your issue?  It disables scatter
 gather for the ax88179_178a device, but only when it's operating at USB
 3.0 speeds.

As expected, this works just fine.


-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFCv2 00/10] xhci: re-work command queue management

2014-01-30 Thread Sarah Sharp
On Thu, Jan 30, 2014 at 02:25:48PM +, David Laight wrote:
 I think it would be much simpler to allocate a parallel array to the actual
 hardware command ring that contains the additional information for the request
 (instead of allocating it pre-request).
 This would immediately solve any problems allocating the memory from interrupt
 context and failing to free in correctly in all the code paths.
 
 A similar solution could be used for the transfer rings thus removing the
 need to the 'td' list - which there are reports of it failing to find 
 transfers
 and the code paths for aborting isoch transfers are badly broken.
 
 Adding another list that will have its own set of bugs seems retrograde top 
 me.

I do not have a problem with it.  The shadow ring is an optimization we
can look at later.

Sarah Sharp
--
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 1/1] usb: Tell xhci when usb data might be misaligned

2014-01-30 Thread Ming Lei
On Fri, Jan 31, 2014 at 6:15 AM, Sarah Sharp
sarah.a.sh...@linux.intel.com wrote:
 On Thu, Jan 30, 2014 at 10:50:21PM +0100, Bjørn Mork wrote:
 FWIW, the plan looks fine to me.  Just adding a couple of hints to
 simplify the implementation.

 Sarah Sharp sarah.a.sh...@linux.intel.com writes:

  Let's do this fix the right way, instead of wall papering over the
  issue.  Here's what we should do:
 
  1. Disable scatter-gather for the ax88179_178a driver when it's under an
 xHCI host.

 No need to make this conditional.  SG is only enabled in the
 ax88179_178a driver if udev-bus-no_sg_constraint is true, so it
 applies only to xHCI hosts in the first place.

 Ah, so you're suggesting just reverting commit
 3804fad45411b48233b48003e33a78f290d227c8 USBNET: ax88179_178a: enable
 tso if usb host supports sg dma?

If I understand the problem correctly, the current issue is that xhci driver
doesn't support the arbitrary dma length not well, but per XHCI spec, it
should be supported, right?

If the above is correct, reverting the commit isn't correct since there isn't
any issue about the commit, so I suggest to disable the flag in xhci
for the buggy devices, and it may be enabled again if the problem is fixed.


  2. Revert the following commits:
 f2d9b991c549 xhci: Set scatter-gather limit to avoid failed block 
  writes.
 d6c9ea9069af xhci: Avoid infinite loop when sg urb requires too many 
  trbs
 35773dac5f86 usb: xhci: Link TRB must not occur within a USB payload 
  burst
 
  3. Dan and Mathias can work together to come up with an overall plan to
 change the xHCI driver architecture to be fully compliant with the TD
 fragment rules.  That can be done over the next few kernel releases.
 
  The end result is that we don't destabilize storage or break userspace
  USB drivers, we don't break people's xHCI host controllers,
  the ax88179_178a USB ethernet devices still work under xHCI (a bit with
  worse performance), and other USB ethernet devices still get the
  performance improvement introduced in 3.12.

 No other usbnet drivers has enabled SG...  Which is why you have only
 seen this problem with the ax88179_178a devices.  So there is no
 performance improvement to keep.

In my test environment, the patch does improve both throughput and
cpu utilization, if you search the previous email for the patch, you can
see the data.

Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


MAX3421E Linux driver?

2014-01-30 Thread David Mosberger
We have a need to graft a USB Host controller onto a SPI bus (never
mind the wisdom of doing that, we have little
choice in this particular instance).

The Cypress CY7C67300 would fit the bill and has a Linux driver, but
is probably too complex and definitely
too expensive for our needs.

The Maxim MAX3421E is the other option, but it has no Linux driver.
The chip basically implements OHCI over SPI
rather than PCI.  Anybody know of any particular reason why there is
no Linux driver?  Has anyone already written or
attempted to write an OHCI-over-SPI driver for MAX3421E?

Best regards,

  --david
-- 
eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768
--
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: MAX3421E Linux driver?

2014-01-30 Thread Felipe Balbi
Hi,

On Thu, Jan 30, 2014 at 05:34:59PM -0700, David Mosberger wrote:
 We have a need to graft a USB Host controller onto a SPI bus (never
 mind the wisdom of doing that, we have little
 choice in this particular instance).
 
 The Cypress CY7C67300 would fit the bill and has a Linux driver, but
 is probably too complex and definitely
 too expensive for our needs.
 
 The Maxim MAX3421E is the other option, but it has no Linux driver.
 The chip basically implements OHCI over SPI
 rather than PCI.  Anybody know of any particular reason why there is
 no Linux driver?  Has anyone already written or
 attempted to write an OHCI-over-SPI driver for MAX3421E?

I don't think anybody has attempted it. If you wanna do it, we're happy
to review the driver.

cheers

-- 
balbi


signature.asc
Description: Digital signature


Re: MAX3421E Linux driver?

2014-01-30 Thread Peter Stuge
David Mosberger wrote:
 The Maxim MAX3421E is the other option, but it has no Linux driver.

You could look at http://arduino.cc/en/Main/ArduinoBoardADK for a
reference that might even work.


//Peter
--
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: MAX3421E Linux driver?

2014-01-30 Thread David Mosberger
Peter,

On Thu, Jan 30, 2014 at 6:09 PM, Peter Stuge pe...@stuge.se wrote:
 David Mosberger wrote:
 The Maxim MAX3421E is the other option, but it has no Linux driver.

 You could look at http://arduino.cc/en/Main/ArduinoBoardADK for a
 reference that might even work.

We are aware of Arduino but the point of our project is to be able to
use normal Linux USB drivers.  As far as I know, Arduino has it's own
(limited) USB stack.  Perhaps I'm missing something?

  --david



 //Peter



-- 
eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768
--
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: MAX3421E Linux driver?

2014-01-30 Thread Peter Stuge
David Mosberger wrote:
  David Mosberger wrote:
  The Maxim MAX3421E is the other option, but it has no Linux driver.
 
  You could look at http://arduino.cc/en/Main/ArduinoBoardADK for a
  reference that might even work.
 
 We are aware of Arduino but the point of our project is to be able to
 use normal Linux USB drivers.  As far as I know, Arduino has it's own
 (limited) USB stack.  Perhaps I'm missing something?

Sorry, I absolutely did not suggest that you actually use the Arduino
for anything, but the ADK code might actually work and could serve as
a reference for driving the MAX.


//Peter
--
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 audio breaks ohci-pci

2014-01-30 Thread Dennis New


On Thu, 23 Jan 2014 16:41:26 -0500 (EST), Alan Stern wrote:
 On Thu, 23 Jan 2014, Dennis New wrote:
 
  On Wed, 15 Jan 2014 17:04:05 -0500 (EST), Alan Stern wrote:
   On Wed, 15 Jan 2014, Dennis New wrote:
   
When listening to my usb-bluetooth audio headset, the usb
subsystem (I think) on my laptop crashes, the bluetooth/audio
is lost, and my mplayer program is left in an uninterruptible
sleep (D) state and cannot be killed. The only way to get my
usb-audio working again is to reboot.
   
   ...
   
The crash is difficult to reproduce, sometimes it can occur
after a few minutes of a clean boot, and sometimes after a few
days. After the audio/usb abruptly stops, the syslog simply
says:

  kernel: ALSA sound/usb/endpoint.c:501 timeout: still 3 active
urbs on EP #3

And then after a few minutes (after enabling some kernel
debugging options), I get some call traces of the hung task(s),
which have ohci_urb_dequeue in common.

  http://dennisn.dyndns.org/guest/pubstuff/debug-usbaudio/crash1.log
  http://dennisn.dyndns.org/guest/pubstuff/debug-usbaudio/crash2.log
  http://dennisn.dyndns.org/guest/pubstuff/debug-usbaudio/crash3.log
   
   The logs show that the khubd thread is hung, waiting for the
   ohci-hcd driver to finish cancelling a transfer.
   
The problem is not with my device, since it occurs with another
identical piece of hardware. It maybe be a problem with my
laptop, since my friend does not seem to be experiencing this
problem, although her laptop does not use ohci-pci.

My usb controllers are:

  1002:4374 00:13.0 USB controller: Advanced Micro Devices [AMD]
nee ATI IXP SB400 USB Host Controller (prog-if 10 [OHCI])
  1002:4375 00:13.1 USB controller: Advanced Micro Devices [AMD]
nee ATI IXP SB400 USB Host Controller (prog-if 10 [OHCI])
  1002:4373 00:13.2 USB controller: Advanced Micro Devices [AMD]
nee ATI IXP SB400 USB2 Host Controller (prog-if 20 [EHCI])
   
   Your first step in attacking this problem should be to use the
   most up-to-date kernel available, which at the moment is
   3.13-rc8.  Be sure to enable CONFIG_FRAME_POINTER along with
   CONFIG_USB_DEBUG.
   
   If the problem occurs again, collect a copy of the output from the
   dmesg command (_not_ a copy of the system log).  Then go into
   /sys/kernel/debug/usb/ohci and collect copies of all the files in
   the two subdirectories.
   
   It may be necessary to apply some debugging patches to get more 
   information.
  
  Okay, so it happened again (after the possibly usbaudio induced
  crash none of my usb ports work at all), and I was able to get some
  info from /sys/kernel/debug/usb this time.
  
  http://dennisn.dyndns.org/guest/pubstuff/debug-usbaudio/crash5.log
 
 Your problem looks a lot like the one in this email thread:
 
   http://marc.info/?l=linux-usbm=139041301317106w=2
 
 Check it out.  That patches in that thread won't help on your
 machine, though, because you're using an ATI motherboard rather than
 NVIDIA.
 
 (Not to mention that it's very hard to test anything in your setup, 
 because the problem occurs so rarely.)

Indeed, ohci_quirk_zfmicro (as mentioned in that marc.info link)
would crash my kernel/system (I think when some graphics switch would
happen) :). So I tried ohci_quirk_amd700, since there were already
some PCI_VENDOR_ID_ATI quirks in ohci-pci.c that were using it, but
alas, no luck. After about 6 days, the hang/crash happened again. I
wasn't able to get debugging info this time around :s.

The mystery continues.

--
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: MAX3421E Linux driver?

2014-01-30 Thread David Mosberger
Great find, I'm impressed!  Yeah, probably not much hope of contacting
the author (it's from 2008), but it sounds like the basics were pretty
straight-forward.  Good to know.

  --david

On Thu, Jan 30, 2014 at 6:54 PM,  si...@mungewell.org wrote:
 We have a need to graft a USB Host controller onto a SPI bus (never
 mind the wisdom of doing that, we have little
 choice in this particular instance).

 The Cypress CY7C67300 would fit the bill and has a Linux driver, but
 is probably too complex and definitely
 too expensive for our needs.

 The Maxim MAX3421E is the other option, but it has no Linux driver.

 Since the device sounds very interesting, I started googling around to
 find this comment:
 http://hary040314.blog.163.com/blog/static/77162442200822125922706/
 --
 Today ,I add max3421e to linux kernel 2.6.13 as usb host controller on
 platform s3c2440 successful.

 For now,I can mount mobile hard disk to sysfs of linux,and list all the
 files and folders on the mobile hard disk.

 But the speed is slow,and even worse,I can't exchange files larger than 8M
 through it.I thought ,the road to success finally ahead is long.
 --

 The rest of the page is Chinese(?), but it might suggest that someone got
 something working at one time
 Simon




-- 
eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768
--
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: MAX3421E Linux driver?

2014-01-30 Thread Paul Zimmerman
 From: linux-usb-ow...@vger.kernel.org 
 [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of David Mosberger
 Sent: Thursday, January 30, 2014 4:35 PM
 
 We have a need to graft a USB Host controller onto a SPI bus (never
 mind the wisdom of doing that, we have little
 choice in this particular instance).
 
 The Cypress CY7C67300 would fit the bill and has a Linux driver, but
 is probably too complex and definitely
 too expensive for our needs.
 
 The Maxim MAX3421E is the other option, but it has no Linux driver.
 The chip basically implements OHCI over SPI
 rather than PCI.  Anybody know of any particular reason why there is
 no Linux driver?  Has anyone already written or
 attempted to write an OHCI-over-SPI driver for MAX3421E?

ISTR that the MAX3421E does not in fact have an OHCI register interface;
I believe it's some proprietary thing. If I'm right, that might help
explain why there's no Linux driver for it.

-- 
Paul

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help testing for USB ethernet/xHCI regression

2014-01-30 Thread Mark Lord
On 14-01-30 06:26 PM, Sarah Sharp wrote:
 On Thu, Jan 30, 2014 at 05:20:40PM -0500, Mark Lord wrote:
 On 14-01-30 04:41 PM, Sarah Sharp wrote:

 Mark and David, can you pull the 3.13-td-changes-reverted branch again,
 and see if the latest patch fixes your issue?  It disables scatter
 gather for the ax88179_178a device, but only when it's operating at USB
 3.0 speeds.

 As expected, this works just fine.
 
 Did it work when plugged into a USB 2.0 hub?

Curiosity, NO.  Dies almost immediately when run at USB 2.0 Hi-Speed.
With a USB 2.0 hub, with a USB 2.0 port on a USB 3.0 hub,
and with a USB 2.0 extension cable in place of a hub.

Near instant hangs.

Plugged directly to the USB 3.0 port, it works fine.
-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: MAX3421E Linux driver?

2014-01-30 Thread simon
 Great find, I'm impressed!  Yeah, probably not much hope of contacting
 the author (it's from 2008), but it sounds like the basics were pretty
 straight-forward.  Good to know.

Yep, probably lost to the sands of time I found a linked copy of that
blog and clicked through for the year but nothing else helpful.

Best help is (as already mentioned I think):
https://bitbucket.org/bufferlabs/usb_host_shield_2.0

Good luck, and publish if you get anything working,
Simon

--
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: MAX3421E Linux driver?

2014-01-30 Thread simon
 We have a need to graft a USB Host controller onto a SPI bus (never
 mind the wisdom of doing that, we have little
 choice in this particular instance).

 The Cypress CY7C67300 would fit the bill and has a Linux driver, but
 is probably too complex and definitely
 too expensive for our needs.

 The Maxim MAX3421E is the other option, but it has no Linux driver.

Since the device sounds very interesting, I started googling around to
find this comment:
http://hary040314.blog.163.com/blog/static/77162442200822125922706/
--
Today ,I add max3421e to linux kernel 2.6.13 as usb host controller on
platform s3c2440 successful.

For now,I can mount mobile hard disk to sysfs of linux,and list all the
files and folders on the mobile hard disk.

But the speed is slow,and even worse,I can't exchange files larger than 8M
through it.I thought ,the road to success finally ahead is long.
--

The rest of the page is Chinese(?), but it might suggest that someone got
something working at one time
Simon

--
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: FW: xhci ASMedia lockups - a theory and a patch

2014-01-30 Thread renevant
Hello,

This will be the last post from me on this, at least for a while, out of my 
depth with this stuff.

I have Linus' tree up and running with patches recommended by Sarah.

1. Disable scatter-gather for the ax88179_178a driver when it's under an
   xHCI host.

2. Revert the following commits:
   f2d9b991c549 xhci: Set scatter-gather limit to avoid failed block writes.
   d6c9ea9069af xhci: Avoid infinite loop when sg urb requires too many trbs
   35773dac5f86 usb: xhci: Link TRB must not occur within a USB payload burst



So far so good with the ax88179_178a connected via the VIA 800 based pcie 
card. Fingers crossed in saying thanks for coming up with a workable solution, 
Seeing 117.8MB/s transfers for sending and receiving with mtu 4088.



Regards,

Will Trives
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/2] usb: host: xhci-plat: Fix build warning when !CONFIG_PM

2014-01-30 Thread Fabio Estevam
From: Fabio Estevam fabio.este...@freescale.com

Building keystone_defconfig leads to the following build warnings:

drivers/usb/host/xhci-plat.c:203:12: warning: 'xhci_plat_suspend' defined but 
not used [-Wunused-function]
drivers/usb/host/xhci-plat.c:211:12: warning: 'xhci_plat_resume' defined but 
not used [-Wunused-function]

Cc: Olof Johansson o...@lixom.net
Reported-by: Olof Johansson o...@lixom.net
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
Build-tested only

Changes since v1:
- none

 drivers/usb/host/xhci-plat.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 9c2e583..104e48f 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -199,7 +199,7 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
 }
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int xhci_plat_suspend(struct device *dev)
 {
struct usb_hcd  *hcd = dev_get_drvdata(dev);
@@ -215,14 +215,9 @@ static int xhci_plat_resume(struct device *dev)
 
return xhci_resume(xhci, 0);
 }
+#endif /* CONFIG_PM_SLEEP */
 
-static const struct dev_pm_ops xhci_plat_pm_ops = {
-   SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume)
-};
-#define DEV_PM_OPS (xhci_plat_pm_ops)
-#else
-#define DEV_PM_OPS NULL
-#endif /* CONFIG_PM */
+static SIMPLE_DEV_PM_OPS(xhci_plat_pm_ops, xhci_plat_suspend, 
xhci_plat_resume);
 
 #ifdef CONFIG_OF
 static const struct of_device_id usb_xhci_of_match[] = {
@@ -237,7 +232,7 @@ static struct platform_driver usb_xhci_driver = {
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
-   .pm = DEV_PM_OPS,
+   .pm = xhci_plat_pm_ops,
.of_match_table = of_match_ptr(usb_xhci_of_match),
},
 };
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/2] usb: host: xhci-plat: Use module_platform_driver()

2014-01-30 Thread Fabio Estevam
From: Fabio Estevam fabio.este...@freescale.com

Using module_platform_driver() can make the code simpler.

Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
Build-tested only

Changes since v1:
- Mark the patch as 1/2

 drivers/usb/host/xhci-plat.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 8abda5c..488e3d4 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -242,13 +242,4 @@ static struct platform_driver usb_xhci_driver = {
},
 };
 MODULE_ALIAS(platform:xhci-hcd);
-
-int xhci_register_plat(void)
-{
-   return platform_driver_register(usb_xhci_driver);
-}
-
-void xhci_unregister_plat(void)
-{
-   platform_driver_unregister(usb_xhci_driver);
-}
+module_platform_driver(usb_xhci_driver);
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html