linguini1 commented on code in PR #17011: URL: https://github.com/apache/nuttx/pull/17011#discussion_r2691953616
########## boards/arm/mps/mps2-an500/include/board.h: ########## @@ -33,6 +33,12 @@ * Pre-processor Definitions ****************************************************************************/ +#warning "CONFIG_BOARD_LOOPSPERMSEC is set to 0 to prevent CI build errors." \ Review Comment: This is confusing me because in an earlier review comment you asked me to remove the default value for LOOPSPERMSEC entirely: > let's remove the default value to enforce the user must provide a value in defconfig and remove static_assert in ALL files https://github.com/apache/nuttx/pull/17011#discussion_r2638794701 > I think I am better understanding your request now, but I have a suggestion for a modified solution: > - Keep CONFIG_BOARD_LOOPSPERMSEC with no default value in general OK. > - Add `default 0 if ALARM_ARCH` so that boards that use the alarm lower-half don't get an error let's add this special handle to https://github.com/apache/nuttx/pull/17923. > - Add a runtime `DEBUGASSERT(CONFIG_BOARD_LOOPSPERMSEC != 0)` in the arch_alarm `udelay_coarse` so that if it attempts to use the busy-loop delay (before the oneshot timer is registered) the assertion will be triggered Yes. > - Remove the warning statements from all the board.h Yes. > Some of the boards which don't have `CONFIG_BOARD_LOOPSPERMSEC` configured also do not use `CONFIG_ALARM_ARCH`. These are the remaining problems that would break CI. So, to match your suggestion, I can place the `#warning` where `up_udelay` is implemented so that if `CONFIG_ALARM_ARCH` is not enabled and `LOOPSPERMSEC=0`, these boards will have a warning. This removes the warning from the `board.h` files. Yes, to avoid add CONFIG_BOARD_LOOPSPERMSEC=0 to all problem defconfig, we can add ``` #ifndef CONFIG_BOARD_LOOPSPERMSEC # warning xxxx #endif ``` near up_delay implementation. If all boards already config the right number, we can change `#warning` to `#error`. What do you think about that solution instead @xiaoxiang781216 ? looks good. -- 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]
