casaroli opened a new pull request, #20923: URL: https://github.com/apache/nuttx/pull/20923
## Summary In NuttX `CLOCKS_PER_SEC` is the system tick rate today (`1000000 / CONFIG_USEC_PER_TICK`), and `clock()` returns `clock_systime_ticks()`. So the kernel uses `CLOCKS_PER_SEC` and `clock()` also where it means the tick rate and the tick counter. These are different things: `CLOCKS_PER_SEC` is the unit of `clock()`, and `CLK_TCK` is the tick rate (the unit of `times()` and `sysconf(_SC_CLK_TCK)`). XSI requires `CLOCKS_PER_SEC` to be 1000000 (issue #3516). The dev@ thread "[DISCUSS] Breaking change: make CLOCKS_PER_SEC 1000000" (https://lists.apache.org/thread/bqxry570g9t63j63zmk4c5l82lybtx4s) proposes that change in three steps; this PR is step 2 and does not change behaviour. This PR uses `CLK_TCK` for the tick rate and the tick counter for tick values: - 23 files used `CLOCKS_PER_SEC` as the tick rate: timer setups (AVR, MIPS, misoc, ESP32-C3 legacy, SPARC, ESP32, ESP32-S2, ESP32-S3, Z16), USB and network driver timeouts (Kinetis, PIC32MX, RM57, AmebaZ, BCM43xxx), the PM activity governor, procfs uptime, CPU load, the critical section monitor and `sysconf(_SC_CLK_TCK)`. They use `CLK_TCK`. - The procfs critical section monitor and `rtw_get_current_time()` of the AmebaZ driver took `clock()` as ticks: kernel code, so they use `clock_systime_ticks()`. - `times()`, the user-space work queues (`libs/libc/wqueue`) and `work_timeleft()` took `clock()` as ticks. User space of a protected or kernel build cannot call `clock_systime_ticks()`, so the new `CLOCK2TICK()` in `<nuttx/clock.h>` converts a `clock()` value to ticks; while `CLOCKS_PER_SEC` equals `CLK_TCK` it is the value itself. `work_timeleft()` uses `clock_systime_ticks()` in the kernel and in a flat build. The first commit fixes older nxstyle errors in five of the files (`kinetis_usbdev.c`, `amebaz_driver.c`, `pic32mx_usbdev.c`, `activity_governor.c`, `bcmf_driver.c`), so that the change passes the style check: blank lines, line length and comment wrapping only. ## Impact None. `CLK_TCK` and `CLOCKS_PER_SEC` expand to the same tokens in `include/time.h`, `CLOCK2TICK(x)` is `(x)` today, and `clock()` in the kernel is `clock_systime_ticks()`. The objects of the changed files are the same as on master, except the calls of `clock()` that became calls of `clock_systime_ticks()` (see Testing). Not a breaking change. ## Testing The changed files built on master and with this PR; the objects compared with `objdump -dr` (code and relocations) and `objdump -s` (all sections but debug information): | Configuration | Objects | Result | |---|---|---| | qemu-armv8a:nsh + `CONFIG_SCHED_CRITMONITOR`, `CONFIG_SCHED_CPULOAD_CRITMONITOR`, `CONFIG_FS_PROCFS` (flat, arm64) | `sched_cpuload.o`, `sched_critmonitor.o`, `fs_procfsuptime.o`, `fs_procfscritmon.o`, `lib_sysconf.o`, `lib_times.o` | same | | qemu-armv8a:knsh + `CONFIG_LIBC_USRWORK`, `CONFIG_NET_TCP`, `CONFIG_NET_TCP_KEEPALIVE` (kernel, arm64) | user and kernel `lib_sysconf.o`, `lib_times.o`, `work_queue.o`, `work_usrthread.o` | same | | the same | `tcp_timer.o` (`work_timeleft()`) | same, except the call of `clock()` is now a call of `clock_systime_ticks()` | | rv-virt:pnsh64 + `CONFIG_LIBC_USRWORK` (protected, rv64) | user and kernel `lib_times.o`, `work_queue.o`, `work_usrthread.o` | same | | esp32s3-devkit:nsh, esp32-devkitc:nsh, esp32s2-saola-1:nsh (xtensa) | `esp32s3_timerisr.o`, `esp32_timerisr.o`, `esp32s2_timerisr.o` | same | | esp32c3-legacy-devkit:nsh (rv32) | `esp32c3_timerisr.o` | same | | teensy-3.x:usbnsh (armv7-m) | `kinetis_usbdev.o` (also the nxstyle commit) | same | | rm57l843-launchxl2:ethnsh (armv7-r) | `rm57_ethernet.o` | same | | stm32f4discovery:pm (armv7-m) | `activity_governor.o` | same | The line of `fs_procfscritmon.c` that changes is not built in this configuration; it is the same one-word change as in `amebaz_depend.c`, from `clock()` to `clock_systime_ticks()`, which `clock()` returns in the kernel. The other files (AVR, MIPS, SPARC, Z16, misoc, rtl8720c, bcm43xxx) need toolchains or vendor code that I do not have; their change is the same one-word replacement. Test program `clktick.c` on qemu-armv8a:knsh (QEMU with HVF) with `CONFIG_LIBC_USRWORK`: `sysconf(_SC_CLK_TCK)` is `CLK_TCK`; `times()` and `clock()` measure a 0.5 s sleep; a user-space work item queued with a delay of `CLK_TCK / 2` ticks has `work_timeleft()` of about `CLK_TCK / 2` and runs after 0.5 s: | Case | master | this PR | |---|---|---| | `sysconf(_SC_CLK_TCK)` == `CLK_TCK` (1000) | PASS | PASS | | `times()` over 0.5 s: 501 ms | PASS | PASS | | `clock()` over 0.5 s: 501 ms | PASS | PASS | | `work_timeleft()` after queueing with 500 ticks: 501 | PASS | PASS | | the work runs after 501 ms | PASS | PASS | ostest exits with status 0 on qemu-armv8a:knsh (with `CONFIG_LIBC_USRWORK`) and qemu-armv8a:nsh. Build, each OK: qemu-armv8a:nsh (flat, arm64), qemu-armv8a:knsh (kernel, arm64), rv-virt:nsh64 (flat, rv64), rv-virt:pnsh64 (protected, rv64), rv-virt:knsh64 (kernel, rv64), lm3s6965-ek:qemu-protected (protected, armv7-m), and the configurations in the table. `tools/checkpatch.sh -c -u -m -g` passes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
