Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Alexandre Bailon <abailon@baylibre.com>
To:     hminas@synopsys.com
Cc:     gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Alexandre Bailon <abailon@baylibre.com>
Subject: [RFC PATCH 1/1] usb: dwc2: Don't set DEV_DMA_L by default for isoc transfer
Date:   Wed, 26 Jul 2023 12:22:52 +0200
Message-ID: <20230726102252.2236314-2-abailon@baylibre.com>
In-Reply-To: <20230726102252.2236314-1-abailon@baylibre.com>
References: <20230726102252.2236314-1-abailon@baylibre.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Currently, by default, we set DEV_DMA_L to notify DMA that the current
descriptor is the last one.
But, to get better performances, the driver doesn't stop the DMA and add
a new descriptor at the end of the list and clear DEV_DMA_L bit from
previous descriptor. This works well except if the DMA has already fetched
the descriptor. Updating the descriptor owned by DMA can cause some issues.

This updates the driver to no update descriptor while it is in use.
Instead, this assumes that we are always going to queue a request
and feed the DMA with new descriptors.
In case where we don't feed the DMA, we stop the transfer using a BNA.

NOTE:
To be tested!!!
Currently, only tested and validated using UAC2.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 drivers/usb/dwc2/gadget.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 8b15742d9e8a..c542811468ce 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -928,10 +928,6 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
 		return 1;
 	}
 
-	/* Clear L bit of previous desc if more than one entries in the chain */
-	if (hs_ep->next_desc)
-		hs_ep->desc_list[index - 1].status &= ~DEV_DMA_L;
-
 	dev_dbg(hsotg->dev, "%s: Filling ep %d, dir %s isoc desc # %d\n",
 		__func__, hs_ep->index, hs_ep->dir_in ? "in" : "out", index);
 
@@ -939,8 +935,9 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
 	desc->status |= (DEV_DMA_BUFF_STS_HBUSY	<< DEV_DMA_BUFF_STS_SHIFT);
 
 	desc->buf = dma_buff;
-	desc->status |= (DEV_DMA_L | DEV_DMA_IOC |
-			 ((len << DEV_DMA_NBYTES_SHIFT) & mask));
+	desc->status |= (DEV_DMA_IOC | ((len << DEV_DMA_NBYTES_SHIFT) & mask));
+	if ((hs_ep->next_desc+1) >= MAX_DMA_DESC_NUM_HS_ISOC)
+		desc->status |= DEV_DMA_L;
 
 	if (hs_ep->dir_in) {
 		if (len)
@@ -968,6 +965,15 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
 	if (hs_ep->next_desc >= MAX_DMA_DESC_NUM_HS_ISOC)
 		hs_ep->next_desc = 0;
 
+
+	/*
+	 * Safeguard: Make sure we stop transfer if we don't have a valid descriptor.
+	 */
+	index = hs_ep->next_desc;
+	desc = &hs_ep->desc_list[index];
+	desc->status = 0;
+	desc->status |= (DEV_DMA_BUFF_STS_HBUSY	<< DEV_DMA_BUFF_STS_SHIFT);
+
 	return 0;
 }
 
-- 
2.41.0