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

stm32: unified up_perf initialization #8040

Merged
merged 2 commits into from Jan 6, 2023

Conversation

Gary-Hobson
Copy link
Contributor

Summary

move up_perf_init from boards to arch

Impact

Testing

Signed-off-by: yinshengkai <yinshengkai@xiaomi.com>
Signed-off-by: yinshengkai <yinshengkai@xiaomi.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 4752dcd into apache:master Jan 6, 2023
@ALTracer
Copy link
Contributor

ALTracer commented Apr 3, 2023

Why does this PR break my build when enabling CONFIG_SCHED_IRQMONITOR? This looks like a regression from 12.0.0, it used to work and show somewhat true numbers in ps. I was building for stm32f411-minimum with modified config.

chip/stm32_start.c: In function '__start':
chip/stm32_start.c:161:24: error: 'STM32_SYSCLK_FREQUENCY' undeclared (first use in this function)
  161 |   up_perf_init((void *)STM32_SYSCLK_FREQUENCY);
      |                        ^~~~~~~~~~~~~~~~~~~~~~
chip/stm32_start.c:161:24: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [Makefile:157: stm32_start.o] Error 1

For example,

#ifdef CONFIG_SCHED_IRQMONITOR
up_perf_init((FAR void *)STM32_SYSCLK_FREQUENCY);
#endif
had #include <arch/board/board.h> which provides STM32_SYSCLK_FREQUENCY macro, visible in this translation unit. Most STM32 boards simply use DWT_CYCCNT (see arch/arm/src/armv7-m/arm_perf.c) and thus need to know MCU frequency.
Does CI check builds for configs with IRQMONITOR, like nucleo-h743zi2:jumbo in arm-12.dat? Yes, it does.
arch/arm/src/stm32/stm32_start.c doesn't have this #include, but arch/arm/src/stm32h7/stm32_start.c does, hence no CI build failure.

diff --git a/arch/arm/src/stm32/stm32_start.c b/arch/arm/src/stm32/stm32_start.c
index 08b7d21f0..72332d731 100644
--- a/arch/arm/src/stm32/stm32_start.c
+++ b/arch/arm/src/stm32/stm32_start.c
@@ -29,6 +29,7 @@
 #include <debug.h>
 
 #include <nuttx/init.h>
+#include <arch/board/board.h>
 
 #include "arm_internal.h"
 #include "nvic.h"

@Gary-Hobson
Copy link
Contributor Author

Gary-Hobson commented Apr 3, 2023

Why does this PR break my build when enabling CONFIG_SCHED_IRQMONITOR? This looks like a regression from 12.0.0, it > used to work and show somewhat true numbers in ps. I was building for stm32f411-minimum with modified config.

chip/stm32_start.c: In function '__start':
chip/stm32_start.c:161:24: error: 'STM32_SYSCLK_FREQUENCY' undeclared (first use in this function)
161 | up_perf_init((void *)STM32_SYSCLK_FREQUENCY);
| ^~~~~~~~~~~~~~~~~~~~~~
chip/stm32_start.c:161:24: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [Makefile:157: stm32_start.o] Error 1
For example,
[nuttx/boards/arm/stm32h7/nucleo-h743zi2/src/stm32_boot.c]> (https://github.com/apache/nuttx/blob/1f7b49d7003c9bf27ade13be6fcd177b4352299d/boards/arm/stm32h7/nucleo-> h743zi2/src/stm32_boot.c#L53-L55)

Lines 53 to 55 in 1f7b49d

#ifdef CONFIG_SCHED_IRQMONITOR
up_perf_init((FAR void *)STM32_SYSCLK_FREQUENCY);
#endif
had #include <arch/board/board.h> which provides STM32_SYSCLK_FREQUENCY macro, visible in this translation unit. Most > STM32 boards simply use DWT_CYCCNT (see arch/arm/src/armv7-m/arm_perf.c) and thus need to know MCU frequency.
Does CI check builds for configs with IRQMONITOR, like nucleo-h743zi2:jumbo in arm-12.dat? Yes, it does.
arch/arm/src/stm32/stm32_start.c doesn't have this #include, but arch/arm/src/stm32h7/stm32_start.c does, hence no CI > build failure.
diff --git a/arch/arm/src/stm32/stm32_start.c b/arch/arm/src/stm32/stm32_start.c
index 08b7d21f0..72332d731 100644
--- a/arch/arm/src/stm32/stm32_start.c
+++ b/arch/arm/src/stm32/stm32_start.c
@@ -29,6 +29,7 @@
#include <debug.h>

#include <nuttx/init.h>
+#include <arch/board/board.h>

#include "arm_internal.h"
#include "nvic.h"

Already submitted a PR to fix it: #8948

@ALTracer
Copy link
Contributor

ALTracer commented Apr 3, 2023

Thanks, I guess, will move the discussion there.
Why is my text in your quote suddenly Chinese? I don't speak it... Auto translation maybe?

@Gary-Hobson
Copy link
Contributor Author

Thanks, I guess, will move the discussion there. Why is my text in your quote suddenly Chinese? I don't speak it... Auto translation maybe?

yes, i fixed it

@jerpelea jerpelea added this to To-Add in Release Notes - 12.1.0 Apr 3, 2023
@jerpelea jerpelea moved this from To-Add to Added in Release Notes - 12.1.0 Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants