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

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

Merged

Conversation

dylad
Copy link
Member

@dylad dylad commented Mar 13, 2024

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.

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>
@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
Contributor

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
Member Author

dylad commented Mar 19, 2024

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
Contributor

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
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

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
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
Contributor

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
27 checks passed
@dylad
Copy link
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
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
Development

Successfully merging this pull request may close these issues.

5 participants