Enabling PM for udc_mcux_ehci #90812

EmilioCBen · 2025-05-29T16:25:43Z

Enables PM support for USB devices that use udc_mcux_ehci.

tmon-nordic · 2025-05-30T08:56:31Z

drivers/usb/udc/udc_mcux_ehci.c

+		data->vbus_present = vbus_present;
+
+		/*
+		 * UDC will now unblock PM


What about USB suspend? VBUS is present but the device should enter as low power consumption as possible (with 2.5 mA suspend current limit for the whole device).

@tmon-nordic, When USB suspend comes in what does the Zephyr USB stack do? Does it keep the USB enabled?

USB suspend is entirely according to USB host. USB must be kept enabled and device must be able to resume within 10 ms.

Universal Serial Bus Specification Revision 2.0 9.2.6.2 Reset/Resume Recovery Time

After a port is reset or resumed, the USB System Software is expected to provide a “recovery” interval of
10 ms before the device attached to the port is expected to respond to data transfers. The device may ignore
any data transfers during the recovery interval.
After the end of the recovery interval (measured from the end of the reset or the end of the EOP at the end
of the resume signaling), the device must accept data transfers at any time.

Ok. Our current use case doesn't include this particular usage. But we are saving some internal information on a ticket if this use case does come up from our customers.

jfischer-no · 2025-05-30T10:03:14Z

drivers/usb/udc/udc_mcux_ehci.c

+#if defined(CONFIG_PM_POLICY_DEVICE_CONSTRAINTS)
+		pm_policy_device_power_lock_get(dev);
+#endif


No #if/#endif, that needs to be fixed in pm subsytem.

@jfischer-no, do you want this author to change the underlying pm subsystem here with this PR?

Fixed with PR #90938 please review :)

jfischer-no · 2025-05-30T10:41:26Z

samples/subsys/usb/console-next/boards/frdm_rw612.overlay

No trivial overlays.

How do we establish these nodes if they are not part of the core board/soc DTS?

jfischer-no · 2025-05-30T10:44:57Z

drivers/usb/udc/udc_mcux_ehci.c

+static int udc_mcux_pm_action(const struct device *dev, enum pm_device_action action)
+{
+	struct udc_mcux_data *priv = udc_get_private(dev);
+


Apart from whether PM usage makes sense or not, it should be like

if (IS_ENABLED(CONFIG_PM)) { return -ENOTSUP; }

if not possible, then it should be fixed in PM subsytem.

@jfischer-no This function needs to be wrapped with CONFIG_PM_DEVICE because of the way the PM macros work. At the bottom, PM_DEVICE_DT_INST_DEFINE() is setting things up IF the application has been defined for PM_DEVICE. If it hasn't, then the PM_DEVICE_DT_INST_DEFINE() is a null and to avoid "unused function" we need to wrap this overall function with the #if.

This is what pm_device_driver_init is for, it allows you to reuse the pm hook implementations if PM_DEVICE is n

If it hasn't, then the PM_DEVICE_DT_INST_DEFINE() is a null and to avoid "unused function" we need to wrap this overall function with the #if.

No.

diff --git a/drivers/usb/udc/udc_mcux_ehci.c b/drivers/usb/udc/udc_mcux_ehci.c index 1c8ddd12b58..5f0edc0b419 100644 --- a/drivers/usb/udc/udc_mcux_ehci.c +++ b/drivers/usb/udc/udc_mcux_ehci.c @@ -886,8 +886,7 @@ static int udc_mcux_driver_preinit(const struct device *dev) return udc_mcux_init_common(dev); } -#ifdef CONFIG_PM_DEVICE -static int udc_mcux_pm_action(const struct device *dev, enum pm_device_action action) +static inline int udc_mcux_pm_action(const struct device *dev, enum pm_device_action action) { struct udc_mcux_data *priv = udc_get_private(dev); @@ -909,7 +908,6 @@ static int udc_mcux_pm_action(const struct device *dev, enum pm_device_action ac } return 0; } -#endif /* CONFIG_PM_DEVICE */ static const struct udc_api udc_mcux_api = { .device_speed = udc_mcux_device_speed,

@jfischer-no No what? Use your words. You did not respond to the point that @dleach02 actually made. Are you claiming that what he said is not true?

Statement "If it hasn't, then the PM_DEVICE_DT_INST_DEFINE() is a null and to avoid "unused function" we need to wrap this overall function with the #if." is false.
Therefore my answer is 'No.', followed by a proof inside a diff that can be applied to the commit.

This diff is not solving the issue "in spirit" :) making it inline is like adding __maybe_unused :) I will try to review the PR, though, I know nothing about USB, other than what it's shorthand for :)

This diff is not solving the issue "in spirit" :) making it inline is like adding __maybe_unused :)

And allow the compiler to optimize and discard it in a similar way to if (IS_ENABLED(CONFIG_PM)) {return ...}. This is nothing new and is used in the project in many places.

jfischer-no · 2025-05-30T10:49:33Z

drivers/usb/udc/udc_mcux_ehci.c

@@ -525,10 +574,10 @@ usb_status_t USB_DeviceNotificationTrigger(void *handle, void *msg)
 	case kUSB_DeviceNotifyLPMSleep:
 		break;
 	case kUSB_DeviceNotifyDetach:
-		udc_submit_event(dev, UDC_EVT_VBUS_REMOVED, 0);
+		udc_mcux_change_state(dev, priv->enabled, 0);


udc_submit_event(dev, UDC_EVT_VBUS_REMOVED, 0); should be submitted here regardless of other states.

If we do the change I suggested in change_state() then this would occur in that function.

I did the change mentioned by @dleach02 on line 109

https://github.com/zephyrproject-rtos/zephyr/pull/90812/files#r2115659238 not addressed.

jfischer-no · 2025-05-30T10:49:45Z

drivers/usb/udc/udc_mcux_ehci.c

 		break;
 	case kUSB_DeviceNotifyAttach:
-		udc_submit_event(dev, UDC_EVT_VBUS_READY, 0);
+		udc_mcux_change_state(dev, priv->enabled, 1);


same as above

In case github gets busy, the change in question is being addressed on line 109

Same as above, not addressed.

jfischer-no · 2025-05-30T10:51:28Z

drivers/usb/udc/udc_mcux_ehci.c

+		data->vbus_present = true;
+
+		/*
+		 * Block PM when usb is active.


Block PM from doing what? What is "usb is active"?

Sorry for not being more clear, this was maybe a bad assumption on my part, but the only context this driver had from a usb interface point of view was rather the driver controller was enabled or if a VBUS signal is detected so I made the wrong assumption that this could only mean the controller is active, I have updated the comment.

jfischer-no · 2025-05-30T10:52:13Z

drivers/usb/udc/udc_mcux_ehci.c

@@ -92,6 +97,50 @@ static int udc_mcux_control(const struct device *dev, usb_device_control_type_t
 	return 0;
 }

+/* Helper function to keep track of the activity state for the udc */
+static void udc_mcux_change_state(const struct device *dev, bool enabled, bool vbus_present)


static void udc_mcux_change_state(const struct device *dev, const bool enabled, const bool vbus_present)

Understood, updated.

jfischer-no · 2025-05-30T10:54:41Z

drivers/usb/udc/udc_mcux_ehci.c

