2014-04-22 12:12:37

by Mathias Nyman

[permalink] [raw]
Subject: [PATCH 0/5] xhci: fixes for 3.15-rc usb-linus

Hi Greg

Here are the xhci fixes for 3.15-rc usb-linus.
Most of them are very small fixes that didn't make
it to 3.14, sitting and waiting for 3.15-rc1 to come out.

Only the "Prefer endpoint context.." patch by Julius has a bit more content.

These patches are picked together with Sarah, they are tested on top of
3.15-rc1, and apply on your current usb-linus branch

-Mathias

Alexander Gordeev (1):
xhci: Use pci_enable_msix_exact() instead of pci_enable_msix()

David Cohen (1):
usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM

Denis Turischev (1):
xhci: Switch Intel Lynx Point ports to EHCI on shutdown.

Igor Gnatenko (1):
xhci: extend quirk for Renesas cards

Julius Werner (1):
usb: xhci: Prefer endpoint context dequeue pointer over stopped_trb

drivers/usb/host/xhci-pci.c | 6 ++--
drivers/usb/host/xhci-ring.c | 67 ++++++++++++++++++++------------------------
drivers/usb/host/xhci.c | 9 +++---
drivers/usb/host/xhci.h | 2 --
4 files changed, 38 insertions(+), 46 deletions(-)

--
1.8.3.2


2014-04-22 12:12:48

by Mathias Nyman

[permalink] [raw]
Subject: [PATCH 5/5] usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM

From: David Cohen <[email protected]>

When CONFIG_PCI and CONFIG_PM are not selected, xhci.c gets this
warning:
drivers/usb/host/xhci.c:409:13: warning: ‘xhci_msix_sync_irqs’ defined
but not used [-Wunused-function]

Instead of creating nested #ifdefs, this patch fixes it by defining the
xHCI PCI stubs as inline.

Signed-off-by: David Cohen <[email protected]>
Signed-off-by: Mathias Nyman <[email protected]>
---
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 b0b8399..9e7e9ae 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -408,16 +408,16 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)

#else

-static int xhci_try_enable_msi(struct usb_hcd *hcd)
+static inline int xhci_try_enable_msi(struct usb_hcd *hcd)
{
return 0;
}

-static void xhci_cleanup_msix(struct xhci_hcd *xhci)
+static inline void xhci_cleanup_msix(struct xhci_hcd *xhci)
{
}

-static void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
+static inline void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
{
}

--
1.8.3.2

2014-04-22 12:12:44

by Mathias Nyman

[permalink] [raw]
Subject: [PATCH 3/5] xhci: Switch Intel Lynx Point ports to EHCI on shutdown.

From: Denis Turischev <[email protected]>

The same issue like with Panther Point chipsets. If the USB ports are
switched to xHCI on shutdown, the xHCI host will send a spurious interrupt,
which will wake the system. Some BIOS have work around for this, but not all.
One example is Compulab's mini-desktop, the Intense-PC2.

The bug can be avoided if the USB ports are switched back to EHCI on
shutdown.

Signed-off-by: Denis Turischev <[email protected]>
Signed-off-by: Mathias Nyman <[email protected]>
---
drivers/usb/host/xhci-pci.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 47390e3..1715063 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -134,6 +134,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
*/
if (pdev->subsystem_vendor == PCI_VENDOR_ID_HP)
xhci->quirks |= XHCI_SPURIOUS_WAKEUP;
+
+ xhci->quirks |= XHCI_SPURIOUS_REBOOT;
}
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
pdev->device == PCI_DEVICE_ID_ASROCK_P67) {
--
1.8.3.2

2014-04-22 12:13:35

by Mathias Nyman

[permalink] [raw]
Subject: [PATCH 4/5] xhci: extend quirk for Renesas cards

From: Igor Gnatenko <[email protected]>

After suspend another Renesas PCI-X USB 3.0 card doesn't work.
[root@fedora-20 ~]# lspci -vmnnd 1912:
Device: 03:00.0
Class: USB controller [0c03]
Vendor: Renesas Technology Corp. [1912]
Device: uPD720202 USB 3.0 Host Controller [0015]
SVendor: Renesas Technology Corp. [1912]
SDevice: uPD720202 USB 3.0 Host Controller [0015]
Rev: 02
ProgIf: 30

Reported-and-tested-by: Anatoly Kharchenko <[email protected]>
Reference: http://redmine.russianfedora.pro/issues/1315
Signed-off-by: Igor Gnatenko <[email protected]>
Signed-off-by: Mathias Nyman <[email protected]>
---
drivers/usb/host/xhci-pci.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 1715063..35d4477 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -145,9 +145,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
xhci->quirks |= XHCI_TRUST_TX_LENGTH;
}
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
- pdev->device == 0x0015 &&
- pdev->subsystem_vendor == PCI_VENDOR_ID_SAMSUNG &&
- pdev->subsystem_device == 0xc0cd)
+ pdev->device == 0x0015)
xhci->quirks |= XHCI_RESET_ON_RESUME;
if (pdev->vendor == PCI_VENDOR_ID_VIA)
xhci->quirks |= XHCI_RESET_ON_RESUME;
--
1.8.3.2

