public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/gpusvm: Drop redundant @flags.* kernel-doc on struct drm_gpusvm_pages
@ 2026-05-01 17:13 Shuicheng Lin
  2026-05-04 23:13 ` Claude review: " Claude Code Review Bot
  2026-05-04 23:13 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Shuicheng Lin @ 2026-05-01 17:13 UTC (permalink / raw)
  To: dri-devel, intel-xe; +Cc: Shuicheng Lin, Matthew Auld

The kernel-doc block above struct drm_gpusvm_pages duplicates the
descriptions of the bit-flags that live in struct drm_gpusvm_pages_flags
using dotted notation (@flags.migrate_devmem, @flags.unmapped, ...).
That dotted notation is intended for nested anonymous structs/unions that
the parser flattens into the parent's parameter list.  Here, however,
flags is of a named external type, so the parser does not flatten its
members and the dotted entries do not match any member of
drm_gpusvm_pages.  They also duplicate the canonical descriptions already
present in the kernel-doc of struct drm_gpusvm_pages_flags itself.

Drop the five @flags.* lines and replace them with a single @flags entry
that cross-references the type via kernel-doc's "&struct ..." syntax.
This eliminates the redundancy and removes warnings emitted by the new
parameterdescs check in scripts/kernel-doc:

  Excess struct member 'flags.migrate_devmem' description in
   'drm_gpusvm_pages'
  Excess struct member 'flags.unmapped' description in 'drm_gpusvm_pages'
  Excess struct member 'flags.partial_unmap' description in
   'drm_gpusvm_pages'
  Excess struct member 'flags.has_devmem_pages' description in
   'drm_gpusvm_pages'
  Excess struct member 'flags.has_dma_mapping' description in
   'drm_gpusvm_pages'

No functional change.

Cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
---
The failure is reported by the new kernel-doc check in: https://patchwork.freedesktop.org/series/164948/
---
 include/drm/drm_gpusvm.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index 2578ac92a8d4..96eaf0328b4d 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -137,12 +137,7 @@ struct drm_gpusvm_pages_flags {
  * @dpagemap: The struct drm_pagemap of the device pages we're dma-mapping.
  *            Note this is assuming only one drm_pagemap per range is allowed.
  * @notifier_seq: Notifier sequence number of the range's pages
- * @flags: Flags for range
- * @flags.migrate_devmem: Flag indicating whether the range can be migrated to device memory
- * @flags.unmapped: Flag indicating if the range has been unmapped
- * @flags.partial_unmap: Flag indicating if the range has been partially unmapped
- * @flags.has_devmem_pages: Flag indicating if the range has devmem pages
- * @flags.has_dma_mapping: Flag indicating if the range has a DMA mapping
+ * @flags: Flags for the range; see &struct drm_gpusvm_pages_flags
  */
 struct drm_gpusvm_pages {
 	struct drm_pagemap_addr *dma_addr;
-- 
2.43.0


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

* Claude review: drm/gpusvm: Drop redundant @flags.* kernel-doc on struct drm_gpusvm_pages
  2026-05-01 17:13 [PATCH] drm/gpusvm: Drop redundant @flags.* kernel-doc on struct drm_gpusvm_pages Shuicheng Lin
  2026-05-04 23:13 ` Claude review: " Claude Code Review Bot
@ 2026-05-04 23:13 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-04 23:13 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/gpusvm: Drop redundant @flags.* kernel-doc on struct drm_gpusvm_pages
Author: Shuicheng Lin <shuicheng.lin@intel.com>
Patches: 1
Reviewed: 2026-05-05T09:13:58.409097

---

This is a single, straightforward kernel-doc cleanup patch. The problem is well-described: `struct drm_gpusvm_pages` uses `@flags.*` dotted notation in its kernel-doc comment to describe the members of the named sub-struct `drm_gpusvm_pages_flags`. Since `flags` is a named external type (not an anonymous inline struct), the kernel-doc parser does not flatten its members, producing "Excess struct member" warnings. The fix correctly removes the redundant dotted entries and replaces them with a cross-reference to the type.

The patch is correct, well-motivated, and the commit message is exemplary — it explains the parser semantics, quotes the exact warnings being fixed, and notes "No functional change."

**Verdict: Patch looks good.**

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/gpusvm: Drop redundant @flags.* kernel-doc on struct drm_gpusvm_pages
  2026-05-01 17:13 [PATCH] drm/gpusvm: Drop redundant @flags.* kernel-doc on struct drm_gpusvm_pages Shuicheng Lin
@ 2026-05-04 23:13 ` Claude Code Review Bot
  2026-05-04 23:13 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-04 23:13 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness:** The change is correct. Looking at the kernel tree, `struct drm_gpusvm_pages_flags` already has its own complete kernel-doc block (lines 108–117 in the header) documenting all five bit-flags (`@migrate_devmem`, `@unmapped`, `@partial_unmap`, `@has_devmem_pages`, `@has_dma_mapping`) plus `@__flags`. The dotted `@flags.*` entries in `struct drm_gpusvm_pages` were purely redundant.

**Replacement text is appropriate:** The new line:
```
 * @flags: Flags for the range; see &struct drm_gpusvm_pages_flags
```
uses the standard kernel-doc cross-reference syntax (`&struct ...`), which will produce a proper hyperlink in generated documentation. This is the idiomatic way to handle named sub-struct types.

**Minor nit (non-blocking):** The original `@flags` description said "Flags for range" and the replacement says "Flags for the range". This is fine — actually slightly more grammatical. However, the canonical doc in `struct drm_gpusvm_pages_flags` itself uses "pages" terminology (e.g., "Flag indicating if the pages has been unmapped"), while this says "range". Both are arguably correct since `drm_gpusvm_pages` is embedded in `drm_gpusvm_range`, but "Flags for the pages" would be more consistent with the naming of both the containing struct (`drm_gpusvm_pages`) and the flags struct (`drm_gpusvm_pages_flags`). This is purely cosmetic and not worth blocking on.

**No other concerns.** The diff is minimal, touches only documentation, and introduces no functional change.

Reviewed-by worthy as-is.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-05-04 23:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-01 17:13 [PATCH] drm/gpusvm: Drop redundant @flags.* kernel-doc on struct drm_gpusvm_pages Shuicheng Lin
2026-05-04 23:13 ` Claude review: " Claude Code Review Bot
2026-05-04 23:13 ` 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