+		return;
+	}
+
+	if (vbus_present != data->vbus_present && enabled) {


That is wrong, must not depend on enabled, it is usually other way around.

@jfischer-no this recipe came from the udc_smartbond driver (see usb_state_change() ). It has the same logic.

I'm guessing the UDC driver is only sending the VBUS_READY if the higher stack has already did the enable. What is the expected USB UDC stack handling of VBUS_READY/REMOVED in general? I can see that our original code sent this event without regard to enable state so is that the expected behavior of UDC drivers?

I think this condition should just remove the "enable". Then the logic would send the VBUS_READY/REMOVED only when there is a change here. It would also debounce the signal (not that I expect that to occur) such that we wouldn't send back to back READY or REMOVED.

I am unclear at the error but I have updated the conditional to be only based on the vbus_present variable.

jfischer-no · 2025-05-30T10:55:55Z

drivers/usb/udc/udc_mcux_ehci.c

The author's goal for this patch is unclear.

Sorry if this was unclear, my only goal is to support the current Power Management Hook that is presented in Zephyr, adding nxp usb as a PM_DEVICE when CONFIG_PM=y and supporting the pm_action callback api.
At the moment, this driver does not have enough context to reinit any lost context about the zephyr USB stack when waking up from STANDBY so the driver will only go into a lower power mode when in Standby.

The point of the PR is to lock out the system power states that disable the USB peripheral when the USB is expected to be in use.

I think that eventually this should be handled by @bjarki-andreasen 's rework of PM from calling pm_device_runtime_get/put at the subsystem level, but for now we are trying to support the coherency of the "system managed device PM" paradigm that currently exists.

The point of the PR is to lock out the system power states that disable the USB peripheral when the USB is expected to be in use.

It does not explain why this needs to be done in the driver rather than in the application, where application can determine the state of the USB device controller and set PM accordingly.

I think that eventually this should be handled by @bjarki-andreasen 's rework of PM from calling pm_device_runtime_get/put at the subsystem level, but for now we are trying to support the coherency of the "system managed device PM" paradigm that currently exists.

It looks like we should wait until the PM issue has been resolved. This does not prevent you from using this patch in your downstream repository, though.

The point of the PR is to lock out the system power states that disable the USB peripheral when the USB is expected to be in use.

It does not explain why this needs to be done in the driver rather than in the application, where application can determine the state of the USB device controller and set PM accordingly.

Does it strictly need to be addressed by the driver, no, but also, the application doesn't strictly need to use a driver in the first place, does it? It doesn't need to even use the Zephyr ecosystem at all. It can just write bare metal code to control a USB without the stack, so why shouldn't it? So what is the problem with drivers being written robustly by contributing their knowledge to the system state?

I think that eventually this should be handled by @bjarki-andreasen 's rework of PM from calling pm_device_runtime_get/put at the subsystem level, but for now we are trying to support the coherency of the "system managed device PM" paradigm that currently exists.

It looks like we should wait until the PM issue has been resolved. This does not prevent you from using this patch in your downstream repository, though.

I don't agree that we either should or even need to wait. This code will be valid both before and after that PM reworking. What I said was that in my opinion, the subsystem level should contribute here, but I don't think that the driver can't also. Any component that has some information about the state of the system and the implications of that state, should utilize whichever mechanisms are available to it to contribute that knowledge to it's architectural connections.

Also, this fallback to "just use the patch in your downstream repository" is not a real thing. Nobody prefers to use a downstream repository. Everybody wants things in upstream releases. Maybe Nordic customers are used to it now, but it's not true universally.

Ain't noone happy to be forced to some downstream that's for sure :)

Does it strictly need to be addressed by the driver, no, but also, the application doesn't strictly need to use a driver in the first place, does it? It doesn't need to even use the Zephyr ecosystem at all. It can just write bare metal code to control a USB without the stack, so why shouldn't it? So what is the problem with drivers being written robustly by contributing their knowledge to the system state?

These changes are anything but robust. You have no idea how the UDC driver and USB device stack work, and you're just poking around. The UDC driver and the stack are already contributing all the bus states to the application. Only the USB application is aware of all the USB bus and USB device related states, such as battery charging. And as I said, the USB application should handle PM on its own in a generic way. Therefore, performing any PM on the driver level is pointless. Changes like these are not going anywhere in USB.

dleach02 · 2025-05-30T15:36:53Z

drivers/usb/udc/udc_mcux_ehci.c

+	return udc_mcux_init_common(dev);
+}
+
+#ifdef CONFIG_PM


@EmilioCBen This needs to be CONFIG_PM_DEVICE to be complete. See

zephyr/include/zephyr/pm/device.h

Line 296 in 5cffb8e

#ifdef CONFIG_PM_DEVICE

EmilioCBen · 2025-06-03T20:05:06Z

Depends on 90938

MarkWangChinese · 2025-06-04T07:11:03Z

drivers/usb/udc/udc_mcux_ehci.c

