drivers: uhc: use correct endpoint type and interval #99578

AidenHu · 2025-11-18T12:07:03Z

For usb xfer, set endpoint type and interval by the selected endpoint desc.

MarkWangChinese · 2025-11-18T12:37:51Z

drivers/usb/uhc/uhc_mcux_common.c

 		if (priv->mcux_eps[i] != NULL &&
 		    priv->mcux_eps[i]->endpointAddress == USB_EP_GET_IDX(xfer->ep) &&
 		    priv->mcux_eps[i]->direction == direction &&
+		    priv->mcux_eps[i]->pipeType == xfer->type &&


Put this line in the follow TODO codes. This part wants to find the initialized endpoint number on the udev, and different endpoints use different endpoint number on the same udev.

tmon-nordic · 2025-11-18T13:55:48Z

drivers/usb/uhc/uhc_mcux_common.c

+	if (mcux_ep != NULL && mcux_ep->pipeType == xfer->type &&
 	    (mcux_ep->maxPacketSize != xfer->mps ||
-	     mcux_ep->interval != xfer->interval)) {
+	     mcux_ep->interval != (uint16_t)(1UL << (xfer->interval - 1U)))) {


I don't know what mcux_ep expects in its interval field, but from USB specification point of view bInterval is not always used as exponent. For full-/low-speed interrupt endpoints, bInterval is value from 1 to 255.

Sorry for the mistake, I have done based on the spec.

MarkWangChinese · 2025-11-19T04:12:56Z

drivers/usb/uhc/uhc_common.c

 	}

 	if (ep_idx == 0) {
 		mps = udev->dev_desc.bMaxPacketSize0;


Add:

interval = 0; type = USB_EP_TYPE_CONTROL;

Add:

interval = 0; type = USB_EP_TYPE_CONTROL;

fixed.

MarkWangChinese · 2025-11-19T05:46:55Z

drivers/usb/uhc/uhc_mcux_common.c

-	/* TODO: need to check endpoint type too */
-	if (mcux_ep != NULL &&
+	if (xfer->type == USB_EP_TYPE_INTERRUPT) {
+		if (xfer->udev->speed == USB_SPEED_SPEED_HS) {
+			actual_interval = (uint16_t)(1UL << (xfer->interval - 1U));
+		} else {
+			actual_interval = uhc_get_power_of_2_floor(xfer->interval);
+		}
+	} else if (xfer->type == USB_EP_TYPE_ISO) {
+		actual_interval = (uint16_t)(1UL << (xfer->interval - 1U));
+	} else {
+		actual_interval = xfer->interval;
+	}


Suggest to add one struct member in struct uhc_mcux_data as follow. mcux_ep->interval is different with the interval value of endpoint descriptor.

usb_host_pipe_t *mcux_eps[USB_HOST_CONFIG_MAX_PIPES]; uint16_t mcux_eps_interval[USB_HOST_CONFIG_MAX_PIPES];

Suggest to add one struct member in struct uhc_mcux_data as follow. mcux_ep->interval is different with the interval value of endpoint descriptor.

usb_host_pipe_t *mcux_eps[USB_HOST_CONFIG_MAX_PIPES]; uint16_t mcux_eps_interval[USB_HOST_CONFIG_MAX_PIPES];

applied.

For usb xfer, set endpoint type and interval by the selected endpoint desc. Signed-off-by: Aiden Hu <[email protected]>

MarkWangChinese

The second commit body description needs updating.

MarkWangChinese · 2025-11-20T01:54:45Z

drivers/usb/uhc/uhc_mcux_common.c

+			priv->mcux_eps[i]->endpointAddress == USB_EP_GET_IDX(xfer->ep) &&
+			priv->mcux_eps[i]->direction == direction &&
+			priv->mcux_eps[i]->deviceHandle == xfer->udev) {


Incorrect indentation

MarkWangChinese · 2025-11-20T01:54:57Z

drivers/usb/uhc/uhc_mcux_common.c

-	    priv->mcux_eps_interval[i] != xfer->interval)) {
+		(mcux_ep->maxPacketSize != xfer->mps ||
+		priv->mcux_eps_interval[i] != xfer->interval)) {
 		/* re-initialize the ep */


Incorrect indentation

josuah · 2025-11-21T11:07:28Z

drivers/usb/uhc/uhc_mcux_common.c

-	/* TODO: need right way to implement it. */
-	if (pipe_init.endpointAddress == 0) {
-		pipe_init.pipeType = USB_ENDPOINT_CONTROL;
-	} else {
-		pipe_init.pipeType = USB_ENDPOINT_BULK;
-	}
+	pipe_init.pipeType = xfer->type;


If wanting to also convert the other drivers, here is where it is on MAX3421E:

zephyr/drivers/usb/uhc/uhc_max3421e.c

Lines 401 to 409 in 2baee8a

/*

* TODO: currently we only support control transfers and

* treat all others as bulk.

*/

if (USB_EP_GET_IDX(priv->last_xfer->ep) == 0) {

return max3421e_xfer_control(dev, priv->last_xfer, hrsl);

}

return max3421e_xfer_bulk(dev, priv->last_xfer, hrsl);

But I do not have that hardware to test.

mcux_eps_interval is added as the new member of uhc_mcux_data. It is used to save endpoint's original interval value and can be compared with xfer->interval. Signed-off-by: Aiden Hu <[email protected]>

maxPacketSize and numberPerUframe of pipe should be set considering additional transactions. Signed-off-by: Aiden Hu <[email protected]>

AidenHu · 2025-11-21T11:14:26Z

The second commit body description needs updating.

updated.

sonarqubecloud · 2025-11-21T11:38:55Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

zephyrbot added area: USB Universal Serial Bus platform: NXP NXP labels Nov 18, 2025

zephyrbot requested review from MarkWangChinese, jfischer-no, josuah, mmahadevan108 and tmon-nordic November 18, 2025 12:08

zephyrbot assigned jfischer-no Nov 18, 2025

MarkWangChinese reviewed Nov 18, 2025

View reviewed changes

AidenHu force-pushed the use-correct-type-and-interval-for-usb-xfer branch from ca1495b to e49a9cf Compare November 18, 2025 12:46

tmon-nordic reviewed Nov 18, 2025

View reviewed changes

AidenHu force-pushed the use-correct-type-and-interval-for-usb-xfer branch 2 times, most recently from 1492868 to 510bf5d Compare November 19, 2025 00:51

MarkWangChinese reviewed Nov 19, 2025

View reviewed changes

drivers: uhc: use correct endpoint type and interval

e8b9af5

For usb xfer, set endpoint type and interval by the selected endpoint desc. Signed-off-by: Aiden Hu <[email protected]>

AidenHu force-pushed the use-correct-type-and-interval-for-usb-xfer branch from 510bf5d to 787ff63 Compare November 19, 2025 11:35

MarkWangChinese reviewed Nov 20, 2025

View reviewed changes

AidenHu force-pushed the use-correct-type-and-interval-for-usb-xfer branch 2 times, most recently from 817a673 to e91426d Compare November 20, 2025 07:01

AidenHu mentioned this pull request Nov 20, 2025

Usb host hub class enablement #99735

Draft

josuah mentioned this pull request Nov 21, 2025

USB Host: integrate class API [4: aggregated] #99775

Draft

josuah reviewed Nov 21, 2025

View reviewed changes

AidenHu added 2 commits November 21, 2025 19:07

drivers: uhc: add mcux_eps_interval to save ep interval

a23284e

mcux_eps_interval is added as the new member of uhc_mcux_data. It is used to save endpoint's original interval value and can be compared with xfer->interval. Signed-off-by: Aiden Hu <[email protected]>

drivers: uhc: set right value for pipe by xfer's mps

4a20cea

maxPacketSize and numberPerUframe of pipe should be set considering additional transactions. Signed-off-by: Aiden Hu <[email protected]>

AidenHu force-pushed the use-correct-type-and-interval-for-usb-xfer branch from e91426d to 4a20cea Compare November 21, 2025 11:12

MarkWangChinese approved these changes Nov 24, 2025

View reviewed changes

	/*
	* TODO: currently we only support control transfers and
	* treat all others as bulk.
	*/
	if (USB_EP_GET_IDX(priv->last_xfer->ep) == 0) {
	return max3421e_xfer_control(dev, priv->last_xfer, hrsl);
	}

	return max3421e_xfer_bulk(dev, priv->last_xfer, hrsl);

drivers: uhc: use correct endpoint type and interval #99578

Are you sure you want to change the base?

drivers: uhc: use correct endpoint type and interval #99578

Conversation

AidenHu commented Nov 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarkWangChinese left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AidenHu commented Nov 21, 2025

Uh oh!

sonarqubecloud bot commented Nov 21, 2025

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants