Skip to content
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

bootloaders/riotboot_dfu: fixes including sys/usb/usbus/dfu #18964

Merged
merged 8 commits into from Dec 2, 2022

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Nov 24, 2022

Contribution description

This PR fixes a number of make problems I encountered while trying bootloaders/riotboot_dfu on a nucleo-f767zi board which are not catched by CI compilation.

  1.  BOARD=nucleo-f767zi make -j8 -C bootloaders/riotboot_dfu flash

    seemed to work without problems (see issue 4), using dfu-util --list command gave the following output:

    Found DFU: [1209:7d02] ver=0100, devnum=69, cfg=1, intf=0, path="1-2.2", alt=1, name="RIOT-OS Slot 1", serial="6591620BCB270283"
    Found DFU: [1209:7d02] ver=0100, devnum=69, cfg=1, intf=0, path="1-2.2", alt=0, name="RIOT-OS Slot 0", serial="6591620BCB270283"

    However when trying to flash an application from riotboot_dfu example

    FEATURES_REQUIRED=riotboot USEMODULE=usbus_dfu BOARD=nucleo-f767zi make -C tests/saul \
    PROGRAMMER=dfu-util riotboot/flash-slot0

    the compilation fails with

    makefiles/boards/stm32.inc.mk:28: *** DFU_USB_ID is not set.

    The reason is, that the STM32 make system rejects the use of dfu-util if variable DFU_USB_ID is not set, although dfu-util.mk generates the variable DFU_USB_ID from USB_VID and USB_PID if needed. Commit 6a76b94 changes the STM32 make system to accept an empty DFU_USB_ID variable when using dfu-util for USBUS DFU (module usbus_dfu is enabled).

  2. The dfu-util command uses --device $(DFU_USB_ID), where DFU_USB_ID is derived from USB_VID and USB_PID to specify the DFU device to use. Without specifying USB_VID and USB_PID in the make command, dfu-util is called with --device :, which only works if there is only one DFU device. The documentation is changed by commit d8490b5 to use USB_VID/USB_PID or DFU_USB_ID in the make command.

  3. With the fix of issue 1, the compilation with command

    FEATURES_REQUIRED=riotboot USEMODULE=usbus_dfu BOARD=nucleo-f767zi make -C tests/saul \
    PROGRAMMER=dfu-util riotboot/flash-slot0

    starts but fails in sys/usb/usbus/dfu.c when FLASHPAGE_SIZE is used to check whether SLOT0_OFFSET is a multiple of the flash page size:

    sys/usb/usbus/dfu/dfu.c:104:28: error: 'FLASHPAGE_SIZE' undeclared (first use in this function); did you mean 'FLASHPAGE_OK'?
      assert((SLOT0_OFFSET % FLASHPAGE_SIZE) == 0);

    The reason is that STM32F2/4/7 MCUs use sectors instead of pages, where the minimum sector size is defined by FLASHPAGE_MIN_SECTOR_SIZE, which is 16KB or 32KB (the first sector) depending on the CPU_MODEL. In this case SLOT0_OFFSET must be a multiple of the minimum sector size to cover a whole sector.

    Commit f58413b fixes this problem.

  4. With fixes of issues 1 and 3, it was then possible to compile and download the application for STM32 F2/F4/F7 using USBUS DFU but the bootloader always starts in DFU mode because BTN_BOOTLOADER_INVERTED is true by default, that is the bootloader button is low-active. However, the USER button is high-active for the Nucleo boards and a number of other STM32 boards. With BTN_BOOTLOADER_INVERTED true, the bootloader then always starts in DFU mode. Using dfu-util --list still gave:

    Found DFU: [1209:7d02] ver=0100, devnum=71, cfg=1, intf=0, path="1-2.2", alt=1, name="RIOT-OS Slot 1", serial="6591620BCB270283"
    Found DFU: [1209:7d02] ver=0100, devnum=71, cfg=1, intf=0, path="1-2.2", alt=0, name="RIOT-OS Slot 0", serial="6591620BCB270283"

    Instead of setting BTN_BOOTLOADER_INVERTED in each board definition, commit 540f5f8 uses BTN0_MODE to set BTN_BOOTLOADER_INVERTED to false if it is GPIO_IN_PD, or to true otherwise.

Testing procedure

Use a Nucleo144 board with STM32 F2, F4 or F7 MCU and execute:

