Thread (8 messages) 8 messages, 3 authors, 2021-01-29

Re: [PATCH] usb: dwc3: gadget: Init only available HW eps

From: Felipe Balbi <balbi@kernel.org>
Date: 2021-01-06 07:52:19
Also in: stable

Hi,

Thinh Nguyen [off-list ref] writes:
quoted hunk ↗ jump to hunk
Typically FPGA devices are configured with CoreConsultant parameter
DWC_USB3x_EN_LOG_PHYS_EP_SUPT=0 to reduce gate count and improve timing.
This means that the number of INs equals to OUTs endpoints. But
typically non-FPGA devices enable this CoreConsultant parameter to
support flexible endpoint mapping and potentially may have unequal
number of INs to OUTs physical endpoints.

The driver must check how many physical endpoints are available for each
direction and initialize them properly.

Cc: stable@vger.kernel.org
Fixes: 47d3946ea220 ("usb: dwc3: refactor gadget endpoint count calculation")
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
 drivers/usb/dwc3/core.c   |  1 +
 drivers/usb/dwc3/core.h   |  2 ++
 drivers/usb/dwc3/gadget.c | 19 ++++++++++++-------
 3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 841daec70b6e..1084aa8623c2 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -529,6 +529,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc)
 	struct dwc3_hwparams	*parms = &dwc->hwparams;
 
 	dwc->num_eps = DWC3_NUM_EPS(parms);
+	dwc->num_in_eps = DWC3_NUM_IN_EPS(parms);
 }
 
 static void dwc3_cache_hwparams(struct dwc3 *dwc)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1b241f937d8f..1295dac019f9 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -990,6 +990,7 @@ struct dwc3_scratchpad_array {
  * @u1sel: parameter from Set SEL request.
  * @u1pel: parameter from Set SEL request.
  * @num_eps: number of endpoints
+ * @num_in_eps: number of IN endpoints
  * @ep0_next_event: hold the next expected event
  * @ep0state: state of endpoint zero
  * @link_state: link state
@@ -1193,6 +1194,7 @@ struct dwc3 {
 	u8			speed;
 
 	u8			num_eps;
+	u8			num_in_eps;
 
 	struct dwc3_hwparams	hwparams;
 	struct dentry		*root;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 25f654b79e48..8a38ee10c00b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2025,7 +2025,7 @@ static void dwc3_stop_active_transfers(struct dwc3 *dwc)
 {
 	u32 epnum;
 
-	for (epnum = 2; epnum < dwc->num_eps; epnum++) {
+	for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
 		struct dwc3_ep *dep;
 
 		dep = dwc->eps[epnum];
@@ -2628,16 +2628,21 @@ static int dwc3_gadget_init_endpoint(struct dwc3 *dwc, u8 epnum)
 	return 0;
 }
 
-static int dwc3_gadget_init_endpoints(struct dwc3 *dwc, u8 total)
+static int dwc3_gadget_init_endpoints(struct dwc3 *dwc)
 {
-	u8				epnum;
+	u8				i;
+	int				ret;
 
 	INIT_LIST_HEAD(&dwc->gadget->ep_list);
 
-	for (epnum = 0; epnum < total; epnum++) {
-		int			ret;
+	for (i = 0; i < dwc->num_in_eps; i++) {
+		ret = dwc3_gadget_init_endpoint(dwc, i * 2 + 1);
+		if (ret)
+			return ret;
+	}
 
-		ret = dwc3_gadget_init_endpoint(dwc, epnum);
+	for (i = 0; i < dwc->num_eps - dwc->num_in_eps; i++) {
+		ret = dwc3_gadget_init_endpoint(dwc, i * 2);
 		if (ret)
 			return ret;
 	}
@@ -3863,7 +3868,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 	 * sure we're starting from a well known location.
 	 */
 
-	ret = dwc3_gadget_init_endpoints(dwc, dwc->num_eps);
+	ret = dwc3_gadget_init_endpoints(dwc);
 	if (ret)
 		goto err4;
heh, looking at original commit, we used to have num_in_eps and
num_out_eps. In fact, this commit will reintroduce another problem that
Bryan tried to solve. num_eps - num_in_eps is not necessarily
num_out_eps.

How have you verified this patch? Did you read Bryan's commit log? This
is likely to reintroduce the problem raised by Bryan.

-- 
balbi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help