From: Randy Dunlap <rdunlap@infradead.org>
To: Vegard Nossum <vegard.nossum@oracle.com>,
Karol Herbst <kherbst@redhat.com>, Lyude Paul <lyude@redhat.com>,
Danilo Krummrich <dakr@redhat.com>
Cc: nouveau@lists.freedesktop.org, Jonathan Corbet <corbet@lwn.net>,
Jani Nikula <jani.nikula@linux.intel.com>,
dri-devel@lists.freedesktop.org, linux-doc@vger.kernel.org
Subject: Re: [PATCH -next] drm/nouveau: uapi: fix kerneldoc warnings
Date: Sun, 24 Dec 2023 23:40:44 -0800 [thread overview]
Message-ID: <009fcdc4-b10a-4ab9-b368-7cea75bb74e2@infradead.org> (raw)
In-Reply-To: <20231225065145.3060754-1-vegard.nossum@oracle.com>
On 12/24/23 22:51, Vegard Nossum wrote:
> As of commit b77fdd6a48e6 ("scripts/kernel-doc: restore warning for
> Excess struct/union"), we see the following warnings when running 'make
> htmldocs':
>
> ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member 'DRM_NOUVEAU_VM_BIND_OP_MAP' description in 'drm_nouveau_vm_bind_op'
> ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member 'DRM_NOUVEAU_VM_BIND_OP_UNMAP' description in 'drm_nouveau_vm_bind_op'
> ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member 'DRM_NOUVEAU_VM_BIND_SPARSE' description in 'drm_nouveau_vm_bind_op'
> ./include/uapi/drm/nouveau_drm.h:336: warning: Excess struct member 'DRM_NOUVEAU_VM_BIND_RUN_ASYNC' description in 'drm_nouveau_vm_bind'
>
> The problem is that these values are #define constants, but had kerneldoc
> comments attached to them as if they were actual struct members.
>
> There are a number of ways we could fix this, but I chose to draw
> inspiration from include/uapi/drm/i915_drm.h, which pulls them into the
> corresponding kerneldoc comment for the struct member that they are
> intended to be used with.
>
> To keep the diff readable, there are a number of things I _didn't_ do in
> this patch, but which we should also consider:
>
> - This is pretty good documentation, but it ends up in gpu/driver-uapi,
> which is part of subsystem-apis/ when it really ought to display under
> userspace-api/ (the "Linux kernel user-space API guide" book of the
> documentation).
>
> - More generally, we might want a warning if include/uapi/ files are
> kerneldoc'd outside userspace-api/.
>
> - I'd consider it cleaner if the #defines appeared between the kerneldoc
> for the member and the member itself (which is something other DRM-
> related UAPI docs do).
>
> - The %IDENTIFIER kerneldoc syntax is intended for "constants", and is
> more appropriate in this context than ``IDENTIFIER`` or &IDENTIFIER.
> The DRM docs aren't very consistent on this.
>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
This all looks good to me. Thanks for your help.
Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
I do see one thing that I don't like in the generated html output.
It's not a problem with this patch.
The #defines for DRM_NOUVEAU_VM_BIND_OP_MAP etc. have a ';' at the
end of each line:
struct drm_nouveau_vm_bind_op {
__u32 op;
#define DRM_NOUVEAU_VM_BIND_OP_MAP 0x0;
#define DRM_NOUVEAU_VM_BIND_OP_UNMAP 0x1;
__u32 flags;
#define DRM_NOUVEAU_VM_BIND_SPARSE (1 << 8);
__u32 handle;
__u32 pad;
__u64 addr;
__u64 bo_offset;
__u64 range;
};
so something else to look into one of these days....
> ---
> include/uapi/drm/nouveau_drm.h | 56 ++++++++++++++++------------------
> 1 file changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
> index 0bade1592f34..c95ef8a4d94a 100644
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
next prev parent reply other threads:[~2023-12-25 7:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-25 6:51 [PATCH -next] drm/nouveau: uapi: fix kerneldoc warnings Vegard Nossum
2023-12-25 7:40 ` Randy Dunlap [this message]
2023-12-25 8:30 ` Vegard Nossum
2023-12-25 17:08 ` Randy Dunlap
2024-01-03 3:10 ` Randy Dunlap
2024-01-05 5:31 ` Randy Dunlap
2024-01-08 19:54 ` Danilo Krummrich
2024-01-09 10:10 ` Jani Nikula
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=009fcdc4-b10a-4ab9-b368-7cea75bb74e2@infradead.org \
--to=rdunlap@infradead.org \
--cc=corbet@lwn.net \
--cc=dakr@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
--cc=kherbst@redhat.com \
--cc=linux-doc@vger.kernel.org \
--cc=lyude@redhat.com \
--cc=nouveau@lists.freedesktop.org \
--cc=vegard.nossum@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).