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>
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: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: Felipe Balbi <balbi@kernel.org>
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: Felipe Balbi <balbi@kernel.org>
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: Peter Chen <peter.chen@nxp.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Update HNP test procedure as HNP polling is supported.
Acked-by: Peter Chen <peter.chen@nxp.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Enable HNP polling support for chipidea gadget and allocate memory
for host request flag when otg fsm init.
Acked-by: Peter Chen <peter.chen@nxp.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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 <jun.li@nxp.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: Peter Chen <peter.chen@freescale.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Since gadget driver will handle this request, so controller driver bypass
it.
Acked-by: Peter Chen <peter.chen@freescale.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: Peter Chen <peter.chen@nxp.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: Peter Chen <peter.chen@freescale.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: Peter Chen <peter.chen@freescale.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Since commit 855ed04a37 ("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: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: Felipe Balbi <balbi@kernel.org>
We added a new error path to this function and we forgot to drop the
lock.
Fixes: e1e3d7ec5d ('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 <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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 <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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 <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
remove a field which is unnecessary. No functional changes.
Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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 <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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 <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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 <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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 <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
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: Felipe Balbi <balbi@kernel.org>
When I wrote the cleanup patch series, it was not clear how
exactly big-endian mode works on ixp4xx, and whether the driver
was doing this correctly. After discussing with Krzysztof Hałasa,
this has been clarified, so I can update the comment let pxa25x
big-endian (which we don't support) work the same way as ixp4xx.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
When dma_addr_t is 64-bit, we get a warning about an invalid cast
in the call to ux500_dma_is_compatible() from ux500_dma_channel_program():
drivers/usb/musb/ux500_dma.c: In function 'ux500_dma_channel_program':
drivers/usb/musb/ux500_dma.c:210:51: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
if (!ux500_dma_is_compatible(channel, packet_sz, (void *)dma_addr, len))
The problem is that ux500_dma_is_compatible() is called from the
main musb driver on the virtual address, but here we pass in a
DMA address, so the types are fundamentally different but it works
because the function only checks the alignment of the buffer and
that is the same.
We could work around this by adding another cast, but I have checked
that the buffer we get passed here is already checked before it
gets mapped, so the second check seems completely unnecessary
and removing it must be the cleanest solution.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
The musb driver prints DMA addresses in a few places, using the
0x%x format string. This is wrong on 64-bit architectures (which
need %lx) and 32-bit ARM with CONFIG_LPAE set (which needs
%llx), otherwise we print the wrong data, as gcc warns:
musb/musbhsdma.c: In function 'configure_channel':
musb/musbhsdma.c:120:53: error: format '%x' expects argument of type 'unsigned int', but argument 6 has type 'dma_addr_t {aka long long unsigned int}' [-Werror=format=]
dev_dbg(musb->controller, "%p, pkt_sz %d, addr 0x%x, len %d, mode %d\n",
musb/musbhsdma.c: In function 'dma_channel_program':
musb/musbhsdma.c:155:53: error: format '%x' expects argument of type 'unsigned int', but argument 7 has type 'dma_addr_t {aka long long unsigned int}' [-Werror=format=]
dev_dbg(musb->controller, "ep%d-%s pkt_sz %d, dma_addr 0x%x length %d, mode %d\n",
musb/tusb6010_omap.c: In function 'tusb_omap_dma_program':
musb/tusb6010_omap.c:313:53: error: format '%x' expects argument of type 'unsigned int', but argument 7 has type 'dma_addr_t {aka long long unsigned int}' [-Werror=format=]
dev_dbg(musb->controller, "ep%i %s dma ch%i dma: %08x len: %u(%u) packet_sz: %i(%i)\n",
This uses the %pad format string, which prints a dma_addr_t that
gets passed by reference, which works for all combinations.
Acked-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
The power_up function is used for otg or udc mode, but nost when
the driver is only configured for host mode:
drivers/usb/phy/phy-isp1301-omap.c:261:13: error: 'power_up' defined but not used [-Werror=unused-function]
This marks the function __maybe_unused to avoid the warning and
silently drop the definition when it is unused.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
The USB_FSL_MPH_DR_OF symbol is used to ensure the code that interprets
the DR device node is built whenever one of the two drivers (EHCI or
UDC) for the platform is enabled. However, if CONFIG_USB is disabled
and we only support gadget mode, this causes a Kconfig warning:
warning: (USB_FSL_USB2) selects USB_FSL_MPH_DR_OF which has unmet direct dependencies (USB_SUPPORT && USB)
We can avoid this warning by simply no longer using the symbol,
and making sure we enter the drivers/usb/host/ directory when
the UDC driver is enabled that needs the file, and then we use
Makefile syntax to ensure the file is built-in if needed.
There is currently a dependency on CONFIG_OF, but this is redundant,
as we already know that this is set unconditionally for the platforms
that use this driver.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
This converts the pxa25x udc driver to use readl/writel as normal
driver should do, rather than dereferencing __iomem pointers
themselves.
Based on the earlier preparation work, we can now also pass
the register start in the device pointer so we no longer need
the global variable.
The unclear part here is for IXP4xx, which supports both big-endian
and little-endian configurations. So far, the driver has done
no byteswap in either case. I suspect that is wrong and it would
actually need to swap in one or the other case, but I don't know
which. It's also possible that there is some magic setting in
the chip that makes the endianess of the MMIO register match the
CPU, and in that case, the code actually does the right thing
for all configurations, both before and after this patch.
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
This removes the dependency on the mach/hardware.h header file
from the pxa25x_udc driver after the register definitions were
already unified in the previous patch.
Following the model of pxa27x_udc (and basically all other drivers
in the kernel), we define the register numbers as offsets from
the register base address and use accessor functions to read/write
them.
For the moment, this still leaves the direct pointer dereference
in place, instead of using readl/writel, so this patch should
not be changing the behavior of the driver, other than using
ioremap() on the platform resource to replace the hardcoded
virtual address pointers.
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
ixp4xx and pxa25x both use this driver and provide a slightly
different set of register definitions for it. Aside from that,
the definition in the ixp4xx-regs.h header conflicts with the
on in the pxa27x device driver when compile-testing that:
In file included from ../drivers/usb/gadget/udc/pxa27x_udc.c:37:0:
../drivers/usb/gadget/udc/pxa27x_udc.h:26:0: warning: "UDCCR" redefined
#define UDCCR 0x0000 /* UDC Control Register */
^
In file included from ../arch/arm/mach-ixp4xx/include/mach/hardware.h:27:0,
from ../arch/arm/mach-ixp4xx/include/mach/io.h:18,
from ../arch/arm/include/asm/io.h:194,
from ../include/linux/io.h:25,
from ../include/linux/irq.h:24,
from ../drivers/usb/gadget/udc/pxa27x_udc.c:23:
../arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h:415:0: note: this is the location of the previous definition
#define UDCCR IXP4XX_USB_REG(IXP4XX_USB_BASE_VIRT+0x0000)
This addresses both issues by moving all the definitions into the
pxa25x_udc driver itself. It turns out the only difference between
them was 'UDCCS_IO_ROF', and that could well be a mistake when it
was incorrectly copied from pxa25x to ixp4xx.
Acked-by: Krzysztof Halasa <khalasa@piap.pl>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Since Some SoCs (e.g. R-Car Gen2) don't have the CSSTS bit in the
pipectrl registers ({DCP,PIPEn}CTR) because such SoCs have peripheral
mode only. So, this driver should not check the CSSTS bit if peripheral
mode is running.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Since the usb2 phy driver for gen3 (phy-rcar-gen3-usb2) cannot access
LPSTS and UGCTRL2 registers in the HSUSB module, this driver have to
initialize the registers. So, this patch adds such handling code into
rcar3.c.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Enable SuperSpeedPlus by programming the DCFG.speed and after
enumerating, set gadget->speed appropriately.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
If the maximum_speed is not set, set it to a known value, either
SuperSpeed or SuperSpeedPlus based on the type of controller we are
using. If we are on DWC_usb31 controller, check the PHY interface to see
if it is capable of SuperSpeedPlus.
Also this check is moved after dwc3_core_init() so that we can check
dwc->revision.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
Update various places where the speed is checked so that it takes into
account SuperSpeedPlus properly.
Signed-off-by: John Youn <johnyoun@synopsys.com>
Signed-off-by: Felipe Balbi <balbi@kernel.org>