@@ -525,10 +564,10 @@ usb_status_t USB_DeviceNotificationTrigger(void *handle, void *msg)
 	case kUSB_DeviceNotifyLPMSleep:
 		break;
 	case kUSB_DeviceNotifyDetach:
-		udc_submit_event(dev, UDC_EVT_VBUS_REMOVED, 0);
+		udc_mcux_change_state(dev, priv->enabled, 0);


It may be in USB isr context, is it OK to call the pm related functions here?

I am unaware of the impact with PM and a device being in ISR.
I will state that it WILL be in ISR as these functions are only
called during the USB ISR. The device seems to work as intended
with or without PM from this PR, so nothing immediate sticks out
as a potential problem.

MarkWangChinese · 2025-06-04T07:15:08Z

drivers/usb/udc/udc_mcux_ehci.c

@@ -676,11 +715,19 @@ static int udc_mcux_set_address(const struct device *dev, const uint8_t addr)

 static int udc_mcux_enable(const struct device *dev)
 {
+	struct udc_mcux_data *priv = udc_get_private(dev);
+
+	udc_mcux_change_state(dev, 1, priv->vbus_present);


udc_mcux_enable may be called from application's tasks, and udc_mcux_change_state is called from the USB isr too, I think protection is needed to make sure udc_mcux_change_state is executed one by one and the enabled and vbus_present are updated rightly.

I have made this variables volatile so they are only updated one at a time.

MarkWangChinese · 2025-06-04T07:16:49Z

drivers/usb/udc/udc_mcux_ehci.c

+
+	udc_mcux_get_hal_driver_id(priv, config);
+	if (priv->controller_id == 0xFFu) {
+		return -ENOMEM;


udc_mcux_init_common has this part of codes too.

Good catch, updated.

MarkWangChinese · 2025-06-04T07:25:05Z

drivers/usb/udc/udc_mcux_ehci.c

+	case PM_DEVICE_ACTION_TURN_OFF:
+		break;
+	case PM_DEVICE_ACTION_TURN_ON:
+		udc_mcux_init_common(dev);


Just my concern from this part of codes: Why the PM_DEVICE_ACTION_TURN_OFF doesn't need any codes? From the name of the ON/OFF, it seems that it is more matched with the controller functions udc_mcux_init/udc_mcux_shutdown. And I think it is dangerous that enable/disable controller function without going through the stack because the stack's states are mismatched with controller states.

good point I can update the code to add udc_mcux_shutdown.

Enables PM support for USB devices that use udc_mcux_ehci. Signed-off-by: Emilio Benavente <[email protected]>

sonarqubecloud · 2025-06-12T16:02:34Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarQube Cloud

EmilioCBen · 2025-06-12T19:50:57Z

Removed DNM Label, dependent PR #90938 has merged.

decsny · 2025-06-16T17:50:05Z

@jfischer-no please return to this PR and check if your review was addressed

no response

jfischer-no · 2025-06-25T10:43:32Z

drivers/usb/udc/udc_mcux_ehci.c

@@ -525,10 +574,10 @@ usb_status_t USB_DeviceNotificationTrigger(void *handle, void *msg)
 	case kUSB_DeviceNotifyLPMSleep:
 		break;
 	case kUSB_DeviceNotifyDetach:
-		udc_submit_event(dev, UDC_EVT_VBUS_REMOVED, 0);
+		udc_mcux_change_state(dev, priv->enabled, 0);


https://github.com/zephyrproject-rtos/zephyr/pull/90812/files#r2115659238 not addressed.

jfischer-no · 2025-06-25T10:44:09Z

drivers/usb/udc/udc_mcux_ehci.c

 		break;
 	case kUSB_DeviceNotifyAttach:
-		udc_submit_event(dev, UDC_EVT_VBUS_READY, 0);
+		udc_mcux_change_state(dev, priv->enabled, 1);


Same as above, not addressed.

jfischer-no · 2025-06-25T10:51:52Z

drivers/usb/udc/udc_mcux_ehci.c

+	volatile bool enabled;
+	volatile bool vbus_present;


What is the intention behind using volatile here? Is it an LLM suggestion? Are there any side effects that the compiler is not aware of?

enabled and vbus_present are being used as atomics, accessed from multiple contexts, volatile lets the compiler know that the value can be changed by another context at any time, so it has to be read every time its used, the compiler can't cache the value. For example:

volatile bool foo = false; volatile bool bar = true; if (foo) { do_foo(); return; } /* ISR happens here and changes the value of foo to true */ /* * compiler will "likely" not check foo again here, it will optimize out this branch entirely since * since foo had to be false to get here, so foo && bar is not possible without volatile */ if (foo && bar) { do_foobar() }

without the volatile keyword, the compiler would not check foo again after the ISR, it will optimize the code to just check foo once, and never check bar :)

now, this should optimally be changed to using an atomic_t, but for archs where you know a reading bool is an atomic op, its not neccesary :)

