Message-ID: <20250906233028.56573fd6@foz.lan>
Date: Sat, 6 Sep 2025 23:30:28 +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 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.

Thanks,
Mauro

The following code snippet is incomplete, not tested, and requires more
work, like placing global vars at the beginning. Feel free
to pick it and use it to produce a GPL code to the Kernel.

Note that this is just the parsing part. for it to work, you also
need to modify scripts/lib/kdoc/kdoc_output.py, which contains how
this will be output at rst and man formats.

diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
index 574972e1f741..b3ffaa541e9d 100644
--- a/scripts/lib/kdoc/kdoc_parser.py
+++ b/scripts/lib/kdoc/kdoc_parser.py
@@ -886,6 +886,25 @@ class KernelDoc:
         self.output_declaration('enum', declaration_name,
                                 purpose=self.entry.declaration_purpose)
 
+   def dump_global(self, ln, proto):
+        # TODO: move this to the beginning of this file
+        VAR_ATTRIBS = [
+            "extern",
+        ]
+        OPTIONAL_VAR_ATTR = "^(?:" + "|".join(VAR_ATTRIBS) + ")?"
+
+        type_var = KernRe(OPTIONAL_VAR_ATTR + r"(\w.*)\s+([\w_]+)(?:#.*)?$")
+
+        # function logic starts here:
+
+        if type_var.match(proto):
+                declaration_name = r.group(2)
+                var_type = r.group(1)
+
+        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 +916,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