Re: FW: xhci ASMedia lockups - a theory and a patch
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
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
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
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
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
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
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
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
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
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()
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()
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,
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
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
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
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
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
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()
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()
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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?
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?
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?
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?
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
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?
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?
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
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?
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?
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
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
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()
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