This change implements the following, necessary for remote wakeup to be
supported:
- implement usb_dc_wakeup_request() call to trigger remote wakeup
- implement interrupts to detect supsended/resumed state
- implement extra logic to simulate resumed state when the resume
is a result of remote wakeup. In this case the rp2040 chip doesn't
send a USB_INTR_DEV_RESUME_FROM_HOST interrupt, or any other
interrupts when the resume condition is detected to be extended
by the upstream port, so we need to simulate this event when we
see activity on the bus.
Signed-off-by: Purdea Andrei <andrei@purdea.ro>
Description of the bad behaviour before this change:
The arming of the control EP0_OUT endpoint was not kept under
control. It could happen that the EP0_OUT endpoint was left
armed, after the completion of a complete control transfer.
It is clear that the intention was to NOT keep EP0_OUT constantly
armed while idle, because usb_dc_ep_enable() doesn't arm it,
and the intention was for when usb_dc_ep_read() is called to
collect the Setup-Stage 8-byte data, that is when EP0_OUT is armed,
and before this call is performed, the host will keep getting NAKs
for the Data-Stage of the to_device control transfer.
This happens correctly on the first to_device control transfer with
wLength > 0. However, because usb_dc_ep_read_continue() indiscriminately
re-arms all OUT endpoints, in the case of to_device control transfers
with wLength > 0, on the Data-Stage, the endpoint is also re-armed,
which is wrong, because then the endpoint will be left armed after
the control transfer is over.
In this case when a new to_device control transfer starts, the
Data-Stage will be accepted on the first try. This would still
have worked without a failure if the Setup-Stage would have been
processed immediately, but because we process everything in a work
queue at a later time, when the Setup-Stage associated 8-byte data
buffer is read both the Setup-Stage and Data-Stage have arrived.
At the end of handling the Setup-Stage we try to re-arm the EP0_OUT,
which already contains data, thereby corrupting the received length
portion of the buf_ctl register. (Obviously other fields are changed
too, but the length field is the one that first causes chaos, cause
it's written to the maximum, which is 64.) The above mentioned Data-Stage
already has a message in its workqueue for it to be processed, but
it is picked up only after the length field has been corrupted.
Because of this usb_dc_ep_read() thinks there is more data in the buffer
than there really is, and everything becomes de-synchronized, with
later reads accessing uninitialized parts of the buffer.
This sounds like a fundamental failure, that should make it impossible
to operate USB, however the reason this behaviour doesn't make it
impossible to enumerate the device is that this only affects
to_device control transfers with wLength > 0, and during enumeration
there are not many of those happening.
When enumerating a HID keyboard, there is only _one_ of those
happening, and it is the initial setting of the lock light led status.
And that first one succeeds because it's the first one. (However, later
lock light setting control transfers can cause problems, which is how
this problem was encountered.)
The solution in this commit is to keep better control over when EP0_OUT
is armed. This forces the Data-Stage to arrive later (the host will keep
re-trying), and that way the corruption of the buffer control register
is avoided.
Summary of the changes:
- Rework the logic around deciding wether to re-arm the out endpoint
after a read. For non-0 endpoint the previous behaviour is kept,
however for EP0 it is only re-armed if more OUT transactions are
expected for that SETUP transfer (be it data-stage or status-stage)
- Force un-arm the EP0_OUT endpoint in case a stall condition is observed.
- When a setup transfer is received check if EP0_OUT is already armed.
If armed then log a warning message, and force-disarm it.
- When a setup req interrupt fires, don't immediately force the next
read to get it, instead, it will be read only after a setup message
is extracted from the message queue.
- When a setup packet is received abort any unfinished previous control
transfers:
- cancel any data buffers given to the EP0_IN endpoint
- drop any new ep0_in writes that are attempted before this newest
setup packet's associated message is extracted from the message
queue.
- In the ISR, check buffer interrupts before setup req interrupts.
This is to make sure that the final 0-length status message from the
previous setup packet is consumed before the new setup packet.
(this is the only case now when both interrupts could be seen as
having fired by the time the interrupt handler routine executes.
Signed-off-by: Purdea Andrei <andrei@purdea.ro>
Co-authored-by: Johann Fischer
I believe this comment and condition may have found its way into the
rp2040 driver from other drivers where it makes more sense. For
example for the stm32 driver performing a read on the EP0_IN endpoint
turns it silently into a read on the EP0_OUT endpoint. As far as I can
tell, this really was only used to consume 0-length Status-Stages of
to_host control transfer in the other drivers.
Note that usb_dc_ep_start_read() is never called in an IN endpoint
in the rp2040 driver, and furthermore, even if it would have been
called like that, the current implementation would not do the silent
change into actually performing a read on the EP0_OUT endpoint instead,
so the condition and comment is just wrong.
Note that 0-length Status-Stage of to_host control transfers is
currently consumed in this driver by usb_dc_ep_read_continue().
Signed-off-by: Purdea Andrei <andrei@purdea.ro>
The data stage of Control transfers that are sent from Host to Device, can
be made out of multiple OUT transactions, if the amount of data to be
transmitted is larger than the endpoint size. When this happens, the DATA
pid should be toggling. The USB Device driver of the pico must correctly
prime the EP0_OUT buffer with the correct data PID, otherwise the hardware
will reject the received transaction.
Before this change the driver used to always prime EP0_OUT with a DATA1
pid.
After this change the driver only uses DATA1 pid after a setup transaction,
and then toggles the pid for each transaction.
Signed-off-by: Purdea Andrei <andrei@purdea.ro>
This driver enables a number of interrupts it does not attempt to handle.
This results in "unhandled IRQ: 0x...." messages being printed, and the
interrupt handler retriggers immediately again, and this happens again
and again forver, because nothing ends up clearing the interrupt.
This change implements very limited handling of these interrupts. A custom
warning is logged, and the interrupt is cleared.
This change does not imply that doing this is sufficient. More changes may
need to be implemented to more gracefully re-start transactions or
re-arm some endpoints, but this is one step in the right direction,
and at least the OS doesn't freeze up.
Signed-off-by: Purdea Andrei <andrei@purdea.ro>
1. Do not copy over the data to the local buffer,
it can be directly sent to the controller. The
cache is flushed before calling the HAL send
function. Also do not allocate a buffer from
the heap pool for the write operation.
2. Remove a length check as this is handled by
the HAL driver.
Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>
In order to clean up clock related code, replace static build time
clock configuration code by a runtime check.
Since I'm not able to guarantee this check will provide a correct
verdict in 100% of the cases (I'm thinking to ULPI or exotic configurations
like F7 OTG-HS), add a config option to disable it if needed.
It also doesn't apply on F1/F3 series. Keep the build time check for now.
Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
This change doesn't consider STM32F1 series.
There are 5 categories of STM32 regarding USB domain clock:
A] USB domain clocks directly driven by CK48 clocks
B] USB domain clocks which can select either CK48 or an alternative clock
C] USB domain clocks which can select between MSI or PLL
D] USB domain clocks relying statically on a PLL output
E] USB domain clocks which can be set on 2 different PLL ouputs
For case A], there is nothing to be done. Clk48 was already configured
in a previous change (51c7643295b02ba26ddcd0fd915484114db7101c).
For case B] and C] USB domain clocks will need to be configured in
each board.
For case D] and E] relying on existing dts configuration. Though
there is no easy build time chekc that can be done.
Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Use STM32_DT_INST_CLOCKS() to access usb device clock information.
This will help accessing domain clock information from dt nodes.
Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Remove this code to clean up the driver.
Clock activation is done from device tree configuration in clock_control
driver.
Users are informed by warning about missing configuration.
Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Vbus detection is required by self-powered, with external non-USB
supply, devices to have an operational USB peripheral. The voltage
sensing, when enabled, is performed via a designated pin (PA9 is
the most common).
For greater details see section 2.6, ST application note AN4879.
Signed-off-by: Raul Pando <raul.pando@bytegrity.co.uk>
Now that USB clock prescaler is done is clock_control driver.
Remove similar part in USB driver.
Note that behavior is different: it is now up to the user to provide
the proper configuration.
Add a warning during the transition period.
Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Implement usb_dc_wakeup_request for STM32 USB DC and default to enable
remote wakeup functionality when the drivers is selected.
This allows the device to wake the host up by calling
usb_wakeup_request().
Signed-off-by: Fabio Baltieri <fabio.baltieri@gmail.com>
The USB_DC_STM32 help message started to miss some
STM32 MCU families. Overtime, the message will
get bigger if we continue to list family names.
Removed family names to simplify the message and
avoid periodic modifications.
Signed-off-by: Georgij Cernysiov <geo.cgv@gmail.com>
The HSI48 is enabled by clock control driver.
It is no more done by each driver that requires this clock
However when using rng or sdmmc or bluetooth/ipm or usb,
the HSI48 clock must be present in the DTS.
Add a warning for this particular check but keep the deprecated
HSI48 clock enable : keeping for legacy but to remove later.
Signed-off-by: Francois Ramu <francois.ramu@st.com>
It is possible for nrfx usbd driver to send NRFX_USBBD_EP_ABORTED during
endpoint disable. The event is passed to event handler registered with
nrfx_usbd_init(). The nrfx_usbd_uninit() removes the registered event
handler, replacing it with NULL. If any event is sent after uninit, the
NULL pointer is executed and device crashes.
Do not uninit nrfx usbd driver on detach so it is possible for the
usb_disable() to disable all the endpoints.
Fixes: 460ca86527 ("drivers: usb_dc_nrfx: Always allow endpoint disable")
Signed-off-by: Tomasz Moń <tomasz.mon@nordicsemi.no>
There is no point in allowing endpoint disable only when device is
attached and ready. Remove the pointless check as it is actually harmful
and prevents endpoints disable on USB stack disable.
Signed-off-by: Tomasz Moń <tomasz.mon@nordicsemi.no>
In usb_dc_ep_disable() mark an endpoint as unoccupied in addition to
being disabled. This allows the endpoint to get enabled properly
with a subsequent call to usb_dc_ep_enable()
See: zephyrproject-rtos/zephyr#51685
Signed-off-by: Milind Paranjpe <mparanjpe@yahoo.com>
The buf_status register is 32 bit wide but was saved in a
uint8_t. This caused some buffers never to be handled which
results in the pico getting stuck in the interrupt handler.
Signed-off-by: Lukas Gehreke <lk.gehreke@gmail.com>
The USB device subsystem driver's set_interface() function calls
usb_dc_ep_configure() followed by usb_dc_ep_enable(). When
switching between alternate settings of a configuration's
interface, set_endpoint() can be followed by reset_endpoint() on
an endpoint. Some time later, set_endpoint() can be called again
on the same endpoint. This results in the HAL's
kUSB_DeviceControlEndpointInit function being called twice in a
row, which causes a memory allocation error. A simple solution is
to call the HAL's kUSB_DeviceControlEndpointDeinit function
before calling kUSB_DeviceControlEndpointInit. This overcomes the
memory allocation error.
Signed-off-by: Milind Paranjpe <mparanjpe@yahoo.com>
Change automated searching for files using "IRQ_CONNECT()" API not
including <zephyr/irq.h>.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
SAM V71 Xplained Ultra failed to re-enumerate on the bus after device
detach-attach sequence. The device was recognized by the host, but it
was not responding to control transfers, i.e. it was not acknowledging
SETUP data. Make device acknowledge SETUP data by reconfiguring EP0
when necessary.
Signed-off-by: Tomasz Moń <tomasz.mon@nordicsemi.no>
Device physically detaches when the DETACH bit is set. Fix the
assignment to actually set the bit instead of clearing it.
Signed-off-by: Tomasz Moń <tomasz.mon@nordicsemi.no>
Sometimes it is desired for the USB device to detach and reattach itself
to the host. USB device controller driver interface supports detach with
usb_dc_detach() function. Subsequently the device can be reattached to
the bus by calling usb_dc_attach().
USB device stack sets the control transfer callbacks in usb_enable().
Nordic usb_dc_detach() implementation inadvertedly overwrote registered
callbacks with zeroes by clearing endpoint configuration structure. This
lead to a NULL pointer dereference during enumeration after device has
reattached inself to the host.
Preserve the callbacks by removing the eps_ctx_uninit() function.
Endpoints runtime data is initialized in usb_dc_attach() so this should
pose no problems.
Signed-off-by: Tomasz Moń <tomasz.mon@nordicsemi.no>
The usb_dc_sam0 driver is leaking memory when the usb device is detached
using usb_disable().
When a device is detached, the driver does not reliably deallocate the
descriptor bank memory. Therefore, upon the next attach and configure,
another block of memory is allocated. After several connect /
disconnect sequences, the usb interface becomes unresponsive.
This commit adds a static function to release all memory allocated
to the ep descriptors when usb_dc_detach() is called. Since detach is
called from usb_disable(), this releases the memory when the
interface is disabled.
Avoid calling k_free() if buffer pointer is already
set to NULL.
Fixes#49177
Signed-off-by: Steven Slupsky <sslupsky@gmail.com>
Ensure that the FIFO of each endpoint is initialized during endpoint
configuration, to prevent a NULL FIFO from being encountered during the
first call to usb_dc_ep_write() for a given endpoint.
Signed-off-by: Nick Kraus <nick@nckraus.com>
1. Move the defines from usb_dc_mcux.h to usb_device_config.h
and fsl_os_abstraction.h. These headers are used by
the SDK USB driver. usb_dc_mcux.h header file is not longer
needed and hence deleted.
2. Delete the Zephyr implementation of the usb_device_struct
driver and use the one implemented inside the SDK USB
driver. This requires updating the references to
usb_device_struct inside the USB driver
3. Move defines and structures used by the driver
out of the header file that is included by the SDK and
into the MCUX USB driver.
4. Use end point defines provided by Zephyr instead of adding
them locally.
5. Add a Kconfig to set the thread stack size
6. Move code to enable interrupts back to usb_attach function.
Interrupts should be enabled after the init is successful,
else we see errors of the ISR getting called before the
init is complete causing Faults
6. Update west.yml to update the NXP HAL to get the updated
SDK USB driver.
Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>
So far thread was created as part of usb_dc_attach() by k_thread_create().
This means that if following function were executed:
* usb_enable()
* usb_disable()
* usb_enable()
then k_thread_create() was called second time. This results in undefined
behavior.
Fix above issue by moving k_thread_create() invocation to function called
during system initialization.
While at it, move IRQ_CONNECT() and irq_enable() invocations to init as
well.
Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
So far thread was created as part of usb_dc_attach() by k_thread_create().
This means that if following function were executed:
* usb_enable()
* usb_disable()
* usb_enable()
then k_thread_create() was called second time. This results in undefined
behavior.
Fix above issue by moving k_thread_create() invocation to function called
during system initialization.
While at it, move IRQ_CONNECT() and irq_enable() invocations to init as
well.
Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
So far thread was created as part of usb_dc_attach() by k_thread_create().
This means that if following function were executed:
* usb_enable()
* usb_disable()
* usb_enable()
then k_thread_create() was called second time. This results in undefined
behavior.
Fix above issue by moving k_thread_create() invocation to function called
during system initialization.
While at it, move IRQ_CONNECT() and irq_enable() invocations to init as
well.
Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Update usb device drivers to use DT_HAS_<compat>_ENABLED Kconfig symbol
to expose the driver and enable it by default based on devicetree.
We remove 'depend on' Kconfig for symbols that would be implied by
the devicetree node existing.
Signed-off-by: Kumar Gala <galak@kernel.org>
Many device pointers are initialized at compile and never changed. This
means that the device pointer can be constified (immutable).
Automated using:
```
perl -i -pe 's/const struct device \*(?!const)(.*)= DEVICE/const struct
device *const $1= DEVICE/g' **/*.c
```
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
