makefiles/boards/stm32: fix DFU_USB_ID handling #19374
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The variable `DFU_USB_ID` is used by `dfu-util` to specify the device to be flashed. The STM32 make system prevents `dfu-util` from being used as programmer if this variable is not set, although `dfu-util.mk` generates a default value for this variable from the `USB_VID` and `USB_PID` variables if necessary. Therefore, checking the `DFU_USB_ID` variable is removed here. If a board requires a specific combination of VID/PID for `dfu_util`, it is responsible for setting the `DFU_USB_ID` variable.
gschorcht
added
Type: bug
maribu
approved these changes
Mar 10, 2023
There was a problem hiding this comment.
This looks like there has been special handling for the black magic peobe DFU bootloader. But I suspect with riotboot_dfu now available there is little reason for users to stick with the bmp dfu bootloader.
And additionally the handling was broken anyway, which indicates that no active user of the bmp dfu bootloader was left anyway.
The only part that could have been special has already been removed with commit 3153a865 in PR #11192. Only the test for |
|
bors merge |
|
🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
|
bors merge |
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>
|
Build failed (retrying...): |
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>
|
Build failed (retrying...): |
|
Build succeeded: |
|
Thanks for reviewing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution description
This PR fixes the still existing problem that an application can't be flashed to a STM32 board if it uses
riotboot_dfuwith 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 examplealways leads to error
even if
DFU_USB_IDvariable is set as described in documentation.The reason is that the variable
DFU_USB_IDisn't exported and the checkRIOT/makefiles/boards/stm32.inc.mk
Lines 27 to 29 in 8dc8bf3
DFU_USB_IDvariable here. It prevents to usedfu-utilevent though the followingdfu-util.mkwill generate a default value for this variable from theUSB_VIDandUSB_PIDvariables 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-f4x1cxuses a DFU boorloader and requires a certain VID/PID combination, board's makefile is responsible to setDFU_USB_IDvariable.Testing procedure
It is not necessary to use a real boad, checking the compilation process is sufficient.
dfu-utilcorrectly with this PR using the default VID/PID:dfu-utilcorrectly with this PR using the default VID/PID:Issues/PRs references