jfischer-no · 2025-06-25T10:55:46Z

samples/subsys/usb/console-next/boards/frdm_rw612.overlay

+/*
+ * Copyright 2025 NXP
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/ {
+	chosen {
+		zephyr,console = &cdc_acm_uart0;
+	};
+};
+
+&zephyr_udc0 {
+	cdc_acm_uart0: cdc_acm_uart0 {
+		compatible = "zephyr,cdc-acm-uart";
+	};
+};


Again, no trivial overlays. This sample must not have any board overlays either.

You have not answered the question asked to you above of how this is supposed to be done otherwise, since this configuration is specific to this sample. For this board, zephyr,console is not normally this CDC ACM USB. What would you have the author do, specifically, besides remove the overlay, because that alone obviously does not align with the goal of the patch.

How do we establish these nodes if they are not part of the core board/soc DTS?

Just as we have been doing for a long time. Have any of you thought about how this sample is used with other boards?

Upon my inspection, it looks like, the sample uses DEVICE_DT_GET_ONE, instead of zephyr,console, and uses a special nodelabel for how to define which node the cdc acm uart will go on with app.overlay . So @EmilioCBen you can make that change. However, @jfischer-no your comments are completely not helpful or collaborative. I am obviously not actually the one working on this PR but even if I was, I would be annoyed at how you are leaving vague riddles as reviews instead of just saying what you know that the author didn't and what exactly you want changed.

Upon my inspection, it looks like, the sample uses DEVICE_DT_GET_ONE, instead of zephyr,console, and uses a special nodelabel for how to define which node the cdc acm uart will go on with app.overlay .

If you really inspected it, you would discover that it uses DEVICE_DT_GET(DT_CHOSEN())

zephyr/samples/subsys/usb/console-next/src/main.c

Line 16 in c906d4a

const struct device *const dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_console));

It is also obvious that you don't even need to look at the main.c, the overlay provides all the information you need.

However, @jfischer-no your comments are completely not helpful or collaborative. I am obviously not actually the one working on this PR but even if I was, I would be annoyed at how you are leaving vague riddles as reviews instead of just saying what you know that the author didn't and what exactly you want changed.

It is the responsibility of the PR author to familiarize themselves with the code they are changing. The same applies to colleagues passing by. It is irresponsible and reckless to poke around, waste other people's time, and expect everything to be explained. It is quite strange that none of you looked at the sample and saw that the app.overlay file contains the same content as the board overlay file, or questioned how that works for the other boards. So please hold back with your accusations, I do not accept them.

Finally, it is not too much to ask that people read the fancy manual
https://docs.zephyrproject.org/latest/build/dts/howtos.html#set-devicetree-overlays
https://docs.zephyrproject.org/latest/connectivity/usb/device/usb_device.html#console-over-cdc-acm-uart

I see, I guess I looked at the old USB stack sample. Again, not working on the PR myself, but I am looking at this PR from an outside perspective and seeing your comments on it which are clearly not giving any indication of what you know that the PR author might have missed, that you are basically just making them play a game show to figure out what you mean. And I don't need to be an expert on USB or even know what a computer is to see when a person is not "playing nicely in the sandbox".

You might be a maintainer, but you seem to be mistaken in thinking that means you have a permit to be a jackass. In fact, you don't have less responsibility to stewarding the upstream, you have more. From being a maintainer, you become an assignee on PRs in your area, such as this one, which means that you are responsible to moderate this thread and resolve the disputes among the authors, maintainers (including yourself), and reviewers in order to avoid having to escalate. I don't see how you can be trusted to be moderating if you are the one causing the issues by intentionally making it a struggle to even understand what you are saying out of spite for when somebody makes a mistake in your eyes on their PR. That is just immature. https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html

Either way. I agree with you to the extent that a PR author obviously should be responsible for making a good faith effort to properly know what they are doing when they make a PR submitting patches. However, that does not equate to meaning that everybody has to be perfect and omniscient or else F off and ignore them. At the end of the day, humans make mistakes, and don't know what they don't know they don't know. We aren't like the AI which you apparently so vehemently hate (although ironically, I suspect you would get along better with) which seem to have unlimited amounts of knowledge and time.

Lastly, simply just read https://docs.zephyrproject.org/latest/contribute/reviewer_expectations.html to understand that, regardless of your opinion, you have to follow these rules of what is expected of you if you are going to review PRs in the upstream. And I think you have either broken or neglected quite a few of them on this thread already.

Just came here to echo, on behalf of the project's Code of Conduct team, what @decsny very articulately said in his comment just above. Please do try to keep your feedback and request for changes clear and actionable.

jfischer-no · 2025-06-25T11:04:01Z

drivers/usb/udc/udc_mcux_ehci.c

@@ -676,11 +716,19 @@ static int udc_mcux_set_address(const struct device *dev, const uint8_t addr)

 static int udc_mcux_enable(const struct device *dev)
 {
+	struct udc_mcux_data *priv = udc_get_private(dev);
+
+	udc_mcux_change_state(dev, 1, priv->vbus_present);


udc_mcux_enable() should not emit UDC_EVT_VBUS_READY because there is no reason to do it at this point.

The point of the way this is called is that udc_mcux_enable is not trying to affect the state tracking of whether vbus is present. Explain how it could be, otherwise your comment here doesn't make sense to me.

bjarki-andreasen

ok, some assumptions from my side:

when udc_init, or any other udc API as called on this device, it must be able to detect power state of the bus, which means it must be ACTIVE from the perspective of device PM.
Detecting power mode works in deeper sleep states than "operational" states
Disabling states in devicetree refer to disabling operation currently, not vbus detection

Summarized solution:

add more sets of system states required for various operations of the hardware, and block these based on UDC API calls/UDC device state, don't rely on zephyr,disabling-states.

Proposed solution in steps:

In the devicetree, update disabling states to only include states which prevent vbus detection
Add a new, device driver specific set of system states to the devicetree (you are allowed to do that), call those op-disabling-states or something, and create your own set of struct pm_state_constraint to manually apply using pm_policy_state_lock_get from your driver as it is requested to be able to do something.
make udc_mcux_driver_preinit the init function of the device driver, call pm_device_driver_init() as the last call from the device driver init function
Implement the following behavior in the udc_mcux_pm_action hook
- TURN_ON -> make sure device is in reset state
- RESUME -> do nothing here, udc_init() will be called at some point after RESUME, there you should block the states
  which prevent vbus detection.
- SUSPEND -> do nothing, udc_shutdown() should have been called before getting here.
- TURN_OFF -> make sure device is ready for being powered off
now, what about the op-disabling system states? When udc_enable() is called for example, block the system states which would block the requirements of this UDC state, namely "Enable powered USB device controller and allow host to recognize and enumerate the device.", using pm_policy_state_lock_get(). unblock these sleep states again when udc_disable() is called, still blocking the sleep state which allows vbus detection

Naively and optimistically, that should be it :)

Last step would be for USB stack to call pm_device_runtime_get() on UDC device before calling udc_init() and put after calling udc_shutdown()

bjarki-andreasen · 2025-06-25T17:22:27Z

drivers/usb/udc/udc_mcux_ehci.c

+	volatile bool enabled;
+	volatile bool vbus_present;


enabled and vbus_present are being used as atomics, accessed from multiple contexts, volatile lets the compiler know that the value can be changed by another context at any time, so it has to be read every time its used, the compiler can't cache the value. For example:

volatile bool foo = false; volatile bool bar = true; if (foo) { do_foo(); return; } /* ISR happens here and changes the value of foo to true */ /* * compiler will "likely" not check foo again here, it will optimize out this branch entirely since * since foo had to be false to get here, so foo && bar is not possible without volatile */ if (foo && bar) { do_foobar() }

