[PATCH v3 0/4] usb: dwc3: Avoid using reserved EPs

[PATCH v3 0/4] usb: dwc3: Avoid using reserved EPs

[Andy Shevchenko]

On some platforms (Intel-based and AFAIK ARM-based) the EPs in the gadget
(USB Device Controller mode) may be reserved for some special means, such as
tracing. This series extends DT schema and driver to avoid using those.
Without this the USB gadget mode won't work properly (those devices that
"luckily" allocated the reserved EPs).

Changelog v3:
- split refactoring as preparatory change (Thinh)
- renamed variable and function (Thinh)
- added a warning in one place (Thinh)
- gathered tags (Ferry, Rob, Thinh)

Changelog v2:
- added minItems to the schema (Rob)
- revisited code and add NULL check to avoid crashes (Thinh)
- rewrote helper function to return error to the user if parsing fails
- elaborated in the commit message why we need this quirk (Thinh)
- addressed miscellaneous style issues (Thinh)

Andy Shevchenko (4):
  dt-bindings: usb: dwc3: Add a property to reserve endpoints
  usb: dwc3: gadget: Refactor loop to avoid NULL endpoints
  usb: dwc3: gadget: Add support for snps,reserved-endpoints property
  usb: dwc3: gadget: Avoid using reserved endpoints on Intel Merrifield

 .../bindings/usb/snps,dwc3-common.yaml        | 11 ++++
 drivers/usb/dwc3/dwc3-pci.c                   | 10 +++
 drivers/usb/dwc3/gadget.c                     | 63 +++++++++++++++++--
 3 files changed, 79 insertions(+), 5 deletions(-)

-- 
2.45.1.3035.g276e886db78b

[PATCH v3 1/4] dt-bindings: usb: dwc3: Add a property to reserve endpoints

[Andy Shevchenko]

Some of the endpoints may be reserved by hardware for different purposes,
e.g., tracing control and output. This is the case, for instance, on
Intel Merrifield and Moorefield platforms that reserve a few and USB driver
may not use them for the regular transfers. Add the respective bindings.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <fntoth@gmail.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../devicetree/bindings/usb/snps,dwc3-common.yaml     | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
index c956053fd036..71249b6ba616 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
@@ -65,6 +65,17 @@ properties:
       mode.
     type: boolean
 
+  snps,reserved-endpoints:
+    description:
+      Reserve endpoints for other needs, e.g, for tracing control and output.
+      When set, the driver will avoid using them for the regular USB transfers.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 1
+    maxItems: 30
+    items:
+      minimum: 2
+      maximum: 31
+
   snps,dis-start-transfer-quirk:
     description:
       When set, disable isoc START TRANSFER command failure SW work-around
-- 
2.45.1.3035.g276e886db78b

[PATCH v3 2/4] usb: dwc3: gadget: Refactor loop to avoid NULL endpoints

[Andy Shevchenko]

Prepare the gadget driver to handle the reserved endpoints that will be
not allocated at the initialisation time.

While at it, add a warning where the NULL endpoint should never happen.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <fntoth@gmail.com>
---
 drivers/usb/dwc3/gadget.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index d27af65eb08a..73cebb7d90c2 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -547,6 +547,7 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3_ep *dep)
 int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index)
 {
 	struct dwc3_gadget_ep_cmd_params params;
+	struct dwc3_ep		*dep;
 	u32			cmd;
 	int			i;
 	int			ret;
@@ -563,8 +564,13 @@ int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index)
 		return ret;
 
 	/* Reset resource allocation flags */
-	for (i = resource_index; i < dwc->num_eps && dwc->eps[i]; i++)
-		dwc->eps[i]->flags &= ~DWC3_EP_RESOURCE_ALLOCATED;
+	for (i = resource_index; i < dwc->num_eps; i++) {
+		dep = dwc->eps[i];
+		if (!dep)
+			continue;
+
+		dep->flags &= ~DWC3_EP_RESOURCE_ALLOCATED;
+	}
 
 	return 0;
 }