BOARD=nucleo-f767zi make -j8 -C bootloaders/riotboot_dfu flash
dfu-util --list
FEATURES_REQUIRED=riotboot USEMODULE=usbus_dfu BOARD=nucleo-f767zi make -C tests/saul \
PROGRAMMER=dfu-util riotboot/flash-slot0
FEATURES_REQUIRED=riotboot USEMODULE=usbus_dfu BOARD=nucleo-f767zi make -C tests/saul \
PROGRAMMER=dfu-util DFU_USB_ID=1209:7d02 riotboot/flash-slot0
dfu-util --list

Without the PR either the compilation fails or the bootloader doesn't start the flashed image, that is dfu-util --list still gives

Found DFU: [1209:7d02] ver=0100, devnum=69, cfg=1, intf=0, path="1-2.2", alt=1, name="RIOT-OS Slot 1", serial="6591620BCB270283"
Found DFU: [1209:7d02] ver=0100, devnum=69, cfg=1, intf=0, path="1-2.2", alt=0, name="RIOT-OS Slot 0", serial="6591620BCB270283"

after downloading the application.

With this PR the compilation should succeed and the bootloader should start the application after downloading. The later can also be checked with dfu-util --list. The output should then be:

Found Runtime: [1209:7d00] ver=0100, devnum=72, cfg=1, intf=0, path="1-2.2", alt=0, name="RIOT-OS bootloader", serial="6591620BCB270283"

Issues/PRs references

@github-actions github-actions bot added Area: build system Area: Build system Area: doc Area: Documentation Area: sys Area: System Area: USB Area: Universal Serial Bus labels Nov 24, 2022
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 24, 2022
@benpicco benpicco requested a review from maribu November 24, 2022 13:31
@gschorcht gschorcht changed the title Fixes for bootloaders/riotboot_dfu and sys/usb/usbus/dfu bootloaders/riotboot_dfu: fixes including sys/usb/usbus/dfu Nov 24, 2022
sys/usb/usbus/dfu/dfu.c Outdated Show resolved Hide resolved
sys/usb/usbus/dfu/dfu.c Outdated Show resolved Hide resolved
Comment on lines 54 to 65
$ FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=particle-xenon \
PROGRAMMER=dfu-util USB_VID=1209 USB_PID=7d02 all riotboot/flash-slot0
```

Instead of setting `USB_VID` and `USB_PID`, the variable `DFU_USB_ID` could also
be used to specify the DFU device to be used.

```
$ FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=particle-xenon \
PROGRAMMER=dfu-util DFU_USB_ID=1209:7d02 all riotboot/flash-slot0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, USB_VID, USB_PID and DFU_USB_ID are all optional. Maybe it is worth a note ?

Copy link
Contributor Author

@gschorcht gschorcht Nov 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When none of them are defined, the dfu-util command looks like:

dfu-util --device : --alt 0 --download ...

So the --device parameter seems to be malformed and is only working because dfu-util seems to be tolerant. If there are more DFU capable devices, DFU_USB_ID or USB_VID and USB_PID have to be defined.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are more DFU capable devices, DFU_USB_ID or USB_VID and USB_PID have to be defined.

True, but that's not the most common setup (unless you have a weird USB based webcam integrated to the laptop which support DFU...).

Still, this is weird because USB_VID and USB_PID should have default values so --device shouldn't looks like that.

Copy link
Contributor Author

@gschorcht gschorcht Nov 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default values of USB_VID and USB_PID are set either in makefiles/usb-codes.inc.mk if CONFIG_USB_VID and CONFIG_USB_PID are defined, or in the makefile of the application.The latter is usually only the case for applications that implement USB device interfaces, such as bootloaders/riotboot_dfu itself. Applications which use usbus_dfu just for downloading usually don't define USB_VID and USB_PID.

The option we have are:

  • We leave it as is with the malformed --device argument.
  • We define the default values of USB_VID and USB_PID in makefiles/usb-codes.inc.mk if module usbus_dfu is used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the latter is the better.

@riot-ci
Copy link

riot-ci commented Nov 24, 2022

Murdock results

✔️ PASSED

7d7c8b1 tests/riotboot_flashwrite: blacklist blxxxpill boards

Success Failures Total Runtime
117882 0 117882 01h:54m:50s

Artifacts

@github-actions github-actions bot added the Area: boards Area: Board ports label Nov 26, 2022
@gschorcht
Copy link
Contributor Author

gschorcht commented Nov 26, 2022

Boards that are shipped with a DFU bootloader define the dfu-util configuration in their Makefile.include. Examples are weact-f4x1cx, nz32-sc151, patricle-* and optionally bluepill. However, when riotboot_dfu is used as the DFU bootloader, the board's dfu-util configuration must be overridden by the configuration as required by riotboot_dfu to use it to flash applications. Therefore, all dfu-util configurations are now defined as overridable in the board's Makefile.include and the configuration as required by riotboot_dfu is included as new file makefiles/boot/riotboot_dfu-util.mk before the board's Makefile.include.

makefiles/boot/riotboot_dfu-util.mk defines the VID/PID pair allocated for the RIOT bootloader (https://pid.codes/1209/7D02/) by variable DFU_USB_ID and sets DFU_USE_DFUSE to 0. Both variables could be overriden by the make command if necessary. This seems to be the most convinient and flexible approach to use riotboot_dfu with default configuration.

This approach also works for the bluepill board so that the USB interface can be used to flash applications once, riotboot_dfu has been flashed via the SWD interface with ST-LINK.

BOARD=bluepill make -j8 -C bootloaders/riotboot_dfu flash
dfu-util --list
FEATURES_REQUIRED=riotboot USEMODULE='stdio_cdc_acm usbus_dfu' BOARD=bluepill \
make -C tests/shell PROGRAMMER=dfu-util riotboot/flash-slot0
dfu-util --list
dfu-util -e
dfu-util --list

@github-actions github-actions bot added the Area: examples Area: Example Applications label Nov 26, 2022
@github-actions github-actions bot added the Area: Kconfig Area: Kconfig integration label Nov 27, 2022
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Nov 27, 2022
.bash_history Outdated Show resolved Hide resolved
@maribu
Copy link
Member

maribu commented Nov 27, 2022

Code looks good to me and the CI is happy. I didn't follow the discussion. Is there still anything needed orher rhan squashing and an ACK?

The `dfu-util` command uses `--device $(DFU_USB_ID)`, where `DFU_USB_ID` is derived from `USB_VID` and `USB_PID` to specify the DFU device to use. Without specifying `USB_VID` and `USB_PID` in the make command, `dfu-util` is called with `--device :`, which only works if there is only one DFU device. Also, the STM32 make system forbids the use of `dfu-util` as programmer without setting the variable `DFU_USB_ID`. Therefore, the documentation is changed to use `USB_VID`/`USB_PID` or `DFU_USB_ID` in the make command.
STM32F2/4/7 MCUs use sectors instead of pages, where the minimum sector size is defined by FLASHPAGE_MIN_SECTOR_SIZE, which is 16KB or 32KB (the first sector) depending on the CPU_MODEL. In this case SLOT0_OFFSET must be a multiple of the minimum sector size to cover a whole sector.
Boards that are shipped with a DFU bootloader define the `dfu-util` configuration in their `Makefile.include`. However, when `riotboot_dfu` is used as the DFU bootloader, the board's `dfu-util` configuration must be overridden by the configuration as required by `riotboot_dfu` to use it to flash applications. Therefore, all `dfu-util` configurations are defined as overridable in the board's `Makefile.include` and the configuration as required by `riotboot_dfu` is included before the board's `Makefile.include`.
@gschorcht gschorcht force-pushed the bootloaders/riotboot_dfu_fixes branch from 4978680 to 7d7c8b1 Compare November 30, 2022 18:43
@dylad
Copy link
Member

dylad commented Nov 30, 2022

I'll give it a try tomorrow. My bluepill has a 10k pullup resistor on D+ 😞
Will fix it at my office.

@dylad
Copy link
Member

dylad commented Dec 1, 2022

Hmmm I'm supposed to have fix my bluepill. I can measure 1.5KOhms between 3V3 and PA12 but somehow USB still doesn't work. Am I missing something ?

@gschorcht
Copy link
Contributor Author

Hmmm I'm supposed to have fix my bluepill. I can measure 1.5KOhms between 3V3 and PA12 but somehow USB still doesn't work. Am I missing something ?

Have you tried any other USB application than riotnoot_dfu to see whether USB is working at all? I have to press the RESET button while starting to flash it and after flashing again.

@dylad
Copy link
Member

dylad commented Dec 1, 2022

Have you tried any other USB application

Give it a try and the results is quite unreliable. I had to reset half of dozen times to finally have the bluepill to properly enumerates.
Same apply on cdc_acm. Once the app starts it works fine (except the PS command which seems to kill the app).
Is my hardware crappy or are you able to reproduce ?

@gschorcht
Copy link
Contributor Author

Give it a try and the results is quite unreliable. I had to reset half of dozen times to finally have the bluepill to properly enumerates.

At the beginning I had the same problems with my bluepills until I found out that it was related to the cable and the USB port used at my USB hub. Now I have found a combination of USB port and USB cable that works absolutely stable.

Maybe @benpicco has made similar experiences and could try this PR on bluepill.

@benpicco
Copy link
Contributor

benpicco commented Dec 2, 2022

I can confirm it's working on my blackpill board.
I made the mistake of not including the usb_board_reset module, so I can only flash it once via DFU (there is no user button) but everything works as expected.

@gschorcht
Copy link
Contributor Author

I made the mistake of not including the usb_board_reset module, so I can only flash it once via DFU (there is no user button) but everything works as expected.

dfu-util -e can also be used to restart the bootloader in DFU mode without having a user button.

@benpicco
Copy link
Contributor

benpicco commented Dec 2, 2022

Ah nice that worked - what kind of magic is this? 😃

@benpicco benpicco merged commit 945af26 into RIOT-OS:master Dec 2, 2022
@maribu maribu added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Dec 2, 2022
@gschorcht
Copy link
Contributor Author

Thanks for reviewing, testing and merging.

@gschorcht
Copy link
Contributor Author

I made the mistake of not including the usb_board_reset module

Should usb_board_reset be enabled by default for blxxxpill boards if usbus_dfu is enabled?

BTW, when investigating the compilation errors in PR #18998 I realized that usb_board_reset has to be supported by tinyUSB if the board uses the usb_board_reset module. Otherwise, the board cannot be flashed anymore once a tinyUSB application has been flashed. I will provide another PR.

@gschorcht gschorcht deleted the bootloaders/riotboot_dfu_fixes branch December 4, 2022 16:10
@kaspar030 kaspar030 added this to the Release 2023.01 milestone Jan 19, 2023
@gschorcht gschorcht restored the bootloaders/riotboot_dfu_fixes branch March 10, 2023 08:23
bors bot added a commit that referenced this pull request Mar 10, 2023
19371: sys/usbus: check for the number of required and provided EPs in static configurations r=benpicco 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.

19374: makefiles/boards/stm32: fix DFU_USB_ID handling r=benpicco a=gschorcht

### Contribution description

This PR fixes the still existing problem that an application can't be flashed to a STM32 board if it uses `riotboot_dfu` with default  VID/PID (1209:7d02).

In PR #18964 item 1, the problem was already described that an application can't be flashed on a board that is using `riotboot_dfu`. Using for example
```python
FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
PROGRAMMER=dfu-util all riotboot/flash-slot0
```
always leads to error
```python
/home/gs/src/RIOT-Xtensa-ESP.esp-idf-4.4/makefiles/boards/stm32.inc.mk:28: *** DFU_USB_ID is not set.  Stop.
/home/gs/src/RIOT-Xtensa-ESP.esp-idf-4.4/makefiles/boot/riotboot.mk:87: recipe for target 'riotboot/bootloader/binfile' failed
```
even if `DFU_USB_ID` variable is set as described in documentation.
```python
FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
PROGRAMMER=dfu-util DFU_USB_ID=1209:7d02 all riotboot/flash-slot0
```
The reason is that the variable `DFU_USB_ID` isn't exported and the check https://github.com/RIOT-OS/RIOT/blob/8dc8bf35678493ecc8ef110e2f0359a67c03894c/makefiles/boards/stm32.inc.mk#L27-L29 sees an empty `DFU_USB_ID` variable here. It prevents to use `dfu-util` event though the following `dfu-util.mk` will generate a default value for this variable from the `USB_VID` and `USB_PID` variables if necessary.

Commit 6a76b94 of PR #18964 tried to fix this problem but wasn't merged for any reason.

To fix this problem, the check is completely removed. If a board such as `weact-f4x1cx` uses a DFU boorloader and requires a certain VID/PID combination, board's makefile is responsible to set `DFU_USB_ID` variable.

### Testing procedure

It is not necessary to use a real boad, checking the compilation process is sufficient.

1. Using default VID/PID as described in documentation:
   ```python
   FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
   PROGRAMMER=dfu-util all riotboot/flash-slot0
   ```
   can't be compiled without this PR but calls `dfu-util` correctly with this PR using the default VID/PID:
   ```python
   dfu-util --device 1209:7d02 --alt 0 --download examples/saul/bin/nucleo-f767zi/riotboot_files/slot0.1678440536.bin
   ```
2. Using a VID/PID as described in documentation:
   ```python
   FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
   DFU_USB_ID=1209:affe PROGRAMMER=dfu-util all riotboot/flash-slot0
   ```
   can't be compiled without this PR but calls `dfu-util` correctly with this PR using the default VID/PID:
   ```python
   dfu-util --device 1209:affe --alt 0 --download examples/saul/bin/nucleo-f767zi/riotboot_files/slot0.1678440536.bin
   ```
3. Compiling a board with DFU bootloader
   ```python
   make -C examples/saul flash BOARD=weact-f411ce
   ```
   should still call dfu-util with correct VID/PID:
   ```python
   dfu-util --device 0483:df11 --alt 0 --download /home/gs/src/RIOT-Xtensa-ESP.esp-idf-4.4/examples/saul/bin/weact-f411ce/saul_example.bin  --dfuse-address 0x8000000:leave
   ```
### Issues/PRs references



19375: tools/renode: add support for target reset r=benpicco a=aabadie



19376: boards/stm32f4discovery: use default port to access stdio via cdc acm r=benpicco a=aabadie



19377: pkg/tinyusb: fix default VID/PID configuration r=benpicco a=gschorcht

### Contribution description

This PR fixes the default VID/PID configuration if tinyUSB board reset feature is used.

While reviewing PR #19086 I was wondering why `esp32s2-wemos-mini` requires to set `USB_VID`/`USB_PID` explicitly to  `USB_VID_TESTING`/`USB_PID_TESTING`. The reason was that tinyUSB board reset feature wasn't declared as RIOT internal.

### Testing procedure

Flashing `esp32s2-wemos-mini` should still work.
```
BOARD=esp32s2-wemos-mini make -C tests/shell flash
```
The VID/PID should be `1209:7d00` and not `1209:7d01`.

### Issues/PRs references



Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Alexandre Abadie <alexandre.abadie@inria.fr>
bors bot added a commit that referenced this pull request Mar 10, 2023
19371: sys/usbus: check for the number of required and provided EPs in static configurations r=benpicco 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.

19374: makefiles/boards/stm32: fix DFU_USB_ID handling r=benpicco a=gschorcht

### Contribution description

This PR fixes the still existing problem that an application can't be flashed to a STM32 board if it uses `riotboot_dfu` with default  VID/PID (1209:7d02).

In PR #18964 item 1, the problem was already described that an application can't be flashed on a board that is using `riotboot_dfu`. Using for example
```python
FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
PROGRAMMER=dfu-util all riotboot/flash-slot0
```
always leads to error
```python
/home/gs/src/RIOT-Xtensa-ESP.esp-idf-4.4/makefiles/boards/stm32.inc.mk:28: *** DFU_USB_ID is not set.  Stop.
/home/gs/src/RIOT-Xtensa-ESP.esp-idf-4.4/makefiles/boot/riotboot.mk:87: recipe for target 'riotboot/bootloader/binfile' failed
```
even if `DFU_USB_ID` variable is set as described in documentation.
```python
FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
PROGRAMMER=dfu-util DFU_USB_ID=1209:7d02 all riotboot/flash-slot0
```
The reason is that the variable `DFU_USB_ID` isn't exported and the check https://github.com/RIOT-OS/RIOT/blob/8dc8bf35678493ecc8ef110e2f0359a67c03894c/makefiles/boards/stm32.inc.mk#L27-L29 sees an empty `DFU_USB_ID` variable here. It prevents to use `dfu-util` event though the following `dfu-util.mk` will generate a default value for this variable from the `USB_VID` and `USB_PID` variables if necessary.

Commit 6a76b94 of PR #18964 tried to fix this problem but wasn't merged for any reason.

To fix this problem, the check is completely removed. If a board such as `weact-f4x1cx` uses a DFU boorloader and requires a certain VID/PID combination, board's makefile is responsible to set `DFU_USB_ID` variable.

### Testing procedure

It is not necessary to use a real boad, checking the compilation process is sufficient.

1. Using default VID/PID as described in documentation:
   ```python
   FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
   PROGRAMMER=dfu-util all riotboot/flash-slot0
   ```
   can't be compiled without this PR but calls `dfu-util` correctly with this PR using the default VID/PID:
   ```python
   dfu-util --device 1209:7d02 --alt 0 --download examples/saul/bin/nucleo-f767zi/riotboot_files/slot0.1678440536.bin
   ```
2. Using a VID/PID as described in documentation:
   ```python
   FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
   DFU_USB_ID=1209:affe PROGRAMMER=dfu-util all riotboot/flash-slot0
   ```
   can't be compiled without this PR but calls `dfu-util` correctly with this PR using the default VID/PID:
   ```python
   dfu-util --device 1209:affe --alt 0 --download examples/saul/bin/nucleo-f767zi/riotboot_files/slot0.1678440536.bin
   ```
3. Compiling a board with DFU bootloader
   ```python
   make -C examples/saul flash BOARD=weact-f411ce
   ```
   should still call dfu-util with correct VID/PID:
   ```python
   dfu-util --device 0483:df11 --alt 0 --download /home/gs/src/RIOT-Xtensa-ESP.esp-idf-4.4/examples/saul/bin/weact-f411ce/saul_example.bin  --dfuse-address 0x8000000:leave
   ```
### Issues/PRs references



Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
bors bot added a commit that referenced this pull request Mar 10, 2023
19374: makefiles/boards/stm32: fix DFU_USB_ID handling r=benpicco a=gschorcht

### Contribution description

This PR fixes the still existing problem that an application can't be flashed to a STM32 board if it uses `riotboot_dfu` with default  VID/PID (1209:7d02).

In PR #18964 item 1, the problem was already described that an application can't be flashed on a board that is using `riotboot_dfu`. Using for example
```python
FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
PROGRAMMER=dfu-util all riotboot/flash-slot0
```
always leads to error
```python
/home/gs/src/RIOT-Xtensa-ESP.esp-idf-4.4/makefiles/boards/stm32.inc.mk:28: *** DFU_USB_ID is not set.  Stop.
/home/gs/src/RIOT-Xtensa-ESP.esp-idf-4.4/makefiles/boot/riotboot.mk:87: recipe for target 'riotboot/bootloader/binfile' failed
```
even if `DFU_USB_ID` variable is set as described in documentation.
```python
FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
PROGRAMMER=dfu-util DFU_USB_ID=1209:7d02 all riotboot/flash-slot0
```
The reason is that the variable `DFU_USB_ID` isn't exported and the check https://github.com/RIOT-OS/RIOT/blob/8dc8bf35678493ecc8ef110e2f0359a67c03894c/makefiles/boards/stm32.inc.mk#L27-L29 sees an empty `DFU_USB_ID` variable here. It prevents to use `dfu-util` event though the following `dfu-util.mk` will generate a default value for this variable from the `USB_VID` and `USB_PID` variables if necessary.

Commit 6a76b94 of PR #18964 tried to fix this problem but wasn't merged for any reason.

To fix this problem, the check is completely removed. If a board such as `weact-f4x1cx` uses a DFU boorloader and requires a certain VID/PID combination, board's makefile is responsible to set `DFU_USB_ID` variable.

### Testing procedure

It is not necessary to use a real boad, checking the compilation process is sufficient.

1. Using default VID/PID as described in documentation:
   ```python
   FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
   PROGRAMMER=dfu-util all riotboot/flash-slot0
   ```
   can't be compiled without this PR but calls `dfu-util` correctly with this PR using the default VID/PID:
   ```python
   dfu-util --device 1209:7d02 --alt 0 --download examples/saul/bin/nucleo-f767zi/riotboot_files/slot0.1678440536.bin
   ```
2. Using a VID/PID as described in documentation:
   ```python
   FEATURES_REQUIRED+=riotboot USEMODULE+=usbus_dfu make -C examples/saul BOARD=nucleo-f767zi \
   DFU_USB_ID=1209:affe PROGRAMMER=dfu-util all riotboot/flash-slot0
   ```
   can't be compiled without this PR but calls `dfu-util` correctly with this PR using the default VID/PID:
   ```python
   dfu-util --device 1209:affe --alt 0 --download examples/saul/bin/nucleo-f767zi/riotboot_files/slot0.1678440536.bin
   ```
3. Compiling a board with DFU bootloader
   ```python
   make -C examples/saul flash BOARD=weact-f411ce
   ```
   should still call dfu-util with correct VID/PID:
   ```python
   dfu-util --device 0483:df11 --alt 0 --download /home/gs/src/RIOT-Xtensa-ESP.esp-idf-4.4/examples/saul/bin/weact-f411ce/saul_example.bin  --dfuse-address 0x8000000:leave
   ```
### Issues/PRs references



Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
@gschorcht gschorcht deleted the bootloaders/riotboot_dfu_fixes branch May 29, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: doc Area: Documentation Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework Area: USB Area: Universal Serial Bus CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants