Re: [PATCH v4] kernel.h: add comments for system_states

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Sun, 7 Sep 2025 15:35:47 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> Em Sat, 6 Sep 2025 23:30:28 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> 
> > Em Sat, 6 Sep 2025 10:13:23 -0700
> > Randy Dunlap <rdunlap@infradead.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@infradead.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@kernel.org> wrote:      
> > > >>>>> Em Fri, 05 Sep 2025 12:02:37 +0300
> > > >>>>> Jani Nikula <jani.nikula@linux.intel.com> escreveu:
> > > >>>>>      
> > > >>>>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@infradead.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@infradead.org>
> > > >>>>>>> Acked-by: Rafael J. Wysocki <rafael@kernel.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@kernel.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"]
> +

Heh, looking at Sphinx doc at:
https://www.sphinx-doc.org/en/master/usage/domains/c.html:

	.. c:member:: declaration
	.. c:var:: declaration

	    Describes a C struct member or variable. Example signature:

	    .. c:member:: PyObject *PyTypeObject.tp_bases

	    The difference between the two directives is only cosmetic.

I guess the best is to encode it as:

	prototype = args.other_stuff["var_type"]
	self.data += f"\n\n.. c:var:: {prototype}\n\n"

And let Sphinx format it for us. With such change, this is
the html output for the two test variables we added at the RFC:

<dl class="c var">
<dt class="sig sig-object c" id="c.tuners">
<span class="k"><span class="pre">extern</span></span><span class="w"> </span><span class="k"><span class="pre">const</span></span><span class="w"> </span><span class="k"><span class="pre">struct</span></span><span class="w"> </span><a class="reference internal" href="#c.tunertype" title="tunertype"><span class="n"><span class="pre">tunertype</span></span></a><span class="w"> </span><span class="sig-name descname"><span class="n"><span class="pre">tuners</span></span></span><span class="p"><span class="pre">[</span></span><span class="p"><span class="pre">]</span></span><span class="p"><span class="pre">;</span></span><a class="headerlink" href="#c.tuners" title="Link to this definition">¶</a><br /></dt>
<dd><p>list of tuners</p>
</dd></dl>

<dl class="c var">
<dt class="sig sig-object c">
<span class="sig-name descname"><span class="pre">extern</span> <span class="pre">unsigned</span> <span class="pre">const</span> <span class="pre">int</span> <span class="pre">tuner_count;</span></span></dt>
<dd><p>number of known tuners</p>

RFC patch with the test vars enclosed. I'll submit the kernel-doc
patch in separate, without V4L2 stuff, for its review.

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@kernel.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..405a5c407522 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,18 @@ 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
+        prototype = args.other_stuff["var_type"]
+
+        self.data += f"\n\n.. c:var:: {prototype}\n\n"
+
+        self.print_lineno(ln)
+        self.lineprefix = "  "
+        self.output_highlight(args.get('purpose', ''))
+        self.data += "\n"
+
     def out_typedef(self, fname, name, args):
 
         oldprefix = self.lineprefix
@@ -772,6 +791,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}')