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

Fix: Simplify debug log guard in imxrt.c #1818

Merged

Conversation

ALTracer
Copy link
Contributor

Detailed description

  • Not a feature.
  • Existing problem is a compiler error for firmware builds with DEBUG_TARGET() log statements enabled (unusual configuration, unreachable through meson/Makefiles alone).
  • The PR solves this problem by updating one macro guard.

This is a contextual follow-up to #1517, also see PR1533-PR1534, and really it's #1684 which enables this PR.

In file included from ../src/target/imxrt.c:34:
../src/target/imxrt.c: In function 'imxrt_probe':
../src/target/imxrt.c:224:50: error: 'boot_mode' undeclared (first use in this function)
  224 |         DEBUG_TARGET("i.MXRT boot mode is %x\n", boot_mode);
      |                                                  ^~~~~~~~~

Your checklist for this pull request

Closing issues

@ALTracer
Copy link
Contributor Author

Extras:

Updating this allows developers to have extra 3 KiB of TARGET loglevel diagnostics in platforms like blackpill-f411ce on top of existing 16 KiB of ERROR+WARN+INFO; or extra 2 KiB on native/stlink/swlink which disable some -Dtargets already.

diff --git a/src/include/general.h b/src/include/general.h
index 83a2e3a3..91f15796 100644
--- a/src/include/general.h
+++ b/src/include/general.h
@@ -102,2 +102,3 @@
 #define DEBUG_INFO(...)  PLATFORM_PRINTF(__VA_ARGS__)
+#define DEBUG_TARGET(...) PLATFORM_PRINTF(__VA_ARGS__)
 #else
@@ -109,2 +110,4 @@
 #define DEBUG_INFO_IS_NOOP
+#define DEBUG_TARGET(...) PRINT_NOOP(__VA_ARGS__)
+#define DEBUG_TARGET_IS_NOOP
 #endif
@@ -112,4 +115,2 @@
 #define DEBUG_GDB_IS_NOOP
-#define DEBUG_TARGET(...) PRINT_NOOP(__VA_ARGS__)
-#define DEBUG_TARGET_IS_NOOP
 #define DEBUG_PROTO(...) PRINT_NOOP(__VA_ARGS__)

Doing a $ grep -rn DEBUG_TARGET\( ./src/target/ | wc -l I see 33 instances of it, and also PROBE() which is an important indication for which cortexm_probe()-dispatched routines are invoked per discovered target.

@dragonmux dragonmux added this to the v2.0 release milestone May 10, 2024
@dragonmux dragonmux added the Enhancement General project improvement label May 10, 2024
Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, we'll merge this in the morning. Thank you for the contribution!

@dragonmux dragonmux merged commit 035542f into blackmagic-debug:main May 11, 2024
23 of 24 checks passed
@ALTracer ALTracer deleted the fix/imxrt_target_debug_noop branch August 20, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement General project improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants