* [PATCH v2 1/4] usb: cdns3: fix uvc failure work since sg support enabled
2023-12-24 15:38 [PATCH v2 0/4] usb: cdns3: usb uvc iso transfer fix Frank Li
@ 2023-12-24 15:38 ` Frank Li
2023-12-24 15:38 ` [PATCH v2 2/4] usb: cdns3: fix iso transfer error when mult is not zero Frank Li
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Frank Li @ 2023-12-24 15:38 UTC (permalink / raw)
To: cugyly
Cc: Frank.Li, a-govindraju, gregkh, imx, linux-kernel, linux-usb,
pawell, peter.chen, rogerq
When IP version >= DEV_VER_V2, gadget:sg_supported is true. So uvc gadget
function driver will use sg to equeue data, first is 8bytes header, the
second is 1016bytes data.
cdns3_prepare_trb: ep2in: trb 0000000000ac755f, dma buf: 0xbf455000, size: 8, burst: 128 ctrl: 0x00000415 (C=1, T=0, ISP, CHAIN, Normal)
cdns3_prepare_trb: ep2in: trb 00000000a574e693, dma buf: 0xc0200fe0, size: 1016, burst: 128 ctrl: 0x00000405 (C=1, T=0, ISP, Normal)
But cdns3_ep_run_transfer() can't correctly handle this case, which only
support one TRB for ISO transfer.
The controller requires duplicate the TD for each SOF if priv_ep->interval
is not 1. DMA will read data from DDR to internal FIFO when get SOF. Send
data to bus when receive IN token. DMA always refill FIFO when get SOF
regardless host send IN token or not. If host send IN token later, some
frames data will be lost.
Fixed it by below major steps:
1. Calculate numembers of TRB base on sg_nums and priv_ep->interval.
2. Remove CHAIN flags for each end TRB of TD when duplicate TD.
3. The controller requires LINK TRB must be first TRB of TD. When check
there are not enough TRBs lefts, just fill LINK TRB for left TRBs.
.... CHAIN_TRB DATA_TRB, CHAIN_TRB DATA_TRB, LINK_TRB ... LINK_TRB
^End of TRB List
Cc: stable@vger.kernel.org
Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/usb/cdns3/cdns3-gadget.c | 51 +++++++++++++++++++++++++-------
1 file changed, 40 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
index 15463b7cddd23..22a31ffa69423 100644
--- a/drivers/usb/cdns3/cdns3-gadget.c
+++ b/drivers/usb/cdns3/cdns3-gadget.c
@@ -1119,6 +1119,7 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
dma_addr_t trb_dma;
u32 togle_pcs = 1;
int sg_iter = 0;
+ int num_trb_req;
int num_trb;
int address;
u32 control;
@@ -1128,15 +1129,13 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
bool sg_supported = !!(request->num_mapped_sgs);
u32 ioc = request->no_interrupt ? 0 : TRB_IOC;
+ num_trb_req = sg_supported ? request->num_mapped_sgs : 1;
+
+ /* ISO transfer require each SOF have a TD, each TD include some TRBs */
if (priv_ep->type == USB_ENDPOINT_XFER_ISOC)
- num_trb = priv_ep->interval;
+ num_trb = priv_ep->interval * num_trb_req;
else
- num_trb = sg_supported ? request->num_mapped_sgs : 1;
-
- if (num_trb > priv_ep->free_trbs) {
- priv_ep->flags |= EP_RING_FULL;
- return -ENOBUFS;
- }
+ num_trb = num_trb_req;
priv_req = to_cdns3_request(request);
address = priv_ep->endpoint.desc->bEndpointAddress;
@@ -1185,14 +1184,31 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
link_trb->control = cpu_to_le32(((priv_ep->pcs) ? TRB_CYCLE : 0) |
TRB_TYPE(TRB_LINK) | TRB_TOGGLE | ch_bit);
+
+ if (priv_ep->type == USB_ENDPOINT_XFER_ISOC) {
+ /*
+ * ISO require LINK TRB must be first one of TD.
+ * Fill LINK TRBs for left trb space to simply software process logic.
+ */
+ while (priv_ep->enqueue) {
+ *trb = *link_trb;
+ trace_cdns3_prepare_trb(priv_ep, trb);
+
+ cdns3_ep_inc_enq(priv_ep);
+ trb = priv_ep->trb_pool + priv_ep->enqueue;
+ priv_req->trb = trb;
+ }
+ }
+ }
+
+ if (num_trb > priv_ep->free_trbs) {
+ priv_ep->flags |= EP_RING_FULL;
+ return -ENOBUFS;
}
if (priv_dev->dev_ver <= DEV_VER_V2)
togle_pcs = cdns3_wa1_update_guard(priv_ep, trb);
- if (sg_supported)
- s = request->sg;
-
/* set incorrect Cycle Bit for first trb*/
control = priv_ep->pcs ? 0 : TRB_CYCLE;
trb->length = 0;
@@ -1210,6 +1226,9 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
do {
u32 length;
+ if (!(sg_iter % num_trb_req) && sg_supported)
+ s = request->sg;
+
/* fill TRB */
control |= TRB_TYPE(TRB_NORMAL);
if (sg_supported) {
@@ -1251,7 +1270,7 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
if (sg_supported) {
trb->control |= cpu_to_le32(TRB_ISP);
/* Don't set chain bit for last TRB */
- if (sg_iter < num_trb - 1)
+ if ((sg_iter % num_trb_req) < num_trb_req - 1)
trb->control |= cpu_to_le32(TRB_CHAIN);
s = sg_next(s);
@@ -1509,6 +1528,12 @@ static void cdns3_transfer_completed(struct cdns3_device *priv_dev,
/* The TRB was changed as link TRB, and the request was handled at ep_dequeue */
while (TRB_FIELD_TO_TYPE(le32_to_cpu(trb->control)) == TRB_LINK) {
+
+ /* ISO ep_traddr may stop at LINK TRB */
+ if (priv_ep->dequeue == cdns3_get_dma_pos(priv_dev, priv_ep) &&
+ priv_ep->type == USB_ENDPOINT_XFER_ISOC)
+ break;
+
trace_cdns3_complete_trb(priv_ep, trb);
cdns3_ep_inc_deq(priv_ep);
trb = priv_ep->trb_pool + priv_ep->dequeue;
@@ -1541,6 +1566,10 @@ static void cdns3_transfer_completed(struct cdns3_device *priv_dev,
}
if (request_handled) {
+ /* TRBs are duplicated by priv_ep->interval time for ISO IN */
+ if (priv_ep->type == USB_ENDPOINT_XFER_ISOC && priv_ep->dir)
+ request->actual /= priv_ep->interval;
+
cdns3_gadget_giveback(priv_ep, priv_req, 0);
request_handled = false;
transfer_end = false;
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 2/4] usb: cdns3: fix iso transfer error when mult is not zero
2023-12-24 15:38 [PATCH v2 0/4] usb: cdns3: usb uvc iso transfer fix Frank Li
2023-12-24 15:38 ` [PATCH v2 1/4] usb: cdns3: fix uvc failure work since sg support enabled Frank Li
@ 2023-12-24 15:38 ` Frank Li
2023-12-24 15:38 ` [PATCH v2 3/4] usb: cdns3: Fix uvc fail when DMA cross 4k boundery since sg enabled Frank Li
2023-12-24 15:38 ` [PATCH v2 4/4] Revert "usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()" Frank Li
3 siblings, 0 replies; 5+ messages in thread
From: Frank Li @ 2023-12-24 15:38 UTC (permalink / raw)
To: cugyly
Cc: Frank.Li, a-govindraju, gregkh, imx, linux-kernel, linux-usb,
pawell, peter.chen, rogerq
ISO basic transfer is
ITP(SOF) Package_0 Package_1 ... Package_n
CDNS3 DMA start dma transfer from memmory to internal FIFO when get SOF,
controller will transfer data to usb bus from internal FIFO when get IN
token.
According USB spec defination:
Maximum number of packets = (bMaxBurst + 1) * (Mult + 1)
Internal memory should be the same as (bMaxBurst + 1) * (Mult + 1). DMA
don't fetch data advance when ISO transfer, so only reserve
(bMaxBurst + 1) * (Mult + 1) internal memory for ISO transfer.
Need save Mult and bMaxBurst information and set it into EP_CFG register,
otherwise only 1 package is sent by controller, other package will be
lost.
Cc: stable@vger.kernel.org
Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/usb/cdns3/cdns3-gadget.c | 59 +++++++++++++++++++-------------
drivers/usb/cdns3/cdns3-gadget.h | 3 ++
2 files changed, 39 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
index 22a31ffa69423..4c6893af22dde 100644
--- a/drivers/usb/cdns3/cdns3-gadget.c
+++ b/drivers/usb/cdns3/cdns3-gadget.c
@@ -2065,11 +2065,10 @@ int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)
bool is_iso_ep = (priv_ep->type == USB_ENDPOINT_XFER_ISOC);
struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
u32 bEndpointAddress = priv_ep->num | priv_ep->dir;
- u32 max_packet_size = 0;
- u8 maxburst = 0;
+ u32 max_packet_size = priv_ep->wMaxPacketSize;
+ u8 maxburst = priv_ep->bMaxBurst;
u32 ep_cfg = 0;
u8 buffering;
- u8 mult = 0;
int ret;
buffering = priv_dev->ep_buf_size - 1;
@@ -2091,8 +2090,7 @@ int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)
break;
default:
ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_ISOC);
- mult = priv_dev->ep_iso_burst - 1;
- buffering = mult + 1;
+ buffering = (priv_ep->bMaxBurst + 1) * (priv_ep->mult + 1) - 1;
}
switch (priv_dev->gadget.speed) {
@@ -2103,17 +2101,8 @@ int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)
max_packet_size = is_iso_ep ? 1024 : 512;
break;
case USB_SPEED_SUPER:
- /* It's limitation that driver assumes in driver. */
- mult = 0;
- max_packet_size = 1024;
- if (priv_ep->type == USB_ENDPOINT_XFER_ISOC) {
- maxburst = priv_dev->ep_iso_burst - 1;
- buffering = (mult + 1) *
- (maxburst + 1);
-
- if (priv_ep->interval > 1)
- buffering++;
- } else {
+ if (priv_ep->type != USB_ENDPOINT_XFER_ISOC) {
+ max_packet_size = 1024;
maxburst = priv_dev->ep_buf_size - 1;
}
break;
@@ -2142,7 +2131,6 @@ int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)
if (priv_dev->dev_ver < DEV_VER_V2)
priv_ep->trb_burst_size = 16;
- mult = min_t(u8, mult, EP_CFG_MULT_MAX);
buffering = min_t(u8, buffering, EP_CFG_BUFFERING_MAX);
maxburst = min_t(u8, maxburst, EP_CFG_MAXBURST_MAX);
@@ -2176,7 +2164,7 @@ int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)
}
ep_cfg |= EP_CFG_MAXPKTSIZE(max_packet_size) |
- EP_CFG_MULT(mult) |
+ EP_CFG_MULT(priv_ep->mult) | /* must match EP setting */
EP_CFG_BUFFERING(buffering) |
EP_CFG_MAXBURST(maxburst);
@@ -2266,6 +2254,13 @@ usb_ep *cdns3_gadget_match_ep(struct usb_gadget *gadget,
priv_ep->type = usb_endpoint_type(desc);
priv_ep->flags |= EP_CLAIMED;
priv_ep->interval = desc->bInterval ? BIT(desc->bInterval - 1) : 0;
+ priv_ep->wMaxPacketSize = usb_endpoint_maxp(desc);
+ priv_ep->mult = USB_EP_MAXP_MULT(priv_ep->wMaxPacketSize);
+ priv_ep->wMaxPacketSize &= USB_ENDPOINT_MAXP_MASK;
+ if (priv_ep->type == USB_ENDPOINT_XFER_ISOC && comp_desc) {
+ priv_ep->mult = USB_SS_MULT(comp_desc->bmAttributes) - 1;
+ priv_ep->bMaxBurst = comp_desc->bMaxBurst;
+ }
spin_unlock_irqrestore(&priv_dev->lock, flags);
return &priv_ep->endpoint;
@@ -3049,22 +3044,40 @@ static int cdns3_gadget_check_config(struct usb_gadget *gadget)
struct cdns3_endpoint *priv_ep;
struct usb_ep *ep;
int n_in = 0;
+ int iso = 0;
+ int out = 1;
int total;
+ int n;
list_for_each_entry(ep, &gadget->ep_list, ep_list) {
priv_ep = ep_to_cdns3_ep(ep);
- if ((priv_ep->flags & EP_CLAIMED) && (ep->address & USB_DIR_IN))
- n_in++;
+ if (!(priv_ep->flags & EP_CLAIMED))
+ continue;
+
+ n = (priv_ep->mult + 1) * (priv_ep->bMaxBurst + 1);
+ if (ep->address & USB_DIR_IN) {
+ /*
+ * ISO transfer: DMA start move data when get ISO, only transfer
+ * data as min(TD size, iso). No benefit for allocate bigger
+ * internal memory than 'iso'.
+ */
+ if (priv_ep->type == USB_ENDPOINT_XFER_ISOC)
+ iso += n;
+ else
+ n_in++;
+ } else {
+ if (priv_ep->type == USB_ENDPOINT_XFER_ISOC)
+ out = max_t(int, out, n);
+ }
}
/* 2KB are reserved for EP0, 1KB for out*/
- total = 2 + n_in + 1;
+ total = 2 + n_in + out + iso;
if (total > priv_dev->onchip_buffers)
return -ENOMEM;
- priv_dev->ep_buf_size = priv_dev->ep_iso_burst =
- (priv_dev->onchip_buffers - 2) / (n_in + 1);
+ priv_dev->ep_buf_size = (priv_dev->onchip_buffers - 2 - iso) / (n_in + out);
return 0;
}
diff --git a/drivers/usb/cdns3/cdns3-gadget.h b/drivers/usb/cdns3/cdns3-gadget.h
index fbe4a8e3aa897..086a7bb838975 100644
--- a/drivers/usb/cdns3/cdns3-gadget.h
+++ b/drivers/usb/cdns3/cdns3-gadget.h
@@ -1168,6 +1168,9 @@ struct cdns3_endpoint {
u8 dir;
u8 num;
u8 type;
+ u8 mult;
+ u8 bMaxBurst;
+ u16 wMaxPacketSize;
int interval;
int free_trbs;
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 3/4] usb: cdns3: Fix uvc fail when DMA cross 4k boundery since sg enabled
2023-12-24 15:38 [PATCH v2 0/4] usb: cdns3: usb uvc iso transfer fix Frank Li
2023-12-24 15:38 ` [PATCH v2 1/4] usb: cdns3: fix uvc failure work since sg support enabled Frank Li
2023-12-24 15:38 ` [PATCH v2 2/4] usb: cdns3: fix iso transfer error when mult is not zero Frank Li
@ 2023-12-24 15:38 ` Frank Li
2023-12-24 15:38 ` [PATCH v2 4/4] Revert "usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()" Frank Li
3 siblings, 0 replies; 5+ messages in thread
From: Frank Li @ 2023-12-24 15:38 UTC (permalink / raw)
To: cugyly
Cc: Frank.Li, a-govindraju, gregkh, imx, linux-kernel, linux-usb,
pawell, peter.chen, rogerq
Supposed DMA cross 4k bounder problem should be fixed at DEV_VER_V2, but
still met problem when do ISO transfer if sg enabled.
Data pattern likes below when sg enabled, package size is 1k and mult is 2
[UVC Header(8B) ] [data(3k - 8)] ...
The received data at offset 0xd000 will get 0xc000 data, len 0x70. Error
happen position as below pattern:
0xd000: wrong
0xe000: wrong
0xf000: correct
0x10000: wrong
0x11000: wrong
0x12000: correct
...
To avoid DMA cross 4k bounder at ISO transfer, reduce burst len according
to start DMA address's alignment.
Cc: stable@vger.kernel.org
Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/usb/cdns3/cdns3-gadget.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
index 4c6893af22dde..aeca902ab6cc4 100644
--- a/drivers/usb/cdns3/cdns3-gadget.c
+++ b/drivers/usb/cdns3/cdns3-gadget.c
@@ -1120,6 +1120,7 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
u32 togle_pcs = 1;
int sg_iter = 0;
int num_trb_req;
+ int trb_burst;
int num_trb;
int address;
u32 control;
@@ -1243,7 +1244,36 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
total_tdl += DIV_ROUND_UP(length,
priv_ep->endpoint.maxpacket);
- trb->length |= cpu_to_le32(TRB_BURST_LEN(priv_ep->trb_burst_size) |
+ trb_burst = priv_ep->trb_burst_size;
+
+ /*
+ * Supposed DMA cross 4k bounder problem should be fixed at DEV_VER_V2, but still
+ * met problem when do ISO transfer if sg enabled.
+ *
+ * Data pattern likes below when sg enabled, package size is 1k and mult is 2
+ * [UVC Header(8B) ] [data(3k - 8)] ...
+ *
+ * The received data at offset 0xd000 will get 0xc000 data, len 0x70. Error happen
+ * as below pattern:
+ * 0xd000: wrong
+ * 0xe000: wrong
+ * 0xf000: correct
+ * 0x10000: wrong
+ * 0x11000: wrong
+ * 0x12000: correct
+ * ...
+ *
+ * But it is still unclear about why error have not happen below 0xd000, it should
+ * cross 4k bounder. But anyway, the below code can fix this problem.
+ *
+ * To avoid DMA cross 4k bounder at ISO transfer, reduce burst len according to 16.
+ */
+ if (priv_ep->type == USB_ENDPOINT_XFER_ISOC && priv_dev->dev_ver <= DEV_VER_V2)
+ if (ALIGN_DOWN(trb->buffer, SZ_4K) !=
+ ALIGN_DOWN(trb->buffer + length, SZ_4K))
+ trb_burst = 16;
+
+ trb->length |= cpu_to_le32(TRB_BURST_LEN(trb_burst) |
TRB_LEN(length));
pcs = priv_ep->pcs ? TRB_CYCLE : 0;
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 4/4] Revert "usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()"
2023-12-24 15:38 [PATCH v2 0/4] usb: cdns3: usb uvc iso transfer fix Frank Li
` (2 preceding siblings ...)
2023-12-24 15:38 ` [PATCH v2 3/4] usb: cdns3: Fix uvc fail when DMA cross 4k boundery since sg enabled Frank Li
@ 2023-12-24 15:38 ` Frank Li
3 siblings, 0 replies; 5+ messages in thread
From: Frank Li @ 2023-12-24 15:38 UTC (permalink / raw)
To: cugyly
Cc: Frank.Li, a-govindraju, gregkh, imx, linux-kernel, linux-usb,
pawell, peter.chen, rogerq
This reverts commit 3c5b006f3ee800b4bd9ed37b3a8f271b8560126e.
gadget_is_{super|dual}speed() API check UDC controller capitblity. It
should pass down highest speed endpoint descriptor to UDC controller. So
UDC controller driver can reserve enough resource at check_config(),
especially mult and maxburst. So UDC driver (such as cdns3) can know need
at least (mult + 1) * (maxburst + 1) * wMaxPacketSize internal memory for
this uvc functions.
Cc: stable@vger.kernel.org
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/usb/gadget/function/f_uvc.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index faa398109431f..3d5c03e1361e4 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -719,13 +719,29 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
}
uvc->enable_interrupt_ep = opts->enable_interrupt_ep;
- ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
+ /*
+ * gadget_is_{super|dual}speed() API check UDC controller capitblity. It should pass down
+ * highest speed endpoint descriptor to UDC controller. So UDC controller driver can reserve
+ * enough resource at check_config(), especially mult and maxburst. So UDC driver (such as
+ * cdns3) can know need at least (mult + 1) * (maxburst + 1) * wMaxPacketSize internal
+ * memory for this uvc functions. This is the only straightforward method to resolve the UDC
+ * resource allocation issue in the current gadget framework.
+ */
+ if (gadget_is_superspeed(c->cdev->gadget))
+ ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
+ &uvc_ss_streaming_comp);
+ else if (gadget_is_dualspeed(cdev->gadget))
+ ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
+ else
+ ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
+
if (!ep) {
uvcg_info(f, "Unable to allocate streaming EP\n");
goto error;
}
uvc->video.ep = ep;
+ uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread