For OUT endpoints, zero-length transfers require MaxPacketSize buffer as
per the DWC_usb3 programming guide 3.30a section 4.2.3.3.
This patch fixes this by explicitly checking zero length
transfer to correctly pad up to MaxPacketSize.
Fixes: c6267a5163 ("usb: dwc3: gadget: align transfers to wMaxPacketSize")
Cc: stable@vger.kernel.org
Signed-off-by: Tejas Joglekar <joglekar@synopsys.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Currently the link_state is uninitialized and the default value is 0(U0)
before the first time we start the udc, and after we start the udc then
stop the udc, the link_state will be undefined.
We may have the following warnings if we start the udc again with
an undefined link_state:
WARNING: CPU: 0 PID: 327 at drivers/usb/dwc3/gadget.c:294 dwc3_send_gadget_ep_cmd+0x304/0x308
dwc3 100e0000.hidwc3_0: wakeup failed --> -22
[...]
Call Trace:
[<c010f270>] (unwind_backtrace) from [<c010b3d8>] (show_stack+0x10/0x14)
[<c010b3d8>] (show_stack) from [<c034a4dc>] (dump_stack+0x84/0x98)
[<c034a4dc>] (dump_stack) from [<c0118000>] (__warn+0xe8/0x100)
[<c0118000>] (__warn) from [<c0118050>](warn_slowpath_fmt+0x38/0x48)
[<c0118050>] (warn_slowpath_fmt) from [<c0442ec0>](dwc3_send_gadget_ep_cmd+0x304/0x308)
[<c0442ec0>] (dwc3_send_gadget_ep_cmd) from [<c0445e68>](dwc3_ep0_start_trans+0x48/0xf4)
[<c0445e68>] (dwc3_ep0_start_trans) from [<c0446750>](dwc3_ep0_out_start+0x64/0x80)
[<c0446750>] (dwc3_ep0_out_start) from [<c04451c0>](__dwc3_gadget_start+0x1e0/0x278)
[<c04451c0>] (__dwc3_gadget_start) from [<c04452e0>](dwc3_gadget_start+0x88/0x10c)
[<c04452e0>] (dwc3_gadget_start) from [<c045ee54>](udc_bind_to_driver+0x88/0xbc)
[<c045ee54>] (udc_bind_to_driver) from [<c045f29c>](usb_gadget_probe_driver+0xf8/0x140)
[<c045f29c>] (usb_gadget_probe_driver) from [<bf005424>](gadget_dev_desc_UDC_store+0xac/0xc4 [libcomposite])
[<bf005424>] (gadget_dev_desc_UDC_store [libcomposite]) from[<c023d8e0>] (configfs_write_file+0xd4/0x160)
[<c023d8e0>] (configfs_write_file) from [<c01d51e8>] (__vfs_write+0x1c/0x114)
[<c01d51e8>] (__vfs_write) from [<c01d5ff4>] (vfs_write+0xa4/0x168)
[<c01d5ff4>] (vfs_write) from [<c01d6d40>] (SyS_write+0x3c/0x90)
[<c01d6d40>] (SyS_write) from [<c0107400>] (ret_fast_syscall+0x0/0x3c)
Signed-off-by: Zeng Tao <prime.zeng@hisilicon.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
OUT endpoint requests may somtimes have this flag set when
preparing to be submitted to HW indicating that there is an
additional TRB chained to the request for alignment purposes.
If that request is removed before the controller can execute the
transfer (e.g. ep_dequeue/ep_disable), the request will not go
through the dwc3_gadget_ep_cleanup_completed_request() handler
and will not have its needs_extra_trb flag cleared when
dwc3_gadget_giveback() is called. This same request could be
later requeued for a new transfer that does not require an
extra TRB and if it is successfully completed, the cleanup
and TRB reclamation will incorrectly process the additional TRB
which belongs to the next request, and incorrectly advances the
TRB dequeue pointer, thereby messing up calculation of the next
requeust's actual/remaining count when it completes.
The right thing to do here is to ensure that the flag is cleared
before it is given back to the function driver. A good place
to do that is in dwc3_gadget_del_and_unmap_request().
Fixes: c6267a5163 ("usb: dwc3: gadget: align transfers to wMaxPacketSize")
Cc: stable@vger.kernel.org
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
We see dwc3 endpoint stopped by unwanted irq during
suspend resume test, which is caused dwc3 ep can't be started
with error "No Resource".
Here, add synchronize_irq before suspend to sync the
pending IRQ handlers complete.
Signed-off-by: Bo He <bo.he@intel.com>
Signed-off-by: Yu Wang <yu.y.wang@intel.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
So it looks like folks are interested in dwc3 again. Almost 64% of the
changes are in dwc3 this time around with some other bits in gadget
functions and dwc2.
There are two important parts here: a. removal of the waitqueue from
dwc3's dequeue implementation, which will guarantee that gadget
functions can dequeue from any context and; b. better method for
starting isochronous transfers to avoid, as much as possible, missed
isoc frames.
Apart from these, we have the usual set of non-critical fixes and new
features all over the place.
-----BEGIN PGP SIGNATURE-----
iQJRBAABCAA7FiEElLzh7wn96CXwjh2IzL64meEamQYFAlwQ7sQdHGZlbGlwZS5i
YWxiaUBsaW51eC5pbnRlbC5jb20ACgkQzL64meEamQbioA//fSpx5SV7undE1skG
lgjx7Uv2lqnU1kJxhTtyH54tZVhGMmaz4mDVJ3bRk4xotdh3BGQ7nRuui9PDGZmC
81bSjRBVvHXG0xtQQ1AjNc6vTf5h2MVRAZ4U2dp7pkNOgkoWQSakyH3Tz4Brhpfh
UWHLaVYrCb2R68b1pmQbJ6ckSkOu9Wt1yj3SJxvSOkRdbSgb9+khTNZJyyWuGort
zw6pcK33AuLwfOuz7qk8Wihqwi6BOgSrCGU8UfspBRfGhZrw3DSTnS3wYZjdR0QF
2/jwoBwt59i2hpFsMHmMYGU+307oVQ6Y9b/PXpnj4KqIG2CROB8eirDFkToxe4hC
Udy06IA70HUTBKzFmK1gCKZCZW3t2sB+zHtoVHJ8EARlFHWdLxLp83hA9p2Llfon
41iKnkf9CTiO2koEEzuOUOQW+KctIawryiUi4m+C5ZhJBKNk4GvIpXAg1JC9QX9E
GyqEICkg1dvc3GG5HYjPU7xq8mEdcHVOrHoiUci4zo9hMh6nHt7dboNeXQ13LNU9
joEqLvKEhUsm0J7cKWYOpxlHmNWGWNgqRas4Mot9zLUP7yYrxqLT5b2hD3PcjPl4
Jl+rbm4/2YMztWAnCU85EEPwVgJx9k7uyAEtXtQaWUValsVsWUWI1/F//IjYeLZI
AUx1euoarbCp6mXIoHLsOHV02uc=
=n8VS
-----END PGP SIGNATURE-----
Merge tag 'usb-for-v4.21' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-next
Felipe writes:
USB changes for v4.21
So it looks like folks are interested in dwc3 again. Almost 64% of the
changes are in dwc3 this time around with some other bits in gadget
functions and dwc2.
There are two important parts here: a. removal of the waitqueue from
dwc3's dequeue implementation, which will guarantee that gadget
functions can dequeue from any context and; b. better method for
starting isochronous transfers to avoid, as much as possible, missed
isoc frames.
Apart from these, we have the usual set of non-critical fixes and new
features all over the place.
* tag 'usb-for-v4.21' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb: (56 commits)
usb: dwc2: Fix disable all EP's on disconnect
usb: dwc3: gadget: Disable CSP for stream OUT ep
usb: dwc2: disable power_down on Amlogic devices
Revert "usb: dwc3: pci: Use devm functions to get the phy GPIOs"
USB: gadget: udc: s3c2410_udc: convert to DEFINE_SHOW_ATTRIBUTE
usb: mtu3: fix dbginfo in qmu_tx_zlp_error_handler
usb: dwc3: trace: add missing break statement to make compiler happy
usb: dwc3: gadget: Report isoc transfer frame number
usb: gadget: Introduce frame_number to usb_request
usb: renesas_usbhs: Use SIMPLE_DEV_PM_OPS macro
usb: renesas_usbhs: Remove dummy runtime PM callbacks
usb: dwc2: host: use hrtimer for NAK retries
usb: mtu3: clear SOFTCONN when clear USB3_EN if work as HS mode
usb: mtu3: enable SETUPENDISR interrupt
usb: mtu3: fix the issue about SetFeature(U1/U2_Enable)
usb: mtu3: enable hardware remote wakeup from L1 automatically
usb: mtu3: remove QMU checksum
usb/mtu3: power down device ip at setup
usb: dwc2: Disable power down feature on Samsung SoCs
usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb()
...
In stream mode, when fast-forwarding TRBs, the stream number
is not cleared causing the new stream to not get assigned. So
we don't want controller to carry on transfers when short packet
is received. So disable the CSP for stream capable endpoint.
This is based on the 3.30a Programming guide, where table 3-1
device descriptor structure field definitions says for CSP bit
If this bit is 0, the controller generates an XferComplete event
and remove the stream. So if we keep CSP as 1 then switching between
streams would not happen as in stream mode, when fast-forwarding
TRBs, the stream number is not cleared causing the new stream to not get
assigned.
Signed-off-by: Tejas Joglekar <joglekar@synopsys.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Implement the new frame_number API to report the isochronous interval
frame number. This patch checks and reports the interval in which the
isoc transfer was transmitted or received via the Isoc-First TRB SOF
number field.
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Availability of TRB's is calculated using dwc3_calc_trbs_left(), which
determines total available TRB's based on the HWO bit set in a TRB.
In the present code, __dwc3_prepare_one_trb() is called with a TRB which
needs to be prepared for transfer. This __dwc3_prepare_one_trb() calls
dwc3_calc_trbs_left() to determine total available TRBs and set IOC bit
if the total available TRBs are zero. Since the present working TRB (which
is passed as an argument to __dwc3_prepare_one_trb() ) doesn't yet have
the HWO bit set before calling dwc3_calc_trbs_left(), there are chances
that dwc3_calc_trbs_left() wrongly calculates this present working TRB
as free(since the HWO bit is not yet set) and returns the total available
TRBs as greater than zero (including the present working TRB). This could
be a problem.
This patch corrects the above mentioned problem in __dwc3_prepare_one_trb()
by increementing the dep->trb_enqueue at the last (after preparing the TRB)
instead of increementing at the start and setting the IOC bit only if the
total available TRBs returned by dwc3_calc_trbs_left() is 1 . Since we are
increementing the dep->trb_enqueue at the last, the present working TRB is
also considered as available by dwc3_calc_trbs_left() and non zero value is
returned . So, according to the modified logic, when the total available
TRBs is equal to 1 that means the total available TRBs in the pool are 0.
Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
Reviewed-by: Thinh Nguyen <thinhn@synopsys.com>
Tested-by: Tejas Joglekar <tejas.joglekar@synopsys.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
The stream capable endpoints require stream id to be given
when issuing START TRANSFER. While issuing no-op trb the
stream id is not yet known, so don't issue no-op trb's on
stream capable endpoints.
Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
For stream capable endpoints, stream id related information
needs to be updated into DEPCMD while issuing START TRANSFER.
This patch does the same.
Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Gadget driver may take an unbounded amount of time to queue requests
after XferNotReady. This is important for isochronous endpoints which
need to be started for a specific (micro-)frame.
If we fail to start a transfer for isochronous endpoint, let's try
queueing to a future interval and see if that helps. We will stop trying
if we fail a start transfer for 5 intervals in the future.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Running out of requests on isochronous endpoints is part of normal
operation. We don't really need to know about it every time it
happens.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Sometimes, errors happen when kicking transfers from
__dwc3_gadget_start_isoc(). In those cases, we need to pass along the
error so gadget driver can make informed decisions.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Now that we have a list of cancelled requests, we can skip over TRBs
when END_TRANSFER command completes.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Whenever we have a request in flight, we can move it to the cancelled
list and later simply iterate over that list and skip over any TRBs we
find.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Extract the logic for skipping over TRBs to its own function. This
makes the code slightly more readable and makes it easier to move this
call to its final resting place as a following patch.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Now that we track how many TRBs a request uses, it's easier to skip
over them in case of a call to usb_ep_dequeue(). Let's do so and
simplify the code a bit.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Both flags are used for the same purpose in dwc3: appending an extra
TRB at the end to deal with controller requirements. By combining both
flags into one, we make it clear that the situation is the same and
that they should be treated equally.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
In DWC_usb31 version 1.70a-ea06 and prior, for highspeed and fullspeed
isochronous IN, BIT[15:14] of the 16-bit microframe number reported by
the XferNotReady event are invalid. The driver uses this number to
schedule the isochronous transfer and passes it to the START TRANSFER
command. Because this number is invalid, the command may fail. If
BIT[15:14] matches the internal 16-bit microframe, the START TRANSFER
command will pass and the transfer will start at the scheduled time, if
it is off by 1, the command will still pass, but the transfer will start
2 seconds in the future. For all other conditions, the START TRANSFER
command will fail with bus-expiry.
In order to workaround this issue, we can test for the correct
combination of BIT[15:14] by sending START TRANSFER commands with
different values of BIT[15:14]: 'b00, 'b01, 'b10, and 'b11. Each
combination is 2^14 uframe apart (or 2 seconds). 4 seconds into the
future will result in a bus-expiry status. As the result, within the 4
possible combinations for BIT[15:14], there will be 2 successful and 2
failure START COMMAND status. One of the 2 successful command status
will result in a 2-second delay start. The smaller BIT[15:14] value is
the correct combination.
Since there are only 4 outcomes and the results are ordered, we can
simply test 2 START TRANSFER commands with BIT[15:14] combinations 'b00
and 'b01 to deduce the smaller successful combination.
Let test0 = test status for combination 'b00 and test1 = test status for
'b01 of BIT[15:14]. The correct combination is as follow:
if test0 fails and test1 passes, BIT[15:14] is 'b01
if test0 fails and test1 fails, BIT[15:14] is 'b10
if test0 passes and test1 fails, BIT[15:14] is 'b11
if test0 passes and test1 passes, BIT[15:14] is 'b00
Synopsys STAR 9001202023: Wrong microframe number for isochronous IN
endpoints.
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This reverts commit ffb80fc672.
Turns out that commit is wrong. Host controllers are allowed to use
Clear Feature HALT as means to sync data toggle between host and
periperal.
Cc: <stable@vger.kernel.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
When chaining ISOC TRBs together, only the first ISOC TRB should be of
type ISOC_FIRST, all others should be of type ISOC. This patch fixes
that.
Fixes: c6267a5163 ("usb: dwc3: gadget: align transfers to wMaxPacketSize")
Cc: <stable@vger.kernel.org> # v4.11+
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Current check for the last extra TRB for zero and unaligned transfers
does not account for isoc OUT. The last TRB of the Buffer Descriptor for
isoc OUT transfers will be retired with HWO=0. As a result, we won't
return early. The req->remaining will be updated to include the BUFSIZ
count of the extra TRB, and the actual number of transferred bytes
calculation will be wrong.
To fix this, check whether it's a short or zero packet and the last TRB
chain bit to return early.
Fixes: c6267a5163 ("usb: dwc3: gadget: align transfers to wMaxPacketSize")
Cc: <stable@vger.kernel.org>
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
When operating in USB 2.0 speeds (HS/FS), if GUSB2PHYCFG.ENBLSLPM or
GUSB2PHYCFG.SUSPHY is set, it must be cleared before issuing an endpoint
command.
Current implementation only save and restore GUSB2PHYCFG.SUSPHY
configuration. We must save and clear both GUSB2PHYCFG.ENBLSLPM and
GUSB2PHYCFG.SUSPHY settings. Restore them after the command is
completed.
DWC_usb3 3.30a and DWC_usb31 1.90a programming guide section 3.2.2
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
endpoint->maxpacket is not updated after setting the
usb_set_maxpacket_limit() on endpoint enable. The MaxPacketSize can be
different than the endpoint->maxpacket_limit.
DWC3 has been consistently using MaxPacketSize from the endpoint's
descriptor, so let's keep it consistent and use the MaxPacketSize from
the endpoint's descriptor instead.
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
dwc3_ep_dequeue() waits for completion of End Transfer command using
wait_event_lock_irq(), which will release the dwc3->lock while waiting
and reacquire after completion. This allows a potential race condition
with ep_disable() which also removes all requests from started_list
and pending_list.
The check for NULL r->trb should catch this but currently it exits to
the wrong 'out1' label which calls dwc3_gadget_giveback(). Since its
list entry was already removed, if CONFIG_DEBUG_LIST is enabled a
'list_del corruption' bug is thrown since its next/prev pointers are
already LIST_POISON1/2. If r->trb is NULL it should simply exit to
'out0'.
Fixes: cf3113d893 ("usb: dwc3: gadget: properly increment dequeue pointer on ep_dequeue")
Cc: stable@vger.kernel.org # v4.12+
Signed-off-by: Mayank Rana <mrana@codeaurora.org>
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Fixes the following sparse warning:
drivers/usb/dwc3/gadget.c:169:6: warning:
symbol 'dwc3_gadget_del_and_unmap_request' was not declared. Should it be static?
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
We dont' need to touch req->direction or req->epnum from
ep_queue(). It's enough that we initialize both fields from
alloc_request() and just keep them for the entire lifetime of the
request.
No functional changes.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Instead of *always* calling dwc3_gadget_ep_get_transfer_index() after
sending a Start Transfer command, we can call it once from
dwc3_send_gadget_ep_cmd() itself.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Instead of returning resource index number just to assign it to a
field inside 'dep' which was passed as argument, we can assing
dep->resource_index from inside dwc3_gadget_ep_get_transfer_index()
itself.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
In case we have many started requests and one of them in the middle is
completed with Missed Isoc, let's not End Transfer as that would
result in us loosing (possibly) many more intervals.
Instead, let's allow the controller to go through its list of started
requests.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Avoid a prototype when the function can be defined earlier. No
functional changes, cleanup only.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Instead of having a prototype for a function that's defined a few
lines down, let's just move definition to the place where prototype
was.
No functional changes, cleanup only.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
In a few places, the argument is completely unnecessary. On places
where it's needed, we can get it from dep->dwc.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Those two arguments refer to a single bitfield in the register. In
order to simplify the code, we can combine them into a single argument
and expect caller to pass the correct action argument at all times.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
We will only have event status of IOC when IOC bit is set in
TRB. There's no need to check both bits.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
We know that only OUT endpoints can trigger SHORT. We also know that
count MUST be > 0 whenever SHORT triggers.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
We are trying to kick transfers on Isochronous endpoints in a more
controlled manner now. And this ended up rendering this piece of code
unnecessary.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
instead of having one big loop, let's split it down into two smaller
handlers: one for linear buffers and one for scatterlist.
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Tejas Joglekar