Hello uvc gadget developers,
V4 series updated with comments to ("usb: gadget: uvc: add configfs
option for sg support") and extra stable cherry-picks dropped in ("usb:
gadget: uvc: fix dropped frame after missed isoc") since a request was
made to stable separately for those.
V3 series updated with fixes for the two issues discussed below, plus
fixes for the configfs interrupt patch.
V2 series with added patches to disable these performance features at
the userspace level for devices that don't work well with the UDC hw,
i.e. dwc3 in this case. Also included are updates to comments for the v1
patch.
Original note:
I'm working on a 5.15.41 based kernel on a qcom chipset with the dwc3
controller and I'm encountering two problems related to the recent performance
improvement changes:
https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/ and
https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/
If I revert these two changes, then I have much improved stability and a
transmission problem I'm seeing is gone. Has there been any success from
others on 5.15 with this uvc improvement and any recommendations for my
current problems? Those being:
1) a smmu panic, snippet here:
<3>[ 718.314900][ T803] arm-smmu 15000000.apps-smmu: Unhandled arm-smmu context fault from a600000.dwc3!
<3>[ 718.314994][ T803] arm-smmu 15000000.apps-smmu: FAR = 0x00000000efe60800
<3>[ 718.315023][ T803] arm-smmu 15000000.apps-smmu: PAR = 0x0000000000000000
<3>[ 718.315048][ T803] arm-smmu 15000000.apps-smmu: FSR = 0x40000402 [TF R SS ]
<3>[ 718.315074][ T803] arm-smmu 15000000.apps-smmu: FSYNR0 = 0x5f0003
<3>[ 718.315096][ T803] arm-smmu 15000000.apps-smmu: FSYNR1 = 0xaa02
<3>[ 718.315117][ T803] arm-smmu 15000000.apps-smmu: context bank# = 0x1b
<3>[ 718.315141][ T803] arm-smmu 15000000.apps-smmu: TTBR0 = 0x001b0000c2a92000
<3>[ 718.315165][ T803] arm-smmu 15000000.apps-smmu: TTBR1 = 0x001b000000000000
<3>[ 718.315192][ T803] arm-smmu 15000000.apps-smmu: SCTLR = 0x0a5f00e7 ACTLR = 0x00000003
<3>[ 718.315245][ T803] arm-smmu 15000000.apps-smmu: CBAR = 0x0001f300
<3>[ 718.315274][ T803] arm-smmu 15000000.apps-smmu: MAIR0 = 0xf404ff44 MAIR1 = 0x0000efe4
<3>[ 718.315297][ T803] arm-smmu 15000000.apps-smmu: SID = 0x40
<3>[ 718.315318][ T803] arm-smmu 15000000.apps-smmu: Client info: BID=0x5, PID=0xa, MID=0x2
<3>[ 718.315377][ T803] arm-smmu 15000000.apps-smmu: soft iova-to-phys=0x0000000000000000
I can reduce this panic with the proposed patch, but it still happens until I
disable the "req->no_interrupt = 1" logic.
2) The frame is not fully transmitted in dwc3 with sg support enabled.
There seems to be a mapping limit I'm seeing where only the roughly first
70% of the total frame is sent. Interestingly, if I allocate a larger
size for the buffer upfront, in uvc_queue_setup(), like sizes[0] =
video->imagesize * 3. Then the issue rarely happens. For example, when I
do YUYV I see green, uninitialized data, at the bottom part of the
frame. If I do MJPG with smaller filled sizes, the transmission is fine.
+-------------------------+
| |
| |
| |
| Good data |
| |
| |
| |
+-------------------------+
|xxxxxxxxxxxxxxxxxxxxxxxxx|
|xxxx Bad data xxxxxxxxx|
|xxxxxxxxxxxxxxxxxxxxxxxxx|
+-------------------------+
Dan Vacura (4):
usb: gadget: uvc: fix dropped frame after missed isoc
usb: gadget: uvc: fix sg handling in error case
usb: gadget: uvc: make interrupt skip logic configurable
usb: gadget: uvc: add configfs option for sg support
Jeff Vanhoof (2):
usb: dwc3: gadget: cancel requests instead of release after missed
isoc
usb: gadget: uvc: fix sg handling during video encode
.../ABI/testing/configfs-usb-gadget-uvc | 2 +
Documentation/usb/gadget-testing.rst | 6 +++
drivers/usb/dwc3/core.h | 1 +
drivers/usb/dwc3/gadget.c | 38 +++++++++++++------
drivers/usb/gadget/function/f_uvc.c | 4 ++
drivers/usb/gadget/function/u_uvc.h | 2 +
drivers/usb/gadget/function/uvc.h | 2 +
drivers/usb/gadget/function/uvc_configfs.c | 4 ++
drivers/usb/gadget/function/uvc_queue.c | 18 +++++++--
drivers/usb/gadget/function/uvc_video.c | 28 ++++++++++----
10 files changed, 81 insertions(+), 24 deletions(-)
--
2.34.1
With the re-use of the previous completion status in 0d1c407b1a749
("usb: dwc3: gadget: Return proper request status") it could be possible
that the next frame would also get dropped if the current frame has a
missed isoc error. Ensure that an interrupt is requested for the start
of a new frame.
Fixes: fc78941d8169 ("usb: gadget: uvc: decrease the interrupt load to a quarter")
Cc: <[email protected]>
Signed-off-by: Dan Vacura <[email protected]>
---
V1 -> V3:
- no change, new patch in series
V3 -> V4:
- no change
drivers/usb/gadget/function/uvc_video.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index bb037fcc90e6..323977716f5a 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -431,7 +431,8 @@ static void uvcg_video_pump(struct work_struct *work)
/* Endpoint now owns the request */
req = NULL;
- video->req_int_count++;
+ if (buf->state != UVC_BUF_STATE_DONE)
+ video->req_int_count++;
}
if (!req)
--
2.34.1
If there is a transmission error the buffer will be returned too early,
causing a memory fault as subsequent requests for that buffer are still
queued up to be sent. Refactor the error handling to wait for the final
request to come in before reporting back the buffer to userspace for all
transfer types (bulk/isoc/isoc_sg). This ensures userspace knows if the
frame was successfully sent.
Fixes: e81e7f9a0eb9 ("usb: gadget: uvc: add scatter gather support")
Cc: <[email protected]>
Signed-off-by: Dan Vacura <[email protected]>
---
V1 -> V2:
- undo error rename
- change uvcg_info to uvcg_dbg
V2 -> V3:
- no changes
V3 -> V4:
- drop extra cc stable cherry-picks, as a request was made to stable
drivers/usb/gadget/function/uvc_queue.c | 8 +++++---
drivers/usb/gadget/function/uvc_video.c | 18 ++++++++++++++----
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index ec500ee499ee..0aa3d7e1f3cc 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -304,6 +304,7 @@ int uvcg_queue_enable(struct uvc_video_queue *queue, int enable)
queue->sequence = 0;
queue->buf_used = 0;
+ queue->flags &= ~UVC_QUEUE_DROP_INCOMPLETE;
} else {
ret = vb2_streamoff(&queue->queue, queue->queue.type);
if (ret < 0)
@@ -329,10 +330,11 @@ int uvcg_queue_enable(struct uvc_video_queue *queue, int enable)
void uvcg_complete_buffer(struct uvc_video_queue *queue,
struct uvc_buffer *buf)
{
- if ((queue->flags & UVC_QUEUE_DROP_INCOMPLETE) &&
- buf->length != buf->bytesused) {
- buf->state = UVC_BUF_STATE_QUEUED;
+ if (queue->flags & UVC_QUEUE_DROP_INCOMPLETE) {
+ queue->flags &= ~UVC_QUEUE_DROP_INCOMPLETE;
+ buf->state = UVC_BUF_STATE_ERROR;
vb2_set_plane_payload(&buf->buf.vb2_buf, 0, 0);
+ vb2_buffer_done(&buf->buf.vb2_buf, VB2_BUF_STATE_ERROR);
return;
}
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index 323977716f5a..5993e083819c 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -88,6 +88,7 @@ uvc_video_encode_bulk(struct usb_request *req, struct uvc_video *video,
struct uvc_buffer *buf)
{
void *mem = req->buf;
+ struct uvc_request *ureq = req->context;
int len = video->req_size;
int ret;
@@ -113,13 +114,14 @@ uvc_video_encode_bulk(struct usb_request *req, struct uvc_video *video,
video->queue.buf_used = 0;
buf->state = UVC_BUF_STATE_DONE;
list_del(&buf->queue);
- uvcg_complete_buffer(&video->queue, buf);
video->fid ^= UVC_STREAM_FID;
+ ureq->last_buf = buf;
video->payload_size = 0;
}
if (video->payload_size == video->max_payload_size ||
+ video->queue.flags & UVC_QUEUE_DROP_INCOMPLETE ||
buf->bytesused == video->queue.buf_used)
video->payload_size = 0;
}
@@ -180,7 +182,8 @@ uvc_video_encode_isoc_sg(struct usb_request *req, struct uvc_video *video,
req->length -= len;
video->queue.buf_used += req->length - header_len;
- if (buf->bytesused == video->queue.buf_used || !buf->sg) {
+ if (buf->bytesused == video->queue.buf_used || !buf->sg ||
+ video->queue.flags & UVC_QUEUE_DROP_INCOMPLETE) {
video->queue.buf_used = 0;
buf->state = UVC_BUF_STATE_DONE;
buf->offset = 0;
@@ -195,6 +198,7 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video,
struct uvc_buffer *buf)
{
void *mem = req->buf;
+ struct uvc_request *ureq = req->context;
int len = video->req_size;
int ret;
@@ -209,12 +213,13 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video,
req->length = video->req_size - len;
- if (buf->bytesused == video->queue.buf_used) {
+ if (buf->bytesused == video->queue.buf_used ||
+ video->queue.flags & UVC_QUEUE_DROP_INCOMPLETE) {
video->queue.buf_used = 0;
buf->state = UVC_BUF_STATE_DONE;
list_del(&buf->queue);
- uvcg_complete_buffer(&video->queue, buf);
video->fid ^= UVC_STREAM_FID;
+ ureq->last_buf = buf;
}
}
@@ -255,6 +260,11 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
case 0:
break;
+ case -EXDEV:
+ uvcg_dbg(&video->uvc->func, "VS request missed xfer.\n");
+ queue->flags |= UVC_QUEUE_DROP_INCOMPLETE;
+ break;
+
case -ESHUTDOWN: /* disconnect from host. */
uvcg_dbg(&video->uvc->func, "VS request cancelled.\n");
uvcg_queue_cancel(queue, 1);
--
2.34.1
Some UDC hw may not support skipping interrupts, but still support the
request. Allow the interrupt frequency to be configurable to the user.
Signed-off-by: Dan Vacura <[email protected]>
---
V1 -> V2:
- no change, new patch in series
V2 -> V3:
- default to baseline value of 4, fix storing the initial value
V3 -> V4:
- no change
Documentation/ABI/testing/configfs-usb-gadget-uvc | 1 +
Documentation/usb/gadget-testing.rst | 2 ++
drivers/usb/gadget/function/f_uvc.c | 3 +++
drivers/usb/gadget/function/u_uvc.h | 1 +
drivers/usb/gadget/function/uvc.h | 2 ++
drivers/usb/gadget/function/uvc_configfs.c | 2 ++
drivers/usb/gadget/function/uvc_queue.c | 6 ++++++
drivers/usb/gadget/function/uvc_video.c | 3 ++-
8 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 611b23e6488d..5dfaa3f7f6a4 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -8,6 +8,7 @@ Description: UVC function directory
streaming_maxpacket 1..1023 (fs), 1..3072 (hs/ss)
streaming_interval 1..16
function_name string [32]
+ req_int_skip_div unsigned int
=================== =============================
What: /config/usb-gadget/gadget/functions/uvc.name/control
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index 2278c9ffb74a..f9b5a09be1f4 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -794,6 +794,8 @@ The uvc function provides these attributes in its function directory:
sending or receiving when this configuration is
selected
function_name name of the interface
+ req_int_skip_div divisor of total requests to aid in calculating
+ interrupt frequency, 0 indicates all interrupt
=================== ================================================
There are also "control" and "streaming" subdirectories, each of which contain
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 6e196e06181e..e40ca26b9c55 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -655,6 +655,8 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
cpu_to_le16(max_packet_size * max_packet_mult *
(opts->streaming_maxburst + 1));
+ uvc->config_skip_int_div = opts->req_int_skip_div;
+
/* Allocate endpoints. */
ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
if (!ep) {
@@ -872,6 +874,7 @@ static struct usb_function_instance *uvc_alloc_inst(void)
opts->streaming_interval = 1;
opts->streaming_maxpacket = 1024;
+ opts->req_int_skip_div = 4;
snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera");
ret = uvcg_attach_configfs(opts);
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 24b8681b0d6f..6f73bd5638ed 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -24,6 +24,7 @@ struct f_uvc_opts {
unsigned int streaming_interval;
unsigned int streaming_maxpacket;
unsigned int streaming_maxburst;
+ unsigned int req_int_skip_div;
unsigned int control_interface;
unsigned int streaming_interface;
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 40226b1f7e14..29f9477c92cc 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -107,6 +107,7 @@ struct uvc_video {
spinlock_t req_lock;
unsigned int req_int_count;
+ unsigned int req_int_skip_div;
void (*encode) (struct usb_request *req, struct uvc_video *video,
struct uvc_buffer *buf);
@@ -155,6 +156,7 @@ struct uvc_device {
/* Events */
unsigned int event_length;
unsigned int event_setup_out : 1;
+ unsigned int config_skip_int_div;
};
static inline struct uvc_device *to_uvc(struct usb_function *f)
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 4303a3283ba0..419e926ab57e 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2350,6 +2350,7 @@ UVC_ATTR(f_uvc_opts_, cname, cname)
UVCG_OPTS_ATTR(streaming_interval, streaming_interval, 16);
UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 3072);
UVCG_OPTS_ATTR(streaming_maxburst, streaming_maxburst, 15);
+UVCG_OPTS_ATTR(req_int_skip_div, req_int_skip_div, UINT_MAX);
#undef UVCG_OPTS_ATTR
@@ -2399,6 +2400,7 @@ static struct configfs_attribute *uvc_attrs[] = {
&f_uvc_opts_attr_streaming_interval,
&f_uvc_opts_attr_streaming_maxpacket,
&f_uvc_opts_attr_streaming_maxburst,
+ &f_uvc_opts_attr_req_int_skip_div,
&f_uvc_opts_string_attr_function_name,
NULL,
};
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index 0aa3d7e1f3cc..02559906a55a 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -63,6 +63,12 @@ static int uvc_queue_setup(struct vb2_queue *vq,
*/
nreq = DIV_ROUND_UP(DIV_ROUND_UP(sizes[0], 2), req_size);
nreq = clamp(nreq, 4U, 64U);
+ if (0 == video->uvc->config_skip_int_div) {
+ video->req_int_skip_div = nreq;
+ } else {
+ video->req_int_skip_div = min_t(unsigned int, nreq,
+ video->uvc->config_skip_int_div);
+ }
video->uvc_num_requests = nreq;
return 0;
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index dd1c6b2ca7c6..c1a80c5d1f63 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -423,7 +423,8 @@ static void uvcg_video_pump(struct work_struct *work)
if (list_empty(&video->req_free) ||
buf->state == UVC_BUF_STATE_DONE ||
!(video->req_int_count %
- DIV_ROUND_UP(video->uvc_num_requests, 4))) {
+ DIV_ROUND_UP(video->uvc_num_requests,
+ video->req_int_skip_div))) {
video->req_int_count = 0;
req->no_interrupt = 0;
} else {
--
2.34.1
The scatter gather support doesn't appear to work well with some UDC hw.
Add the ability to turn off the feature depending on the controller in
use or other platform quirks. The default is for the uvc gadget to
support sg as long as the UDC hw supports it.
The specific failure was with the dwc3 controller, but fixes and
improvements are pending for those failures. This capability is now
more intended for future unexpected failures or poor sg support on a
given platform.
Signed-off-by: Dan Vacura <[email protected]>
---
V1 -> V2:
- no change, new patch in series
V2 -> V3:
- default on, same as baseline
V3 -> V4:
- update comment and documentation, refactor use of opts->sg_supported
directly in uvc_queue
Documentation/ABI/testing/configfs-usb-gadget-uvc | 1 +
Documentation/usb/gadget-testing.rst | 4 ++++
drivers/usb/gadget/function/f_uvc.c | 1 +
drivers/usb/gadget/function/u_uvc.h | 1 +
drivers/usb/gadget/function/uvc_configfs.c | 2 ++
drivers/usb/gadget/function/uvc_queue.c | 4 +++-
6 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 5dfaa3f7f6a4..839a75fc28ee 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -9,6 +9,7 @@ Description: UVC function directory
streaming_interval 1..16
function_name string [32]
req_int_skip_div unsigned int
+ sg_supported 0..1
=================== =============================
What: /config/usb-gadget/gadget/functions/uvc.name/control
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index f9b5a09be1f4..be72577a03e9 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -796,6 +796,10 @@ The uvc function provides these attributes in its function directory:
function_name name of the interface
req_int_skip_div divisor of total requests to aid in calculating
interrupt frequency, 0 indicates all interrupt
+ sg_supported allow for scatter gather to be used if the UDC
+ hw supports it, this is default on and only
+ intended to be temporally turned off if a given
+ platform doesn't work well with scatter gather
=================== ================================================
There are also "control" and "streaming" subdirectories, each of which contain
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index e40ca26b9c55..83d96e81c05f 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -875,6 +875,7 @@ static struct usb_function_instance *uvc_alloc_inst(void)
opts->streaming_interval = 1;
opts->streaming_maxpacket = 1024;
opts->req_int_skip_div = 4;
+ opts->sg_supported = 1;
snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera");
ret = uvcg_attach_configfs(opts);
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 6f73bd5638ed..5ccced629925 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -25,6 +25,7 @@ struct f_uvc_opts {
unsigned int streaming_maxpacket;
unsigned int streaming_maxburst;
unsigned int req_int_skip_div;
+ unsigned int sg_supported;
unsigned int control_interface;
unsigned int streaming_interface;
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 419e926ab57e..3784c0e02d01 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2351,6 +2351,7 @@ UVCG_OPTS_ATTR(streaming_interval, streaming_interval, 16);
UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 3072);
UVCG_OPTS_ATTR(streaming_maxburst, streaming_maxburst, 15);
UVCG_OPTS_ATTR(req_int_skip_div, req_int_skip_div, UINT_MAX);
+UVCG_OPTS_ATTR(sg_supported, sg_supported, 1);
#undef UVCG_OPTS_ATTR
@@ -2401,6 +2402,7 @@ static struct configfs_attribute *uvc_attrs[] = {
&f_uvc_opts_attr_streaming_maxpacket,
&f_uvc_opts_attr_streaming_maxburst,
&f_uvc_opts_attr_req_int_skip_div,
+ &f_uvc_opts_attr_sg_supported,
&f_uvc_opts_string_attr_function_name,
NULL,
};
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index 02559906a55a..aee405663d6e 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -21,6 +21,7 @@
#include <media/videobuf2-vmalloc.h>
#include "uvc.h"
+#include "u_uvc.h"
/* ------------------------------------------------------------------------
* Video buffers queue management.
@@ -141,6 +142,7 @@ int uvcg_queue_init(struct uvc_video_queue *queue, struct device *dev, enum v4l2
{
struct uvc_video *video = container_of(queue, struct uvc_video, queue);
struct usb_composite_dev *cdev = video->uvc->func.config->cdev;
+ struct f_uvc_opts *opts = fi_to_f_uvc_opts(video->uvc->func.fi);
int ret;
queue->queue.type = type;
@@ -149,7 +151,7 @@ int uvcg_queue_init(struct uvc_video_queue *queue, struct device *dev, enum v4l2
queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
queue->queue.ops = &uvc_queue_qops;
queue->queue.lock = lock;
- if (cdev->gadget->sg_supported) {
+ if (opts->sg_supported && cdev->gadget->sg_supported) {
queue->queue.mem_ops = &vb2_dma_sg_memops;
queue->use_sg = 1;
} else {
--
2.34.1
From: Jeff Vanhoof <[email protected]>
arm-smmu related crashes seen after a Missed ISOC interrupt when
no_interrupt=1 is used. This can happen if the hardware is still using
the data associated with a TRB after the usb_request's ->complete call
has been made. Instead of immediately releasing a request when a Missed
ISOC interrupt has occurred, this change will add logic to cancel the
request instead where it will eventually be released when the
END_TRANSFER command has completed. This logic is similar to some of the
cleanup done in dwc3_gadget_ep_dequeue.
Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status")
Cc: <[email protected]>
Signed-off-by: Jeff Vanhoof <[email protected]>
Co-developed-by: Dan Vacura <[email protected]>
Signed-off-by: Dan Vacura <[email protected]>
---
V1 -> V3:
- no change, new patch in series
V3 -> V4:
- no change
drivers/usb/dwc3/core.h | 1 +
drivers/usb/dwc3/gadget.c | 38 ++++++++++++++++++++++++++------------
2 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 8f9959ba9fd4..9b005d912241 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -943,6 +943,7 @@ struct dwc3_request {
#define DWC3_REQUEST_STATUS_DEQUEUED 3
#define DWC3_REQUEST_STATUS_STALLED 4
#define DWC3_REQUEST_STATUS_COMPLETED 5
+#define DWC3_REQUEST_STATUS_MISSED_ISOC 6
#define DWC3_REQUEST_STATUS_UNKNOWN -1
u8 epnum;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 079cd333632e..411532c5c378 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2021,6 +2021,9 @@ static void dwc3_gadget_ep_cleanup_cancelled_requests(struct dwc3_ep *dep)
case DWC3_REQUEST_STATUS_STALLED:
dwc3_gadget_giveback(dep, req, -EPIPE);
break;
+ case DWC3_REQUEST_STATUS_MISSED_ISOC:
+ dwc3_gadget_giveback(dep, req, -EXDEV);
+ break;
default:
dev_err(dwc->dev, "request cancelled with wrong reason:%d\n", req->status);
dwc3_gadget_giveback(dep, req, -ECONNRESET);
@@ -3402,21 +3405,32 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
struct dwc3 *dwc = dep->dwc;
bool no_started_trb = true;
- dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
+ if (status == -EXDEV) {
+ struct dwc3_request *tmp;
+ struct dwc3_request *req;
- if (dep->flags & DWC3_EP_END_TRANSFER_PENDING)
- goto out;
+ if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING))
+ dwc3_stop_active_transfer(dep, true, true);
- if (!dep->endpoint.desc)
- return no_started_trb;
+ list_for_each_entry_safe(req, tmp, &dep->started_list, list)
+ dwc3_gadget_move_cancelled_request(req,
+ DWC3_REQUEST_STATUS_MISSED_ISOC);
+ } else {
+ dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
- if (usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
- list_empty(&dep->started_list) &&
- (list_empty(&dep->pending_list) || status == -EXDEV))
- dwc3_stop_active_transfer(dep, true, true);
- else if (dwc3_gadget_ep_should_continue(dep))
- if (__dwc3_gadget_kick_transfer(dep) == 0)
- no_started_trb = false;
+ if (dep->flags & DWC3_EP_END_TRANSFER_PENDING)
+ goto out;
+
+ if (!dep->endpoint.desc)
+ return no_started_trb;
+
+ if (usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
+ list_empty(&dep->started_list) && list_empty(&dep->pending_list))
+ dwc3_stop_active_transfer(dep, true, true);
+ else if (dwc3_gadget_ep_should_continue(dep))
+ if (__dwc3_gadget_kick_transfer(dep) == 0)
+ no_started_trb = false;
+ }
out:
/*
--
2.34.1
From: Jeff Vanhoof <[email protected]>
In uvc_video_encode_isoc_sg, the uvc_request's sg list is
incorrectly being populated leading to corrupt video being
received by the remote end. When building the sg list the
usage of buf->sg's 'dma_length' field is not correct and
instead its 'length' field should be used.
Fixes: e81e7f9a0eb9 ("usb: gadget: uvc: add scatter gather support")
Cc: <[email protected]>
Signed-off-by: Jeff Vanhoof <[email protected]>
Signed-off-by: Dan Vacura <[email protected]>
---
V1 -> V3:
- no change, new patch in series
V3 -> V4:
- no change
drivers/usb/gadget/function/uvc_video.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index 5993e083819c..dd1c6b2ca7c6 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -157,10 +157,10 @@ uvc_video_encode_isoc_sg(struct usb_request *req, struct uvc_video *video,
sg = sg_next(sg);
for_each_sg(sg, iter, ureq->sgt.nents - 1, i) {
- if (!len || !buf->sg || !sg_dma_len(buf->sg))
+ if (!len || !buf->sg || !buf->sg->length)
break;
- sg_left = sg_dma_len(buf->sg) - buf->offset;
+ sg_left = buf->sg->length - buf->offset;
part = min_t(unsigned int, len, sg_left);
sg_set_page(iter, sg_page(buf->sg), part, buf->offset);
--
2.34.1
On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
> From: Jeff Vanhoof <[email protected]>
>
> arm-smmu related crashes seen after a Missed ISOC interrupt when
> no_interrupt=1 is used. This can happen if the hardware is still using
> the data associated with a TRB after the usb_request's ->complete call
> has been made. Instead of immediately releasing a request when a Missed
> ISOC interrupt has occurred, this change will add logic to cancel the
> request instead where it will eventually be released when the
> END_TRANSFER command has completed. This logic is similar to some of the
> cleanup done in dwc3_gadget_ep_dequeue.
>
> Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status")
> Cc: <[email protected]>
> Signed-off-by: Jeff Vanhoof <[email protected]>
> Co-developed-by: Dan Vacura <[email protected]>
> Signed-off-by: Dan Vacura <[email protected]>
> ---
> V1 -> V3:
> - no change, new patch in series
> V3 -> V4:
> - no change
I need an ack from the dwc3 maintainer before I can take this one.
thanks,
greg k-h
On Tue, Oct 18, 2022 at 04:50:42PM -0500, Dan Vacura wrote:
> The scatter gather support doesn't appear to work well with some UDC hw.
> Add the ability to turn off the feature depending on the controller in
> use or other platform quirks. The default is for the uvc gadget to
> support sg as long as the UDC hw supports it.
>
> The specific failure was with the dwc3 controller, but fixes and
> improvements are pending for those failures. This capability is now
> more intended for future unexpected failures or poor sg support on a
> given platform.
>
> Signed-off-by: Dan Vacura <[email protected]>
Again, this should be dynamic. Can't we detect this based on the packet
size and either do sg or not?
If the UDC hardware says it is supported, it should be supported.
Otherwise we need to fix the UDC hardware or it saying it is allowed.
> --- a/Documentation/usb/gadget-testing.rst
> +++ b/Documentation/usb/gadget-testing.rst
> @@ -796,6 +796,10 @@ The uvc function provides these attributes in its function directory:
> function_name name of the interface
> req_int_skip_div divisor of total requests to aid in calculating
> interrupt frequency, 0 indicates all interrupt
> + sg_supported allow for scatter gather to be used if the UDC
> + hw supports it, this is default on and only
> + intended to be temporally turned off if a given
> + platform doesn't work well with scatter gather
How do you know if it "doesn't work well"?
That's vague and not good and nothing we want to support for forever,
sorry.
thanks,
greg k-h
On Tue, Oct 18, 2022 at 04:50:41PM -0500, Dan Vacura wrote:
> Some UDC hw may not support skipping interrupts, but still support the
> request. Allow the interrupt frequency to be configurable to the user.
>
> Signed-off-by: Dan Vacura <[email protected]>
> ---
> V1 -> V2:
> - no change, new patch in series
> V2 -> V3:
> - default to baseline value of 4, fix storing the initial value
> V3 -> V4:
> - no change
>
> Documentation/ABI/testing/configfs-usb-gadget-uvc | 1 +
> Documentation/usb/gadget-testing.rst | 2 ++
Adding more tunable knobs always just adds complexity and fragility as
things do not get tested.
Can't we just make this dynamic and work properly on all systems? What
UDC hardware types do not allow skipping interrupts? Why can't we just
detect that and only do that if this is the case or not?
> drivers/usb/gadget/function/f_uvc.c | 3 +++
> drivers/usb/gadget/function/u_uvc.h | 1 +
> drivers/usb/gadget/function/uvc.h | 2 ++
> drivers/usb/gadget/function/uvc_configfs.c | 2 ++
> drivers/usb/gadget/function/uvc_queue.c | 6 ++++++
> drivers/usb/gadget/function/uvc_video.c | 3 ++-
> 8 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> index 611b23e6488d..5dfaa3f7f6a4 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> @@ -8,6 +8,7 @@ Description: UVC function directory
> streaming_maxpacket 1..1023 (fs), 1..3072 (hs/ss)
> streaming_interval 1..16
> function_name string [32]
> + req_int_skip_div unsigned int
> =================== =============================
>
> What: /config/usb-gadget/gadget/functions/uvc.name/control
> diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
> index 2278c9ffb74a..f9b5a09be1f4 100644
> --- a/Documentation/usb/gadget-testing.rst
> +++ b/Documentation/usb/gadget-testing.rst
> @@ -794,6 +794,8 @@ The uvc function provides these attributes in its function directory:
> sending or receiving when this configuration is
> selected
> function_name name of the interface
> + req_int_skip_div divisor of total requests to aid in calculating
> + interrupt frequency, 0 indicates all interrupt
This provides no information that would help to determine how to
configure this at all. I wouldn't know what to do with this, so that's
a huge sign that this is not a good interface :(
thanks,
greg k-h
Hi Greg,
On Sat, Oct 22, 2022 at 01:31:24PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
> > From: Jeff Vanhoof <[email protected]>
> >
> > arm-smmu related crashes seen after a Missed ISOC interrupt when
> > no_interrupt=1 is used. This can happen if the hardware is still using
> > the data associated with a TRB after the usb_request's ->complete call
> > has been made. Instead of immediately releasing a request when a Missed
> > ISOC interrupt has occurred, this change will add logic to cancel the
> > request instead where it will eventually be released when the
> > END_TRANSFER command has completed. This logic is similar to some of the
> > cleanup done in dwc3_gadget_ep_dequeue.
> >
> > Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status")
> > Cc: <[email protected]>
> > Signed-off-by: Jeff Vanhoof <[email protected]>
> > Co-developed-by: Dan Vacura <[email protected]>
> > Signed-off-by: Dan Vacura <[email protected]>
> > ---
> > V1 -> V3:
> > - no change, new patch in series
> > V3 -> V4:
> > - no change
>
> I need an ack from the dwc3 maintainer before I can take this one.
>
> thanks,
>
> greg k-h
Thinh has rejected this version of the patch. He has provided an alternative
implementation which has been testing well for us so far. Either Thinh or Dan
will formalize this patch within the next few days.
The latest proposed changes are:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index dfaf9ac24c4f..50287437d6de 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3195,6 +3195,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
if (event->status & DEPEVT_STATUS_SHORT && !chain)
return 1;
+ if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC && !chain)
+ return 1;
+
if ((trb->ctrl & DWC3_TRB_CTRL_IOC) ||
(trb->ctrl & DWC3_TRB_CTRL_LST))
return 1;
@@ -3211,6 +3214,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
struct scatterlist *s;
unsigned int num_queued = req->num_queued_sgs;
unsigned int i;
+ bool missed_isoc = false;
int ret = 0;
for_each_sg(sg, s, num_queued, i) {
@@ -3219,12 +3223,18 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
req->sg = sg_next(s);
req->num_queued_sgs--;
+ if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)
+ missed_isoc = true;
+
ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
trb, event, status, true);
if (ret)
break;
}
+ if (missed_isoc)
+ ret = 1;
+
return ret;
}
Thanks,
Jeff
On Sat, Oct 22, 2022, Jeff Vanhoof wrote:
> Hi Greg,
>
> On Sat, Oct 22, 2022 at 01:31:24PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
> > > From: Jeff Vanhoof <[email protected]>
> > >
> > > arm-smmu related crashes seen after a Missed ISOC interrupt when
> > > no_interrupt=1 is used. This can happen if the hardware is still using
> > > the data associated with a TRB after the usb_request's ->complete call
> > > has been made. Instead of immediately releasing a request when a Missed
> > > ISOC interrupt has occurred, this change will add logic to cancel the
> > > request instead where it will eventually be released when the
> > > END_TRANSFER command has completed. This logic is similar to some of the
> > > cleanup done in dwc3_gadget_ep_dequeue.
> > >
> > > Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status")
> > > Cc: <[email protected]>
> > > Signed-off-by: Jeff Vanhoof <[email protected]>
> > > Co-developed-by: Dan Vacura <[email protected]>
> > > Signed-off-by: Dan Vacura <[email protected]>
> > > ---
> > > V1 -> V3:
> > > - no change, new patch in series
> > > V3 -> V4:
> > > - no change
> >
> > I need an ack from the dwc3 maintainer before I can take this one.
> >
> > thanks,
> >
> > greg k-h
>
> Thinh has rejected this version of the patch. He has provided an alternative
> implementation which has been testing well for us so far. Either Thinh or Dan
> will formalize this patch within the next few days.
> The latest proposed changes are:
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index dfaf9ac24c4f..50287437d6de 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3195,6 +3195,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
> if (event->status & DEPEVT_STATUS_SHORT && !chain)
> return 1;
>
> + if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC && !chain)
> + return 1;
> +
> if ((trb->ctrl & DWC3_TRB_CTRL_IOC) ||
> (trb->ctrl & DWC3_TRB_CTRL_LST))
> return 1;
> @@ -3211,6 +3214,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
> struct scatterlist *s;
> unsigned int num_queued = req->num_queued_sgs;
> unsigned int i;
> + bool missed_isoc = false;
> int ret = 0;
>
> for_each_sg(sg, s, num_queued, i) {
> @@ -3219,12 +3223,18 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
> req->sg = sg_next(s);
> req->num_queued_sgs--;
>
> + if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)
> + missed_isoc = true;
> +
> ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
> trb, event, status, true);
> if (ret)
> break;
> }
>
> + if (missed_isoc)
> + ret = 1;
> +
> return ret;
> }
>
>
That's just a debug patch. I'll send out proper fix patches.
Thanks,
Thinh
Hi Thinh,
On Mon, Oct 24, 2022 at 10:47:53PM +0000, Thinh Nguyen wrote:
>On Sat, Oct 22, 2022, Jeff Vanhoof wrote:
>> Hi Greg,
>>
>> On Sat, Oct 22, 2022 at 01:31:24PM +0200, Greg Kroah-Hartman wrote:
>> > On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
>> > > From: Jeff Vanhoof <[email protected]>
>> > >
>> > > arm-smmu related crashes seen after a Missed ISOC interrupt when
>> > > no_interrupt=1 is used. This can happen if the hardware is still using
>> > > the data associated with a TRB after the usb_request's ->complete call
>> > > has been made. Instead of immediately releasing a request when a Missed
>> > > ISOC interrupt has occurred, this change will add logic to cancel the
>> > > request instead where it will eventually be released when the
>> > > END_TRANSFER command has completed. This logic is similar to some of the
>> > > cleanup done in dwc3_gadget_ep_dequeue.
>> > >
>> > > Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status")
>> > > Cc: <[email protected]>
>> > > Signed-off-by: Jeff Vanhoof <[email protected]>
>> > > Co-developed-by: Dan Vacura <[email protected]>
>> > > Signed-off-by: Dan Vacura <[email protected]>
>> > > ---
>> > > V1 -> V3:
>> > > - no change, new patch in series
>> > > V3 -> V4:
>> > > - no change
>> >
>> > I need an ack from the dwc3 maintainer before I can take this one.
>> >
>> > thanks,
>> >
>> > greg k-h
>>
>> Thinh has rejected this version of the patch. He has provided an alternative
>> implementation which has been testing well for us so far. Either Thinh or Dan
>> will formalize this patch within the next few days.
>> The latest proposed changes are:
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index dfaf9ac24c4f..50287437d6de 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -3195,6 +3195,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>> if (event->status & DEPEVT_STATUS_SHORT && !chain)
>> return 1;
>>
>> + if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC && !chain)
>> + return 1;
>> +
>> if ((trb->ctrl & DWC3_TRB_CTRL_IOC) ||
>> (trb->ctrl & DWC3_TRB_CTRL_LST))
>> return 1;
>> @@ -3211,6 +3214,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
>> struct scatterlist *s;
>> unsigned int num_queued = req->num_queued_sgs;
>> unsigned int i;
>> + bool missed_isoc = false;
>> int ret = 0;
>>
>> for_each_sg(sg, s, num_queued, i) {
>> @@ -3219,12 +3223,18 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
>> req->sg = sg_next(s);
>> req->num_queued_sgs--;
>>
>> + if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)
>> + missed_isoc = true;
>> +
>> ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
>> trb, event, status, true);
>> if (ret)
>> break;
>> }
>>
>> + if (missed_isoc)
>> + ret = 1;
>> +
>> return ret;
>> }
>>
>>
>
>That's just a debug patch. I'll send out proper fix patches.
Ping!
While digging out this thread, I did not find any followup patch
for this suggestion. Did it hit the mailinglist anywhere?
If not, will you send one?
Regards,
Michael
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, Sep 19, 2023 at 11:10:55AM +0200, Michael Grzeschik wrote:
>On Mon, Oct 24, 2022 at 10:47:53PM +0000, Thinh Nguyen wrote:
>>On Sat, Oct 22, 2022, Jeff Vanhoof wrote:
>>>Hi Greg,
>>>
>>>On Sat, Oct 22, 2022 at 01:31:24PM +0200, Greg Kroah-Hartman wrote:
>>>> On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
>>>> > From: Jeff Vanhoof <[email protected]>
>>>> >
>>>> > arm-smmu related crashes seen after a Missed ISOC interrupt when
>>>> > no_interrupt=1 is used. This can happen if the hardware is still using
>>>> > the data associated with a TRB after the usb_request's ->complete call
>>>> > has been made. Instead of immediately releasing a request when a Missed
>>>> > ISOC interrupt has occurred, this change will add logic to cancel the
>>>> > request instead where it will eventually be released when the
>>>> > END_TRANSFER command has completed. This logic is similar to some of the
>>>> > cleanup done in dwc3_gadget_ep_dequeue.
>>>> >
>>>> > Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status")
>>>> > Cc: <[email protected]>
>>>> > Signed-off-by: Jeff Vanhoof <[email protected]>
>>>> > Co-developed-by: Dan Vacura <[email protected]>
>>>> > Signed-off-by: Dan Vacura <[email protected]>
>>>> > ---
>>>> > V1 -> V3:
>>>> > - no change, new patch in series
>>>> > V3 -> V4:
>>>> > - no change
>>>>
>>>> I need an ack from the dwc3 maintainer before I can take this one.
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>>
>>>Thinh has rejected this version of the patch. He has provided an alternative
>>>implementation which has been testing well for us so far. Either Thinh or Dan
>>>will formalize this patch within the next few days.
>>>The latest proposed changes are:
>>>
>>>diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>index dfaf9ac24c4f..50287437d6de 100644
>>>--- a/drivers/usb/dwc3/gadget.c
>>>+++ b/drivers/usb/dwc3/gadget.c
>>>@@ -3195,6 +3195,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>>> if (event->status & DEPEVT_STATUS_SHORT && !chain)
>>> return 1;
>>>
>>>+ if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC && !chain)
>>>+ return 1;
>>>+
>>> if ((trb->ctrl & DWC3_TRB_CTRL_IOC) ||
>>> (trb->ctrl & DWC3_TRB_CTRL_LST))
>>> return 1;
>>>@@ -3211,6 +3214,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
>>> struct scatterlist *s;
>>> unsigned int num_queued = req->num_queued_sgs;
>>> unsigned int i;
>>>+ bool missed_isoc = false;
>>> int ret = 0;
>>>
>>> for_each_sg(sg, s, num_queued, i) {
>>>@@ -3219,12 +3223,18 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
>>> req->sg = sg_next(s);
>>> req->num_queued_sgs--;
>>>
>>>+ if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)
>>>+ missed_isoc = true;
>>>+
>>> ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
>>> trb, event, status, true);
>>> if (ret)
>>> break;
>>> }
>>>
>>>+ if (missed_isoc)
>>>+ ret = 1;
>>>+
>>> return ret;
>>> }
>>>
>>>
>>
>>That's just a debug patch. I'll send out proper fix patches.
>
>Ping!
>
>While digging out this thread, I did not find any followup patch
>for this suggestion. Did it hit the mailinglist anywhere?
>
>If not, will you send one?
Nevermind, I think I found the hunk in a variated version in this series.
https://lore.kernel.org/linux-usb/[email protected]/
Michael
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |