- 04 Mar, 2016 40 commits
-
-
Douglas Anderson authored
This no-op change splits code out of dwc2_schedule_periodic() into a dwc2_do_reserve() function. This makes it a little easier to follow the logic. Acked-by:
John Youn <johnyoun@synopsys.com> Signed-off-by:
Douglas Anderson <dianders@chromium.org> Tested-by:
Heiko Stuebner <heiko@sntech.de> Tested-by:
Stefan Wahren <stefan.wahren@i2se.com> Signed-off-by:
Felipe Balbi <balbi@kernel.org>
-
Douglas Anderson authored
This no-op change just reorders a few functions in hcd_queue.c in order to prepare for future changes. Motivations here: The functions dwc2_hcd_qh_free() and dwc2_hcd_qh_create() are exported functions. They are not called within the file. That means that they should be near the bottom so that they can easily call static helpers. The function dwc2_qh_init() is only called by dwc2_hcd_qh_create() and should move near the bottom with it. The only reason that the dwc2_unreserve_timer_fn() timer function (and its subroutine dwc2_do_unreserve()) were so high in the file was that they needed to be above dwc2_qh_init(). Now that dwc2_qh_init() has been moved down it can be moved down a bit. A later patch will split the reserve code out of dwc2_schedule_periodic() and the reserve function should be near the unreserve function. The reserve function needs to be below dwc2_find_uframe() since it calls that. Acked-by:
-
Douglas Anderson authored
This no-op change just does some renames to simplify a future patch. 1. The "interval" field is renamed to "host_interval" to make it more obvious that this interval may be 8 times the interval that the device sees (if we're doing split transactions). A future patch will also add the "device_interval" field. 2. The "usecs" field is renamed to "host_us" again to make it more obvious that this is the time for the transaction as seen by the host. For split transactions the device may see a much longer transaction time. A future patch will also add "device_us". 3. The "sched_frame" field is renamed to "next_active_frame". The name "sched_frame" kept confusing me because it felt like something more permament (the QH's reservation or something). The name "next_active_frame" makes it more obvious that this field is constantly changing. Acked-by:
-
Douglas Anderson authored
The old code in dwc2_process_periodic_channels() would only enable the "periodic empty" interrupt if we weren't using DMA. That wasn't right since we can still get into cases where we have small FIFOs even on systems that have DMA (the rk3288 is a prime example). Let's always enable/disable the "periodic empty" when appropriate. As part of this: * Always call dwc2_process_periodic_channels() even if there's nothing in periodic_sched_assigned (we move the queue empty check so we still avoid the extra work). That will make extra certain that we will properly disable the "periodic empty" interrupt even if there's nothing queued up. * Move the enable of "periodic empty" due to non-empty periodic_sched_assigned to be for slave mode (non-DMA mode) only. Presumably this was the original intention of the check for DMA since it seems to match the comments above where in slave mode we leave things on the assigned queue. Note that even before this change slave mode didn't work for me, so I can't say for sure that my understanding of slave mode is correct. However, this shouldn't change anything for slave mode so if slave mode worked for someone in the past it ought to still work. With this change, I no longer get constant misses reported by my other debugging code (and with future patches) when I've got: * Rockchip rk3288 Chromebook, using port ff540000 -> Pluggable 7-port Hub with Charging (powered) -> Microsoft Wireless Keyboard 2000 in port 1. -> Das Keyboard in port 2. -> Jabra Speaker in port 3 -> Logitech, Inc. Webcam C600 in port 4 -> Microsoft Sidewinder X6 Keyboard in port 5 ...and I'm playing music on the USB speaker and capturing video from the webcam. Acked-by: -
Douglas Anderson authored
I find that when I plug a full speed (NOT high speed) hub into a dwc2 port and then I plug a bunch of devices into that full speed hub that dwc2 goes bat guano crazy. Specifically, it just spews errors like this in the console: usb usb1: clear tt 1 (9043) error -22 The specific test case I used looks like this: /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M |__ Port 1: Dev 17, If 0, Class=Hub, Driver=hub/4p, 12M |__ Port 2: Dev 19, If 0, ..., Driver=usbhid, 1.5M |__ Port 4: Dev 20, If 0, ..., Driver=usbhid, 12M |__ Port 4: Dev 20, If 1, ..., Driver=usbhid, 12M |__ Port 4: Dev 20, If 2, ..., Driver=usbhid, 12M Showing VID/PID: Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub Bus 001 Device 017: ID 03eb:3301 Atmel Corp. at43301 4-Port Hub Bus 001 Device 020: ID 045e:0745 Microsoft Corp. Nano Transceiver ... Bus 001 Device 019: ID 046d:c404 Logitech, Inc. TrackMan Wheel I spent a bunch of time trying to figure out why there are errors to begin with. I believe that the issue may be a hardware issue where the transceiver sometimes accidentally sends a PREAMBLE packet if you send a packet to a full speed device right after one to a low speed device. Luckily the USB driver retries and the second time things work OK. In any case, things kinda seem work despite the errors, except for the "clear tt" spew mucking up my console. Chalk it up for a win for retries and robust protocols. So getting back to the "clear tt" problem, it appears that we get those because there's not actually a TT here to clear. It's my understanding that when dwc2 operates in low speed or full speed mode that there's no real TT out there. That makes all these attempts to "clear the TT" somewhat meaningless and also causes the spew in the log. Let's just skip all the useless TT clears. Eventually we should root cause the errors, but even if we do this is still a proper fix and is likely to avoid the "clear tt" error in the future. Note that hooking up a Full Speed USB Audio Device (Jabra 510) to this same hub with the keyboard / trackball shows that even audio works over this janky connection. As a point to note, this particular change (skip bogus TT clears) compared to just commenting out the dev_err() in hub_tt_work() actually produces better audio. Note: don't ask me where I got a full speed USB hub or whether the massive amount of dust that accumulated on it while it was in my junk box affected its funtionality. Just smile and nod. Acked-by:Kever Yang <kever.yang@rock-chips.com> Signed-off-by:
-
Douglas Anderson authored
According to the most up to date version of the dwc2 databook, the FRINT field of the HFIR register should be programmed to: * 125 us * (PHY clock freq for HS) - 1 * 1000 us * (PHY clock freq for FS/LS) - 1 This is opposed to older versions of the doc that claimed it should be: * 125 us * (PHY clock freq for HS) * 1000 us * (PHY clock freq for FS/LS) In case you didn't spot it, the difference is the "- 1". Let's add the "- 1" to match the newest user manual. It's presumed that the "- 1" should have always been there and that this was always a documentation error. If some hardware needs the "- 1" and other hardware doesn't, we'll have to add a configuration parameter for it in the future. I checked things before and after this patch on rk3288 using a Total Phase Beagle 5000 analyzer. Before this patch, a low speed mouse shows constant Frame Timing Jitter errors. After this patch errors have gone away. Before this patch SOF packets move forward about 1 us per 4 ms. After this patch the SOF packets move backward about 1 us per 255 ms. Some specific SOF timestamps from the analyzer are below. Before: 6.603.790 6.603.916 6.604.041 6.604.166 ... 6.607.541 6.607.667 6.607.792 6.607.917 ... 6.611.417 6.611.543 6.611.668 6.611.793 After: 6.215.159 6.215.284 6.215.408 6.215.533 6.215.658 ... 6.470.658 6.470.783 6.470.907 ... 6.726.032 6.726.157 6.725.281 6.725.406 Acked-by:
-
Douglas Anderson authored
In commit 94dfd7ed ("USB: HCD: support giveback of URB in tasklet context") support was added to give back the URB in tasklet context. Let's take advantage of this in dwc2. This speeds up the dwc2 interrupt handler considerably. Note that this requires the change ("usb: dwc2: host: Add a delay before releasing periodic bandwidth") to come first. Note that, as per Alan Stern in <https://patchwork.kernel.org/patch/7555771/>, we also need to make sure that the extra delay before the device drivers submit more data doesn't break the scheduler. At the moment the scheduler is pretty broken (see future patches) so it's hard to be 100% certain, but I have yet to see any new breakage introduced by this delay. ...and speeding up interrupt processing for dwc2 is a huge deal because it means we've got a better chance of not missing SOF interrupts. That means we've got an overall win here. Note that when playing USB audio and using a USB webcam and having several USB keyboards plugged in, the crackling on the USB audio device is noticably reduced with this patch. Acked-by:
-
Douglas Anderson authored
We'd like to be able to use HCD_BH in order to speed up the dwc2 host interrupt handler quite a bit. However, according to the kernel doc for usb_submit_urb() (specifically the part about "Reserved Bandwidth Transfers"), we need to keep a reservation active as long as a device driver keeps submitting. That was easy to do when we gave back the URB in the interrupt context: we just looked at when our queue was empty and released the reserved bandwidth then. ...but now we need a little more complexity. We'll follow EHCI's lead in commit 9118f9eb ("USB: EHCI: improve interrupt qh unlink") and add a 5ms delay. Since we don't have a whole timer infrastructure in dwc2, we'll just add a timer per QH. The overhead for this is very small. Note that the dwc2 scheduler is pretty broken (see future patches to fix it). This patch attempts to replicate all old behavior and just add the proper delay. Acked-by:
-
Douglas Anderson authored
In preparation for future changes to the scheduler let's add some tracing that makes it easy for us to see what's happening. By default this tracing will be off. By changing "core.h" you can easily trace to ftrace, the console, or nowhere. Acked-by:
-
Douglas Anderson authored
We're supposed to keep outstanding splits in order. Keep track of a list of the order of splits and process channel interrupts in that order. Without this change and the following setup: * Rockchip rk3288 Chromebook, using port ff540000 -> Pluggable 7-port Hub with Charging (powered) -> Microsoft Wireless Keyboard 2000 in port 1. -> Das Keyboard in port 2. ...I find that I get dropped keys on the Microsoft keyboard (I'm sure there are other combinations that fail, but this documents my test). Specifically I've been typing "hahahahahahaha" on the keyboard and often see keys dropped or repeated. After this change the above setup works properly. This patch is based on a previous patch proposed by Yunzhi Li ("usb: dwc2: hcd: fix periodic transfer schedule sequence") Acked-by:Yunzhi Li <lyz@rock-chips.com> Reviewed-by:
-
Douglas Anderson authored
The queues the the dwc2 host controller used are truly queues. That means FIFO or first in first out. Unfortunately though the code was iterating through these queues starting from the head, some places in the code was adding things to the queue by adding at the head instead of the tail. That means last in first out. Doh. Go through and just always add to the tail. Doing this makes things much happier when I've got: * 7-port USB 2.0 Single-TT hub * - Microsoft 2.4 GHz Transceiver v7.0 dongle * - Jabra speakerphone playing music Acked-by:
-
Douglas Anderson authored
When poking around with USB devices with slub_debug enabled, I found another obvious use after free. Turns out that in dwc2_hc_n_intr() I was in a state when the contents of chan->qh was filled with 0x6b, indicating that chan->qh was freed but chan still had a reference to it. Let's make sure that whenever we free qh we also make sure we remove a reference from its channel. The bug fixed here doesn't appear to be new--I believe I just got lucky and happened to see it while stress testing. Acked-by:
-
Douglas Anderson authored
As documented in dwc2_calculate_dynamic_fifo(), host_rx_fifo_size should really be: 2 * ((Largest Packet size / 4) + 1 + 1) + n with n = number of host channel. We have 9 host channels, so 2 * ((1024/4) + 2) + 9 = 516 + 9 = 525 We've got 960 / 972 total_fifo_size on rk3288 (and presumably on rk3066) and 525 + 128 + 256 = 909 so we're still under on both ports even when we increment by 5. In the future, it would be nice if dwc2_calculate_dynamic_fifo() could handle the "too small" FIFO case and come up with something more dynamically. When we do that we can figure out how to allocate the extra 48 / 60 bytes of FIFO that we're currently wasting. NOTE: no known bugs are fixed by this patch, but it seems like a simple fix and ought to fix someone. Acked-by:
-
Douglas Anderson authored
All other host controllers who want aligned buffers for DMA do it a certain way. Let's do that too instead of working behind the USB core's back. This makes our interrupt handler not take forever and also rips out a lot of code, simplifying things a bunch. This also has the side effect of removing the 65535 max transfer size limit. NOTE: The actual code to allocate the aligned buffers is ripped almost completely from the tegra EHCI driver. At some point in the future we may want to add this functionality to the USB core to share more code everywhere. Signed-off-by:
-
Douglas Anderson authored
Previously we needed to set the max_transfer_size to explicitly be 65535 because the old driver would detect that our hardware could support much bigger transfers and then would try to do them. This wouldn't work since the DMA alignment code couldn't support it. Later in commit e8f8c14d ("usb: dwc2: clip max_transfer_size to 65535") upstream added support for clipping this automatically. Since that commit it has been OK to just use "-1" (default), but nobody bothered to change it. Let's change it to default now for two reasons: - It's nice to use autodetected params. - If we can remove the 65535 limit, we can transfer more! Signed-off-by:
-
John Youn authored
Check that dwc->maximum_speed is set to a valid value. Also add an error when we use it later if we encounter an invalid value. Signed-off-by:
-
Emilio López authored
MODULE_DEVICE_TABLE() is missing, so the module isn't auto-loading on sunxi systems using the OTG controller. This commit adds the missing line so it loads automatically when building it as a module and running on a system with an USB OTG port. Signed-off-by:
Emilio López <emilio.lopez@collabora.co.uk> Signed-off-by:
-
Li Jun authored
B-device detects that bus is idle for more than TB_AIDL_BDIS min and begins HNP by turning off pullup on DP, this allows the bus to discharge to the SE0 state. This timer was missed and failed with PET test: 6.8.5 B-UUT HNP of USB OTG and EH automated compliance plan v1.2, this patch is to fix this timing issue. Acked-by:
Peter Chen <peter.chen@nxp.com> Signed-off-by:
Li Jun <jun.li@nxp.com> Signed-off-by:
-
Li Jun authored
Add A-idle to B-disconnect timer, B-device detects that bus is idle for more than TB_AIDL_BDIS min and begins HNP by turning off pullup on D+. This allows the bus to discharge to the SE0 state. Acked-by:
-
Li Jun authored
Update HNP test procedure as HNP polling is supported. Acked-by:
-
Li Jun authored
Enable HNP polling support for chipidea gadget and allocate memory for host request flag when otg fsm init. Acked-by:
-
Li Jun authored
Set host_request_flag if the current peripheral wants to take host role via changing a_bus_req or b_bus_req by user application. Acked-by:
Peter Chen <peter.chen@freescale.com> Signed-off-by:
-
Li Jun authored
If gadget with HNP polling support receives GetStatus request of otg status selector, it feedback to host with host request flag to indicate if it wants to take host role. Acked-by:
-
Li Jun authored
Since gadget driver will handle this request, so controller driver bypass it. Acked-by:
-
Li Jun authored
Adds HNP polling timer when transits to host state, the OTG status request will be sent to peripheral after timeout, if host request flag is set, it will switch to peripheral state, otherwise it will repeat HNP polling every 1.5s and maintain the current session. Acked-by:
-
Li Jun authored
A host is required to use the GetStatus command, with wIndex set to the OTG status selector(F000H) to request the Host request flag from the peripheral. Acked-by:
-
Li Jun authored
Add 2 flags for USB OTG HNP polling, hnp_polling_support is to indicate if the gadget can support HNP polling, host_request_flag is used for gadget to store host request information from application, which can be used to respond to HNP polling from host. Acked-by:
-
Marek Szyprowski authored
Since commit 855ed04a ("usb: gadget: udc-core: independent registration of gadgets and gadget drivers") gadget drivers can not assume that UDC drivers are already available on their initialization. This broke the HACK, which was used in gadgetfs driver, to get UDC controller name. This patch removes this hack and replaces it by additional function in the UDC core (which is usefully only for legacy drivers, please don't use it in the new code). Reported-by:
Vegard Nossum <vegard.nossum@oracle.com> Signed-off-by:
Marek Szyprowski <m.szyprowski@samsung.com> Tested-by:
-
Michal Nazarewicz authored
For every in_substream, there must be a corresponding gmidi_in_port structure so it is perfectly viable and some might argue sensible to stash pointer to the input substream in the gmidi_in_port structure. This has an added benefit that if in_ports < MAX_PORTS, the whole f_midi structure takes up less space because only in_ports number of pointers for in_substream are allocated instead of MAX_PORTS lots of them. Signed-off-by:
Michal Nazarewicz <mina86@mina86.com> Signed-off-by:
-
Dan Carpenter authored
We added a new error path to this function and we forgot to drop the lock. Fixes: e1e3d7ec ('usb: gadget: f_midi: pre-allocate IN requests') Signed-off-by:
Dan Carpenter <dan.carpenter@oracle.com> [mina86@mina86.com: rebased on top of refactoring commit] Signed-off-by:
-
Michal Nazarewicz authored
Reduce number of allocations, simplify memory management and reduce memory usage by stacking the gmidi_in_port elements at the end of the f_midi structure using a flexible array. Signed-off-by:
-
Michal Nazarewicz authored
In general case, all of midi->in_port pointers may be non-NULL which implies that the ‘if (\!port)’ condition will never execute thus never zeroing midi->in_last_port. Fix by rewriting the loop such that the field is set to zero if \!port or end of loop has been reached. Signed-off-by:
-
Michal Nazarewicz authored
Move some of the f_midi_transmit to a separate f_midi_do_transmit function so the massive indention levels are not so jarring. This introduces no changes in behaviour. Signed-off-by:
-
Felipe F. Tonello authored
remove a field which is unnecessary. No functional changes. Signed-off-by:
Felipe F. Tonello <eu@felipetonello.com> Signed-off-by:
-
Du, Changbin authored
ffs_epfile_io and ffs_epfile_io_complete runs in different context, but there is no synchronization between them. consider the following scenario: 1) ffs_epfile_io interrupted by sigal while wait_for_completion_interruptible 2) then ffs_epfile_io set ret to -EINTR 3) just before or during usb_ep_dequeue, the request completed 4) ffs_epfile_io return with -EINTR In this case, ffs_epfile_io tell caller no transfer success but actually it may has been done. This break the caller's pipe. Below script can help test it (adbd is the process which lies on f_fs). while true do pkill -19 adbd #SIGSTOP pkill -18 adbd #SIGCONT sleep 0.1 done To avoid this, just dequeue the request first. After usb_ep_dequeue, the request must be done or canceled. With this change, we can ensure no race condition in f_fs driver. But actually I found some of the udc driver has analogical issue in its dequeue implementation. For example, 1) the dequeue function hold the controller's lock. 2) before driver request controller to stop transfer, a request completed. 3) the controller trigger a interrupt, but its irq handler need wait dequeue function to release the lock. 4) dequeue function give back the request with negative status, and release lock. 5) irq handler get lock but the request has already been given back. So, the dequeue implementation should take care of this case. IMO, it can be done as below steps to dequeue a already started request, 1) request controller to stop transfer on the given ep. HW know the actual transfer status. 2) after hw stop transfer, driver scan if there are any completed one. 3) if found, process it with real status. if no, the request can canceled. Signed-off-by:
"Du, Changbin" <changbin.du@intel.com> [mina86@mina86.com: rebased on top of refactoring commits] Signed-off-by:
-
Michal Nazarewicz authored
Eliminate one of the return paths by using a ‘goto error_mutex’ and rearrange some if-bodies which results in reduction of the indention level and thus hopefully makes the function easier to read and reason about. Signed-off-by:
-
Michal Nazarewicz authored
In ffs_epfile_io error label points to a return path which includes a kfree(data) call. However, at the beginning of the function data is always NULL so some of the early ‘goto error’ can safely be replaced with a trivial return statement. Signed-off-by:
-
Michal Nazarewicz authored
In the AIO path, if allocating of a request failse, the function simply goes to the error_lock path whose end result is returning value of ret. However, at this point ret’s value is zero (assigned as return value from ffs_mutex_lock). Fix by adding ‘ret = -ENOMEM’ statement. Signed-off-by:
-
Michal Nazarewicz authored
In the ffs_epfile_io function, data buffer is allocated for non-halt requests. Later, after grabing a mutex, the function checks that epfile->ep is still ep and if it’s not, it set ret to -ESHUTDOWN and follow a path including spin_unlock_irq (just after ‘ret = -ESHUTDOWN’), mutex_unlock (after if-else-if-else chain) and returns ret. Noticeably, this does not include freeing of the data buffer. Fix by introducing a goto which moves control flow to the the end of the function where spin_unlock_irq, mutex_unlock and kfree are all called. Signed-off-by:
-
Bjorn Helgaas authored
phy-am335x.c doesn't use any interfaces from linux/regulator/consumer.h, so stop including it. Signed-off-by:
Bjorn Helgaas <bhelgaas@google.com> CC: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by:
-
