Re: [RFC 03/21] xhci: fix incorrect type in assignment in xhci_address_device()
On 11/14/2013 10:52 PM, Sarah Sharp wrote: Hi Xenia, This patch doesn't apply any more either, could you resend when you have time? Thanks, Sarah Sharp Yes, sure! Do you want me to resend all the patch series or just the individual patches? regards, ksenia On Mon, Sep 09, 2013 at 09:03:08PM +0300, Xenia Ragiadakou wrote: The field 'dev_info' in struct xhci_slot_ctx has type __le32 and it needs to be converted to CPU byteorder for the correct retrieval of its subfield 'Context Entries'. This field is used by the trace event 'xhci_address_ctx' to trace only the contexts of valid endpoints. This bug was found using sparse. Signed-off-by: Xenia Ragiadakou --- drivers/usb/host/xhci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index bda0cdf..9f22ddf 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3762,7 +3762,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) xhci_dbg(xhci, "Slot ID %d Input Context:\n", udev->slot_id); xhci_dbg_ctx(xhci, virt_dev->in_ctx, 2); trace_xhci_address_ctx(xhci, virt_dev->in_ctx, - slot_ctx->dev_info >> 27); + le32_to_cpu(slot_ctx->dev_info) >> 27); spin_lock_irqsave(&xhci->lock, flags); cmd_trb = xhci->cmd_ring->dequeue; @@ -3841,7 +3841,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) xhci_dbg(xhci, "Slot ID %d Input Context:\n", udev->slot_id); xhci_dbg_ctx(xhci, virt_dev->in_ctx, 2); trace_xhci_address_ctx(xhci, virt_dev->in_ctx, - slot_ctx->dev_info >> 27); + le32_to_cpu(slot_ctx->dev_info) >> 27); xhci_dbg(xhci, "Slot ID %d Output Context:\n", udev->slot_id); xhci_dbg_ctx(xhci, virt_dev->out_ctx, 2); /* @@ -3850,7 +3850,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) */ slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->out_ctx); trace_xhci_address_ctx(xhci, virt_dev->out_ctx, - slot_ctx->dev_info >> 27); + le32_to_cpu(slot_ctx->dev_info) >> 27); /* Use kernel assigned address for devices; store xHC assigned * address locally. */ virt_dev->address = (le32_to_cpu(slot_ctx->dev_state) & DEV_ADDR_MASK) -- 1.8.3.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
[RFC v2] xhci: fix incorrect type in assignment in xhci_address_device()
The field 'dev_info' in struct xhci_slot_ctx has type __le32 and it needs to be converted to CPU byteorder for the correct retrieval of its subfield 'Context Entries'. This field is used by the trace event 'xhci_address_ctx' to trace only the contexts of valid endpoints. This bug was found using sparse. Signed-off-by: Xenia Ragiadakou --- Changes from v1: * fix patch to apply cleanly on for-usb-next-queue branch of xhci tree drivers/usb/host/xhci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 1d26b3f..900ba36 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3771,7 +3771,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) xhci_dbg(xhci, "Slot ID %d Input Context:\n", udev->slot_id); xhci_dbg_ctx(xhci, virt_dev->in_ctx, 2); trace_xhci_address_ctx(xhci, virt_dev->in_ctx, - slot_ctx->dev_info >> 27); + le32_to_cpu(slot_ctx->dev_info) >> 27); spin_lock_irqsave(&xhci->lock, flags); cmd_trb = xhci_find_next_enqueue(xhci->cmd_ring); @@ -3850,7 +3850,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) xhci_dbg(xhci, "Slot ID %d Input Context:\n", udev->slot_id); xhci_dbg_ctx(xhci, virt_dev->in_ctx, 2); trace_xhci_address_ctx(xhci, virt_dev->in_ctx, - slot_ctx->dev_info >> 27); + le32_to_cpu(slot_ctx->dev_info) >> 27); xhci_dbg(xhci, "Slot ID %d Output Context:\n", udev->slot_id); xhci_dbg_ctx(xhci, virt_dev->out_ctx, 2); /* @@ -3859,7 +3859,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) */ slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->out_ctx); trace_xhci_address_ctx(xhci, virt_dev->out_ctx, - slot_ctx->dev_info >> 27); + le32_to_cpu(slot_ctx->dev_info) >> 27); /* Zero the input context control for later use */ ctrl_ctx->add_flags = 0; ctrl_ctx->drop_flags = 0; -- 1.8.3.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
[RFC v2] xhci: remove conversion from generic to pci device in xhci_mem.c
This patch removes the to_pci_dev() conversion performed to generic struct
device since it is not actually useful (the pointer to the generic device
can be used directly rather through a conversion to pci_dev) and it is pci
bus specific.
Signed-off-by: Xenia Ragiadakou
---
Changes from v1:
* fix patch to apply cleanly on for-usb-next-queue branch of xhci tree
drivers/usb/host/xhci-mem.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 1445e08..99e7251 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -433,10 +433,10 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
{
- struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+ struct device *dev = xhci_to_hcd(xhci)->self.controller;
if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
- dma_free_coherent(&pdev->dev,
+ dma_free_coherent(dev,
sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
stream_ctx, dma);
else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
@@ -461,10 +461,10 @@ static struct xhci_stream_ctx
*xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs, dma_addr_t *dma,
gfp_t mem_flags)
{
- struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+ struct device *dev = xhci_to_hcd(xhci)->self.controller;
if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
- return dma_alloc_coherent(&pdev->dev,
+ return dma_alloc_coherent(dev,
sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
dma, mem_flags);
else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
@@ -1616,7 +1616,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
{
int num_sp;
int i;
- struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+ struct device *dev = xhci_to_hcd(xhci)->self.controller;
if (!xhci->scratchpad)
return;
@@ -1624,13 +1624,13 @@ static void scratchpad_free(struct xhci_hcd *xhci)
num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
for (i = 0; i < num_sp; i++) {
- dma_free_coherent(&pdev->dev, xhci->page_size,
+ dma_free_coherent(dev, xhci->page_size,
xhci->scratchpad->sp_buffers[i],
xhci->scratchpad->sp_dma_buffers[i]);
}
kfree(xhci->scratchpad->sp_dma_buffers);
kfree(xhci->scratchpad->sp_buffers);
- dma_free_coherent(&pdev->dev, num_sp * sizeof(u64),
+ dma_free_coherent(dev, num_sp * sizeof(u64),
xhci->scratchpad->sp_array,
xhci->scratchpad->sp_dma);
kfree(xhci->scratchpad);
@@ -1692,7 +1692,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
void xhci_mem_cleanup(struct xhci_hcd *xhci)
{
- struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+ struct device *dev = xhci_to_hcd(xhci)->self.controller;
struct xhci_cd *cur_cd, *next_cd;
int size;
int i, j, num_ports;
@@ -1700,7 +1700,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
/* Free the Event Ring Segment Table and the actual Event Ring */
size = sizeof(struct xhci_erst_entry)*(xhci->erst.num_entries);
if (xhci->erst.entries)
- dma_free_coherent(&pdev->dev, size,
+ dma_free_coherent(dev, size,
xhci->erst.entries, xhci->erst.erst_dma_addr);
xhci->erst.entries = NULL;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST");
@@ -1748,7 +1748,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
"Freed medium stream array pool");
if (xhci->dcbaa)
- dma_free_coherent(&pdev->dev, sizeof(*xhci->dcbaa),
+ dma_free_coherent(dev, sizeof(*xhci->dcbaa),
xhci->dcbaa, xhci->dcbaa->dma);
xhci->dcbaa = NULL;
--
1.8.3.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
[RFC v2 1/4] xhci: replace xhci_readl() with readl()
Function xhci_readl() is used to read 32bit xHC registers residing in MMIO address space. It takes as first argument a pointer to the xhci_hcd although it does not use it. xhci_readl() internally simply calls readl(). This creates an illusion that xhci_readl() is an xhci specific function that has to be called in a context where a pointer to xhci_hcd is available. Remove the unnecessary xhci_readl() wrapper function and replace its calls to with calls to readl() to make the code more straightforward. Signed-off-by: Xenia Ragiadakou --- drivers/usb/host/xhci-dbg.c | 36 +-- drivers/usb/host/xhci-hub.c | 71 ++-- drivers/usb/host/xhci-mem.c | 20 +-- drivers/usb/host/xhci-ring.c | 12 +++ drivers/usb/host/xhci.c | 86 ++-- drivers/usb/host/xhci.h | 5 --- 6 files changed, 112 insertions(+), 118 deletions(-) diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c index 73503a8..eb009a4 100644 --- a/drivers/usb/host/xhci-dbg.c +++ b/drivers/usb/host/xhci-dbg.c @@ -32,7 +32,7 @@ void xhci_dbg_regs(struct xhci_hcd *xhci) xhci_dbg(xhci, "// xHCI capability registers at %p:\n", xhci->cap_regs); - temp = xhci_readl(xhci, &xhci->cap_regs->hc_capbase); + temp = readl(&xhci->cap_regs->hc_capbase); xhci_dbg(xhci, "// @%p = 0x%x (CAPLENGTH AND HCIVERSION)\n", &xhci->cap_regs->hc_capbase, temp); xhci_dbg(xhci, "// CAPLENGTH: 0x%x\n", @@ -44,13 +44,13 @@ void xhci_dbg_regs(struct xhci_hcd *xhci) xhci_dbg(xhci, "// xHCI operational registers at %p:\n", xhci->op_regs); - temp = xhci_readl(xhci, &xhci->cap_regs->run_regs_off); + temp = readl(&xhci->cap_regs->run_regs_off); xhci_dbg(xhci, "// @%p = 0x%x RTSOFF\n", &xhci->cap_regs->run_regs_off, (unsigned int) temp & RTSOFF_MASK); xhci_dbg(xhci, "// xHCI runtime registers at %p:\n", xhci->run_regs); - temp = xhci_readl(xhci, &xhci->cap_regs->db_off); + temp = readl(&xhci->cap_regs->db_off); xhci_dbg(xhci, "// @%p = 0x%x DBOFF\n", &xhci->cap_regs->db_off, temp); xhci_dbg(xhci, "// Doorbell array at %p:\n", xhci->dba); } @@ -61,7 +61,7 @@ static void xhci_print_cap_regs(struct xhci_hcd *xhci) xhci_dbg(xhci, "xHCI capability registers at %p:\n", xhci->cap_regs); - temp = xhci_readl(xhci, &xhci->cap_regs->hc_capbase); + temp = readl(&xhci->cap_regs->hc_capbase); xhci_dbg(xhci, "CAPLENGTH AND HCIVERSION 0x%x:\n", (unsigned int) temp); xhci_dbg(xhci, "CAPLENGTH: 0x%x\n", @@ -69,7 +69,7 @@ static void xhci_print_cap_regs(struct xhci_hcd *xhci) xhci_dbg(xhci, "HCIVERSION: 0x%x\n", (unsigned int) HC_VERSION(temp)); - temp = xhci_readl(xhci, &xhci->cap_regs->hcs_params1); + temp = readl(&xhci->cap_regs->hcs_params1); xhci_dbg(xhci, "HCSPARAMS 1: 0x%x\n", (unsigned int) temp); xhci_dbg(xhci, " Max device slots: %u\n", @@ -79,7 +79,7 @@ static void xhci_print_cap_regs(struct xhci_hcd *xhci) xhci_dbg(xhci, " Max ports: %u\n", (unsigned int) HCS_MAX_PORTS(temp)); - temp = xhci_readl(xhci, &xhci->cap_regs->hcs_params2); + temp = readl(&xhci->cap_regs->hcs_params2); xhci_dbg(xhci, "HCSPARAMS 2: 0x%x\n", (unsigned int) temp); xhci_dbg(xhci, " Isoc scheduling threshold: %u\n", @@ -87,7 +87,7 @@ static void xhci_print_cap_regs(struct xhci_hcd *xhci) xhci_dbg(xhci, " Maximum allowed segments in event ring: %u\n", (unsigned int) HCS_ERST_MAX(temp)); - temp = xhci_readl(xhci, &xhci->cap_regs->hcs_params3); + temp = readl(&xhci->cap_regs->hcs_params3); xhci_dbg(xhci, "HCSPARAMS 3 0x%x:\n", (unsigned int) temp); xhci_dbg(xhci, " Worst case U1 device exit latency: %u\n", @@ -95,14 +95,14 @@ static void xhci_print_cap_regs(struct xhci_hcd *xhci) xhci_dbg(xhci, " Worst case U2 device exit latency: %u\n", (unsigned int) HCS_U2_LATENCY(temp)); - temp = xhci_readl(xhci, &xhci->cap_regs->hcc_params); + temp = readl(&xhci->cap_regs->hcc_params); xhci_dbg(xhci, "HCC PARAMS 0x%x:\n", (unsigned int) temp); xhci_dbg(xhci, " HC generates %s bit addresses\n",
[RFC v2 2/4] xhci: replace xhci_writel() with writel()
Function xhci_writel() is used to write a 32bit value in xHC registers residing
in MMIO address space. It takes as first argument a pointer to the xhci_hcd
although it does not use it. xhci_writel() internally simply calls writel().
This creates an illusion that xhci_writel() is an xhci specific function that
has to be called in a context where a pointer to xhci_hcd is available.
Remove xhci_writel() wrapper function and replace its calls with calls to
writel() to make the code more straight-forward.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-hub.c | 35 ++
drivers/usb/host/xhci-mem.c | 6 +++---
drivers/usb/host/xhci-ring.c | 8 +++
drivers/usb/host/xhci.c | 51 +---
drivers/usb/host/xhci.h | 8 ---
5 files changed, 47 insertions(+), 61 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 70ed7c9..9992fbf 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -342,7 +342,7 @@ static void xhci_disable_port(struct usb_hcd *hcd, struct
xhci_hcd *xhci,
}
/* Write 1 to disable the port */
- xhci_writel(xhci, port_status | PORT_PE, addr);
+ writel(port_status | PORT_PE, addr);
port_status = readl(addr);
xhci_dbg(xhci, "disable port, actual port %d status = 0x%x\n",
wIndex, port_status);
@@ -388,7 +388,7 @@ static void xhci_clear_port_change_bit(struct xhci_hcd
*xhci, u16 wValue,
return;
}
/* Change bits are all write 1 to clear */
- xhci_writel(xhci, port_status | status, addr);
+ writel(port_status | status, addr);
port_status = readl(addr);
xhci_dbg(xhci, "clear port %s change, actual port %d status = 0x%x\n",
port_change_bit, wIndex, port_status);
@@ -419,7 +419,7 @@ void xhci_set_link_state(struct xhci_hcd *xhci, __le32
__iomem **port_array,
temp = xhci_port_state_to_neutral(temp);
temp &= ~PORT_PLS_MASK;
temp |= PORT_LINK_STROBE | link_state;
- xhci_writel(xhci, temp, port_array[port_id]);
+ writel(temp, port_array[port_id]);
}
static void xhci_set_remote_wake_mask(struct xhci_hcd *xhci,
@@ -445,7 +445,7 @@ static void xhci_set_remote_wake_mask(struct xhci_hcd *xhci,
else
temp &= ~PORT_WKOC_E;
- xhci_writel(xhci, temp, port_array[port_id]);
+ writel(temp, port_array[port_id]);
}
/* Test and clear port RWC bit */
@@ -458,7 +458,7 @@ void xhci_test_and_clear_bit(struct xhci_hcd *xhci, __le32
__iomem **port_array,
if (temp & port_bit) {
temp = xhci_port_state_to_neutral(temp);
temp |= port_bit;
- xhci_writel(xhci, temp, port_array[port_id]);
+ writel(temp, port_array[port_id]);
}
}
@@ -838,8 +838,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
temp |= PORT_CSC | PORT_PEC | PORT_WRC |
PORT_OCC | PORT_RC | PORT_PLC |
PORT_CEC;
- xhci_writel(xhci, temp | PORT_PE,
- port_array[wIndex]);
+ writel(temp | PORT_PE, port_array[wIndex]);
temp = readl(port_array[wIndex]);
break;
}
@@ -894,8 +893,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
* However, khubd will ignore the roothub events until
* the roothub is registered.
*/
- xhci_writel(xhci, temp | PORT_POWER,
- port_array[wIndex]);
+ writel(temp | PORT_POWER, port_array[wIndex]);
temp = readl(port_array[wIndex]);
xhci_dbg(xhci, "set port power, actual port %d status
= 0x%x\n", wIndex, temp);
@@ -910,7 +908,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
break;
case USB_PORT_FEAT_RESET:
temp = (temp | PORT_RESET);
- xhci_writel(xhci, temp, port_array[wIndex]);
+ writel(temp, port_array[wIndex]);
temp = readl(port_array[wIndex]);
xhci_dbg(xhci, "set port reset, actual port %d status
= 0x%x\n", wIndex, temp);
@@ -925,7 +923,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
break;
case USB_PORT_FEAT_BH_PORT_RESET:
temp |= PORT_WR;
- xhci_writel(xhci, temp, port_array[wIndex]);
+
[RFC v2 3/4] xhci: replace xhci_read_64() with readq()
Function xhci_read_64() is used to read 64bit xHC registers residing in MMIO.
On 32bit systems, xHC registers need to be read with 32bit accesses by
reading first the lower 32bits and then the higher 32bits.
Replace all calls to xhci_read_64() with calls to readq() and include
asm-generic/io-64-nonatomic-lo-hi.h header file, so that if the system
is not 64bit, readq() will read registers in 32bit chunks with low-high order.
This is done to reduce code duplication since 64bit low-high read logic
is already implemented and to take advantage of inherent "atomic" 64bit
read operations on 64bit systems.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-dbg.c | 6 +++---
drivers/usb/host/xhci-mem.c | 6 +++---
drivers/usb/host/xhci-ring.c | 6 +++---
drivers/usb/host/xhci.c | 12 ++--
drivers/usb/host/xhci.h | 10 ++
5 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index eb009a4..b016d38 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -203,12 +203,12 @@ void xhci_print_ir_set(struct xhci_hcd *xhci, int set_num)
addr, (unsigned int)temp);
addr = &ir_set->erst_base;
- temp_64 = xhci_read_64(xhci, addr);
+ temp_64 = readq(addr);
xhci_dbg(xhci, " %p: ir_set.erst_base = @%08llx\n",
addr, temp_64);
addr = &ir_set->erst_dequeue;
- temp_64 = xhci_read_64(xhci, addr);
+ temp_64 = readq(addr);
xhci_dbg(xhci, " %p: ir_set.erst_dequeue = @%08llx\n",
addr, temp_64);
}
@@ -412,7 +412,7 @@ void xhci_dbg_cmd_ptrs(struct xhci_hcd *xhci)
{
u64 val;
- val = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
+ val = readq(&xhci->op_regs->cmd_ring);
xhci_dbg(xhci, "// xHC command ring deq ptr low bits + flags = @%08x\n",
lower_32_bits(val));
xhci_dbg(xhci, "// xHC command ring deq ptr high bits = @%08x\n",
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index bce4391..4b87026 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1958,7 +1958,7 @@ static void xhci_set_hc_event_deq(struct xhci_hcd *xhci)
xhci_warn(xhci, "WARN something wrong with SW event ring "
"dequeue ptr.\n");
/* Update HC event ring dequeue pointer */
- temp = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue);
+ temp = readq(&xhci->ir_set->erst_dequeue);
temp &= ERST_PTR_MASK;
/* Don't clear the EHB bit (which is RW1C) because
* there might be more events to service.
@@ -2312,7 +2312,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
(unsigned long long)xhci->cmd_ring->first_seg->dma);
/* Set the address in the Command Ring Control register */
- val_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
+ val_64 = readq(&xhci->op_regs->cmd_ring);
val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
(xhci->cmd_ring->first_seg->dma & (u64) ~CMD_RING_RSVD_BITS) |
xhci->cmd_ring->cycle_state;
@@ -2396,7 +2396,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"// Set ERST base address for ir_set 0 = 0x%llx",
(unsigned long long)xhci->erst.erst_dma_addr);
- val_64 = xhci_read_64(xhci, &xhci->ir_set->erst_base);
+ val_64 = readq(&xhci->ir_set->erst_base);
val_64 &= ERST_PTR_MASK;
val_64 |= (xhci->erst.erst_dma_addr & (u64) ~ERST_PTR_MASK);
xhci_write_64(xhci, val_64, &xhci->ir_set->erst_base);
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index bc46cce..339733b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -313,7 +313,7 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
return 0;
}
- temp_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
+ temp_64 = readq(&xhci->op_regs->cmd_ring);
if (!(temp_64 & CMD_RING_RUNNING)) {
xhci_dbg(xhci, "Command ring had been stopped\n");
return 0;
@@ -2871,7 +2871,7 @@ hw_died:
/* Clear the event handler busy flag (RW1C);
* the event ring should be empty.
*/
- temp_64 = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue);
+ temp_64 = readq(&xhci->ir_set->erst_dequeue);
xhci_write_64(xhci, temp_64 | ERST_EHB,
[RFC v2 0/4] remove unnecessary code related to MMIO reads/writes
Changes from v1: * create one patch per type of conversion instead of multiple patches associated with specific xhci files * change commit messages to be more readable and to reflect the above change * add an empty line between #include and the rest of the headers to improve readability. Xenia Ragiadakou (4): xhci: replace xhci_readl() with readl() xhci: replace xhci_writel() with writel() xhci: replace xhci_read_64() with readq() xhci: replace xhci_write_64() with writeq() drivers/usb/host/xhci-dbg.c | 42 ++-- drivers/usb/host/xhci-hub.c | 106 ++--- drivers/usb/host/xhci-mem.c | 40 +-- drivers/usb/host/xhci-ring.c | 34 +- drivers/usb/host/xhci.c | 157 +-- drivers/usb/host/xhci.h | 52 +++--- 6 files changed, 196 insertions(+), 235 deletions(-) -- 1.8.3.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
[RFC v2 4/4] xhci: replace xhci_write_64() with writeq()
Function xhci_write_64() is used to write 64bit xHC registers residing in MMIO.
On 32bit systems, xHC registers need to be written with 32bit accesses by
writing first the lower 32bits and then the higher 32bits. The header file
asm-generic/io-64-nonatomic-lo-hi.h ensures that on 32bit systems writeq() will
will write 64bit registers in 32bit chunks with low-high order.
Replace all calls to xhci_write_64() with calls to writeq().
This is done to reduce code duplication since 64bit low-high write logic
is already implemented and to take advantage of inherent "atomic" 64bit
write operations on 64bit systems.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-mem.c | 8
drivers/usb/host/xhci-ring.c | 8 +++-
drivers/usb/host/xhci.c | 8
drivers/usb/host/xhci.h | 29 +
4 files changed, 20 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 4b87026..873c272 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1967,7 +1967,7 @@ static void xhci_set_hc_event_deq(struct xhci_hcd *xhci)
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"// Write event ring dequeue pointer, "
"preserving EHB bit");
- xhci_write_64(xhci, ((u64) deq & (u64) ~ERST_PTR_MASK) | temp,
+ writeq(((u64) deq & (u64) ~ERST_PTR_MASK) | temp,
&xhci->ir_set->erst_dequeue);
}
@@ -2269,7 +2269,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
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);
+ writeq(dma, &xhci->op_regs->dcbaa_ptr);
/*
* Initialize the ring segment pool. The ring must be a contiguous
@@ -2318,7 +2318,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
xhci->cmd_ring->cycle_state;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"// Setting command ring address to 0x%x", val);
- xhci_write_64(xhci, val_64, &xhci->op_regs->cmd_ring);
+ writeq(val_64, &xhci->op_regs->cmd_ring);
xhci_dbg_cmd_ptrs(xhci);
xhci->lpm_command = xhci_alloc_command(xhci, true, true, flags);
@@ -2399,7 +2399,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
val_64 = readq(&xhci->ir_set->erst_base);
val_64 &= ERST_PTR_MASK;
val_64 |= (xhci->erst.erst_dma_addr & (u64) ~ERST_PTR_MASK);
- xhci_write_64(xhci, val_64, &xhci->ir_set->erst_base);
+ writeq(val_64, &xhci->ir_set->erst_base);
/* Set the event ring dequeue address */
xhci_set_hc_event_deq(xhci);
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 339733b..fe9208a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -319,8 +319,7 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
return 0;
}
xhci->cmd_ring_state = CMD_RING_STATE_ABORTED;
- xhci_write_64(xhci, temp_64 | CMD_RING_ABORT,
- &xhci->op_regs->cmd_ring);
+ writeq(temp_64 | CMD_RING_ABORT, &xhci->op_regs->cmd_ring);
/* Section 4.6.1.2 of xHCI 1.0 spec says software should
* time the completion od all xHCI commands, including
@@ -2872,8 +2871,7 @@ hw_died:
* the event ring should be empty.
*/
temp_64 = readq(&xhci->ir_set->erst_dequeue);
- xhci_write_64(xhci, temp_64 | ERST_EHB,
- &xhci->ir_set->erst_dequeue);
+ writeq(temp_64 | ERST_EHB, &xhci->ir_set->erst_dequeue);
spin_unlock(&xhci->lock);
return IRQ_HANDLED;
@@ -2900,7 +2898,7 @@ hw_died:
/* Clear the event handler busy flag (RW1C); event ring is empty. */
temp_64 |= ERST_EHB;
- xhci_write_64(xhci, temp_64, &xhci->ir_set->erst_dequeue);
+ writeq(temp_64, &xhci->ir_set->erst_dequeue);
spin_unlock(&xhci->lock);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 858e992..7fe6f66 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -762,11 +762,11 @@ static void xhci_restore_registers(struct xhci_hcd *xhci)
{
writel(xhci->s3.command, &xhci->op_regs->command);
writel(xhci->s3.dev_nt, &xhci->op_regs->dev_notification);
- xhci_write_64(xhci, xhci->s3.dcbaa_ptr, &xhci->op_regs->dcbaa_ptr);
+
Re: [PATCH 1/2] xhci: fix reset for not halted endpoints
Hi Alan and Sarah, Sorry for my late response but i had some personal issues that held me back for a while. My comments follow below. On 10/17/2013 06:11 PM, Alan Stern wrote: On Wed, 16 Oct 2013, Sarah Sharp wrote: I think there's some nasty race conditions here. There's several different structures this code and other functions are manipulating: - the endpoint ring contents - the endpoint ring dequeue pointer - the endpoint's cur_td and cancelled_td lists The other functions you need to look at are xhci_urb_enqueue, xhci_urb_dequeue, and xhci_handle_cmd_stop_ep. Arguably, we should be doing something about drivers attempting to change alternate interface settings at the same time they're resetting endpoints, but I think the solution should be that drivers just shouldn't do that. However, we do need to handle the case where the reset endpoint races with URB cancellation and enqueue. You can simplify part of the problem by not allowing an endpoint to be reset if it has any pending URBs. Just fail the reset in this case. Yes, you are right since, from what i understand, it is the responsibility of the device driver to unlink any pending URBs before issuing a clear halt to the endpoint. So we expect the device driver to call usb_unlink_urb() for each pending URB, which in turn calls xhci_urb_dequeue(). The function xhci_urb_dequeue() will add the URB to endpoint's cancelled_td_list and will issue a Stop Endpoint command. The completion handler of the Stop Endpoint command, xhci_handle_cmd_stop_ep(), will eventually remove the unlinked URBs from endpoint's both cancelled_td_list and td_list and give them back to core. However, since xhci_urb_dequeue() does not wait on the completion of the Stop Endpoint command, it is still possible for the URB not to have been removed yet from the td_list when the xhci_reset_endpoint() is called, right? In that case, is it better to check if td_list is not empty and fail the reset after xhci_stop_endpoint_for_config() rather than before? Also, while you're going through the whole remove-and-add procedure for endpoints that aren't halted, do you want to hold the bandwidth mutex? If the procedure isn't atomic, there's a possibility that some other device could change configs in the middle. This would also prevent concurrent altsetting changes. Yes, i need to lock the bandwidth_mutex. I will fix this. Alan Stern Also, regarding the case that there is an attempt to enqueue a URB while the endpoint is being reset, is it ok to fail the enqueue by adding a check in prepare_ring() to fail when the endpoint's state is EP_CONFIG_PENDING ? regards, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] xhci: fix reset for not halted endpoints
On 11/25/2013 01:50 AM, Xenia Ragiadakou wrote: [snip] Also, while you're going through the whole remove-and-add procedure for endpoints that aren't halted, do you want to hold the bandwidth mutex? If the procedure isn't atomic, there's a possibility that some other device could change configs in the middle. This would also prevent concurrent altsetting changes. Yes, i need to lock the bandwidth_mutex. I will fix this. Alan Stern After i looked at it more carefully, there will be a deadlock if i try to aqcuire the bandwidth_mutex in reset endpoint code. In usb_set_configuration(), the bandwidth_mutex is held while usb_enable_interface() is called for each interface and usb_enable_interface() will attempt to reset the endpoints via usb_enable_endpoint() since the 3rd argument is set to true. So, I think it's not a good idea to try to lock the bandwidth_mutex given that, right? ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] xhci: fix reset for not halted endpoints
On 11/25/2013 04:41 AM, Alan Stern wrote: On Mon, 25 Nov 2013, Xenia Ragiadakou wrote: You can simplify part of the problem by not allowing an endpoint to be reset if it has any pending URBs. Just fail the reset in this case. Yes, you are right since, from what i understand, it is the responsibility of the device driver to unlink any pending URBs before issuing a clear halt to the endpoint. So we expect the device driver to call usb_unlink_urb() for each pending URB, which in turn calls Actually you should expect the driver to call usb_kill_urb(), not usb_unlink_urb(). The difference is that usb_kill_urb() waits for the URB to complete. xhci_urb_dequeue(). The function xhci_urb_dequeue() will add the URB to endpoint's cancelled_td_list and will issue a Stop Endpoint command. The completion handler of the Stop Endpoint command, xhci_handle_cmd_stop_ep(), will eventually remove the unlinked URBs from endpoint's both cancelled_td_list and td_list and give them back to core. However, since xhci_urb_dequeue() does not wait on the completion of the Stop Endpoint command, it is still possible for the URB not to have been removed yet from the td_list when the xhci_reset_endpoint() is called, right? Not if the driver uses usb_kill_urb(). In that case the URB will already be removed from the td_list. In that case, is it better to check if td_list is not empty and fail the reset after xhci_stop_endpoint_for_config() rather than before? It probably doesn't matter. Also, regarding the case that there is an attempt to enqueue a URB while the endpoint is being reset, is it ok to fail the enqueue by adding a check in prepare_ring() to fail when the endpoint's state is EP_CONFIG_PENDING ? That's up to Sarah (but I can't think of any alternative). If you decide to do this, add an explanation of the new error code to Documentation/usb/error-codes.txt. Alan Stern Thanks Alan for your explanation, i understand better now how to handle this. regards, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] xhci: fix reset for not halted endpoints
On 11/25/2013 04:44 AM, Alan Stern wrote: On Mon, 25 Nov 2013, Xenia Ragiadakou wrote: On 11/25/2013 01:50 AM, Xenia Ragiadakou wrote: [snip] Also, while you're going through the whole remove-and-add procedure for endpoints that aren't halted, do you want to hold the bandwidth mutex? If the procedure isn't atomic, there's a possibility that some other device could change configs in the middle. This would also prevent concurrent altsetting changes. Yes, i need to lock the bandwidth_mutex. I will fix this. Alan Stern After i looked at it more carefully, there will be a deadlock if i try to aqcuire the bandwidth_mutex in reset endpoint code. In usb_set_configuration(), the bandwidth_mutex is held while usb_enable_interface() is called for each interface and usb_enable_interface() will attempt to reset the endpoints via usb_enable_endpoint() since the 3rd argument is set to true. So, I think it's not a good idea to try to lock the bandwidth_mutex given that, right? Right. We don't have to worry about config changes anyway, because the config can't be changed until all the old endpoints are disabled. You just have to make sure that an endpoint can't be disabled until a pending reset is finished. Alan Stern Actually, xhci driver does not implement endpoint_disable(). So, how can I force the disable not to be finished until the pending reset is finished? The only way i see is by implementing endpoint_disable() callback for xhci. Do you have something else in mind maybe? Also, probably there was an implementation for it, xhci_endpoint_disable(), but it seems to have been reverted. I just saw this function name to appear in a comment in xhci_add_endpoint(). thanks, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 1/3] xhci: trace debug statements for urb cancellation
This patch defines a new trace event, which is called xhci_dbg_cancel_urb and belongs to the event class xhci_log_msg, and adds tracepoints that trace the debug messages related to the removal of a cancelled URB from the endpoint's transfer ring. Signed-off-by: Xenia Ragiadakou --- drivers/usb/host/xhci-ring.c | 64 +++ drivers/usb/host/xhci-trace.h | 5 drivers/usb/host/xhci.c | 13 + 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0bb275e..3a2277b 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -556,7 +556,8 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, return; } state->new_cycle_state = 0; - xhci_dbg(xhci, "Finding segment containing stopped TRB.\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Finding segment containing stopped TRB."); state->new_deq_seg = find_trb_seg(cur_td->start_seg, dev->eps[ep_index].stopped_trb, &state->new_cycle_state); @@ -566,12 +567,14 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, } /* Dig out the cycle state saved by the xHC during the stop ep cmd */ - xhci_dbg(xhci, "Finding endpoint context\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Finding endpoint context"); ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index); state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq); state->new_deq_ptr = cur_td->last_trb; - xhci_dbg(xhci, "Finding segment containing last TRB in TD.\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Finding segment containing last TRB in TD."); state->new_deq_seg = find_trb_seg(state->new_deq_seg, state->new_deq_ptr, &state->new_cycle_state); @@ -598,13 +601,16 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, if (ep_ring->first_seg == ep_ring->first_seg->next && state->new_deq_ptr < dev->eps[ep_index].stopped_trb) state->new_cycle_state ^= 0x1; - xhci_dbg(xhci, "Cycle state = 0x%x\n", state->new_cycle_state); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Cycle state = 0x%x", state->new_cycle_state); /* Don't update the ring cycle state for the producer (us). */ - xhci_dbg(xhci, "New dequeue segment = %p (virtual)\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "New dequeue segment = %p (virtual)", state->new_deq_seg); addr = xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr); - xhci_dbg(xhci, "New dequeue pointer = 0x%llx (DMA)\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "New dequeue pointer = 0x%llx (DMA)", (unsigned long long) addr); } @@ -632,9 +638,11 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, if (flip_cycle) cur_trb->generic.field[3] ^= cpu_to_le32(TRB_CYCLE); - xhci_dbg(xhci, "Cancel (unchain) link TRB\n"); - xhci_dbg(xhci, "Address = %p (0x%llx dma); " - "in seg %p (0x%llx dma)\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Cancel (unchain) link TRB"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Address = %p (0x%llx dma); " + "in seg %p (0x%llx dma)", cur_trb, (unsigned long long)xhci_trb_virt_to_dma(cur_seg, cur_trb), cur_seg, @@ -652,7 +660,8 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, cpu_to_le32(TRB_CYCLE); cur_trb->generic.field[3] |= cpu_to_le32( TRB_TYPE(TRB_TR_NOOP)); - xhci_dbg(xhci, "TRB to noop at offset 0x%llx\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "TRB to noop at offset 0x%llx", (
[RFC 2/3] xhci: trace debug messages related to driver initialization and unload
This patch defines a new trace event, which is called xhci_dbg_init
and belongs to the event class xhci_log_msg, and adds tracepoints that
trace the debug statements in the functions used to start and stop the
xhci-hcd driver.
Also, it removes an unnecessary cast of variable val to unsigned int
in xhci_mem_init(), since val is already declared as unsigned int.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-mem.c | 110 ++
drivers/usb/host/xhci-trace.h | 5 ++
drivers/usb/host/xhci.c | 66 +++--
3 files changed, 113 insertions(+), 68 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b1bb59b..ef27470 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1545,7 +1545,8 @@ static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t
flags)
struct device *dev = xhci_to_hcd(xhci)->self.controller;
int num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
- xhci_dbg(xhci, "Allocating %d scratchpad buffers\n", num_sp);
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+ "Allocating %d scratchpad buffers", num_sp);
if (!num_sp)
return 0;
@@ -1702,11 +1703,11 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
dma_free_coherent(&pdev->dev, size,
xhci->erst.entries, xhci->erst.erst_dma_addr);
xhci->erst.entries = NULL;
- xhci_dbg(xhci, "Freed ERST\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST");
if (xhci->event_ring)
xhci_ring_free(xhci, xhci->event_ring);
xhci->event_ring = NULL;
- xhci_dbg(xhci, "Freed event ring\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed event ring");
if (xhci->lpm_command)
xhci_free_command(xhci, xhci->lpm_command);
@@ -1714,7 +1715,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
if (xhci->cmd_ring)
xhci_ring_free(xhci, xhci->cmd_ring);
xhci->cmd_ring = NULL;
- xhci_dbg(xhci, "Freed command ring\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed command ring");
list_for_each_entry_safe(cur_cd, next_cd,
&xhci->cancel_cmd_list, cancel_cmd_list) {
list_del(&cur_cd->cancel_cmd_list);
@@ -1727,22 +1728,24 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
if (xhci->segment_pool)
dma_pool_destroy(xhci->segment_pool);
xhci->segment_pool = NULL;
- xhci_dbg(xhci, "Freed segment pool\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed segment pool");
if (xhci->device_pool)
dma_pool_destroy(xhci->device_pool);
xhci->device_pool = NULL;
- xhci_dbg(xhci, "Freed device context pool\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed device context pool");
if (xhci->small_streams_pool)
dma_pool_destroy(xhci->small_streams_pool);
xhci->small_streams_pool = NULL;
- xhci_dbg(xhci, "Freed small stream array pool\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+ "Freed small stream array pool");
if (xhci->medium_streams_pool)
dma_pool_destroy(xhci->medium_streams_pool);
xhci->medium_streams_pool = NULL;
- xhci_dbg(xhci, "Freed medium stream array pool\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+ "Freed medium stream array pool");
if (xhci->dcbaa)
dma_free_coherent(&pdev->dev, sizeof(*xhci->dcbaa),
@@ -1968,8 +1971,9 @@ static void xhci_set_hc_event_deq(struct xhci_hcd *xhci)
* there might be more events to service.
*/
temp &= ~ERST_EHB;
- xhci_dbg(xhci, "// Write event ring dequeue pointer, "
- "preserving EHB bit\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+ "// Write event ring dequeue pointer, "
+ "preserving EHB bit");
xhci_write_64(xhci, ((u64) deq & (u64) ~ERST_PTR_MASK) | temp,
&xhci->ir_set->erst_dequeue);
}
@@ -1992,8 +1996,9 @@ static void xhci_add_in_port(struct xhci_hcd *xhci,
unsigned int num_ports,
temp = xhci_readl(xhci, addr + 2);
port_offset = XHCI_EXT_PORT_OFF(temp);
port_count = XHCI_EXT_PORT_COUNT(temp);
- xhci_dbg(xhci, "Ext Cap %p, port offset = %u, "
- "count = %u, revision = 0x%x\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+
[RFC 3/3] xhci: trace debug statements related to ring expansion
This patch defines a new trace event, which is called xhci_dbg_ring_expansion
and belongs to the event class xhci_log_msg, and adds tracepoints that trace
the debug messages associated with the expansion of endpoint ring when there
is not enough space allocated to hold all pending TRBs.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-mem.c | 3 ++-
drivers/usb/host/xhci-ring.c | 4 ++--
drivers/usb/host/xhci-trace.h | 5 +
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index ef27470..b150360 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -348,7 +348,8 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct
xhci_ring *ring,
return -ENOMEM;
xhci_link_rings(xhci, ring, first, last, num_segs);
- xhci_dbg(xhci, "ring expansion succeed, now has %d segments\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion,
+ "ring expansion succeed, now has %d segments",
ring->num_segs);
return 0;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 3a2277b..a8f9e05 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2904,8 +2904,8 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
return -ENOMEM;
}
- xhci_dbg(xhci, "ERROR no room on ep ring, "
- "try ring expansion\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion,
+ "ERROR no room on ep ring, try ring expansion");
num_trbs_needed = num_trbs - ep_ring->num_trbs_free;
if (xhci_ring_expansion(xhci, ep_ring, num_trbs_needed,
mem_flags)) {
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 11025fe..20364cc 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -62,6 +62,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_init,
TP_ARGS(vaf)
);
+DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
+ TP_PROTO(struct va_format *vaf),
+ TP_ARGS(vaf)
+);
+
DECLARE_EVENT_CLASS(xhci_log_ctx,
TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
unsigned int ep_num),
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v6] xhci: fix dma mask setup in xhci.c
The function dma_set_mask() tests internally whether the dma_mask pointer
for the device is initialized and fails if the dma_mask pointer is NULL.
On pci platforms, the device dma_mask pointer is initialized, when pci
devices are enumerated, to point to the pci_dev->dma_mask which is 0x.
However, for non-pci platforms, the dma_mask pointer may not be initialized
and in that case dma_set_mask() will fail.
This patch initializes the dma_mask and the coherent_dma_mask to 32bits
in xhci_plat_probe(), before the call to usb_create_hcd() that sets the
"uses_dma" flag for the usb bus and the call to usb_add_hcd() that creates
coherent dma pools for the usb hcd.
Moreover, a call to dma_set_mask() does not set the device coherent_dma_mask.
Since the xhci-hcd driver calls dma_alloc_coherent() and dma_pool_alloc()
to allocate consistent DMA memory blocks, the coherent DMA address mask
has to be set explicitly.
This patch sets the coherent_dma_mask to 64bits in xhci_gen_setup() when
the xHC is capable for 64-bit DMA addressing.
If dma_set_mask() succeeds, for a given bitmask, it is guaranteed that
the given bitmask is also supported for consistent DMA mappings.
Other changes introduced in this patch are:
- The return value of dma_set_mask() is checked to ensure that the required
dma bitmask conforms with the host system's addressing capabilities.
- The dma_mask setup code for the non-primary hcd was removed since both
primary and non-primary hcd refer to the same generic device whose
dma_mask and coherent_dma_mask are already set during the setup of
the primary hcd.
- The code for reading the HCCPARAMS register to find out the addressing
capabilities of xHC was removed since its value is already cached in
xhci->hccparams.
- hcd->self.controller was replaced with the dev variable since it is
already available.
Signed-off-by: Xenia Ragiadakou
---
Differences from version 5:
Include linux/dma-mapping.h to fix compilation errors
drivers/usb/host/xhci-plat.c | 10 ++
drivers/usb/host/xhci.c | 19 +--
2 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 412fe8d..c1b0c4a 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include "xhci.h"
@@ -104,6 +105,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (!res)
return -ENODEV;
+ /* Initialize dma_mask and coherent_dma_mask to 32-bits */
+ ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+ if (!pdev->dev.dma_mask)
+ pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+ else
+ dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+
hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
if (!hcd)
return -ENOMEM;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 246de89..fc54724 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4837,7 +4837,6 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t
get_quirks)
struct xhci_hcd *xhci;
struct device *dev = hcd->self.controller;
int retval;
- u32 temp;
/* Accept arbitrarily long scatter-gather lists */
hcd->self.sg_tablesize = ~0;
@@ -4869,14 +4868,6 @@ int xhci_gen_setup(struct usb_hcd *hcd,
xhci_get_quirks_t get_quirks)
/* xHCI private pointer was set in xhci_pci_probe for the second
* registered roothub.
*/
- xhci = hcd_to_xhci(hcd);
- temp = xhci_readl(xhci, &xhci->cap_regs->hcc_params);
- if (HCC_64BIT_ADDR(temp)) {
- xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
- dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64));
- } else {
- dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32));
- }
return 0;
}
@@ -4915,12 +4906,12 @@ int xhci_gen_setup(struct usb_hcd *hcd,
xhci_get_quirks_t get_quirks)
goto error;
xhci_dbg(xhci, "Reset complete\n");
- temp = xhci_readl(xhci, &xhci->cap_regs->hcc_params);
- if (HCC_64BIT_ADDR(temp)) {
+ /* Set dma_mask and coherent_dma_mask to 64-bits,
+* if xHC supports 64-bit addressing */
+ if (HCC_64BIT_ADDR(xhci->hcc_params) &&
+ !dma_set_mask(dev, DMA_BIT_MASK(64))) {
xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
- dma_set_mask(hcd->self.controller, DMA_BIT_MASK(
[RFC v2 1/3] xhci: trace debug statements for urb cancellation
This patch defines a new trace event, which is called xhci_dbg_cancel_urb and belongs to the event class xhci_log_msg, and adds tracepoints that trace the debug messages related to the removal of a cancelled URB from the endpoint's transfer ring. Signed-off-by: Xenia Ragiadakou --- Difference from version 1: The patchset was rebased against for-usb-next-queue branch of pub/scm/linux/kernel/git/sarah/xhci drivers/usb/host/xhci-ring.c | 64 +++ drivers/usb/host/xhci-trace.h | 5 drivers/usb/host/xhci.c | 13 + 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 86971ac..f908e20 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -556,7 +556,8 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, return; } state->new_cycle_state = 0; - xhci_dbg(xhci, "Finding segment containing stopped TRB.\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Finding segment containing stopped TRB."); state->new_deq_seg = find_trb_seg(cur_td->start_seg, dev->eps[ep_index].stopped_trb, &state->new_cycle_state); @@ -566,12 +567,14 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, } /* Dig out the cycle state saved by the xHC during the stop ep cmd */ - xhci_dbg(xhci, "Finding endpoint context\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Finding endpoint context"); ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index); state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq); state->new_deq_ptr = cur_td->last_trb; - xhci_dbg(xhci, "Finding segment containing last TRB in TD.\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Finding segment containing last TRB in TD."); state->new_deq_seg = find_trb_seg(state->new_deq_seg, state->new_deq_ptr, &state->new_cycle_state); @@ -598,13 +601,16 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, if (ep_ring->first_seg == ep_ring->first_seg->next && state->new_deq_ptr < dev->eps[ep_index].stopped_trb) state->new_cycle_state ^= 0x1; - xhci_dbg(xhci, "Cycle state = 0x%x\n", state->new_cycle_state); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Cycle state = 0x%x", state->new_cycle_state); /* Don't update the ring cycle state for the producer (us). */ - xhci_dbg(xhci, "New dequeue segment = %p (virtual)\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "New dequeue segment = %p (virtual)", state->new_deq_seg); addr = xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr); - xhci_dbg(xhci, "New dequeue pointer = 0x%llx (DMA)\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "New dequeue pointer = 0x%llx (DMA)", (unsigned long long) addr); } @@ -632,9 +638,11 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, if (flip_cycle) cur_trb->generic.field[3] ^= cpu_to_le32(TRB_CYCLE); - xhci_dbg(xhci, "Cancel (unchain) link TRB\n"); - xhci_dbg(xhci, "Address = %p (0x%llx dma); " - "in seg %p (0x%llx dma)\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Cancel (unchain) link TRB"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Address = %p (0x%llx dma); " + "in seg %p (0x%llx dma)", cur_trb, (unsigned long long)xhci_trb_virt_to_dma(cur_seg, cur_trb), cur_seg, @@ -652,7 +660,8 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, cpu_to_le32(TRB_CYCLE); cur_trb->generic.field[3] |= cpu_to_le32( TRB_TYPE(TRB_TR_NOOP)); - xhci_dbg(xhci, "TRB to noop at offset 0x%llx\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_ur
[RFC v2 3/3] xhci: trace debug statements related to ring expansion
This patch defines a new trace event, which is called xhci_dbg_ring_expansion
and belongs to the event class xhci_log_msg, and adds tracepoints that trace
the debug messages associated with the expansion of endpoint ring when there
is not enough space allocated to hold all pending TRBs.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-mem.c | 3 ++-
drivers/usb/host/xhci-ring.c | 4 ++--
drivers/usb/host/xhci-trace.h | 5 +
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index ef27470..b150360 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -348,7 +348,8 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct
xhci_ring *ring,
return -ENOMEM;
xhci_link_rings(xhci, ring, first, last, num_segs);
- xhci_dbg(xhci, "ring expansion succeed, now has %d segments\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion,
+ "ring expansion succeed, now has %d segments",
ring->num_segs);
return 0;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f908e20..7b35af1 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2904,8 +2904,8 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
return -ENOMEM;
}
- xhci_dbg(xhci, "ERROR no room on ep ring, "
- "try ring expansion\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion,
+ "ERROR no room on ep ring, try ring expansion");
num_trbs_needed = num_trbs - ep_ring->num_trbs_free;
if (xhci_ring_expansion(xhci, ep_ring, num_trbs_needed,
mem_flags)) {
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 11025fe..20364cc 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -62,6 +62,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_init,
TP_ARGS(vaf)
);
+DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
+ TP_PROTO(struct va_format *vaf),
+ TP_ARGS(vaf)
+);
+
DECLARE_EVENT_CLASS(xhci_log_ctx,
TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
unsigned int ep_num),
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2 2/3] xhci: trace debug messages related to driver initialization and unload
This patch defines a new trace event, which is called xhci_dbg_init
and belongs to the event class xhci_log_msg, and adds tracepoints that
trace the debug statements in the functions used to start and stop the
xhci-hcd driver.
Also, it removes an unnecessary cast of variable val to unsigned int
in xhci_mem_init(), since val is already declared as unsigned int.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-mem.c | 110 ++
drivers/usb/host/xhci-trace.h | 5 ++
drivers/usb/host/xhci.c | 66 +++--
3 files changed, 113 insertions(+), 68 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b1bb59b..ef27470 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1545,7 +1545,8 @@ static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t
flags)
struct device *dev = xhci_to_hcd(xhci)->self.controller;
int num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
- xhci_dbg(xhci, "Allocating %d scratchpad buffers\n", num_sp);
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+ "Allocating %d scratchpad buffers", num_sp);
if (!num_sp)
return 0;
@@ -1702,11 +1703,11 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
dma_free_coherent(&pdev->dev, size,
xhci->erst.entries, xhci->erst.erst_dma_addr);
xhci->erst.entries = NULL;
- xhci_dbg(xhci, "Freed ERST\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST");
if (xhci->event_ring)
xhci_ring_free(xhci, xhci->event_ring);
xhci->event_ring = NULL;
- xhci_dbg(xhci, "Freed event ring\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed event ring");
if (xhci->lpm_command)
xhci_free_command(xhci, xhci->lpm_command);
@@ -1714,7 +1715,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
if (xhci->cmd_ring)
xhci_ring_free(xhci, xhci->cmd_ring);
xhci->cmd_ring = NULL;
- xhci_dbg(xhci, "Freed command ring\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed command ring");
list_for_each_entry_safe(cur_cd, next_cd,
&xhci->cancel_cmd_list, cancel_cmd_list) {
list_del(&cur_cd->cancel_cmd_list);
@@ -1727,22 +1728,24 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
if (xhci->segment_pool)
dma_pool_destroy(xhci->segment_pool);
xhci->segment_pool = NULL;
- xhci_dbg(xhci, "Freed segment pool\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed segment pool");
if (xhci->device_pool)
dma_pool_destroy(xhci->device_pool);
xhci->device_pool = NULL;
- xhci_dbg(xhci, "Freed device context pool\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed device context pool");
if (xhci->small_streams_pool)
dma_pool_destroy(xhci->small_streams_pool);
xhci->small_streams_pool = NULL;
- xhci_dbg(xhci, "Freed small stream array pool\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+ "Freed small stream array pool");
if (xhci->medium_streams_pool)
dma_pool_destroy(xhci->medium_streams_pool);
xhci->medium_streams_pool = NULL;
- xhci_dbg(xhci, "Freed medium stream array pool\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+ "Freed medium stream array pool");
if (xhci->dcbaa)
dma_free_coherent(&pdev->dev, sizeof(*xhci->dcbaa),
@@ -1968,8 +1971,9 @@ static void xhci_set_hc_event_deq(struct xhci_hcd *xhci)
* there might be more events to service.
*/
temp &= ~ERST_EHB;
- xhci_dbg(xhci, "// Write event ring dequeue pointer, "
- "preserving EHB bit\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+ "// Write event ring dequeue pointer, "
+ "preserving EHB bit");
xhci_write_64(xhci, ((u64) deq & (u64) ~ERST_PTR_MASK) | temp,
&xhci->ir_set->erst_dequeue);
}
@@ -1992,8 +1996,9 @@ static void xhci_add_in_port(struct xhci_hcd *xhci,
unsigned int num_ports,
temp = xhci_readl(xhci, addr + 2);
port_offset = XHCI_EXT_PORT_OFF(temp);
port_count = XHCI_EXT_PORT_COUNT(temp);
- xhci_dbg(xhci, "Ext Cap %p, port offset = %u, "
- "count = %u, revision = 0x%x\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+
[RFC 1/2] xhci: remove unused argument from xhci_giveback_urb_in_irq()
This patch removes the "adjective" argument from xhci_giveback_urb_in_irq(),
since it is not used in the function anymore.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7b35af1..ddbda35 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -716,7 +716,7 @@ static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd
*xhci,
/* Must be called with xhci->lock held in interrupt context */
static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
- struct xhci_td *cur_td, int status, char *adjective)
+ struct xhci_td *cur_td, int status)
{
struct usb_hcd *hcd;
struct urb *urb;
@@ -877,7 +877,7 @@ remove_finished_td:
/* Doesn't matter what we pass for status, since the core will
* just overwrite it (because the URB has been unlinked).
*/
- xhci_giveback_urb_in_irq(xhci, cur_td, 0, "cancelled");
+ xhci_giveback_urb_in_irq(xhci, cur_td, 0);
/* Stop processing the cancelled list if the watchdog timer is
* running.
@@ -987,7 +987,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
if (!list_empty(&cur_td->cancelled_td_list))
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
while (!list_empty(&temp_ep->cancelled_td_list)) {
cur_td = list_first_entry(
@@ -996,7 +996,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
cancelled_td_list);
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
}
}
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 2/2] xhci: refactor handle_cmd_completion() switch into separate function
This patch refactors the big switch statement in handle_cmd_completion()
into a separate function, called handle_cmd_completion2() (this name is
temporary, we will need a different name).
There have been declared some additional local variables, such as
cmd_trb, cmd_comp_code, cmd_type, add_flags and drop_flags, mainly
to reduce line length and code dublication.
Also, the variable ep_ring, that was assigned in the case TRB_CONFIG_EP
but never used, was removed.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 158 +++
1 file changed, 83 insertions(+), 75 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ddbda35..02cd900 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1372,70 +1372,40 @@ static int handle_stopped_cmd_ring(struct xhci_hcd
*xhci,
return cur_trb_is_good;
}
-static void handle_cmd_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event)
+static void handle_cmd_completion2(struct xhci_hcd *xhci, u32 cmd_comp_code,
+ union xhci_trb *cmd_trb, struct xhci_event_cmd *event)
{
- int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
- u64 cmd_dma;
- dma_addr_t cmd_dequeue_dma;
- struct xhci_input_control_ctx *ctrl_ctx;
+ int slot_id;
+ u32 cmd_type;
struct xhci_virt_device *virt_dev;
+ struct xhci_input_control_ctx *ctrl_ctx;
unsigned int ep_index;
- struct xhci_ring *ep_ring;
unsigned int ep_state;
+ u32 add_flags, drop_flags;
- cmd_dma = le64_to_cpu(event->cmd_trb);
- cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
- xhci->cmd_ring->dequeue);
- /* Is the command ring deq ptr out of sync with the deq seg ptr? */
- if (cmd_dequeue_dma == 0) {
- xhci->error_bitmask |= 1 << 4;
- return;
- }
- /* Does the DMA address match our internal dequeue pointer address? */
- if (cmd_dma != (u64) cmd_dequeue_dma) {
- xhci->error_bitmask |= 1 << 5;
- return;
- }
-
- trace_xhci_cmd_completion(&xhci->cmd_ring->dequeue->generic,
- (struct xhci_generic_trb *) event);
+ cmd_type = TRB_FIELD_TO_TYPE(le32_to_cpu(cmd_trb->generic.field[3]));
+ slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
+ virt_dev = xhci->devs[slot_id];
- if ((GET_COMP_CODE(le32_to_cpu(event->status)) == COMP_CMD_ABORT) ||
- (GET_COMP_CODE(le32_to_cpu(event->status)) == COMP_CMD_STOP)) {
- /* If the return value is 0, we think the trb pointed by
-* command ring dequeue pointer is a good trb. The good
-* trb means we don't want to cancel the trb, but it have
-* been stopped by host. So we should handle it normally.
-* Otherwise, driver should invoke inc_deq() and return.
-*/
- if (handle_stopped_cmd_ring(xhci,
- GET_COMP_CODE(le32_to_cpu(event->status {
- inc_deq(xhci, xhci->cmd_ring);
- return;
- }
- }
+ switch (cmd_type) {
- switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
- & TRB_TYPE_BITMASK) {
- case TRB_TYPE(TRB_ENABLE_SLOT):
- if (GET_COMP_CODE(le32_to_cpu(event->status)) == COMP_SUCCESS)
+ case TRB_ENABLE_SLOT:
+ if (cmd_comp_code == COMP_SUCCESS)
xhci->slot_id = slot_id;
else
xhci->slot_id = 0;
complete(&xhci->addr_dev);
break;
- case TRB_TYPE(TRB_DISABLE_SLOT):
- if (xhci->devs[slot_id]) {
+ case TRB_DISABLE_SLOT:
+ if (virt_dev) {
if (xhci->quirks & XHCI_EP_LIMIT_QUIRK)
/* Delete default control endpoint resources */
xhci_free_device_endpoint_resources(xhci,
- xhci->devs[slot_id], true);
+ virt_dev, true);
xhci_free_virt_device(xhci, slot_id);
}
break;
- case TRB_TYPE(TRB_CONFIG_EP):
- virt_dev = xhci->devs[slot_id];
+ case TRB_CONFIG_EP:
if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
break;
/*
@@ -1446,14 +1416,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
* If the command was for a halted endpoint, the xHCI driver
* is not w
[RFC v2 2/2] xhci: refactor handle_cmd_completion() switch branches into functions
This patch refactors the switch statement in handle_cmd_completion() by
creating a separate function for each branch, named 'xhci_handle_cmd_'.
Other changes introduced by this patch are:
- Renaming the already existed functions by adding the prefix
'xhci_handle_cmd_' for consistency. Also, for handle_stopped_endpoint()
the order of arguments was changed too, so that the prototype of
'xhci_handle_cmd_' be similar and easy to follow.
- Additional local variables were added, such as cmd_trb, cmd_comp_code,
cmd_type, add_flags and drop_flags, and the label 'update_ring',
mainly to reduce code dublication and line length.
- The variable ep_ring, that was assigned in the case TRB_CONFIG_EP
but never used, was removed.
Signed-off-by: Xenia Ragiadakou
---
Differences from version 1:
Create separate functions for each switch case branch, instead of a single
function holding the entire switch
drivers/usb/host/xhci-ring.c | 310 ++-
1 file changed, 185 insertions(+), 125 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ddbda35..bc9ce03 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,8 +755,8 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
* 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the
chain
* bit cleared) so that the HW will skip over them.
*/
-static void handle_stopped_endpoint(struct xhci_hcd *xhci,
- union xhci_trb *trb, struct xhci_event_cmd *event)
+static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
unsigned int slot_id;
unsigned int ep_index;
@@ -1063,9 +1063,8 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* endpoint doorbell to restart the ring, but only if there aren't more
* cancellations pending.
*/
-static void handle_set_deq_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
unsigned int slot_id;
unsigned int ep_index;
@@ -1157,9 +1156,8 @@ static void handle_set_deq_completion(struct xhci_hcd
*xhci,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
-static void handle_reset_ep_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
int slot_id;
unsigned int ep_index;
@@ -1372,21 +1370,160 @@ static int handle_stopped_cmd_ring(struct xhci_hcd
*xhci,
return cur_trb_is_good;
}
-static void handle_cmd_completion(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, u32 cmd_comp_code)
+{
+ int slot_id;
+
+ slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
+ if (cmd_comp_code == COMP_SUCCESS)
+ xhci->slot_id = slot_id;
+ else
+ xhci->slot_id = 0;
+ complete(&xhci->addr_dev);
+}
+
+static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
- int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
- u64 cmd_dma;
- dma_addr_t cmd_dequeue_dma;
- struct xhci_input_control_ctx *ctrl_ctx;
+ int slot_id;
struct xhci_virt_device *virt_dev;
+
+ slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
+ virt_dev = xhci->devs[slot_id];
+ if (!virt_dev)
+ return;
+ if (xhci->quirks & XHCI_EP_LIMIT_QUIRK)
+ /* Delete default control endpoint resources */
+ xhci_free_device_endpoint_resources(xhci, virt_dev, true);
+ xhci_free_virt_device(xhci, slot_id);
+}
+
+static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, u32 cmd_comp_code)
+{
+ int slot_id;
+ struct xhci_virt_device *virt_dev;
+ struct xhci_input_control_ctx *ctrl_ctx;
unsigned int ep_index;
- struct xhci_ring *ep_ring;
unsigned int ep_state;
+ u32 add_flags, drop_flags;
+
+ slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
+ virt_dev = xhci->devs[slot_id];
+ if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
+ return;
+ /*
+* Configure endpoint commands can come from the USB core
+* configuration or alt setting changes, or because the HW
+* needed an extra configure endpoint command after a reset
+* endpoint command or streams were bei
[RFC] xhci: add trace for missed periodic transfers
This patch adds a trace event to the class 'xhci_log_msg', called
'xhci_dbg_missed_periodic_tx', that traces the debug statements which
signal that the xHC is unable to service an isochronous endpoint within
its service interval either because the endpoint's ring is full and can
not receive further data, for IN endpoints, or because the endpoint's ring
is empty, for OUT endpoints, or due to xHC internal buffers' overrun or
underrun caused by excessive latency on the transfer path.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 19 ---
drivers/usb/host/xhci-trace.h | 5 +
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index bc9ce03..06d3a8f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2510,18 +2510,22 @@ static int handle_tx_event(struct xhci_hcd *xhci,
* a Ring Overrun Event for IN Isoch endpoint or Ring
* Underrun Event for OUT Isoch endpoint.
*/
- xhci_dbg(xhci, "underrun event on endpoint\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "underrun event on endpoint");
if (!list_empty(&ep_ring->td_list))
- xhci_dbg(xhci, "Underrun Event for slot %d ep %d "
- "still with TDs queued?\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Underrun Event for slot %d ep %d "
+ "still with TDs queued?",
TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
ep_index);
goto cleanup;
case COMP_OVERRUN:
- xhci_dbg(xhci, "overrun event on endpoint\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "overrun event on endpoint");
if (!list_empty(&ep_ring->td_list))
- xhci_dbg(xhci, "Overrun Event for slot %d ep %d "
- "still with TDs queued?\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Overrun Event for slot %d ep %d "
+ "still with TDs queued?",
TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
ep_index);
goto cleanup;
@@ -2537,7 +2541,8 @@ 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");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Miss service interval error, set skip flag");
goto cleanup;
default:
if (xhci_is_vendor_info_code(xhci, trb_comp_code)) {
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 20364cc..c156685 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -67,6 +67,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
TP_ARGS(vaf)
);
+DEFINE_EVENT(xhci_log_msg, xhci_dbg_missed_periodic_tx,
+ TP_PROTO(struct va_format *vaf),
+ TP_ARGS(vaf)
+);
+
DECLARE_EVENT_CLASS(xhci_log_ctx,
TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
unsigned int ep_num),
--
1.8.3.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
[RFC v2 1/2] xhci: remove unused argument from xhci_giveback_urb_in_irq()
This patch removes the "adjective" argument from xhci_giveback_urb_in_irq(),
since it is not used in the function anymore.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7b35af1..ddbda35 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -716,7 +716,7 @@ static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd
*xhci,
/* Must be called with xhci->lock held in interrupt context */
static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
- struct xhci_td *cur_td, int status, char *adjective)
+ struct xhci_td *cur_td, int status)
{
struct usb_hcd *hcd;
struct urb *urb;
@@ -877,7 +877,7 @@ remove_finished_td:
/* Doesn't matter what we pass for status, since the core will
* just overwrite it (because the URB has been unlinked).
*/
- xhci_giveback_urb_in_irq(xhci, cur_td, 0, "cancelled");
+ xhci_giveback_urb_in_irq(xhci, cur_td, 0);
/* Stop processing the cancelled list if the watchdog timer is
* running.
@@ -987,7 +987,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
if (!list_empty(&cur_td->cancelled_td_list))
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
while (!list_empty(&temp_ep->cancelled_td_list)) {
cur_td = list_first_entry(
@@ -996,7 +996,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
cancelled_td_list);
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
}
}
--
1.8.3.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: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On 08/14/2013 08:24 AM, Sarah Sharp wrote: On Fri, Aug 09, 2013 at 05:14:48PM -0400, Alan Stern wrote: On Fri, 9 Aug 2013, Greg KH wrote: On Thu, Aug 08, 2013 at 10:07:19PM +0200, Martin MOKREJŠ wrote: Hi, I get plenty of these in /sys/kernel/debug/kmemleak: unreferenced object 0x88019f675268 (size 32): comm "usb-storage", pid 11411, jiffies 4310515592 (age 1538.100s) hex dump (first 32 bytes): 05 0f 16 00 02 07 10 02 02 00 00 00 0a 10 03 00 0e 00 01 0a ff 07 00 00 00 00 00 00 00 00 00 00 backtrace: [] kmemleak_alloc+0x21/0x50 [] __kmalloc+0xce/0x170 [] usb_get_bos_descriptor+0xc5/0x230 [] hub_port_init+0x768/0xa20 [] usb_reset_and_verify_device+0xd6/0x540 [] usb_reset_device+0x110/0x190 [] usb_stor_port_reset+0x74/0x80 [] usb_stor_invoke_transport+0x8f/0x550 [] usb_stor_transparent_scsi_command+0x9/0x10 [] usb_stor_control_thread+0x16b/0x280 [] kthread+0xe5/0xf0 [] ret_from_fork+0x7c/0xb0 [] 0x Odd, Andiry and Sarah, any thoughts? dmesg left below for completeness. Martin is right; the BOS descriptors are leaked in usb_reset_and_verify_device(). We need to store the old descriptor, compare it with the new one following the reset, and delete one of them afterward. I haven't had time to fix this. I'll put it in my list of future tasks to give to Xenia. :) Alan, if you have any other small tasks for OPW interns, please let me know. Sarah Sharp Hi, I will try to fix this leak :) First i would like to ensure that i understood the problem so ... If i understood well the usb device's bos descriptor is allocated everytime hub_port_init() is called without never being freed, right? I do not understand why to compare the old bos with the new one, and not just release the old one or store the new in the memory allocated for the old one. best regards, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 18/18] xhci: add trace for missed periodic transfers
This patch adds a trace event to the class 'xhci_log_msg', called
'xhci_dbg_missed_periodic_tx', that traces the debug statements which
signal that the xHC is unable to service an isochronous endpoint within
its service interval either because the endpoint's ring is full and can
not receive further data, for IN endpoints, or because the endpoint's ring
is empty, for OUT endpoints, or due to xHC internal buffers' overrun or
underrun caused by excessive latency on the transfer path.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 19 ---
drivers/usb/host/xhci-trace.h | 5 +
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 03f65dc..cbf3e2a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2484,18 +2484,22 @@ static int handle_tx_event(struct xhci_hcd *xhci,
* a Ring Overrun Event for IN Isoch endpoint or Ring
* Underrun Event for OUT Isoch endpoint.
*/
- xhci_dbg(xhci, "underrun event on endpoint\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "underrun event on endpoint");
if (!list_empty(&ep_ring->td_list))
- xhci_dbg(xhci, "Underrun Event for slot %d ep %d "
- "still with TDs queued?\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Underrun Event for slot %d ep %d "
+ "still with TDs queued?",
TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
ep_index);
goto cleanup;
case COMP_OVERRUN:
- xhci_dbg(xhci, "overrun event on endpoint\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "overrun event on endpoint");
if (!list_empty(&ep_ring->td_list))
- xhci_dbg(xhci, "Overrun Event for slot %d ep %d "
- "still with TDs queued?\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Overrun Event for slot %d ep %d "
+ "still with TDs queued?",
TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
ep_index);
goto cleanup;
@@ -2511,7 +2515,8 @@ 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");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Miss service interval error, set skip flag");
goto cleanup;
default:
if (xhci_is_vendor_info_code(xhci, trb_comp_code)) {
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 20364cc..c156685 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -67,6 +67,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
TP_ARGS(vaf)
);
+DEFINE_EVENT(xhci_log_msg, xhci_dbg_missed_periodic_tx,
+ TP_PROTO(struct va_format *vaf),
+ TP_ARGS(vaf)
+);
+
DECLARE_EVENT_CLASS(xhci_log_ctx,
TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
unsigned int ep_num),
--
1.8.3.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
[RFC v3 06/18] xhci: refactor TRB_RESET_DEV case into function
This patch refactors the code in TRB_RESET_DEV switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_reset_dev().
Here, in the original code, the slot id is retrieved by the command TRB.
Since this slot id matches the one in Slot ID field of the command completion
event, which is available, there is no need to determine it again.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f00d9ef..0f9863e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1400,6 +1400,20 @@ static void xhci_handle_cmd_addr_dev(struct xhci_hcd
*xhci, int slot_id,
complete(&xhci->addr_dev);
}
+static void xhci_handle_cmd_reset_dev(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event)
+{
+ struct xhci_virt_device *virt_dev;
+
+ xhci_dbg(xhci, "Completed reset device command.\n");
+ virt_dev = xhci->devs[slot_id];
+ if (virt_dev)
+ handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
+ else
+ xhci_warn(xhci, "Reset device command completion "
+ "for disabled slot %u\n", slot_id);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1527,15 +1541,7 @@ bandwidth_change:
xhci_handle_cmd_reset_ep(xhci, event, xhci->cmd_ring->dequeue);
break;
case TRB_TYPE(TRB_RESET_DEV):
- xhci_dbg(xhci, "Completed reset device command.\n");
- slot_id = TRB_TO_SLOT_ID(
- le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3]));
- virt_dev = xhci->devs[slot_id];
- if (virt_dev)
- handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
- else
- xhci_warn(xhci, "Reset device command completion "
- "for disabled slot %u\n", slot_id);
+ xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
case TRB_TYPE(TRB_NEC_GET_FW):
if (!(xhci->quirks & XHCI_NEC_HOST)) {
--
1.8.3.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
[RFC v3 07/18] xhci: refactor TRB_NEC_GET_FW case into function
This patch refactors the code in TRB_NEC_GET_FW switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_nec_get_fw().
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0f9863e..89b4c05 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1414,6 +1414,19 @@ static void xhci_handle_cmd_reset_dev(struct xhci_hcd
*xhci, int slot_id,
"for disabled slot %u\n", slot_id);
}
+static void xhci_handle_cmd_nec_get_fw(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event)
+{
+ if (!(xhci->quirks & XHCI_NEC_HOST)) {
+ xhci->error_bitmask |= 1 << 6;
+ return;
+ }
+ xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
+ "NEC firmware version %2x.%02x",
+ NEC_FW_MAJOR(le32_to_cpu(event->status)),
+ NEC_FW_MINOR(le32_to_cpu(event->status)));
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1544,14 +1557,7 @@ bandwidth_change:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
case TRB_TYPE(TRB_NEC_GET_FW):
- if (!(xhci->quirks & XHCI_NEC_HOST)) {
- xhci->error_bitmask |= 1 << 6;
- break;
- }
- xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
- "NEC firmware version %2x.%02x",
-NEC_FW_MAJOR(le32_to_cpu(event->status)),
-NEC_FW_MINOR(le32_to_cpu(event->status)));
+ xhci_handle_cmd_nec_get_fw(xhci, event);
break;
default:
/* Skip over unknown commands on the event ring */
--
1.8.3.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
[RFC v3 16/18] xhci: replace 'event' with 'cmd_comp_code' in set_deq and reset_ep handlers
This patch replaces the 'event' argument of xhci_handle_cmd_set_deq() and
xhci_handle_cmd_reset_ep(), which is used to retrieve the command completion
status code, with the cmd_comp_code directly since it is available.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 405db44..e13531f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1059,7 +1059,7 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* cancellations pending.
*/
static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
- struct xhci_event_cmd *event, union xhci_trb *trb)
+ union xhci_trb *trb, u32 cmd_comp_code)
{
unsigned int ep_index;
unsigned int stream_id;
@@ -1085,11 +1085,11 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd
*xhci, int slot_id,
ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
slot_ctx = xhci_get_slot_ctx(xhci, dev->out_ctx);
- if (GET_COMP_CODE(le32_to_cpu(event->status)) != COMP_SUCCESS) {
+ if (cmd_comp_code != COMP_SUCCESS) {
unsigned int ep_state;
unsigned int slot_state;
- switch (GET_COMP_CODE(le32_to_cpu(event->status))) {
+ switch (cmd_comp_code) {
case COMP_TRB_ERR:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid
because "
"of stream ID configuration\n");
@@ -1112,7 +1112,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd
*xhci, int slot_id,
default:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd with unknown "
"completion code of %u.\n",
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ cmd_comp_code);
break;
}
/* OK what do we do now? The endpoint state is hosed, and we
@@ -1150,7 +1150,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd
*xhci, int slot_id,
}
static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id,
- struct xhci_event_cmd *event, union xhci_trb *trb)
+ union xhci_trb *trb, u32 cmd_comp_code)
{
unsigned int ep_index;
@@ -1159,8 +1159,7 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd
*xhci, int slot_id,
* but we don't care.
*/
xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
- "Ignoring reset ep completion code of %u",
-GET_COMP_CODE(le32_to_cpu(event->status)));
+ "Ignoring reset ep completion code of %u", cmd_comp_code);
/* HW with the reset endpoint quirk needs to have a configure endpoint
* command complete before the endpoint can be used. Queue that here
@@ -1550,12 +1549,12 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_stop_ep(xhci, slot_id, cmd_trb, event);
break;
case TRB_SET_DEQ:
- xhci_handle_cmd_set_deq(xhci, slot_id, event, cmd_trb);
+ xhci_handle_cmd_set_deq(xhci, slot_id, cmd_trb, cmd_comp_code);
break;
case TRB_CMD_NOOP:
break;
case TRB_RESET_EP:
- xhci_handle_cmd_reset_ep(xhci, slot_id, event, cmd_trb);
+ xhci_handle_cmd_reset_ep(xhci, slot_id, cmd_trb, cmd_comp_code);
break;
case TRB_RESET_DEV:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
--
1.8.3.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
[RFC v3 02/18] xhci: rename existing functions
This patch renames the function handlers of a triggered Command Completion
Event that correspond to each command type into 'xhci_handle_cmd_'.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ddbda35..ffd224c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,7 +755,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
* 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the
chain
* bit cleared) so that the HW will skip over them.
*/
-static void handle_stopped_endpoint(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
union xhci_trb *trb, struct xhci_event_cmd *event)
{
unsigned int slot_id;
@@ -1063,9 +1063,8 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* endpoint doorbell to restart the ring, but only if there aren't more
* cancellations pending.
*/
-static void handle_set_deq_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
unsigned int slot_id;
unsigned int ep_index;
@@ -1157,9 +1156,8 @@ static void handle_set_deq_completion(struct xhci_hcd
*xhci,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
-static void handle_reset_ep_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
int slot_id;
unsigned int ep_index;
@@ -1497,15 +1495,15 @@ bandwidth_change:
complete(&xhci->addr_dev);
break;
case TRB_TYPE(TRB_STOP_RING):
- handle_stopped_endpoint(xhci, xhci->cmd_ring->dequeue, event);
+ xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
break;
case TRB_TYPE(TRB_SET_DEQ):
- handle_set_deq_completion(xhci, event, xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_set_deq(xhci, event, xhci->cmd_ring->dequeue);
break;
case TRB_TYPE(TRB_CMD_NOOP):
break;
case TRB_TYPE(TRB_RESET_EP):
- handle_reset_ep_completion(xhci, event,
xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_reset_ep(xhci, event, xhci->cmd_ring->dequeue);
break;
case TRB_TYPE(TRB_RESET_DEV):
xhci_dbg(xhci, "Completed reset device command.\n");
--
1.8.3.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
[RFC v3 08/18] xhci: refactor TRB_EVAL_CONTEXT case into function
This patch refactors the code in TRB_EVAL_CONTEXT switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_eval_ctx().
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 89b4c05..0f571c5 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1393,6 +1393,18 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd
*xhci, int slot_id)
xhci_free_virt_device(xhci, slot_id);
}
+static void xhci_handle_cmd_eval_ctx(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event, u32 cmd_comp_code)
+{
+ struct xhci_virt_device *virt_dev;
+
+ virt_dev = xhci->devs[slot_id];
+ if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
+ return;
+ virt_dev->cmd_status = cmd_comp_code;
+ complete(&virt_dev->cmd_completion);
+}
+
static void xhci_handle_cmd_addr_dev(struct xhci_hcd *xhci, int slot_id,
u32 cmd_comp_code)
{
@@ -1532,11 +1544,8 @@ bandwidth_change:
complete(&xhci->devs[slot_id]->cmd_completion);
break;
case TRB_TYPE(TRB_EVAL_CONTEXT):
- virt_dev = xhci->devs[slot_id];
- if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
- break;
- xhci->devs[slot_id]->cmd_status =
GET_COMP_CODE(le32_to_cpu(event->status));
- complete(&xhci->devs[slot_id]->cmd_completion);
+ xhci_handle_cmd_eval_ctx(xhci, slot_id, event,
+ GET_COMP_CODE(le32_to_cpu(event->status)));
break;
case TRB_TYPE(TRB_ADDR_DEV):
xhci_handle_cmd_addr_dev(xhci, slot_id,
--
1.8.3.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
[RFC v3 04/18] xhci: refactor TRB_DISABLE_SLOT case into function
This patch refactors the code in TRB_DISABLE_SLOT switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_disable_slot().
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0abf88c..f9c380e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1380,6 +1380,19 @@ static void xhci_handle_cmd_enable_slot(struct xhci_hcd
*xhci, int slot_id,
complete(&xhci->addr_dev);
}
+static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id)
+{
+ struct xhci_virt_device *virt_dev;
+
+ virt_dev = xhci->devs[slot_id];
+ if (!virt_dev)
+ return;
+ if (xhci->quirks & XHCI_EP_LIMIT_QUIRK)
+ /* Delete default control endpoint resources */
+ xhci_free_device_endpoint_resources(xhci, virt_dev, true);
+ xhci_free_virt_device(xhci, slot_id);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1431,13 +1444,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
GET_COMP_CODE(le32_to_cpu(event->status)));
break;
case TRB_TYPE(TRB_DISABLE_SLOT):
- if (xhci->devs[slot_id]) {
- if (xhci->quirks & XHCI_EP_LIMIT_QUIRK)
- /* Delete default control endpoint resources */
- xhci_free_device_endpoint_resources(xhci,
- xhci->devs[slot_id], true);
- xhci_free_virt_device(xhci, slot_id);
- }
+ xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
case TRB_TYPE(TRB_CONFIG_EP):
virt_dev = xhci->devs[slot_id];
--
1.8.3.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
[RFC v3 11/18] xhci: add variable 'cmd_comp_code' in handle_cmd_completion()
This patch adds a new variable 'cmd_comp_code' to hold the command completion
status code aiming to reduce code duplication and to improve code readability.
Signed-off-by: Xenia Ragiadakou
---
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 7103bd9..5b85582 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1503,6 +1503,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
+ u32 cmd_comp_code;
cmd_dma = le64_to_cpu(event->cmd_trb);
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
@@ -1521,16 +1522,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
trace_xhci_cmd_completion(&xhci->cmd_ring->dequeue->generic,
(struct xhci_generic_trb *) event);
- if ((GET_COMP_CODE(le32_to_cpu(event->status)) == COMP_CMD_ABORT) ||
- (GET_COMP_CODE(le32_to_cpu(event->status)) == COMP_CMD_STOP)) {
+ cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
+ if (cmd_comp_code == COMP_CMD_ABORT || cmd_comp_code == COMP_CMD_STOP) {
/* If the return value is 0, we think the trb pointed by
* command ring dequeue pointer is a good trb. The good
* trb means we don't want to cancel the trb, but it have
* been stopped by host. So we should handle it normally.
* Otherwise, driver should invoke inc_deq() and return.
*/
- if (handle_stopped_cmd_ring(xhci,
- GET_COMP_CODE(le32_to_cpu(event->status {
+ if (handle_stopped_cmd_ring(xhci, cmd_comp_code)) {
inc_deq(xhci, xhci->cmd_ring);
return;
}
@@ -1539,23 +1539,19 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
& TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
- xhci_handle_cmd_enable_slot(xhci, slot_id,
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_DISABLE_SLOT):
xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
case TRB_TYPE(TRB_CONFIG_EP):
- xhci_handle_cmd_config_ep(xhci, slot_id, event,
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ xhci_handle_cmd_config_ep(xhci, slot_id, event, cmd_comp_code);
break;
case TRB_TYPE(TRB_EVAL_CONTEXT):
- xhci_handle_cmd_eval_ctx(xhci, slot_id, event,
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ xhci_handle_cmd_eval_ctx(xhci, slot_id, event, cmd_comp_code);
break;
case TRB_TYPE(TRB_ADDR_DEV):
- xhci_handle_cmd_addr_dev(xhci, slot_id,
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_STOP_RING):
xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
--
1.8.3.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
[RFC v3 10/18] xhci: refactor TRB_CONFIG_EP case into function
This patch refactors the code in TRB_CONFIG_EP switch case, in
handle_cmd_completion(), into a fuction named xhci_handle_cmd_config_ep().
There were added two additional variables, 'add_flags' and 'drop_flags',
to reduce line length below 80 chars and improve code readability.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 112 +++
1 file changed, 60 insertions(+), 52 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b96..7103bd9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1393,6 +1393,64 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd
*xhci, int slot_id)
xhci_free_virt_device(xhci, slot_id);
}
+static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event, u32 cmd_comp_code)
+{
+ struct xhci_virt_device *virt_dev;
+ struct xhci_input_control_ctx *ctrl_ctx;
+ unsigned int ep_index;
+ unsigned int ep_state;
+ u32 add_flags, drop_flags;
+
+ virt_dev = xhci->devs[slot_id];
+ if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
+ return;
+ /*
+* Configure endpoint commands can come from the USB core
+* configuration or alt setting changes, or because the HW
+* needed an extra configure endpoint command after a reset
+* endpoint command or streams were being configured.
+* If the command was for a halted endpoint, the xHCI driver
+* is not waiting on the configure endpoint command.
+*/
+ ctrl_ctx = xhci_get_input_control_ctx(xhci, virt_dev->in_ctx);
+ if (!ctrl_ctx) {
+ xhci_warn(xhci, "Could not get input context, bad type.\n");
+ return;
+ }
+ add_flags = le32_to_cpu(ctrl_ctx->add_flags);
+ drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
+ /* Input ctx add_flags are the endpoint index plus one */
+ ep_index = xhci_last_valid_endpoint(add_flags) - 1;
+ /* A usb_set_interface() call directly after clearing a halted
+* condition may race on this quirky hardware. Not worth
+* worrying about, since this is prototype hardware. Not sure
+* if this will work for streams, but streams support was
+* untested on this prototype.
+*/
+ if (xhci->quirks & XHCI_RESET_EP_QUIRK &&
+ ep_index != (unsigned int) -1 &&
+ add_flags - SLOT_FLAG == drop_flags) {
+ ep_state = virt_dev->eps[ep_index].ep_state;
+ if (!(ep_state & EP_HALTED))
+ goto bandwidth_change;
+ xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
+ "Completed config ep cmd - "
+ "last ep index = %d, state = %d",
+ ep_index, ep_state);
+ /* Clear internal halted state and restart ring(s) */
+ virt_dev->eps[ep_index].ep_state &= ~EP_HALTED;
+ ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
+ return;
+ }
+bandwidth_change:
+ xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
+ "Completed config ep cmd");
+ virt_dev->cmd_status = cmd_comp_code;
+ complete(&virt_dev->cmd_completion);
+ return;
+}
+
static void xhci_handle_cmd_eval_ctx(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event, u32 cmd_comp_code)
{
@@ -1445,10 +1503,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
- struct xhci_input_control_ctx *ctrl_ctx;
- struct xhci_virt_device *virt_dev;
- unsigned int ep_index;
- unsigned int ep_state;
cmd_dma = le64_to_cpu(event->cmd_trb);
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
@@ -1492,54 +1546,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
case TRB_TYPE(TRB_CONFIG_EP):
- virt_dev = xhci->devs[slot_id];
- if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
- break;
- /*
-* Configure endpoint commands can come from the USB core
-* configuration or alt setting changes, or because the HW
-* needed an extra configure endpoint command after a reset
-* endpoint command or streams were being configured.
-* If the command was for a halted endpoint, the xHCI driver
-
[RFC v3 05/18] xhci: refactor TRB_ADDR_DEV case into function
This patch refactors the code in TRB_ADDR_DEV switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_addr_dev().
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f9c380e..f00d9ef 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1393,6 +1393,13 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd
*xhci, int slot_id)
xhci_free_virt_device(xhci, slot_id);
}
+static void xhci_handle_cmd_addr_dev(struct xhci_hcd *xhci, int slot_id,
+ u32 cmd_comp_code)
+{
+ xhci->devs[slot_id]->cmd_status = cmd_comp_code;
+ complete(&xhci->addr_dev);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1505,8 +1512,8 @@ bandwidth_change:
complete(&xhci->devs[slot_id]->cmd_completion);
break;
case TRB_TYPE(TRB_ADDR_DEV):
- xhci->devs[slot_id]->cmd_status =
GET_COMP_CODE(le32_to_cpu(event->status));
- complete(&xhci->addr_dev);
+ xhci_handle_cmd_addr_dev(xhci, slot_id,
+ GET_COMP_CODE(le32_to_cpu(event->status)));
break;
case TRB_TYPE(TRB_STOP_RING):
xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
--
1.8.3.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
[RFC v3 14/18] xhci: replace 'xhci->cmd_ring->dequeue' with 'trb' in stop_ep cmd handler
This patch replaces 'xhci->cmd_ring->dequeue' with 'trb', the address of
the command TRB, since it is available to reduce line length.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 60cf547..7a58095 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -769,10 +769,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
struct xhci_dequeue_state deq_state;
- if (unlikely(TRB_TO_SUSPEND_PORT(
-
le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3] {
- slot_id = TRB_TO_SLOT_ID(
- le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3]));
+ if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb->generic.field[3] {
+ slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
virt_dev = xhci->devs[slot_id];
if (virt_dev)
handle_cmd_in_cmd_wait_list(xhci, virt_dev,
--
1.8.3.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
[RFC v3 15/18] xhci: add argument 'slot_id' in stop_ep, set_deq and reset_ep cmd handlers
Since the Slot ID field in the command completion event matches the Slot ID
field in the associated command TRB for the Stop Endpoint, Set Dequeue Pointer
and Reset Endpoint commands, this patch adds in the handlers of their
completion events a 'slot_id' argument and removes the slot id calculation
in each of them.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7a58095..405db44 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,10 +755,9 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
* 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the
chain
* bit cleared) so that the HW will skip over them.
*/
-static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
union xhci_trb *trb, struct xhci_event_cmd *event)
{
- unsigned int slot_id;
unsigned int ep_index;
struct xhci_virt_device *virt_dev;
struct xhci_ring *ep_ring;
@@ -770,7 +769,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
struct xhci_dequeue_state deq_state;
if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb->generic.field[3] {
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
virt_dev = xhci->devs[slot_id];
if (virt_dev)
handle_cmd_in_cmd_wait_list(xhci, virt_dev,
@@ -783,7 +781,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
}
memset(&deq_state, 0, sizeof(deq_state));
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
ep = &xhci->devs[slot_id]->eps[ep_index];
@@ -1061,10 +1058,9 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* endpoint doorbell to restart the ring, but only if there aren't more
* cancellations pending.
*/
-static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event, union xhci_trb *trb)
{
- unsigned int slot_id;
unsigned int ep_index;
unsigned int stream_id;
struct xhci_ring *ep_ring;
@@ -1072,7 +1068,6 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
struct xhci_ep_ctx *ep_ctx;
struct xhci_slot_ctx *slot_ctx;
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2]));
dev = xhci->devs[slot_id];
@@ -1154,13 +1149,11 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd
*xhci,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
-static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event, union xhci_trb *trb)
{
- int slot_id;
unsigned int ep_index;
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
/* This command will only fail if the endpoint wasn't halted,
* but we don't care.
@@ -1554,15 +1547,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_STOP_RING:
- xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
+ xhci_handle_cmd_stop_ep(xhci, slot_id, cmd_trb, event);
break;
case TRB_SET_DEQ:
- xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
+ xhci_handle_cmd_set_deq(xhci, slot_id, event, cmd_trb);
break;
case TRB_CMD_NOOP:
break;
case TRB_RESET_EP:
- xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
+ xhci_handle_cmd_reset_ep(xhci, slot_id, event, cmd_trb);
break;
case TRB_RESET_DEV:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
--
1.8.3.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
[RFC v3 13/18] xhci: add variable 'cmd_type' in handle_cmd_completion()
This patch adds a new variable 'cmd_type' to hold the command type so that
switch cases can be simplified by removing TRB_TYPE() macro.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4ddb72e..60cf547 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1505,6 +1505,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
dma_addr_t cmd_dequeue_dma;
u32 cmd_comp_code;
union xhci_trb *cmd_trb;
+ u32 cmd_type;
cmd_dma = le64_to_cpu(event->cmd_trb);
cmd_trb = xhci->cmd_ring->dequeue;
@@ -1537,38 +1538,38 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
}
- switch (le32_to_cpu(cmd_trb->generic.field[3])
- & TRB_TYPE_BITMASK) {
- case TRB_TYPE(TRB_ENABLE_SLOT):
+ cmd_type = TRB_FIELD_TO_TYPE(le32_to_cpu(cmd_trb->generic.field[3]));
+ switch (cmd_type) {
+ case TRB_ENABLE_SLOT:
xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
break;
- case TRB_TYPE(TRB_DISABLE_SLOT):
+ case TRB_DISABLE_SLOT:
xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
- case TRB_TYPE(TRB_CONFIG_EP):
+ case TRB_CONFIG_EP:
xhci_handle_cmd_config_ep(xhci, slot_id, event, cmd_comp_code);
break;
- case TRB_TYPE(TRB_EVAL_CONTEXT):
+ case TRB_EVAL_CONTEXT:
xhci_handle_cmd_eval_ctx(xhci, slot_id, event, cmd_comp_code);
break;
- case TRB_TYPE(TRB_ADDR_DEV):
+ case TRB_ADDR_DEV:
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
- case TRB_TYPE(TRB_STOP_RING):
+ case TRB_STOP_RING:
xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
break;
- case TRB_TYPE(TRB_SET_DEQ):
+ case TRB_SET_DEQ:
xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
break;
- case TRB_TYPE(TRB_CMD_NOOP):
+ case TRB_CMD_NOOP:
break;
- case TRB_TYPE(TRB_RESET_EP):
+ case TRB_RESET_EP:
xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
break;
- case TRB_TYPE(TRB_RESET_DEV):
+ case TRB_RESET_DEV:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
- case TRB_TYPE(TRB_NEC_GET_FW):
+ case TRB_NEC_GET_FW:
xhci_handle_cmd_nec_get_fw(xhci, event);
break;
default:
--
1.8.3.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
[RFC v3 12/18] xhci: add variable 'cmd_trb' in handle_cmd_completion()
This patch adds a new variable 'cmd_trb' to hold the address of the
command TRB, that is associated with the command completion event,
and to replace repetitions of xhci->cmd_ring->dequeue into the code.
Signed-off-by: Xenia Ragiadakou
---
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 5b85582..4ddb72e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1504,10 +1504,12 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
u32 cmd_comp_code;
+ union xhci_trb *cmd_trb;
cmd_dma = le64_to_cpu(event->cmd_trb);
+ cmd_trb = xhci->cmd_ring->dequeue;
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
- xhci->cmd_ring->dequeue);
+ cmd_trb);
/* Is the command ring deq ptr out of sync with the deq seg ptr? */
if (cmd_dequeue_dma == 0) {
xhci->error_bitmask |= 1 << 4;
@@ -1519,8 +1521,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
return;
}
- trace_xhci_cmd_completion(&xhci->cmd_ring->dequeue->generic,
- (struct xhci_generic_trb *) event);
+ trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event);
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
if (cmd_comp_code == COMP_CMD_ABORT || cmd_comp_code == COMP_CMD_STOP) {
@@ -1536,7 +1537,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
}
- switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
+ switch (le32_to_cpu(cmd_trb->generic.field[3])
& TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
@@ -1554,15 +1555,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_STOP_RING):
- xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
+ xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
break;
case TRB_TYPE(TRB_SET_DEQ):
- xhci_handle_cmd_set_deq(xhci, event, xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
break;
case TRB_TYPE(TRB_CMD_NOOP):
break;
case TRB_TYPE(TRB_RESET_EP):
- xhci_handle_cmd_reset_ep(xhci, event, xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
break;
case TRB_TYPE(TRB_RESET_DEV):
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
--
1.8.3.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
[RFC v3 09/18] xhci: remove unused 'ep_ring' variable in handle_cmd_completion()
This patch removes the variable 'ep_ring' that is assigned in
TRB_CONFIG_EP switch case but never used.
Signed-off-by: Xenia Ragiadakou
---
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 0f571c5..b96 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1448,7 +1448,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_input_control_ctx *ctrl_ctx;
struct xhci_virt_device *virt_dev;
unsigned int ep_index;
- struct xhci_ring *ep_ring;
unsigned int ep_state;
cmd_dma = le64_to_cpu(event->cmd_trb);
@@ -1522,7 +1521,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
ep_index != (unsigned int) -1 &&
le32_to_cpu(ctrl_ctx->add_flags) - SLOT_FLAG ==
le32_to_cpu(ctrl_ctx->drop_flags)) {
- ep_ring = xhci->devs[slot_id]->eps[ep_index].ring;
ep_state = xhci->devs[slot_id]->eps[ep_index].ep_state;
if (!(ep_state & EP_HALTED))
goto bandwidth_change;
--
1.8.3.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
[RFC v3 03/18] xhci: refactor TRB_ENABLE_SLOT case into function
This patch refactors the code in TRB_ENABLE_SLOT switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_enable_slot().
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ffd224c..0abf88c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1370,6 +1370,16 @@ static int handle_stopped_cmd_ring(struct xhci_hcd *xhci,
return cur_trb_is_good;
}
+static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci, int slot_id,
+ u32 cmd_comp_code)
+{
+ if (cmd_comp_code == COMP_SUCCESS)
+ xhci->slot_id = slot_id;
+ else
+ xhci->slot_id = 0;
+ complete(&xhci->addr_dev);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1417,11 +1427,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
& TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
- if (GET_COMP_CODE(le32_to_cpu(event->status)) == COMP_SUCCESS)
- xhci->slot_id = slot_id;
- else
- xhci->slot_id = 0;
- complete(&xhci->addr_dev);
+ xhci_handle_cmd_enable_slot(xhci, slot_id,
+ GET_COMP_CODE(le32_to_cpu(event->status)));
break;
case TRB_TYPE(TRB_DISABLE_SLOT):
if (xhci->devs[slot_id]) {
--
1.8.3.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
[RFC v3 17/18] xhci: add label 'update_ring' in handle_cmd_completion()
This patch adds the label 'update_ring' for the common code path:
inc_deq(xhci, xhci->cmd_ring);
return;
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e13531f..03f65dc 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1522,10 +1522,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
* been stopped by host. So we should handle it normally.
* Otherwise, driver should invoke inc_deq() and return.
*/
- if (handle_stopped_cmd_ring(xhci, cmd_comp_code)) {
- inc_deq(xhci, xhci->cmd_ring);
- return;
- }
+ if (handle_stopped_cmd_ring(xhci, cmd_comp_code))
+ goto update_ring;
}
cmd_type = TRB_FIELD_TO_TYPE(le32_to_cpu(cmd_trb->generic.field[3]));
@@ -1567,7 +1565,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci->error_bitmask |= 1 << 6;
break;
}
+update_ring:
inc_deq(xhci, xhci->cmd_ring);
+ return;
}
static void handle_vendor_event(struct xhci_hcd *xhci,
--
1.8.3.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
[RFC v3 01/18] xhci: remove unused argument from xhci_giveback_urb_in_irq()
This patch removes the "adjective" argument from xhci_giveback_urb_in_irq(),
since it is not used in the function anymore.
Signed-off-by: Xenia Ragiadakou
---
Differences from version 2:
The original patchset was splitted into multiple 'single change'
patches to help patch review. Also, there were added some cleanup
patches that change the existed command handlers to use the new
variables created.
Finally, in the patchset was added the patch for missed periodic
transfer trace so that it can be applied cleanly over the previous
patches, since it affects the same file.
drivers/usb/host/xhci-ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7b35af1..ddbda35 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -716,7 +716,7 @@ static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd
*xhci,
/* Must be called with xhci->lock held in interrupt context */
static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
- struct xhci_td *cur_td, int status, char *adjective)
+ struct xhci_td *cur_td, int status)
{
struct usb_hcd *hcd;
struct urb *urb;
@@ -877,7 +877,7 @@ remove_finished_td:
/* Doesn't matter what we pass for status, since the core will
* just overwrite it (because the URB has been unlinked).
*/
- xhci_giveback_urb_in_irq(xhci, cur_td, 0, "cancelled");
+ xhci_giveback_urb_in_irq(xhci, cur_td, 0);
/* Stop processing the cancelled list if the watchdog timer is
* running.
@@ -987,7 +987,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
if (!list_empty(&cur_td->cancelled_td_list))
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
while (!list_empty(&temp_ep->cancelled_td_list)) {
cur_td = list_first_entry(
@@ -996,7 +996,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
cancelled_td_list);
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
}
}
--
1.8.3.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: [RFC v3 17/18] xhci: add label 'update_ring' in handle_cmd_completion()
On 08/23/2013 04:19 PM, Sergei Shtylyov wrote: Hello. On 23-08-2013 12:15, Xenia Ragiadakou wrote: This patch adds the label 'update_ring' for the common code path: inc_deq(xhci, xhci->cmd_ring); return; Signed-off-by: Xenia Ragiadakou --- drivers/usb/host/xhci-ring.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index e13531f..03f65dc 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xcorrect ithci-ring.c [...] @@ -1567,7 +1565,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, xhci->error_bitmask |= 1 << 6; break; } +update_ring: inc_deq(xhci, xhci->cmd_ring); +return; *return* not needed here, at end of function. } WBR, Sergei Hi Sergei, Yes, you are right. Thx! I will wait Sarah to review the patchset and i will send another version to fix this. If you have any other suggestions, please let me know :) best regards, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v3 03/18] xhci: refactor TRB_ENABLE_SLOT case into function
On 08/25/2013 12:48 AM, Oliver Neukum wrote: On Fri, 2013-08-23 at 11:15 +0300, Xenia Ragiadakou wrote: This patch refactors the code in TRB_ENABLE_SLOT switch case in handle_cmd_completion() into a fuction named xhci_handle_cmd_enable_slot(). Hi, it would be nice, in fact probably a requirement, for the commit message to explain what the benefit of the change is. That is, does it improve readability or something else? Regards Oliver I see, i will fix this in the next version of this RFC. Thanks, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On 08/23/2013 05:07 PM, Alan Stern wrote: On Fri, 23 Aug 2013, Xenia Ragiadakou wrote: Martin is right; the BOS descriptors are leaked in usb_reset_and_verify_device(). We need to store the old descriptor, compare it with the new one following the reset, and delete one of them afterward. I haven't had time to fix this. I'll put it in my list of future tasks to give to Xenia. :) Alan, if you have any other small tasks for OPW interns, please let me know. Sarah Sharp Hi, I will try to fix this leak :) First i would like to ensure that i understood the problem so ... If i understood well the usb device's bos descriptor is allocated everytime hub_port_init() is called without never being freed, right? Yes. I do not understand why to compare the old bos with the new one, and not just release the old one or store the new in the memory allocated for the old one. usb_reset_and_verify_device() checks to see if the descriptors changed when the device was reset. (If they did then we need to re-enumerate the device.) It calls descriptors_changed() to do this checking. Originally there were no BOS descriptors, so of course the routine didn't try to compare them. But now that they exist, we need to compare the old and new descriptor values, same as for the device and config descriptors. And of course we need to deallocate one of the BOS descriptors, instead of leaking it. Alan Stern Thanks for the clarification. I will send a patch as RFC because i am not quite familiar and because i have a problem to run kmemleak effectively (i can only scan for a few seconds, although i have set all necessary kmemleak configuration, i have not figured out why yet). I perform the deallocation outside the descriptors_changed(). That leads to code duplication, but if i perform it inside, the function will do something that is not expected to do, right? Also, another thing is that even if the device descriptors have not change (and thus the bcdUSB), there is still the possibility one of the two bos descriptors to be null while the other one is not, either due to a kmalloc() failure or in case the usb device version is in the range [0x201, 0x210) where bos descriptors are optional and if new firmware is downloaded in the device may has as result to add or remove the bos descriptor support. Am i thinking correctly here? regards, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.
This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/core/hub.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..2801619 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
} /* usb_hub_cleanup() */
static int descriptors_changed(struct usb_device *udev,
- struct usb_device_descriptor *old_device_descriptor)
+ struct usb_device_descriptor *old_device_descriptor,
+ struct usb_host_bos *old_bos)
{
int changed = 0;
unsignedindex;
@@ -4953,6 +4954,19 @@ static int descriptors_changed(struct usb_device *udev,
sizeof(*old_device_descriptor)) != 0)
return 1;
+ if (!udev->wusb && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201) {
+ if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
+ return 1;
+ if (udev->bos) {
+ len = udev->bos->desc->wTotalLength;
+ if (len != old_bos->desc->wTotalLength)
+ return 1;
+ if (memcmp(udev->bos->desc, old_bos->desc,
+ le16_to_cpu(len)))
+ return 1;
+ }
+ }
+
/* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
@@ -5049,6 +5063,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
struct usb_hub *parent_hub;
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
struct usb_device_descriptordescriptor = udev->descriptor;
+ struct usb_host_bos *bos = udev->bos;
int i, ret = 0;
int port1 = udev->portnum;
@@ -5099,12 +5114,17 @@ static int usb_reset_and_verify_device(struct
usb_device *udev)
goto re_enumerate;
/* Device might have changed firmware (DFU or similar) */
- if (descriptors_changed(udev, &descriptor)) {
+ if (descriptors_changed(udev, &descriptor, bos)) {
dev_info(&udev->dev, "device firmware changed\n");
udev->descriptor = descriptor; /* for disconnect() calls */
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
goto re_enumerate;
}
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
+
/* Restore the device's previous configuration */
if (!udev->actconfig)
goto done;
--
1.8.3.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
[RFC v2] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.
This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.
Signed-off-by: Xenia Ragiadakou
---
Differences from version 2:
- fix identation
- initialize udev->bos to null so that check fails if bos is uninitialized
- move bos deallocation inside 'done' and 're_enumerate' paths to ensure
that the deallocation will be performed even if hub_port_init() fails
- remove check (!udev->wusb && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201)
since the checks that follow suffice
drivers/usb/core/hub.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..2455001 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
} /* usb_hub_cleanup() */
static int descriptors_changed(struct usb_device *udev,
- struct usb_device_descriptor *old_device_descriptor)
+ struct usb_device_descriptor *old_device_descriptor,
+ struct usb_host_bos *old_bos)
{
int changed = 0;
unsignedindex;
@@ -4953,6 +4954,16 @@ static int descriptors_changed(struct usb_device *udev,
sizeof(*old_device_descriptor)) != 0)
return 1;
+ if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
+ return 1;
+ if (udev->bos) {
+ len = udev->bos->desc->wTotalLength;
+ if (len != old_bos->desc->wTotalLength)
+ return 1;
+ if (memcmp(udev->bos->desc, old_bos->desc, le16_to_cpu(len)))
+ return 1;
+ }
+
/* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
@@ -5049,6 +5060,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
struct usb_hub *parent_hub;
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
struct usb_device_descriptordescriptor = udev->descriptor;
+ struct usb_host_bos *bos;
int i, ret = 0;
int port1 = udev->portnum;
@@ -5066,6 +5078,9 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
}
parent_hub = usb_hub_to_struct_hub(parent_hdev);
+ bos = udev->bos;
+ udev->bos = NULL;
+
/* Disable LPM and LTM while we reset the device and reinstall the alt
* settings. Device-initiated LPM settings, and system exit latency
* settings are cleared when the device is reset, so we have to set
@@ -5099,7 +5114,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
goto re_enumerate;
/* Device might have changed firmware (DFU or similar) */
- if (descriptors_changed(udev, &descriptor)) {
+ if (descriptors_changed(udev, &descriptor, bos)) {
dev_info(&udev->dev, "device firmware changed\n");
udev->descriptor = descriptor; /* for disconnect() calls */
goto re_enumerate;
@@ -5172,11 +5187,15 @@ done:
/* Now that the alt settings are re-installed, enable LTM and LPM. */
usb_unlocked_enable_lpm(udev);
usb_enable_ltm(udev);
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
return 0;
re_enumerate:
/* LPM state doesn't matter when we're about to destroy the device. */
hub_port_logical_disconnect(parent_hub, port1);
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
return -ENODEV;
}
--
1.8.3.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
[RFC 1/4] xhci: replace USB_MAXINTERFACES with config->desc.bNumInterface
This patch replaces USB_MAXINTERFACES with config->desc.bNumInterface in
the termination condition for the loop that updates the LPM timeout of the
endpoints on the cofiguration's interfaces, in xhci_calculate_lpm_timeout(),
to avoid unnecessary loop cycles since most configurations come with 1-2
interfaces while USB_MAXINTERFACES is 32.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ba0ec0a..787076e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4574,7 +4574,7 @@ static u16 xhci_calculate_lpm_timeout(struct usb_hcd *hcd,
if (!config)
return timeout;
- for (i = 0; i < USB_MAXINTERFACES; i++) {
+ for (i = 0; i < config->desc.bNumInterfaces; i++) {
struct usb_driver *driver;
struct usb_interface *intf = config->interface[i];
--
1.8.3.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
[RFC 2/4] xhci: fix SCT_FOR_CTX(p) macro
SCT_FOR_CTX(p) is defined as (((p) << 1) & 0x7) in which case if we want
to set the stream context type to SCT_SSA_256 i.e 0x7 (although secondary
stream arrays are not yet supported) using this macro definition we will
get actually 0x6 which is not what we want.
This patch fixes the above issue by defining the SCT_FOR_CTX(p) macro as
(((p) & 0x7) << 1)
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 46aa148..9575088 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -751,7 +751,7 @@ struct xhci_stream_ctx {
};
/* Stream Context Types (section 6.4.1) - bits 3:1 of stream ctx deq ptr */
-#defineSCT_FOR_CTX(p) (((p) << 1) & 0x7)
+#defineSCT_FOR_CTX(p) (((p) & 0x7) << 1)
/* Secondary stream array type, dequeue pointer is to a transfer ring */
#defineSCT_SEC_TR 0
/* Primary stream array type, dequeue pointer is to a transfer ring */
--
1.8.3.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
[RFC 4/4] xhci: remove conversion from generic to pci device in xhci_mem.c
This patch removes the to_pci_dev() conversion performed to generic struct
device since it is not actually useful (the pointer to the generic device
can be used directly rather through a conversion to pci_dev) and it is pci
bus specific.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-mem.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6dc5c8b..f201990 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -432,10 +432,10 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
{
- struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+ struct device *dev = xhci_to_hcd(xhci)->self.controller;
if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
- dma_free_coherent(&pdev->dev,
+ dma_free_coherent(dev,
sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
stream_ctx, dma);
else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
@@ -460,10 +460,10 @@ static struct xhci_stream_ctx
*xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs, dma_addr_t *dma,
gfp_t mem_flags)
{
- struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+ struct device *dev = xhci_to_hcd(xhci)->self.controller;
if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE)
- return dma_alloc_coherent(&pdev->dev,
+ return dma_alloc_coherent(dev,
sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
dma, mem_flags);
else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE)
@@ -1615,7 +1615,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
{
int num_sp;
int i;
- struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+ struct device *dev = xhci_to_hcd(xhci)->self.controller;
if (!xhci->scratchpad)
return;
@@ -1623,13 +1623,13 @@ static void scratchpad_free(struct xhci_hcd *xhci)
num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
for (i = 0; i < num_sp; i++) {
- dma_free_coherent(&pdev->dev, xhci->page_size,
+ dma_free_coherent(dev, xhci->page_size,
xhci->scratchpad->sp_buffers[i],
xhci->scratchpad->sp_dma_buffers[i]);
}
kfree(xhci->scratchpad->sp_dma_buffers);
kfree(xhci->scratchpad->sp_buffers);
- dma_free_coherent(&pdev->dev, num_sp * sizeof(u64),
+ dma_free_coherent(dev, num_sp * sizeof(u64),
xhci->scratchpad->sp_array,
xhci->scratchpad->sp_dma);
kfree(xhci->scratchpad);
@@ -1691,7 +1691,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
void xhci_mem_cleanup(struct xhci_hcd *xhci)
{
- struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+ struct device *dev = xhci_to_hcd(xhci)->self.controller;
struct dev_info *dev_info, *next;
struct xhci_cd *cur_cd, *next_cd;
unsigned long flags;
@@ -1701,7 +1701,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
/* Free the Event Ring Segment Table and the actual Event Ring */
size = sizeof(struct xhci_erst_entry)*(xhci->erst.num_entries);
if (xhci->erst.entries)
- dma_free_coherent(&pdev->dev, size,
+ dma_free_coherent(dev, size,
xhci->erst.entries, xhci->erst.erst_dma_addr);
xhci->erst.entries = NULL;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST");
@@ -1749,7 +1749,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
"Freed medium stream array pool");
if (xhci->dcbaa)
- dma_free_coherent(&pdev->dev, sizeof(*xhci->dcbaa),
+ dma_free_coherent(dev, sizeof(*xhci->dcbaa),
xhci->dcbaa, xhci->dcbaa->dma);
xhci->dcbaa = NULL;
--
1.8.3.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
[RFC 3/4] xhci: remove unnecessary check in xhci_free_stream_info()
This patch removes the unneccessary check 'if (stream_info)' because there is already a check few lines above which ensures that stream_info is not NULL. Signed-off-by: Xenia Ragiadakou --- drivers/usb/host/xhci-mem.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 53b972c..6dc5c8b 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -721,8 +721,7 @@ void xhci_free_stream_info(struct xhci_hcd *xhci, stream_info->stream_ctx_array, stream_info->ctx_array_dma); - if (stream_info) - kfree(stream_info->stream_rings); + kfree(stream_info->stream_rings); kfree(stream_info); } -- 1.8.3.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
[PATCH 1/4] ehci: remove ehci_vdbg() verbose debugging statements
This patch removes ehci_vdbg debugging statements from EHCI host controller
driver because they produce too much information, lowering the signal to noise
ratio when debugging, and because they are not used anymore.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/ehci-hub.c | 6 -
drivers/usb/host/ehci-q.c | 7 --
drivers/usb/host/ehci-sched.c | 56 ++-
drivers/usb/host/ehci.h | 5
4 files changed, 7 insertions(+), 67 deletions(-)
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 3bf9f48..835fc08 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -211,8 +211,6 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd
*ehci,
else
t2 |= PORT_WKOC_E | PORT_WKCONN_E;
}
- ehci_vdbg(ehci, "port %d, %08x -> %08x\n",
- port + 1, t1, t2);
ehci_writel(ehci, t2, reg);
}
@@ -302,8 +300,6 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
}
if (t1 != t2) {
- ehci_vdbg (ehci, "port %d, %08x -> %08x\n",
- port + 1, t1, t2);
ehci_writel(ehci, t2, reg);
changed = 1;
}
@@ -483,7 +479,6 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
if (test_bit(i, &resume_needed)) {
temp &= ~(PORT_RWC_BITS | PORT_SUSPEND | PORT_RESUME);
ehci_writel(ehci, temp, &ehci->regs->port_status [i]);
- ehci_vdbg (ehci, "resumed port %d\n", i + 1);
}
}
@@ -1204,7 +1199,6 @@ static int ehci_hub_control (
wIndex + 1);
temp |= PORT_OWNER;
} else {
- ehci_vdbg (ehci, "port %d reset\n", wIndex + 1);
temp |= PORT_RESET;
temp &= ~PORT_PE;
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 687..cf9f2fb 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -240,13 +240,6 @@ static int qtd_copy_status (
} else {/* unknown */
status = -EPROTO;
}
-
- ehci_vdbg (ehci,
- "dev%d ep%d%s qtd token %08x --> status %d\n",
- usb_pipedevice (urb->pipe),
- usb_pipeendpoint (urb->pipe),
- usb_pipein (urb->pipe) ? "in" : "out",
- token, status);
}
return status;
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 6631089..833c35c 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -327,17 +327,8 @@ static int tt_available (
periodic_tt_usecs (ehci, dev, frame, tt_usecs);
- ehci_vdbg(ehci, "tt frame %d check %d usecs start uframe %d in"
- " schedule %d/%d/%d/%d/%d/%d/%d/%d\n",
- frame, usecs, uframe,
- tt_usecs[0], tt_usecs[1], tt_usecs[2], tt_usecs[3],
- tt_usecs[4], tt_usecs[5], tt_usecs[6], tt_usecs[7]);
-
- if (max_tt_usecs[uframe] <= tt_usecs[uframe]) {
- ehci_vdbg(ehci, "frame %d uframe %d fully scheduled\n",
- frame, uframe);
+ if (max_tt_usecs[uframe] <= tt_usecs[uframe])
return 0;
- }
/* special case for isoc transfers larger than 125us:
* the first and each subsequent fully used uframe
@@ -348,13 +339,8 @@ static int tt_available (
int ufs = (usecs / 125);
int i;
for (i = uframe; i < (uframe + ufs) && i < 8; i++)
- if (0 < tt_usecs[i]) {
- ehci_vdbg(ehci,
- "multi-uframe xfer can't fit "
- "in frame %d uframe %d\n",
- frame, i);
+ if (0 < tt_usecs[i])
return 0;
- }
}
tt_usecs[uframe] += usecs;
@@ -362,12 +348,8 @@ static int tt_available (
carryover_tt_bandwidth(tt_usecs);
/* fail if the carryover pushed bw past the las
[PATCH 4/4] ehci: enable debugging code when CONFIG_DYNAMIC_DEBUG is set
The debugging code for ehci is enabled to run if the DEBUG flag is defined.
This patch enables the debugging code also when the kernel is configured
with dynamic debugging on.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/ehci-dbg.c | 8
drivers/usb/host/ehci-fsl.c | 2 +-
drivers/usb/host/ehci-hcd.c | 6 +++---
drivers/usb/host/ehci-q.c | 4 ++--
drivers/usb/host/ehci-sched.c | 2 +-
drivers/usb/host/ehci.h | 8
6 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index c2f4489..aa5b603 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -18,7 +18,7 @@
/* this file is part of ehci-hcd.c */
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
/* check the values in the HCSPARAMS register
* (host controller _Structural_ parameters)
@@ -62,7 +62,7 @@ static inline void dbg_hcs_params (struct ehci_hcd *ehci,
char *label) {}
#endif
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
/* check the values in the HCCPARAMS register
* (host controller _Capability_ parameters)
@@ -101,7 +101,7 @@ static inline void dbg_hcc_params (struct ehci_hcd *ehci,
char *label) {}
#endif
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
static void __maybe_unused
dbg_qtd (const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
@@ -301,7 +301,7 @@ static inline int __maybe_unused
dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status)
{ return 0; }
-#endif /* DEBUG */
+#endif /* DEBUG || CONFIG_DYNAMIC_DEBUG */
/* functions have the "wrong" filename when they're output... */
#define dbg_status(ehci, label, status) { \
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index e44f442..947b009 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -418,7 +418,7 @@ static int ehci_fsl_mpc512x_drv_suspend(struct device *dev)
struct fsl_usb2_platform_data *pdata = dev_get_platdata(dev);
u32 tmp;
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
u32 mode = ehci_readl(ehci, hcd->regs + FSL_SOC_USB_USBMODE);
mode &= USBMODE_CM_MASK;
tmp = ehci_readl(ehci, hcd->regs + 0x140); /* usbcmd */
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 09a01fb..5d6022f 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1292,7 +1292,7 @@ static int __init ehci_hcd_init(void)
sizeof(struct ehci_qh), sizeof(struct ehci_qtd),
sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
ehci_debug_root = debugfs_create_dir("ehci", usb_debug_root);
if (!ehci_debug_root) {
retval = -ENOENT;
@@ -1341,7 +1341,7 @@ clean2:
platform_driver_unregister(&PLATFORM_DRIVER);
clean0:
#endif
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
debugfs_remove(ehci_debug_root);
ehci_debug_root = NULL;
err_debug:
@@ -1365,7 +1365,7 @@ static void __exit ehci_hcd_cleanup(void)
#ifdef PS3_SYSTEM_BUS_DRIVER
ps3_ehci_driver_unregister(&PS3_SYSTEM_BUS_DRIVER);
#endif
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
debugfs_remove(ehci_debug_root);
#endif
clear_bit(USB_EHCI_LOADED, &usb_hcds_loaded);
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index cf9f2fb..e321804 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -168,13 +168,13 @@ static void ehci_clear_tt_buffer(struct ehci_hcd *ehci,
struct ehci_qh *qh,
* Note: this routine is never called for Isochronous transfers.
*/
if (urb->dev->tt && !usb_pipeint(urb->pipe) && !qh->clearing_tt) {
-#ifdef DEBUG
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
struct usb_device *tt = urb->dev->tt->hub;
dev_dbg(&tt->dev,
"clear tt buffer port %d, a%d ep%d t%08x\n",
urb->dev->ttport, urb->dev->devnum,
usb_pipeendpoint(urb->pipe), token);
-#endif /* DEBUG */
+#endif /* DEBUG || CONFIG_DYNAMIC_DEBUG */
if (!ehci_is_TDI(ehci)
|| urb->dev->tt->hub !=
ehci_to_hcd(ehci)->self.root_hub) {
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 833c35c..85dd24e 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -169,7 +169,7 @@ periodic_usecs (struct ehci_hcd *ehci, unsigned frame,
unsigned uframe)
break;
}
}
-#ifdef DEBUG
+#if defined(D
[PATCH 3/4] ehci: remove duplicate debug_async_open() prototype in ehci-dbg.c
This patch removes the duplicate of debug_async_open() prototype following
three lines below the debug_async_open() declaration.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/ehci-dbg.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 5429d26..c2f4489 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -336,7 +336,6 @@ static inline void remove_debug_files (struct ehci_hcd
*bus) { }
static int debug_async_open(struct inode *, struct file *);
static int debug_periodic_open(struct inode *, struct file *);
static int debug_registers_open(struct inode *, struct file *);
-static int debug_async_open(struct inode *, struct file *);
static ssize_t debug_output(struct file*, char __user*, size_t, loff_t*);
static int debug_close(struct inode *, struct file *);
--
1.8.3.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
[PATCH 2/4] ehci: remove debugging statement with ehci statistics in ehci_stop()
This patch removes the ehci statictics information output in ehci_stop() because they do not provide interesting info. At any case, the current statistics can be viewed by reading the 'registers' file in debugfs. Signed-off-by: Xenia Ragiadakou --- drivers/usb/host/ehci-hcd.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 73c7299..09a01fb 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -440,14 +440,6 @@ static void ehci_stop (struct usb_hcd *hcd) if (ehci->amd_pll_fix == 1) usb_amd_dev_put(); -#ifdef EHCI_STATS - ehci_dbg(ehci, "irq normal %ld err %ld iaa %ld (lost %ld)\n", - ehci->stats.normal, ehci->stats.error, ehci->stats.iaa, - ehci->stats.lost_iaa); - ehci_dbg (ehci, "complete %ld unlink %ld\n", - ehci->stats.complete, ehci->stats.unlink); -#endif - dbg_status (ehci, "ehci_stop completed", ehci_readl(ehci, &ehci->regs->status)); } -- 1.8.3.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: [RFC v2] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On 08/29/2013 02:21 PM, Martin MOKREJŠ wrote: Hi Xenia, thank you for the patch. I tried to reproduce the error with patched 3.10.9 kernel but it seems the kmemleak is indeed gone. Provided I get only these lines logged which used to be followed by kmemleak findings I believe the original fixed: [15885.206032] usb 4-2.1: reset SuperSpeed USB device number 3 using xhci_hcd [15885.225856] usb 4-2.1: Parent hub missing LPM exit latency info. Power management will be impacted. [15885.228445] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ff81b1c8 [15885.228453] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ff81b208 [41990.166310] usb 4-2.1: reset SuperSpeed USB device number 9 using xhci_hcd [41990.187276] usb 4-2.1: Parent hub missing LPM exit latency info. Power management will be impacted. [41990.189285] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ca0381c8 [41990.189287] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ca038208 [65622.903882] usb 4-2.2: reset SuperSpeed USB device number 10 using xhci_hcd [65622.927980] usb 4-2.2: Parent hub missing LPM exit latency info. Power management will be impacted. [65622.929986] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ff81b130 [65622.929989] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 8803ff81b170 Thank you, Martin Hi Martin, Thank you for testing it. By the way, if you find something else that needs to be fixed and you don't have time, let me know. I learn a lot when i try to fix something and by my mistakes. regards, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Include parent hub number in current warning message "Parent hub missing LPM exit latency info"
On 08/29/2013 04:31 PM, Martin MOKREJŠ wrote: Actually, there is some new bug I haven't seen before (this is 3.10.9 kernel). First of all, I see my TI XHCI controller does not use MSI-X anymore, will have to check my .config why is it so. Second, it should have IRQ 45 and 46 according to dmesg. But lspci reports IRQ 16 is used by TI XHCI controller. Funny! I would say this is linux-pci issue but provided XHCI_HCD is special and manages interrupts somewhat on its own you may look into that first before we ask linux-pci developers. Martin MOKREJŠ wrote: Hi Martin, regarding the different IRQ lines reported by lspci and dmesg maybe it is due to the fact that lspci reports the irq field of the pci_dev (when the option -b is not set, because if it is set it will report the physical IRQ line stored the xhc pci configuration register) while dmesg and /proc/interruputs output the msi/msix vector entries. So i suspect that if the controller had not msi/msix capabilities the pci_dev->irq would be also reported in /proc/interrupts. However, i am so new to the field that the most probable is that i say nonsense :) That is what i came to by having a quick look in xhci and pciutils source code. Somebody else would be more suitable to clear out this discrepancy between lspci and /proc/interrupts IRQ line values. best regards, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.
This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/core/hub.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..2455001 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
} /* usb_hub_cleanup() */
static int descriptors_changed(struct usb_device *udev,
- struct usb_device_descriptor *old_device_descriptor)
+ struct usb_device_descriptor *old_device_descriptor,
+ struct usb_host_bos *old_bos)
{
int changed = 0;
unsignedindex;
@@ -4953,6 +4954,16 @@ static int descriptors_changed(struct usb_device *udev,
sizeof(*old_device_descriptor)) != 0)
return 1;
+ if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
+ return 1;
+ if (udev->bos) {
+ len = udev->bos->desc->wTotalLength;
+ if (len != old_bos->desc->wTotalLength)
+ return 1;
+ if (memcmp(udev->bos->desc, old_bos->desc, le16_to_cpu(len)))
+ return 1;
+ }
+
/* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
@@ -5049,6 +5060,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
struct usb_hub *parent_hub;
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
struct usb_device_descriptordescriptor = udev->descriptor;
+ struct usb_host_bos *bos;
int i, ret = 0;
int port1 = udev->portnum;
@@ -5066,6 +5078,9 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
}
parent_hub = usb_hub_to_struct_hub(parent_hdev);
+ bos = udev->bos;
+ udev->bos = NULL;
+
/* Disable LPM and LTM while we reset the device and reinstall the alt
* settings. Device-initiated LPM settings, and system exit latency
* settings are cleared when the device is reset, so we have to set
@@ -5099,7 +5114,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
goto re_enumerate;
/* Device might have changed firmware (DFU or similar) */
- if (descriptors_changed(udev, &descriptor)) {
+ if (descriptors_changed(udev, &descriptor, bos)) {
dev_info(&udev->dev, "device firmware changed\n");
udev->descriptor = descriptor; /* for disconnect() calls */
goto re_enumerate;
@@ -5172,11 +5187,15 @@ done:
/* Now that the alt settings are re-installed, enable LTM and LPM. */
usb_unlocked_enable_lpm(udev);
usb_enable_ltm(udev);
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
return 0;
re_enumerate:
/* LPM state doesn't matter when we're about to destroy the device. */
hub_port_logical_disconnect(parent_hub, port1);
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
return -ENODEV;
}
--
1.8.3.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] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On 08/29/2013 08:53 PM, Greg KH wrote: On Thu, Aug 29, 2013 at 08:37:57PM +0300, Xenia Ragiadakou wrote: In usb_reset_and_verify_device(), hub_port_init() allocates a new bos descriptor to hold the value read by the device. The new bos descriptor has to be compared with the old one in order to figure out if device 's firmware has changed in which case the device has to be reenumerated. In the original code, none of the two descriptors was deallocated leading to memory leaks. This patch compares the old bos descriptor with the new one to detect change in firmware and releases the newly allocated bos descriptor to prevent memory leak. Signed-off-by: Xenia Ragiadakou Shouldn't there be a "Reported-by:" and "Tested-by:" field here as well? thanks, greg k-h Do you mean i need to add, under the signed-off, Reported-by: Martin MOKREJŠ Tested-by: Martin MOKREJŠ and resend as PATCH v2 or just PATCH? Also does it need a Reviewed-by: Alan Stern ? ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.
This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.
Signed-off-by: Xenia Ragiadakou
Reported-by: Martin MOKREJS
Tested-by: Martin MOKREJS
Suggested-by: Alan Stern
---
drivers/usb/core/hub.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..2455001 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
} /* usb_hub_cleanup() */
static int descriptors_changed(struct usb_device *udev,
- struct usb_device_descriptor *old_device_descriptor)
+ struct usb_device_descriptor *old_device_descriptor,
+ struct usb_host_bos *old_bos)
{
int changed = 0;
unsignedindex;
@@ -4953,6 +4954,16 @@ static int descriptors_changed(struct usb_device *udev,
sizeof(*old_device_descriptor)) != 0)
return 1;
+ if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
+ return 1;
+ if (udev->bos) {
+ len = udev->bos->desc->wTotalLength;
+ if (len != old_bos->desc->wTotalLength)
+ return 1;
+ if (memcmp(udev->bos->desc, old_bos->desc, le16_to_cpu(len)))
+ return 1;
+ }
+
/* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
@@ -5049,6 +5060,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
struct usb_hub *parent_hub;
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
struct usb_device_descriptordescriptor = udev->descriptor;
+ struct usb_host_bos *bos;
int i, ret = 0;
int port1 = udev->portnum;
@@ -5066,6 +5078,9 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
}
parent_hub = usb_hub_to_struct_hub(parent_hdev);
+ bos = udev->bos;
+ udev->bos = NULL;
+
/* Disable LPM and LTM while we reset the device and reinstall the alt
* settings. Device-initiated LPM settings, and system exit latency
* settings are cleared when the device is reset, so we have to set
@@ -5099,7 +5114,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
goto re_enumerate;
/* Device might have changed firmware (DFU or similar) */
- if (descriptors_changed(udev, &descriptor)) {
+ if (descriptors_changed(udev, &descriptor, bos)) {
dev_info(&udev->dev, "device firmware changed\n");
udev->descriptor = descriptor; /* for disconnect() calls */
goto re_enumerate;
@@ -5172,11 +5187,15 @@ done:
/* Now that the alt settings are re-installed, enable LTM and LPM. */
usb_unlocked_enable_lpm(udev);
usb_enable_ltm(udev);
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
return 0;
re_enumerate:
/* LPM state doesn't matter when we're about to destroy the device. */
hub_port_logical_disconnect(parent_hub, port1);
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
return -ENODEV;
}
--
1.8.3.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: Include parent hub number in current warning message "Parent hub missing LPM exit latency info"
On 08/29/2013 11:32 PM, Alan Stern wrote:
On Thu, 29 Aug 2013, Martin MOKREJŠ wrote:
[snip]
I understand your point but don't think this is the case. OK, please show me why
is LPM not available. If USB is complaining about that being disabled it could
tell me based on what attribute at least. And printing the PCI device ID at
least
would be helpful. Yeah, will look what that journald does.
Actually, it looks like the missing LPM functionality is caused by a
bug in drivers/usb/core/hcd.c. The register_root_hub() routine does
this:
if (usb_dev->speed == USB_SPEED_SUPER) {
retval = usb_get_bos_descriptor(usb_dev);
if (retval < 0) {
mutex_unlock(&usb_bus_list_lock);
dev_dbg(parent_dev, "can't read %s bos descriptor %d\n",
dev_name(&usb_dev->dev), retval);
return retval;
}
}
Compare this to the code in hub.c:
if (udev->wusb == 0 && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201) {
retval = usb_get_bos_descriptor(udev);
if (!retval) {
udev->lpm_capable = usb_device_supports_lpm(udev);
usb_set_lpm_parameters(udev);
}
}
You can see that register_root_hub() doesn't set udev->lpm_capable.
The missing call to usb_set_lpm_parameters() doesn't matter, because
root hubs don't have parent hubs.
Ksenia, can you send Martin patch to set udev->lpm_capable for root
hubs? Note that this will require you to fix usb_device_supports_lpm()
as well; that routine dereferences udev->parent, but for root hubs
udev->parent is NULL.
[snip]
Alan Stern
Yes, sure! I will work on this tomorrow.
regards,
ksenia
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Include parent hub number in current warning message "Parent hub missing LPM exit latency info"
On 08/29/2013 04:31 PM, Martin MOKREJŠ wrote: Actually, there is some new bug I haven't seen before (this is 3.10.9 kernel). First of all, I see my TI XHCI controller does not use MSI-X anymore, will have to check my .config why is it so. Why are you saying that xhci does not use MSI-X? Can you send me the entire lspci -vv output for your xhci host controllers? The lines that interest me are the Capabilities: MSI: and Capabilities: MSI-X: so you can send just those. If there is a '+' near to Enabled then that means that your xhci uses MSI or MSI-X accordingly. If you have one xhci host controller (and not two), from your /proc/interrupts output i think that it uses MSI-X because if it didn't there could not had been assigned two irqs to it. I say that, because the xhci (as i saw from the implementation of xhci_setup_msi()) it would have requested one irq if xhci driver had falled back to MSI after a failure to setup MSI-X. Second, it should have IRQ 45 and 46 according to dmesg. But lspci reports IRQ 16 is used by TI XHCI controller. Funny! I would say this is linux-pci issue but provided XHCI_HCD is special and manages interrupts somewhat on its own you may look into that first before we ask linux-pci developers. Maybe here lspci code had to check if MSI is enabled and in that case print the irq numbers of the msi descriptors and not the irq field of the pci_dev structure. 0b:00.0 USB controller: Texas Instruments TUSB73x0 SuperSpeed USB 3.0 xHCI Host Controller (rev 02) (prog-if 30 [XHCI]) Subsystem: Dell Device 04b3 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- # cat /proc/interrupts CPU0 0: 22 IO-APIC-edge timer 1: 16 IO-APIC-edge i8042 8: 55 IO-APIC-edge rtc0 9: 0 IO-APIC-fasteoi acpi 12:5090314 IO-APIC-edge i8042 16: 66 IO-APIC-fasteoi ehci_hcd:usb1 23: 28835 IO-APIC-fasteoi ehci_hcd:usb2 40: 0 PCI-MSI-edge pciehp 41: 15985 PCI-MSI-edge i915 42: 14 PCI-MSI-edge mei_me 43: 227029 PCI-MSI-edge ahci 44: 208160 PCI-MSI-edge enp5s0 45: 729889 PCI-MSI-edge xhci_hcd 46: 0 PCI-MSI-edge xhci_hcd 47:940 PCI-MSI-edge snd_hda_intel 48: 1 PCI-MSI-edge iwlwifi NMI: 21635 Non-maskable interrupts LOC:7545378 Local timer interrupts SPU: 0 Spurious interrupts PMI: 21635 Performance monitoring interrupts IWI:1583914 IRQ work interrupts RTR: 0 APIC ICR read retries RES: 0 Rescheduling interrupts CAL: 0 Function call interrupts TLB: 0 TLB shootdowns TRM: 0 Thermal event interrupts THR: 0 Threshold APIC interrupts MCE: 0 Machine check exceptions MCP:254 Machine check polls ERR: 0 MIS: 0 # dmesg | head -n 2 [0.00] Linux version 3.10.9-default-pciehp (root@vostro) (gcc version 4.6.3 (Gentoo 4.6.3 p1.8, pie-0.5.2) ) #8 SMP Tue Aug 27 01:37:03 MEST 2013 [0.00] Command line: root=/dev/sda5 slub_debug=AFPZ pciehp.pciehp_debug=1 pciehp_debug=1 Martin regards, ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.
This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.
Signed-off-by: Xenia Ragiadakou
Reported-by: Martin MOKREJS
Tested-by: Martin MOKREJS
Suggested-by: Alan Stern
---
Differences from v2:
fix incorrect type in assignment ('le16' was assigned to 'unsigned int')
drivers/usb/core/hub.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..46ce3aa 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
} /* usb_hub_cleanup() */
static int descriptors_changed(struct usb_device *udev,
- struct usb_device_descriptor *old_device_descriptor)
+ struct usb_device_descriptor *old_device_descriptor,
+ struct usb_host_bos *old_bos)
{
int changed = 0;
unsignedindex;
@@ -4953,6 +4954,16 @@ static int descriptors_changed(struct usb_device *udev,
sizeof(*old_device_descriptor)) != 0)
return 1;
+ if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
+ return 1;
+ if (udev->bos) {
+ len = le16_to_cpu(udev->bos->desc->wTotalLength);
+ if (len != le16_to_cpu(old_bos->desc->wTotalLength))
+ return 1;
+ if (memcmp(udev->bos->desc, old_bos->desc, len))
+ return 1;
+ }
+
/* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
@@ -5049,6 +5060,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
struct usb_hub *parent_hub;
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
struct usb_device_descriptordescriptor = udev->descriptor;
+ struct usb_host_bos *bos;
int i, ret = 0;
int port1 = udev->portnum;
@@ -5066,6 +5078,9 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
}
parent_hub = usb_hub_to_struct_hub(parent_hdev);
+ bos = udev->bos;
+ udev->bos = NULL;
+
/* Disable LPM and LTM while we reset the device and reinstall the alt
* settings. Device-initiated LPM settings, and system exit latency
* settings are cleared when the device is reset, so we have to set
@@ -5099,7 +5114,7 @@ static int usb_reset_and_verify_device(struct usb_device
*udev)
goto re_enumerate;
/* Device might have changed firmware (DFU or similar) */
- if (descriptors_changed(udev, &descriptor)) {
+ if (descriptors_changed(udev, &descriptor, bos)) {
dev_info(&udev->dev, "device firmware changed\n");
udev->descriptor = descriptor; /* for disconnect() calls */
goto re_enumerate;
@@ -5172,11 +5187,15 @@ done:
/* Now that the alt settings are re-installed, enable LTM and LPM. */
usb_unlocked_enable_lpm(udev);
usb_enable_ltm(udev);
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
return 0;
re_enumerate:
/* LPM state doesn't matter when we're about to destroy the device. */
hub_port_logical_disconnect(parent_hub, port1);
+ usb_release_bos_descriptor(udev);
+ udev->bos = bos;
return -ENODEV;
}
--
1.8.3.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
[PATCH] usbcore: fix incorrect type in assignment in descriptors_changed()
This patch fixes the incorrect assignment of a variable with type 'le16'
to a variable with type 'unsigned int'.
Signed-off-by: Xenia Ragiadakou
Reported-by: kbuild test robot
---
drivers/usb/core/hub.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 2455001..46ce3aa 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4957,10 +4957,10 @@ static int descriptors_changed(struct usb_device *udev,
if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
return 1;
if (udev->bos) {
- len = udev->bos->desc->wTotalLength;
- if (len != old_bos->desc->wTotalLength)
+ len = le16_to_cpu(udev->bos->desc->wTotalLength);
+ if (len != le16_to_cpu(old_bos->desc->wTotalLength))
return 1;
- if (memcmp(udev->bos->desc, old_bos->desc, le16_to_cpu(len)))
+ if (memcmp(udev->bos->desc, old_bos->desc, len))
return 1;
}
--
1.8.3.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 v3] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On 08/31/2013 04:25 AM, Greg KH wrote:
On Sat, Aug 31, 2013 at 04:20:09AM +0300, Xenia Ragiadakou wrote:
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.
This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.
Signed-off-by: Xenia Ragiadakou
Reported-by: Martin MOKREJS
Tested-by: Martin MOKREJS
Suggested-by: Alan Stern
---
Differences from v2:
fix incorrect type in assignment ('le16' was assigned to 'unsigned int')
I don't want to revert your previous patch, as it's already in my tree,
so can you just send a single patch that fixes the issue, giving the
proper "Reported-by:" mark for the person who told you about this
problem.
thanks,.
greg k-h
By the way the other incorrect assignments in core/hub.c reported by the
robot probably is a bug.
Because bU2DevExitLat is __le16 in struct usb_ss_cap_descriptor and it
needs to be converted to
cpu byteorder before assigned to the usb device's lpm parameters.
And since it is reported by robot i suspect it has not been fixed right?
ksenia
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()
On 08/31/2013 07:52 AM, Greg KH wrote:
On Sat, Aug 31, 2013 at 04:52:47AM +0300, Xenia Ragiadakou wrote:
On 08/31/2013 04:25 AM, Greg KH wrote:
On Sat, Aug 31, 2013 at 04:20:09AM +0300, Xenia Ragiadakou wrote:
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.
This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.
Signed-off-by: Xenia Ragiadakou
Reported-by: Martin MOKREJS
Tested-by: Martin MOKREJS
Suggested-by: Alan Stern
---
Differences from v2:
fix incorrect type in assignment ('le16' was assigned to 'unsigned int')
I don't want to revert your previous patch, as it's already in my tree,
so can you just send a single patch that fixes the issue, giving the
proper "Reported-by:" mark for the person who told you about this
problem.
thanks,.
greg k-h
By the way the other incorrect assignments in core/hub.c reported by the
robot probably is a bug.
Because bU2DevExitLat is __le16 in struct usb_ss_cap_descriptor and it
needs to be converted to
cpu byteorder before assigned to the usb device's lpm parameters.
And since it is reported by robot i suspect it has not been fixed right?
Does sparse complain about it? If you build the kernel with:
make C=1
Yes, when run with endian check on. It complains about some other stuff.
I will send a fixing patch.
sparse is run on the files before gcc, and it catches these types of
things. Don't rely on the 0-day buildbot for finding all of the
problems :)
thanks,
greg k-h
I installed sparse and i will use it in the future before submitting
patches.
thanks,
ksenia
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] usbcore: set lpm_capable field for LPM capable root hubs
This patch sets the lpm_capable field for root hubs with LPM capabilities.
Signed-off-by: Xenia Ragiadakou
Reported-by: Martin MOKREJS
Suggested-by: Alan Stern
---
drivers/usb/core/hcd.c | 1 +
drivers/usb/core/hub.c | 7 ++-
drivers/usb/core/usb.h | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 19ad3d2..36598e4 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1038,6 +1038,7 @@ static int register_root_hub(struct usb_hcd *hcd)
dev_name(&usb_dev->dev), retval);
return retval;
}
+ usb_dev->lpm_capable = usb_device_supports_lpm(usb_dev);
}
retval = usb_new_device (usb_dev);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 46ce3aa..14371f8 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -135,7 +135,7 @@ struct usb_hub *usb_hub_to_struct_hub(struct usb_device
*hdev)
return usb_get_intfdata(hdev->actconfig->interface[0]);
}
-static int usb_device_supports_lpm(struct usb_device *udev)
+int usb_device_supports_lpm(struct usb_device *udev)
{
/* USB 2.1 (and greater) devices indicate LPM support through
* their USB 2.0 Extended Capabilities BOS descriptor.
@@ -156,6 +156,11 @@ static int usb_device_supports_lpm(struct usb_device *udev)
"Power management will be impacted.\n");
return 0;
}
+
+ /* udev is root hub */
+ if (!udev->parent)
+ return 1;
+
if (udev->parent->lpm_capable)
return 1;
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 8238577..c493836 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -35,6 +35,7 @@ extern int usb_get_device_descriptor(struct usb_device *dev,
unsigned int size);
extern int usb_get_bos_descriptor(struct usb_device *dev);
extern void usb_release_bos_descriptor(struct usb_device *dev);
+extern int usb_device_supports_lpm(struct usb_device *udev);
extern char *usb_cache_string(struct usb_device *udev, int index);
extern int usb_set_configuration(struct usb_device *dev, int configuration);
extern int usb_choose_configuration(struct usb_device *udev);
--
1.8.3.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
[PATCH 3/3] usbcore: fix read of usbdevfs_ctrltransfer fields in proc_control()
Urb fields are stored in struct usbdevfs_ctrltransfer in CPU byteorder
and not in little endian, so there is no need to be converted.
This bug was reported by sparse.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/core/devio.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 737e3c1..f4f2300 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -898,10 +898,8 @@ static int proc_control(struct dev_state *ps, void __user
*arg)
snoop(&dev->dev, "control urb: bRequestType=%02x "
"bRequest=%02x wValue=%04x "
"wIndex=%04x wLength=%04x\n",
- ctrl.bRequestType, ctrl.bRequest,
- __le16_to_cpup(&ctrl.wValue),
- __le16_to_cpup(&ctrl.wIndex),
- __le16_to_cpup(&ctrl.wLength));
+ ctrl.bRequestType, ctrl.bRequest, ctrl.wValue,
+ ctrl.wIndex, ctrl.wLength);
if (ctrl.bRequestType & 0x80) {
if (ctrl.wLength && !access_ok(VERIFY_WRITE, ctrl.data,
ctrl.wLength)) {
--
1.8.3.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
[PATCH 2/3] usbcore: fix incorrect type in assignment in usb_set_lpm_parameters()
In the bos usb_ss_cap_descriptor structure, bU2DevExitLat is of type __le16. This value is used as it is, without being first converted to the CPU byteorder, for the setup of usb device's usb3_lpm_parameters. This patch fixes that by converting bU2DevExitLat field to the CPU byteorder before the assignmenment to [udev/hub]_u2_del variables. Signed-off-by: Xenia Ragiadakou Reported-by: kbuild test robot --- drivers/usb/core/hub.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 14371f8..fe8d95d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -315,9 +315,9 @@ static void usb_set_lpm_parameters(struct usb_device *udev) return; udev_u1_del = udev->bos->ss_cap->bU1devExitLat; - udev_u2_del = udev->bos->ss_cap->bU2DevExitLat; + udev_u2_del = le16_to_cpu(udev->bos->ss_cap->bU2DevExitLat); hub_u1_del = udev->parent->bos->ss_cap->bU1devExitLat; - hub_u2_del = udev->parent->bos->ss_cap->bU2DevExitLat; + hub_u2_del = le16_to_cpu(udev->parent->bos->ss_cap->bU2DevExitLat); usb_set_lpm_mel(udev, &udev->u1_params, udev_u1_del, hub, &udev->parent->u1_params, hub_u1_del); -- 1.8.3.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/3] usbcore: set lpm_capable field for LPM capable root hubs
On 09/01/2013 06:04 AM, Greg KH wrote: On Sun, Sep 01, 2013 at 02:56:42AM +0200, Martin MOKREJŠ wrote: Martin MOKREJŠ wrote: Hi Xenia, I tried these 3 patches and ... I will rather leave it up to you to decide if everything went right. Attached is a diff of dmesg from unpatched and patched 3.10.9 kernel. USB3 devices were connected before cold bootup, sadly in latter test the ordering changed a bit so that added to the length of the diff. Can't say what those Prolific-related messages mean. Just in case you need more info I attach "lsub -v" as well. One more addition. When I disconnected the external hard drives from the external HUB I got: [ 1677.615301] usb 4-1.1: USB disconnect, device number 4 [ 1677.619345] usb 4-1.1: Set SEL for device-initiated U1 failed. [ 1677.619369] usb 4-1.1: Set SEL for device-initiated U2 failed. I'm seeing these on the 3.10 kernels, and it's really starting to annoy me... greg k-h I think this message is generated because usb_disable_device() calls usb_enable_lpm() (through a call to usb_disable_lpm()) that submits a USB_REQ_SET_SEL request to the device which fails since the device state has been already set (before the call to usb_disable_device()) to the NOTATTACHED state. So maybe we should not call usb_disable_lpm() if the device is not attached. ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbcore: add check on usb device's state before trying to disable lpm
This patch adds a check to ensure that the device's state is not NOTATTACHED,
ATTACHED, POWERED or RECONNECTING before trying to disable lpm, because if
the device is in one of those states the control transfer to disable
device-initiated LPM will fail (as well as any transfer, since usb_submit_urb()
will fail).
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/core/hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index fe8d95d..a6c10f0 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3716,7 +3716,7 @@ int usb_disable_lpm(struct usb_device *udev)
{
struct usb_hcd *hcd;
- if (!udev || !udev->parent ||
+ if (!udev || udev->state < USB_STATE_UNAUTHENTICATED || !udev->parent ||
udev->speed != USB_SPEED_SUPER ||
!udev->lpm_capable)
return 0;
--
1.8.3.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
[PATCH v2] usbcore: add check on usb device's state before trying to disable lpm
This patch adds a check to ensure that the device's state is not NOTATTACHED,
ATTACHED, POWERED or RECONNECTING before trying to disable lpm, because if
the device is in one of those states the control transfer to disable
device-initiated LPM will fail (as well as any transfer, since usb_submit_urb()
will fail).
Signed-off-by: Xenia Ragiadakou
Reported-by: Martin MOKREJS
---
drivers/usb/core/hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index fe8d95d..a6c10f0 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3716,7 +3716,7 @@ int usb_disable_lpm(struct usb_device *udev)
{
struct usb_hcd *hcd;
- if (!udev || !udev->parent ||
+ if (!udev || udev->state < USB_STATE_UNAUTHENTICATED || !udev->parent ||
udev->speed != USB_SPEED_SUPER ||
!udev->lpm_capable)
return 0;
--
1.8.3.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
[RFC v4 13/19] xhci: add variable 'cmd_trb' in handle_cmd_completion()
This patch adds a new variable 'cmd_trb' to hold the address of the
command TRB, that is associated with the command completion event,
and to replace repetitions of xhci->cmd_ring->dequeue into the code.
Signed-off-by: Xenia Ragiadakou
Acked-by: Sarah Sharp
---
drivers/usb/host/xhci-ring.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index dee663a..778c04b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1506,10 +1506,12 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
u32 cmd_comp_code;
+ union xhci_trb *cmd_trb;
cmd_dma = le64_to_cpu(event->cmd_trb);
+ cmd_trb = xhci->cmd_ring->dequeue;
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
- xhci->cmd_ring->dequeue);
+ cmd_trb);
/* Is the command ring deq ptr out of sync with the deq seg ptr? */
if (cmd_dequeue_dma == 0) {
xhci->error_bitmask |= 1 << 4;
@@ -1521,8 +1523,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
return;
}
- trace_xhci_cmd_completion(&xhci->cmd_ring->dequeue->generic,
- (struct xhci_generic_trb *) event);
+ trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event);
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
if (cmd_comp_code == COMP_CMD_ABORT || cmd_comp_code == COMP_CMD_STOP) {
@@ -1539,9 +1540,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
WARN_ON(slot_id != TRB_TO_SLOT_ID(
- le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])));
+ le32_to_cpu(cmd_trb->generic.field[3])));
- switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
+ switch (le32_to_cpu(cmd_trb->generic.field[3])
& TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
@@ -1559,15 +1560,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_STOP_RING):
- xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
+ xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
break;
case TRB_TYPE(TRB_SET_DEQ):
- xhci_handle_cmd_set_deq(xhci, event, xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
break;
case TRB_TYPE(TRB_CMD_NOOP):
break;
case TRB_TYPE(TRB_RESET_EP):
- xhci_handle_cmd_reset_ep(xhci, event, xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
break;
case TRB_TYPE(TRB_RESET_DEV):
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
--
1.8.3.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
[RFC v4 11/19] xhci: refactor TRB_CONFIG_EP case into function
The function that handles xHCI command completion is much too long and
there is need to be broken up into individual functions for each command
completion to improve code readablity.
This patch refactors the code in TRB_CONFIG_EP switch case, in
handle_cmd_completion(), into a fuction named xhci_handle_cmd_config_ep().
There were added two additional variables, 'add_flags' and 'drop_flags',
to reduce line length below 80 chars and improve code readability.
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
-add reason for new function creation in change log
-add a couple of newlines for clarity
drivers/usb/host/xhci-ring.c | 114 +++
1 file changed, 62 insertions(+), 52 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 8450fa2..ec6bd3e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1393,6 +1393,66 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd
*xhci, int slot_id)
xhci_free_virt_device(xhci, slot_id);
}
+static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event, u32 cmd_comp_code)
+{
+ struct xhci_virt_device *virt_dev;
+ struct xhci_input_control_ctx *ctrl_ctx;
+ unsigned int ep_index;
+ unsigned int ep_state;
+ u32 add_flags, drop_flags;
+
+ virt_dev = xhci->devs[slot_id];
+ if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
+ return;
+ /*
+* Configure endpoint commands can come from the USB core
+* configuration or alt setting changes, or because the HW
+* needed an extra configure endpoint command after a reset
+* endpoint command or streams were being configured.
+* If the command was for a halted endpoint, the xHCI driver
+* is not waiting on the configure endpoint command.
+*/
+ ctrl_ctx = xhci_get_input_control_ctx(xhci, virt_dev->in_ctx);
+ if (!ctrl_ctx) {
+ xhci_warn(xhci, "Could not get input context, bad type.\n");
+ return;
+ }
+
+ add_flags = le32_to_cpu(ctrl_ctx->add_flags);
+ drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
+ /* Input ctx add_flags are the endpoint index plus one */
+ ep_index = xhci_last_valid_endpoint(add_flags) - 1;
+
+ /* A usb_set_interface() call directly after clearing a halted
+* condition may race on this quirky hardware. Not worth
+* worrying about, since this is prototype hardware. Not sure
+* if this will work for streams, but streams support was
+* untested on this prototype.
+*/
+ if (xhci->quirks & XHCI_RESET_EP_QUIRK &&
+ ep_index != (unsigned int) -1 &&
+ add_flags - SLOT_FLAG == drop_flags) {
+ ep_state = virt_dev->eps[ep_index].ep_state;
+ if (!(ep_state & EP_HALTED))
+ goto bandwidth_change;
+ xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
+ "Completed config ep cmd - "
+ "last ep index = %d, state = %d",
+ ep_index, ep_state);
+ /* Clear internal halted state and restart ring(s) */
+ virt_dev->eps[ep_index].ep_state &= ~EP_HALTED;
+ ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
+ return;
+ }
+bandwidth_change:
+ xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
+ "Completed config ep cmd");
+ virt_dev->cmd_status = cmd_comp_code;
+ complete(&virt_dev->cmd_completion);
+ return;
+}
+
static void xhci_handle_cmd_eval_ctx(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event, u32 cmd_comp_code)
{
@@ -1445,10 +1505,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
- struct xhci_input_control_ctx *ctrl_ctx;
- struct xhci_virt_device *virt_dev;
- unsigned int ep_index;
- unsigned int ep_state;
cmd_dma = le64_to_cpu(event->cmd_trb);
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
@@ -1495,54 +1551,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
case TRB_TYPE(TRB_CONFIG_EP):
- virt_dev = xhci->devs[slot_id];
- if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
- break;
- /*
-* Configure endpoint commands can come from the USB core
-
[RFC v4 18/19] xhci: add label 'update_ring' in handle_cmd_completion()
This patch adds the label 'update_ring' for the common code path:
inc_deq(xhci, xhci->cmd_ring);
return;
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
remove return statement
drivers/usb/host/xhci-ring.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c7fe489..4af43ab 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1524,10 +1524,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
* been stopped by host. So we should handle it normally.
* Otherwise, driver should invoke inc_deq() and return.
*/
- if (handle_stopped_cmd_ring(xhci, cmd_comp_code)) {
- inc_deq(xhci, xhci->cmd_ring);
- return;
- }
+ if (handle_stopped_cmd_ring(xhci, cmd_comp_code))
+ goto update_ring;
}
WARN_ON(slot_id != TRB_TO_SLOT_ID(
@@ -1572,6 +1570,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci->error_bitmask |= 1 << 6;
break;
}
+update_ring:
inc_deq(xhci, xhci->cmd_ring);
}
--
1.8.3.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
[RFC v4 04/19] xhci: refactor TRB_DISABLE_SLOT case into function
The function that handles xHCI command completion is much too long and
there is need to be broken up into individual functions for each command
completion to improve code readablity.
This patch refactors the code in TRB_DISABLE_SLOT switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_disable_slot().
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
add reason for new function creation in change log
drivers/usb/host/xhci-ring.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0abf88c..f9c380e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1380,6 +1380,19 @@ static void xhci_handle_cmd_enable_slot(struct xhci_hcd
*xhci, int slot_id,
complete(&xhci->addr_dev);
}
+static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id)
+{
+ struct xhci_virt_device *virt_dev;
+
+ virt_dev = xhci->devs[slot_id];
+ if (!virt_dev)
+ return;
+ if (xhci->quirks & XHCI_EP_LIMIT_QUIRK)
+ /* Delete default control endpoint resources */
+ xhci_free_device_endpoint_resources(xhci, virt_dev, true);
+ xhci_free_virt_device(xhci, slot_id);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1431,13 +1444,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
GET_COMP_CODE(le32_to_cpu(event->status)));
break;
case TRB_TYPE(TRB_DISABLE_SLOT):
- if (xhci->devs[slot_id]) {
- if (xhci->quirks & XHCI_EP_LIMIT_QUIRK)
- /* Delete default control endpoint resources */
- xhci_free_device_endpoint_resources(xhci,
- xhci->devs[slot_id], true);
- xhci_free_virt_device(xhci, slot_id);
- }
+ xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
case TRB_TYPE(TRB_CONFIG_EP):
virt_dev = xhci->devs[slot_id];
--
1.8.3.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
[RFC v4 02/19] xhci: rename existing Command Completion Event handlers
This patch renames the function handlers of a triggered Command Completion
Event that correspond to each command type into 'xhci_handle_cmd_'.
That is done to give a consistent naming space to all the functions that
handle Command Completion Events and that will permit the code reader to
reference to them more easily.
Signed-off-by: Xenia Ragiadakou
Acked-by: Sarah Sharp
---
drivers/usb/host/xhci-ring.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ddbda35..ffd224c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,7 +755,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
* 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the
chain
* bit cleared) so that the HW will skip over them.
*/
-static void handle_stopped_endpoint(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
union xhci_trb *trb, struct xhci_event_cmd *event)
{
unsigned int slot_id;
@@ -1063,9 +1063,8 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* endpoint doorbell to restart the ring, but only if there aren't more
* cancellations pending.
*/
-static void handle_set_deq_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
unsigned int slot_id;
unsigned int ep_index;
@@ -1157,9 +1156,8 @@ static void handle_set_deq_completion(struct xhci_hcd
*xhci,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
-static void handle_reset_ep_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
int slot_id;
unsigned int ep_index;
@@ -1497,15 +1495,15 @@ bandwidth_change:
complete(&xhci->addr_dev);
break;
case TRB_TYPE(TRB_STOP_RING):
- handle_stopped_endpoint(xhci, xhci->cmd_ring->dequeue, event);
+ xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
break;
case TRB_TYPE(TRB_SET_DEQ):
- handle_set_deq_completion(xhci, event, xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_set_deq(xhci, event, xhci->cmd_ring->dequeue);
break;
case TRB_TYPE(TRB_CMD_NOOP):
break;
case TRB_TYPE(TRB_RESET_EP):
- handle_reset_ep_completion(xhci, event,
xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_reset_ep(xhci, event, xhci->cmd_ring->dequeue);
break;
case TRB_TYPE(TRB_RESET_DEV):
xhci_dbg(xhci, "Completed reset device command.\n");
--
1.8.3.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
[RFC v4 10/19] xhci: remove unused 'ep_ring' variable in handle_cmd_completion()
This patch removes the variable 'ep_ring' that is assigned in
TRB_CONFIG_EP switch case but never used.
Signed-off-by: Xenia Ragiadakou
---
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 f9d377a..8450fa2 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1448,7 +1448,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_input_control_ctx *ctrl_ctx;
struct xhci_virt_device *virt_dev;
unsigned int ep_index;
- struct xhci_ring *ep_ring;
unsigned int ep_state;
cmd_dma = le64_to_cpu(event->cmd_trb);
@@ -1525,7 +1524,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
ep_index != (unsigned int) -1 &&
le32_to_cpu(ctrl_ctx->add_flags) - SLOT_FLAG ==
le32_to_cpu(ctrl_ctx->drop_flags)) {
- ep_ring = xhci->devs[slot_id]->eps[ep_index].ring;
ep_state = xhci->devs[slot_id]->eps[ep_index].ep_state;
if (!(ep_state & EP_HALTED))
goto bandwidth_change;
--
1.8.3.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
[RFC v4 16/19] xhci: add argument 'slot_id' in stop_ep, set_deq and reset_ep cmd handlers
Since the Slot ID field in the command completion event matches the Slot ID
field in the associated command TRB for the Stop Endpoint, Set Dequeue Pointer
and Reset Endpoint commands, this patch adds in the handlers of their
completion events a 'slot_id' argument and removes the slot id calculation
in each of them.
Signed-off-by: Xenia Ragiadakou
---
drivers/usb/host/xhci-ring.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 485ce5c..9cb0f42 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,10 +755,9 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
* 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the
chain
* bit cleared) so that the HW will skip over them.
*/
-static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
union xhci_trb *trb, struct xhci_event_cmd *event)
{
- unsigned int slot_id;
unsigned int ep_index;
struct xhci_virt_device *virt_dev;
struct xhci_ring *ep_ring;
@@ -770,7 +769,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
struct xhci_dequeue_state deq_state;
if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb->generic.field[3] {
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
virt_dev = xhci->devs[slot_id];
if (virt_dev)
handle_cmd_in_cmd_wait_list(xhci, virt_dev,
@@ -783,7 +781,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
}
memset(&deq_state, 0, sizeof(deq_state));
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
ep = &xhci->devs[slot_id]->eps[ep_index];
@@ -1061,10 +1058,9 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* endpoint doorbell to restart the ring, but only if there aren't more
* cancellations pending.
*/
-static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event, union xhci_trb *trb)
{
- unsigned int slot_id;
unsigned int ep_index;
unsigned int stream_id;
struct xhci_ring *ep_ring;
@@ -1072,7 +1068,6 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
struct xhci_ep_ctx *ep_ctx;
struct xhci_slot_ctx *slot_ctx;
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2]));
dev = xhci->devs[slot_id];
@@ -1154,13 +1149,11 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd
*xhci,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
-static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event, union xhci_trb *trb)
{
- int slot_id;
unsigned int ep_index;
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
/* This command will only fail if the endpoint wasn't halted,
* but we don't care.
@@ -1559,15 +1552,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_STOP_RING:
- xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
+ xhci_handle_cmd_stop_ep(xhci, slot_id, cmd_trb, event);
break;
case TRB_SET_DEQ:
- xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
+ xhci_handle_cmd_set_deq(xhci, slot_id, event, cmd_trb);
break;
case TRB_CMD_NOOP:
break;
case TRB_RESET_EP:
- xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
+ xhci_handle_cmd_reset_ep(xhci, slot_id, event, cmd_trb);
break;
case TRB_RESET_DEV:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
--
1.8.3.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
[RFC v4 15/19] xhci: replace 'xhci->cmd_ring->dequeue' with 'trb' in stop_ep cmd handler
This patch replaces 'xhci->cmd_ring->dequeue' with 'trb', the address of
the command TRB, since it is available to reduce line length.
Signed-off-by: Xenia Ragiadakou
Acked-by: Sarah Sharp
---
drivers/usb/host/xhci-ring.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 829bd88..485ce5c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -769,10 +769,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
struct xhci_dequeue_state deq_state;
- if (unlikely(TRB_TO_SUSPEND_PORT(
-
le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3] {
- slot_id = TRB_TO_SLOT_ID(
- le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3]));
+ if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb->generic.field[3] {
+ slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb->generic.field[3]));
virt_dev = xhci->devs[slot_id];
if (virt_dev)
handle_cmd_in_cmd_wait_list(xhci, virt_dev,
--
1.8.3.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
[RFC v4 08/19] xhci: refactor TRB_NEC_GET_FW case into function
The function that handles xHCI command completion is much too long and
there is need to be broken up into individual functions for each command
completion to improve code readablity.
This patch refactors the code in TRB_NEC_GET_FW switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_nec_get_fw().
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
add reason for new function creation in change log
drivers/usb/host/xhci-ring.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index fb5ba0d..c598d88 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1414,6 +1414,19 @@ static void xhci_handle_cmd_reset_dev(struct xhci_hcd
*xhci, int slot_id,
"for disabled slot %u\n", slot_id);
}
+static void xhci_handle_cmd_nec_get_fw(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event)
+{
+ if (!(xhci->quirks & XHCI_NEC_HOST)) {
+ xhci->error_bitmask |= 1 << 6;
+ return;
+ }
+ xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
+ "NEC firmware version %2x.%02x",
+ NEC_FW_MAJOR(le32_to_cpu(event->status)),
+ NEC_FW_MINOR(le32_to_cpu(event->status)));
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1547,14 +1560,7 @@ bandwidth_change:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
case TRB_TYPE(TRB_NEC_GET_FW):
- if (!(xhci->quirks & XHCI_NEC_HOST)) {
- xhci->error_bitmask |= 1 << 6;
- break;
- }
- xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
- "NEC firmware version %2x.%02x",
-NEC_FW_MAJOR(le32_to_cpu(event->status)),
-NEC_FW_MINOR(le32_to_cpu(event->status)));
+ xhci_handle_cmd_nec_get_fw(xhci, event);
break;
default:
/* Skip over unknown commands on the event ring */
--
1.8.3.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
[RFC v4 07/19] xhci: refactor TRB_RESET_DEV case into function
The function that handles xHCI command completion is much too long and
there is need to be broken up into individual functions for each command
completion to improve code readablity.
This patch refactors the code in TRB_RESET_DEV switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_reset_dev().
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
add reason for new function creation in change log
drivers/usb/host/xhci-ring.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f68a1f0..fb5ba0d 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1400,6 +1400,20 @@ static void xhci_handle_cmd_addr_dev(struct xhci_hcd
*xhci, int slot_id,
complete(&xhci->addr_dev);
}
+static void xhci_handle_cmd_reset_dev(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event)
+{
+ struct xhci_virt_device *virt_dev;
+
+ xhci_dbg(xhci, "Completed reset device command.\n");
+ virt_dev = xhci->devs[slot_id];
+ if (virt_dev)
+ handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
+ else
+ xhci_warn(xhci, "Reset device command completion "
+ "for disabled slot %u\n", slot_id);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1530,13 +1544,7 @@ bandwidth_change:
xhci_handle_cmd_reset_ep(xhci, event, xhci->cmd_ring->dequeue);
break;
case TRB_TYPE(TRB_RESET_DEV):
- xhci_dbg(xhci, "Completed reset device command.\n");
- virt_dev = xhci->devs[slot_id];
- if (virt_dev)
- handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
- else
- xhci_warn(xhci, "Reset device command completion "
- "for disabled slot %u\n", slot_id);
+ xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
case TRB_TYPE(TRB_NEC_GET_FW):
if (!(xhci->quirks & XHCI_NEC_HOST)) {
--
1.8.3.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
[RFC v4 14/19] xhci: add variable 'cmd_type' in handle_cmd_completion()
This patch adds a new variable 'cmd_type' to hold the command type so that
switch cases can be simplified by removing TRB_TYPE() macro improving
code readability.
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
update changelog to report the reason for such change
drivers/usb/host/xhci-ring.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 778c04b..829bd88 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1507,6 +1507,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
dma_addr_t cmd_dequeue_dma;
u32 cmd_comp_code;
union xhci_trb *cmd_trb;
+ u32 cmd_type;
cmd_dma = le64_to_cpu(event->cmd_trb);
cmd_trb = xhci->cmd_ring->dequeue;
@@ -1542,38 +1543,38 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
WARN_ON(slot_id != TRB_TO_SLOT_ID(
le32_to_cpu(cmd_trb->generic.field[3])));
- switch (le32_to_cpu(cmd_trb->generic.field[3])
- & TRB_TYPE_BITMASK) {
- case TRB_TYPE(TRB_ENABLE_SLOT):
+ cmd_type = TRB_FIELD_TO_TYPE(le32_to_cpu(cmd_trb->generic.field[3]));
+ switch (cmd_type) {
+ case TRB_ENABLE_SLOT:
xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
break;
- case TRB_TYPE(TRB_DISABLE_SLOT):
+ case TRB_DISABLE_SLOT:
xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
- case TRB_TYPE(TRB_CONFIG_EP):
+ case TRB_CONFIG_EP:
xhci_handle_cmd_config_ep(xhci, slot_id, event, cmd_comp_code);
break;
- case TRB_TYPE(TRB_EVAL_CONTEXT):
+ case TRB_EVAL_CONTEXT:
xhci_handle_cmd_eval_ctx(xhci, slot_id, event, cmd_comp_code);
break;
- case TRB_TYPE(TRB_ADDR_DEV):
+ case TRB_ADDR_DEV:
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
- case TRB_TYPE(TRB_STOP_RING):
+ case TRB_STOP_RING:
xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
break;
- case TRB_TYPE(TRB_SET_DEQ):
+ case TRB_SET_DEQ:
xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
break;
- case TRB_TYPE(TRB_CMD_NOOP):
+ case TRB_CMD_NOOP:
break;
- case TRB_TYPE(TRB_RESET_EP):
+ case TRB_RESET_EP:
xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
break;
- case TRB_TYPE(TRB_RESET_DEV):
+ case TRB_RESET_DEV:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
- case TRB_TYPE(TRB_NEC_GET_FW):
+ case TRB_NEC_GET_FW:
xhci_handle_cmd_nec_get_fw(xhci, event);
break;
default:
--
1.8.3.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
[RFC v4 09/19] xhci: refactor TRB_EVAL_CONTEXT case into function
The function that handles xHCI command completion is much too long and
there is need to be broken up into individual functions for each command
completion to improve code readablity.
This patch refactors the code in TRB_EVAL_CONTEXT switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_eval_ctx().
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
add reason for new function creation in change log
drivers/usb/host/xhci-ring.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c598d88..f9d377a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1393,6 +1393,18 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd
*xhci, int slot_id)
xhci_free_virt_device(xhci, slot_id);
}
+static void xhci_handle_cmd_eval_ctx(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event, u32 cmd_comp_code)
+{
+ struct xhci_virt_device *virt_dev;
+
+ virt_dev = xhci->devs[slot_id];
+ if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
+ return;
+ virt_dev->cmd_status = cmd_comp_code;
+ complete(&virt_dev->cmd_completion);
+}
+
static void xhci_handle_cmd_addr_dev(struct xhci_hcd *xhci, int slot_id,
u32 cmd_comp_code)
{
@@ -1535,11 +1547,8 @@ bandwidth_change:
complete(&xhci->devs[slot_id]->cmd_completion);
break;
case TRB_TYPE(TRB_EVAL_CONTEXT):
- virt_dev = xhci->devs[slot_id];
- if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
- break;
- xhci->devs[slot_id]->cmd_status =
GET_COMP_CODE(le32_to_cpu(event->status));
- complete(&xhci->devs[slot_id]->cmd_completion);
+ xhci_handle_cmd_eval_ctx(xhci, slot_id, event,
+ GET_COMP_CODE(le32_to_cpu(event->status)));
break;
case TRB_TYPE(TRB_ADDR_DEV):
xhci_handle_cmd_addr_dev(xhci, slot_id,
--
1.8.3.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
[RFC v4 12/19] xhci: add variable 'cmd_comp_code' in handle_cmd_completion()
This patch adds a new variable 'cmd_comp_code' to hold the command completion
status code aiming to reduce code duplication and to improve code readability.
Signed-off-by: Xenia Ragiadakou
Acked-by: Sarah Sharp
---
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 ec6bd3e..dee663a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1505,6 +1505,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
+ u32 cmd_comp_code;
cmd_dma = le64_to_cpu(event->cmd_trb);
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
@@ -1523,16 +1524,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
trace_xhci_cmd_completion(&xhci->cmd_ring->dequeue->generic,
(struct xhci_generic_trb *) event);
- if ((GET_COMP_CODE(le32_to_cpu(event->status)) == COMP_CMD_ABORT) ||
- (GET_COMP_CODE(le32_to_cpu(event->status)) == COMP_CMD_STOP)) {
+ cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
+ if (cmd_comp_code == COMP_CMD_ABORT || cmd_comp_code == COMP_CMD_STOP) {
/* If the return value is 0, we think the trb pointed by
* command ring dequeue pointer is a good trb. The good
* trb means we don't want to cancel the trb, but it have
* been stopped by host. So we should handle it normally.
* Otherwise, driver should invoke inc_deq() and return.
*/
- if (handle_stopped_cmd_ring(xhci,
- GET_COMP_CODE(le32_to_cpu(event->status {
+ if (handle_stopped_cmd_ring(xhci, cmd_comp_code)) {
inc_deq(xhci, xhci->cmd_ring);
return;
}
@@ -1544,23 +1544,19 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
& TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
- xhci_handle_cmd_enable_slot(xhci, slot_id,
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_DISABLE_SLOT):
xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
case TRB_TYPE(TRB_CONFIG_EP):
- xhci_handle_cmd_config_ep(xhci, slot_id, event,
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ xhci_handle_cmd_config_ep(xhci, slot_id, event, cmd_comp_code);
break;
case TRB_TYPE(TRB_EVAL_CONTEXT):
- xhci_handle_cmd_eval_ctx(xhci, slot_id, event,
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ xhci_handle_cmd_eval_ctx(xhci, slot_id, event, cmd_comp_code);
break;
case TRB_TYPE(TRB_ADDR_DEV):
- xhci_handle_cmd_addr_dev(xhci, slot_id,
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_STOP_RING):
xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
--
1.8.3.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
[RFC v4 19/19] xhci: add trace for missed periodic transfers
There are situations under which xHC is unable to service an isochronous
endpoint within its service interval. For an IN isoc endpoint, this is the case
when its ring is full, while for an OUT isoc endpoint when its ring is empty.
This patch adds a trace event to the class 'xhci_log_msg', called
'xhci_dbg_missed_periodic_tx', to trace the debug statements related to
missed periodic transfers.
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
-make changelog more clear
-add to additional tracepoints
drivers/usb/host/xhci-ring.c | 26 --
drivers/usb/host/xhci-trace.h | 5 +
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4af43ab..e9912cf 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2488,18 +2488,22 @@ static int handle_tx_event(struct xhci_hcd *xhci,
* a Ring Overrun Event for IN Isoch endpoint or Ring
* Underrun Event for OUT Isoch endpoint.
*/
- xhci_dbg(xhci, "underrun event on endpoint\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "underrun event on endpoint");
if (!list_empty(&ep_ring->td_list))
- xhci_dbg(xhci, "Underrun Event for slot %d ep %d "
- "still with TDs queued?\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Underrun Event for slot %d ep %d "
+ "still with TDs queued?",
TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
ep_index);
goto cleanup;
case COMP_OVERRUN:
- xhci_dbg(xhci, "overrun event on endpoint\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "overrun event on endpoint");
if (!list_empty(&ep_ring->td_list))
- xhci_dbg(xhci, "Overrun Event for slot %d ep %d "
- "still with TDs queued?\n",
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Overrun Event for slot %d ep %d "
+ "still with TDs queued?",
TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
ep_index);
goto cleanup;
@@ -2515,7 +2519,8 @@ 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");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Miss service interval error, set skip flag");
goto cleanup;
default:
if (xhci_is_vendor_info_code(xhci, trb_comp_code)) {
@@ -2559,8 +2564,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
/* We've skipped all the TDs on the ep ring when ep->skip set */
if (ep->skip && td_num == 0) {
ep->skip = false;
- xhci_dbg(xhci, "All tds on the ep_ring skipped. "
- "Clear skip flag.\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "All tds on ep_ring skipped. Clear skip flag.");
ret = 0;
goto cleanup;
}
@@ -2615,7 +2620,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
ep_ring->last_td_was_short = false;
if (ep->skip) {
- xhci_dbg(xhci, "Found td. Clear skip flag.\n");
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ "Found td. Clear skip flag.");
ep->skip = false;
}
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 20364cc..c156685 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -67,6 +67,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
TP_ARGS(vaf)
);
+DEFINE_EVENT(xhci_log_msg, xhci_dbg_missed_periodic_tx,
+ TP_PROTO(struct va_format *vaf),
+ TP_ARGS(vaf)
+);
+
DECLARE_EVENT_CLASS(xhci_log_ctx,
TP_PROTO(struct xhci_hcd *xhci, st
[RFC v4 17/19] xhci: replace 'event' with 'cmd_comp_code' in set_deq and reset_ep handlers
This patch replaces the 'event' argument of xhci_handle_cmd_set_deq() and
xhci_handle_cmd_reset_ep(), which is used to retrieve the command completion
status code, with the cmd_comp_code directly, since it is available.
Signed-off-by: Xenia Ragiadakou
Acked-by: Sarah Sharp
---
drivers/usb/host/xhci-ring.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9cb0f42..c7fe489 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1059,7 +1059,7 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* cancellations pending.
*/
static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
- struct xhci_event_cmd *event, union xhci_trb *trb)
+ union xhci_trb *trb, u32 cmd_comp_code)
{
unsigned int ep_index;
unsigned int stream_id;
@@ -1085,11 +1085,11 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd
*xhci, int slot_id,
ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
slot_ctx = xhci_get_slot_ctx(xhci, dev->out_ctx);
- if (GET_COMP_CODE(le32_to_cpu(event->status)) != COMP_SUCCESS) {
+ if (cmd_comp_code != COMP_SUCCESS) {
unsigned int ep_state;
unsigned int slot_state;
- switch (GET_COMP_CODE(le32_to_cpu(event->status))) {
+ switch (cmd_comp_code) {
case COMP_TRB_ERR:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid
because "
"of stream ID configuration\n");
@@ -1112,7 +1112,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd
*xhci, int slot_id,
default:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd with unknown "
"completion code of %u.\n",
- GET_COMP_CODE(le32_to_cpu(event->status)));
+ cmd_comp_code);
break;
}
/* OK what do we do now? The endpoint state is hosed, and we
@@ -1150,7 +1150,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd
*xhci, int slot_id,
}
static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id,
- struct xhci_event_cmd *event, union xhci_trb *trb)
+ union xhci_trb *trb, u32 cmd_comp_code)
{
unsigned int ep_index;
@@ -1159,8 +1159,7 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd
*xhci, int slot_id,
* but we don't care.
*/
xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
- "Ignoring reset ep completion code of %u",
-GET_COMP_CODE(le32_to_cpu(event->status)));
+ "Ignoring reset ep completion code of %u", cmd_comp_code);
/* HW with the reset endpoint quirk needs to have a configure endpoint
* command complete before the endpoint can be used. Queue that here
@@ -1555,12 +1554,12 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_stop_ep(xhci, slot_id, cmd_trb, event);
break;
case TRB_SET_DEQ:
- xhci_handle_cmd_set_deq(xhci, slot_id, event, cmd_trb);
+ xhci_handle_cmd_set_deq(xhci, slot_id, cmd_trb, cmd_comp_code);
break;
case TRB_CMD_NOOP:
break;
case TRB_RESET_EP:
- xhci_handle_cmd_reset_ep(xhci, slot_id, event, cmd_trb);
+ xhci_handle_cmd_reset_ep(xhci, slot_id, cmd_trb, cmd_comp_code);
break;
case TRB_RESET_DEV:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
--
1.8.3.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
[RFC v4 01/19] xhci: remove unused argument from xhci_giveback_urb_in_irq()
This patch removes the "adjective" argument from xhci_giveback_urb_in_irq(),
since it is not used in the function anymore.
Signed-off-by: Xenia Ragiadakou
Acked-by: Sarah Sharp
---
drivers/usb/host/xhci-ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7b35af1..ddbda35 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -716,7 +716,7 @@ static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd
*xhci,
/* Must be called with xhci->lock held in interrupt context */
static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
- struct xhci_td *cur_td, int status, char *adjective)
+ struct xhci_td *cur_td, int status)
{
struct usb_hcd *hcd;
struct urb *urb;
@@ -877,7 +877,7 @@ remove_finished_td:
/* Doesn't matter what we pass for status, since the core will
* just overwrite it (because the URB has been unlinked).
*/
- xhci_giveback_urb_in_irq(xhci, cur_td, 0, "cancelled");
+ xhci_giveback_urb_in_irq(xhci, cur_td, 0);
/* Stop processing the cancelled list if the watchdog timer is
* running.
@@ -987,7 +987,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
if (!list_empty(&cur_td->cancelled_td_list))
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
while (!list_empty(&temp_ep->cancelled_td_list)) {
cur_td = list_first_entry(
@@ -996,7 +996,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
cancelled_td_list);
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
}
}
--
1.8.3.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
[RFC v4 03/19] xhci: refactor TRB_ENABLE_SLOT case into function
The function that handles xHCI command completion is much too long and
there is need to be broken up into individual functions for each command
completion to improve code readablity.
This patch refactors the code in TRB_ENABLE_SLOT switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_enable_slot().
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
add reason for new function creation in change log
drivers/usb/host/xhci-ring.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ffd224c..0abf88c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1370,6 +1370,16 @@ static int handle_stopped_cmd_ring(struct xhci_hcd *xhci,
return cur_trb_is_good;
}
+static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci, int slot_id,
+ u32 cmd_comp_code)
+{
+ if (cmd_comp_code == COMP_SUCCESS)
+ xhci->slot_id = slot_id;
+ else
+ xhci->slot_id = 0;
+ complete(&xhci->addr_dev);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1417,11 +1427,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
& TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
- if (GET_COMP_CODE(le32_to_cpu(event->status)) == COMP_SUCCESS)
- xhci->slot_id = slot_id;
- else
- xhci->slot_id = 0;
- complete(&xhci->addr_dev);
+ xhci_handle_cmd_enable_slot(xhci, slot_id,
+ GET_COMP_CODE(le32_to_cpu(event->status)));
break;
case TRB_TYPE(TRB_DISABLE_SLOT):
if (xhci->devs[slot_id]) {
--
1.8.3.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
[RFC v4 06/19] xhci: use completion event's slot id rather than dig it out of command
Since the slot id retrieved from the command TRB matches the one in Slot ID
field of the command completion event, which is available, there is no need
to determine it again.
This patch removes the uneccessary reassignment to slot id and adds a WARN_ON
in case the two Slot ID fields differ.
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
-the above change is performed now in a separate patch
-a warning is triggered in case the slot id reported in event trb is
different from the slot id renoted in command trb
drivers/usb/host/xhci-ring.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f00d9ef..f68a1f0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1444,6 +1444,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
}
+ WARN_ON(slot_id != TRB_TO_SLOT_ID(
+ le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])));
+
switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
& TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
@@ -1528,8 +1531,6 @@ bandwidth_change:
break;
case TRB_TYPE(TRB_RESET_DEV):
xhci_dbg(xhci, "Completed reset device command.\n");
- slot_id = TRB_TO_SLOT_ID(
- le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3]));
virt_dev = xhci->devs[slot_id];
if (virt_dev)
handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
--
1.8.3.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
[RFC v4 05/19] xhci: refactor TRB_ADDR_DEV case into function
The function that handles xHCI command completion is much too long and
there is need to be broken up into individual functions for each command
completion to improve code readablity.
This patch refactors the code in TRB_ADDR_DEV switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_addr_dev().
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
add reason for new function creation in change log
drivers/usb/host/xhci-ring.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f9c380e..f00d9ef 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1393,6 +1393,13 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd
*xhci, int slot_id)
xhci_free_virt_device(xhci, slot_id);
}
+static void xhci_handle_cmd_addr_dev(struct xhci_hcd *xhci, int slot_id,
+ u32 cmd_comp_code)
+{
+ xhci->devs[slot_id]->cmd_status = cmd_comp_code;
+ complete(&xhci->addr_dev);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1505,8 +1512,8 @@ bandwidth_change:
complete(&xhci->devs[slot_id]->cmd_completion);
break;
case TRB_TYPE(TRB_ADDR_DEV):
- xhci->devs[slot_id]->cmd_status =
GET_COMP_CODE(le32_to_cpu(event->status));
- complete(&xhci->addr_dev);
+ xhci_handle_cmd_addr_dev(xhci, slot_id,
+ GET_COMP_CODE(le32_to_cpu(event->status)));
break;
case TRB_TYPE(TRB_STOP_RING):
xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
--
1.8.3.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
[PATCH v3] usbcore: check usb device's state before sending a Set SEL control transfer
Set SEL control urbs cannot be sent to a device in unconfigured state. This patch adds a check in usb_req_set_sel() to ensure the usb device's state is USB_STATE_CONFIGURED. Signed-off-by: Xenia Ragiadakou Reported-by: Martin MOKREJS Suggested-by: Sarah Sharp --- Differences from v2: -push the check on device's state down to usb_req_set_sel() -check only that the device is in configured state -update commit title and changelog drivers/usb/core/hub.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index fe8d95d..cc468f8 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3424,6 +3424,9 @@ static int usb_req_set_sel(struct usb_device *udev, enum usb3_link_state state) unsigned long long u2_pel; int ret; + if (udev->state != USB_STATE_CONFIGURED) + return 0; + /* Convert SEL and PEL stored in ns to us */ u1_sel = DIV_ROUND_UP(udev->u1_params.sel, 1000); u1_pel = DIV_ROUND_UP(udev->u1_params.pel, 1000); -- 1.8.3.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 v3] usbcore: check usb device's state before sending a Set SEL control transfer
On 09/04/2013 09:25 PM, Martin MOKREJŠ wrote: Hi Xenia, thank you. I tested this patch on 3.11 kernel and the messages don't appear anymore upon LPM-capable device disconnect (tested with ASMedia AS2105 devices). Not much to show here, there is just no error/warning related to LPM while handling these devices. Probably better test is with Prolific-based device for which I get during connect: [ 999.906164] usb 4-2.1: new SuperSpeed USB device number 20 using xhci_hcd [ 999.928475] usb 4-2.1: New USB device found, idVendor=067b, idProduct=2773 [ 999.928488] usb 4-2.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 999.928495] usb 4-2.1: Product: USB-SATA Bridge [ 999.928501] usb 4-2.1: Manufacturer: Prolific Technology Inc. [ 999.928506] usb 4-2.1: SerialNumber: PROLIFICMP7 [ 999.929633] usb 4-2.1: Set SEL for device-initiated U1 failed. [ 999.930008] usb 4-2.1: Set SEL for device-initiated U2 failed. [ 999.930114] usb-storage 4-2.1:1.0: USB Mass Storage device detected [ 999.930178] scsi22 : usb-storage 4-2.1:1.0 [ 999.930804] usb 4-2.1: Set SEL for device-initiated U1 failed. [ 999.931681] usb 4-2.1: Set SEL for device-initiated U2 failed. and during disconnect kernel is also quiet and doesn't complain anymore about the LPM of already disconnected device: [ 1582.039612] usb 4-2.1: USB disconnect, device number 20 I think your patch works correctly and has shutdown the bogus/annoying message. ;-) Thank you. Martin Hi Martin, that 's great! if you notice anything else pls let me know :) ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v4 06/19] xhci: use completion event's slot id rather than dig it out of command
On 09/04/2013 08:39 AM, Xenia Ragiadakou wrote:
Since the slot id retrieved from the command TRB matches the one in Slot ID
field of the command completion event, which is available, there is no need
to determine it again.
This patch removes the uneccessary reassignment to slot id and adds a WARN_ON
in case the two Slot ID fields differ.
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
-the above change is performed now in a separate patch
-a warning is triggered in case the slot id reported in event trb is
different from the slot id renoted in command trb
drivers/usb/host/xhci-ring.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f00d9ef..f68a1f0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1444,6 +1444,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
}
+ WARN_ON(slot_id != TRB_TO_SLOT_ID(
+ le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])));
+
switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
& TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
@@ -1528,8 +1531,6 @@ bandwidth_change:
break;
case TRB_TYPE(TRB_RESET_DEV):
xhci_dbg(xhci, "Completed reset device command.\n");
- slot_id = TRB_TO_SLOT_ID(
- le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3]));
virt_dev = xhci->devs[slot_id];
if (virt_dev)
handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
In this patch i did the stupid thing to place the WARN_ON() in the wrong
place, since the Slot ID field of the Command TRB and the Slot ID field
of the Command Completion Event TRB may differ for other commands. So, I
will fix this and I will resend the patchset.
ksenia
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v5 02/19] xhci: rename existing Command Completion Event handlers
This patch renames the function handlers of a triggered Command Completion
Event that correspond to each command type into 'xhci_handle_cmd_'.
That is done to give a consistent naming space to all the functions that
handle Command Completion Events and that will permit the code reader to
reference to them more easily.
Signed-off-by: Xenia Ragiadakou
Acked-by: Sarah Sharp
---
drivers/usb/host/xhci-ring.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ddbda35..ffd224c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,7 +755,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
* 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the
chain
* bit cleared) so that the HW will skip over them.
*/
-static void handle_stopped_endpoint(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
union xhci_trb *trb, struct xhci_event_cmd *event)
{
unsigned int slot_id;
@@ -1063,9 +1063,8 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* endpoint doorbell to restart the ring, but only if there aren't more
* cancellations pending.
*/
-static void handle_set_deq_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
unsigned int slot_id;
unsigned int ep_index;
@@ -1157,9 +1156,8 @@ static void handle_set_deq_completion(struct xhci_hcd
*xhci,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
-static void handle_reset_ep_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
int slot_id;
unsigned int ep_index;
@@ -1497,15 +1495,15 @@ bandwidth_change:
complete(&xhci->addr_dev);
break;
case TRB_TYPE(TRB_STOP_RING):
- handle_stopped_endpoint(xhci, xhci->cmd_ring->dequeue, event);
+ xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
break;
case TRB_TYPE(TRB_SET_DEQ):
- handle_set_deq_completion(xhci, event, xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_set_deq(xhci, event, xhci->cmd_ring->dequeue);
break;
case TRB_TYPE(TRB_CMD_NOOP):
break;
case TRB_TYPE(TRB_RESET_EP):
- handle_reset_ep_completion(xhci, event,
xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_reset_ep(xhci, event, xhci->cmd_ring->dequeue);
break;
case TRB_TYPE(TRB_RESET_DEV):
xhci_dbg(xhci, "Completed reset device command.\n");
--
1.8.3.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
[RFC v5 01/19] xhci: remove unused argument from xhci_giveback_urb_in_irq()
This patch removes the "adjective" argument from xhci_giveback_urb_in_irq(),
since it is not used in the function anymore.
Signed-off-by: Xenia Ragiadakou
Acked-by: Sarah Sharp
---
drivers/usb/host/xhci-ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7b35af1..ddbda35 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -716,7 +716,7 @@ static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd
*xhci,
/* Must be called with xhci->lock held in interrupt context */
static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
- struct xhci_td *cur_td, int status, char *adjective)
+ struct xhci_td *cur_td, int status)
{
struct usb_hcd *hcd;
struct urb *urb;
@@ -877,7 +877,7 @@ remove_finished_td:
/* Doesn't matter what we pass for status, since the core will
* just overwrite it (because the URB has been unlinked).
*/
- xhci_giveback_urb_in_irq(xhci, cur_td, 0, "cancelled");
+ xhci_giveback_urb_in_irq(xhci, cur_td, 0);
/* Stop processing the cancelled list if the watchdog timer is
* running.
@@ -987,7 +987,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
if (!list_empty(&cur_td->cancelled_td_list))
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
while (!list_empty(&temp_ep->cancelled_td_list)) {
cur_td = list_first_entry(
@@ -996,7 +996,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
cancelled_td_list);
list_del_init(&cur_td->cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, "killed");
+ -ESHUTDOWN);
}
}
}
--
1.8.3.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
[RFC v5 13/19] xhci: add variable 'cmd_trb' in handle_cmd_completion()
This patch adds a new variable 'cmd_trb' to hold the address of the
command TRB, that is associated with the command completion event,
and to replace repetitions of xhci->cmd_ring->dequeue into the code.
Signed-off-by: Xenia Ragiadakou
Acked-by: Sarah Sharp
---
drivers/usb/host/xhci-ring.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index dbf2051..1df73c9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1506,10 +1506,12 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
u32 cmd_comp_code;
+ union xhci_trb *cmd_trb;
cmd_dma = le64_to_cpu(event->cmd_trb);
+ cmd_trb = xhci->cmd_ring->dequeue;
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
- xhci->cmd_ring->dequeue);
+ cmd_trb);
/* Is the command ring deq ptr out of sync with the deq seg ptr? */
if (cmd_dequeue_dma == 0) {
xhci->error_bitmask |= 1 << 4;
@@ -1521,8 +1523,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
return;
}
- trace_xhci_cmd_completion(&xhci->cmd_ring->dequeue->generic,
- (struct xhci_generic_trb *) event);
+ trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event);
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
if (cmd_comp_code == COMP_CMD_ABORT || cmd_comp_code == COMP_CMD_STOP) {
@@ -1538,7 +1539,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
}
- switch (le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])
+ switch (le32_to_cpu(cmd_trb->generic.field[3])
& TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
@@ -1556,19 +1557,19 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_STOP_RING):
- xhci_handle_cmd_stop_ep(xhci, xhci->cmd_ring->dequeue, event);
+ xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
break;
case TRB_TYPE(TRB_SET_DEQ):
- xhci_handle_cmd_set_deq(xhci, event, xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
break;
case TRB_TYPE(TRB_CMD_NOOP):
break;
case TRB_TYPE(TRB_RESET_EP):
- xhci_handle_cmd_reset_ep(xhci, event, xhci->cmd_ring->dequeue);
+ xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
break;
case TRB_TYPE(TRB_RESET_DEV):
WARN_ON(slot_id != TRB_TO_SLOT_ID(
-
le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])));
+ le32_to_cpu(cmd_trb->generic.field[3])));
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
case TRB_TYPE(TRB_NEC_GET_FW):
--
1.8.3.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
[RFC v5 10/19] xhci: remove unused 'ep_ring' variable in handle_cmd_completion()
This patch removes the variable 'ep_ring' that is assigned in
TRB_CONFIG_EP switch case but never used.
Signed-off-by: Xenia Ragiadakou
---
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 08ed322..195c6e7 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1448,7 +1448,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_input_control_ctx *ctrl_ctx;
struct xhci_virt_device *virt_dev;
unsigned int ep_index;
- struct xhci_ring *ep_ring;
unsigned int ep_state;
cmd_dma = le64_to_cpu(event->cmd_trb);
@@ -1522,7 +1521,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
ep_index != (unsigned int) -1 &&
le32_to_cpu(ctrl_ctx->add_flags) - SLOT_FLAG ==
le32_to_cpu(ctrl_ctx->drop_flags)) {
- ep_ring = xhci->devs[slot_id]->eps[ep_index].ring;
ep_state = xhci->devs[slot_id]->eps[ep_index].ep_state;
if (!(ep_state & EP_HALTED))
goto bandwidth_change;
--
1.8.3.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
[RFC v5 07/19] xhci: refactor TRB_RESET_DEV case into function
The function that handles xHCI command completion is much too long and
there is need to be broken up into individual functions for each command
completion to improve code readablity.
This patch refactors the code in TRB_RESET_DEV switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_reset_dev().
Signed-off-by: Xenia Ragiadakou
---
Differences from v3:
add reason for new function creation in change log
drivers/usb/host/xhci-ring.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 8a74002..829ede8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1400,6 +1400,20 @@ static void xhci_handle_cmd_addr_dev(struct xhci_hcd
*xhci, int slot_id,
complete(&xhci->addr_dev);
}
+static void xhci_handle_cmd_reset_dev(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event)
+{
+ struct xhci_virt_device *virt_dev;
+
+ xhci_dbg(xhci, "Completed reset device command.\n");
+ virt_dev = xhci->devs[slot_id];
+ if (virt_dev)
+ handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
+ else
+ xhci_warn(xhci, "Reset device command completion "
+ "for disabled slot %u\n", slot_id);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1529,13 +1543,7 @@ bandwidth_change:
case TRB_TYPE(TRB_RESET_DEV):
WARN_ON(slot_id != TRB_TO_SLOT_ID(
le32_to_cpu(xhci->cmd_ring->dequeue->generic.field[3])));
- xhci_dbg(xhci, "Completed reset device command.\n");
- virt_dev = xhci->devs[slot_id];
- if (virt_dev)
- handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
- else
- xhci_warn(xhci, "Reset device command completion "
- "for disabled slot %u\n", slot_id);
+ xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
case TRB_TYPE(TRB_NEC_GET_FW):
if (!(xhci->quirks & XHCI_NEC_HOST)) {
--
1.8.3.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
