Re: JMS56x not working reliably with uas driver
On Tue, 2016-12-27 at 21:19 -0500, Alan Stern wrote: > On Tue, 27 Dec 2016, George Cherian wrote: > > True. I am afraid that there necessarily is a window for resetting a > > disconnected device. But the check you proposed is better. > > however, I'd like to encapsulate that together with a test for > > logical disconnect. Uas is unlikely to be the only driver that has > > this issue. > > True enough. We could have a usb_device_is_disconnected() inline > helper for this purpose. There's no need to try to distinguish > between physical and logical disconnects, as far as I can see. There is no such need. There is a need for both checks though. > However, as George points out, the "Transfer event" error has nothing > to do with disconnection. It was triggered by the device's bogus > response to a REPORT OPCODES command, or something of the sort. We > should be able to handle bogus responses without logging ERROR > messages, because such messages indicate a bug in a kernel driver. Indeed. We have two independent issues. We should have two independent fixes. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe 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 01/37] usb: host: xhci: dynamically allocate devs array
Instead of always defaulting to a 256-entry array, we can dynamically allocate devs based on what HW tells us it supports. Note that we can't, yet, purge MAX_HC_SLOTS completely because of struct xhci_device_context_array reliance on it. Signed-off-by: Felipe Balbi --- Changes since v1: - move allocation/deallocation of array to xhci_mem_init/cleanup() --- drivers/usb/host/xhci-hub.c | 2 +- drivers/usb/host/xhci-mem.c | 11 +-- drivers/usb/host/xhci-ring.c | 2 +- drivers/usb/host/xhci.h | 2 +- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 0ef16900efed..ba5ffeef305d 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -356,7 +356,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci, enum usb_device_speed speed; slot_id = 0; - for (i = 0; i < MAX_HC_SLOTS; i++) { + for (i = 0; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) { if (!xhci->devs[i]) continue; speed = xhci->devs[i]->udev->speed; diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 321de2e0161b..e7edc79851f2 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1828,7 +1828,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) } } - for (i = 1; i < MAX_HC_SLOTS; ++i) + for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); ++i) xhci_free_virt_device(xhci, i); dma_pool_destroy(xhci->segment_pool); @@ -1867,6 +1867,8 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) } } + kfree(xhci->devs); + no_bw: xhci->cmd_ring_reserved_trbs = 0; xhci->num_usb2_ports = 0; @@ -2378,6 +2380,11 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) "// Setting Max device slots reg = 0x%x.", val); writel(val, &xhci->op_regs->config_reg); + xhci->devs = kcalloc(HCS_MAX_SLOTS(xhci->hcs_params1) + 1, + sizeof(struct xhci_virt_device *), GFP_KERNEL); + if (!xhci->devs) + goto fail; + /* * Section 5.4.8 - doorbell array must be * "physically contiguous and 64-byte (cache line) aligned". @@ -2535,7 +2542,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) * something other than the default (~1ms minimum between interrupts). * See section 5.5.1.2. */ - for (i = 0; i < MAX_HC_SLOTS; ++i) + for (i = 0; i <= HCS_MAX_SLOTS(xhci->hcs_params1); ++i) xhci->devs[i] = NULL; for (i = 0; i < USB_MAXCHILDREN; ++i) { xhci->bus_state[0].resume_done[i] = 0; diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index bdf6b13d9b67..76402b44f832 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -895,7 +895,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) * doesn't touch the memory. */ } - for (i = 0; i < MAX_HC_SLOTS; i++) { + for (i = 0; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) { if (!xhci->devs[i]) continue; for (j = 0; j < 31; j++) diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 8ccc11a974b8..f6e5bc3d819b 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1583,7 +1583,7 @@ struct xhci_hcd { /* For USB 3.0 LPM enable/disable. */ struct xhci_command *lpm_command; /* Internal mirror of the HW's dcbaa */ - struct xhci_virt_device *devs[MAX_HC_SLOTS]; + struct xhci_virt_device **devs; /* For keeping track of bandwidth domains per roothub. */ struct xhci_root_port_bw_info *rh_bw; -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/37] usb: host: xhci: big cleanup series
Hi Mathias, Here's a resend of my previous series. This time I've added a few more patches which I have been working on and also cherry-pick three old patches which have been pending since May this year. Note, in particular, the work done for XHCI tracers which will help us a lot on debugging sessions. Note, also, that we're starting to save a lot of memory with XHCI after these patches. Only allocating memory for slots which we support, instead of default to static 256 entry arrays. There's still a lot of work to be done, but I suppose this is a step in the right direction. I've been running these patches for several weeks now using SKL most of the time. Tested with our tree of devices we have in the office. Mass Storage, mouse, keyboard, cameras, all still working fine. While testing, I've found a regression on v4.10-rc1 (vanilla), btw. One of the devices in our tree dies after 5 consecutive USB Resets are driven on the bus. Not sure if this is a problem with the device, but on a different laptop with i5-4200U (Broadwell?) running v4.4 the same test works fine. I'm building v4.10-rc1 to see if the devices fails with Broadwell too. Anyway, please consider taking these patches to v4.11 merge window. cheers Felipe Balbi (37): usb: host: xhci: dynamically allocate devs array usb: host: xhci: handle COMP_STOP from SETUP phase too usb: host: xhci: change pre-increments to post-increments usb: host: xhci: print HCIVERSION on debug usb: host: xhci: rename completion codes to match spec usb: host: xhci: WARN on unexpected COMP_SUCCESS usb: host: xhci: WARN() if we interrupt without event_ring usb: host: xhci: simplify irq handler return usb: host: xhci: clear only STS_EINT usb: host: xhci: remove unneded semicolon usb: host: xhci: use slightly better list helpers usb: host: xhci: major rewrite of process_ctrl_td() usb: host: xhci: major rewrite of process_bulk_intr_td() usb: host: xhci: cleanup finish_td() usb: host: xhci: reorder variable definitions usb: host: xhci: introduce xhci_td_cleanup() usb: host: xhci: remove bogus __releases()/__acquires() annotation usb: host: xhci: check for a valid ring when unmapping bounce buffer usb: host: xhci: unconditionally call xhci_unmap_td_bounce_buffer() usb: host: xhci: don't try to mask critical errors usb: host: xhci: remove debug argument from trb_in_td() usb: host: xhci: remove unnecessary list_for_each_entry_safe() usb: host: xhci: introduce helper to convert a single TRB in no-op usb: host: xhci: use trb_to_noop() from xhci_handle_stopped_cmd_ring() usb: host: xhci: simplify implementation of trb_in_td() usb: host: xhci: avoid code duplication usb: host: xhci: convert to list_for_each_entry_safe() usb: host: xhci: mark trb_in_td() static usb: host: xhci: combine event TRB completion debugging messages usb: host: xhci: make a generic TRB tracer usb: host: xhci: add urb_enqueue/dequeue/giveback tracers usb: host: xhci: switch to running avg trb length usb: host: xhci: convert several if() to a single switch statement usb: host: xhci: refactor xhci_hub_control() usb: host: xhci: remove newline from tracer usb: host: xhci: add xhci_virt_device tracer usb: host: xhci: dynamically allocate dcbaa drivers/usb/host/xhci-dbg.c | 22 +- drivers/usb/host/xhci-ext-caps.h | 2 +- drivers/usb/host/xhci-hub.c | 659 + drivers/usb/host/xhci-mem.c | 268 +-- drivers/usb/host/xhci-ring.c | 695 +++ drivers/usb/host/xhci-trace.h| 184 +-- drivers/usb/host/xhci.c | 68 ++-- drivers/usb/host/xhci.h | 509 +++- 8 files changed, 1421 insertions(+), 986 deletions(-) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/37] usb: host: xhci: clear only STS_EINT
Many other bits in USBSTS register are "clear-by-writing-1". Let's make sure that we clear *only* STS_EINT and not any of the other bits as they might be needed later. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index b4ec80d18078..116b4a0dadbb 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2630,8 +2630,7 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd) * so we can receive interrupts from other MSI-X interrupters. * Write 1 to clear the interrupt status. */ - status |= STS_EINT; - writel(status, &xhci->op_regs->status); + writel(STS_EINT, &xhci->op_regs->status); /* FIXME when MSI-X is supported and there are multiple vectors */ /* Clear the MSI-X event interrupt status */ -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/37] usb: host: xhci: handle COMP_STOP from SETUP phase too
Stop Endpoint command can come at any point and we have no control of that. We should make sure to handle COMP_STOP on SETUP phase as well, otherwise urb->actual_lenght might be set to negative values in some occasions such as below: urb->length = 4; build_control_transfer_td_for(urb, ep); stop_endpoint(ep); COMP_STOP: [...] urb->actual_length = urb->length - trb->length; trb->length is 8 for SETUP stage (8 control request bytes), so actual_length would be set to -4 in this case. While doing that, also make sure to use TRB_TYPE field of the actual TRB instead of matching pointers to figure out in which stage of the control transfer we got our completion event. Signed-off-by: Felipe Balbi --- changes since v1 o handle multi-trb Data Stage --- drivers/usb/host/xhci-ring.c | 34 ++ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 76402b44f832..02506c44380d 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1939,8 +1939,9 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_ep_ctx *ep_ctx; u32 trb_comp_code; u32 remaining, requested; - bool on_data_stage; + u32 trb_type; + trb_type = TRB_FIELD_TO_TYPE(le32_to_cpu(ep_trb->generic.field[3])); slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags)); xdev = xhci->devs[slot_id]; ep_index = TRB_TO_EP_ID(le32_to_cpu(event->flags)) - 1; @@ -1950,14 +1951,11 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, requested = td->urb->transfer_buffer_length; remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); - /* not setup (dequeue), or status stage means we are at data stage */ - on_data_stage = (ep_trb != ep_ring->dequeue && ep_trb != td->last_trb); - switch (trb_comp_code) { case COMP_SUCCESS: - if (ep_trb != td->last_trb) { + if (trb_type != TRB_STATUS) { xhci_warn(xhci, "WARN: Success on ctrl %s TRB without IOC set?\n", - on_data_stage ? "data" : "setup"); + (trb_type == TRB_DATA) ? "data" : "setup"); *status = -ESHUTDOWN; break; } @@ -1967,15 +1965,25 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, *status = 0; break; case COMP_STOP_SHORT: - if (on_data_stage) + if (trb_type == TRB_DATA || + trb_type == TRB_NORMAL) td->urb->actual_length = remaining; else xhci_warn(xhci, "WARN: Stopped Short Packet on ctrl setup or status TRB\n"); goto finish_td; case COMP_STOP: - if (on_data_stage) + switch (trb_type) { + case TRB_SETUP: + td->urb->actual_length = 0; + goto finish_td; + case TRB_DATA: + case TRB_NORMAL: td->urb->actual_length = requested - remaining; - goto finish_td; + goto finish_td; + default: + xhci_warn(xhci, "WARN: unexpected TRB Type %d\n", trb_type); + goto finish_td; + } case COMP_STOP_INVAL: goto finish_td; default: @@ -1987,7 +1995,8 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, /* else fall through */ case COMP_STALL: /* Did we transfer part of the data (middle) phase? */ - if (on_data_stage) + if (trb_type == TRB_DATA || + trb_type == TRB_NORMAL) td->urb->actual_length = requested - remaining; else if (!td->urb_length_set) td->urb->actual_length = 0; @@ -1995,14 +2004,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, } /* stopped at setup stage, no data transferred */ - if (ep_trb == ep_ring->dequeue) + if (trb_type == TRB_SETUP) goto finish_td; /* * if on data stage then update the actual_length of the URB and flag it * as set, so it won't be overwritten in the event for the last TRB. */ - if (on_data_stage) { + if (trb_type == TRB_DATA || + trb_type == TRB_NORMAL) { td->urb_length_set = true; td->urb->actual_length = requested - remaining; xhci_dbg(xhci, "Waiting for status stage event\n"); -- 2.11.0 -- To unsubscribe from this list:
[PATCH 06/37] usb: host: xhci: WARN on unexpected COMP_SUCCESS
COMP_SUCCESS should only be asserted on a *true* sucessful transfer. Any other cases are bogus and we should aim to catch them. One easy way to get bug reports is to trigger a WARN_ONCE() on such cases. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 28 +++- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 17b878b53b46..772e07e2ef16 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1934,6 +1934,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, { struct xhci_virt_device *xdev; struct xhci_ring *ep_ring; + struct device *dev; unsigned int slot_id; int ep_index; struct xhci_ep_ctx *ep_ctx; @@ -1950,15 +1951,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len)); requested = td->urb->transfer_buffer_length; remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); + dev = xhci_to_hcd(xhci)->self.controller; switch (trb_comp_code) { case COMP_SUCCESS: - if (trb_type != TRB_STATUS) { - xhci_warn(xhci, "WARN: Success on ctrl %s TRB without IOC set?\n", - (trb_type == TRB_DATA) ? "data" : "setup"); - *status = -ESHUTDOWN; - break; - } + dev_WARN_ONCE(dev, trb_type != TRB_STATUS, + "ep%d%s: unexpected success! TRB Type %d\n", + usb_endpoint_num(&td->urb->ep->desc), + usb_endpoint_dir_in(&td->urb->ep->desc) ? + "in" : "out", trb_type); *status = 0; break; case COMP_SHORT_PACKET: @@ -2150,6 +2151,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_virt_ep *ep, int *status) { struct xhci_ring *ep_ring; + struct device *dev; u32 trb_comp_code; u32 remaining, requested, ep_trb_len; @@ -2158,16 +2160,16 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td, remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); ep_trb_len = TRB_LEN(le32_to_cpu(ep_trb->generic.field[2])); requested = td->urb->transfer_buffer_length; + dev = xhci_to_hcd(xhci)->self.controller; switch (trb_comp_code) { case COMP_SUCCESS: - /* handle success with untransferred data as short packet */ - if (ep_trb != td->last_trb || remaining) { - xhci_warn(xhci, "WARN Successful completion on short TX\n"); - xhci_dbg(xhci, "ep %#x - asked for %d bytes, %d bytes untransferred\n", -td->urb->ep->desc.bEndpointAddress, -requested, remaining); - } + dev_WARN_ONCE(dev, (ep_trb != td->last_trb) || remaining, + "ep%d%s: unexpected success! TRB %p/%p size %d/%d\n", + usb_endpoint_num(&td->urb->ep->desc), + usb_endpoint_dir_in(&td->urb->ep->desc) ? + "in" : "out", ep_trb, td->last_trb, remaining, + requested); *status = 0; break; case COMP_SHORT_PACKET: -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/37] usb: host: xhci: simplify irq handler return
Instead of having several return points, let's use a local variable and a single place to return. This makes the code slightly easier to read. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 31 --- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0fd990603f77..b4ec80d18078 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2601,27 +2601,28 @@ static int xhci_handle_event(struct xhci_hcd *xhci) irqreturn_t xhci_irq(struct usb_hcd *hcd) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); - u32 status; - u64 temp_64; union xhci_trb *event_ring_deq; + irqreturn_t ret = IRQ_NONE; dma_addr_t deq; + u64 temp_64; + u32 status; spin_lock(&xhci->lock); /* Check if the xHC generated the interrupt, or the irq is shared */ status = readl(&xhci->op_regs->status); - if (status == 0x) - goto hw_died; - - if (!(status & STS_EINT)) { - spin_unlock(&xhci->lock); - return IRQ_NONE; + if (status == 0x) { + ret = IRQ_HANDLED; + goto out; } + + if (!(status & STS_EINT)) + goto out; + if (status & STS_FATAL) { xhci_warn(xhci, "WARNING: Host System Error\n"); xhci_halt(xhci); -hw_died: - spin_unlock(&xhci->lock); - return IRQ_HANDLED; + ret = IRQ_HANDLED; + goto out; } /* @@ -2652,9 +2653,8 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd) temp_64 = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue); xhci_write_64(xhci, temp_64 | ERST_EHB, &xhci->ir_set->erst_dequeue); - spin_unlock(&xhci->lock); - - return IRQ_HANDLED; + ret = IRQ_HANDLED; + goto out; } event_ring_deq = xhci->event_ring->dequeue; @@ -2680,9 +2680,10 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd) temp_64 |= ERST_EHB; xhci_write_64(xhci, temp_64, &xhci->ir_set->erst_dequeue); +out: spin_unlock(&xhci->lock); - return IRQ_HANDLED; + return ret; } irqreturn_t xhci_msi_irq(int irq, void *hcd) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/37] usb: host: xhci: remove unneded semicolon
it does no good, let's remove it. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ext-caps.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h index e0244fb3903d..28deea584884 100644 --- a/drivers/usb/host/xhci-ext-caps.h +++ b/drivers/usb/host/xhci-ext-caps.h @@ -117,7 +117,7 @@ static inline int xhci_find_next_ext_cap(void __iomem *base, u32 start, int id) offset = XHCI_HCC_EXT_CAPS(val) << 2; if (!offset) return 0; - }; + } do { val = readl(base + offset); if (val == ~0) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/37] usb: host: xhci: WARN() if we interrupt without event_ring
If we get an interrupt while we don't have an allocated event_ring, that means we're enabling IRQs far too early. Instead of just printing an error message, let's make sure to add a scary-looking Stack Trace so people report these things and we fix them. Eventually, when we're happy that this doesn't happen, we should just remove this altogether. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 772e07e2ef16..0fd990603f77 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2526,14 +2526,16 @@ static int handle_tx_event(struct xhci_hcd *xhci, static int xhci_handle_event(struct xhci_hcd *xhci) { union xhci_trb *event; + struct device *dev; int update_ptrs = 1; int ret; + dev = xhci_to_hcd(xhci)->self.controller; + /* Event ring hasn't been allocated yet. */ - if (!xhci->event_ring || !xhci->event_ring->dequeue) { - xhci_err(xhci, "ERROR event ring not ready\n"); + if (dev_WARN_ONCE(dev, !xhci->event_ring || !xhci->event_ring->dequeue, + "event ring not ready\n")) return -ENOMEM; - } event = xhci->event_ring->dequeue; /* Does the HC or OS own the TRB? */ -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/37] usb: host: xhci: print HCIVERSION on debug
When calling xhci_dbg_regs() we actually _do_ want to know XHCI's version. This might help figure out why certain problems only happen in some cases. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-dbg.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c index a3b67f33d4d8..363d125300ea 100644 --- a/drivers/usb/host/xhci-dbg.c +++ b/drivers/usb/host/xhci-dbg.c @@ -37,10 +37,8 @@ void xhci_dbg_regs(struct xhci_hcd *xhci) &xhci->cap_regs->hc_capbase, temp); xhci_dbg(xhci, "// CAPLENGTH: 0x%x\n", (unsigned int) HC_LENGTH(temp)); -#if 0 xhci_dbg(xhci, "// HCIVERSION: 0x%x\n", (unsigned int) HC_VERSION(temp)); -#endif xhci_dbg(xhci, "// xHCI operational registers at %p:\n", xhci->op_regs); -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/37] usb: host: xhci: rename completion codes to match spec
Cleanup only. This patch is a mechaninal rename to make sure our macros for TRB completion codes match what the specification uses to refer to such errors. The idea behind this is that it makes it far easier to grep the specification and match it with implementation. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-hub.c | 3 +- drivers/usb/host/xhci-ring.c | 126 +-- drivers/usb/host/xhci.c | 48 - drivers/usb/host/xhci.h | 106 +--- 4 files changed, 125 insertions(+), 158 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index ba5ffeef305d..dd6de282e48f 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -418,7 +418,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) /* Wait for last stop endpoint command to finish */ wait_for_completion(cmd->completion); - if (cmd->status == COMP_CMD_ABORT || cmd->status == COMP_CMD_STOP) { + if (cmd->status == COMP_COMMAND_ABORTED || + cmd->status == COMP_STOPPED) { xhci_warn(xhci, "Timeout while waiting for stop endpoint command\n"); ret = -ETIME; } diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 02506c44380d..17b878b53b46 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -995,10 +995,10 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, unsigned int slot_state; switch (cmd_comp_code) { - case COMP_TRB_ERR: + case COMP_TRB_ERROR: xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid because of stream ID configuration\n"); break; - case COMP_CTX_STATE: + case COMP_CONTEXT_STATE_ERROR: xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state.\n"); ep_state = GET_EP_CTX_STATE(ep_ctx); slot_state = le32_to_cpu(slot_ctx->dev_state); @@ -1007,7 +1007,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, "Slot state = %u, EP state = %u", slot_state, ep_state); break; - case COMP_EBADSLT: + case COMP_SLOT_NOT_ENABLED_ERROR: xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed because slot %u was not enabled.\n", slot_id); break; @@ -1204,7 +1204,7 @@ void xhci_cleanup_command_queue(struct xhci_hcd *xhci) { struct xhci_command *cur_cmd, *tmp_cmd; list_for_each_entry_safe(cur_cmd, tmp_cmd, &xhci->cmd_list, cmd_list) - xhci_complete_del_and_free_cmd(cur_cmd, COMP_CMD_ABORT); + xhci_complete_del_and_free_cmd(cur_cmd, COMP_COMMAND_ABORTED); } /* @@ -1222,10 +1222,10 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci, list_for_each_entry_safe(i_cmd, tmp_cmd, &xhci->cmd_list, cmd_list) { - if (i_cmd->status != COMP_CMD_ABORT) + if (i_cmd->status != COMP_COMMAND_ABORTED) continue; - i_cmd->status = COMP_CMD_STOP; + i_cmd->status = COMP_STOPPED; xhci_dbg(xhci, "Turn aborted command %p to no-op\n", i_cmd->command_trb); @@ -1270,9 +1270,9 @@ void xhci_handle_command_timeout(unsigned long data) /* mark this command to be cancelled */ spin_lock_irqsave(&xhci->lock, flags); if (xhci->current_cmd) { - if (xhci->current_cmd->status == COMP_CMD_ABORT) + if (xhci->current_cmd->status == COMP_COMMAND_ABORTED) second_timeout = true; - xhci->current_cmd->status = COMP_CMD_ABORT; + xhci->current_cmd->status = COMP_COMMAND_ABORTED; } /* Make sure command ring is running before aborting it */ @@ -1340,7 +1340,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status)); /* If CMD ring stopped we own the trbs between enqueue and dequeue */ - if (cmd_comp_code == COMP_CMD_STOP) { + if (cmd_comp_code == COMP_STOPPED) { xhci_handle_stopped_cmd_ring(xhci, cmd); return; } @@ -1357,9 +1357,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, * The command ring is stopped now, but the xHC will issue a Command * Ring Stopped event which will cause us to restart it. */ - if (cmd_comp_code == COMP_CMD_ABORT) { + if (cmd_comp_code == COMP_COMMAND_ABORTE
[PATCH 11/37] usb: host: xhci: use slightly better list helpers
Replace list_entry() with list_first_entry() and list_for_each() with list_for_each_entry(). This makes the code slightly more readable. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 116b4a0dadbb..4464a8ea3b85 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -635,7 +635,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, unsigned int ep_index; struct xhci_ring *ep_ring; struct xhci_virt_ep *ep; - struct list_head *entry; struct xhci_td *cur_td = NULL; struct xhci_td *last_unlinked_td; @@ -652,6 +651,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, memset(&deq_state, 0, sizeof(deq_state)); ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); ep = &xhci->devs[slot_id]->eps[ep_index]; + last_unlinked_td = list_last_entry(&ep->cancelled_td_list, + struct xhci_td, cancelled_td_list); if (list_empty(&ep->cancelled_td_list)) { xhci_stop_watchdog_timer_in_irq(xhci, ep); @@ -665,8 +666,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, * it. We're also in the event handler, so we can't get re-interrupted * if another Stop Endpoint command completes */ - list_for_each(entry, &ep->cancelled_td_list) { - cur_td = list_entry(entry, struct xhci_td, cancelled_td_list); + list_for_each_entry(cur_td, &ep->cancelled_td_list, cancelled_td_list) { xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Removing canceled TD starting at 0x%llx (dma).", (unsigned long long)xhci_trb_virt_to_dma( @@ -708,7 +708,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, */ list_del_init(&cur_td->td_list); } - last_unlinked_td = cur_td; + xhci_stop_watchdog_timer_in_irq(xhci, ep); /* If necessary, queue a Set Transfer Ring Dequeue Pointer command */ @@ -730,7 +730,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, * So stop when we've completed the URB for the last TD we unlinked. */ do { - cur_td = list_entry(ep->cancelled_td_list.next, + cur_td = list_first_entry(&ep->cancelled_td_list, struct xhci_td, cancelled_td_list); list_del_init(&cur_td->cancelled_td_list); @@ -1331,7 +1331,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, return; } - cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list); + cmd = list_first_entry(&xhci->cmd_list, struct xhci_command, cmd_list); del_timer(&xhci->cmd_timer); @@ -1419,7 +1419,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, /* restart timer if this wasn't the last command */ if (cmd->cmd_list.next != &xhci->cmd_list) { - xhci->current_cmd = list_entry(cmd->cmd_list.next, + xhci->current_cmd = list_first_entry(&cmd->cmd_list, struct xhci_command, cmd_list); mod_timer(&xhci->cmd_timer, jiffies + XHCI_CMD_DEFAULT_TIMEOUT); } @@ -2415,7 +2415,7 @@ static int handle_tx_event(struct xhci_hcd *xhci, goto cleanup; } - td = list_entry(ep_ring->td_list.next, struct xhci_td, td_list); + td = list_first_entry(&ep_ring->td_list, struct xhci_td, td_list); if (ep->skip) td_num--; -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/37] usb: host: xhci: change pre-increments to post-increments
This is a cleanup patch only, no functional changes. The idea is just to make sure for loops look the same all over the driver. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-dbg.c | 20 ++-- drivers/usb/host/xhci-mem.c | 10 +- drivers/usb/host/xhci.c | 14 +++--- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c index 74c42f722678..a3b67f33d4d8 100644 --- a/drivers/usb/host/xhci-dbg.c +++ b/drivers/usb/host/xhci-dbg.c @@ -177,7 +177,7 @@ static void xhci_print_ports(struct xhci_hcd *xhci) ports = HCS_MAX_PORTS(xhci->hcs_params1); addr = &xhci->op_regs->port_status_base; for (i = 0; i < ports; i++) { - for (j = 0; j < NUM_PORT_REGS; ++j) { + for (j = 0; j < NUM_PORT_REGS; j++) { xhci_dbg(xhci, "%p port %s reg = 0x%x\n", addr, names[j], (unsigned int) readl(addr)); @@ -240,7 +240,7 @@ void xhci_print_run_regs(struct xhci_hcd *xhci) xhci_dbg(xhci, " %p: Microframe index = 0x%x\n", &xhci->run_regs->microframe_index, (unsigned int) temp); - for (i = 0; i < 7; ++i) { + for (i = 0; i < 7; i++) { temp = readl(&xhci->run_regs->rsvd[i]); if (temp != XHCI_INIT_VALUE) xhci_dbg(xhci, " WARN: %p: Rsvd[%i] = 0x%x\n", @@ -259,7 +259,7 @@ void xhci_print_registers(struct xhci_hcd *xhci) void xhci_print_trb_offsets(struct xhci_hcd *xhci, union xhci_trb *trb) { int i; - for (i = 0; i < 4; ++i) + for (i = 0; i < 4; i++) xhci_dbg(xhci, "Offset 0x%x = 0x%x\n", i*4, trb->generic.field[i]); } @@ -332,7 +332,7 @@ void xhci_debug_segment(struct xhci_hcd *xhci, struct xhci_segment *seg) u64 addr = seg->dma; union xhci_trb *trb = seg->trbs; - for (i = 0; i < TRBS_PER_SEGMENT; ++i) { + for (i = 0; i < TRBS_PER_SEGMENT; i++) { trb = &seg->trbs[i]; xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n", addr, lower_32_bits(le64_to_cpu(trb->link.segment_ptr)), @@ -413,7 +413,7 @@ void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst *erst) int i; struct xhci_erst_entry *entry; - for (i = 0; i < erst->num_entries; ++i) { + for (i = 0; i < erst->num_entries; i++) { entry = &erst->entries[i]; xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n", addr, @@ -440,7 +440,7 @@ void xhci_dbg_cmd_ptrs(struct xhci_hcd *xhci) static void dbg_rsvd64(struct xhci_hcd *xhci, u64 *ctx, dma_addr_t dma) { int i; - for (i = 0; i < 4; ++i) { + for (i = 0; i < 4; i++) { xhci_dbg(xhci, "@%p (virt) @%08llx " "(dma) %#08llx - rsvd64[%d]\n", &ctx[4 + i], (unsigned long long)dma, @@ -496,7 +496,7 @@ static void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx * &slot_ctx->dev_state, (unsigned long long)dma, slot_ctx->dev_state); dma += field_size; - for (i = 0; i < 4; ++i) { + for (i = 0; i < 4; i++) { xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd[%d]\n", &slot_ctx->reserved[i], (unsigned long long)dma, slot_ctx->reserved[i], i); @@ -519,7 +519,7 @@ static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci, if (last_ep < 31) last_ep_ctx = last_ep + 1; - for (i = 0; i < last_ep_ctx; ++i) { + for (i = 0; i < last_ep_ctx; i++) { unsigned int epaddr = xhci_get_endpoint_address(i); struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, ctx, i); dma_addr_t dma = ctx->dma + @@ -544,7 +544,7 @@ static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci, &ep_ctx->tx_info, (unsigned long long)dma, ep_ctx->tx_info); dma += field_size; - for (j = 0; j < 3; ++j) { + for (j = 0; j < 3; j++) { xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd[%d]\n", &ep_ctx->reserved[j], (unsigned long long)dma, @@ -583,7 +583,7 @@ void xhci_dbg_ctx(struct xhci_hcd *xhci, &ctrl_ctx->add_flags, (unsigned long long)dma, ctrl_ctx->add_flags); dma += field_size; - for (i = 0; i < 6; ++i) { + for (i = 0; i < 6; i++) { xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd2[%d]\n", &ctrl_
[PATCH 14/37] usb: host: xhci: cleanup finish_td()
Now that finish_td() is the only place calling xhci_requires_manual_halt_cleanup() we can remove that function and use that to cleanup finish_td() by converting it into a switch statement using trb_comp_code. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 59 +++- 1 file changed, 25 insertions(+), 34 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 872b9af70f33..b74c27b8e110 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1773,32 +1773,6 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd *xhci, xhci_ring_cmd_db(xhci); } -/* Check if an error has halted the endpoint ring. The class driver will - * cleanup the halt for a non-default control endpoint if we indicate a stall. - * However, a babble and other errors also halt the endpoint ring, and the class - * driver won't clear the halt in that case, so we need to issue a Set Transfer - * Ring Dequeue Pointer command manually. - */ -static int xhci_requires_manual_halt_cleanup(struct xhci_hcd *xhci, - struct xhci_ep_ctx *ep_ctx, - unsigned int trb_comp_code) -{ - /* TRB completion codes that may require a manual halt cleanup */ - if (trb_comp_code == COMP_USB_TRANSACTION_ERROR || - trb_comp_code == COMP_BABBLE_DETECTED_ERROR || - trb_comp_code == COMP_SPLIT_TRANSACTION_ERROR) - /* The 0.95 spec says a babbling control endpoint -* is not halted. The 0.96 spec says it is. Some HW -* claims to be 0.95 compliant, but it halts the control -* endpoint anyway. Check if a babble halted the -* endpoint. -*/ - if (GET_EP_CTX_STATE(ep_ctx) == EP_STATE_HALTED) - return 1; - - return 0; -} - int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code) { if (trb_comp_code >= 224 && trb_comp_code <= 255) { @@ -1840,19 +1814,35 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, if (skip) goto td_cleanup; - if (trb_comp_code == COMP_STOPPED_LENGTH_INVALID || - trb_comp_code == COMP_STOPPED || - trb_comp_code == COMP_STOPPED_SHORT_PACKET) { + switch (trb_comp_code) { + case COMP_STOPPED_LENGTH_INVALID: + case COMP_STOPPED: + case COMP_STOPPED_SHORT_PACKET: /* The Endpoint Stop Command completion will take care of any * stopped TDs. A stopped TD may be restarted, so don't update * the ring dequeue pointer or take this TD off any lists yet. */ ep->stopped_td = td; return 0; - } - if (trb_comp_code == COMP_STALL_ERROR || - xhci_requires_manual_halt_cleanup(xhci, ep_ctx, - trb_comp_code)) { + case COMP_USB_TRANSACTION_ERROR: + case COMP_BABBLE_DETECTED_ERROR: + case COMP_SPLIT_TRANSACTION_ERROR: + /* Check if an error has halted the endpoint ring. The class +* driver will cleanup the halt for a non-default control +* endpoint if we indicate a stall. However, a babble and other +* errors also halt the endpoint ring, and the class driver +* won't clear the halt in that case, so we need to issue a Set +* Transfer Ring Dequeue Pointer command manually. +* +* Note that the 0.95 spec says a babbling control endpoint is +* not halted. The 0.96 spec says it is. Some HW claims to be +* 0.95 compliant, but it halts the control endpoint anyway. +* Check if a babble halted the endpoint. +*/ + if (GET_EP_CTX_STATE(ep_ctx) != EP_STATE_HALTED) + break; + /* FALLTHROUGH */ + case COMP_STALL_ERROR: /* Issue a reset endpoint command to clear the host side * halt, followed by a set dequeue command to move the * dequeue pointer past the TD. @@ -1860,7 +1850,8 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, */ xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index, ep_ring->stream_id, td, ep_trb); - } else { + break; + default: /* Update ring dequeue pointer */ while (ep_ring->dequeue != td->last_trb) inc_deq(xhci, ep_ring); -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/37] usb: host: xhci: major rewrite of process_ctrl_td()
process_ctrl_td() is too complex and difficult to read. We can clean it up a lot and maintain same functionality as before. Note that while cleaning up, one comment was introduced to explain the special case - it wasn't clear before from code. Much of the changes are related to removal of duplicated work done in both process_ctrl_td() and finish_td(). By removing the duplicated code, we could remove a few local variables and shuffle things around so the implementation is more straight forward. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 89 +--- 1 file changed, 35 insertions(+), 54 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 4464a8ea3b85..1886e004feee 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1932,98 +1932,79 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, union xhci_trb *ep_trb, struct xhci_transfer_event *event, struct xhci_virt_ep *ep, int *status) { - struct xhci_virt_device *xdev; - struct xhci_ring *ep_ring; - struct device *dev; - unsigned int slot_id; - int ep_index; - struct xhci_ep_ctx *ep_ctx; u32 trb_comp_code; - u32 remaining, requested; + u32 remaining; + u32 requested; u32 trb_type; trb_type = TRB_FIELD_TO_TYPE(le32_to_cpu(ep_trb->generic.field[3])); - slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags)); - xdev = xhci->devs[slot_id]; - ep_index = TRB_TO_EP_ID(le32_to_cpu(event->flags)) - 1; - ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer)); - ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index); trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len)); requested = td->urb->transfer_buffer_length; remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); - dev = xhci_to_hcd(xhci)->self.controller; + + if (!td->urb_length_set) + td->urb->actual_length = requested - remaining; switch (trb_comp_code) { case COMP_SUCCESS: - dev_WARN_ONCE(dev, trb_type != TRB_STATUS, - "ep%d%s: unexpected success! TRB Type %d\n", - usb_endpoint_num(&td->urb->ep->desc), - usb_endpoint_dir_in(&td->urb->ep->desc) ? - "in" : "out", trb_type); *status = 0; break; case COMP_SHORT_PACKET: *status = 0; + + if (td->urb->transfer_flags & URB_SHORT_NOT_OK) + *status = -EREMOTEIO; + break; case COMP_STOPPED_SHORT_PACKET: - if (trb_type == TRB_DATA || - trb_type == TRB_NORMAL) - td->urb->actual_length = remaining; - else + if (trb_type == TRB_SETUP || + trb_type == TRB_STATUS) xhci_warn(xhci, "WARN: Stopped Short Packet on ctrl setup or status TRB\n"); + *status = -ESHUTDOWN; + + /* +* NOTICE: according to section 6.4.2 Table 91 of xHCI rev 1.1 +* Specification, if completion code is Stopped - Short Packet, +* Transfer Length field (referred to as 'remaining' here) +* contain the value of EDTLA (see section 4.11.5.2 for +* details), which means that remaining contains actual_length, +* not the amount of bytes remaining to be transferred as usual. +*/ + td->urb->actual_length = remaining; goto finish_td; case COMP_STOPPED: - switch (trb_type) { - case TRB_SETUP: + if (trb_type == TRB_SETUP) td->urb->actual_length = 0; - goto finish_td; - case TRB_DATA: - case TRB_NORMAL: - td->urb->actual_length = requested - remaining; - goto finish_td; - default: - xhci_warn(xhci, "WARN: unexpected TRB Type %d\n", trb_type); - goto finish_td; - } + *status = -ESHUTDOWN; + goto finish_td; case COMP_STOPPED_LENGTH_INVALID: + td->urb->actual_length = 0; + *status = -EINVAL; goto finish_td; - default: - if (!xhci_requires_manual_halt_cleanup(xhci, - ep_ctx, trb_comp_code)) - break; - xhci_dbg(xhci, "TRB error %u, halted endpoint index = %u\n", -trb_comp_code, ep_index); - /* else fall through */ case COMP_STALL_ERROR: -
[PATCH 13/37] usb: host: xhci: major rewrite of process_bulk_intr_td()
Similar to previous patch on process_ctrl_td(), we can also clean up process_bulk_intr_td() and turn it into an easier to read and maintain function. This patch shuffles code around, defines each variable in one line, removes unnecessary debugging messages and an unnecessary goto statement. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 52 +++- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 1886e004feee..872b9af70f33 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2132,58 +2132,50 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_virt_ep *ep, int *status) { struct xhci_ring *ep_ring; - struct device *dev; u32 trb_comp_code; - u32 remaining, requested, ep_trb_len; + u32 ep_trb_len; + u32 remaining; + u32 requested; ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer)); trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len)); remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); ep_trb_len = TRB_LEN(le32_to_cpu(ep_trb->generic.field[2])); requested = td->urb->transfer_buffer_length; - dev = xhci_to_hcd(xhci)->self.controller; + + if (ep_trb == td->last_trb) + td->urb->actual_length = requested - remaining; + else + td->urb->actual_length = + sum_trb_lengths(xhci, ep_ring, ep_trb) + + ep_trb_len - remaining; switch (trb_comp_code) { case COMP_SUCCESS: - dev_WARN_ONCE(dev, (ep_trb != td->last_trb) || remaining, - "ep%d%s: unexpected success! TRB %p/%p size %d/%d\n", - usb_endpoint_num(&td->urb->ep->desc), - usb_endpoint_dir_in(&td->urb->ep->desc) ? - "in" : "out", ep_trb, td->last_trb, remaining, - requested); - *status = 0; - break; case COMP_SHORT_PACKET: - xhci_dbg(xhci, "ep %#x - asked for %d bytes, %d bytes untransferred\n", -td->urb->ep->desc.bEndpointAddress, -requested, remaining); *status = 0; break; case COMP_STOPPED_SHORT_PACKET: + /* +* NOTICE: according to section 6.4.2 Table 91 of xHCI rev 1.1 +* Specification, if completion code is Stopped - Short Packet, +* Transfer Length field (referred to as 'remaining' here) +* contain the value of EDTLA (see section 4.11.5.2 for +* details), which means that remaining contains actual_length, +* not the amount of bytes remaining to be transferred as usual. +*/ td->urb->actual_length = remaining; - goto finish_td; + *status = -ESHUTDOWN; + break; case COMP_STOPPED_LENGTH_INVALID: - /* stopped on ep trb with invalid length, exclude it */ - ep_trb_len = 0; - remaining = 0; + td->urb->actual_length = 0; + *status = -ESHUTDOWN; break; default: /* do nothing */ break; } - if (ep_trb == td->last_trb) - td->urb->actual_length = requested - remaining; - else - td->urb->actual_length = - sum_trb_lengths(xhci, ep_ring, ep_trb) + - ep_trb_len - remaining; -finish_td: - if (remaining > requested) { - xhci_warn(xhci, "bad transfer trb length %d in event trb\n", - remaining); - td->urb->actual_length = 0; - } return finish_td(xhci, td, ep_trb, event, ep, status, false); } -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/37] usb: host: xhci: reorder variable definitions
no functional changes. Simple cleanup to make sure variables are ordered in a 'reverse christmas tree' fashion. While at that, also remove an obsolete comment which doesn't apply anymore. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index b74c27b8e110..5b5fcedca075 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1787,22 +1787,18 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code) return 0; } -/* - * Finish the td processing, remove the td from td list; - * Return 1 if the urb can be given back. - */ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, union xhci_trb *ep_trb, struct xhci_transfer_event *event, struct xhci_virt_ep *ep, int *status, bool skip) { struct xhci_virt_device *xdev; - struct xhci_ring *ep_ring; - unsigned int slot_id; - int ep_index; - struct urb *urb = NULL; struct xhci_ep_ctx *ep_ctx; + struct xhci_ring *ep_ring; struct urb_priv *urb_priv; + struct urb *urb = NULL; + unsigned int slot_id; u32 trb_comp_code; + int ep_index; slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags)); xdev = xhci->devs[slot_id]; -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/37] usb: host: xhci: introduce xhci_td_cleanup()
By extracting xhci_td_cleanup() from finish_td(), code before clearer and easier to follow. There are no functional changes with this patch. It's merely a cleanup. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 92 1 file changed, 50 insertions(+), 42 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 5b5fcedca075..19647b8aa8a1 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1787,6 +1787,55 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code) return 0; } +static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td, + struct xhci_ring *ep_ring, int *status) +{ + struct urb_priv *urb_priv; + struct urb *urb = NULL; + + /* Clean up the endpoint's TD list */ + urb = td->urb; + urb_priv = urb->hcpriv; + + /* if a bounce buffer was used to align this td then unmap it */ + if (td->bounce_seg) + xhci_unmap_td_bounce_buffer(xhci, ep_ring, td); + + /* Do one last check of the actual transfer length. +* If the host controller said we transferred more data than the buffer +* length, urb->actual_length will be a very big number (since it's +* unsigned). Play it safe and say we didn't transfer anything. +*/ + if (urb->actual_length > urb->transfer_buffer_length) { + xhci_warn(xhci, "URB req %u and actual %u transfer length mismatch\n", + urb->transfer_buffer_length, urb->actual_length); + urb->actual_length = 0; + *status = 0; + } + list_del_init(&td->td_list); + /* Was this TD slated to be cancelled but completed anyway? */ + if (!list_empty(&td->cancelled_td_list)) + list_del_init(&td->cancelled_td_list); + + inc_td_cnt(urb); + /* Giveback the urb when all the tds are completed */ + if (last_td_in_urb(td)) { + if ((urb->actual_length != urb->transfer_buffer_length && +(urb->transfer_flags & URB_SHORT_NOT_OK)) || + (*status != 0 && !usb_endpoint_xfer_isoc(&urb->ep->desc))) + xhci_dbg(xhci, "Giveback URB %p, len = %d, expected = %d, status = %d\n", +urb, urb->actual_length, +urb->transfer_buffer_length, *status); + + /* set isoc urb status to 0 just as EHCI, UHCI, and OHCI */ + if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS) + *status = 0; + xhci_giveback_urb_in_irq(xhci, td, *status); + } + + return 0; +} + static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, union xhci_trb *ep_trb, struct xhci_transfer_event *event, struct xhci_virt_ep *ep, int *status, bool skip) @@ -1794,8 +1843,6 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_virt_device *xdev; struct xhci_ep_ctx *ep_ctx; struct xhci_ring *ep_ring; - struct urb_priv *urb_priv; - struct urb *urb = NULL; unsigned int slot_id; u32 trb_comp_code; int ep_index; @@ -1855,46 +1902,7 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, } td_cleanup: - /* Clean up the endpoint's TD list */ - urb = td->urb; - urb_priv = urb->hcpriv; - - /* if a bounce buffer was used to align this td then unmap it */ - if (td->bounce_seg) - xhci_unmap_td_bounce_buffer(xhci, ep_ring, td); - - /* Do one last check of the actual transfer length. -* If the host controller said we transferred more data than the buffer -* length, urb->actual_length will be a very big number (since it's -* unsigned). Play it safe and say we didn't transfer anything. -*/ - if (urb->actual_length > urb->transfer_buffer_length) { - xhci_warn(xhci, "URB req %u and actual %u transfer length mismatch\n", - urb->transfer_buffer_length, urb->actual_length); - urb->actual_length = 0; - *status = 0; - } - list_del_init(&td->td_list); - /* Was this TD slated to be cancelled but completed anyway? */ - if (!list_empty(&td->cancelled_td_list)) - list_del_init(&td->cancelled_td_list); - - inc_td_cnt(urb); - /* Giveback the urb when all the tds are completed */ - if (last_td_in_urb(td)) { - if ((urb->actual_length != urb->transfer_buffer_length && -(urb->transfer_flags & URB_SHORT_NOT_OK)) || - (*status != 0 && !usb_endpoint_xfer_isoc(&urb->ep->desc))) - xhci_dbg(xhci, "Giveback URB %p, len = %d, expected = %d, status = %d\n", -urb, urb->act
[PATCH 19/37] usb: host: xhci: unconditionally call xhci_unmap_td_bounce_buffer()
xhci_unmap_td_bounce_buffer() already checks for a valid td->bounce_seg and bails out early if that's invalid. There's no need to check for this twice. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 5a52e1f0050d..75a0b73289ef 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -739,8 +739,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, * just overwrite it (because the URB has been unlinked). */ ep_ring = xhci_urb_to_transfer_ring(xhci, cur_td->urb); - if (ep_ring && cur_td->bounce_seg) - xhci_unmap_td_bounce_buffer(xhci, ep_ring, cur_td); + xhci_unmap_td_bounce_buffer(xhci, ep_ring, cur_td); inc_td_cnt(cur_td->urb); if (last_td_in_urb(cur_td)) xhci_giveback_urb_in_irq(xhci, cur_td, 0); @@ -766,8 +765,7 @@ static void xhci_kill_ring_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring) if (!list_empty(&cur_td->cancelled_td_list)) list_del_init(&cur_td->cancelled_td_list); - if (cur_td->bounce_seg) - xhci_unmap_td_bounce_buffer(xhci, ring, cur_td); + xhci_unmap_td_bounce_buffer(xhci, ring, cur_td); inc_td_cnt(cur_td->urb); if (last_td_in_urb(cur_td)) @@ -1798,8 +1796,7 @@ static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td, urb_priv = urb->hcpriv; /* if a bounce buffer was used to align this td then unmap it */ - if (td->bounce_seg) - xhci_unmap_td_bounce_buffer(xhci, ep_ring, td); + xhci_unmap_td_bounce_buffer(xhci, ep_ring, td); /* Do one last check of the actual transfer length. * If the host controller said we transferred more data than the buffer -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 18/37] usb: host: xhci: check for a valid ring when unmapping bounce buffer
This way we can remove checks for valid ring from call sites of xhci_unmap_td_bounce_buffer() Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index a6a93636a68d..5a52e1f0050d 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -601,7 +601,7 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci, struct xhci_segment *seg = td->bounce_seg; struct urb *urb = td->urb; - if (!seg || !urb) + if (!ring || !seg || !urb) return; if (usb_urb_dir_out(urb)) { -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/37] usb: host: xhci: remove bogus __releases()/__acquires() annotation
handle_tx_event() is not releasing xhci->lock nor reacquiring it, remove the bogus annotation. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 19647b8aa8a1..a6a93636a68d 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2181,8 +2181,6 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td, */ static int handle_tx_event(struct xhci_hcd *xhci, struct xhci_transfer_event *event) - __releases(&xhci->lock) - __acquires(&xhci->lock) { struct xhci_virt_device *xdev; struct xhci_virt_ep *ep; -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 36/37] usb: host: xhci: add xhci_virt_device tracer
Let's start tracing at least part of an xhci_virt_device lifetime. We might want to extend this tracepoint class later, but for now it already exposes quite a bit of valuable information. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-hub.c | 2 ++ drivers/usb/host/xhci-mem.c | 7 ++ drivers/usb/host/xhci-trace.h | 57 +++ drivers/usb/host/xhci.c | 1 + 4 files changed, 67 insertions(+) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index b99f06f4c421..6fdea9e5e3a5 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -389,6 +389,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) if (!virt_dev) return -ENODEV; + trace_xhci_stop_device(virt_dev); + cmd = xhci_alloc_command(xhci, false, true, GFP_NOIO); if (!cmd) { xhci_dbg(xhci, "Couldn't allocate command structure.\n"); diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index b12631ef160b..2b7c3504a95a 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -936,6 +936,9 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id) return; dev = xhci->devs[slot_id]; + + trace_xhci_free_virt_device(dev); + xhci->dcbaa->dev_context_ptrs[slot_id] = 0; if (!dev) return; @@ -1041,6 +1044,8 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id, &xhci->dcbaa->dev_context_ptrs[slot_id], le64_to_cpu(xhci->dcbaa->dev_context_ptrs[slot_id])); + trace_xhci_alloc_virt_device(dev); + return 1; fail: xhci_free_virt_device(xhci, slot_id); @@ -1215,6 +1220,8 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud ep0_ctx->deq = cpu_to_le64(dev->eps[0].ring->first_seg->dma | dev->eps[0].ring->cycle_state); + trace_xhci_setup_addressable_virt_device(dev); + /* Steps 7 and 8 were done in xhci_alloc_virt_device() */ return 0; diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 08bed2f07e50..1e85c893247d 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -158,6 +158,63 @@ DEFINE_EVENT(xhci_log_trb, xhci_queue_trb, TP_ARGS(ring, trb) ); +DECLARE_EVENT_CLASS(xhci_log_virt_dev, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev), + TP_STRUCT__entry( + __field(void *, vdev) + __field(unsigned long long, out_ctx) + __field(unsigned long long, in_ctx) + __field(int, devnum) + __field(int, state) + __field(int, speed) + __field(u8, portnum) + __field(u8, level) + __field(int, slot_id) + ), + TP_fast_assign( + __entry->vdev = vdev; + __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma; + __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma; + __entry->devnum = vdev->udev->devnum; + __entry->state = vdev->udev->state; + __entry->speed = vdev->udev->speed; + __entry->portnum = vdev->udev->portnum; + __entry->level = vdev->udev->level; + __entry->slot_id = vdev->udev->slot_id; + ), + TP_printk("vdev %p ctx %llx | %llx num %d state %d speed %d port %d level %d slot %d", + __entry->vdev, __entry->in_ctx, __entry->out_ctx, __entry->devnum, + __entry->state, __entry->speed, __entry->portnum, __entry->level, + __entry->slot_id + ) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_alloc_virt_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_free_virt_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_setup_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_setup_addressable_virt_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + +DEFINE_EVENT(xhci_log_virt_dev, xhci_stop_device, + TP_PROTO(struct xhci_virt_device *vdev), + TP_ARGS(vdev) +); + DECLARE_EVENT_CLASS(xhci_log_urb, TP_PROTO(struct urb *urb), TP_ARGS(urb), diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index c0f3670e5a51..e5f095b276fc 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3860,6 +3860,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, le32_to_cpu(slot_ctx->dev_info) >> 27); spin_lock_irqsave(&xhci->lock, flags); + trace_xhci_setup_de
[PATCH 34/37] usb: host: xhci: refactor xhci_hub_control()
In order to make xhci_hub_control() easier to read, let's break it into smaller functions. This will aid maintainability and readability of the code. There are no functional changes here, just shuffling code around. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-hub.c | 652 1 file changed, 363 insertions(+), 289 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index dd6de282e48f..b99f06f4c421 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -873,17 +873,97 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, return status; } -int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, +static int xhci_ctrl_get_hub_status(struct usb_hcd *hcd, u16 wValue, u16 wIndex, char *buf, u16 wLength) { + /* No power source, over-current reported per port */ + memset(buf, 0, 4); + return 0; +} + +static int xhci_ctrl_get_hub_descriptor(struct usb_hcd *hcd, u16 wValue, + u16 wIndex, char *buf, u16 wLength) +{ + struct xhci_hcd *xhci = hcd_to_xhci(hcd); + + /* Check to make sure userspace is asking for the USB 3.0 hub +* descriptor for the USB 3.0 roothub. If not, we stall the +* endpoint, like external hubs do. +*/ + if (hcd->speed >= HCD_USB3 && + (wLength < USB_DT_SS_HUB_SIZE || + wValue != (USB_DT_SS_HUB << 8))) { + xhci_dbg(xhci, "Wrong hub descriptor type for " + "USB 3.0 roothub.\n"); + return -EPIPE; + } + xhci_hub_descriptor(hcd, xhci, + (struct usb_hub_descriptor *) buf); + + return 0; +} + +static int xhci_ctrl_get_port_status(struct usb_hcd *hcd, u16 wValue, + u16 wIndex, char *buf, u16 wLength, unsigned long flags) +{ struct xhci_hcd *xhci = hcd_to_xhci(hcd); + struct xhci_bus_state *bus_state; + + u32 status; + u32 temp; + int max_ports; - unsigned long flags; - u32 temp, status; - int retval = 0; + __le32 __iomem **port_array; - int slot_id; + + max_ports = xhci_get_ports(hcd, &port_array); + bus_state = &xhci->bus_state[hcd_index(hcd)]; + + if (!wIndex || wIndex > max_ports) + return -EINVAL; + + wIndex--; + temp = readl(port_array[wIndex]); + if (temp == 0x) + return -ENODEV; + status = xhci_get_port_status(hcd, bus_state, port_array, + wIndex, temp, flags); + if (status == 0x) + return -ENODEV; + + xhci_dbg(xhci, "get port status, actual port %d status = 0x%x\n", + wIndex, temp); + xhci_dbg(xhci, "Get port status returned 0x%x\n", status); + + put_unaligned(cpu_to_le32(status), (__le32 *) buf); + /* if USB 3.1 extended port status return additional 4 bytes */ + if (wValue == 0x02) { + u32 port_li; + + if (hcd->speed < HCD_USB31 || wLength != 8) { + xhci_err(xhci, "get ext port status invalid parameter\n"); + return -EINVAL; + } + port_li = readl(port_array[wIndex] + PORTLI); + status = xhci_get_ext_port_status(temp, port_li); + put_unaligned_le32(cpu_to_le32(status), &buf[4]); + } + + return 0; +} + +static int xhci_ctrl_set_port_feature(struct usb_hcd *hcd, u16 wValue, + u16 wIndex, char *buf, u16 wLength, unsigned long flags) +{ + struct xhci_hcd *xhci = hcd_to_xhci(hcd); struct xhci_bus_state *bus_state; + + int max_ports; + int slot_id; + u32 temp; + + __le32 __iomem **port_array; + u16 link_state = 0; u16 wake_mask = 0; u16 timeout = 0; @@ -891,328 +971,322 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, max_ports = xhci_get_ports(hcd, &port_array); bus_state = &xhci->bus_state[hcd_index(hcd)]; - spin_lock_irqsave(&xhci->lock, flags); - switch (typeReq) { - case GetHubStatus: - /* No power source, over-current reported per port */ - memset(buf, 0, 4); - break; - case GetHubDescriptor: - /* Check to make sure userspace is asking for the USB 3.0 hub -* descriptor for the USB 3.0 roothub. If not, we stall the -* endpoint, like external hubs do. + if (wValue == USB_PORT_FEAT_LINK_STATE) + link_state = (wIndex & 0xff00) >> 3; + if (wValue == USB_PORT_FEAT_REMOTE_WAKE_MASK) + wake_mask = wIndex & 0xff00; + /* The MSB of wIndex is the U1/U2 timeout */ + timeout = (wIndex & 0xff00) >> 8; + wIndex &= 0xff; + if (!wIndex || wIndex > max_port
[PATCH 37/37] usb: host: xhci: dynamically allocate dcbaa
Instead of *always* allocating an array of 256 pointers, we can dynamically allocate it based on number of maximum slots $this XHCI implementation supports. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-mem.c | 45 ++--- drivers/usb/host/xhci.h | 4 +--- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 2b7c3504a95a..916eb2e30029 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1815,6 +1815,7 @@ void xhci_free_command(struct xhci_hcd *xhci, void xhci_mem_cleanup(struct xhci_hcd *xhci) { struct device *dev = xhci_to_hcd(xhci)->self.controller; + int max_slots; int size; int i, j, num_ports; @@ -1851,7 +1852,8 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) } } - for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) + max_slots = HCS_MAX_SLOTS(xhci->hcs_params1) + 1; + for (i = 1; i < max_slots; i++) xhci_free_virt_device(xhci, i); dma_pool_destroy(xhci->segment_pool); @@ -1872,11 +1874,13 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed medium stream array pool"); - if (xhci->dcbaa) - dma_free_coherent(dev, sizeof(*xhci->dcbaa), - xhci->dcbaa, xhci->dcbaa->dma); - xhci->dcbaa = NULL; + if (xhci->dcbaa->dev_context_ptrs) + dma_free_coherent(dev, sizeof(__le64) * max_slots, + xhci->dcbaa->dev_context_ptrs, + xhci->dcbaa->dma); + kfree(xhci->dcbaa); + xhci->dcbaa = NULL; scratchpad_free(xhci); if (!xhci->rh_bw) @@ -2205,6 +2209,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) u64 val_64; struct xhci_segment *seg; u32 page_size, temp; + int max_slots; int i; INIT_LIST_HEAD(&xhci->cmd_list); @@ -2236,17 +2241,22 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) * Program the Number of Device Slots Enabled field in the CONFIG * register with the max value of slots the HC can handle. */ - val = HCS_MAX_SLOTS(readl(&xhci->cap_regs->hcs_params1)); + max_slots = HCS_MAX_SLOTS(readl(&xhci->cap_regs->hcs_params1)); xhci_dbg_trace(xhci, trace_xhci_dbg_init, - "// xHC can handle at most %d device slots.", val); + "// xHC can handle at most %d device slots.", max_slots); + val2 = readl(&xhci->op_regs->config_reg); - val |= (val2 & ~HCS_SLOTS_MASK); + val = max_slots | (val2 & ~HCS_SLOTS_MASK); + + /* XHCI needs offset 0 to be unused */ + max_slots += 1; + xhci_dbg_trace(xhci, trace_xhci_dbg_init, "// Setting Max device slots reg = 0x%x.", val); writel(val, &xhci->op_regs->config_reg); - xhci->devs = kcalloc(HCS_MAX_SLOTS(xhci->hcs_params1) + 1, - sizeof(struct xhci_virt_device *), GFP_KERNEL); + xhci->devs = kcalloc(max_slots, sizeof(struct xhci_virt_device *), + GFP_KERNEL); if (!xhci->devs) goto fail; @@ -2254,16 +2264,21 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) * Section 5.4.8 - doorbell array must be * "physically contiguous and 64-byte (cache line) aligned". */ - xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa), &dma, - flags); + xhci->dcbaa = kzalloc(sizeof(*xhci->dcbaa), GFP_KERNEL); if (!xhci->dcbaa) goto fail; - memset(xhci->dcbaa, 0, sizeof *(xhci->dcbaa)); - xhci->dcbaa->dma = dma; + + xhci->dcbaa->dev_context_ptrs = dma_alloc_coherent(dev, sizeof(__le64) * + max_slots, &xhci->dcbaa->dma, flags); + if (!xhci->dcbaa->dev_context_ptrs) + goto fail; + + memset(xhci->dcbaa->dev_context_ptrs, 0, sizeof (__le64) * + max_slots); xhci_dbg_trace(xhci, trace_xhci_dbg_init, "// Device context base array address = 0x%llx (DMA), %p (virt)", (unsigned long long)xhci->dcbaa->dma, xhci->dcbaa); - xhci_write_64(xhci, dma, &xhci->op_regs->dcbaa_ptr); + xhci_write_64(xhci, xhci->dcbaa->dma, &xhci->op_regs->dcbaa_ptr); /* * Initialize the ring segment pool. The ring must be a contiguous diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 57731f6dba6c..37fd2b27f5b7 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -37,8 +37,6 @@ /* xHCI PCI Configuration Registers */ #define XHCI_SBRN_OFFSET (0x60) -/* Max number of USB devices for any host controller - limit in secti
[PATCH 28/37] usb: host: xhci: mark trb_in_td() static
trb_in_td() has a single user in the same file where it is defined, there's no need to expose it anywhere else. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 2 +- drivers/usb/host/xhci.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index c9a3907362d3..dc436d0c900f 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1717,7 +1717,7 @@ static void handle_port_status(struct xhci_hcd *xhci, * TRB in this TD, this function returns that TRB's segment. Otherwise it * returns 0. */ -struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, +static struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, struct xhci_td *td, dma_addr_t suspect_dma) { struct xhci_segment *cur_seg = td->start_seg; diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 8fc77bea8580..8e7da82d5033 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1856,8 +1856,6 @@ void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev); /* xHCI ring, segment, TRB, and TD functions */ dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg, union xhci_trb *trb); -struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, struct xhci_td *td, - dma_addr_t suspect_dma); int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code); void xhci_ring_cmd_db(struct xhci_hcd *xhci); int xhci_queue_slot_control(struct xhci_hcd *xhci, struct xhci_command *cmd, -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 29/37] usb: host: xhci: combine event TRB completion debugging messages
If we just provide a helper to convert completion code to string, we can combine all debugging messages into a single print. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 28 drivers/usb/host/xhci.h | 80 2 files changed, 86 insertions(+), 22 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index dc436d0c900f..393c64f8acef 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2230,6 +2230,10 @@ static int handle_tx_event(struct xhci_hcd *xhci, ep_trb_dma = le64_to_cpu(event->buffer); trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len)); + + xhci_dbg(xhci, "Event TRB Status '%s' (%d)\n", + xhci_trb_comp_code_string(trb_comp_code), trb_comp_code); + /* Look for common error cases */ switch (trb_comp_code) { /* Skip codes that require special handling depending on @@ -2244,51 +2248,35 @@ static int handle_tx_event(struct xhci_hcd *xhci, xhci_warn_ratelimited(xhci, "WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk?\n"); case COMP_SHORT_PACKET: - break; case COMP_STOPPED: - xhci_dbg(xhci, "Stopped on Transfer TRB\n"); - break; case COMP_STOPPED_LENGTH_INVALID: - xhci_dbg(xhci, "Stopped on No-op or Link TRB\n"); - break; case COMP_STOPPED_SHORT_PACKET: - xhci_dbg(xhci, "Stopped with short packet transfer detected\n"); + case COMP_BANDWIDTH_OVERRUN_ERROR: + case COMP_ISOCH_BUFFER_OVERRUN: break; case COMP_STALL_ERROR: - xhci_dbg(xhci, "Stalled endpoint\n"); ep->ep_state |= EP_HALTED; status = -EPIPE; break; case COMP_TRB_ERROR: - xhci_warn(xhci, "WARN: TRB error on endpoint\n"); status = -EILSEQ; break; case COMP_SPLIT_TRANSACTION_ERROR: case COMP_USB_TRANSACTION_ERROR: - xhci_dbg(xhci, "Transfer error on endpoint\n"); status = -EPROTO; break; case COMP_BABBLE_DETECTED_ERROR: - xhci_dbg(xhci, "Babble error on endpoint\n"); status = -EOVERFLOW; break; case COMP_DATA_BUFFER_ERROR: - xhci_warn(xhci, "WARN: HC couldn't access mem fast enough\n"); status = -ENOSR; break; - case COMP_BANDWIDTH_OVERRUN_ERROR: - xhci_warn(xhci, "WARN: bandwidth overrun event on endpoint\n"); - break; - case COMP_ISOCH_BUFFER_OVERRUN: - xhci_warn(xhci, "WARN: buffer overrun event on endpoint\n"); - break; case COMP_RING_UNDERRUN: /* * When the Isoch ring is empty, the xHC will generate * a Ring Overrun Event for IN Isoch endpoint or Ring * Underrun Event for OUT Isoch endpoint. */ - xhci_dbg(xhci, "underrun event on endpoint\n"); if (!list_empty(&ep_ring->td_list)) xhci_dbg(xhci, "Underrun Event for slot %d ep %d " "still with TDs queued?\n", @@ -2296,7 +2284,6 @@ static int handle_tx_event(struct xhci_hcd *xhci, ep_index); goto cleanup; case COMP_RING_OVERRUN: - xhci_dbg(xhci, "overrun event on endpoint\n"); if (!list_empty(&ep_ring->td_list)) xhci_dbg(xhci, "Overrun Event for slot %d ep %d " "still with TDs queued?\n", @@ -2304,7 +2291,6 @@ static int handle_tx_event(struct xhci_hcd *xhci, ep_index); goto cleanup; case COMP_INCOMPATIBLE_DEVICE_ERROR: - xhci_warn(xhci, "WARN: detect an incompatible device"); status = -EPROTO; break; case COMP_MISSED_SERVICE_ERROR: @@ -2315,11 +2301,9 @@ static int handle_tx_event(struct xhci_hcd *xhci, * short transfer when process the ep_ring next time. */ ep->skip = true; - xhci_dbg(xhci, "Miss service interval error, set skip flag\n"); goto cleanup; case COMP_NO_PING_RESPONSE_ERROR: ep->skip = true; - xhci_dbg(xhci, "No Ping response error, Skip one Isoc TD\n"); goto cleanup; default: if (xhci_is_vendor_info_code(xhci, trb_comp_code)) { diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 8e7da82d5033..c7e95a6e39a7 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci
[PATCH 33/37] usb: host: xhci: convert several if() to a single switch statement
when getting endpoint type, a switch statement looks better than a series of if () branches. There are no functional changes with this patch, cleanup only. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-mem.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 9e1ccfadf128..b12631ef160b 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1380,14 +1380,16 @@ static u32 xhci_get_endpoint_type(struct usb_host_endpoint *ep) in = usb_endpoint_dir_in(&ep->desc); - if (usb_endpoint_xfer_control(&ep->desc)) + switch (usb_endpoint_type(&ep->desc)) { + case USB_ENDPOINT_XFER_CONTROL: return CTRL_EP; - if (usb_endpoint_xfer_bulk(&ep->desc)) + case USB_ENDPOINT_XFER_BULK: return in ? BULK_IN_EP : BULK_OUT_EP; - if (usb_endpoint_xfer_isoc(&ep->desc)) + case USB_ENDPOINT_XFER_ISOC: return in ? ISOC_IN_EP : ISOC_OUT_EP; - if (usb_endpoint_xfer_int(&ep->desc)) + case USB_ENDPOINT_XFER_INT: return in ? INT_IN_EP : INT_OUT_EP; + } return 0; } -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 35/37] usb: host: xhci: remove newline from tracer
If we add that newline, the output will like like the following: kworker/2:1-42[002] 169.811435: xhci_address_ctx: ctx_64=0, ctx_type=2, ctx_dma=@153fbd000, ctx_va=@880153fbd000 We would rather have that in a single line. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 4bad0d6d2c8a..08bed2f07e50 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -103,7 +103,7 @@ DECLARE_EVENT_CLASS(xhci_log_ctx, ((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) * ((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1)); ), - TP_printk("\nctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p", + TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p", __entry->ctx_64, __entry->ctx_type, (unsigned long long) __entry->ctx_dma, __entry->ctx_va ) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 20/37] usb: host: xhci: don't try to mask critical errors
Instead of fixing actual_length and setting status to 0, let's make sure the error is propagated and we print a big scary stack trace so errors are reported and we have a chance of fixing them. While at that, also remove unnecessary initialization of urb variable. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 75a0b73289ef..a05010521fd0 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1789,7 +1789,10 @@ static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_ring *ep_ring, int *status) { struct urb_priv *urb_priv; - struct urb *urb = NULL; + struct device *dev; + struct urb *urb; + + dev = xhci_to_hcd(xhci)->self.controller; /* Clean up the endpoint's TD list */ urb = td->urb; @@ -1798,17 +1801,10 @@ static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td, /* if a bounce buffer was used to align this td then unmap it */ xhci_unmap_td_bounce_buffer(xhci, ep_ring, td); - /* Do one last check of the actual transfer length. -* If the host controller said we transferred more data than the buffer -* length, urb->actual_length will be a very big number (since it's -* unsigned). Play it safe and say we didn't transfer anything. -*/ - if (urb->actual_length > urb->transfer_buffer_length) { - xhci_warn(xhci, "URB req %u and actual %u transfer length mismatch\n", - urb->transfer_buffer_length, urb->actual_length); - urb->actual_length = 0; - *status = 0; - } + dev_WARN_ONCE(dev, urb->actual_length > urb->transfer_buffer_length, + "URB transfer length mismatch. Requested %u got %u\n", + urb->transfer_buffer_length, urb->actual_length); + list_del_init(&td->td_list); /* Was this TD slated to be cancelled but completed anyway? */ if (!list_empty(&td->cancelled_td_list)) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 24/37] usb: host: xhci: use trb_to_noop() from xhci_handle_stopped_cmd_ring()
instead of open coding how to convert a TRB to no-op, let's use our newly introduced helper. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 33 +++-- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index f369d97f663d..0b8f728d6e77 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -542,6 +542,27 @@ static void trb_to_noop(union xhci_trb *trb) trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); trb->generic.field[3] |= cpu_to_le32(TRB_TYPE(TRB_TR_NOOP)); break; + case TRB_ENABLE_SLOT: + case TRB_DISABLE_SLOT: + case TRB_ADDR_DEV: + case TRB_CONFIG_EP: + case TRB_EVAL_CONTEXT: + case TRB_RESET_EP: + case TRB_STOP_RING: + case TRB_SET_DEQ: + case TRB_RESET_DEV: + case TRB_FORCE_EVENT: + case TRB_NEG_BANDWIDTH: + case TRB_SET_LT: + case TRB_GET_BW: + case TRB_FORCE_HEADER: + trb->generic.field[0] = 0; + trb->generic.field[1] = 0; + trb->generic.field[2] = 0; + /* Preserve only the cycle bit of this TRB */ + trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); + trb->generic.field[3] = cpu_to_le32(TRB_TYPE(TRB_CMD_NOOP)); + break; default: /* nothing */ break; @@ -1229,7 +1250,6 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci, struct xhci_command *cur_cmd) { struct xhci_command *cmd; - u32 cycle_state; /* Turn all aborted commands in list to no-ops, then restart */ list_for_each_entry(cmd, &xhci->cmd_list, @@ -1242,15 +1262,8 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci, xhci_dbg(xhci, "Turn aborted command %p to no-op\n", cmd->command_trb); - /* get cycle state from the original cmd trb */ - cycle_state = le32_to_cpu( - cmd->command_trb->generic.field[3]) & TRB_CYCLE; - /* modify the command trb to no-op command */ - cmd->command_trb->generic.field[0] = 0; - cmd->command_trb->generic.field[1] = 0; - cmd->command_trb->generic.field[2] = 0; - cmd->command_trb->generic.field[3] = cpu_to_le32( - TRB_TYPE(TRB_CMD_NOOP) | cycle_state); + + trb_to_noop(cmd->command_trb); /* * caller waiting for completion is called when command -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 22/37] usb: host: xhci: remove unnecessary list_for_each_entry_safe()
the _safe() version of list iterators are supposed to be used when list_entry is going to be removed from the list while iterating. Since xhci_handle_stopped_cmd_ring() is not removing anything from the list, just converting commands into No-Op TRBs, we don't need to use the safe version. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 4e5d66594fd1..edb5fcc5a12b 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1213,28 +1213,28 @@ void xhci_cleanup_command_queue(struct xhci_hcd *xhci) static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci, struct xhci_command *cur_cmd) { - struct xhci_command *i_cmd, *tmp_cmd; + struct xhci_command *cmd; u32 cycle_state; /* Turn all aborted commands in list to no-ops, then restart */ - list_for_each_entry_safe(i_cmd, tmp_cmd, &xhci->cmd_list, + list_for_each_entry(cmd, &xhci->cmd_list, cmd_list) { - if (i_cmd->status != COMP_COMMAND_ABORTED) + if (cmd->status != COMP_COMMAND_ABORTED) continue; - i_cmd->status = COMP_STOPPED; + cmd->status = COMP_STOPPED; xhci_dbg(xhci, "Turn aborted command %p to no-op\n", -i_cmd->command_trb); +cmd->command_trb); /* get cycle state from the original cmd trb */ cycle_state = le32_to_cpu( - i_cmd->command_trb->generic.field[3]) & TRB_CYCLE; + cmd->command_trb->generic.field[3]) & TRB_CYCLE; /* modify the command trb to no-op command */ - i_cmd->command_trb->generic.field[0] = 0; - i_cmd->command_trb->generic.field[1] = 0; - i_cmd->command_trb->generic.field[2] = 0; - i_cmd->command_trb->generic.field[3] = cpu_to_le32( + cmd->command_trb->generic.field[0] = 0; + cmd->command_trb->generic.field[1] = 0; + cmd->command_trb->generic.field[2] = 0; + cmd->command_trb->generic.field[3] = cpu_to_le32( TRB_TYPE(TRB_CMD_NOOP) | cycle_state); /* -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 32/37] usb: host: xhci: switch to running avg trb length
It's unlikely that we will ever know the avg so instead of assuming it'll be something really large, we will calculate the avg as we go as mentioned in XHCI specification section 4.14.1.1. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-mem.c | 34 -- drivers/usb/host/xhci-ring.c | 20 drivers/usb/host/xhci.h | 3 ++- 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index f7cfbab5ba4c..9e1ccfadf128 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1451,18 +1451,34 @@ int xhci_endpoint_init(struct xhci_hcd *xhci, ring_type = usb_endpoint_type(&ep->desc); - /* -* Get values to fill the endpoint context, mostly from ep descriptor. -* The average TRB buffer lengt for bulk endpoints is unclear as we -* have no clue on scatter gather list entry size. For Isoc and Int, -* set it to max available. See xHCI 1.1 spec 4.14.1.1 for details. -*/ + /* Get values to fill the endpoint context, mostly from ep descriptor. */ max_esit_payload = xhci_get_max_esit_payload(udev, ep); interval = xhci_get_endpoint_interval(udev, ep); mult = xhci_get_endpoint_mult(udev, ep); max_packet = usb_endpoint_maxp(&ep->desc); max_burst = xhci_get_endpoint_max_burst(udev, ep); - avg_trb_len = max_esit_payload; + + /* +* We are using a running avg for our endpoint's avg_trb_len. The reason +* is that we have no clue about average transfer sizes for any +* endpoints because the HCD does not know which USB Class is running on +* the other end. +* +* See xHCI 1.1 spec 4.14.1.1 for details about initial avg_trb_len +* setting. +*/ + switch (usb_endpoint_type(&ep->desc)) { + case USB_ENDPOINT_XFER_CONTROL: + avg_trb_len = 8; + break; + case USB_ENDPOINT_XFER_INT: + avg_trb_len = 1024; + break; + case USB_ENDPOINT_XFER_ISOC: + case USB_ENDPOINT_XFER_BULK: + avg_trb_len = 3072; + break; + } /* FIXME dig Mult and streams info out of ep companion desc */ @@ -1472,9 +1488,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci, /* Some devices get this wrong */ if (usb_endpoint_xfer_bulk(&ep->desc) && udev->speed == USB_SPEED_HIGH) max_packet = 512; - /* xHCI 1.0 and 1.1 indicates that ctrl ep avg TRB Length should be 8 */ - if (usb_endpoint_xfer_control(&ep->desc) && xhci->hci_version >= 0x100) - avg_trb_len = 8; + /* xhci 1.1 with LEC support doesn't use mult field, use RsvdZ */ if ((xhci->hci_version > 0x100) && HCC2_LEC(xhci->hcc_params2)) mult = 0; diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 1431e0651e78..f5f999d8a72c 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2752,6 +2752,8 @@ static int prepare_transfer(struct xhci_hcd *xhci, struct xhci_td *td; struct xhci_ring *ep_ring; struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index); + unsigned int avg_trb_len; + unsigned int tx_info; ep_ring = xhci_stream_id_to_ring(xdev, ep_index, stream_id); if (!ep_ring) { @@ -2760,6 +2762,24 @@ static int prepare_transfer(struct xhci_hcd *xhci, return -EINVAL; } + /* +* Here we update avg_trb_len so that, over time, we get a better +* representation of what the actual average length for this endpoint's +* TRBs are going to be. +*/ + if (urb->transfer_buffer_length > 0) { + tx_info = le32_to_cpu(ep_ctx->tx_info); + + avg_trb_len = EP_AVG_TRB_LENGTH(tx_info); + avg_trb_len += urb->transfer_buffer_length; + avg_trb_len /= 2; + + tx_info &= ~EP_AVG_TRB_LENGTH_MASK; + tx_info |= EP_AVG_TRB_LENGTH(avg_trb_len); + + ep_ctx->tx_info = cpu_to_le32(tx_info); + } + ret = prepare_ring(xhci, ep_ring, GET_EP_CTX_STATE(ep_ctx), num_trbs, mem_flags); if (ret) diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index eb72ad4c0d72..57731f6dba6c 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -750,7 +750,8 @@ struct xhci_ep_ctx { #define MAX_PACKET_DECODED(p) (((p) >> 16) & 0x) /* tx_info bitmasks */ -#define EP_AVG_TRB_LENGTH(p) ((p) & 0x) +#define EP_AVG_TRB_LENGTH_MASK 0x +#define EP_AVG_TRB_LENGTH(p) ((p) & EP_AVG_TRB_LENGTH_MASK) #define EP_MAX_ESIT_PAYLOAD_LO(p) (((p) & 0x) << 16) #define EP_MAX_ESIT_PAYLOAD_HI(p) p) >> 16) & 0xff) << 24) #define CTX_TO_MAX_ESI
[PATCH 31/37] usb: host: xhci: add urb_enqueue/dequeue/giveback tracers
These three new tracers will help us tie TRBs into URBs by *also* looking into URB lifetime. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 1 + drivers/usb/host/xhci-trace.h | 70 +++ drivers/usb/host/xhci.c | 5 3 files changed, 76 insertions(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0ee7d358b812..1431e0651e78 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -627,6 +627,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, usb_hcd_unlink_urb_from_ep(hcd, urb); spin_unlock(&xhci->lock); usb_hcd_giveback_urb(hcd, urb, status); + trace_xhci_urb_giveback(urb); spin_lock(&xhci->lock); } diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index d01524b9fb14..4bad0d6d2c8a 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -158,6 +158,76 @@ DEFINE_EVENT(xhci_log_trb, xhci_queue_trb, TP_ARGS(ring, trb) ); +DECLARE_EVENT_CLASS(xhci_log_urb, + TP_PROTO(struct urb *urb), + TP_ARGS(urb), + TP_STRUCT__entry( + __field(void *, urb) + __field(unsigned int, pipe) + __field(unsigned int, stream) + __field(int, status) + __field(unsigned int, flags) + __field(int, num_mapped_sgs) + __field(int, num_sgs) + __field(int, length) + __field(int, actual) + __field(int, epnum) + __field(int, dir_in) + __field(int, type) + ), + TP_fast_assign( + __entry->urb = urb; + __entry->pipe = urb->pipe; + __entry->stream = urb->stream_id; + __entry->status = urb->status; + __entry->flags = urb->transfer_flags; + __entry->num_mapped_sgs = urb->num_mapped_sgs; + __entry->num_sgs = urb->num_sgs; + __entry->length = urb->transfer_buffer_length; + __entry->actual = urb->actual_length; + __entry->epnum = usb_endpoint_num(&urb->ep->desc); + __entry->dir_in = usb_endpoint_dir_in(&urb->ep->desc); + __entry->type = usb_endpoint_type(&urb->ep->desc); + ), + TP_printk("ep%d%s-%s: urb %p pipe %u length %d/%d sgs %d/%d stream %d flags %08x", + __entry->epnum, __entry->dir_in ? "in" : "out", + ({ char *s; + switch (__entry->type) { + case USB_ENDPOINT_XFER_INT: + s = "intr"; + break; + case USB_ENDPOINT_XFER_CONTROL: + s = "control"; + break; + case USB_ENDPOINT_XFER_BULK: + s = "bulk"; + break; + case USB_ENDPOINT_XFER_ISOC: + s = "isoc"; + break; + default: + s = "UNKNOWN"; + } s; }), __entry->urb, __entry->pipe, __entry->actual, + __entry->length, __entry->num_mapped_sgs, + __entry->num_sgs, __entry->stream, __entry->flags + ) +); + +DEFINE_EVENT(xhci_log_urb, xhci_urb_enqueue, + TP_PROTO(struct urb *urb), + TP_ARGS(urb) +); + +DEFINE_EVENT(xhci_log_urb, xhci_urb_giveback, + TP_PROTO(struct urb *urb), + TP_ARGS(urb) +); + +DEFINE_EVENT(xhci_log_urb, xhci_urb_dequeue, + TP_PROTO(struct urb *urb), + TP_ARGS(urb) +); + #endif /* __XHCI_TRACE_H */ /* this part must be outside header guard */ diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 06d294d7e01f..c0f3670e5a51 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1383,6 +1383,8 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) urb_priv->td_cnt = 0; urb->hcpriv = urb_priv; + trace_xhci_urb_enqueue(urb); + if (usb_endpoint_xfer_control(&urb->ep->desc)) { /* Check to see if the max packet size for the default control * endpoint changed during FS device enumeration @@ -1509,6 +1511,9 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) xhci = hcd_to_xhci(hcd); spin_lock_irqsave(&xhci->lock, flags); + + trace_xhci_urb_dequeue(urb); + /* Make sure the URB hasn't completed or been unlinked already */ ret = usb_hcd_check_unlink_urb(hcd, urb, status); if (ret || !urb->hcpriv) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo i
[PATCH 21/37] usb: host: xhci: remove debug argument from trb_in_td()
By just replacing xhci_warn() with xhci_dbg() we can get rid of the extra argument. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-mem.c | 5 ++--- drivers/usb/host/xhci-ring.c | 10 -- drivers/usb/host/xhci.h | 2 +- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 916151dc74ee..38067628d4df 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1907,7 +1907,7 @@ static int xhci_test_trb_in_td(struct xhci_hcd *xhci, start_dma = xhci_trb_virt_to_dma(input_seg, start_trb); end_dma = xhci_trb_virt_to_dma(input_seg, end_trb); - seg = trb_in_td(xhci, input_seg, start_trb, end_trb, input_dma, false); + seg = trb_in_td(xhci, input_seg, start_trb, end_trb, input_dma); if (seg != result_seg) { xhci_warn(xhci, "WARN: %s TRB math test %d failed!\n", test_name, test_number); @@ -1921,8 +1921,7 @@ static int xhci_test_trb_in_td(struct xhci_hcd *xhci, end_trb, end_dma); xhci_warn(xhci, "Expected seg %p, got seg %p\n", result_seg, seg); - trb_in_td(xhci, input_seg, start_trb, end_trb, input_dma, - true); + trb_in_td(xhci, input_seg, start_trb, end_trb, input_dma); return -1; } return 0; diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index a05010521fd0..4e5d66594fd1 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1692,8 +1692,7 @@ struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, struct xhci_segment *start_seg, union xhci_trb *start_trb, union xhci_trb *end_trb, - dma_addr_t suspect_dma, - booldebug) + dma_addr_t suspect_dma) { dma_addr_t start_dma; dma_addr_t end_seg_dma; @@ -1712,8 +1711,7 @@ struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, /* If the end TRB isn't in this segment, this is set to 0 */ end_trb_dma = xhci_trb_virt_to_dma(cur_seg, end_trb); - if (debug) - xhci_warn(xhci, + xhci_dbg(xhci, "Looking for event-dma %016llx trb-start %016llx trb-end %016llx seg-start %016llx seg-end %016llx\n", (unsigned long long)suspect_dma, (unsigned long long)start_dma, @@ -2380,7 +2378,7 @@ static int handle_tx_event(struct xhci_hcd *xhci, /* Is this a TRB in the currently executing TD? */ ep_seg = trb_in_td(xhci, ep_ring->deq_seg, ep_ring->dequeue, - td->last_trb, ep_trb_dma, false); + td->last_trb, ep_trb_dma); /* * Skip the Force Stopped Event. The event_trb(event_dma) of FSE @@ -2415,7 +2413,7 @@ static int handle_tx_event(struct xhci_hcd *xhci, trb_comp_code); trb_in_td(xhci, ep_ring->deq_seg, ep_ring->dequeue, td->last_trb, - ep_trb_dma, true); + ep_trb_dma); return -ESHUTDOWN; } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 395f3dc20db8..3320b9c240a9 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1858,7 +1858,7 @@ void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev); dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg, union xhci_trb *trb); struct xhci_segment *trb_in_td(struct xhci_hcd *xhci, struct xhci_segment *start_seg, union xhci_trb *start_trb, - union xhci_trb *end_trb, dma_addr_t suspect_dma, bool debug); + union xhci_trb *end_trb, dma_addr_t suspect_dma); int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code); void xhci_ring_cmd_db(struct xhci_hcd *xhci); int xhci_queue_slot_control(struct xhci_hcd *xhci, struct xhci_command *cmd, -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 30/37] usb: host: xhci: make a generic TRB tracer
instead of having a tracer that can only trace command completions, let's promote this tracer so it can trace and decode any TRB. With that, it will be easier to extrapolate the lifetime of any TRB which might help debugging certain issues. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 14 +- drivers/usb/host/xhci-trace.h | 55 +--- drivers/usb/host/xhci.h | 311 ++ 3 files changed, 357 insertions(+), 23 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 393c64f8acef..0ee7d358b812 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1346,6 +1346,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, cmd_dma = le64_to_cpu(event->cmd_trb); cmd_trb = xhci->cmd_ring->dequeue; + + trace_xhci_handle_command(xhci->cmd_ring, &cmd_trb->generic); + cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg, cmd_trb); /* @@ -1362,8 +1365,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, del_timer(&xhci->cmd_timer); - trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event); - cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status)); /* If CMD ring stopped we own the trbs between enqueue and dequeue */ @@ -2407,6 +2408,10 @@ static int handle_tx_event(struct xhci_hcd *xhci, ep_trb = &ep_seg->trbs[(ep_trb_dma - ep_seg->dma) / sizeof(*ep_trb)]; + + trace_xhci_handle_transfer(ep_ring, + (struct xhci_generic_trb *) ep_trb); + /* * No-op TRB should not trigger interrupts. * If ep_trb is a no-op TRB, it means the @@ -2475,6 +2480,8 @@ static int xhci_handle_event(struct xhci_hcd *xhci) xhci->event_ring->cycle_state) return 0; + trace_xhci_handle_event(xhci->event_ring, &event->generic); + /* * Barrier between reading the TRB_CYCLE (valid) flag above and any * speculative reads of the event's flags/data below. @@ -2642,6 +2649,9 @@ static void queue_trb(struct xhci_hcd *xhci, struct xhci_ring *ring, trb->field[1] = cpu_to_le32(field2); trb->field[2] = cpu_to_le32(field3); trb->field[3] = cpu_to_le32(field4); + + trace_xhci_queue_trb(ring, trb); + inc_enq(xhci, ring, more_trbs_coming); } diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 59c05653b2ea..d01524b9fb14 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -115,34 +115,47 @@ DEFINE_EVENT(xhci_log_ctx, xhci_address_ctx, TP_ARGS(xhci, ctx, ep_num) ); -DECLARE_EVENT_CLASS(xhci_log_event, - TP_PROTO(void *trb_va, struct xhci_generic_trb *ev), - TP_ARGS(trb_va, ev), +DECLARE_EVENT_CLASS(xhci_log_trb, + TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb), + TP_ARGS(ring, trb), TP_STRUCT__entry( - __field(void *, va) - __field(u64, dma) - __field(u32, status) - __field(u32, flags) - __dynamic_array(u8, trb, sizeof(struct xhci_generic_trb)) + __field(u32, type) + __field(u32, field0) + __field(u32, field1) + __field(u32, field2) + __field(u32, field3) ), TP_fast_assign( - __entry->va = trb_va; - __entry->dma = ((u64)le32_to_cpu(ev->field[1])) << 32 | - le32_to_cpu(ev->field[0]); - __entry->status = le32_to_cpu(ev->field[2]); - __entry->flags = le32_to_cpu(ev->field[3]); - memcpy(__get_dynamic_array(trb), trb_va, - sizeof(struct xhci_generic_trb)); + __entry->type = ring->type; + __entry->field0 = le32_to_cpu(trb->field[0]); + __entry->field1 = le32_to_cpu(trb->field[1]); + __entry->field2 = le32_to_cpu(trb->field[2]); + __entry->field3 = le32_to_cpu(trb->field[3]); ), - TP_printk("\ntrb_dma=@%llx, trb_va=@%p, status=%08x, flags=%08x", - (unsigned long long) __entry->dma, __entry->va, - __entry->status, __entry->flags + TP_printk("%s: %s", xhci_ring_type_string(__entry->type), + xhci_decode_trb(__entry->field0, __entry->field1, + __entry->field2, __entry->field3) ) ); -DEFINE_EVENT(xhci_log_event, xhci_cmd_completion, - TP_PROTO(void *trb_va, struct xhci_generic_trb *ev), - TP_ARGS(trb_va, ev) +DEFINE_EVENT(xhci_log_trb, xhci_handle_event, + TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb), + TP_ARGS(ring, trb)
[PATCH 25/37] usb: host: xhci: simplify implementation of trb_in_td()
trb_in_td() has the task of checking whether a given TRB belongs to a given TD. Currently implementation is far too complex and used wrongly in some places. Let's simplify the implementation to the point that we can assume it to be working always, this means we can remove xhci_test_trb_in_td() and xhci_check_trb_in_td_math() because we can assume those will always work. While at that, also remove two calls for trb_in_td() which existed for the sole purpose of printing out debugging messages!! Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-mem.c | 160 --- drivers/usb/host/xhci-ring.c | 70 +-- drivers/usb/host/xhci.h | 5 +- 3 files changed, 18 insertions(+), 217 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 38067628d4df..f7cfbab5ba4c 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1892,163 +1892,6 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) xhci->bus_state[1].bus_suspended = 0; } -static int xhci_test_trb_in_td(struct xhci_hcd *xhci, - struct xhci_segment *input_seg, - union xhci_trb *start_trb, - union xhci_trb *end_trb, - dma_addr_t input_dma, - struct xhci_segment *result_seg, - char *test_name, int test_number) -{ - unsigned long long start_dma; - unsigned long long end_dma; - struct xhci_segment *seg; - - start_dma = xhci_trb_virt_to_dma(input_seg, start_trb); - end_dma = xhci_trb_virt_to_dma(input_seg, end_trb); - - seg = trb_in_td(xhci, input_seg, start_trb, end_trb, input_dma); - if (seg != result_seg) { - xhci_warn(xhci, "WARN: %s TRB math test %d failed!\n", - test_name, test_number); - xhci_warn(xhci, "Tested TRB math w/ seg %p and " - "input DMA 0x%llx\n", - input_seg, - (unsigned long long) input_dma); - xhci_warn(xhci, "starting TRB %p (0x%llx DMA), " - "ending TRB %p (0x%llx DMA)\n", - start_trb, start_dma, - end_trb, end_dma); - xhci_warn(xhci, "Expected seg %p, got seg %p\n", - result_seg, seg); - trb_in_td(xhci, input_seg, start_trb, end_trb, input_dma); - return -1; - } - return 0; -} - -/* TRB math checks for xhci_trb_in_td(), using the command and event rings. */ -static int xhci_check_trb_in_td_math(struct xhci_hcd *xhci) -{ - struct { - dma_addr_t input_dma; - struct xhci_segment *result_seg; - } simple_test_vector [] = { - /* A zeroed DMA field should fail */ - { 0, NULL }, - /* One TRB before the ring start should fail */ - { xhci->event_ring->first_seg->dma - 16, NULL }, - /* One byte before the ring start should fail */ - { xhci->event_ring->first_seg->dma - 1, NULL }, - /* Starting TRB should succeed */ - { xhci->event_ring->first_seg->dma, xhci->event_ring->first_seg }, - /* Ending TRB should succeed */ - { xhci->event_ring->first_seg->dma + (TRBS_PER_SEGMENT - 1)*16, - xhci->event_ring->first_seg }, - /* One byte after the ring end should fail */ - { xhci->event_ring->first_seg->dma + (TRBS_PER_SEGMENT - 1)*16 + 1, NULL }, - /* One TRB after the ring end should fail */ - { xhci->event_ring->first_seg->dma + (TRBS_PER_SEGMENT)*16, NULL }, - /* An address of all ones should fail */ - { (dma_addr_t) (~0), NULL }, - }; - struct { - struct xhci_segment *input_seg; - union xhci_trb *start_trb; - union xhci_trb *end_trb; - dma_addr_t input_dma; - struct xhci_segment *result_seg; - } complex_test_vector [] = { - /* Test feeding a valid DMA address from a different ring */ - { .input_seg = xhci->event_ring->first_seg, - .start_trb = xhci->event_ring->first_seg->trbs, - .end_trb = &xhci->event_ring->first_seg->trbs[TRBS_PER_SEGMENT - 1], - .input_dma = xhci->cmd_ring->first_seg->dma, - .result_seg = NULL, - }, - /* Test feeding a valid end TRB from a different ring */ - { .input_seg = xhci->event_ring->first_seg, - .start_trb = xhci->event_ring->first_seg->trbs, - .end_trb = &xhci->cmd_ring->first_seg->trbs[TRBS_PER_SEGMENT - 1], -
[PATCH 27/37] usb: host: xhci: convert to list_for_each_entry_safe()
instead of using while(!list_empty()) followed by list_first_entry(), we can actually use list_for_each_entry_safe(). Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 95b5df235f8d..c9a3907362d3 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -793,11 +793,11 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, static void xhci_kill_ring_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring) { struct xhci_td *cur_td; + struct xhci_td *tmp; - while (!list_empty(&ring->td_list)) { - cur_td = list_first_entry(&ring->td_list, - struct xhci_td, td_list); + list_for_each_entry_safe(cur_td, tmp, &ring->td_list, td_list) { list_del_init(&cur_td->td_list); + if (!list_empty(&cur_td->cancelled_td_list)) list_del_init(&cur_td->cancelled_td_list); @@ -813,6 +813,7 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, int slot_id, int ep_index) { struct xhci_td *cur_td; + struct xhci_td *tmp; struct xhci_virt_ep *ep; struct xhci_ring *ring; @@ -838,12 +839,12 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, slot_id, ep_index); xhci_kill_ring_urbs(xhci, ring); } - while (!list_empty(&ep->cancelled_td_list)) { - cur_td = list_first_entry(&ep->cancelled_td_list, - struct xhci_td, cancelled_td_list); - list_del_init(&cur_td->cancelled_td_list); + list_for_each_entry_safe(cur_td, tmp, &ep->cancelled_td_list, + cancelled_td_list) { + list_del_init(&cur_td->cancelled_td_list); inc_td_cnt(cur_td->urb); + if (last_td_in_urb(cur_td)) xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN); } -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 23/37] usb: host: xhci: introduce helper to convert a single TRB in no-op
this will be used in several places to convert a single TRB into No-Op. No functional changes in this patch. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 39 +++ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index edb5fcc5a12b..f369d97f663d 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -523,6 +523,31 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, (unsigned long long) addr); } +static void trb_to_noop(union xhci_trb *trb) +{ + switch (TRB_FIELD_TO_TYPE(cpu_to_le32(trb->generic.field[3]))) { + case TRB_LINK: + /* unchain chained link TRBs */ + trb->link.control &= cpu_to_le32(~TRB_CHAIN); + break; + case TRB_NORMAL: + case TRB_SETUP: + case TRB_DATA: + case TRB_STATUS: + case TRB_ISOC: + trb->generic.field[0] = 0; + trb->generic.field[1] = 0; + trb->generic.field[2] = 0; + /* Preserve only the cycle bit of this TRB */ + trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); + trb->generic.field[3] |= cpu_to_le32(TRB_TYPE(TRB_TR_NOOP)); + break; + default: + /* nothing */ + break; + } +} + /* flip_cycle means flip the cycle bit of all but the first and last TRB. * (The last TRB actually points to the ring enqueue pointer, which is not part * of this TD.) This is used to remove partially enqueued isoc TDs from a ring. @@ -534,18 +559,8 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, union xhci_trb *trb = td->first_trb; while (1) { - if (trb_is_link(trb)) { - /* unchain chained link TRBs */ - trb->link.control &= cpu_to_le32(~TRB_CHAIN); - } else { - trb->generic.field[0] = 0; - trb->generic.field[1] = 0; - trb->generic.field[2] = 0; - /* Preserve only the cycle bit of this TRB */ - trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); - trb->generic.field[3] |= cpu_to_le32( - TRB_TYPE(TRB_TR_NOOP)); - } + trb_to_noop(trb); + /* flip cycle if asked to */ if (flip_cycle && trb != td->first_trb && trb != td->last_trb) trb->generic.field[3] ^= cpu_to_le32(TRB_CYCLE); -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 26/37] usb: host: xhci: avoid code duplication
extract __trb_to_noop() to avoid a minor duplication of code in trb_to_noop(). Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index cce12f993937..95b5df235f8d 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -523,6 +523,16 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, (unsigned long long) addr); } +static void __trb_to_noop(union xhci_trb *trb, u32 noop) +{ + trb->generic.field[0] = 0; + trb->generic.field[1] = 0; + trb->generic.field[2] = 0; + /* Preserve only the cycle bit of this TRB */ + trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); + trb->generic.field[3] |= cpu_to_le32(TRB_TYPE(noop)); +} + static void trb_to_noop(union xhci_trb *trb) { switch (TRB_FIELD_TO_TYPE(cpu_to_le32(trb->generic.field[3]))) { @@ -535,12 +545,7 @@ static void trb_to_noop(union xhci_trb *trb) case TRB_DATA: case TRB_STATUS: case TRB_ISOC: - trb->generic.field[0] = 0; - trb->generic.field[1] = 0; - trb->generic.field[2] = 0; - /* Preserve only the cycle bit of this TRB */ - trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); - trb->generic.field[3] |= cpu_to_le32(TRB_TYPE(TRB_TR_NOOP)); + __trb_to_noop(trb, TRB_TR_NOOP); break; case TRB_ENABLE_SLOT: case TRB_DISABLE_SLOT: @@ -556,12 +561,7 @@ static void trb_to_noop(union xhci_trb *trb) case TRB_SET_LT: case TRB_GET_BW: case TRB_FORCE_HEADER: - trb->generic.field[0] = 0; - trb->generic.field[1] = 0; - trb->generic.field[2] = 0; - /* Preserve only the cycle bit of this TRB */ - trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); - trb->generic.field[3] = cpu_to_le32(TRB_TYPE(TRB_CMD_NOOP)); + __trb_to_noop(trb, TRB_CMD_NOOP); break; default: /* nothing */ -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] dt-bindings: leds: document new usb-ports property
From: Rafał Miłecki Some LEDs can be related to particular USB ports (common case for home routers). This property allows describing such a relation. Signed-off-by: Rafał Miłecki --- This patch is based on top of commit 52e847dc431 ("DT: leds: Improve examples by adding some context") sitting in the linux-leds.git (for-4.11). --- Documentation/devicetree/bindings/leds/common.txt | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt index 24b6560..fcfe661 100644 --- a/Documentation/devicetree/bindings/leds/common.txt +++ b/Documentation/devicetree/bindings/leds/common.txt @@ -49,6 +49,14 @@ Optional properties for child nodes: - panic-indicator : This property specifies that the LED should be used, if at all possible, as a panic indicator. +- usb-ports : List of USB ports related to this LED. Some devices have LEDs that + should be used to indicate USB device activity. This can be + described with this property. + There can be more than one LED like this, e.g. some vendors use + one controller per USB version. It's then common to use different + color LEDs depending on device USB standard (like USB 2.0 vs. + USB 3.0). + Required properties for flash LED child nodes: - flash-max-microamp : Maximum flash LED supply current in microamperes. - flash-max-timeout-us : Maximum timeout in microseconds after which the flash @@ -69,6 +77,11 @@ gpio-leds { linux,default-trigger = "heartbeat"; gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>; }; + + usb { + gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>; + usb-ports = <&ohci_port1>, <&ehci_port1>; + }; }; max77693-led { -- 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: core: read USB ports from DT in the usbport LED trigger driver
From: Rafał Miłecki This adds support for using description of relation between LEDs and USB ports from device tree. If device has a properly described LEDs, trigger will know when to turn them on. Signed-off-by: Rafał Miłecki --- drivers/usb/core/ledtrig-usbport.c | 54 ++ 1 file changed, 54 insertions(+) diff --git a/drivers/usb/core/ledtrig-usbport.c b/drivers/usb/core/ledtrig-usbport.c index 1713248..70ad558 100644 --- a/drivers/usb/core/ledtrig-usbport.c +++ b/drivers/usb/core/ledtrig-usbport.c @@ -11,8 +11,10 @@ #include #include #include +#include #include #include +#include struct usbport_trig_data { struct led_classdev *led_cdev; @@ -123,6 +125,55 @@ static const struct attribute_group ports_group = { * Adding & removing ports ***/ +/** + * usbport_trig_port_observed - Check if port should be observed + */ +static bool usbport_trig_port_observed(struct usbport_trig_data *usbport_data, + struct usb_device *usb_dev, int port1) +{ + struct device *dev = usbport_data->led_cdev->dev; + struct device_node *led_np = dev->of_node; + struct of_phandle_args args; + struct device_node *port_np; + int count, i; + + if (!led_np) + return false; + + /* Get node of port being added */ + port_np = usb_of_get_child_node(usb_dev->dev.of_node, port1); + if (!port_np) + return false; + + /* Amount of ports this LED references */ + count = of_count_phandle_with_args(led_np, "usb-ports", NULL); + if (count < 0) { + dev_warn(dev, "Failed to get USB ports for %s\n", +led_np->full_name); + return false; + } + + /* Check if port is on this LED's list */ + for (i = 0; i < count; i++) { + int err; + + err = of_parse_phandle_with_args(led_np, "usb-ports", NULL, i, +&args); + if (err) { + dev_err(dev, "Failed to get USB port phandle at index %d: %d\n", + i, err); + continue; + } + + of_node_put(args.np); + + if (args.np == port_np) + return true; + } + + return false; +} + static int usbport_trig_add_port(struct usbport_trig_data *usbport_data, struct usb_device *usb_dev, const char *hub_name, int portnum) @@ -141,6 +192,8 @@ static int usbport_trig_add_port(struct usbport_trig_data *usbport_data, port->data = usbport_data; port->hub = usb_dev; port->portnum = portnum; + port->observed = usbport_trig_port_observed(usbport_data, usb_dev, + portnum); len = strlen(hub_name) + 8; port->port_name = kzalloc(len, GFP_KERNEL); @@ -255,6 +308,7 @@ static void usbport_trig_activate(struct led_classdev *led_cdev) if (err) goto err_free; usb_for_each_dev(usbport_data, usbport_trig_add_usb_dev_ports); + usbport_trig_update_count(usbport_data); /* Notifications */ usbport_data->nb.notifier_call = usbport_trig_notify, -- 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[EXAMPLE 3/2] ARM: BCM53573: Specify ports for USB LED for Tenda AC9
From: Rafał Miłecki Signed-off-by: Rafał Miłecki --- This patch was tested & works as expected. It's marked as EXAMPLE just because it should go through ARM tree. --- arch/arm/boot/dts/bcm47189-tenda-ac9.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts index 4403ae8..00bbe48 100644 --- a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts +++ b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts @@ -24,6 +24,7 @@ compatible = "gpio-leds"; usb { + usb-ports = <&ohci_port1>, <&ehci_port1>; label = "bcm53xx:blue:usb"; gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>; linux,default-trigger = "default-off"; -- 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: musb: blackfin: fix unused warnings on suspend/resume
On Mon, 19 Dec 2016 14:56:00 -0600 Bin Liu wrote: > On Fri, Dec 16, 2016 at 07:19:39PM -0500, Jérémy Lefaure wrote: > > When CONFIG_PM_SLEEP is disabled, SIMPLE_DEV_PM_OPS does not use > > bfin_resume and bfin_suspend even if CONFIG_PM is enabled: > > > > drivers/usb/musb/blackfin.c:602:12: warning: ‘bfin_resume’ defined but > > not used [-Wunused-function] > > static int bfin_resume(struct device *dev) > > ^~~ > > drivers/usb/musb/blackfin.c:585:12: warning: ‘bfin_suspend’ defined but > > not used [-Wunused-function] > > static int bfin_suspend(struct device *dev) > > ^~~~ > > > > The preprocessor condition should be on CONFIG_PM_SLEEP, not on CONFIG_PM. > > However it is better to mark these functions as __maybe_unused. > > Based on discussion in [1], __may_be_unused adds unnecessary image size, > right? > > [1] http://www.spinics.net/lists/linux-usb/msg149994.html > Hi Bin, I made my own tests and I think that __maybe_unused does not add unnecessary image size in our case. When I compile with CONFIG_PM_SLEEP disabled with or without this patch, the sections of both blackfin.o have the same size (I used with readelf -S). I think that the compiler discards the unused functions and the __maybe_unused stuff only tells to the compiler to not raise a warning. Furthermore, the coding style [1] recommends to use __maybe_unused to fix this kind of warning. [1]https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=refs/tags/v4.10-rc1#n1009 Regards, Jérémy > Regards, > -Bin. > > > > > Signed-off-by: Jérémy Lefaure > > --- > > drivers/usb/musb/blackfin.c | 6 ++ > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c > > index 310238c6b5cd..f43edc43268b 100644 > > --- a/drivers/usb/musb/blackfin.c > > +++ b/drivers/usb/musb/blackfin.c > > @@ -580,8 +580,7 @@ static int bfin_remove(struct platform_device *pdev) > > return 0; > > } > > > > -#ifdef CONFIG_PM > > -static int bfin_suspend(struct device *dev) > > +static int __maybe_unused bfin_suspend(struct device *dev) > > { > > struct bfin_glue*glue = dev_get_drvdata(dev); > > struct musb *musb = glue_to_musb(glue); > > @@ -598,7 +597,7 @@ static int bfin_suspend(struct device *dev) > > return 0; > > } > > > > -static int bfin_resume(struct device *dev) > > +static int __maybe_unused bfin_resume(struct device *dev) > > { > > struct bfin_glue*glue = dev_get_drvdata(dev); > > struct musb *musb = glue_to_musb(glue); > > @@ -607,7 +606,6 @@ static int bfin_resume(struct device *dev) > > > > return 0; > > } > > -#endif > > > > static SIMPLE_DEV_PM_OPS(bfin_pm_ops, bfin_suspend, bfin_resume); > > > > -- > > 2.11.0 > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: gadget: udc: core: fix return code of usb_gadget_probe_driver()
This fixes a regression which was introduced by commit f1bddbb, by reverting a small fragment of commit 855ed04. If the following conditions were met, usb_gadget_probe_driver() returned 0, although the call was unsuccessful: 1. A particular UDC was specified by thge gadget driver (using member "udc_name" of struct usb_gadget_driver). 2. The UDC with this name is available. 3. Another gadget driver is already bound to this gadget. 4. The gadget driver has the "match_existing_only" flag set. In this case, the return code variable "ret" is set to 0, the return code of a strcmp() call (to check for the second condition). This also fixes an oops which could occur in the following scenario: 1. Two usb gadget instances were configured using configfs. 2. The first gadget configuration was bound to a UDC (using the configfs attribute "UDC"). 3. It was tried to bind the second gadget configuration to the same UDC in the same way. This operation was then wrongly reported as being successful. 4. The second gadget configuration's "UDC" attribute is cleared, to unbind the (not really bound) second gadget configuration from the UDC. ] __list_del_entry+0x29/0xc0 PGD 41b4c5067 PUD 41a598067 PMD 0 Oops: [#1] SMP Modules linked in: cdc_acm usb_f_fs usb_f_serial usb_f_acm u_serial libcomposite configfs dummy_hcd bnep intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm snd_hda_codec_hdmi irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd snd_hda_codec_realtek snd_hda_codec_generic serio_raw uvcvideo videobuf2_vmalloc btusb snd_usb_audio snd_hda_intel videobuf2_memops btrtl snd_hda_codec snd_hda_core snd_usbmidi_lib btbcm videobuf2_v4l2 btintel snd_hwdep videobuf2_core snd_seq_midi bluetooth snd_seq_midi_event videodev xpad efi_pstore snd_pcm_oss rfkill joydev media crc16 ff_memless snd_mixer_oss snd_rawmidi nls_ascii snd_pcm snd_seq snd_seq_device nls_cp437 mei_me snd_timer vfat sg udc_core lpc_ich fat efivars mfd_core mei snd soundcore battery nuvoton_cir rc_core evdev intel_smartconnect ie31200_edac edac_core shpchp tpm_tis tpm_tis_core tpm parport_pc ppdev lp parport efivarfs autofs4 btrfs xor raid6_pq hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid uas usb_storage sr_mod cdrom sd_mod ahci libahci nouveau i915 crc32c_intel i2c_algo_bit psmouse ttm xhci_pci libata scsi_mod ehci_pci drm_kms_helper xhci_hcd ehci_hcd r8169 mii usbcore drm nvme nvme_core fjes button [last unloaded: net2280] CPU: 5 PID: 829 Comm: bash Not tainted 4.9.0-rc7 #1 Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z77 Extreme3, BIOS P1.50 07/11/2013 task: 880419ce4040 task.stack: c90002ed4000 RIP: 0010:[] [] __list_del_entry+0x29/0xc0 RSP: 0018:c90002ed7d68 EFLAGS: 00010207 RAX: RBX: 88041787ec30 RCX: dead0200 RDX: RSI: 880417482002 RDI: 88041787ec30 RBP: c90002ed7d68 R08: R09: 0010 R10: R11: 880419ce4040 R12: 88041787eb68 R13: 88041787eaa8 R14: 88041560a2c0 R15: 0001 FS: 7fe4e49b8700() GS:88042f34() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 00041b4c4000 CR4: 001406e0 Stack: c90002ed7d80 94f5e68d c0ae5ef0 c90002ed7da0 c0ae22aa 88041787e800 88041787e800 c90002ed7dc0 c0d7a727 952273fa 88041aba5760 c90002ed7df8 Call Trace: [] list_del+0xd/0x30 [] usb_gadget_unregister_driver+0xaa/0xc0 [udc_core] [] unregister_gadget+0x27/0x60 [libcomposite] [] ? mutex_lock+0x1a/0x30 [] gadget_dev_desc_UDC_store+0x88/0xe0 [libcomposite] [] configfs_write_file+0xa0/0x100 [configfs] [] __vfs_write+0x37/0x160 [] ? __fd_install+0x30/0xd0 [] ? _raw_spin_unlock+0xe/0x10 [] vfs_write+0xb8/0x1b0 [] SyS_write+0x58/0xc0 [] ? __close_fd+0x94/0xc0 [] entry_SYSCALL_64_fastpath+0x1e/0xad Code: 66 90 55 48 8b 07 48 b9 00 01 00 00 00 00 ad de 48 8b 57 08 48 89 e5 48 39 c8 74 29 48 b9 00 02 00 00 00 00 ad de 48 39 ca 74 3a <4c> 8b 02 4c 39 c7 75 52 4c 8b 40 08 4c 39 c7 75 66 48 89 50 08 RIP [] __list_del_entry+0x29/0xc0 RSP CR2: ---[ end trace 99fc090ab3ff6cbc ]--- Fixes: f1bddbb ("usb: gadget: Fix binding to UDC via configfs interface") Signed-off-by: Felix Hädicke Tested-by: Krzysztof Opasiak --- changes in v2: - added "Fixes:" tag for commit f1bddbb - added "Tested-by:" tag for Krzysztof Opasiak drivers/usb/gadget/udc/core.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 9483489080f6..0402177f93cd 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -1317,7 +1317,11 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver) if (!ret) break;
Re: [PATCH 06/37] usb: host: xhci: WARN on unexpected COMP_SUCCESS
Hi, On 12/29/2016 07:00 PM, Felipe Balbi wrote: > COMP_SUCCESS should only be asserted on a *true* sucessful transfer. Any > other cases are bogus and we should aim to catch them. One easy way to > get bug reports is to trigger a WARN_ONCE() on such cases. > > Signed-off-by: Felipe Balbi > --- > drivers/usb/host/xhci-ring.c | 28 +++- > 1 file changed, 15 insertions(+), 13 deletions(-) > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index 17b878b53b46..772e07e2ef16 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -1934,6 +1934,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, > struct xhci_td *td, > { > struct xhci_virt_device *xdev; > struct xhci_ring *ep_ring; > + struct device *dev; > unsigned int slot_id; > int ep_index; > struct xhci_ep_ctx *ep_ctx; > @@ -1950,15 +1951,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, > struct xhci_td *td, > trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len)); > requested = td->urb->transfer_buffer_length; > remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); > + dev = xhci_to_hcd(xhci)->self.controller; > > switch (trb_comp_code) { > case COMP_SUCCESS: > - if (trb_type != TRB_STATUS) { > - xhci_warn(xhci, "WARN: Success on ctrl %s TRB without > IOC set?\n", > - (trb_type == TRB_DATA) ? "data" : > "setup"); > - *status = -ESHUTDOWN; > - break; > - } > + dev_WARN_ONCE(dev, trb_type != TRB_STATUS, > + "ep%d%s: unexpected success! TRB Type %d\n", > + usb_endpoint_num(&td->urb->ep->desc), > + usb_endpoint_dir_in(&td->urb->ep->desc) ? > + "in" : "out", trb_type); > *status = 0; Still setting status to 0 even "trb_type != TRB_STATUS"? Previous code set it to -ESHUTDOWN. > break; > case COMP_SHORT_PACKET: > @@ -2150,6 +2151,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, > struct xhci_td *td, > struct xhci_virt_ep *ep, int *status) > { > struct xhci_ring *ep_ring; > + struct device *dev; > u32 trb_comp_code; > u32 remaining, requested, ep_trb_len; > > @@ -2158,16 +2160,16 @@ static int process_bulk_intr_td(struct xhci_hcd > *xhci, struct xhci_td *td, > remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); > ep_trb_len = TRB_LEN(le32_to_cpu(ep_trb->generic.field[2])); > requested = td->urb->transfer_buffer_length; > + dev = xhci_to_hcd(xhci)->self.controller; > > switch (trb_comp_code) { > case COMP_SUCCESS: > - /* handle success with untransferred data as short packet */ > - if (ep_trb != td->last_trb || remaining) { > - xhci_warn(xhci, "WARN Successful completion on short > TX\n"); > - xhci_dbg(xhci, "ep %#x - asked for %d bytes, %d bytes > untransferred\n", > - td->urb->ep->desc.bEndpointAddress, > - requested, remaining); > - } > + dev_WARN_ONCE(dev, (ep_trb != td->last_trb) || remaining, > + "ep%d%s: unexpected success! TRB %p/%p size > %d/%d\n", > + usb_endpoint_num(&td->urb->ep->desc), > + usb_endpoint_dir_in(&td->urb->ep->desc) ? > + "in" : "out", ep_trb, td->last_trb, remaining, > + requested); > *status = 0; The same consideration here. Best regards, Lu Baolu > break; > case COMP_SHORT_PACKET: -- To unsubscribe from this list: send the line "unsubscribe 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 09/37] usb: host: xhci: clear only STS_EINT
Hi, On 12/29/2016 07:00 PM, Felipe Balbi wrote: > Many other bits in USBSTS register are "clear-by-writing-1". Let's make > sure that we clear *only* STS_EINT and not any of the other bits as they > might be needed later. Bit 13~31 in status register is for RsvdP (Reserved and Preserved). This means these bits are reserved for future RW implementations. Software shall preserve the value read for writes. How about below helper? static inline void clear_status_rw1c_bit(u32 bit) { u32 status; status = readl(&xhci->op_regs->status); /* preserve RsvP bits and clear RO/RW1C/RsvZ bits */ status &= ~0x1fff; status |= bit; writel(status, &xhci->op_regs->status); } Look into the code, I still find other two places where RW1C bits are not cleared correctly. In xhci_stop() and xhci_resume(), the RW1C bit is treated as a RW type. 724 temp = readl(&xhci->op_regs->status); 725 writel(temp & ~STS_EINT, &xhci->op_regs->status); I will correct these in a separated patch and cc stable as well. Best regards, Lu Baolu > > Signed-off-by: Felipe Balbi > --- > drivers/usb/host/xhci-ring.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index b4ec80d18078..116b4a0dadbb 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -2630,8 +2630,7 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd) >* so we can receive interrupts from other MSI-X interrupters. >* Write 1 to clear the interrupt status. >*/ > - status |= STS_EINT; > - writel(status, &xhci->op_regs->status); > + writel(STS_EINT, &xhci->op_regs->status); > /* FIXME when MSI-X is supported and there are multiple vectors */ > /* Clear the MSI-X event interrupt status */ > -- To unsubscribe from this list: send the line "unsubscribe 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 24/37] usb: host: xhci: use trb_to_noop() from xhci_handle_stopped_cmd_ring()
Hi, On 12/29/2016 07:00 PM, Felipe Balbi wrote: > instead of open coding how to convert a TRB to no-op, let's use our > newly introduced helper. > > Signed-off-by: Felipe Balbi > --- > drivers/usb/host/xhci-ring.c | 33 +++-- > 1 file changed, 23 insertions(+), 10 deletions(-) > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index f369d97f663d..0b8f728d6e77 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -542,6 +542,27 @@ static void trb_to_noop(union xhci_trb *trb) > trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); > trb->generic.field[3] |= cpu_to_le32(TRB_TYPE(TRB_TR_NOOP)); > break; > + case TRB_ENABLE_SLOT: > + case TRB_DISABLE_SLOT: > + case TRB_ADDR_DEV: > + case TRB_CONFIG_EP: > + case TRB_EVAL_CONTEXT: > + case TRB_RESET_EP: > + case TRB_STOP_RING: > + case TRB_SET_DEQ: > + case TRB_RESET_DEV: > + case TRB_FORCE_EVENT: > + case TRB_NEG_BANDWIDTH: > + case TRB_SET_LT: > + case TRB_GET_BW: > + case TRB_FORCE_HEADER: How about merging + case TRB_ENABLE_SLOT: + case TRB_DISABLE_SLOT: + case TRB_ADDR_DEV: + case TRB_CONFIG_EP: + case TRB_EVAL_CONTEXT: + case TRB_RESET_EP: + case TRB_STOP_RING: + case TRB_SET_DEQ: + case TRB_RESET_DEV: + case TRB_FORCE_EVENT: + case TRB_NEG_BANDWIDTH: + case TRB_SET_LT: + case TRB_GET_BW: + case TRB_FORCE_HEADER: into +case TRB_ENABLE_SLOT ... TRB_FORCE_HEADER: ? > + trb->generic.field[0] = 0; > + trb->generic.field[1] = 0; > + trb->generic.field[2] = 0; > + /* Preserve only the cycle bit of this TRB */ > + trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); > + trb->generic.field[3] = cpu_to_le32(TRB_TYPE(TRB_CMD_NOOP)); > + break; > default: > /* nothing */ Need a warning? Best regards, Lu Baolu > break; > @@ -1229,7 +1250,6 @@ static void xhci_handle_stopped_cmd_ring(struct > xhci_hcd *xhci, >struct xhci_command *cur_cmd) > { > struct xhci_command *cmd; > - u32 cycle_state; > > /* Turn all aborted commands in list to no-ops, then restart */ > list_for_each_entry(cmd, &xhci->cmd_list, > @@ -1242,15 +1262,8 @@ static void xhci_handle_stopped_cmd_ring(struct > xhci_hcd *xhci, > > xhci_dbg(xhci, "Turn aborted command %p to no-op\n", >cmd->command_trb); > - /* get cycle state from the original cmd trb */ > - cycle_state = le32_to_cpu( > - cmd->command_trb->generic.field[3]) & TRB_CYCLE; > - /* modify the command trb to no-op command */ > - cmd->command_trb->generic.field[0] = 0; > - cmd->command_trb->generic.field[1] = 0; > - cmd->command_trb->generic.field[2] = 0; > - cmd->command_trb->generic.field[3] = cpu_to_le32( > - TRB_TYPE(TRB_CMD_NOOP) | cycle_state); > + > + trb_to_noop(cmd->command_trb); > > /* >* caller waiting for completion is called when command -- To unsubscribe from this list: send the line "unsubscribe 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 30/37] usb: host: xhci: make a generic TRB tracer
Hi, On 12/29/2016 07:01 PM, Felipe Balbi wrote: > instead of having a tracer that can only trace command completions, > let's promote this tracer so it can trace and decode any TRB. > > With that, it will be easier to extrapolate the lifetime of any TRB > which might help debugging certain issues. > > Signed-off-by: Felipe Balbi > --- > drivers/usb/host/xhci-ring.c | 14 +- > drivers/usb/host/xhci-trace.h | 55 +--- > drivers/usb/host/xhci.h | 311 > ++ > 3 files changed, 357 insertions(+), 23 deletions(-) > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index 393c64f8acef..0ee7d358b812 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -1346,6 +1346,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, > > cmd_dma = le64_to_cpu(event->cmd_trb); > cmd_trb = xhci->cmd_ring->dequeue; > + > + trace_xhci_handle_command(xhci->cmd_ring, &cmd_trb->generic); > + > cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg, > cmd_trb); > /* > @@ -1362,8 +1365,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, > > del_timer(&xhci->cmd_timer); > > - trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event); > - > cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status)); > > /* If CMD ring stopped we own the trbs between enqueue and dequeue */ > @@ -2407,6 +2408,10 @@ static int handle_tx_event(struct xhci_hcd *xhci, > > ep_trb = &ep_seg->trbs[(ep_trb_dma - ep_seg->dma) / > sizeof(*ep_trb)]; > + > + trace_xhci_handle_transfer(ep_ring, > + (struct xhci_generic_trb *) ep_trb); > + > /* >* No-op TRB should not trigger interrupts. >* If ep_trb is a no-op TRB, it means the > @@ -2475,6 +2480,8 @@ static int xhci_handle_event(struct xhci_hcd *xhci) > xhci->event_ring->cycle_state) > return 0; > > + trace_xhci_handle_event(xhci->event_ring, &event->generic); > + > /* >* Barrier between reading the TRB_CYCLE (valid) flag above and any >* speculative reads of the event's flags/data below. > @@ -2642,6 +2649,9 @@ static void queue_trb(struct xhci_hcd *xhci, struct > xhci_ring *ring, > trb->field[1] = cpu_to_le32(field2); > trb->field[2] = cpu_to_le32(field3); > trb->field[3] = cpu_to_le32(field4); > + > + trace_xhci_queue_trb(ring, trb); > + > inc_enq(xhci, ring, more_trbs_coming); > } > > diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h > index 59c05653b2ea..d01524b9fb14 100644 > --- a/drivers/usb/host/xhci-trace.h > +++ b/drivers/usb/host/xhci-trace.h > @@ -115,34 +115,47 @@ DEFINE_EVENT(xhci_log_ctx, xhci_address_ctx, > TP_ARGS(xhci, ctx, ep_num) > ); > > -DECLARE_EVENT_CLASS(xhci_log_event, > - TP_PROTO(void *trb_va, struct xhci_generic_trb *ev), > - TP_ARGS(trb_va, ev), > +DECLARE_EVENT_CLASS(xhci_log_trb, > + TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb), > + TP_ARGS(ring, trb), > TP_STRUCT__entry( > - __field(void *, va) > - __field(u64, dma) > - __field(u32, status) > - __field(u32, flags) > - __dynamic_array(u8, trb, sizeof(struct xhci_generic_trb)) > + __field(u32, type) > + __field(u32, field0) > + __field(u32, field1) > + __field(u32, field2) > + __field(u32, field3) > ), > TP_fast_assign( > - __entry->va = trb_va; > - __entry->dma = ((u64)le32_to_cpu(ev->field[1])) << 32 | > - le32_to_cpu(ev->field[0]); > - __entry->status = le32_to_cpu(ev->field[2]); > - __entry->flags = le32_to_cpu(ev->field[3]); > - memcpy(__get_dynamic_array(trb), trb_va, > - sizeof(struct xhci_generic_trb)); > + __entry->type = ring->type; > + __entry->field0 = le32_to_cpu(trb->field[0]); > + __entry->field1 = le32_to_cpu(trb->field[1]); > + __entry->field2 = le32_to_cpu(trb->field[2]); > + __entry->field3 = le32_to_cpu(trb->field[3]); > ), > - TP_printk("\ntrb_dma=@%llx, trb_va=@%p, status=%08x, flags=%08x", > - (unsigned long long) __entry->dma, __entry->va, > - __entry->status, __entry->flags > + TP_printk("%s: %s", xhci_ring_type_string(__entry->type), > + xhci_decode_trb(__entry->field0, __entry->field1, > + __entry->field2, __entry->field3) > ) > ); > > -DEFINE_EVENT(xhci_log_event, xhci_cmd_completion, > - TP_PROTO(void *trb_va, struct xhci_generic_trb *ev), > - TP_ARGS(trb_va
Re: [PATCH 06/12] usb: dwc3: omap: Replace the extcon API
Hi Felipe, On 2016년 12월 02일 18:03, Felipe Balbi wrote: > > Hi, > > Chanwoo Choi writes: >> Hi Felipe, >> >> On 2016년 11월 30일 19:36, Felipe Balbi wrote: >>> >>> Hi, >>> >>> Chanwoo Choi writes: This patch uses the resource-managed extcon API for extcon_register_notifier() and replaces the deprecated extcon API as following: - extcon_get_cable_state_() -> extcon_get_state() Signed-off-by: Chanwoo Choi >>> >>> Acked-by: Felipe Balbi >>> >> >> Thanks for your review. >> >> Each patch has no any dependency among patches. >> So, If possible, could you pick the patch6/8/9/10/11/12 on your tree? > > my tree is closed for v4.10, I can pick it up for v4.11 Could you please pick up these patches(patch6/8/9/10/11/12)? These patches were already reviewed by you. -- Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe 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 5/6] usb: phy: tahvo: Replace the deprecated extcon API
This patch replaces the deprecated extcon API as following: - extcon_set_cable_state_() -> extcon_set_state_sync() Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi --- drivers/usb/phy/phy-tahvo.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c index ab5d364f6e8c..a31c8682e998 100644 --- a/drivers/usb/phy/phy-tahvo.c +++ b/drivers/usb/phy/phy-tahvo.c @@ -121,7 +121,7 @@ static void check_vbus_state(struct tahvo_usb *tu) prev_state = tu->vbus_state; tu->vbus_state = reg & TAHVO_STAT_VBUS; if (prev_state != tu->vbus_state) { - extcon_set_cable_state_(tu->extcon, EXTCON_USB, tu->vbus_state); + extcon_set_state_sync(tu->extcon, EXTCON_USB, tu->vbus_state); sysfs_notify(&tu->pt_dev->dev.kobj, NULL, "vbus_state"); } } @@ -130,7 +130,7 @@ static void tahvo_usb_become_host(struct tahvo_usb *tu) { struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent); - extcon_set_cable_state_(tu->extcon, EXTCON_USB_HOST, true); + extcon_set_state_sync(tu->extcon, EXTCON_USB_HOST, true); /* Power up the transceiver in USB host mode */ retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND | @@ -149,7 +149,7 @@ static void tahvo_usb_become_peripheral(struct tahvo_usb *tu) { struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent); - extcon_set_cable_state_(tu->extcon, EXTCON_USB_HOST, false); + extcon_set_state_sync(tu->extcon, EXTCON_USB_HOST, false); /* Power up transceiver and set it in USB peripheral mode */ retu_write(rdev, TAHVO_REG_USBR, USBR_SLAVE_CONTROL | USBR_REGOUT | @@ -379,9 +379,9 @@ static int tahvo_usb_probe(struct platform_device *pdev) } /* Set the initial cable state. */ - extcon_set_cable_state_(tu->extcon, EXTCON_USB_HOST, + extcon_set_state_sync(tu->extcon, EXTCON_USB_HOST, tu->tahvo_mode == TAHVO_MODE_HOST); - extcon_set_cable_state_(tu->extcon, EXTCON_USB, tu->vbus_state); + extcon_set_state_sync(tu->extcon, EXTCON_USB, tu->vbus_state); /* Create OTG interface */ tahvo_usb_power_off(tu); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe 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 3/6] usb: phy: omap-otg: Replace the extcon API
This patch uses the resource-managed extcon API for extcon_register_notifier() and replaces the deprecated extcon API as following: - extcon_get_cable_state_() -> extcon_get_state() Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi --- drivers/usb/phy/phy-omap-otg.c | 24 ++-- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/drivers/usb/phy/phy-omap-otg.c b/drivers/usb/phy/phy-omap-otg.c index 6523af4f8f93..800d1d90753d 100644 --- a/drivers/usb/phy/phy-omap-otg.c +++ b/drivers/usb/phy/phy-omap-otg.c @@ -118,19 +118,19 @@ static int omap_otg_probe(struct platform_device *pdev) otg_dev->id_nb.notifier_call = omap_otg_id_notifier; otg_dev->vbus_nb.notifier_call = omap_otg_vbus_notifier; - ret = extcon_register_notifier(extcon, EXTCON_USB_HOST, &otg_dev->id_nb); + ret = devm_extcon_register_notifier(&pdev->dev, extcon, + EXTCON_USB_HOST, &otg_dev->id_nb); if (ret) return ret; - ret = extcon_register_notifier(extcon, EXTCON_USB, &otg_dev->vbus_nb); + ret = devm_extcon_register_notifier(&pdev->dev, extcon, + EXTCON_USB, &otg_dev->vbus_nb); if (ret) { - extcon_unregister_notifier(extcon, EXTCON_USB_HOST, - &otg_dev->id_nb); return ret; } - otg_dev->id = extcon_get_cable_state_(extcon, EXTCON_USB_HOST); - otg_dev->vbus = extcon_get_cable_state_(extcon, EXTCON_USB); + otg_dev->id = extcon_get_state(extcon, EXTCON_USB_HOST); + otg_dev->vbus = extcon_get_state(extcon, EXTCON_USB); omap_otg_set_mode(otg_dev); rev = readl(otg_dev->base); @@ -145,20 +145,8 @@ static int omap_otg_probe(struct platform_device *pdev) return 0; } -static int omap_otg_remove(struct platform_device *pdev) -{ - struct otg_device *otg_dev = platform_get_drvdata(pdev); - struct extcon_dev *edev = otg_dev->extcon; - - extcon_unregister_notifier(edev, EXTCON_USB_HOST, &otg_dev->id_nb); - extcon_unregister_notifier(edev, EXTCON_USB, &otg_dev->vbus_nb); - - return 0; -} - static struct platform_driver omap_otg_driver = { .probe = omap_otg_probe, - .remove = omap_otg_remove, .driver = { .name = "omap_otg", }, -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe 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 4/6] usb: phy: qcom-8x16-usb: Replace the extcon API
This patch uses the resource-managed extcon API for extcon_register_notifier() and replaces the deprecated extcon API as following: - extcon_get_cable_state_() -> extcon_get_state() Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi --- drivers/usb/phy/phy-qcom-8x16-usb.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/usb/phy/phy-qcom-8x16-usb.c b/drivers/usb/phy/phy-qcom-8x16-usb.c index d8593adb3621..fdf686398772 100644 --- a/drivers/usb/phy/phy-qcom-8x16-usb.c +++ b/drivers/usb/phy/phy-qcom-8x16-usb.c @@ -187,7 +187,7 @@ static int phy_8x16_init(struct usb_phy *phy) val = ULPI_PWR_OTG_COMP_DISABLE; usb_phy_io_write(phy, val, ULPI_SET(ULPI_PWR_CLK_MNG_REG)); - state = extcon_get_cable_state_(qphy->vbus_edev, EXTCON_USB); + state = extcon_get_state(qphy->vbus_edev, EXTCON_USB); if (state) phy_8x16_vbus_on(qphy); else @@ -316,23 +316,20 @@ static int phy_8x16_probe(struct platform_device *pdev) goto off_clks; qphy->vbus_notify.notifier_call = phy_8x16_vbus_notify; - ret = extcon_register_notifier(qphy->vbus_edev, EXTCON_USB, - &qphy->vbus_notify); + ret = devm_extcon_register_notifier(&pdev->dev, qphy->vbus_edev, + EXTCON_USB, &qphy->vbus_notify); if (ret < 0) goto off_power; ret = usb_add_phy_dev(&qphy->phy); if (ret) - goto off_extcon; + goto off_power; qphy->reboot_notify.notifier_call = phy_8x16_reboot_notify; register_reboot_notifier(&qphy->reboot_notify); return 0; -off_extcon: - extcon_unregister_notifier(qphy->vbus_edev, EXTCON_USB, - &qphy->vbus_notify); off_power: regulator_bulk_disable(ARRAY_SIZE(qphy->regulator), qphy->regulator); off_clks: @@ -347,8 +344,6 @@ static int phy_8x16_remove(struct platform_device *pdev) struct phy_8x16 *qphy = platform_get_drvdata(pdev); unregister_reboot_notifier(&qphy->reboot_notify); - extcon_unregister_notifier(qphy->vbus_edev, EXTCON_USB, - &qphy->vbus_notify); /* * Ensure that D+/D- lines are routed to uB connector, so -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe 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 6/6] usb: renesas_usbhs: Replace the deprecated extcon API
This patch replaces the deprecated extcon API as following: - extcon_get_cable_state_() -> extcon_get_state() Cc: Rob Herring Cc: Geert Uytterhoeven Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi Acked-by: Yoshihiro Shimoda --- drivers/usb/renesas_usbhs/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c index 012a37aa3e0d..623c51300393 100644 --- a/drivers/usb/renesas_usbhs/common.c +++ b/drivers/usb/renesas_usbhs/common.c @@ -389,7 +389,7 @@ static void usbhsc_hotplug(struct usbhs_priv *priv) if (enable && !mod) { if (priv->edev) { - cable = extcon_get_cable_state_(priv->edev, EXTCON_USB_HOST); + cable = extcon_get_state(priv->edev, EXTCON_USB_HOST); if ((cable > 0 && id != USBHS_HOST) || (!cable && id != USBHS_GADGET)) { dev_info(&pdev->dev, -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/6] usb: Replace the deprecated extcon API
This patches just replace the deprecated extcon API without any change of extcon operation and use the resource-managed function for extcon_register_notifier(). The new extcon API instead of deprecated API. - extcon_set_cable_state_() -> extcon_set_state_sync(); - extcon_get_cable_state_() -> extcon_get_state(); Changes from v1: - Rebase these patches based on v4.10-rc1. - Add acked-by tag from usb maintainer and reviewer. - Drop the phy/power-supply/chipidea patches. Chanwoo Choi (6): usb: dwc3: omap: Replace the extcon API usb: phy: msm: Replace the extcon API usb: phy: omap-otg: Replace the extcon API usb: phy: qcom-8x16-usb: Replace the extcon API usb: phy: tahvo: Replace the deprecated extcon API usb: renesas_usbhs: Replace the deprecated extcon API drivers/usb/dwc3/dwc3-omap.c| 20 +++- drivers/usb/phy/phy-msm-usb.c | 33 +++-- drivers/usb/phy/phy-omap-otg.c | 24 ++-- drivers/usb/phy/phy-qcom-8x16-usb.c | 13 - drivers/usb/phy/phy-tahvo.c | 10 +- drivers/usb/renesas_usbhs/common.c | 2 +- 6 files changed, 34 insertions(+), 68 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe 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/6] usb: dwc3: omap: Replace the extcon API
This patch uses the resource-managed extcon API for extcon_register_notifier() and replaces the deprecated extcon API as following: - extcon_get_cable_state_() -> extcon_get_state() Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi --- drivers/usb/dwc3/dwc3-omap.c | 20 +++- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 29e80cc9b634..2d2e9aa1db08 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -425,20 +425,20 @@ static int dwc3_omap_extcon_register(struct dwc3_omap *omap) } omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier; - ret = extcon_register_notifier(edev, EXTCON_USB, - &omap->vbus_nb); + ret = devm_extcon_register_notifier(omap->dev, edev, + EXTCON_USB, &omap->vbus_nb); if (ret < 0) dev_vdbg(omap->dev, "failed to register notifier for USB\n"); omap->id_nb.notifier_call = dwc3_omap_id_notifier; - ret = extcon_register_notifier(edev, EXTCON_USB_HOST, - &omap->id_nb); + ret = devm_extcon_register_notifier(omap->dev, edev, + EXTCON_USB_HOST, &omap->id_nb); if (ret < 0) dev_vdbg(omap->dev, "failed to register notifier for USB-HOST\n"); - if (extcon_get_cable_state_(edev, EXTCON_USB) == true) + if (extcon_get_state(edev, EXTCON_USB) == true) dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID); - if (extcon_get_cable_state_(edev, EXTCON_USB_HOST) == true) + if (extcon_get_state(edev, EXTCON_USB_HOST) == true) dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND); omap->edev = edev; @@ -527,17 +527,13 @@ static int dwc3_omap_probe(struct platform_device *pdev) ret = of_platform_populate(node, NULL, NULL, dev); if (ret) { dev_err(&pdev->dev, "failed to create dwc3 core\n"); - goto err2; + goto err1; } dwc3_omap_enable_irqs(omap); return 0; -err2: - extcon_unregister_notifier(omap->edev, EXTCON_USB, &omap->vbus_nb); - extcon_unregister_notifier(omap->edev, EXTCON_USB_HOST, &omap->id_nb); - err1: pm_runtime_put_sync(dev); pm_runtime_disable(dev); @@ -549,8 +545,6 @@ static int dwc3_omap_remove(struct platform_device *pdev) { struct dwc3_omap*omap = platform_get_drvdata(pdev); - extcon_unregister_notifier(omap->edev, EXTCON_USB, &omap->vbus_nb); - extcon_unregister_notifier(omap->edev, EXTCON_USB_HOST, &omap->id_nb); dwc3_omap_disable_irqs(omap); of_platform_depopulate(omap->dev); pm_runtime_put_sync(&pdev->dev); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe 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] usb: chipdata: Replace the extcon API
This patch uses the resource-managed extcon API for extcon_register_notifier() and replaces the deprecated extcon API as following: - extcon_get_cable_state_() -> extcon_get_state() Cc: Peter Chen Signed-off-by: Chanwoo Choi --- Changes from v1: - Rebase these patches based on v4.10-rc1. - Drop the phy/power-supply/dwc3/omap patches. drivers/usb/chipidea/core.c | 30 ++ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 3dbb4a21ab44..5c35f25e9bce 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -742,7 +742,7 @@ static int ci_get_platdata(struct device *dev, cable->edev = ext_vbus; if (!IS_ERR(ext_vbus)) { - ret = extcon_get_cable_state_(cable->edev, EXTCON_USB); + ret = extcon_get_state(cable->edev, EXTCON_USB); if (ret) cable->state = true; else @@ -754,7 +754,7 @@ static int ci_get_platdata(struct device *dev, cable->edev = ext_id; if (!IS_ERR(ext_id)) { - ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST); + ret = extcon_get_state(cable->edev, EXTCON_USB_HOST); if (ret) cable->state = false; else @@ -771,8 +771,8 @@ static int ci_extcon_register(struct ci_hdrc *ci) id = &ci->platdata->id_extcon; id->ci = ci; if (!IS_ERR(id->edev)) { - ret = extcon_register_notifier(id->edev, EXTCON_USB_HOST, - &id->nb); + ret = devm_extcon_register_notifier(ci->dev, id->edev, + EXTCON_USB_HOST, &id->nb); if (ret < 0) { dev_err(ci->dev, "register ID failed\n"); return ret; @@ -782,11 +782,9 @@ static int ci_extcon_register(struct ci_hdrc *ci) vbus = &ci->platdata->vbus_extcon; vbus->ci = ci; if (!IS_ERR(vbus->edev)) { - ret = extcon_register_notifier(vbus->edev, EXTCON_USB, - &vbus->nb); + ret = devm_extcon_register_notifier(ci->dev, vbus->edev, + EXTCON_USB, &vbus->nb); if (ret < 0) { - extcon_unregister_notifier(id->edev, EXTCON_USB_HOST, - &id->nb); dev_err(ci->dev, "register VBUS failed\n"); return ret; } @@ -795,20 +793,6 @@ static int ci_extcon_register(struct ci_hdrc *ci) return 0; } -static void ci_extcon_unregister(struct ci_hdrc *ci) -{ - struct ci_hdrc_cable *cable; - - cable = &ci->platdata->id_extcon; - if (!IS_ERR(cable->edev)) - extcon_unregister_notifier(cable->edev, EXTCON_USB_HOST, - &cable->nb); - - cable = &ci->platdata->vbus_extcon; - if (!IS_ERR(cable->edev)) - extcon_unregister_notifier(cable->edev, EXTCON_USB, &cable->nb); -} - static DEFINE_IDA(ci_ida); struct platform_device *ci_hdrc_add_device(struct device *dev, @@ -1054,7 +1038,6 @@ static int ci_hdrc_probe(struct platform_device *pdev) if (!ret) return 0; - ci_extcon_unregister(ci); stop: ci_role_destroy(ci); deinit_phy: @@ -1074,7 +1057,6 @@ static int ci_hdrc_remove(struct platform_device *pdev) } dbg_remove_files(ci); - ci_extcon_unregister(ci); ci_role_destroy(ci); ci_hdrc_enter_lpm(ci, true); ci_usb_phy_exit(ci); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe 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/6] usb: phy: msm: Replace the extcon API
This patch uses the resource-managed extcon API for extcon_register_notifier() and replaces the deprecated extcon API as following: - extcon_get_cable_state_() -> extcon_get_state() Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi --- drivers/usb/phy/phy-msm-usb.c | 33 +++-- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 8a34759727bb..a15a89d4235d 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1742,14 +1742,14 @@ static int msm_otg_read_dt(struct platform_device *pdev, struct msm_otg *motg) if (!IS_ERR(ext_vbus)) { motg->vbus.extcon = ext_vbus; motg->vbus.nb.notifier_call = msm_otg_vbus_notifier; - ret = extcon_register_notifier(ext_vbus, EXTCON_USB, - &motg->vbus.nb); + ret = devm_extcon_register_notifier(&pdev->dev, ext_vbus, + EXTCON_USB, &motg->vbus.nb); if (ret < 0) { dev_err(&pdev->dev, "register VBUS notifier failed\n"); return ret; } - ret = extcon_get_cable_state_(ext_vbus, EXTCON_USB); + ret = extcon_get_state(ext_vbus, EXTCON_USB); if (ret) set_bit(B_SESS_VLD, &motg->inputs); else @@ -1759,16 +1759,14 @@ static int msm_otg_read_dt(struct platform_device *pdev, struct msm_otg *motg) if (!IS_ERR(ext_id)) { motg->id.extcon = ext_id; motg->id.nb.notifier_call = msm_otg_id_notifier; - ret = extcon_register_notifier(ext_id, EXTCON_USB_HOST, - &motg->id.nb); + ret = devm_extcon_register_notifier(&pdev->dev, ext_id, + EXTCON_USB_HOST, &motg->id.nb); if (ret < 0) { dev_err(&pdev->dev, "register ID notifier failed\n"); - extcon_unregister_notifier(motg->vbus.extcon, - EXTCON_USB, &motg->vbus.nb); return ret; } - ret = extcon_get_cable_state_(ext_id, EXTCON_USB_HOST); + ret = extcon_get_state(ext_id, EXTCON_USB_HOST); if (ret) clear_bit(ID, &motg->inputs); else @@ -1883,10 +1881,9 @@ static int msm_otg_probe(struct platform_device *pdev) */ if (motg->phy_number) { phy_select = devm_ioremap_nocache(&pdev->dev, USB2_PHY_SEL, 4); - if (!phy_select) { - ret = -ENOMEM; - goto unregister_extcon; - } + if (!phy_select) + return -ENOMEM; + /* Enable second PHY with the OTG port */ writel(0x1, phy_select); } @@ -1897,7 +1894,7 @@ static int msm_otg_probe(struct platform_device *pdev) if (motg->irq < 0) { dev_err(&pdev->dev, "platform_get_irq failed\n"); ret = motg->irq; - goto unregister_extcon; + return motg->irq; } regs[0].supply = "vddcx"; @@ -1906,7 +1903,7 @@ static int msm_otg_probe(struct platform_device *pdev) ret = devm_regulator_bulk_get(motg->phy.dev, ARRAY_SIZE(regs), regs); if (ret) - goto unregister_extcon; + return ret; motg->vddcx = regs[0].consumer; motg->v3p3 = regs[1].consumer; @@ -2003,11 +2000,6 @@ static int msm_otg_probe(struct platform_device *pdev) clk_disable_unprepare(motg->clk); if (!IS_ERR(motg->core_clk)) clk_disable_unprepare(motg->core_clk); -unregister_extcon: - extcon_unregister_notifier(motg->id.extcon, - EXTCON_USB_HOST, &motg->id.nb); - extcon_unregister_notifier(motg->vbus.extcon, - EXTCON_USB, &motg->vbus.nb); return ret; } @@ -2029,9 +2021,6 @@ static int msm_otg_remove(struct platform_device *pdev) */ gpiod_set_value_cansleep(motg->switch_gpio, 0); - extcon_unregister_notifier(motg->id.extcon, EXTCON_USB_HOST, &motg->id.nb); - extcon_unregister_notifier(motg->vbus.extcon, EXTCON_USB, &motg->vbus.nb); - msm_otg_debugfs_cleanup(); cancel_delayed_work_sync(&motg->chg_work); cancel_work_sync(&motg->sm_work); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe 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] usb: musb: sunxi: Uses the resource-managed extcon API when registering extcon notifier
This patch just uses the resource-managed extcon API when registering the extcon notifier. Signed-off-by: Chanwoo Choi Acked-by: Maxime Ripard Acked-by: Bin Liu --- Changes from v1: - Rebase this patch based on v4.10-rc1. - Add acked-by tag from Maxime Ripard and Bin Lin. - Drop the phy/power-supply/chipidea patches. drivers/usb/musb/sunxi.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c index d0be0eadd0d9..2332294dee0f 100644 --- a/drivers/usb/musb/sunxi.c +++ b/drivers/usb/musb/sunxi.c @@ -251,14 +251,14 @@ static int sunxi_musb_init(struct musb *musb) writeb(SUNXI_MUSB_VEND0_PIO_MODE, musb->mregs + SUNXI_MUSB_VEND0); /* Register notifier before calling phy_init() */ - ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST, - &glue->host_nb); + ret = devm_extcon_register_notifier(glue->dev, glue->extcon, + EXTCON_USB_HOST, &glue->host_nb); if (ret) goto error_reset_assert; ret = phy_init(glue->phy); if (ret) - goto error_unregister_notifier; + goto error_reset_assert; musb->isr = sunxi_musb_interrupt; @@ -267,9 +267,6 @@ static int sunxi_musb_init(struct musb *musb) return 0; -error_unregister_notifier: - extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST, - &glue->host_nb); error_reset_assert: if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags)) reset_control_assert(glue->rst); @@ -293,9 +290,6 @@ static int sunxi_musb_exit(struct musb *musb) phy_exit(glue->phy); - extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST, - &glue->host_nb); - if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags)) reset_control_assert(glue->rst); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] usb: xhci: clear EINT bit in status correctly
EINT(Event Interrupt) is a write-1-to-clear type of bit in xhci status register. It should be cleared by writing a 1. Writing 0 to this bit has no effect. Xhci driver tries to clear this bit by writing 0 to it. This is not the right way to go. This patch corrects this by reading the register first, then clearing all RO/RW1C/RsvZ bits and setting the clearing bit, and writing back the new value at last. Xhci spec requires that software that uses EINT shall clear it prior to clearing any IP flags in section 5.4.2. This is the reason why this patch is CC'ed stable as well. Cc: # v3.14+ CC: Felipe Balbi Signed-off-by: Lu Baolu --- drivers/usb/host/xhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 1cd5641..18ea6b8 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -721,7 +721,7 @@ void xhci_stop(struct usb_hcd *hcd) xhci_dbg_trace(xhci, trace_xhci_dbg_init, "// Disabling event ring interrupts"); temp = readl(&xhci->op_regs->status); - writel(temp & ~STS_EINT, &xhci->op_regs->status); + writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); temp = readl(&xhci->ir_set->irq_pending); writel(ER_IRQ_DISABLE(temp), &xhci->ir_set->irq_pending); xhci_print_ir_set(xhci, 0); @@ -1054,7 +1054,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) xhci_dbg(xhci, "// Disabling event ring interrupts\n"); temp = readl(&xhci->op_regs->status); - writel(temp & ~STS_EINT, &xhci->op_regs->status); + writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); temp = readl(&xhci->ir_set->irq_pending); writel(ER_IRQ_DISABLE(temp), &xhci->ir_set->irq_pending); xhci_print_ir_set(xhci, 0); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: xhci: clear EINT bit in status correctly
Hi Lu, [auto build test WARNING on usb/usb-testing] [also build test WARNING on v4.10-rc1 next-20161224] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Lu-Baolu/usb-xhci-clear-EINT-bit-in-status-correctly/20161230-133444 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: i386-randconfig-x003-201652 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): drivers/usb/host/xhci.c: In function 'xhci_stop': >> drivers/usb/host/xhci.c:724:14: warning: suggest parentheses around >> arithmetic in operand of '|' [-Wparentheses] writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); ~^ drivers/usb/host/xhci.c: In function 'xhci_resume': drivers/usb/host/xhci.c:1057:15: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses] writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); ~^ vim +724 drivers/usb/host/xhci.c 708 709 /* Deleting Compliance Mode Recovery Timer */ 710 if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && 711 (!(xhci_all_ports_seen_u0(xhci { 712 del_timer_sync(&xhci->comp_mode_recovery_timer); 713 xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, 714 "%s: compliance mode recovery timer deleted", 715 __func__); 716 } 717 718 if (xhci->quirks & XHCI_AMD_PLL_FIX) 719 usb_amd_dev_put(); 720 721 xhci_dbg_trace(xhci, trace_xhci_dbg_init, 722 "// Disabling event ring interrupts"); 723 temp = readl(&xhci->op_regs->status); > 724 writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); 725 temp = readl(&xhci->ir_set->irq_pending); 726 writel(ER_IRQ_DISABLE(temp), &xhci->ir_set->irq_pending); 727 xhci_print_ir_set(xhci, 0); 728 729 xhci_dbg_trace(xhci, trace_xhci_dbg_init, "cleaning up memory"); 730 xhci_mem_cleanup(xhci); 731 xhci_dbg_trace(xhci, trace_xhci_dbg_init, 732 "xhci_stop completed - status = %x", --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH 1/1] usb: xhci: clear EINT bit in status correctly
Hi Lu, [auto build test WARNING on usb/usb-testing] [also build test WARNING on v4.10-rc1 next-20161224] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Lu-Baolu/usb-xhci-clear-EINT-bit-in-status-correctly/20161230-133444 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: m68k-allmodconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 4.9.0 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k All warnings (new ones prefixed by >>): In file included from include/linux/swab.h:4:0, from include/uapi/linux/byteorder/big_endian.h:12, from include/linux/byteorder/big_endian.h:4, from arch/m68k/include/uapi/asm/byteorder.h:4, from include/asm-generic/bitops/le.h:5, from arch/m68k/include/asm/bitops.h:518, from include/linux/bitops.h:36, from include/linux/kernel.h:10, from include/linux/list.h:8, from include/linux/pci.h:25, from drivers/usb/host/xhci.c:23: drivers/usb/host/xhci.c: In function 'xhci_stop': drivers/usb/host/xhci.c:724:14: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses] writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); ^ include/uapi/linux/swab.h:116:32: note: in definition of macro '__swab32' (__builtin_constant_p((__u32)(x)) ? \ ^ include/linux/byteorder/generic.h:87:21: note: in expansion of macro '__cpu_to_le32' #define cpu_to_le32 __cpu_to_le32 ^ >> arch/m68k/include/asm/io_mm.h:452:26: note: in expansion of macro 'out_le32' #define writel(val,addr) out_le32((addr),(val)) ^ drivers/usb/host/xhci.c:724:2: note: in expansion of macro 'writel' writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); ^ drivers/usb/host/xhci.c:724:14: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses] writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); ^ include/uapi/linux/swab.h:17:12: note: in definition of macro '___constant_swab32' (((__u32)(x) & (__u32)0x00ffUL) << 24) | \ ^ include/uapi/linux/byteorder/big_endian.h:32:43: note: in expansion of macro '__swab32' #define __cpu_to_le32(x) ((__force __le32)__swab32((x))) ^ include/linux/byteorder/generic.h:87:21: note: in expansion of macro '__cpu_to_le32' #define cpu_to_le32 __cpu_to_le32 ^ >> arch/m68k/include/asm/io_mm.h:452:26: note: in expansion of macro 'out_le32' #define writel(val,addr) out_le32((addr),(val)) ^ drivers/usb/host/xhci.c:724:2: note: in expansion of macro 'writel' writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); ^ drivers/usb/host/xhci.c:724:14: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses] writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); ^ include/uapi/linux/swab.h:18:12: note: in definition of macro '___constant_swab32' (((__u32)(x) & (__u32)0xff00UL) << 8) | \ ^ include/uapi/linux/byteorder/big_endian.h:32:43: note: in expansion of macro '__swab32' #define __cpu_to_le32(x) ((__force __le32)__swab32((x))) ^ include/linux/byteorder/generic.h:87:21: note: in expansion of macro '__cpu_to_le32' #define cpu_to_le32 __cpu_to_le32 ^ >> arch/m68k/include/asm/io_mm.h:452:26: note: in expansion of macro 'out_le32' #define writel(val,addr) out_le32((addr),(val)) ^ drivers/usb/host/xhci.c:724:2: note: in expansion of macro 'writel' writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); ^ drivers/usb/host/xhci.c:724:14: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses] writel(temp & ~0x1fff | STS_EINT, &xhci->op_regs->status); ^ include/uapi/linux/swab.h:19:12: note: in definition of macro '___constant_swab32' (((__u32)(x) & (__u32)0x00ffUL) >> 8) | \ ^ include/uapi/linux/byteorder/big_endian.h:32:43: note: in expansion of macro '__swab32' #define __cpu_to_le32(x) ((__force __le32)__swab32((x))) ^ include/linux/byteorder/generic.h:87:21: note: in expansion of macro '__cpu_to_le32' #define cpu_to_le32 __cpu_to_l
Re: [PATCH 06/12] usb: dwc3: omap: Replace the extcon API
Hi Felipe, On 2016년 12월 30일 11:46, Chanwoo Choi wrote: > Hi Felipe, > > On 2016년 12월 02일 18:03, Felipe Balbi wrote: >> >> Hi, >> >> Chanwoo Choi writes: >>> Hi Felipe, >>> >>> On 2016년 11월 30일 19:36, Felipe Balbi wrote: Hi, Chanwoo Choi writes: > This patch uses the resource-managed extcon API for > extcon_register_notifier() > and replaces the deprecated extcon API as following: > - extcon_get_cable_state_() -> extcon_get_state() > > Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi >>> >>> Thanks for your review. >>> >>> Each patch has no any dependency among patches. >>> So, If possible, could you pick the patch6/8/9/10/11/12 on your tree? >> >> my tree is closed for v4.10, I can pick it up for v4.11 > > Could you please pick up these patches(patch6/8/9/10/11/12)? > These patches were already reviewed by you. > I sent v2 patchset. [1] https://www.spinics.net/lists/kernel/msg2410950.html - "[PATCH v2 0/6] usb: Replace the deprecated extcon API" -- Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe 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 30/37] usb: host: xhci: make a generic TRB tracer
Hi, Lu Baolu writes: >> +DEFINE_EVENT(xhci_log_trb, xhci_handle_event, >> +TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb), >> +TP_ARGS(ring, trb) >> +); >> + >> +DEFINE_EVENT(xhci_log_trb, xhci_handle_command, >> +TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb), >> +TP_ARGS(ring, trb) >> +); >> + >> +DEFINE_EVENT(xhci_log_trb, xhci_handle_transfer, >> +TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb), >> +TP_ARGS(ring, trb) >> +); >> + >> +DEFINE_EVENT(xhci_log_trb, xhci_queue_trb, >> +TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb), >> +TP_ARGS(ring, trb) >> ); > > How about add xhci_dequeue_trb for those dequeued from event ring? Sure, and where would the tracer be? When do we "dequeue" TRBs? > It should be helpful, if we can track the link trb as well. right -- balbi -- To unsubscribe from this list: send the line "unsubscribe 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 06/37] usb: host: xhci: WARN on unexpected COMP_SUCCESS
Hi, Lu Baolu writes: > On 12/29/2016 07:00 PM, Felipe Balbi wrote: >> COMP_SUCCESS should only be asserted on a *true* sucessful transfer. Any >> other cases are bogus and we should aim to catch them. One easy way to >> get bug reports is to trigger a WARN_ONCE() on such cases. >> >> Signed-off-by: Felipe Balbi >> --- >> drivers/usb/host/xhci-ring.c | 28 +++- >> 1 file changed, 15 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c >> index 17b878b53b46..772e07e2ef16 100644 >> --- a/drivers/usb/host/xhci-ring.c >> +++ b/drivers/usb/host/xhci-ring.c >> @@ -1934,6 +1934,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, >> struct xhci_td *td, >> { >> struct xhci_virt_device *xdev; >> struct xhci_ring *ep_ring; >> +struct device *dev; >> unsigned int slot_id; >> int ep_index; >> struct xhci_ep_ctx *ep_ctx; >> @@ -1950,15 +1951,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, >> struct xhci_td *td, >> trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len)); >> requested = td->urb->transfer_buffer_length; >> remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); >> +dev = xhci_to_hcd(xhci)->self.controller; >> >> switch (trb_comp_code) { >> case COMP_SUCCESS: >> -if (trb_type != TRB_STATUS) { >> -xhci_warn(xhci, "WARN: Success on ctrl %s TRB without >> IOC set?\n", >> -(trb_type == TRB_DATA) ? "data" : >> "setup"); >> -*status = -ESHUTDOWN; >> -break; >> -} >> +dev_WARN_ONCE(dev, trb_type != TRB_STATUS, >> +"ep%d%s: unexpected success! TRB Type %d\n", >> +usb_endpoint_num(&td->urb->ep->desc), >> +usb_endpoint_dir_in(&td->urb->ep->desc) ? >> +"in" : "out", trb_type); >> *status = 0; > > Still setting status to 0 even "trb_type != TRB_STATUS"? yeah, the reason is that I don't think this will *ever* happen. If it does, I really wanna know about it. Moreover, it shouldn't be such a fatal error if that happens. Mathias, any comments? -- balbi -- To unsubscribe from this list: send the line "unsubscribe 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 09/37] usb: host: xhci: clear only STS_EINT
Hi, Lu Baolu writes: > On 12/29/2016 07:00 PM, Felipe Balbi wrote: >> Many other bits in USBSTS register are "clear-by-writing-1". Let's make >> sure that we clear *only* STS_EINT and not any of the other bits as they >> might be needed later. > > Bit 13~31 in status register is for RsvdP (Reserved and Preserved). totally missed the "Preserved" part. Thanks > This means these bits are reserved for future RW implementations. > Software shall preserve the value read for writes. > > How about below helper? > > static inline void clear_status_rw1c_bit(u32 bit) > { > u32 status; > > status = readl(&xhci->op_regs->status); > /* preserve RsvP bits and clear RO/RW1C/RsvZ bits */ > status &= ~0x1fff; > status |= bit; > > writel(status, &xhci->op_regs->status); > } not sure, I have no problems with it, but it's for mathias to decide. If anything, it should be prefixed by xhci_ so it's easier to use ftrace. > Look into the code, I still find other two places where RW1C bits > are not cleared correctly. > > In xhci_stop() and xhci_resume(), the RW1C bit is treated as a RW type. > > 724 temp = readl(&xhci->op_regs->status); > 725 writel(temp & ~STS_EINT, &xhci->op_regs->status); > > I will correct these in a separated patch and cc stable as well. That's cool. Then I can drop this patch from my series. -- balbi -- To unsubscribe from this list: send the line "unsubscribe 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 24/37] usb: host: xhci: use trb_to_noop() from xhci_handle_stopped_cmd_ring()
Hi, Lu Baolu writes: > On 12/29/2016 07:00 PM, Felipe Balbi wrote: >> instead of open coding how to convert a TRB to no-op, let's use our >> newly introduced helper. >> >> Signed-off-by: Felipe Balbi >> --- >> drivers/usb/host/xhci-ring.c | 33 +++-- >> 1 file changed, 23 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c >> index f369d97f663d..0b8f728d6e77 100644 >> --- a/drivers/usb/host/xhci-ring.c >> +++ b/drivers/usb/host/xhci-ring.c >> @@ -542,6 +542,27 @@ static void trb_to_noop(union xhci_trb *trb) >> trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); >> trb->generic.field[3] |= cpu_to_le32(TRB_TYPE(TRB_TR_NOOP)); >> break; >> +case TRB_ENABLE_SLOT: >> +case TRB_DISABLE_SLOT: >> +case TRB_ADDR_DEV: >> +case TRB_CONFIG_EP: >> +case TRB_EVAL_CONTEXT: >> +case TRB_RESET_EP: >> +case TRB_STOP_RING: >> +case TRB_SET_DEQ: >> +case TRB_RESET_DEV: >> +case TRB_FORCE_EVENT: >> +case TRB_NEG_BANDWIDTH: >> +case TRB_SET_LT: >> +case TRB_GET_BW: >> +case TRB_FORCE_HEADER: > > How about merging > > + case TRB_ENABLE_SLOT: > + case TRB_DISABLE_SLOT: > + case TRB_ADDR_DEV: > + case TRB_CONFIG_EP: > + case TRB_EVAL_CONTEXT: > + case TRB_RESET_EP: > + case TRB_STOP_RING: > + case TRB_SET_DEQ: > + case TRB_RESET_DEV: > + case TRB_FORCE_EVENT: > + case TRB_NEG_BANDWIDTH: > + case TRB_SET_LT: > + case TRB_GET_BW: > + case TRB_FORCE_HEADER: > > into > > > +case TRB_ENABLE_SLOT ... TRB_FORCE_HEADER: > > ? to me it's pointless obfuscation :-) > > >> +trb->generic.field[0] = 0; >> +trb->generic.field[1] = 0; >> +trb->generic.field[2] = 0; >> +/* Preserve only the cycle bit of this TRB */ >> +trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE); >> +trb->generic.field[3] = cpu_to_le32(TRB_TYPE(TRB_CMD_NOOP)); >> +break; >> default: >> /* nothing */ > > Need a warning? We have two users for this function and we know we won't pass unexistent TRB types to it. Maybe we can defer that warning until a problem shows up? -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html