2014-04-22 12:13:59

by Mathias Nyman

[permalink] [raw]
Subject: [PATCH 2/5] xhci: Use pci_enable_msix_exact() instead of pci_enable_msix()

From: Alexander Gordeev <[email protected]>

As result of deprecation of MSI-X/MSI enablement functions
pci_enable_msix() and pci_enable_msi_block() all drivers
using these two interfaces need to be updated to use the
new pci_enable_msi_range() or pci_enable_msi_exact()
and pci_enable_msix_range() or pci_enable_msix_exact()
interfaces.

Signed-off-by: Alexander Gordeev <[email protected]>
Cc: Sarah Sharp <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Mathias Nyman <[email protected]>
---
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 988ed5f..b0b8399 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -291,7 +291,7 @@ static int xhci_setup_msix(struct xhci_hcd *xhci)
xhci->msix_entries[i].vector = 0;
}

- ret = pci_enable_msix(pdev, xhci->msix_entries, xhci->msix_count);
+ ret = pci_enable_msix_exact(pdev, xhci->msix_entries, xhci->msix_count);
if (ret) {
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"Failed to enable MSI-X");
--
1.8.3.2

2014-04-22 12:14:31

by Mathias Nyman

[permalink] [raw]
Subject: [PATCH 1/5] usb: xhci: Prefer endpoint context dequeue pointer over stopped_trb

From: Julius Werner <[email protected]>

We have observed a rare cycle state desync bug after Set TR Dequeue
Pointer commands on Intel LynxPoint xHCs (resulting in an endpoint that
doesn't fetch new TRBs and thus an unresponsive USB device). It always
triggers when a previous Set TR Dequeue Pointer command has set the
pointer to the final Link TRB of a segment, and then another URB gets
enqueued and cancelled again before it can be completed. Further
investigation showed that the xHC had returned the Link TRB in the TRB
Pointer field of the Transfer Event (CC == Stopped -- Length Invalid),
but when xhci_find_new_dequeue_state() later accesses the Endpoint
Context's TR Dequeue Pointer field it is set to the first TRB of the
next segment.

The driver expects those two values to be the same in this situation,
and uses the cycle state of the latter together with the address of the
former. This should be fine according to the XHCI specification, since
the endpoint ring should be stopped when returning the Transfer Event
and thus should not advance over the Link TRB before it gets restarted.
However, real-world XHCI implementations apparently don't really care
that much about these details, so the driver should follow a more
defensive approach to try to work around HC spec violations.

This patch removes the stopped_trb variable that had been used to store
the TRB Pointer from the last Transfer Event of a stopped TRB. Instead,
xhci_find_new_dequeue_state() now relies only on the Endpoint Context,
requiring a small amount of additional processing to find the virtual
address corresponding to the TR Dequeue Pointer. Some other parts of the
function were slightly rearranged to better fit into this model.

This patch should be backported to kernels as old as 2.6.31 that contain
the commit ae636747146ea97efa18e04576acd3416e2514f5 "USB: xhci: URB
cancellation support."

Signed-off-by: Julius Werner <[email protected]>
Signed-off-by: Mathias Nyman <[email protected]>
---
drivers/usb/host/xhci-ring.c | 67 ++++++++++++++++++++------------------------
drivers/usb/host/xhci.c | 1 -
drivers/usb/host/xhci.h | 2 --
3 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5f926be..7a0e3c7 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -550,6 +550,7 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
struct xhci_ring *ep_ring;
struct xhci_generic_trb *trb;
dma_addr_t addr;
+ u64 hw_dequeue;

ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id,
ep_index, stream_id);
@@ -559,16 +560,6 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
stream_id);
return;
}
- state->new_cycle_state = 0;
- 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);
- if (!state->new_deq_seg) {
- WARN_ON(1);
- return;
- }

/* Dig out the cycle state saved by the xHC during the stop ep cmd */
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
@@ -577,46 +568,57 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
if (ep->ep_state & EP_HAS_STREAMS) {
struct xhci_stream_ctx *ctx =
&ep->stream_info->stream_ctx_array[stream_id];
- state->new_cycle_state = 0x1 & le64_to_cpu(ctx->stream_ring);
+ hw_dequeue = le64_to_cpu(ctx->stream_ring);
} else {
struct xhci_ep_ctx *ep_ctx
= xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
- state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq);
+ hw_dequeue = le64_to_cpu(ep_ctx->deq);
}

+ /* Find virtual address and segment of hardware dequeue pointer */
+ state->new_deq_seg = ep_ring->deq_seg;
+ state->new_deq_ptr = ep_ring->dequeue;
+ while (xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr)
+ != (dma_addr_t)(hw_dequeue & ~0xf)) {
+ next_trb(xhci, ep_ring, &state->new_deq_seg,
+ &state->new_deq_ptr);
+ if (state->new_deq_ptr == ep_ring->dequeue) {
+ WARN_ON(1);
+ return;
+ }
+ }
+ /*
+ * Find cycle state for last_trb, starting at old cycle state of
+ * hw_dequeue. If there is only one segment ring, find_trb_seg() will
+ * return immediately and cannot toggle the cycle state if this search
+ * wraps around, so add one more toggle manually in that case.
+ */
+ state->new_cycle_state = hw_dequeue & 0x1;
+ if (ep_ring->first_seg == ep_ring->first_seg->next &&
+ cur_td->last_trb < state->new_deq_ptr)
+ state->new_cycle_state ^= 0x1;
+
state->new_deq_ptr = cur_td->last_trb;
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);
+ state->new_deq_ptr, &state->new_cycle_state);
if (!state->new_deq_seg) {
WARN_ON(1);
return;
}

+ /* Increment to find next TRB after last_trb. Cycle if appropriate. */
trb = &state->new_deq_ptr->generic;
if (TRB_TYPE_LINK_LE32(trb->field[3]) &&
(trb->field[3] & cpu_to_le32(LINK_TOGGLE)))
state->new_cycle_state ^= 0x1;
next_trb(xhci, ep_ring, &state->new_deq_seg, &state->new_deq_ptr);

- /*
- * If there is only one segment in a ring, find_trb_seg()'s while loop
- * will not run, and it will return before it has a chance to see if it
- * needs to toggle the cycle bit. It can't tell if the stalled transfer
- * ended just before the link TRB on a one-segment ring, or if the TD
- * wrapped around the top of the ring, because it doesn't have the TD in
- * question. Look for the one-segment case where stalled TRB's address
- * is greater than the new dequeue pointer address.
- */
- 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;
+ /* Don't update the ring cycle state for the producer (us). */
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_trace(xhci, trace_xhci_dbg_cancel_urb,
"New dequeue segment = %p (virtual)",
state->new_deq_seg);
@@ -799,7 +801,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
if (list_empty(&ep->cancelled_td_list)) {
xhci_stop_watchdog_timer_in_irq(xhci, ep);
ep->stopped_td = NULL;
- ep->stopped_trb = NULL;
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
return;
}
@@ -867,11 +868,9 @@ remove_finished_td:
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}

- /* Clear stopped_td and stopped_trb if endpoint is not halted */
- if (!(ep->ep_state & EP_HALTED)) {
+ /* Clear stopped_td if endpoint is not halted */
+ if (!(ep->ep_state & EP_HALTED))
ep->stopped_td = NULL;
- ep->stopped_trb = NULL;
- }

/*
* Drop the lock and complete the URBs in the cancelled TD list.
@@ -1941,14 +1940,12 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd *xhci,
struct xhci_virt_ep *ep = &xhci->devs[slot_id]->eps[ep_index];
ep->ep_state |= EP_HALTED;
ep->stopped_td = td;
- ep->stopped_trb = event_trb;
ep->stopped_stream = stream_id;

xhci_queue_reset_ep(xhci, slot_id, ep_index);
xhci_cleanup_stalled_ring(xhci, td->urb->dev, ep_index);

ep->stopped_td = NULL;
- ep->stopped_trb = NULL;
ep->stopped_stream = 0;

xhci_ring_cmd_db(xhci);
@@ -2030,7 +2027,6 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
* the ring dequeue pointer or take this TD off any lists yet.
*/
ep->stopped_td = td;
- ep->stopped_trb = event_trb;
return 0;
} else {
if (trb_comp_code == COMP_STALL) {
@@ -2042,7 +2038,6 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
* USB class driver clear the stall later.
*/
ep->stopped_td = td;
- ep->stopped_trb = event_trb;
ep->stopped_stream = ep_ring->stream_id;
} else if (xhci_requires_manual_halt_cleanup(xhci,
ep_ctx, trb_comp_code)) {
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8fe4e12..988ed5f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2954,7 +2954,6 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
xhci_ring_cmd_db(xhci);
}
virt_ep->stopped_td = NULL;
- virt_ep->stopped_trb = NULL;
virt_ep->stopped_stream = 0;
spin_unlock_irqrestore(&xhci->lock, flags);

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d280e92..4746816 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -865,8 +865,6 @@ struct xhci_virt_ep {
#define EP_GETTING_NO_STREAMS (1 << 5)
/* ---- Related to URB cancellation ---- */
struct list_head cancelled_td_list;
- /* The TRB that was last reported in a stopped endpoint ring */
- union xhci_trb *stopped_trb;
struct xhci_td *stopped_td;
unsigned int stopped_stream;
/* Watchdog timer for stop endpoint command to cancel URBs */
--
1.8.3.2

2014-04-24 19:46:14

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/5] usb: xhci: Prefer endpoint context dequeue pointer over stopped_trb

On Tue, Apr 22, 2014 at 03:22:58PM +0300, Mathias Nyman wrote:
> From: Julius Werner <[email protected]>
>
> We have observed a rare cycle state desync bug after Set TR Dequeue
> Pointer commands on Intel LynxPoint xHCs (resulting in an endpoint that
> doesn't fetch new TRBs and thus an unresponsive USB device). It always
> triggers when a previous Set TR Dequeue Pointer command has set the
> pointer to the final Link TRB of a segment, and then another URB gets
> enqueued and cancelled again before it can be completed. Further
> investigation showed that the xHC had returned the Link TRB in the TRB
> Pointer field of the Transfer Event (CC == Stopped -- Length Invalid),
> but when xhci_find_new_dequeue_state() later accesses the Endpoint
> Context's TR Dequeue Pointer field it is set to the first TRB of the
> next segment.
>
> The driver expects those two values to be the same in this situation,
> and uses the cycle state of the latter together with the address of the
> former. This should be fine according to the XHCI specification, since
> the endpoint ring should be stopped when returning the Transfer Event
> and thus should not advance over the Link TRB before it gets restarted.
> However, real-world XHCI implementations apparently don't really care
> that much about these details, so the driver should follow a more
> defensive approach to try to work around HC spec violations.
>
> This patch removes the stopped_trb variable that had been used to store
> the TRB Pointer from the last Transfer Event of a stopped TRB. Instead,
> xhci_find_new_dequeue_state() now relies only on the Endpoint Context,
> requiring a small amount of additional processing to find the virtual
> address corresponding to the TR Dequeue Pointer. Some other parts of the
> function were slightly rearranged to better fit into this model.
>
> This patch should be backported to kernels as old as 2.6.31 that contain
> the commit ae636747146ea97efa18e04576acd3416e2514f5 "USB: xhci: URB
> cancellation support."

Ok, but:

>
> Signed-off-by: Julius Werner <[email protected]>
> Signed-off-by: Mathias Nyman <[email protected]>

You don't actually add the stable@ tag here, why not?

You have read Documentation/stable_kernel_rules.txt for how stable
kernel trees work, so why not add the label here?

greg k-h

2014-04-24 19:46:35

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/5] xhci: Use pci_enable_msix_exact() instead of pci_enable_msix()

On Tue, Apr 22, 2014 at 03:22:59PM +0300, Mathias Nyman wrote:
> From: Alexander Gordeev <[email protected]>
>
> As result of deprecation of MSI-X/MSI enablement functions
> pci_enable_msix() and pci_enable_msi_block() all drivers
> using these two interfaces need to be updated to use the
> new pci_enable_msi_range() or pci_enable_msi_exact()
> and pci_enable_msix_range() or pci_enable_msix_exact()
> interfaces.
>
> Signed-off-by: Alexander Gordeev <[email protected]>
> Cc: Sarah Sharp <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Mathias Nyman <[email protected]>
> ---
> 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 988ed5f..b0b8399 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -291,7 +291,7 @@ static int xhci_setup_msix(struct xhci_hcd *xhci)
> xhci->msix_entries[i].vector = 0;
> }
>
> - ret = pci_enable_msix(pdev, xhci->msix_entries, xhci->msix_count);
> + ret = pci_enable_msix_exact(pdev, xhci->msix_entries, xhci->msix_count);

Why is this change needed for 3.15-final?

2014-04-24 19:46:57

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 3/5] xhci: Switch Intel Lynx Point ports to EHCI on shutdown.

On Tue, Apr 22, 2014 at 03:23:00PM +0300, Mathias Nyman wrote:
> From: Denis Turischev <[email protected]>
>
> The same issue like with Panther Point chipsets. If the USB ports are
> switched to xHCI on shutdown, the xHCI host will send a spurious interrupt,
> which will wake the system. Some BIOS have work around for this, but not all.
> One example is Compulab's mini-desktop, the Intense-PC2.
>
> The bug can be avoided if the USB ports are switched back to EHCI on
> shutdown.
>
> Signed-off-by: Denis Turischev <[email protected]>
> Signed-off-by: Mathias Nyman <[email protected]>
> ---
> drivers/usb/host/xhci-pci.c | 2 ++
> 1 file changed, 2 insertions(+)

No stable marking? Why not?

2014-04-24 19:47:13

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 4/5] xhci: extend quirk for Renesas cards

On Tue, Apr 22, 2014 at 03:23:01PM +0300, Mathias Nyman wrote:
> From: Igor Gnatenko <[email protected]>
>
> After suspend another Renesas PCI-X USB 3.0 card doesn't work.
> [root@fedora-20 ~]# lspci -vmnnd 1912:
> Device: 03:00.0
> Class: USB controller [0c03]
> Vendor: Renesas Technology Corp. [1912]
> Device: uPD720202 USB 3.0 Host Controller [0015]
> SVendor: Renesas Technology Corp. [1912]
> SDevice: uPD720202 USB 3.0 Host Controller [0015]
> Rev: 02
> ProgIf: 30
>
> Reported-and-tested-by: Anatoly Kharchenko <[email protected]>
> Reference: http://redmine.russianfedora.pro/issues/1315
> Signed-off-by: Igor Gnatenko <[email protected]>
> Signed-off-by: Mathias Nyman <[email protected]>
> ---
> drivers/usb/host/xhci-pci.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)

Again, no stable?

2014-04-24 19:47:42

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 5/5] usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM

On Tue, Apr 22, 2014 at 03:23:02PM +0300, Mathias Nyman wrote:
> From: David Cohen <[email protected]>
>
> When CONFIG_PCI and CONFIG_PM are not selected, xhci.c gets this
> warning:
> drivers/usb/host/xhci.c:409:13: warning: ‘xhci_msix_sync_irqs’ defined
> but not used [-Wunused-function]
>
> Instead of creating nested #ifdefs, this patch fixes it by defining the
> xHCI PCI stubs as inline.
>
> Signed-off-by: David Cohen <[email protected]>
> Signed-off-by: Mathias Nyman <[email protected]>
> ---
> drivers/usb/host/xhci.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

What patch caused this problem? Does it show up in 3.14? 3.10? 2.0?
:)

Please be more specific...

2014-04-24 19:47:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 0/5] xhci: fixes for 3.15-rc usb-linus

On Tue, Apr 22, 2014 at 03:22:57PM +0300, Mathias Nyman wrote:
> Hi Greg
>
> Here are the xhci fixes for 3.15-rc usb-linus.
> Most of them are very small fixes that didn't make
> it to 3.14, sitting and waiting for 3.15-rc1 to come out.
>
> Only the "Prefer endpoint context.." patch by Julius has a bit more content.
>
> These patches are picked together with Sarah, they are tested on top of
> 3.15-rc1, and apply on your current usb-linus branch

Please redo all of these based on my comments.

thanks,

greg k-h

2014-04-24 20:15:18

by David Cohen

[permalink] [raw]
Subject: Re: [PATCH 5/5] usb/xhci: fix compilation warning when !CONFIG_PCI && !CONFIG_PM

Hi Greg,

On Thu, Apr 24, 2014 at 12:50:32PM -0700, Greg KH wrote:
> On Tue, Apr 22, 2014 at 03:23:02PM +0300, Mathias Nyman wrote:
> > From: David Cohen <[email protected]>
> >
> > When CONFIG_PCI and CONFIG_PM are not selected, xhci.c gets this
> > warning:
> > drivers/usb/host/xhci.c:409:13: warning: ‘xhci_msix_sync_irqs’ defined
> > but not used [-Wunused-function]
> >
> > Instead of creating nested #ifdefs, this patch fixes it by defining the
> > xHCI PCI stubs as inline.
> >
> > Signed-off-by: David Cohen <[email protected]>
> > Signed-off-by: Mathias Nyman <[email protected]>
> > ---
> > drivers/usb/host/xhci.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
>
> What patch caused this problem? Does it show up in 3.14? 3.10? 2.0?
> :)
>
> Please be more specific...

I first noticed this warning on 3.13, but I believe it is older :)
You may consider this patch as an extension of this other one:

commit d5c82feb5cf8026cd4af048330fdcd46e861c686
Author: Olof Johansson <[email protected]>
Date: Tue Jul 23 11:58:20 2013 -0700

usb: xhci: Mark two functions __maybe_unused
--

IMO it makes sense to apply whenever the other patch exists.

Br, David

2014-04-24 20:15:33

by Julius Werner

[permalink] [raw]
Subject: Re: [PATCH 1/5] usb: xhci: Prefer endpoint context dequeue pointer over stopped_trb

> You don't actually add the stable@ tag here, why not?
>
> You have read Documentation/stable_kernel_rules.txt for how stable
> kernel trees work, so why not add the label here?

Sorry... I actually didn't know about that file, I just added the
"should be backported" line because I have seen that in other commits.
I will adhere to those rules in the future.

The patch is a little over the line limit (150+ lines with context,
although most of it are scattered single-line changes). The issue it
solves is that a USB device may silently stop working until it is
disconnected, not sure if that's enough to qualify as "oh, that's not
good". I'll let you decide whether it still should be queued up for
stable backports or not.

2014-04-25 07:59:56

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH 0/5] xhci: fixes for 3.15-rc usb-linus

On 04/24/2014 10:50 PM, Greg KH wrote:
> On Tue, Apr 22, 2014 at 03:22:57PM +0300, Mathias Nyman wrote:
>> Hi Greg
>>
>> Here are the xhci fixes for 3.15-rc usb-linus.
>> Most of them are very small fixes that didn't make
>> it to 3.14, sitting and waiting for 3.15-rc1 to come out.
>>
>> Only the "Prefer endpoint context.." patch by Julius has a bit more content.
>>
>> These patches are picked together with Sarah, they are tested on top of
>> 3.15-rc1, and apply on your current usb-linus branch
>
> Please redo all of these based on my comments.
>
>

Will do
Thanks for the feedback

-Mathias

2014-04-25 10:36:44

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH 2/5] xhci: Use pci_enable_msix_exact() instead of pci_enable_msix()

On 04/24/2014 10:49 PM, Greg KH wrote:
> On Tue, Apr 22, 2014 at 03:22:59PM +0300, Mathias Nyman wrote:
>> From: Alexander Gordeev <[email protected]>
>>
>> As result of deprecation of MSI-X/MSI enablement functions
>> pci_enable_msix() and pci_enable_msi_block() all drivers
>> using these two interfaces need to be updated to use the
>> new pci_enable_msi_range() or pci_enable_msi_exact()
>> and pci_enable_msix_range() or pci_enable_msix_exact()
>> interfaces.
>>
>> Signed-off-by: Alexander Gordeev <[email protected]>
>> Cc: Sarah Sharp <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Mathias Nyman <[email protected]>
>> ---
>> 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 988ed5f..b0b8399 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -291,7 +291,7 @@ static int xhci_setup_msix(struct xhci_hcd *xhci)
>> xhci->msix_entries[i].vector = 0;
>> }
>>
>> - ret = pci_enable_msix(pdev, xhci->msix_entries, xhci->msix_count);
>> + ret = pci_enable_msix_exact(pdev, xhci->msix_entries, xhci->msix_count);
>
> Why is this change needed for 3.15-final?
>

Can't remember why I thought this was needed for 3.15
Looks like other subsystems (like PCI) just queued similar MSI changes
for 3.16.

I'll move it to the patchseries for usb-next

-Mathias