This totally reimplements the microframe scheduler in dwc2 to attempt to
handle periodic splits properly. The old code didn't even try, so this
was a significant effort since periodic splits are one of the most
complicated things in USB.
I've attempted to keep the old "don't use the microframe" schduler
around for now, but not sure it's needed. It has also only been lightly
tested.
I think it's pretty certain that this scheduler isn't perfect and might
have some bugs, but it seems much better than what was there before.
With this change my stressful USB test (USB webcam + USB audio + some
keyboards) crackles less.
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>
When setting up ISO and INT transfers dwc2 needs to specify whether the
transfer is for an even or an odd frame (or microframe if the controller
is running in high speed mode).
The controller appears to use this as a simple way to figure out if a
transfer should happen right away (in the current microframe) or should
happen at the start of the next microframe. Said another way:
- If you set "odd" and the current frame number is odd it appears that
the controller will try to transfer right away. Same thing if you set
"even" and the current frame number is even.
- If the oddness you set and the oddness of the frame number are
_different_, the transfer will be delayed until the frame number
changes.
As I understand it, the above technique allows you to plan ahead of time
where possible by always working on the next frame. ...but it still
allows you to properly respond immediately to things that happened in
the previous frame.
The old dwc2_hc_set_even_odd_frame() didn't really handle this concept.
It always looked at the frame number and setup the transfer to happen in
the next frame. In some cases that meant that certain transactions
would be transferred in the wrong frame.
We'll try our best to set the even / odd to do the transfer in the
scheduled frame. If that fails then we'll do an ugly "schedule ASAP".
We'll also modify the scheduler code to handle this and not try to
schedule a second transfer for the same frame.
Note that this change relies on the work to redo the microframe
scheduler. It can work atop ("usb: dwc2: host: Manage frame nums better
in scheduler") but it works even better after ("usb: dwc2: host: Totally
redo the microframe scheduler").
With this change my stressful USB test (USB webcam + USB audio +
keyboards) has less audio crackling than before.
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>
As we start getting more exact about our scheduling it's becoming more
and more important to know exactly how far through the current frame we
are. This lets us make decisions about whether there's still time left
to start a new transaction in the current frame.
We'll add dwc2_hcd_get_future_frame_number() which will tell you what
the frame number will be a certain number of microseconds (us) from
now. We can use this information to help decide if there's enough time
left in the frame for a transaction that will take a certain duration.
This is expected to be used by a future change ("usb: dwc2: host:
Properly set even/odd frame").
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>
The dwc2 scheduler (contained in hcd_queue.c) was a bit confusing in the
way it initted / kept track of which frames a QH was going to be active
in. Let's clean things up a little bit in preparation for a rewrite of
the microframe scheduler.
Specifically:
* Old code would pick a frame number in dwc2_qh_init() and would try to
pick it "in a slightly future (micro)frame". As far as I can tell the
reason for this was that there was a delay between dwc2_qh_init() and
when we actually wanted to dwc2_hcd_qh_add(). ...but apparently this
attempt to be slightly in the future wasn't enough because
dwc2_hcd_qh_add() then had code to reset things if the frame _wasn't_
in the future. There's no reason not to just pick the frame later.
For non-periodic QH we now pick the frame in dwc2_hcd_qh_add(). For
periodic QH we pick the frame at dwc2_schedule_periodic() time.
* The old "dwc2_qh_init() actually assigned to "hsotg->frame_number".
This doesn't seem like a great idea since that variable is supposed to
be used to keep track of which SOF the interrupt handler has seen.
Let's be clean: anyone who wants the current frame number (instead of
the one as of the last interrupt) should ask for it.
* The old code wasn't terribly consistent about trying to use the frame
that the microframe scheduler assigned to it. In
dwc2_sched_periodic_split() when it was scheduling the first frame it
always "ORed" in 0x7 (!). Since the frame goes on the wire 1 uFrame
after next_active_frame it meant that the SSPLIT would always try for
uFrame 0 and the transaction would happen on the low speed bus during
uFrame 1. This is irregardless of what the microframe scheduler
said.
* The old code assumed it would get called to schedule the next in a
periodic split very quickly. That is if next_active_frame was
0 (transfer on wire in uFrame 1) it assumed it was getting called to
schedule the next uFrame during uFrame 1 too (so it could queue
something up for uFrame 2). It should be possible to actually queue
something up for uFrame 2 while in uFrame 2 (AKA queue up ASAP). To
do this, code needs to look at the previously scheduled frame when
deciding when to next be active, not look at the current frame number.
* If there was no microframe scheduler, the old code would check for
whether we should be active using "qh->next_active_frame ==
frame_number". This seemed like a race waiting to happen. ...plus
there's no way that you wouldn't want to schedule if next_active_frame
was actually less than frame number.
Note that this change doesn't make 100% sense on its own since it's
expecting some sanity in the frame numbers assigned by the microframe
scheduler and (as per the future patch which rewries it) I think that
the current microframe scheduler is quite insane. However, it seems
like splitting this up from the microframe scheduler patch makes things
into smaller chunks and hopefully adds to clarity rather than reduces
it. The two patches could certainly be squashed. Not that in the very
least, I don't see any obvious bad behavior introduced with just this
patch.
I've attempted to keep the config parameter to disable the microframe
scheduler in tact in this change, though I'm not sure it's worth it.
Obviously the code is touched a lot so it's possible I regressed
something when the microframe scheduler is disabled, though I did some
basic testing and it seemed to work OK. I'm still not 100% sure why you
wouldn't want the microframe scheduler (presuming it works), so maybe a
future patch (or a future version of this patch?) could remove that
parameter.
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>
We'll use the new "scheduler verbose debugging" macro to log missed
SOFs. This is fast enough (assuming you configure it to use the ftrace
buffer) that we can do it without worrying about the speed hit. The
overhead hit if the scheduler tracing is set to "no_printk" should be
near zero.
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>
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>
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: 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>
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: 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>
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: 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>
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: John Youn <johnyoun@synopsys.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: John Youn <johnyoun@synopsys.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
In commit 94dfd7edfd ("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: 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>
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 9118f9eb4f ("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: 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>
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: John Youn <johnyoun@synopsys.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: John Youn <johnyoun@synopsys.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Yunzhi Li <lyz@rock-chips.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Kever Yang <kever.yang@rock-chips.com>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: John Youn <johnyoun@synopsys.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: John Youn <johnyoun@synopsys.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: John Youn <johnyoun@synopsys.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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 <dianders@chromium.org>
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: John Youn <johnyoun@synopsys.com>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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 e8f8c14d9d ("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: Douglas Anderson <dianders@chromium.org>
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
There will be data toggle error happen for full speed buld-out transfer.
The data toggle bit is saved in qh for non-control transfers, it is wrong
to check the qtd for that case.
Also fix one static analysis tool issue after fix the data toggle error.
John Youn:
* Added WARN() to warn on improper usage of the
dwc2_hcd_save_data_toggle() function.
Signed-off-by: Dyson Lee <dyson.lee@intel.com>
Signed-off-by: Tang, Jianqiang <jianqiang.tang@intel.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Fixes memory manipulation issues and makes Host DDMA bulk transfers
work.
dwc2_process_non_isoc_desc() must return non zero value ONLY when
failure happens in one of the queued descriptors. After receiving
non zero value the caller must stop processing of remaining
QTDs and their descriptors from chain.
Commit 26a19ea699 ("usb: dwc2: host: fix use of qtd after
free in desc dma mode") breaks non_isoc transaction completion logic
in Host DDMA mode. There were bugs before that, but after this patch
dwc2_process_non_isoc_desc() returns fail status even if descriptor
was processed normally. This causes break from loop which is processing
remaining descriptors assigned to QTD, which is not correct for QTDs
containing more than one descriptor.
Current dwc2 driver gathers queued BULK URBs until receiving URB
without URB_NO_INTERRUPT flag. Once getting it, SW creates
descriptor chain, stores it in qh structure and passes start
address to HW. Multiple URB data is contained in that chain.
Hence on getting error on descriptor after its processing by HW,
SW should go out of both loops(qh->qtd, qtd->descs) and report
the failure.
Fixes: 26a19ea699 ("usb: dwc2: host: fix use of qtd after free in desc dma mode")
Cc: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Vardan Mikayelyan <mvardan@synopsys.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Fixes an issue found on rockchip rk3036 and rk3188 SOC platforms. For
some reason, the existing msleep(25) is not enough after the force mode.
The following patch was reported to fix the issue.
This does increase the probe delay again slightly, but not up to the
level it was before the original series of patches that this fixes. It
does not cause any other issues when tested on Synopsys HAPS and Altera
socfpga platforms.
Need to revisit this series next release to see if we can address these
issues without having an unconditional delay.
Fixes: 09c96980dc ("usb: dwc2: Add functions to set and clear force mode")
Reported-by: Caesar Wang <caesar.upstream@gmail.com>
Reported-by: Michael Niewoehner <linux@mniewoehner.de>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Caesar Wang <caesar.upstream@gmail.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Fixes an issue found on Raspberry PI platform that prevents probe. Don't
skip setting the force mode if it's already set.
Fixes: 09c96980dc ("usb: dwc2: Add functions to set and clear force mode")
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Reported-by: Stefan Wahren <stefan.wahren@i2se.com>
Reported-by: Remi Pommarel <repk@triplefau.lt>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Tested-by: Remi Pommarel <repk@triplefau.lt>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
This reverts commit 263b7fb557 ("usb: dwc2: Move reset into
dwc2_get_hwparams()") due to regression found on bcm2835 platform. USB
ethernet fails, due to being unable to pick up proper parameters when
performing a plain reset before reading hw params.
Below shows the results of the gnptxfsiz and hptxfsiz with and before
and after reverting this (from Stefan Wahren):
So here is the probe result before Patch 1 is applied:
[ 1.283148] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to host
[ 1.313894] dwc2 20980000.usb: gnptxfsiz=00201000
[ 1.314104] dwc2 20980000.usb: hptxfsiz=00000000
[ 1.353908] dwc2 20980000.usb: 256 invalid for host_nperio_tx_fifo_size. Check HW configuration.
[ 1.354262] dwc2 20980000.usb: 512 invalid for host_perio_tx_fifo_size. Check HW configuration.
[ 1.394249] dwc2 20980000.usb: DWC OTG Controller
[ 1.394561] dwc2 20980000.usb: new USB bus registered, assigned bus number 1
[ 1.394917] dwc2 20980000.usb: irq 33, io mem 0x00000000
And here is the probe result after Patch 1 is applied:
[ 1.280107] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to host
[ 1.353949] dwc2 20980000.usb: gnptxfsiz=01001000
[ 1.354166] dwc2 20980000.usb: hptxfsiz=02002000
[ 1.434301] dwc2 20980000.usb: DWC OTG Controller
[ 1.434616] dwc2 20980000.usb: new USB bus registered, assigned bus number 1
[ 1.434973] dwc2 20980000.usb: irq 33, io mem 0x00000000
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Reported-by: Stefan Wahren <stefan.wahren@i2se.com>
Reported-by: Remi Pommarel <repk@triplefau.lt>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Tested-by: Remi Pommarel <repk@triplefau.lt>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
In specific conditions (involving usb hubs) dwc2 devices can create a
lot of interrupts, even to the point of overwhelming devices running
at low frequencies. Some devices need to do special clock handling
at shutdown-time which may bring the system clock below the threshold
of being able to handle the dwc2 interrupts. Disabling dwc2-irqs
in a shutdown callbacks prevents reboots/poweroffs from getting stuck
in such cases.
The hsotg struct already contains an unused irq element, so we can
just use it to store the irq number for the shutdown callback.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@collabora.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
The "enumspd" field is located in register DSTS[2:1], but the code
which checks the bitfield does not shift the value accordingly. This
in turn causes incorrect detection of gadget link partner speed in
dwc2_hsotg_irq_enumdone() .
Shift the value accordingly to fix the problem with speed detection.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Remove call to dwc2_hsotg_init() from dwc2_gadget_init(). The
gadget_init function should not access any device registers because the
mode isn't guaranteed here.
Also, this is already called elsewhere before anything starts on the
gadget so it is not necessary here.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Reset already happens before this so just force the dr_mode.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
The delay for force mode is only 25ms according to the databook.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
The dwc2_core_reset() function exists in the core so use that one
instead.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Use the previously cached hw params in the gadget. This saves a reset
and force mode in the gadget initialization during probe and makes
getting the hardware parameters consistent between gadget and host.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Adds separate functions to get the host and device specific hardware
parameters. The functions check whether the parameters need to be read
at all, depending on dr_mode, and forces the mode only if necessary.
This saves some delays during probe. This also adds two device mode
parameters that will be used by the gadget.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Added functions to set force mode for host and device. These functions
will check the current mode and only force if needed thus avoiding
unnecessary force mode delays. However clearing the mode is currently
done unconditionally and with the delay in place. This is needed during
the connector ID status change interrupt in order to ensure that the
mode has changed properly. This preserves the old behavior only for this
case. The warning comment about this is moved into the clear mode
condition.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
The reset is required to get reset values of the hardware parameters but
the force mode is not. Move the base reset into dwc2_get_hwparams() and
do the reset and force mode afterwards.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
These functions should go in core.h where they can be called from core,
device, or host.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
The dr_mode parameter was being checked against how the dwc2 module
was being configured at compile time. But it wasn't checked against
the hardware capabilities, nor were the hardware capabilities checked
against the compilation parameters.
This commit adds those checks and adjusts dr_mode to an appropriate
value, if needed. If the hardware capabilities and module compilation
do not match then we fail as it wouldn't be possible to run properly.
The hardware, module, and dr_mode, can each be set to host, device,
or otg. Check that all these values are compatible and adjust the
value of dr_mode if possible.
The following table summarizes the behavior:
actual
HW MOD dr_mode dr_mode
------------------------------
HST HST any : HST
HST DEV any : ---
HST OTG any : HST
DEV HST any : ---
DEV DEV any : DEV
DEV OTG any : DEV
OTG HST any : HST
OTG DEV any : DEV
OTG OTG any : dr_mode
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Added functions to query the GHWCFG2.OTG_MODE. This tells us whether the
controller hardware is configured for OTG, device-only, or host-only.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Tested-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
dwc2_core_reset() was previously renamed to
dwc2_core_reset_and_dr_force_mode(). Now add back dwc2_core_reset() which
performs only a basic core reset without forcing the mode.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Renamed dwc2_core_reset() to dwc2_core_reset_and_force_dr_mode(). This
describes what it is doing more accurately. This is in preparation of
introducing a plain dwc2_core_reset() function that only performs the
reset and doesn't force the mode.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
According to the databook, the core soft reset should be done before
checking for AHBIDLE. The gadget version of core reset had it correct
but the hcd version did not. This fixes the hcd version.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Calls to dwc2_core_reset() are currently very slow, taking at least
150ms (possibly more). It behooves us to take as many of these calls
out as possible.
It turns out that the calls in dwc2_fs_phy_init() and dwc2_hs_phy_init()
should (as documented in the code) only be needed if we need to do a PHY
SELECT. That means that if we see that we can avoid the PHY SELECT then
we can avoid the reset.
This patch appears to successfully bypass two resets (one per USB
device) on rk3288-based ARM Chromebooks.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
I found that the probe function of dwc2 driver takes much time
when kernel boot up. There are many long delays in the probe
function these take almost 1 second.
This patch trying to reduce unnecessary delay time.
In dwc2_core_reset() I see it use two at least 20ms delays to
wait AHB idle and core soft reset, but dwc2 data book said that
dwc2 core soft reset and AHB idle just need a few clocks (I think
it refers to AHB clock, and AHB clock run at 150MHz in my RK3288
board), so 20ms is too long, delay 1us for wait AHB idle and soft
reset is enough.
And in dwc2_get_hwparams() it takes 150ms to wait ForceHostMode
and ForceDeviceMode valid but in data book it said software must
wait at least 25ms before the change to take effect, so I reduce
this time to 25ms~50ms. By the way, is there any state bit show
that the force mode take effect ? Could we poll curmod bit for
figuring out if the change take effect ?
It seems that usleep_range() at boot time will pick the longest
value in the range. In dwc2_core_reset() there is a very long
delay takes 200ms, and this function run twice when probe, could
any one tell me is this delay time resonable ?
I have tried this patch in my RK3288-evb board. It works well.
Signed-off-by: Yunzhi Li <lyz@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
On some host-only DWC2 ports (like the one in rk3288) when we set
GUSBCFG_FORCEHOSTMODE in GUSBCFG and then read back, we don't see the
bit set. Presumably that's because the port is always forced to HOST
mode so there's no reason to implement these status bits.
Since we know dwc2_core_reset() is always called before
dwc2_get_hwparams() and we know dwc2_core_reset() should have set
GUSBCFG_FORCEHOSTMODE whenever hsotg->dr_mode == USB_DR_MODE_HOST, we
can just check hsotg->dr_mode to decide that we can skip the delays in
dwc2_get_hwparams().
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
In (usb: dwc2: reset dwc2 core before dwc2_get_hwparams()) we added an
extra reset to the probe path for the dwc2 USB controllers. This
allowed proper detection of parameters even if the firmware had already
used the USB part.
Unfortunately, this extra reset is quite slow and is affecting boot
speed. We can avoid the double-reset by skipping the extra reset that
would happen just after the one we added. Logic that explains why this
is safe:
* As of the CL mentioned above, we now always call dwc2_core_reset() in
dwc2_driver_probe() before dwc2_hcd_init().
* The only caller of dwc2_hcd_init() is dwc2_driver_probe(), so we're
guaranteed that dwc2_core_reset() was called before dwc2_hdc_init().
* dwc2_hdc_init() is the only caller that passes an irq other than -1 to
dwc2_core_init(). Thus if dwc2_core_init() is called with an irq
other than -1 we're guaranteed that dwc2_core_reset was called before
dwc2_core_init().
...this allows us to remove the dwc2_core_reset() in dwc2_core_init() if
irq is not < 0.
Note that since "irq" wasn't used in the function dwc2_core_init()
anyway and since select_phy was always set at exactly the same times we
could avoid the reset, we remove "irq" and rename "select_phy" to
"initial_setup" and adjust the callers accordingly.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
We initiate dwc2 usb controller in BIOS, dwc2_core_reset() should
be called before dwc2_get_hwparams() to reset core registers to
default value. Without this the FIFO setting might be incorrect
because calculating FIFO size need power-on value of
GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers.
This patch could avoid warnning massage like in rk3288 platform:
[ 2.074764] dwc2 ff580000.usb: 256 invalid for
host_perio_tx_fifo_size. Check HW configuration.
Signed-off-by: Yunzhi Li <lyz@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Previously dwc2_get_hwparams() was changing GUSBCFG and not putting it
back the way it was (specifically it set and cleared FORCEHOSTMODE).
Since we want to move dwc2_core_reset() _before_ dwc2_get_hwparams() we
should make sure dwc2_get_hwparams() isn't messing with things in a
permanent way.
Since we're now looking at GUSBCFG, it's obvious that we shouldn't need
all the extra delays if FORCEHOSTMODE was already set. This will avoid
some delays for any ports that have forced host mode.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
When receiving GINTSTS_GINNAKEFF or GINTSTS_GOUTNAKEFF interrupt,
DCTL will be overwritten with DCTL_CGOUTNAK or DCTL_CGNPINNAK values.
Instead of overwriting it, write only needed bits.
It could cause an issue if GINTSTS_GINNAKEFF or GINTSTS_GOUTNAKEFF
interrupt is received after dwc2 disabled pullup by writing
DCTL_SFTDISCON bit.
Pullup will then be re-enabled whereas it should not.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
To stop an out endpoint, software should set sets the Global OUT NAK,
but not the Global Non-periodic IN NAK. This driver bug leads the out-ep
failed be in disabled state with below error.
dwc2_hsotg_ep_stop_xfr: timeout DOEPCTL.EPDisable
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Du, Changbin <changbin.du@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
In general it is wise to clear interrupts before processing them. If
you don't do that, you can get:
1. Interrupt happens
2. You look at system state and process interrupt
3. A new interrupt happens
4. You clear interrupt without processing it.
This patch was actually a first attempt to fix missing device insertions
as described in (usb: dwc2: host: Fix missing device insertions) and it
did solve some of the signal bouncing problems but not all of
them (which is why I submitted the other patch). Specifically, this
patch itself would sometimes change:
1. hardware sees connect
2. hardware sees disconnect
3. hardware sees connect
4. dwc2_port_intr() - clears connect interrupt
5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()
...to:
1. hardware sees connect
2. hardware sees disconnect
3. dwc2_port_intr() - clears connect interrupt
4. hardware sees connect
5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()
...but with different timing then sometimes we'd still miss cable
insertions.
In any case, though this patch doesn't fix any (known) problems, it
still seems wise as a general policy to clear interrupt before handling
them.
Note that for dwc2_handle_usb_port_intr(), instead of moving the clear
of PRTINT to the beginning of the function we remove it completely. The
only way to clear PRTINT is to clear the sources that set it in the
first place.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
The dwc2_hcd_reset_func() function is only ever called directly by a
delayed work function. As such no locks are already held when the
function is called.
Doing a read-modify-write of CPU registers and setting fields in the
main hsotg data structure is a bad idea without locks. Let's add
locks.
The bug was found by code inspection only. It turns out that the
dwc2_hcd_reset_func() is only ever called today if the
"host_support_fs_ls_low_power" parameter is enabled and no code in
mainline enables that parameter. Thus no known issues in mainline are
fixed by this patch, but it's still probably wise to fix the function.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
In some cases, like when you've got a "Microsoft Wireless Keyboard 2000"
connected to dwc2 with a hub, expected that we'll get some transfer
errors sometimes. The controller is expected to try at least 3 times
before giving up. See figure "Figure A-67. Normal HS CSPLIT 3 Strikes
Smash" in the USB spec.
The dwc2 controller has a way to support this by using the "EC_MC"
field. The Raspberry Pi driver has logic for setting this right. See
fiq_fsm_queue_split_transaction() in their "dwc_otg_hcd.c". Let's use
the same logic.
After making this change, we no longer get dropped characters from the
above mentioned keyboard. Other devices on the same bus as the keyboard
also behave more properly.
Thanks for Julius Werner for the expert analysis and suggestions.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
If you've got your interrupt signals bouncing a bit as you insert your
USB device, you might end up in a state when the device is connected but
the driver doesn't know it.
Specifically, the observed order is:
1. hardware sees connect
2. hardware sees disconnect
3. hardware sees connect
4. dwc2_port_intr() - clears connect interrupt
5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()
Now you'll be stuck with the cable plugged in and no further interrupts
coming in but the driver will think we're disconnected.
We'll fix this by checking for the missing connect interrupt and
re-connecting after the disconnect is posted. We don't skip the
disconnect because if there is a transitory disconnect we really want to
de-enumerate and re-enumerate.
Notes:
1. As part of this change we add a "force" parameter to
dwc2_hcd_disconnect() so that when we're unloading the module we
avoid the new behavior. The need for this was pointed out by John
Youn.
2. The bit of code needed at the end of dwc2_hcd_disconnect() is
exactly the same bit of code from dwc2_port_intr(). To avoid
duplication, we refactor that code out into a new function
dwc2_hcd_connect().
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Support hisilicon,hi6220-usb for HiKey board
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Kmem caches help to get correct boundary for descriptor buffers
which need to be 512 bytes aligned for dwc2 controller.
Two kmem caches are needed for generic descriptors and for
hs isochronous descriptors which doesn't have same size.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Masks for HCDMA.CTD and HCDMA.DMAAddr are incorrect. As we always
start from first descriptor, no need to mask the address anyway.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Use Streaming DMA mappings to handle cache coherency of frame list and
descriptor list. Cache are always flushed before controller access it
or before cpu access it.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
As descriptor dma mode does not support split transfers, it can't be
enabled for high speed devices. Add a core parameter to enable it for
full speed devices.
Ensure frame list and descriptor list are correctly freed during
disconnect.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Process all completed urbs, if more urbs are complete by the time
driver processes completion interrupt.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
When releasing a channel, increment hsotg->available_host_channels even
in case a periodic channel is released.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Isochronous descriptor is currently programmed for the frame
after the last descriptor was programmed.
If the last descriptor frame underrun, then current descriptor must
take this into account and must be programmed on the current frame + 1.
This overrun usually happens when system is loaded and dwc2 can't init
descriptor list in time.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
This function allow comparing frame index used for
descriptor list which has 64 entries.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Prevent dwc2 driver from accessing channel while it frees it.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
When completing non isoc xfer, dwc2_complete_non_isoc_xfer_ddma()
is relying on qtd->n_desc to process the corresponding number of
descriptors.
During the processing of these descriptors, qtd could be unlinked
and freed if xfer is done and urb is no more in progress.
In this case, dwc2_complete_non_isoc_xfer_ddma() will read again
qtd->n_desc whereas qtd has been freed. This will lead to unpredictable
results since qtd->n_desc is no more valid value.
To avoid this error, return a result != 0 in dwc2_process_non_isoc_desc(),
so that dwc2_complete_non_isoc_xfer_ddma() stops desc processing.
This has been seen with Slub debug enabled.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
When a channel is halted because of urb dequeue during transfer
completion, no other qtds must be scheduled until halt is done.
Moreover, all in progress qtds must be given back.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Active bit must be enabled in all scheduled descriptors. Else transfer
never start.
Remove previous code which was not correctly configuring descriptors.
Active bit was set before calling dwc2_fill_host_isoc_dma_desc() which
is erasing dma_desc->status.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Increment qtd->isoc_frame_index_last before testing it, else below
check will never be true and IOC (Interrupt On Complete) bit for
last frame will never be set in descriptor status.
/* Set IOC for each descriptor corresponding to last frame of URB */
if (qtd->isoc_frame_index_last == qtd->urb->packet_count)
dma_desc->status |= HOST_DMA_IOC;
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
This patch make sure that all necessary members of dwc2_hsotg
are initialized before the irq handler is requested. So
the kernel oops triggered by dwc2_handle_common_intr has
been fixed.
dwc2 20980000.usb: Configuration mismatch. Forcing host mode
dwc2 20980000.usb: no platform data or transceiver defined
Unable to handle kernel paging request at virtual address cc860040
pgd = c0004000
[cc860040] *pgd=0b41e811, *pte=00000000, *ppte=00000000
Internal error: Oops: 7 [#1] ARM
CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0-rc3+ #19
Hardware name: BCM2835
task: cb494000 ti: cb4d0000 task.ti: cb4d0000
PC is at dwc2_is_controller_alive+0x18/0x34
LR is at dwc2_handle_common_intr+0x24/0xb60
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Fixes commit 09a75e8577
("usb: dwc2: refactor common low-level hw code to platform.c")
The above commit consolidated the low-level phy access into a common
location. This change made the otg clk a requirement and broke some
platforms when it was moved into platform.c.
So make clk handling optional again.
Acked-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Fixes: 09a75e8577 ("usb: dwc2: refactor common low-level hw code to platform.c")
Signed-off-by: Felipe Balbi <balbi@ti.com>
When searching for PHYs, any error was treated as if the PHY did not
exist or was not specified. Thus the probe function did not
correctly return error conditions such as -EPROBE_DEFER.
Fixed so that only a non-existing PHY is ignored and any other error
is returned.
Acked-by: Eric Anholt <eric@anholt.net>
Reported-by: Alexander Aring <alex.aring@gmail.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Fixes commit 09a75e8577
("usb: dwc2: refactor common low-level hw code to platform.c")
The above commit consolidated the low-level phy access into a common
location. This change introduced a check from the gadget requiring
that a PHY is specified. This requirement never existed on the host
side and broke some platforms when it was moved into platform.c.
The gadget doesn't require the PHY either so remove the check.
Acked-by: Eric Anholt <eric@anholt.net>
Reported-by: Stefan Wahren <info@lategoodbye.de>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Fixes: 09a75e8577 ("usb: dwc2: refactor common low-level hw code to platform.c")
Signed-off-by: Felipe Balbi <balbi@ti.com>
In commit 734643dfbd ("usb: dwc2: host: add flag to reflect bus
state") we changed dwc2_port_suspend() not to set the lx_state
anymore (instead it sets the new bus_suspended variable). This
introduced a bug where we would fail to detect device insertions if:
1. Plug empty hub into dwc2
2. Plug USB flash drive into the empty hub.
3. Wait a few seconds
4. Unplug USB flash drive
5. Less than 2 seconds after step 4, plug the USB flash drive in again.
The dwc2_hcd_rem_wakeup() function should have been changed to look at
the new bus_suspended variable.
Let's fix it. Since commit b46146d59f ("usb: dwc2: host: resume root
hub on remote wakeup") talks about needing the root hub resumed if the
bus was suspended, we'll include it in our test.
It appears that the "port_l1_change" should only be set to 1 if we were
in DWC2_L1 (the driver currently never sets this), so we'll update the
former "else" case based on this test.
Fixes: 734643dfbd ("usb: dwc2: host: add flag to reflect bus state")
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: Gregory Herrero <gregory.herrero@intel.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
The comment for ahbcfg for rk3066 parameters (also used for rk3288)
claimed that ahbcfg was INCR16, but it wasn't. Since the bits weren't
shifted properly, the 0x7 ended up being masked and we ended up
programming 0x3 for the HBstLen. Let's set it to INCR16 properly.
As per Wu Liang Feng at Rockchip this may increase transmission
efficiency. I did blackbox tests with writing 0s to a USB-based SD
reader (forcefully capping CPU Freq to try to measure efficiency):
cd /sys/devices/system/cpu/cpu0/cpufreq
echo userspace > scaling_governor
echo 126000 > scaling_setspeed
for i in $(seq 10); do
dd if=/dev/zero of=/dev/sdb bs=1M count=750
done
With the above tests I found that speeds went from ~15MB/s to ~18MB/s.
Note that most other tests I did (including reading from the same USB
reader) didn't show any difference in performance.
Tested-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: John Youn <johnyoun@synopsys.com>
Reviewed-by: Liangfeng Wu <wulf@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
While plugging / unplugging on a DWC2 host port with "slub_debug=FZPUA"
enabled, I found a crash that was quite obviously a use after free.
It appears that in some cases when we handle the various sub-cases of
HCINT we may end up freeing the QTD. If there is more than one bit set
in HCINT we may then end up continuing to use the QTD, which is bad.
Let's be paranoid and check for this after each sub-case. This should
be safe since we officially have the "hsotg->lock" (it was grabbed in
dwc2_handle_hcd_intr).
The specific crash I found was:
Unable to handle kernel paging request at virtual address 6b6b6b9f
At the time of the crash, the kernel reported:
(dwc2_hc_nak_intr+0x5c/0x198)
(dwc2_handle_hcd_intr+0xa84/0xbf8)
(_dwc2_hcd_irq+0x1c/0x20)
(usb_hcd_irq+0x34/0x48)
Popping into kgdb found that "*qtd" was filled with "0x6b", AKA qtd had
been freed and filled with slub_debug poison.
kgdb gave a little better stack crawl:
0 dwc2_hc_nak_intr (hsotg=hsotg@entry=0xec42e058,
chan=chan@entry=0xec546dc0, chnum=chnum@entry=4,
qtd=qtd@entry=0xec679600) at drivers/usb/dwc2/hcd_intr.c:1237
1 dwc2_hc_n_intr (chnum=4, hsotg=0xec42e058) at
drivers/usb/dwc2/hcd_intr.c:2041
2 dwc2_hc_intr (hsotg=0xec42e058) at drivers/usb/dwc2/hcd_intr.c:2078
3 dwc2_handle_hcd_intr (hsotg=0xec42e058) at
drivers/usb/dwc2/hcd_intr.c:2128
4 _dwc2_hcd_irq (hcd=<optimized out>) at drivers/usb/dwc2/hcd.c:2837
5 usb_hcd_irq (irq=<optimized out>, __hcd=<optimized out>) at
drivers/usb/core/hcd.c:2353
Popping up to frame #1 (dwc2_hc_n_intr) found:
(gdb) print /x hcint
$12 = 0x12
AKA:
#define HCINTMSK_CHHLTD (1 << 1)
#define HCINTMSK_NAK (1 << 4)
Further debugging found that by simulating receiving those two
interrupts at the same time it was trivial to replicate the
use-after-free. See <http://crosreview.com/305712> for a patch and
instructions. This lead to getting the following stack crawl of the
actual free:
0 arch_kgdb_breakpoint () at arch/arm/include/asm/outercache.h:103
1 kgdb_breakpoint () at kernel/debug/debug_core.c:1054
2 dwc2_hcd_qtd_unlink_and_free (hsotg=<optimized out>, qh=<optimized
out>, qtd=0xe4479a00) at drivers/usb/dwc2/hcd.h:488
3 dwc2_deactivate_qh (free_qtd=<optimized out>, qh=0xe5efa280,
hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:671
4 dwc2_release_channel (hsotg=hsotg@entry=0xed424618,
chan=chan@entry=0xed5be000, qtd=<optimized out>,
halt_status=<optimized out>) at drivers/usb/dwc2/hcd_intr.c:742
5 dwc2_halt_channel (hsotg=0xed424618, chan=0xed5be000, qtd=<optimized
out>, halt_status=<optimized out>) at
drivers/usb/dwc2/hcd_intr.c:804
6 dwc2_complete_non_periodic_xfer (chnum=<optimized out>,
halt_status=<optimized out>, qtd=<optimized out>, chan=<optimized
out>, hsotg=<optimized out>) at drivers/usb/dwc2/hcd_intr.c:889
7 dwc2_hc_xfercomp_intr (hsotg=hsotg@entry=0xed424618,
chan=chan@entry=0xed5be000, chnum=chnum@entry=6,
qtd=qtd@entry=0xe4479a00) at drivers/usb/dwc2/hcd_intr.c:1065
8 dwc2_hc_chhltd_intr_dma (qtd=0xe4479a00, chnum=6, chan=0xed5be000,
hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:1823
9 dwc2_hc_chhltd_intr (qtd=0xe4479a00, chnum=6, chan=0xed5be000,
hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:1944
10 dwc2_hc_n_intr (chnum=6, hsotg=0xed424618) at
drivers/usb/dwc2/hcd_intr.c:2052
11 dwc2_hc_intr (hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:2097
12 dwc2_handle_hcd_intr (hsotg=0xed424618) at
drivers/usb/dwc2/hcd_intr.c:2147
13 _dwc2_hcd_irq (hcd=<optimized out>) at drivers/usb/dwc2/hcd.c:2837
14 usb_hcd_irq (irq=<optimized out>, __hcd=<optimized out>) at
drivers/usb/core/hcd.c:2353
Though we could add specific code to handle this case, adding the
general purpose code to check for all cases where qtd might be freed
seemed safer.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
From code inspection, it appears to be unsafe to do a read-modify-write
of PCGCTL in dwc2_port_resume(). Let's make sure the spinlock is held
around this operation.
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
DWC2 module on some platforms needs three additional hardware
resources: phy controller, clock and power supply. All of them must be
enabled/activated to properly initialize and operate. This was initially
handled in s3c-hsotg driver, which has been converted to 'gadget' part
of dwc2 driver. Unfortunately, not all of this code got moved to common
platform code, what resulted in accessing DWC2 registers without
enabling low-level hardware resources. This fails for example on Exynos
SoCs. This patch moves all the code for managing those resources to
common platform.c file and provides convenient wrappers for controlling
them.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
init_mutex is a leftover from the time, when s3c-hsotg driver did not
implement proper pull up/down control and emulated it by enabling
enabling/disabling usb phy. Proper pull up/down control has been added
by commit 5b9451f8c4 ("usb: dwc2: gadget:
use soft-disconnect udc feature in pullup() method"), so init_muxtex can
be removed now to avoid potential deadlocks with other locks.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Fix a duplicate argument warning reported by 0-DAY kernel test
infrastructure in the following patch:
77dbf71 usb: dwc2: host: add disconnect interrupt to host only interrupts
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
USB OTG driver in isochronous mode has to set the parity of the receiving
microframe. The parity is set to even by default. This causes problems for
an audio gadget, if the host starts transmitting on odd microframes.
This fix uses Incomplete Periodic Transfer interrupt to toggle between
even and odd parity until the Transfer Complete interrupt is received.
Signed-off-by: Roman Bacik <rbacik@broadcom.com>
Reviewed-by: Abhinav Ratna <aratna@broadcom.com>
Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
Signed-off-by: Scott Branden <sbranden@broadcom.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Even when DWC2 is in (internal) suspended state, it should disable PHY
in suspend and then enable it in resume. This patch fixes unbalanced PHY
control sequence.
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
During typical gadget operation, dwc2 clock was enabled 3 times: from
dwc2_gadget_init(), dwc2_hsotg_udc_start() and dwc2_hsotg_pullup(), and
then disabled in s3c_hsotg_pullup(), s3c_hsotg_udc_stop() and
dwc2_hsotg_remove(). This really makes no sense, so leave clock control
code only in dwc2_gadget_init/remove functions.
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
This patch removes doubled call to dwc2_hsotg_of_probe() function.
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
When dwc2_hsotg_handle_unaligned_buf_complete() hs_req->req.buf
already destroyed, in dwc2_hsotg_unmap_dma(), it touches
hs_req->req.dma again, so dwc2_hsotg_unmap_dma() should be called
before dwc2_hsotg_handle_unaligned_buf_complete(). Otherwise, it
will cause a bad_page BUG, when allocate this memory page next
time.
This bug led to the following crash:
BUG: Bad page state in process swapper/0 pfn:2bdbc
[ 26.820440] page:eed76780 count:0 mapcount:0 mapping: (null) index:0x0
[ 26.854710] page flags: 0x200(arch_1)
[ 26.885836] page dumped because: PAGE_FLAGS_CHECK_AT_PREP flag set
[ 26.919179] bad because of flags:
[ 26.948917] page flags: 0x200(arch_1)
[ 26.979100] Modules linked in:
[ 27.008401] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W3.14.0 #17
[ 27.041816] [<c010e1f8>] (unwind_backtrace) from [<c010a704>] (show_stack+0x20/0x24)
[ 27.076108] [<c010a704>] (show_stack) from [<c087eea8>] (dump_stack+0x70/0x8c)
[ 27.110246] [<c087eea8>] (dump_stack) from [<c01ce0b8>] (bad_page+0xfc/0x12c)
[ 27.143958] [<c01ce0b8>] (bad_page) from [<c01ce65c>] (get_page_from_freelist+0x3e4/0x50c)
[ 27.179298] [<c01ce65c>] (get_page_from_freelist) from [<c01ce9a0>] (__alloc_pages_nodemask)
[ 27.216296] [<c01ce9a0>] (__alloc_pages_nodemask) from [<c01cf00c>] (__get_free_pages+0x20/)
[ 27.252326] [<c01cf00c>] (__get_free_pages) from [<c01e5bec>] (kmalloc_order_trace+0x34/0xa)
[ 27.288295] [<c01e5bec>] (kmalloc_order_trace) from [<c0203304>] (__kmalloc+0x40/0x1ac)
[ 27.323751] [<c0203304>] (__kmalloc) from [<c052abc0>] (dwc2_hsotg_ep_queue.isra.12+0x7c/0x1)
[ 27.359937] [<c052abc0>] (dwc2_hsotg_ep_queue.isra.12) from [<c052af88>] (dwc2_hsotg_ep_queue)
[ 27.397478] [<c052af88>] (dwc2_hsotg_ep_queue_lock) from [<c0554110>] (rx_submit+0xfc/0x164)
[ 27.433619] [<c0554110>] (rx_submit) from [<c05546e8>] (rx_complete+0x22c/0x230)
[ 27.468872] [<c05546e8>] (rx_complete) from [<c052b528>] (dwc2_hsotg_complete_request+0xfc/0)
[ 27.506240] [<c052b528>] (dwc2_hsotg_complete_request) from [<c052bba0>] (dwc2_hsotg_handle_o)
[ 27.545401] [<c052bba0>] (dwc2_hsotg_handle_outdone) from [<c052be70>] (dwc2_hsotg_epint+0x2c)
[ 27.583689] [<c052be70>] (dwc2_hsotg_epint) from [<c052c750>] (dwc2_hsotg_irq+0x1dc/0x4ac)
[ 27.621041] [<c052c750>] (dwc2_hsotg_irq) from [<c01682e0>] (handle_irq_event_percpu+0x70/0x)
[ 27.659066] [<c01682e0>] (handle_irq_event_percpu) from [<c01684ec>] (handle_irq_event+0x4c)
[ 27.697322] [<c01684ec>] (handle_irq_event) from [<c016bae0>] (handle_fasteoi_irq+0xc8/0x11)
[ 27.735451] [<c016bae0>] (handle_fasteoi_irq) from [<c0167b8c>] (generic_handle_irq+0x30/0x)
[ 27.773918] [<c0167b8c>] (generic_handle_irq) from [<c0167ca4>] (__handle_domain_irq+0x84/0)
[ 27.812018] [<c0167ca4>] (__handle_domain_irq) from [<c01003b0>] (gic_handle_irq+0x48/0x6c)
[ 27.849695] [<c01003b0>] (gic_handle_irq) from [<c010b340>] (__irq_svc+0x40/0x50)
[ 27.886907] Exception stack(0xc0d01ee0 to 0xc0d01f28)
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Signed-off-by: Yunzhi Li <lyz@rock-chips.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Controller enters hibernation through suspend interrupt on
disconnection. On connection, session request interrupt is generated.
dwc2 must exit hibernation and restore state from this interrupt
before continuing.
In host mode, exit from hibernation is handled by bus_resume function.
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
If system is loaded, reset, enum-done and setup interrupts can occur
at the same time. Current interrupt handling sequence will handle
setup packet's interrupt before handling reset interrupt. Which will
break the enumeration process. Correct sequence is to handle reset,
enum-done and then any other endpoint interrupts.
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
When disconnecting cable, controller will detect a suspend condition
and enter partial power down. If vbus_session is called by the phy
driver during hibernation, make sure controller exit hibernation
before it is accessed.
Signed-off-by: Jianqiang Tang <jianqiang.tang@intel.com>
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
idstschng interrupt should not be used when id pin control is
external. This is already handled on dwc2 host part. Fix it on gadget
part as well.
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
There is a 200ms guard period to avoid unnecessary resets of the dwc2
ip. This delay sometimes prove to be too large when usbcv is run with
an ehci host. dwc2 only needs to be reset after addressed state.
Change the logic to reset ip after addressed state.
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Make sure there are no requests pending on ep0 before reinitializing
core. Otherwise, dwc2_hsotg_enqueue_setup will fail afterwards.
Also, take hsotg->lock before calling
dwc2_hsotg_core_init_disconnected() from dwc2_conn_id_status_change()
as dwc2_hsotg_complete_request() expect lock to be held.
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
If the request being dequeued is already started, disable endpoint
to stop the transfer and then call dwc2_hsotg_complete_request().
Endpoint will be re-enabled on next call to dwc2_hsotg_start_req().
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
wIndex field was missing. Also print in natural order instead of
Req first, so that its easier to compare for example against
bus analyzer logs.
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
dwc2_hsotg_start_req starts a request only if endpoint is not stalled.
Ignore this check for ep0 as core will clear DOEPCTL0.Stall after
sending stall handshake. Prepare instead for receiving next setup
packet.
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
No point of continue with initialization if core is not in a sane
state.
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Some device may have external id pin control enabled, so op_state
will not be set on id pin interrupt change.
Thus, ensure op_state is set to peripheral during vbus detection.
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Modifying the pullup state during host mode trig a new enumeration
of attached device. Thus, avoid modifying pullup in host mode.
Signed-off-by: Gregory Herrero <gregory.herrero@intel.com>
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
If dual role configuration is not selected, check and force dr_mode
based on the selected configuration.
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Tested-by: John Youn <johnyoun@synopsys.com>
Acked-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
