Skip to content

cpu/stm32/periph/usbdev_fs: avoid using ztimer when not needed#20467

Merged
Teufelchen1 merged 1 commit intoRIOT-OS:masterfrom
dylad:pr/cpu/stm32/usbdev_fs/dont_autopull_ztimer
Mar 20, 2024
Merged

cpu/stm32/periph/usbdev_fs: avoid using ztimer when not needed#20467
Teufelchen1 merged 1 commit intoRIOT-OS:masterfrom
dylad:pr/cpu/stm32/usbdev_fs/dont_autopull_ztimer

Conversation

@dylad
Copy link
Copy Markdown
Member

@dylad dylad commented Mar 13, 2024 •
edited
Loading

Contribution description

Avoid pulling ztimer and ztimer_msec dependencies by default when using usbdev_fs peripheral driver.
If another module needs ztimer, then we can keep using it but otherwise use busy_wait() to perform the small delay needed by the driver.
My main idea is to avoid pulling ztimer when it is possible, to save ROM for riotboot_dfu bootloader application.

Testing procedure

make BOARD=bluepill-stm32f103c8 -C tests/sys/usbus_cdc_acm_stdio must compile (and ztimer should not appear if you use info-build)

make BOARD=bluepill-stm32f103c8 -C tests/sys/usbus_cdc_ecm must compile (and ztimer should appear if you use info-build because ztimer dependency is pulled by another module

Issues/PRs references

None.

@dylad dylad requested review from aabadie and vincent-d as code owners March 13, 2024 21:02
@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Mar 13, 2024
@Teufelchen1
Copy link
Copy Markdown
Contributor

Teufelchen1 commented Mar 19, 2024

I can not compile make BOARD=bluepill-stm32f103c8 -C tests/sys/usbus_cdc_ecm:

 .. region `rom' overflowed by 1656 bytes

But thats also the case on master? 😕

@dylad
Copy link
Copy Markdown
Member Author

dylad commented Mar 19, 2024 •
edited
Loading

That's weird, I just compile make BOARD=bluepill-stm32f103c8 -C tests/sys/usbus_cdc_ecm from master and from this PR. Both compile flawlessly.
May I ask you your output of dist/tools/ci/print_toolchain_versions.sh ?

@Teufelchen1
Copy link
Copy Markdown
Contributor

Teufelchen1 commented Mar 19, 2024

Sure!

Operating System Environment
----------------------------
         Operating System: "Ubuntu" "22.04.4 LTS (Jammy Jellyfish)"
                   Kernel: Linux 6.5.0-25-generic x86_64 x86_64
             System shell: /usr/bin/dash (probably dash)
             make's shell: /usr/bin/dash (probably dash)

Installed compiler toolchains
-----------------------------
               native gcc: gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
        arm-none-eabi-gcc: arm-none-eabi-gcc (15:10.3-2021.07-4) 10.3.1 20210621 (release)
                  avr-gcc: avr-gcc (GCC) 5.4.0
           msp430-elf-gcc: missing
       riscv-none-elf-gcc: missing
  riscv64-unknown-elf-gcc: riscv64-unknown-elf-gcc () 10.2.0
      riscv32-esp-elf-gcc: riscv32-esp-elf-gcc (crosstool-NG esp-12.2.0_20230208) 12.2.0
     xtensa-esp32-elf-gcc: missing
   xtensa-esp32s2-elf-gcc: missing
   xtensa-esp32s3-elf-gcc: missing
   xtensa-esp8266-elf-gcc: missing
                    clang: Ubuntu clang version 14.0.0-1ubuntu1.1

Installed compiler libs
-----------------------
     arm-none-eabi-newlib: "3.3.0"
        msp430-elf-newlib: missing
    riscv-none-elf-newlib: missing
riscv64-unknown-elf-newlib: missing
   riscv32-esp-elf-newlib: "4.1.0"
  xtensa-esp32-elf-newlib: missing
xtensa-esp32s2-elf-newlib: missing
xtensa-esp32s3-elf-newlib: missing
xtensa-esp8266-elf-newlib: missing
                 avr-libc: "2.0.0" ("20150208")

Installed development tools
---------------------------
                   ccache: missing
                    cmake: cmake version 3.22.1
                 cppcheck: missing
                  doxygen: 1.9.1
                      git: git version 2.34.1
                     make: GNU Make 4.3
                  openocd: Open On-Chip Debugger 0.11.0
                   python: missing
                  python2: missing
                  python3: Python 3.10.12
                   flake8: 4.0.1 (mccabe: 0.6.1, pycodestyle: 2.8.0, pyflakes: 2.4.0) CPython 3.10.12 on
               coccinelle: spatch version 1.1.1 compiled with OCaml version 4.13.1

@dylad
Copy link
Copy Markdown
Member Author

dylad commented Mar 19, 2024

I'll try to reproduce this week on a computer at my office w/ Ubuntu on it.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 19, 2024
@riot-ci
Copy link
Copy Markdown

riot-ci commented Mar 19, 2024

Murdock results

✔️ PASSED

df044f4 cpu/stm32/periph/usbdev_fs: avoid using ztimer when not needed

Success Failures Total Runtime
10009 0 10009 09m:18s

Artifacts

@dylad
Copy link
Copy Markdown
Member Author

dylad commented Mar 20, 2024

@Teufelchen1 It seems you're hitting a toolchain issue w/ Ubuntu default one.
I can reproduce it with an Ubuntu 20.4.6 with Ubuntu default toolchain. But I have no issue using a standalone toolchain from ARM. I guess Ubuntu default toolchain is using newlib instead of newlib-nano...
But anyways, this is not related to this PR. If you want to test this PR, I suggest you to try a standalone toolchain from ARM or to switch to picolibc (if it's installed on your system) by using FEATURES_PROVIDED=picolibc

@Teufelchen1
Copy link
Copy Markdown
Contributor

Teufelchen1 commented Mar 20, 2024

Thanks for checking up!
I already saw the expected behavior in the build log, even though the build failed. Since it's clear the failure is unrelated and murdock is happy, we can move this forward :)

@Teufelchen1 Teufelchen1 added this pull request to the merge queue Mar 20, 2024
Merged via the queue into RIOT-OS:master with commit 30e745d Mar 20, 2024
@dylad
Copy link
Copy Markdown
Member Author

dylad commented Mar 21, 2024

Thanks @Teufelchen1 !

@dylad dylad deleted the pr/cpu/stm32/usbdev_fs/dont_autopull_ztimer branch March 21, 2024 08:09
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Teufelchen1 Teufelchen1 Teufelchen1 approved these changes

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@vincent-d vincent-d Awaiting requested review from vincent-d vincent-d is a code owner

Assignees

No one assigned

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms

Projects

None yet

Milestone

Release 2024.04

Development

Successfully merging this pull request may close these issues.

5 participants

@dylad @Teufelchen1 @riot-ci @benpicco @MrKevinWeiss