without the volatile keyword, the compiler would not check foo again after the ISR, it will optimize the code to just check foo once, and never check bar :)

now, this should optimally be changed to using an atomic_t, but for archs where you know a reading bool is an atomic op, its not neccesary :)

bjarki-andreasen · 2025-06-25T17:28:38Z

drivers/usb/udc/udc_mcux_ehci.c

+static int udc_mcux_driver_preinit(const struct device *dev)
+{
+	struct udc_data *data = dev->data;
+	struct udc_mcux_data *priv = data->priv;
+
+	k_mutex_init(&data->mutex);
+	k_fifo_init(&priv->fifo);
+	k_work_init(&priv->work, udc_mcux_work_handler);
+	return udc_mcux_init_common(dev);
+}


preinit here is what the device init function is supposed to do :)

jfischer-no · 2025-06-25T22:17:31Z

@kartben Stop dismissing my reviews of the area I maintain! It is none of your business!

EmilioCBen added this to the v4.2.0 milestone May 29, 2025

EmilioCBen requested review from mmahadevan108 and decsny May 29, 2025 16:25

EmilioCBen added area: Power Management area: USB Universal Serial Bus platform: NXP NXP labels May 29, 2025

github-actions bot added platform: NXP Drivers NXP Semiconductors, drivers area: Samples Samples labels May 29, 2025

github-actions bot requested review from bperseghetti, butok, DerekSnell, dleach02, jfischer-no, kartben, manuargue, MarkWangChinese, nashif, PetervdPerk-NXP and tmon-nordic May 29, 2025 16:26

github-actions bot assigned jfischer-no May 29, 2025

EmilioCBen force-pushed the feature/rw61x_usb_pm_support branch from 4e46e52 to 146ddc5 Compare May 29, 2025 16:42

tmon-nordic requested a review from bjarki-andreasen May 30, 2025 08:53

tmon-nordic reviewed May 30, 2025

View reviewed changes

jfischer-no previously requested changes May 30, 2025

View reviewed changes

dleach02 reviewed May 30, 2025

View reviewed changes

EmilioCBen force-pushed the feature/rw61x_usb_pm_support branch from 146ddc5 to 1d9a6df Compare June 3, 2025 20:03

EmilioCBen added the DNM This PR should not be merged (Do Not Merge) label Jun 3, 2025

MarkWangChinese reviewed Jun 4, 2025

View reviewed changes

EmilioCBen force-pushed the feature/rw61x_usb_pm_support branch from 1d9a6df to 641b3e0 Compare June 4, 2025 17:38

EmilioCBen force-pushed the feature/rw61x_usb_pm_support branch 2 times, most recently from e8a1deb to d8e1ab8 Compare June 4, 2025 17:56

drivers: usb: udc: Enabling PM to udc_mcux_ehci

5e355ab

Enables PM support for USB devices that use udc_mcux_ehci. Signed-off-by: Emilio Benavente <[email protected]>

EmilioCBen force-pushed the feature/rw61x_usb_pm_support branch from d8e1ab8 to 5e355ab Compare June 12, 2025 15:59

EmilioCBen removed the DNM This PR should not be merged (Do Not Merge) label Jun 12, 2025

kartben added the DNM This PR should not be merged (Do Not Merge) label Jun 23, 2025

jfischer-no requested changes Jun 25, 2025

View reviewed changes

bjarki-andreasen reviewed Jun 25, 2025

View reviewed changes

Enabling PM for udc_mcux_ehci #90812

Are you sure you want to change the base?

Enabling PM for udc_mcux_ehci #90812

Conversation

EmilioCBen commented May 29, 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!

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!

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

dleach02 May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

decsny Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen Jun 25, 2025 •

edited

Loading

dleach02 May 30, 2025 •

edited

Loading

decsny Jun 3, 2025 •

edited

Loading