This patch (as1663) fixes a regression caused by commit
6e0c3339a6 (USB: EHCI: unlink one async
QH at a time). In order to avoid keeping multiple QHs in an unusable
intermediate state, that commit changed unlink_empty_async() so that
it unlinks only one empty QH at a time.
However, when the EHCI root hub is suspended, _all_ async QHs need to
be unlinked. ehci_bus_suspend() used to do this by calling
unlink_empty_async(), but now this only unlinks one of the QHs, not
all of them.
The symptom is that when the root hub is resumed, USB communications
don't work for some period of time. This is because ehci-hcd doesn't
realize it needs to restart the async schedule; it assumes that
because some QHs are already on the schedule, the schedule must be
running.
The easiest way to fix the problem is add a new function that unlinks
all the async QHs when the root hub is suspended.
This patch should be applied to all kernels that have the 6e0c3339a6
commit.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: Adrian Bassett <adrian.bassett@hotmail.co.uk>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1661) fixes a rather obscure bug in ehci-hcd. In a
couple of places, the driver compares the DMA address stored in a QH's
overlay region with the address of a particular qTD, in order to see
whether that qTD is the one currently being processed by the hardware.
(If it is then the status in the QH's overlay region is more
up-to-date than the status in the qTD, and if it isn't then the
overlay's value needs to be adjusted when the QH is added back to the
active schedule.)
However, DMA address in the overlay region isn't always valid. It
sometimes will contain a stale value, which may happen by coincidence
to be equal to a qTD's DMA address. Instead of checking the DMA
address, we should check whether the overlay region is active and
valid. The patch tests the ACTIVE bit in the overlay, and clears this
bit when the overlay becomes invalid (which happens when the
currently-executing URB is unlinked).
This is the second part of a fix for the regression reported at:
https://bugs.launchpad.net/bugs/1088733
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Joseph Salisbury <joseph.salisbury@canonical.com>
Reported-and-tested-by: Stephen Thirlwall <sdt@dr.com>
CC: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1660) works around a hardware problem present in some
(if not all) Intel EHCI controllers. After a QH has been unlinked
from the async schedule and the corresponding IAA interrupt has
occurred, the controller is not supposed access the QH and its qTDs.
There certainly shouldn't be any more DMA writes to those structures.
Nevertheless, Intel's controllers have been observed to perform a
final writeback to the QH's overlay region and to the most recent qTD.
For more information and a test program to determine whether this
problem is present in a particular controller, see
http://marc.info/?l=linux-usb&m=135492071812265&w=2http://marc.info/?l=linux-usb&m=136182570800963&w=2
This patch works around the problem by always waiting for two IAA
cycles when unlinking an async QH. The extra IAA delay gives the
controller time to perform its final writeback.
Surprisingly enough, the effects of this silicon bug have gone
undetected until quite recently. More through luck than anything
else, it hasn't caused any apparent problems. However, it does
interact badly with the path that follows this one, so it needs to be
addressed.
This is the first part of a fix for the regression reported at:
https://bugs.launchpad.net/bugs/1088733
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Tested-by: Stephen Thirlwall <sdt@dr.com>
CC: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1648) fixes a regression affecting nVidia EHCI
controllers. Evidently they don't like to have more than one async QH
unlinked at a time. I can't imagine how they manage to mess it up,
but at least one of them does.
The patch changes the async unlink logic in two ways:
Each time an IAA cycle is started, only the first QH on the
async unlink list is handled (rather than all of them).
Async QHs do not all get unlinked as soon as they have been
empty for long enough. Instead, only the last one (i.e., the
one that has been on the schedule the longest) is unlinked,
and then only if no other unlinks are in progress at the time.
This means that when multiple QHs are empty, they won't be unlinked as
quickly as before. That's okay; it won't affect correct operation of
the driver or add an excessive load. Multiple unlinks tend to be
relatively rare in any case.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: Piergiorgio Sartor <piergiorgio.sartor@nexgo.de>
Cc: stable <stable@vger.kernel.org> # 3.6
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1632b) fixes a bug in ehci-hcd. The USB core uses
urb->hcpriv to determine whether or not an URB is active; host
controller drivers are supposed to set this pointer to a non-NULL
value when an URB is queued. However ehci-hcd sets it to NULL for
isochronous URBs, which defeats the check in usbcore.
In itself this isn't a big deal. But people have recently found that
certain sequences of actions will cause the snd-usb-audio driver to
reuse URBs without waiting for them to complete. In the absence of
proper checking by usbcore, the URBs get added to their endpoint list
twice. This leads to list corruption and a system freeze.
The patch makes ehci-hcd assign a meaningful value to urb->hcpriv for
isochronous URBs. Improving robustness always helps.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Artem S. Tashkinov <t.artem@lycos.com>
Reported-by: Christof Meerwald <cmeerw@cmeerw.org>
CC: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
There is a possibility of QH overlay region having reference to a stale
qTD pointer during unlink.
Consider an endpoint having two pending qTD before unlink process begins.
The endpoint's QH queue looks like this.
qTD1 --> qTD2 --> Dummy
To unlink qTD2, QH is removed from asynchronous list and Asynchronous
Advance Doorbell is programmed. The qTD1's next qTD pointer is set to
qTD2'2 next qTD pointer and qTD2 is retired upon controller's doorbell
interrupt. If QH's current qTD pointer points to qTD1, transfer overlay
region still have reference to qTD2. But qtD2 is just unlinked and freed.
This may cause EHCI system error. Fix this by updating qTD next pointer
in QH overlay region with the qTD next pointer of the current qTD.
Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1586) replaces the kernel timer used by ehci-hcd as an
I/O watchdog with an hrtimer event.
Unlike in the current code, the watchdog event is now always enabled
whenever any isochronous URBs are active. This will prevent bugs
caused by the periodic schedule wrapping around with no completion
interrupts; the watchdog handler is guaranteed to scan the isochronous
transfers at least once during each iteration of the schedule. The
extra overhead will be negligible: one timer interrupt every 100 ms.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1585) fixes a bug in ehci-hcd's scheme for scanning
interrupt QHs.
Currently a single routine takes care of scanning everything on the
periodic schedule. Whenever an interrupt occurs, it scans all
isochronous and interrupt URBs scheduled for frames that have elapsed
since the last scan.
This has two disadvantages. The first is relatively minor: An
interrupt QH is likely to end up getting scanned multiple times,
particularly if the last scan was not fairly recent. (The current
code avoids this by maintaining a periodic_stamp in each interrupt
QH.)
The second is more serious. The periodic schedule wraps around. If
the last scan occurred during frame N, and the next scan occurs when
the schedule has gone through an entire cycle and is back at frame N,
the scanning code won't look at any frames other than N. Consequently
it won't see any QHs that completed during frame N-1 or earlier.
The patch replaces the entire frame-based approach for scanning
interrupt QHs with a new routine using a list-based approach, the same
as for async QHs. This has a slight disadvantage, because it means
that all interrupt QHs have to be scanned every time. But it is more
robust than the current approach.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1583) changes ehci-hcd to use an hrtimer event for
unlinking empty (unused) async QHs instead of using a kernel timer.
The check for empty QHs is moved to a new routine, where it doesn't
require going through an entire scan of both the async and periodic
schedules. And it can unlink multiple QHs at once, unlike the current
code.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1582) changes ehci-hcd's strategy for unlinking async
QHs. Currently the driver never unlinks more than one QH at a time.
This can be inefficient and cause unnecessary delays, since a QH
cannot be reused while it is waiting to be unlinked.
The new strategy unlinks all the waiting QHs at once. In practice the
improvement won't be very big, because it's somewhat uncommon to have
two or more QHs waiting to be unlinked at any time. But it does
happen, and in any case, doing things this way makes more sense IMO.
The change requires the async unlinking code to be refactored
slightly. Now in addition to the routines for starting and ending an
unlink, there are new routines for unlinking a single QH and starting
an IAA cycle. This approach is needed because there are two separate
paths for unlinking async QHs:
When a transfer error occurs or an URB is cancelled, the QH
must be unlinked right away;
When a QH has been idle sufficiently long, it is unlinked
to avoid consuming DMA bandwidth uselessly.
In the first case we want the unlink to proceed as quickly as
possible, whereas in the second case we can afford to batch several
QHs together and unlink them all at once. Hence the division of
labor.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1581) replaces the iaa_watchdog kernel timer used by
ehci-hcd with an hrtimer event, in keeping with the general conversion
to high-res timers.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1576) adds hrtimer support for managing ehci-hcd's
async schedule. Just as with the earlier change to the periodic
schedule management, two new hrtimer events take care of everything.
One event polls at 1-ms intervals to see when the Asynchronous
Schedule Status (ASS) flag matches the Asynchronous Schedule Enable
(ASE) value; the schedule's state must not be changed until it does.
The other event delays for 15 ms after the async schedule becomes
empty before turning it off.
The new events replace a busy-wait poll and a kernel timer usage.
They also replace the rather illogical method currently used for
indicating the async schedule should be turned off: attempting to
unlink the dedicated QH at the head of the async list.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1571) adds a new state for ehci-hcd's root hubs:
EHCI_RH_STOPPING. This value is used at times when the root hub is
being stopped and we don't know whether or not the hardware has
finished all its DMA yet.
Although the purpose may not be apparent, this distinction will come
in useful later on. Future patches will avoid actions that depend on
the root hub being operational (like turning on the async or periodic
schedules) when they see the state is EHCI_RH_STOPPING.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1570) adds a pointer for the end of ehci-hcd's
async-unlink list. The list (which is actually a queue) is singly
linked, so having a pointer to its end makes adding new entries easier
-- there's no longer any need to scan through the whole list.
In principle it could be changed to a standard doubly-linked list. It
turns out that doing so actually makes the code less clear, so I'm
leaving it as is.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1569) renames the ehci->reclaim list in ehci-hcd. The
word "reclaim" is used in the EHCI specification to mean something
quite different, and "unlink_next" is more descriptive of the list's
purpose anyway.
Similarly, the "reclaim" field in the ehci_stats structure is renamed
"iaa", which is more meaningful (to experts, anyway) and is a better
match for the "lost_iaa" field.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1568) introduces symbolic constants for some of the
less-frequently used bitfields in the QH structure. This makes the
code a little easier to read and understand.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch (as1567) removes ehci-hcd's reference counting of QH
structures. It's not necessary to refcount these things because they
always get deallocated at exactly one spot in ehci_endpoint_disable()
(except for two special QHs, ehci->async and ehci->dummy) and are
never used again.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
dbg() was a very old USB-specific macro that should no longer
be used. This patch removes it from being used in the driver
and uses dev_dbg() instead.
CC: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The ehci-hcd driver is a little haphazard about keeping track of the
state of the USBCMD register. The ehci->command field is supposed to
hold the register's value (apart from a few special bits) at all
times, but it isn't maintained properly.
This patch (as1543) cleans up the situation. It keeps ehci->command
up-to-date, and uses that value rather than reading the register from
the hardware whenever possible.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add a new field num_mapped_sgs to struct urb so that we have a place to
store the number of mapped entries and can also retain the original
value of entries in num_sgs. Previously, usb_hcd_map_urb_for_dma()
would overwrite this with the number of mapped entries, which would
break dma_unmap_sg() because it requires the original number of entries.
This fixes warnings like the following when using USB storage devices:
------------[ cut here ]------------
WARNING: at lib/dma-debug.c:902 check_unmap+0x4e4/0x695()
ehci_hcd 0000:00:12.2: DMA-API: device driver frees DMA sg list with different entry count [map count=4] [unmap count=1]
Modules linked in: ohci_hcd ehci_hcd
Pid: 0, comm: kworker/0:1 Not tainted 3.2.0-rc2+ #319
Call Trace:
<IRQ> [<ffffffff81036d3b>] warn_slowpath_common+0x80/0x98
[<ffffffff81036de7>] warn_slowpath_fmt+0x41/0x43
[<ffffffff811fa5ae>] check_unmap+0x4e4/0x695
[<ffffffff8105e92c>] ? trace_hardirqs_off+0xd/0xf
[<ffffffff8147208b>] ? _raw_spin_unlock_irqrestore+0x33/0x50
[<ffffffff811fa84a>] debug_dma_unmap_sg+0xeb/0x117
[<ffffffff8137b02f>] usb_hcd_unmap_urb_for_dma+0x71/0x188
[<ffffffff8137b166>] unmap_urb_for_dma+0x20/0x22
[<ffffffff8137b1c5>] usb_hcd_giveback_urb+0x5d/0xc0
[<ffffffffa0000d02>] ehci_urb_done+0xf7/0x10c [ehci_hcd]
[<ffffffffa0001140>] qh_completions+0x429/0x4bd [ehci_hcd]
[<ffffffffa000340a>] ehci_work+0x95/0x9c0 [ehci_hcd]
...
---[ end trace f29ac88a5a48c580 ]---
Mapped at:
[<ffffffff811faac4>] debug_dma_map_sg+0x45/0x139
[<ffffffff8137bc0b>] usb_hcd_map_urb_for_dma+0x22e/0x478
[<ffffffff8137c494>] usb_hcd_submit_urb+0x63f/0x6fa
[<ffffffff8137d01c>] usb_submit_urb+0x2c7/0x2de
[<ffffffff8137dcd4>] usb_sg_wait+0x55/0x161
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Data Buffer Error as per spec section 4.15.1.1.2
results when there is Underrun or Overrun condition.
This error is considered non-fatal and never gets reported.
Its a very good indication on things going wrong at system level,
like running memory at much slower speed.
This is a good error to flag allowing system level corrections.
An issue was found with OMAP4460 board where DDR had to be run
at full speed and this logging helped.
Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
According to ehci spec 4.10.2, Advance Queue
If the fetched qTD has its Active bit set to a zero, the
host controller aborts the queue advance and follows the
queue head's horizontal pointer to the next schedule data
structure.
the 'qtd' will be linked into qh hardware queue after the line
below
*dummy = *qtd;
is executed and observed by EHCI HC, but EHCI HC won't have chance to
fetch the qtd descriptor pointed by 'qtd' in qh_append_tds until the
line below
dummy->hw_token = token; #set Active bit here
is executed by CPU and observed by EHCI HC.
There is already one 'wmb' to order writing to 'dummy'/'qtd' descriptors
and writing 'token' to 'dummy' descriptor(set Active bit), so the 1st
wmb is not needed and can be removed.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Obviously, ZLP is only required for transfer of OUT direction,
so just take same policy with UHCI for ZLP packet.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
qh_refresh is always called when the qh is idle and has not been
linked into hardware queue, so EHCI will not access overlay of
the qh at this time. Just before linking qh into hardware queue, there
has already one wmb to order writing qh descriptor and writing dma
address of the qh into hardware queue, so HC can always see
up-to-date qh descriptor once the qh is fetched with its dma address
by EHCI.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1483) improves the ehci-hcd driver family by getting rid
of the reliance on the hcd->state variable. It has no clear owner and
it isn't protected by the usual HCD locks. In its place, the patch
adds a new, private ehci->rh_state field to record the state of the
root hub.
Along the way, the patch removes a couple of lines containing
redundant assignments to the state variable. Also, the QUIESCING
state simply gets changed to the RUNNING state, because the driver
doesn't make any distinction between them.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1480) fixes a rather obscure bug in ehci-hcd. The
qh_update() routine needs to know the number and direction of the
endpoint corresponding to its QH argument. The number can be taken
directly from the QH data structure, but the direction isn't stored
there. The direction is taken instead from the first qTD linked to
the QH.
However, it turns out that for interrupt transfers, qh_update() gets
called before the qTDs are linked to the QH. As a result, qh_update()
computes a bogus direction value, which messes up the endpoint toggle
handling. Under the right combination of circumstances this causes
usb_reset_endpoint() not to work correctly, which causes packets to be
dropped and communications to fail.
Now, it's silly for the QH structure not to have direct access to all
the descriptor information for the corresponding endpoint. Ultimately
it may get a pointer to the usb_host_endpoint structure; for now,
adding a copy of the direction flag solves the immediate problem.
This allows the Spyder2 color-calibration system (a low-speed USB
device that sends all its interrupt data packets with the toggle set
to 0 and hance requires constant use of usb_reset_endpoint) to work
when connected through a high-speed hub. Thanks to Graeme Gill for
supplying the hardware that allowed me to track down this bug.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Graeme Gill <graeme@argyllcms.com>
CC: <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1477) fixes a problem affecting a few types of EHCI
controller. Contrary to what one might expect, these controllers
automatically stop their internal frame counter when no ports are
enabled. Since ehci-hcd currently relies on the frame counter for
determining when it should unlink QHs from the async schedule, those
controllers run into trouble: The frame counter stops and the QHs
never get unlinked.
Some systems have also experienced other problems traced back to
commit b963801164 (USB: ehci-hcd unlink
speedups), which made the original switch from using the system clock
to using the frame counter. It never became clear what the reason was
for these problems, but evidently it is related to use of the frame
counter.
To fix all these problems, this patch more or less reverts that commit
and goes back to using the system clock. But this can't be done
cleanly because other changes have since been made to the scan_async()
subroutine. One of these changes involved the tricky logic that tries
to avoid rescanning QHs that have already been seen when the scanning
loop is restarted, which happens whenever an URB is given back.
Switching back to clock-based unlinks would make this logic even more
complicated.
Therefore the new code doesn't rescan the entire async list whenever a
giveback occurs. Instead it rescans only the current QH and continues
on from there. This requires the use of a separate pointer to keep
track of the next QH to scan, since the current QH may be unlinked
while the scanning is in progress. That new pointer must be global,
so that it can be adjusted forward whenever the _next_ QH gets
unlinked. (uhci-hcd uses this same trick.)
Simplification of the scanning loop removes a level of indentation,
which accounts for the size of the patch. The amount of code changed
is relatively small, and it isn't exactly a reversion of the
b963801164 commit.
This fixes Bugzilla #32432.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: <stable@kernel.org>
Tested-by: Matej Kenda <matejken@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1466) speeds up processing of ehci-hcd's periodic list.
The existing code will pointlessly rescan an interrupt endpoint queue
each time it encounters the queue's QH in the periodic list, which can
happen quite a few times if the endpoint's period is low. On some
embedded systems, this useless overhead can waste so much time that
the driver falls hopelessly behind and loses events.
The patch introduces a "periodic_stamp" variable, which gets
incremented each time scan_periodic() runs and each time the scan
advances to a new frame. If the corresponding stamp in an interrupt
QH is equal to the current periodic_stamp, we assume the QH has
already been scanned and skip over it. Otherwise we scan the QH as
usual, and if none of its URBs have completed then we store the
current periodic_stamp in the QH's stamp, preventing it from being
scanned again.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
A Synopsys USB core used in various SoCs has a bug which might cause
that the host controller not issuing ping.
When software uses the Doorbell mechanism to remove queue heads, the
host controller still has references to the removed queue head even
after indicating an Interrupt on Async Advance. This happens if the last
executed queue head's Next Link queue head is removed.
Consequences of the defect:
The Host controller fetches the removed queue head, using memory that
would otherwise be deallocated.This results in incorrect transactions on
both the USB and system memory. This may result in undefined behavior.
Workarounds:
1) If no queue head is active (no Status field's Active bit is set)
after removing the queue heads, the software can write one of the valid
queue head addresses to the ASYNCLISTADDR register and deallocate the
removed queue head's memory after 2 microframes.
If one or more of the queue heads is active (the Active bit is set in
the Status field) after removing the queue heads, the software can delay
memory deallocation after time X, where X is the time required for the
Host Controller to go through all the queue heads once. X varies with
the number of queue heads and the time required to process periodic
transactions: if more periodic transactions must be performed, the Host
Controller has less time to process asynchronous transaction processing.
2) Do not use the Doorbell mechanism to remove the queue heads. Disable
the Asynchronous Schedule Enable bit instead.
The bug has been discussed on the linux-usb-devel mailing-list
four years ago, the original thread can be found here:
http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg45345.html
This patch implements the first workaround as suggested by David Brownell.
The built-in USB host controller of the Atheros AR7130/AR7141/AR7161 SoCs
requires this to work properly.
Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1458) fixes a problem affecting ultra-reliable systems:
When hardware failover of an EHCI controller occurs, the data
structures do not get released correctly. This is because the routine
responsible for removing unused QHs from the async schedule assumes
the controller is running properly (the frame counter is used in
determining how long the QH has been idle) -- but when a failover
causes the controller to be electronically disconnected from the PCI
bus, obviously it stops running.
The solution is simple: Allow scan_async() to remove a QH from the
async schedule if it has been idle for long enough _or_ if the
controller is stopped.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-Tested-by: Dan Duval <dan.duval@stratus.com>
CC: <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1453) fixes a long-standing bug in the ehci-hcd driver.
There is no need to set the Halt bit in the overlay region for an
unlinked or blocked QH. Contrary to what the comment says, setting
the Halt bit does not cause the QH to be patched later; that decision
(made in qh_refresh()) depends only on whether the QH is currently
pointing to a valid qTD. Likewise, setting the Halt bit does not
prevent completions from activating the QH while it is "stopped"; they
are prevented by the fact that qh_completions() temporarily changes
qh->qh_state to QH_STATE_COMPLETING.
On the other hand, there are circumstances in which the QH will be
reactivated _without_ being patched; this happens after an URB beyond
the head of the queue is unlinked. Setting the Halt bit will then
cause the hardware to see the QH with both the Active and Halt bits
set, an invalid combination that will prevent the queue from
advancing and may even crash some controllers.
Apparently the only reason this hasn't been reported before is that
unlinking URBs from the middle of a running queue is quite uncommon.
However Test 17, recently added to the usbtest driver, does exactly
this, and it confirms the presence of the bug.
In short, there is no reason to set the Halt bit for an unlinked or
blocked QH, and there is a very good reason not to set it. Therefore
the code that sets it is removed.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Tested-by: Andiry Xu <andiry.xu@amd.com>
CC: David Brownell <david-b@pacbell.net>
CC: <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
With pre-release GCC-4.6, we get a 'set but not used' warning when
EHCI_URB_TRACE is not set because we set the qtd variable without
using it.
Rearrange the statements so that we only set qtd if it will be used.
Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1393) converts several of the single-bit fields in
struct usb_hcd to atomic flags. This is for safety's sake; not all
CPUs can update bitfield values atomically, and these flags are used
in multiple contexts.
The flag fields that are set only during registration or removal can
remain as they are, since non-atomic accesses at those times will not
cause any problems.
(Strictly speaking, the authorized_default flag should become atomic
as well. I didn't bother with it because it gets changed only via
sysfs. It can be done later, if anyone wants.)
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Change the type of the URB's 'sg' pointer from a usb_sg_request to
a scatterlist. This allows drivers to submit scatter-gather lists
without using the usb_sg_wait() interface. It has the added benefit
of removing the typecasts that were added as part of patch as1368 (and
slightly decreasing the number of pointer dereferences).
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
Tested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1320) fixes two problems related to interrupt-URB
scheduling in ehci-hcd.
URBs with an interval of 2 or 4 microframes aren't handled.
For the time being, the patch reduces to interval to 1 uframe.
URBs are constrained to have an interval no larger than 1024
frames by usb_submit_urb(). But some EHCI controllers allow
use of a schedule as short as 256 frames; for these
controllers we may have to decrease the interval to the
actual schedule length.
The second problem isn't very significant since few devices expose
interrupt endpoints with an interval larger than 256 frames. But the
first problem is critical; it will prevent the kernel from working
with devices having interrupt intervals of 2 or 4 uframes.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Tested-by: Glynn Farrow <farrowg@sg.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1300) adds native scatter-gather support to ehci-hcd.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1304) fixes a regression in ehci-hcd. Evidently some
hubs don't handle Clear-TT-Buffer requests correctly, so we should
avoid sending them when they don't appear to be absolutely necessary.
The reported symptom is that output on a downstream audio device cuts
out because the hub stops relaying isochronous packets.
The patch prevents Clear-TT-Buffer requests from being sent following
a STALL handshake. In theory a STALL indicates either that the
downstream device sent a STALL or that no matching TT buffer could be
found. In either case, the transfer is completed and the TT buffer
does not remain busy, so it doesn't need to be cleared.
Also, the patch fixes a minor flaw in the code that actually sends the
Clear-TT-Buffer requests. Although the pipe direction isn't really
used for control transfers, it should be a Send rather than a Receive.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Javier Kohen <jkohen@users.sourceforge.net>
CC: David Brownell <david-b@pacbell.net>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1281) changes the way ehci-hcd deschedules interrupt
QHs, copying the approach used for async QHs. The caller is no longer
responsible for rescheduling the QH if its queue is non-empty; instead
the reschedule is done directly by intr_deschedule(), after calling
qh_completions(). This is exactly the same as how end_unlink_async()
works.
ehci_urb_dequeue() and intr_deschedule() now correctly handle the case
where they are called while another interrupt URB for the same QH is
being given back. This was a surprisingly large blind spot. And
scan_periodic() now respects the new needs_rescan flag.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: David Brownell <david-b@pacbell.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1280) fixes an obscure bug in ehci-hcd's dequeuing logic
for async URBs. If a later URB is unlinked and the completion
routine unlinks an earlier URB, then the earlier URB won't be given
back in a timely manner because the endpoint queue isn't rescanned as
it should be.
Similar bugs occur if an endpoint is reset or a halt is cleared while
a completion routine is running, because the subroutines don't test
for the COMPLETING state.
All these problems are solved by adding a new needs_rescan flag to the
ehci_qh structure. If the flag is set while scanning through an idle
QH, the scan will be repeated. If the QH isn't idle then an unlink
cycle will be initiated, and the proper action will be taken when it
becomes idle.
Also, an unnecessary test is removed from qh_link_async(): That
routine is never called if the QH's state isn't IDLE.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: David Brownell <david-b@pacbell.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
The ehci_qh structure merged hw and sw together which is not good:
1. More and more items are being added into ehci_qh, the ehci_qh software
part are unnecessary to be allocated in DMA qh_pool.
2. If HCD has local SRAM, the sw part will consume it too, and it won't
bring any benefit.
3. For non-cache-coherence system, the entire ehci_qh is uncachable, actually
we only need the hw part to be uncacheable. Spliting them will let the sw
part to be cacheable.
Signed-off-by: Alek Du <alek.du@intel.com>
Cc: David Brownell <dbrownell@users.sourceforge.net>
CC: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1274) simplifies the counting of transaction-error
retries. Now we will count up from 0 to QH_XACTERR_MAX instead of
down from QH_XACTERR_MAX to 0.
The patch also fixes a small bug: qh->xacterr was not getting
initialized for interrupt endpoints.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Tested-by: Matthijs Kooijman <matthijs@stdin.nl>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1273) fixes two(!) bugs introduced by the new
Clear-TT-Buffer implementation in ehci-hcd.
It is now possible for an idle QH to have some URBs on its
queue -- this will happen if a Clear-TT-Buffer is pending for
the QH's endpoint. Consequently we should not issue a warning
when someone tries to unlink an URB from an idle QH; instead
we should process the request immediately.
The refcounts for QHs could get messed up, because
submit_async() would increment the refcount when calling
qh_link_async() and qh_link_async() would then refuse to link
the QH into the schedule if a Clear-TT-Buffer was pending.
Instead we should increment the refcount only when the QH
actually is added to the schedule. The current code tries to
be clever by leaving the refcount alone if an unlink is
immediately followed by a relink; the patch changes this to an
unconditional decrement and increment (although they occur in
the opposite order).
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: David Brownell <david-b@pacbell.net>
Tested-by: Manuel Lauss <manuel.lauss@gmail.com>
Tested-by: Matthijs Kooijman <matthijs@stdin.nl>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1257) revises the way ehci-hcd detects STALLs. The
logic is a little peculiar because there's no hardware status bit
specifically meant to indicate a STALL. You just have to guess that a
STALL was received if the BABBLE bit (which is fatal) isn't set and
the transfer stopped before all its retries were used up.
The existing code doesn't do this properly, because it tests for MMF
(Missed MicroFrame) and DBE (Data Buffer Error) before testing the
retry counter. Thus, if a transaction gets either MMF or DBE the
corresponding flag is set and the transaction is retried. If the
second attempt receives a STALL then -EPIPE is the correct return
value. But the existing code would see the MMF or DBE flag instead
and return -EPROTO, -ENOSR, or -ECOMM.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Acked-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1256) changes ehci-hcd and all the other drivers in the
EHCI family to make use of the new clear_tt_buffer callbacks. When a
Clear-TT-Buffer request is in progress for a QH, the QH is not allowed
to be linked into the async schedule until the request is finished.
At that time, if there are any URBs queued for the QH, it is linked
into the async schedule.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1255) updates the interface for calling
usb_hub_clear_tt_buffer(). Even the name of the function is changed!
When an async URB (i.e., Control or Bulk) going through a high-speed
hub to a non-high-speed device is cancelled or fails, the hub's
Transaction Translator buffer may be left busy still trying to
complete the transaction. The buffer has to be cleared; that's what
usb_hub_clear_tt_buffer() does.
It isn't safe to send any more URBs to the same endpoint until the TT
buffer is fully clear. Therefore the HCD needs to be told when the
Clear-TT-Buffer request has finished. This patch adds a callback
method to struct hc_driver for that purpose, and makes the hub driver
invoke the callback at the proper time.
The patch also changes a couple of names; "hub_tt_kevent" and
"tt.kevent" now look rather antiquated.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This is an update to the "usb-ehci-update-toggle-state-for-linked-qhs"
patch. Since an HCD's endpoint_reset method can be called in
interrupt context, it mustn't assume that interrupts are enabled or
that it can sleep.
So we revert to the original way of refreshing QHs' toggle bits. Now
the endpoint_reset method merely clears the toggle flag in the device
structure (as was done before) and starts an async QH unlink. When the
QH is linked again, after the unlink finishes and an URB is queued,
the qh_refresh() routine will update the QH's toggle bit.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Tested-by: David <david@unsolicited.net>
CC: David Brownell <david-b@pacbell.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1245) fixes a bug in ehci-hcd. When an URB is queued
for an endpoint whose QH is already in the LINKED state, the QH
doesn't get refreshed. As a result, if usb_clear_halt() was called
during the time that the QH was linked but idle, the data toggle value
in the QH doesn't get reset.
The symptom is that after a clear_halt, data gets lost and transfers
time out. This problem is starting to show up now because the
"ehci-hcd unlink speedups" patch causes QHs with no queued URBs to
remain linked for a suitable time.
The patch utilizes the new endpoint_reset mechanism to fix the
problem. When an endpoint is reset, the new method forcibly unlinks
the QH (if necessary) and safely updates the toggle value. This
allows qh_update() to be simplified and avoids using usb_device's
toggle bits in a rather unintuitive way.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: David Brownell <david-b@pacbell.net>
Tested-by: David <david@unsolicited.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Fix ehci printk formats:
drivers/usb/host/ehci-q.c:351: warning: format '%d' expects type 'int', but argument 4 has type 'size_t'
drivers/usb/host/ehci-q.c:351: warning: format '%d' expects type 'int', but argument 5 has type 'size_t'
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1204) adds a software retry mechanism to ehci-hcd. It
gets invoked when the driver encounters transaction errors on an
asynchronous endpoint. On many systems, hardware deficiencies cause
such errors to occur if one device is unplugged while the host is
communicating with another device. With the patch, the failed
transactions are retried and generally succeed the second or third
time through.
This is based on code originally written by Koichiro Saito.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Tested by: Koichiro Saito <Saito.Koichiro@adniss.jp>
CC: David Brownell <david-b@pacbell.net>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>