public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm: Fix DYNAMIC_DEBUG_MODULE propagation to subdirectories
@ 2026-04-20 16:04 Sean Paul
  2026-04-22 22:49 ` Claude review: " Claude Code Review Bot
  2026-04-22 22:49 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Sean Paul @ 2026-04-20 16:04 UTC (permalink / raw)
  To: dri-devel
  Cc: Sean Paul, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Jim Cromie

From: Sean Paul <seanpaul@google.com>

Switch incompatible CFLAGS-y to ccflags-* and propagate the flag
to subdirectories to avoid implicit declaration errors for
_dynamic_func_call_cls when DRM_USE_DYNAMIC_DEBUG is enabled.

Note that this flag is still disabled due to depends on BROKEN.

Fixes: 84ec67288c10 ("drm_print: wrap drm_*_dbg in dyndbg descriptor factory macro")
Signed-off-by: Sean Paul <seanpaul@google.com>
---

a wild seanpaul appears...

 drivers/gpu/drm/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e97faabcd783..7082ff6449fb 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -3,7 +3,8 @@
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)	+= -DDYNAMIC_DEBUG_MODULE
+ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)	+= -DDYNAMIC_DEBUG_MODULE
+subdir-ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)	+= -DDYNAMIC_DEBUG_MODULE
 
 # Unconditionally enable W=1 warnings locally
 # --- begin copy-paste W=1 warnings from scripts/Makefile.warn
-- 
Sean Paul, Software Engineer, Google / Chromium OS


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: drm: Fix DYNAMIC_DEBUG_MODULE propagation to subdirectories
  2026-04-20 16:04 [PATCH] drm: Fix DYNAMIC_DEBUG_MODULE propagation to subdirectories Sean Paul
@ 2026-04-22 22:49 ` Claude Code Review Bot
  2026-04-22 22:49 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 22:49 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm: Fix DYNAMIC_DEBUG_MODULE propagation to subdirectories
Author: Sean Paul <sean@poorly.run>
Patches: 1
Reviewed: 2026-04-23T08:49:02.797692

---

This is a single-patch fix for a kbuild variable naming bug in the DRM subsystem Makefile. The original code used `CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)` which expands to `CFLAGS-y` — a variable name that kbuild does not recognize. As a result, `-DDYNAMIC_DEBUG_MODULE` was never actually passed to the compiler, meaning `DRM_USE_DYNAMIC_DEBUG` was silently broken at the build level (on top of being gated by `depends on BROKEN` in Kconfig).

The fix is correct and straightforward. The patch replaces the bogus variable with proper kbuild variables and adds subdirectory propagation. The practical impact is near-zero today since `DRM_USE_DYNAMIC_DEBUG` depends on `BROKEN`, but it's the right fix for when/if someone removes that dependency.

**Recommendation:** Looks good. One minor observation below.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: drm: Fix DYNAMIC_DEBUG_MODULE propagation to subdirectories
  2026-04-20 16:04 [PATCH] drm: Fix DYNAMIC_DEBUG_MODULE propagation to subdirectories Sean Paul
  2026-04-22 22:49 ` Claude review: " Claude Code Review Bot
@ 2026-04-22 22:49 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 22:49 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good.** The two changes are:

```makefile
-CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)	+= -DDYNAMIC_DEBUG_MODULE
+ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)	+= -DDYNAMIC_DEBUG_MODULE
+subdir-ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)	+= -DDYNAMIC_DEBUG_MODULE
```

1. `CFLAGS-y` is not a kbuild-recognized variable — this was simply dead code. Changing to `ccflags-y` is the correct fix (per `Documentation/kbuild/makefiles.rst:321`).

2. Adding `subdir-ccflags-y` propagates the flag to all subdirectories under `drivers/gpu/drm/`, which is essential since DRM drivers live in subdirectories like `amd/`, `i915/`, `nouveau/`, etc. Without this, files calling `drm_dbg()` and friends in those subdirectories would still get implicit-declaration errors for `_dynamic_func_call_cls`.

**Minor observation:** Per the kbuild documentation (`makefiles.rst:352-355`), `subdir-ccflags-y` applies to "*the kbuild file where they are present and all subdirectories*" — meaning it already covers the current directory. So the `ccflags-y` line is technically redundant; `subdir-ccflags-y` alone would suffice. This is harmless (duplicate `-D` flags for files directly in `drivers/gpu/drm/` don't cause problems), and being explicit about both scopes is a reasonable style choice. Not worth blocking over.

**Commit message:** Clear and accurate. The Fixes tag correctly references the commit that introduced the original `CFLAGS-y` typo. The note about `depends on BROKEN` is helpful context.

**Reviewed-by worthy:** Yes — this is a correct, minimal fix.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-22 22:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-20 16:04 [PATCH] drm: Fix DYNAMIC_DEBUG_MODULE propagation to subdirectories Sean Paul
2026-04-22 22:49 ` Claude review: " Claude Code Review Bot
2026-04-22 22:49 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox