[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250907153547.5d4271d9@foz.lan>
Date: Sun, 7 Sep 2025 15:35:47 +0200
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: Jani Nikula <jani.nikula@...ux.intel.com>, linux-kernel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>, Greg Kroah-Hartman
<gregkh@...uxfoundation.org>, Pavel Machek <pavel@....cz>, Len Brown
<len.brown@...el.com>, linux-pm@...r.kernel.org, Jonathan Corbet
<corbet@....net>, linux-doc@...r.kernel.org, "James E.J. Bottomley"
<James.Bottomley@...senpartnership.com>
Subject: Re: [PATCH v4] kernel.h: add comments for system_states
Em Sat, 6 Sep 2025 23:30:28 +0200
Mauro Carvalho Chehab <mchehab+huawei@...nel.org> escreveu:
> Em Sat, 6 Sep 2025 10:13:23 -0700
> Randy Dunlap <rdunlap@...radead.org> escreveu:
>
> > On 9/6/25 1:56 AM, Mauro Carvalho Chehab wrote:
> > > Em Fri, 5 Sep 2025 15:07:31 -0700
> > > Randy Dunlap <rdunlap@...radead.org> escreveu:
> > >
> > >> Hi,
> > >>
> > >> On 9/5/25 6:38 AM, Mauro Carvalho Chehab wrote:
> > >>> On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote:
> > >>>> On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@...nel.org> wrote:
> > >>>>> Em Fri, 05 Sep 2025 12:02:37 +0300
> > >>>>> Jani Nikula <jani.nikula@...ux.intel.com> escreveu:
> > >>>>>
> > >>>>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@...radead.org> wrote:
> > >>>>>>> Provide some basic comments about the system_states and what they imply.
> > >>>>>>> Also convert the comments to kernel-doc format.
> > >>>>>>>
> > >>>>>>> However, kernel-doc does not support kernel-doc notation on extern
> > >>>>>>> struct/union/typedef/enum/etc. So I made this a DOC: block so that
> > >>>>>>> I can use (insert) it into a Documentation (.rst) file and have it
> > >>>>>>> look decent.
> > >>>>>>
> > >>>>>> The simple workaround is to separate the enum type and the variable:
> > >>>>>>
> > >>>>>> /**
> > >>>>>> * kernel-doc for the enum
> > >>>>>> */
> > >>>>>> enum system_states {
> > >>>>>> ...
> > >>>>>> };
> > >>>>>>
> > >>>>>> /**
> > >>>>>> * kernel-doc for the variable
> > >>>>>> */
> > >>>>>> extern enum system_states system_state;
> > >>>>>>
> > >>>>>> BR,
> > >>>>>> Jani.
> > >>>>>>
> > >>>>>>>
> > >>>>>>> Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
> > >>>>>>> Acked-by: Rafael J. Wysocki <rafael@...nel.org> # v1
> > >>>>>>> ---
> > >>
> > >> [snip]
> > >>>>> If the problem is with "extern" before enum, fixing kdoc be
> > >>>>> fairly trivial.
> > >>>>
> > >>>> The non-trivial part is deciding whether you're documenting the enum
> > >>>> type or the variable. Both are equally valid options.
> > >>>
> > >>> True.
> > >>>
> > >>> I'd say that, if the variable is under EXPORT_SYMBOL macros, it
> > >>> should be documented.
> > >>
> > >> Do you mean documented with kernel-doc? How do we do that?
> > >> I was under the impression that we don't currently have a way to
> > >> use kernel-doc for variables (definitions, only for declarations).
> > >
> > > No, but it shouldn't be hard to add something like:
> > >
> > > /**
> > > * global system_state - stores system state
> > > * <some description>
> > > */
> > > extern enum system_states system_state;
> > >
> > > and place a dump_global() function at kdoc parser. Ok, one might use
> > > DOC:, but IMHO the best is to have a proper handler for it that would
> > > automatically pick the type.
> >
> > Nitpick, I would s/global/var/. But I won't complain about "global".
>
> Either way works for me. Yet, I would expect it to be used only for
> global variables, as documenting local ones using kernel-doc is
> probably not what we expect inside Kernel documentation. So, naming it
> "global" may help.
>
> > More importantly, I have seen several patches where people try to document a
> > variable, so it seems like something that we should support at some point.
>
> Agreed.
>
> Adding a parsing rule for the variable doesn't sound hard, as they follow,
> in principle, this regex(*):
>
> OPTIONAL_ATTRIBS = ["
> "extern"
> ]
>
> optional = "|".join(OPTIONAL_ATTRIBS)
>
> "^(?:extern\s+)?(\w.*)\s+([\w\_]+)(?:#.*)$"
>
> (*) eventually, some might have extra attributes, but we can
> start with a simpler regex, adding a more complex parser if/when
> needed.
>
> I added at the end a one-minute hacky prototype I wrote, completely
> untested and incomplete.
Heh, it doesn't hurt spending 15 mins or so to write something that actually
works and implement all functions.
The example below produces:
$ ./scripts/kernel-doc include/media/tuner-types.h
...
.. c:var:: tuners
list of tuners
extern const struct tunertype tuners[];
.. c:var:: tuner_count
number of known tuners
$ ./scripts/kernel-doc include/media/tuner-types.h --man
...
.TH "Kernel API" 9 "global tuner_count" "September 2025" "API Manual" LINUX
.SH NAME
extern unsigned const int tuner_count; \- number of known tuners
.SH SYNOPSIS
enum tuner_count {
.SH "SEE ALSO"
.PP
Still not ready for kernel merge (plus I placed bogus descriptions for
two externs from media that IMO doesn't make sense to document), but it
has all needed steps for someone wanting to extend kernel-doc to see
how to do it.
Feel free to modify it - even renaming from "global" to "var" and
submit upstream.
Thanks,
Mauro
[PATCH] [RFC] kernel-doc: add support for handling global varaibles
Add support for documenting global variables with kernel-doc.
Please notice that this is mostly an example, as:
1. I'm documenting just two random variables from media, that
doesn't make sense to actually be documented. I did it just
to have some example and be able to test it;
2. the html output requires tweak: right now, it is just printing
the entire variable prototype as-is, without any formatting,
and witout making sense at the output
Feel free to modify this patch to make it something actually
mergeable.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
diff --git a/include/media/tuner-types.h b/include/media/tuner-types.h
index c79b773f750c..cf074beaeccc 100644
--- a/include/media/tuner-types.h
+++ b/include/media/tuner-types.h
@@ -199,7 +199,18 @@ struct tunertype {
u8 *sleepdata;
};
+/**
+ * global tuner - list of tuners
+ *
+ * List of all tuners defined via v4l2 API
+ */
extern const struct tunertype tuners[];
+
+/**
+ * global tune_count - number of known tuners
+ *
+ * Number of tuners at @tuners list
+ */
extern unsigned const int tuner_count;
#endif
diff --git a/scripts/lib/kdoc/kdoc_output.py b/scripts/lib/kdoc/kdoc_output.py
index 1eca9a918558..a58562bef35a 100644
--- a/scripts/lib/kdoc/kdoc_output.py
+++ b/scripts/lib/kdoc/kdoc_output.py
@@ -199,6 +199,10 @@ class OutputFormat:
self.out_enum(fname, name, args)
return self.data
+ if dtype == "global":
+ self.out_global(fname, name, args)
+ return self.data
+
if dtype == "typedef":
self.out_typedef(fname, name, args)
return self.data
@@ -227,6 +231,9 @@ class OutputFormat:
def out_enum(self, fname, name, args):
"""Outputs an enum"""
+ def out_global(self, fname, name, args):
+ """Outputs a global variable"""
+
def out_typedef(self, fname, name, args):
"""Outputs a typedef"""
@@ -472,6 +479,20 @@ class RestFormat(OutputFormat):
self.lineprefix = oldprefix
self.out_section(args)
+ def out_global(self, fname, name, args):
+ oldprefix = self.lineprefix
+ ln = args.declaration_start_line
+
+ self.data += f"\n\n.. c:var:: {name}\n\n"
+
+ self.print_lineno(ln)
+ self.lineprefix = " "
+ self.output_highlight(args.get('purpose', ''))
+ self.data += "\n"
+
+ # FIXME: better handle it
+ self.data += args.other_stuff["var_type"]
+
def out_typedef(self, fname, name, args):
oldprefix = self.lineprefix
@@ -772,6 +793,18 @@ class ManFormat(OutputFormat):
self.data += f'.SH "{section}"' + "\n"
self.output_highlight(text)
+ def out_global(self, fname, name, args):
+ out_name = self.arg_name(args, name)
+ prototype = args.other_stuff["var_type"]
+
+ self.data += f'.TH "{self.modulename}" 9 "{out_name}" "{self.man_date}" "API Manual" LINUX' + "\n"
+
+ self.data += ".SH NAME\n"
+ self.data += f"{prototype} \\- {args['purpose']}\n"
+
+ self.data += ".SH SYNOPSIS\n"
+ self.data += f"enum {name}" + " {\n"
+
def out_typedef(self, fname, name, args):
module = self.modulename
purpose = args.get('purpose')
diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
index 574972e1f741..e2a3f4574894 100644
--- a/scripts/lib/kdoc/kdoc_parser.py
+++ b/scripts/lib/kdoc/kdoc_parser.py
@@ -64,7 +64,7 @@ type_param = KernRe(r"@(\w*((\.\w+)|(->\w+))*(\.\.\.)?)", cache=False)
# Tests for the beginning of a kerneldoc block in its various forms.
#
doc_block = doc_com + KernRe(r'DOC:\s*(.*)?', cache=False)
-doc_begin_data = KernRe(r"^\s*\*?\s*(struct|union|enum|typedef)\b\s*(\w*)", cache = False)
+doc_begin_data = KernRe(r"^\s*\*?\s*(struct|union|enum|typedef|global)\b\s*(\w*)", cache = False)
doc_begin_func = KernRe(str(doc_com) + # initial " * '
r"(?:\w+\s*\*\s*)?" + # type (not captured)
r'(?:define\s+)?' + # possible "define" (not captured)
@@ -886,6 +886,27 @@ class KernelDoc:
self.output_declaration('enum', declaration_name,
purpose=self.entry.declaration_purpose)
+ def dump_global(self, ln, proto):
+ """
+ Stores global variables that are part of kAPI.
+ """
+ VAR_ATTRIBS = [
+ "extern",
+ ]
+ OPTIONAL_VAR_ATTR = "^(?:" + "|".join(VAR_ATTRIBS) + ")?"
+
+ r= KernRe(OPTIONAL_VAR_ATTR + r"(\w.*)\s+([\w_]+)[\d\]\[]*\s*;(?:#.*)?$")
+ if not r.match(proto):
+ self.emit_msg(ln,f"{proto}: can't parse variable")
+ return
+
+ declaration_name = r.group(2)
+ var_type = r.group(0)
+
+ self.output_declaration("global", declaration_name,
+ var_type=var_type,
+ purpose=self.entry.declaration_purpose)
+
def dump_declaration(self, ln, prototype):
"""
Stores a data declaration inside self.entries array.
@@ -897,6 +918,8 @@ class KernelDoc:
self.dump_typedef(ln, prototype)
elif self.entry.decl_type in ["union", "struct"]:
self.dump_struct(ln, prototype)
+ elif self.entry.decl_type == "global":
+ self.dump_global(ln, prototype)
else:
# This would be a bug
self.emit_message(ln, f'Unknown declaration type: {self.entry.decl_type}')
Powered by blists - more mailing lists