Skip to content

USB cherrypicks for NCS 3.0.0 #2672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Mar 28, 2025
Merged

Conversation

tmon-nordic
Copy link
Contributor

Bring in USB fixes, mostly in DWC2 driver.

tmon-nordic and others added 12 commits March 27, 2025 08:34
USB stack does not check api->lock() and api->unlock() return value and
all UDC drivers block without timeout in its lock() and unlock() api
implementations. There is no realistic way to handle lock() and unlock()
errors without making USB device stack API unnecessarily complex.

Remove the return type from lock() and unlock() to make it clear that
the functions must not fail.

Signed-off-by: Tomasz Moń <[email protected]>
(cherry picked from commit 23232e6)
DWC2 thread must acquire UDC mutex before accessing shared resources
(peripheral registers and software data structures). Whenever software
enqueues a buffer, the caller first obtains mutex, adds the buffer to
the list, posts event to wake up DWC2 thread and releases mutex. If DWC2
thread has higher priority than the task currently holding a mutex,
there will be two completely unnecessary task switches: DWC2 will switch
in, try to obtain mutex, and then the control will be returned to the
mutex holder.

Avoid the unnecessary task switches by locking scheduler prior to
obtaining the mutex and unlocking scheduler after releasing the mutex.

Signed-off-by: Tomasz Moń <[email protected]>
(cherry picked from commit 962a53e)
When the endpoint is disabled while the core is hibernated, there are
timeouts waiting for interrupts. It is not clear how the stack should
behave when class and/or application wants to disable the endpoint while
device is suspended. The problem was originally observed when the
endpoints were disabled as a result of usbd_disable() call.

Avoid the timeouts by modifying the backup values instead of the real
registers (which are not accessible when hibernated).

Split the 32-bit txf_set variable into two 16-bit variables (txf_set and
pending_tx_flush) because maximum number of TxFIFO instances is 16.
The txf_set variable is used as-is, while the pending_tx_flush is used
to keep track of TxFIFOs that have to be flushed on hibernation exit.

Signed-off-by: Tomasz Moń <[email protected]>
(cherry picked from commit 2be960a)
Wait for PHY clock before triggering START task to ensure overlapped
reset for PHY and DWC2 core.

Signed-off-by: Tomasz Moń <[email protected]>
(cherry picked from commit 437983e)
OUT endpoint 0 cannot be disabled and therefore the only way to forcibly
reclaim the buffer is to reset the core. The reset does not finish if
PHY clock is not running, but just triggering the reset seems to be
enough to be able to reclaim the buffer.

Signed-off-by: Tomasz Moń <[email protected]>
(cherry picked from commit 2395452)
There is no point in calling k_event_test() to determine what events are
posted and then passing that value to k_event_clear(). Simply pass
UINT32_MAX to k_event_clear() and use the return value to slightly
reduce overhead.

Signed-off-by: Tomasz Moń <[email protected]>
(cherry picked from commit 2aa26ad)
… necessary

SETUP data is unconditionally ACKed by the controller. Other DATA
packets sent to OUT control endpoint 0 (i.e. OUT Data Stage packets
and OUT Status Stage packet) are ACKed by the device only if the
endpoint was enabled with CNAK bit set.

In Buffer DMA mode controller will lock up in following scenario:
  * OUT EP0 is not enabled, e.g. OUT Status Stage has finished
  * Host starts Control Write transfer, i.e. sends SETUP DATA0 and
    device ACKs (regardless if endpoint is enabled or not)
  * host sends OUT Data Stage (OUT DATA1)
      - software enables endpoint to be able to receive next SETUP data
	while host is transmitting the OUT token. If CNAK bit is set
        alongside the EPENA bit, the device will ACK the OUT Data Stage.
        If CNAK bit is not set, the device will NAK the OUT Data Stage.

When the lockup occurs, from host perspective the OUT Data Stage packet
was successfully transmitted. This can result in host starting IN Status
Stage if there was only one OUT Data Stage packet. This in turn results
in device never getting the DOEPTINT0 SetUp interrupt. Besides just not
getting the SetUp interrupt, any subsequent control transfer won't be
noticed by device at all.

The lockup was first observed while stress testing. The host was issuing
endless sequence of Control Write, Control Read, Control Write, Control
Read, ... commands. When the controller did lock up in Buffer DMA mode,
from host perspective the device was timing out all control transfers.

Avoid the Buffer DMA lockup by setting CNAK bit only when necessary.

Signed-off-by: Tomasz Moń <[email protected]>
(cherry picked from commit 6e3f8d4)
Fix "warning: 'cdc_acm_send_notification' defined but not used"
when Kconfig option UART_USE_RUNTIME_CONFIGURE is not used and
properly handle enqueue error.

Signed-off-by: Johann Fischer <[email protected]>
(cherry picked from commit 7b34456)
Signed-off-by: Tomasz Moń <[email protected]>
… and enabled at boot

There are some boards and samples in the tree that use the CDC ACM UART
as the default serial backend, just like a real UART controller would.
The new device stack requires more detailed configuration than the old
one. In order to use the CDC ACM UART as the serial backend with the new
device stack in the same way as with the legacy stack, we need to
provide a solution to initialise and enable the CDC ACM UART at boot
time. We cannot use snippets as they do not support Kconfig files or
source code. Shields would be an option, but they cannot be used for
virtual devices such as the CDC ACM UART. The remaining solution is to
put the code and Kconfig file for it in the subsys directory.

Allow CDC ACM UART instance and USB device stack to be initialized and
enabled at boot time and to use it as serial backend for logging or
shell.

Signed-off-by: Johann Fischer <[email protected]>
(cherry picked from commit 7ffa620)
Signed-off-by: Tomasz Moń <[email protected]>
…e stack

Add sample to shows how to use the CDC ACM UART provided by the new
experimental USB device stack as a serial backend for the console.

Signed-off-by: Johann Fischer <[email protected]>
(cherry picked from commit be5e666)
Signed-off-by: Tomasz Moń <[email protected]>
Remove the configuration for the new USB device stack, as there is now a
separate sample that demonstrates how to use it with Kconfig option
CONFIG_CDC_ACM_SERIAL_INITIALIZE_AT_BOOT.

Signed-off-by: Johann Fischer <[email protected]>
(cherry picked from commit 5914131)
Signed-off-by: Tomasz Moń <[email protected]>
For many devices iSerialNumber is not required. The only device class
currently supported in Zephyr that requires iSerialNumber is MSC. If the
serial number is not available then iSerialNumber must be 0. Failure to
read the string descriptor for non-zero iSerialNumber fails USB Command
Verifier Chapter 9 tests.

When USBD_DESC_SERIAL_NUMBER_DEFINE() was used without CONFIG_HWINFO
then it did lead to runtime failure when requesting the string (thus
failing certification).

Fail USBD_DESC_SERIAL_NUMBER_DEFINE() at build-time if CONFIG_HWINFO is
not set to prevent the surprise at runtime. Only define serial number
descriptors if CONFIG_HWINFO is enabled.

Signed-off-by: Tomasz Moń <[email protected]>
(cherry picked from commit 7510970)
@tmon-nordic tmon-nordic merged commit 9442059 into nrfconnect:main Mar 28, 2025
20 checks passed
@tmon-nordic tmon-nordic deleted the usb-cherry-picks branch March 28, 2025 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants