Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
19371: sys/usbus: check for the number of required and provided EPs in static configurations r=kaspar030 a=gschorcht

### Contribution description

This PR provides a static check at compile time whether the number of EPs required in a static configuration does not exceed the number of EPs provided by the USB device.

#### Background

In issue #19359 the problem was reported that `usbus_cdc_ecm` didn't work together with `stdio_cdc_acm` on some STM32 boards. The reason for some of the boards was simply that the application tried to allocate more EPs than available and simply ignored this and just didn't work.

#### Solution

Since `auto_init_usb` uses a static configuration with exactly one USBUS stack instance and one USB device, at least in case `auto_init` is used a static check can be carried out to make sure that the number of EPs required by the application doesn't exceed the number of EPs provided by the USB device. For this purpose, each `usbus_*` module defines the number of IN and OUT EPs required by that module. Each USB device driver defines the number of EPs provided by USB device if it differs from the default of 8 EPs. During the auto initialization the total number of required IN and OUT EPs is then compared with the number of EPs provided by the USB device using a static assert.

### Testing procedure

1. Green CI
2. Compilation of
   ```python
   USEMODULE='stdio_cdc_acm' BOARD=nucleo-f439zi make -j8 -C tests/usbus_cdc_ecm
   ```
   should lead to compilation error
   ```python
   sys/auto_init/usb/auto_init_usb.c:81:1: error: static assertion failed: "Number of required IN endpoints exceeded"
    _Static_assert(USBUS_EP_IN_REQUIRED_NUMOF <= USBDEV_NUM_ENDPOINTS,
    ^~~~~~~~~~~~~~
   Makefile.base:146: recipe for target 'tests/usbus_cdc_ecm/bin/nucleo-f439zi/auto_init_usbus/auto_init_usb.o' failed
   ```
   while compilation of
   ```
   USEMODULE='stdio_cdc_acm' BOARD=nucleo-f767zi make -j8 -C tests/usbus_cdc_ecm
   ```
   should work.

### Issues/PRs references

Fixes issue #19359 partially.

19382: tests/pkg_nanors: use static allocation r=kaspar030 a=benpicco



19388: drivers/usbdev_synopsys_dwc2: disable DMA mode r=kaspar030 a=gschorcht

### Contribution description

This PR disables the DMA mode for HS cores due to several problems found:

 - The STALL bit of the OUT control endpoint does not seem to be cleared automatically on the next SETUP received. At least the USB OTG HS core does not generate an interrupt on the next SETUP received. This happens, for example, when CDC ACM is used and the host sends the `SET_LINE_CODING` request which is answered with setting the STALL bit of the OUT endpoint. In this case the enumeration of further interfaces, for example CDC ECM, is stopped. This problem was described in #17085 (comment)

- The Enumeration fails for CDC ECM interface which uses URB support (PR #17091) 

### Testing procedure

Use a STM32 board with USH OTG HS interface:
```python
USEMODULE='stdio_cdc_acm periph_usbdev_hs_utmi' BOARD=stm32f723e-disco make -j8 -C tests/usbus_cdc_ecm flash
USEMODULE='stdio_cdc_acm periph_usbdev_hs_ulpi' BOARD=stm32f746g-disco make -j8 -C tests/usbus_cdc_ecm flash
```
Without this PR, either the enumeration completely fails (mostly for `stm32f723e-disco`)
```python
[377629.753895] usb 1-2.3: new high-speed USB device number 76 using xhci_hcd
[377629.854349] usb 1-2.3: device descriptor read/all, error -71
[377629.937990] usb 1-2.3: new high-speed USB device number 77 using xhci_hcd
[377630.038261] usb 1-2.3: device descriptor read/all, error -71
[377630.038711] usb 1-2-port3: attempt power cycle
[377630.641970] usb 1-2.3: new high-speed USB device number 78 using xhci_hcd
[377630.666066] usb 1-2.3: device descriptor read/8, error -71
[377630.794076] usb 1-2.3: device descriptor read/8, error -71
[377630.981806] usb 1-2.3: new high-speed USB device number 79 using xhci_hcd
[377631.002092] usb 1-2.3: device descriptor read/8, error -71
[377631.130091] usb 1-2.3: device descriptor read/8, error -71
[377631.238344] usb 1-2-port3: unable to enumerate USB device
```
or the enumeration of the CDC ECM interface stops with error.
```python
[377972.828168] usb 1-2.3: new high-speed USB device number 100 using xhci_hcd
[377972.928762] usb 1-2.3: config 1 interface 0 altsetting 0 endpoint 0x81 has an invalid bInterval 255, changing to 11
[377972.928765] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 64
[377972.928767] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x82 has invalid maxpacket 64
[377972.929225] usb 1-2.3: New USB device found, idVendor=1209, idProduct=7d00, bcdDevice= 1.00
[377972.929228] usb 1-2.3: New USB device strings: Mfr=3, Product=2, SerialNumber=4
[377972.929230] usb 1-2.3: Product: stm32f723e-disco
[377972.929232] usb 1-2.3: Manufacturer: RIOT-os.org
[377972.929233] usb 1-2.3: SerialNumber: A6BAC4E1B1E0806B
[377972.932399] cdc_acm 1-2.3:1.0: ttyACM1: USB ACM device
[377972.933905] cdc_ether: probe of 1-2.3:1.2 failed with error -32
[377973.184377] usb 1-4.3.4: reset high-speed USB device number 32 using xhci_hcd
```
With this PR the enumeration should work as it should:
```python
[378480.097974] usb 1-4.3.4: reset high-speed USB device number 32 using xhci_hcd
[378484.289762] usb 1-2.3: new high-speed USB device number 16 using xhci_hcd
[378484.394638] usb 1-2.3: config 1 interface 0 altsetting 0 endpoint 0x81 has an invalid bInterval 255, changing to 11
[378484.394642] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 64
[378484.394644] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x82 has invalid maxpacket 64
[378484.395296] usb 1-2.3: New USB device found, idVendor=1209, idProduct=7d00, bcdDevice= 1.00
[378484.395299] usb 1-2.3: New USB device strings: Mfr=3, Product=2, SerialNumber=4
[378484.395301] usb 1-2.3: Product: stm32f723e-disco
[378484.395303] usb 1-2.3: Manufacturer: RIOT-os.org
[378484.395304] usb 1-2.3: SerialNumber: A6BAC4E1B1E0806B
[378484.398547] cdc_acm 1-2.3:1.0: ttyACM1: USB ACM device
[378484.401007] cdc_ether 1-2.3:1.2 usb0: register 'cdc_ether' at usb-0000:00:14.0-2.3, CDC Ethernet Device, e6:75:97:3a:74:ba
[378484.449870] cdc_ether 1-2.3:1.2 enp0s20f0u2u3i2: renamed from usb0
```

### Issues/PRs references



Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
  • Loading branch information
3 people authored Mar 14, 2023
4 parents d7dba62 + 82a520e + 250c561 + 93c547a commit fb2e1fe
Show file tree
Hide file tree
Showing 16 changed files with 214 additions and 29 deletions.
7 changes: 7 additions & 0 deletions cpu/efm32/include/periph_cpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,13 @@ typedef struct {
#define WDT_HAS_STOP (1U)
/** @} */

/**
* @name USB device definitions
* @{
*/
#define USBDEV_NUM_ENDPOINTS 7 /**< Number of USB OTG FS endpoints including EP0 */
/** @} */

/* GPIO_LL's overrides */
#ifndef DOXYGEN

Expand Down
5 changes: 5 additions & 0 deletions cpu/esp32/include/periph_cpu_esp32s2.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,11 @@ extern "C" {
* @brief Buffers have to be word aligned for DMA
*/
#define USBDEV_CPU_DMA_ALIGNMENT (4)

/**
* @brief Number of USB IN and OUT endpoints available
*/
#define USBDEV_NUM_ENDPOINTS DWC2_USB_OTG_FS_NUM_EP
/** @} */


Expand Down
5 changes: 5 additions & 0 deletions cpu/esp32/include/periph_cpu_esp32s3.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ extern "C" {
* @brief Buffers have to be word aligned for DMA
*/
#define USBDEV_CPU_DMA_ALIGNMENT (4)

/**
* @brief Number of USB IN and OUT endpoints available
*/
#define USBDEV_NUM_ENDPOINTS DWC2_USB_OTG_FS_NUM_EP
/** @} */

#ifdef __cplusplus
Expand Down
7 changes: 7 additions & 0 deletions cpu/gd32v/include/periph_cpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,13 @@ typedef struct {
#define RTT_MIN_FREQUENCY (1U) /**< minimum RTT frequency in Hz */
/** @} */

/**
* @name USB device definitions
* @{
*/
#define USBDEV_NUM_ENDPOINTS 4 /**< Number of USB OTG FS endpoints including EP0 */
/** @} */

/**
* @brief Enable the given peripheral clock
*
Expand Down
51 changes: 51 additions & 0 deletions cpu/stm32/include/periph_cpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,57 @@ typedef struct {
#define HAVE_PTP_TIMER_SET_ABSOLUTE 1 /**< Native implementation available */
/** @} */

#if !DOXYGEN /* hide implementation details */
/**
* @name USB device definitions
* @{
*/
/* Detect the IP version based on the available register define */
#if defined(USB_OTG_GCCFG_NOVBUSSENS)
#define STM32_USB_OTG_CID_1x /**< USB OTG FS version 0x00001200 */
#elif defined(USB_OTG_GCCFG_VBDEN)
#define STM32_USB_OTG_CID_2x /**< USB OTG FS version 0x00002000 */
#elif defined(USB)
#define STM32_USB_FS_CID_1x /**< USB FS version 0x00001200 */
#endif

/**
* @brief Number of endpoints available with the OTG FS peripheral
* including the control endpoint
*/
#ifdef STM32_USB_OTG_CID_1x
#define STM32_USB_OTG_FS_NUM_EP (4) /**< OTG FS with 4 endpoints */
#elif defined(STM32_USB_OTG_CID_2x)
#define STM32_USB_OTG_FS_NUM_EP (6) /**< OTG FS with 6 endpoints */
#endif

/**
* @brief Number of endpoints available with the OTG HS peripheral
* including the control endpoint
*/
#ifdef STM32_USB_OTG_CID_1x
#define STM32_USB_OTG_HS_NUM_EP (6) /**< OTG HS with 6 endpoints */
#elif defined(STM32_USB_OTG_CID_2x)
#define STM32_USB_OTG_HS_NUM_EP (9) /**< OTG HS with 9 endpoints */
#endif

/**
* @brief Number of IN/OUT endpoints including EP0 as used by USBUS
*
* @note Since only a single number of EPs can be defined for USBUS that is
* valid for all devices, the smallest number of EPs must be used for
* multiple USB devices.
*/
#if defined(STM32_USB_OTG_FS_NUM_EP)
#define USBDEV_NUM_ENDPOINTS STM32_USB_OTG_FS_NUM_EP
#elif defined(STM32_USB_OTG_HS_NUM_EP)
#define USBDEV_NUM_ENDPOINTS STM32_USB_OTG_HS_NUM_EP
#else
#define USBDEV_NUM_ENDPOINTS 8
#endif

#endif /* !DOXYGEN */

#ifdef __cplusplus
}
#endif
Expand Down
27 changes: 5 additions & 22 deletions cpu/stm32/include/usbdev_stm32.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,42 +41,22 @@
#include <stdint.h>
#include <stdlib.h>
#include "periph_cpu.h"
#include "periph/usbdev.h"

#ifdef __cplusplus
extern "C" {
#endif

/* Detect the IP version based on the available register define */
#if defined(USB_OTG_GCCFG_NOVBUSSENS)
#define STM32_USB_OTG_CID_1x /**< USB OTG FS version 0x00001200 */
#elif defined(USB_OTG_GCCFG_VBDEN)
#define STM32_USB_OTG_CID_2x /**< USB OTG FS version 0x00002000 */
#elif defined(USB)
#define STM32_USB_FS_CID_1x /**< USB FS version 0x00001200 */
#else
#error Unknown USB peripheral version
#endif

/**
* @brief Number of endpoints available with the OTG FS peripheral
* including the control endpoint
*/
#ifdef STM32_USB_OTG_CID_1x
#define DWC2_USB_OTG_FS_NUM_EP (4) /**< OTG FS with 4 endpoints */
#elif defined(STM32_USB_OTG_CID_2x)
#define DWC2_USB_OTG_FS_NUM_EP (6) /**< OTG FS with 6 endpoints */
#endif
#define DWC2_USB_OTG_FS_NUM_EP STM32_USB_OTG_FS_NUM_EP

/**
* @brief Number of endpoints available with the OTG HS peripheral
* including the control endpoint
*/
#ifdef STM32_USB_OTG_CID_1x
#define DWC2_USB_OTG_HS_NUM_EP (6) /**< OTG HS with 6 endpoints */
#elif defined(STM32_USB_OTG_CID_2x)
#define DWC2_USB_OTG_HS_NUM_EP (9) /**< OTG HS with 9 endpoints */
#endif
#define DWC2_USB_OTG_HS_NUM_EP STM32_USB_OTG_HS_NUM_EP

/**
* @brief USB OTG FS FIFO reception buffer space in 32-bit words
Expand Down Expand Up @@ -144,6 +124,9 @@ extern "C" {
#endif
#endif

/* periph/usbdev.h is included after the definitions above by intention */
#include "periph/usbdev.h"

/**
* @brief stm32 USB Device FS only peripheral device context
*/
Expand Down
10 changes: 9 additions & 1 deletion drivers/usbdev_synopsys_dwc2/usbdev_synopsys_dwc2.c
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,15 @@ static size_t _max_endpoints(const dwc2_usb_otg_fshs_config_t *config)

static bool _uses_dma(const dwc2_usb_otg_fshs_config_t *config)
{
#if defined(DWC2_USB_OTG_HS_ENABLED) && STM32_USB_OTG_HS_USE_DMA
/* DMA mode is disabled for now due to several problems:
* - The STALL bit of the OUT control endpoint does not seem to be cleared
* automatically on the next SETUP received. At least the USB OTG HS core
* does not generate an interrupt on the next SETUP received. This happens,
* for example, when CDC ACM is used and the host sends the SET_LINE_CODING
* request. In this case the enumeration of further interfaces, for example
* CDC ECM is stopped.
* - The Enumeration fails for CDC ECM interface which uses URB support. */
#if 0 /* defined(DWC2_USB_OTG_HS_ENABLED) && STM32_USB_OTG_HS_USE_DMA */
return config->type == DWC2_USB_OTG_HS;
#else
(void)config;
Expand Down
2 changes: 1 addition & 1 deletion pkg/nanors/Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
PKG_NAME=nanors
PKG_URL=https://github.com/sleepybishop/nanors.git
PKG_VERSION=395e5ada44dd8d5974eaf6bb6b17f23406e3ca72
PKG_VERSION=e9e242e98e27037830490b2a752895ca68f75f8b
PKG_LICENSE=MIT

include $(RIOTBASE)/pkg/pkg.mk
Expand Down
42 changes: 42 additions & 0 deletions sys/auto_init/usb/auto_init_usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <assert.h>

#include "usb/usbus.h"
#include "usb/usbus/control.h"

#ifdef MODULE_USBUS_CDC_ECM
#include "usb/usbus/cdc/ecm.h"
Expand All @@ -39,11 +40,52 @@ usbus_cdcecm_device_t cdcecm;
#include "usb/usbus/dfu.h"
static usbus_dfu_device_t dfu;
#endif
#ifdef MODULE_USBUS_HID
#include "usb/usbus/hid.h"
#endif
#ifdef MODULE_USBUS_MSC
#include "usb/usbus/msc.h"
static usbus_msc_device_t msc;
#endif

#ifndef MODULE_USBUS_CDC_ACM
#define USBUS_CDC_ACM_EP_IN_REQUIRED_NUMOF 0
#define USBUS_CDC_ACM_EP_OUT_REQUIRED_NUMOF 0
#endif

#ifndef MODULE_USBUS_CDC_ECM
#define USBUS_CDC_ECM_EP_IN_REQUIRED_NUMOF 0
#define USBUS_CDC_ECM_EP_OUT_REQUIRED_NUMOF 0
#endif

#ifndef MODULE_USBUS_HID
#define USBUS_HID_EP_IN_REQUIRED_NUMOF 0
#define USBUS_HID_EP_OUT_REQUIRED_NUMOF 0
#endif

#ifndef MODULE_USBUS_MSC
#define USBUS_MSC_EP_IN_REQUIRED_NUMOF 0
#define USBUS_MSC_EP_OUT_REQUIRED_NUMOF 0
#endif

#define USBUS_EP_IN_REQUIRED_NUMOF (USBUS_CONTROL_EP_IN_REQUIRED_NUMOF + \
USBUS_CDC_ACM_EP_IN_REQUIRED_NUMOF + \
USBUS_CDC_ECM_EP_IN_REQUIRED_NUMOF + \
USBUS_HID_EP_IN_REQUIRED_NUMOF + \
USBUS_MSC_EP_IN_REQUIRED_NUMOF)

#define USBUS_EP_OUT_REQUIRED_NUMOF (USBUS_CONTROL_EP_OUT_REQUIRED_NUMOF + \
USBUS_CDC_ACM_EP_OUT_REQUIRED_NUMOF + \
USBUS_CDC_ECM_EP_OUT_REQUIRED_NUMOF + \
USBUS_HID_EP_OUT_REQUIRED_NUMOF + \
USBUS_MSC_EP_OUT_REQUIRED_NUMOF)

static_assert(USBUS_EP_IN_REQUIRED_NUMOF <= USBDEV_NUM_ENDPOINTS,
"Number of required IN endpoints exceeded");

static_assert(USBUS_EP_OUT_REQUIRED_NUMOF <= USBDEV_NUM_ENDPOINTS,
"Number of required OUT endpoints exceeded");

static char _stack[USBUS_STACKSIZE];
static usbus_t usbus;

Expand Down
10 changes: 10 additions & 0 deletions sys/include/usb/hid.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,16 @@ extern "C" {
#define USB_HID_REQUEST_SET_PROTOCOL 0x0b
/** @} */

/**
* @brief Number of IN EPs required for the HID interface
*/
#define USBUS_HID_EP_IN_REQUIRED_NUMOF 1

/**
* @brief Number of Out EPs required for the HID interface
*/
#define USBUS_HID_EP_OUT_REQUIRED_NUMOF 1

/**
* @brief USB HID descriptor struct
*
Expand Down
10 changes: 10 additions & 0 deletions sys/include/usb/usbus/cdc/acm.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ extern "C" {
*/
#define USBUS_CDC_ACM_INT_EP_SIZE (8)

/**
* @brief Number of IN EPs required for the CDC ACM interface
*/
#define USBUS_CDC_ACM_EP_IN_REQUIRED_NUMOF 2

/**
* @brief Number of Out EPs required for the CDC ACM interface
*/
#define USBUS_CDC_ACM_EP_OUT_REQUIRED_NUMOF 1

/**
* @brief CDC ACM line state as reported by the host computer
*/
Expand Down
10 changes: 10 additions & 0 deletions sys/include/usb/usbus/cdc/ecm.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ extern "C" {
*/
#define USBUS_ETHERNET_FRAME_BUF MATH_ALIGN(ETHERNET_FRAME_LEN, USBUS_CDCECM_EP_DATA_SIZE)

/**
* @brief Number of IN EPs required for the CDC ECM interface
*/
#define USBUS_CDC_ECM_EP_IN_REQUIRED_NUMOF 2

/**
* @brief Number of Out EPs required for the CDC ECM interface
*/
#define USBUS_CDC_ECM_EP_OUT_REQUIRED_NUMOF 1

/**
* @brief notification state, used to track which information must be send to
* the host
Expand Down
10 changes: 10 additions & 0 deletions sys/include/usb/usbus/control.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@
#ifndef USB_USBUS_CONTROL_H
#define USB_USBUS_CONTROL_H

/**
* @brief Number of IN EPs required for the control interface
*/
#define USBUS_CONTROL_EP_IN_REQUIRED_NUMOF 1

/**
* @brief Number of IN EPs required for the control interface
*/
#define USBUS_CONTROL_EP_OUT_REQUIRED_NUMOF 1

#include "usb/usbus.h"

#ifdef __cplusplus
Expand Down
10 changes: 10 additions & 0 deletions sys/include/usb/usbus/msc.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@
extern "C" {
#endif

/**
* @brief Number of IN EPs required for the MSC interface
*/
#define USBUS_MSC_EP_IN_REQUIRED_NUMOF 1

/**
* @brief Number of Out EPs required for the MSC interface
*/
#define USBUS_MSC_EP_OUT_REQUIRED_NUMOF 1

/**
* @brief USBUS MSC Number of exported MTD device through USB
*/
Expand Down
12 changes: 7 additions & 5 deletions tests/pkg_nanors/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,14 @@ int main(void)
static uint8_t data[(DATA_SHARDS + RECOVERY_SHARDS) * SHARD_SIZE];
/* copy of data for comparison */
static uint8_t data_cmp[DATA_SHARDS * SHARD_SIZE];
/* nanors work buffer */
static uint8_t rs_buf[reed_solomon_bufsize(DATA_SHARDS, RECOVERY_SHARDS)];

/* pointer to shards */
static uint8_t *shards[DATA_SHARDS + RECOVERY_SHARDS];
uint8_t *shards[DATA_SHARDS + RECOVERY_SHARDS];
/* map of missing shards */
static uint8_t marks[DATA_SHARDS + RECOVERY_SHARDS];
uint8_t marks[DATA_SHARDS + RECOVERY_SHARDS];
memset(marks, 0, sizeof(marks));

/* generate random data */
random_bytes(data, sizeof(data_cmp));
Expand All @@ -51,8 +55,7 @@ int main(void)
}

puts("START");
reed_solomon_init();
rs_t *rs = reed_solomon_new(DATA_SHARDS, RECOVERY_SHARDS);
rs_t *rs = reed_solomon_new_static(rs_buf, sizeof(rs_buf), DATA_SHARDS, RECOVERY_SHARDS);
if (!rs) {
puts("failed to init codec");
return -1;
Expand All @@ -79,7 +82,6 @@ int main(void)
} else {
puts("done.");
}
reed_solomon_release(rs);

if (memcmp(data, data_cmp, sizeof(data_cmp))) {
puts("FAILED");
Expand Down
Loading

0 comments on commit fb2e1fe

Please sign in to comment.