@@ -751,9 +757,11 @@ void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc)
 
 	dwc->last_fifo_depth = fifo_depth;
 	/* Clear existing TXFIFO for all IN eps except ep0 */
-	for (num = 3; num < min_t(int, dwc->num_eps, DWC3_ENDPOINTS_NUM);
-	     num += 2) {
+	for (num = 3; num < min_t(int, dwc->num_eps, DWC3_ENDPOINTS_NUM); num += 2) {
 		dep = dwc->eps[num];
+		if (!dep)
+			continue;
+
 		/* Don't change TXFRAMNUM on usb31 version */
 		size = DWC3_IP_IS(DWC3) ? 0 :
 			dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(num >> 1)) &
@@ -3669,6 +3677,8 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
 
 		for (i = 0; i < DWC3_ENDPOINTS_NUM; i++) {
 			dep = dwc->eps[i];
+			if (!dep)
+				continue;
 
 			if (!(dep->flags & DWC3_EP_ENABLED))
 				continue;
@@ -3818,6 +3828,10 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
 	u8			epnum = event->endpoint_number;
 
 	dep = dwc->eps[epnum];
+	if (!dep) {
+		dev_warn(dwc->dev, "spurious event, endpoint %u is not allocated\n", epnum);
+		return;
+	}
 
 	if (!(dep->flags & DWC3_EP_ENABLED)) {
 		if ((epnum > 1) && !(dep->flags & DWC3_EP_TRANSFER_STARTED))
-- 
2.45.1.3035.g276e886db78b

[PATCH v3 3/4] usb: dwc3: gadget: Add support for snps,reserved-endpoints property

[Andy Shevchenko]

The snps,reserved-endpoints property lists the reserved endpoints
that shouldn't be used for normal transfers. Add support for that
to the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <fntoth@gmail.com>
---
 drivers/usb/dwc3/gadget.c | 41 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 73cebb7d90c2..f3ad8434366e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3403,14 +3403,53 @@ static int dwc3_gadget_init_endpoint(struct dwc3 *dwc, u8 epnum)
 	return 0;
 }
 
+static int dwc3_gadget_get_reserved_endpoints(struct dwc3 *dwc, const char *propname,
+					      u8 *eps, u8 num)
+{
+	u8 count;
+	int ret;
+
+	if (!device_property_present(dwc->dev, propname))
+		return 0;
+
+	ret = device_property_count_u8(dwc->dev, propname);
+	if (ret < 0)
+		return ret;
+	count = ret;
+
+	ret = device_property_read_u8_array(dwc->dev, propname, eps, min(num, count));
+	if (ret)
+		return ret;
+
+	return count;
+}
+
 static int dwc3_gadget_init_endpoints(struct dwc3 *dwc, u8 total)
 {
+	const char			*propname = "snps,reserved-endpoints";
 	u8				epnum;
+	u8				reserved_eps[DWC3_ENDPOINTS_NUM];
+	u8				count;
+	u8				num;
+	int				ret;
 
 	INIT_LIST_HEAD(&dwc->gadget->ep_list);
 
+	ret = dwc3_gadget_get_reserved_endpoints(dwc, propname,
+						 reserved_eps, ARRAY_SIZE(reserved_eps));
+	if (ret < 0) {
+		dev_err(dwc->dev, "failed to read %s\n", propname);
+		return ret;
+	}
+	count = ret;
+
 	for (epnum = 0; epnum < total; epnum++) {
-		int			ret;
+		for (num = 0; num < count; num++) {
+			if (epnum == reserved_eps[num])
+				break;
+		}
+		if (num < count)
+			continue;
 
 		ret = dwc3_gadget_init_endpoint(dwc, epnum);
 		if (ret)
-- 
2.45.1.3035.g276e886db78b

[PATCH v3 4/4] usb: dwc3: gadget: Avoid using reserved endpoints on Intel Merrifield

[Andy Shevchenko]

Intel Merrifield SoC uses these endpoints for tracing and they cannot
be re-allocated if being used because the side band flow control signals
are hard wired to certain endpoints:

• 1 High BW Bulk IN (IN#1) (RTIT)
• 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)

In device mode, since RTIT (EP#1) and EXI/RunControl (EP#8) uses
External Buffer Control (EBC) mode, these endpoints are to be mapped to
EBC mode (to be done by EXI target driver). Additionally TRB for RTIT
and EXI are maintained in STM (System Trace Module) unit and the EXI
target driver will as well configure the TRB location for EP #1 IN
and EP#8 (IN and OUT). Since STM/PTI and EXI hardware blocks manage
these endpoints and interface to OTG3 controller through EBC interface,
there is no need to enable any events (such as XferComplete etc)
for these end points.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <fntoth@gmail.com>
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
 drivers/usb/dwc3/dwc3-pci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 052852f80146..54a4ee2b90b7 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -148,11 +148,21 @@ static const struct property_entry dwc3_pci_intel_byt_properties[] = {
 	{}
 };
 
+/*
+ * Intel Merrifield SoC uses these endpoints for tracing and they cannot
+ * be re-allocated if being used because the side band flow control signals
+ * are hard wired to certain endpoints:
+ * - 1 High BW Bulk IN (IN#1) (RTIT)
+ * - 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)
+ */
+static const u8 dwc3_pci_mrfld_reserved_endpoints[] = { 3, 16, 17 };
+
 static const struct property_entry dwc3_pci_mrfld_properties[] = {
 	PROPERTY_ENTRY_STRING("dr_mode", "otg"),
 	PROPERTY_ENTRY_STRING("linux,extcon-name", "mrfld_bcove_pwrsrc"),
 	PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
 	PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
+	PROPERTY_ENTRY_U8_ARRAY("snps,reserved-endpoints", dwc3_pci_mrfld_reserved_endpoints),
 	PROPERTY_ENTRY_BOOL("snps,usb2-gadget-lpm-disable"),
 	PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
 	{}
-- 
2.45.1.3035.g276e886db78b

Re: [PATCH v3 2/4] usb: dwc3: gadget: Refactor loop to avoid NULL endpoints

[Thinh Nguyen]

On Wed, Feb 12, 2025, Andy Shevchenko wrote:
> Prepare the gadget driver to handle the reserved endpoints that will be
> not allocated at the initialisation time.
> 
> While at it, add a warning where the NULL endpoint should never happen.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Ferry Toth <fntoth@gmail.com>
> ---
>  drivers/usb/dwc3/gadget.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index d27af65eb08a..73cebb7d90c2 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -547,6 +547,7 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3_ep *dep)
>  int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index)
>  {
>  	struct dwc3_gadget_ep_cmd_params params;
> +	struct dwc3_ep		*dep;
>  	u32			cmd;
>  	int			i;
>  	int			ret;
> @@ -563,8 +564,13 @@ int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index)
>  		return ret;
>  
>  	/* Reset resource allocation flags */
> -	for (i = resource_index; i < dwc->num_eps && dwc->eps[i]; i++)
> -		dwc->eps[i]->flags &= ~DWC3_EP_RESOURCE_ALLOCATED;
> +	for (i = resource_index; i < dwc->num_eps; i++) {
> +		dep = dwc->eps[i];
> +		if (!dep)
> +			continue;
> +
> +		dep->flags &= ~DWC3_EP_RESOURCE_ALLOCATED;
> +	}
>  
>  	return 0;
>  }
> @@ -751,9 +757,11 @@ void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc)
>  
>  	dwc->last_fifo_depth = fifo_depth;
>  	/* Clear existing TXFIFO for all IN eps except ep0 */
> -	for (num = 3; num < min_t(int, dwc->num_eps, DWC3_ENDPOINTS_NUM);
> -	     num += 2) {
> +	for (num = 3; num < min_t(int, dwc->num_eps, DWC3_ENDPOINTS_NUM); num += 2) {
>  		dep = dwc->eps[num];
> +		if (!dep)
> +			continue;
> +
>  		/* Don't change TXFRAMNUM on usb31 version */
>  		size = DWC3_IP_IS(DWC3) ? 0 :
>  			dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(num >> 1)) &
> @@ -3669,6 +3677,8 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
>  
>  		for (i = 0; i < DWC3_ENDPOINTS_NUM; i++) {
>  			dep = dwc->eps[i];
> +			if (!dep)
> +				continue;
>  
>  			if (!(dep->flags & DWC3_EP_ENABLED))
>  				continue;
> @@ -3818,6 +3828,10 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
>  	u8			epnum = event->endpoint_number;
>  
>  	dep = dwc->eps[epnum];
> +	if (!dep) {
> +		dev_warn(dwc->dev, "spurious event, endpoint %u is not allocated\n", epnum);
> +		return;
> +	}
>  
>  	if (!(dep->flags & DWC3_EP_ENABLED)) {
>  		if ((epnum > 1) && !(dep->flags & DWC3_EP_TRANSFER_STARTED))
> -- 
> 2.45.1.3035.g276e886db78b
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

Re: [PATCH v3 3/4] usb: dwc3: gadget: Add support for snps,reserved-endpoints property

[Thinh Nguyen]

On Wed, Feb 12, 2025, Andy Shevchenko wrote:
> The snps,reserved-endpoints property lists the reserved endpoints
> that shouldn't be used for normal transfers. Add support for that
> to the driver.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Ferry Toth <fntoth@gmail.com>
> ---
>  drivers/usb/dwc3/gadget.c | 41 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 73cebb7d90c2..f3ad8434366e 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3403,14 +3403,53 @@ static int dwc3_gadget_init_endpoint(struct dwc3 *dwc, u8 epnum)
>  	return 0;
>  }
>  
> +static int dwc3_gadget_get_reserved_endpoints(struct dwc3 *dwc, const char *propname,
> +					      u8 *eps, u8 num)
> +{
> +	u8 count;
> +	int ret;
> +
> +	if (!device_property_present(dwc->dev, propname))
> +		return 0;
> +
> +	ret = device_property_count_u8(dwc->dev, propname);
> +	if (ret < 0)
> +		return ret;
> +	count = ret;
> +
> +	ret = device_property_read_u8_array(dwc->dev, propname, eps, min(num, count));
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
>  static int dwc3_gadget_init_endpoints(struct dwc3 *dwc, u8 total)
>  {
> +	const char			*propname = "snps,reserved-endpoints";
>  	u8				epnum;
> +	u8				reserved_eps[DWC3_ENDPOINTS_NUM];
> +	u8				count;
> +	u8				num;
> +	int				ret;
>  
>  	INIT_LIST_HEAD(&dwc->gadget->ep_list);
>  
> +	ret = dwc3_gadget_get_reserved_endpoints(dwc, propname,
> +						 reserved_eps, ARRAY_SIZE(reserved_eps));
> +	if (ret < 0) {
> +		dev_err(dwc->dev, "failed to read %s\n", propname);
> +		return ret;
> +	}
> +	count = ret;
> +
>  	for (epnum = 0; epnum < total; epnum++) {
> -		int			ret;
> +		for (num = 0; num < count; num++) {
> +			if (epnum == reserved_eps[num])
> +				break;
> +		}
> +		if (num < count)
> +			continue;
>  
>  		ret = dwc3_gadget_init_endpoint(dwc, epnum);
>  		if (ret)
> -- 
> 2.45.1.3035.g276e886db78b
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh