Re: JMS56x not working reliably with uas driver

2016-12-29 Thread Oliver Neukum
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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()

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Felipe Balbi
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

2016-12-29 Thread Rafał Miłecki
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

2016-12-29 Thread Rafał Miłecki
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

2016-12-29 Thread Rafał Miłecki
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

2016-12-29 Thread Jérémy Lefaure
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()

2016-12-29 Thread Felix Hädicke
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

2016-12-29 Thread Lu Baolu
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

2016-12-29 Thread Lu Baolu
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()

2016-12-29 Thread Lu Baolu
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

2016-12-29 Thread Lu Baolu
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Lu Baolu
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

2016-12-29 Thread kbuild test robot
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

2016-12-29 Thread kbuild test robot
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

2016-12-29 Thread Chanwoo Choi
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

2016-12-29 Thread Felipe Balbi

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

2016-12-29 Thread Felipe Balbi

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

2016-12-29 Thread Felipe Balbi

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()

2016-12-29 Thread Felipe Balbi

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