Em Fri, 5 Sep 2025 15:07:31 -0700
Randy Dunlap <rdunlap@xxxxxxxxxxxxx> 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@xxxxxxxxxx> wrote:
> >>> Em Fri, 05 Sep 2025 12:02:37 +0300
> >>> Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> escreveu:
> >>>
> >>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> >>>>> Acked-by: Rafael J. Wysocki <rafael@xxxxxxxxxx> # 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.
> > The same applies to the enum: if it is used by kAPI, it should
> > also be documented.
> >
> > So, yeah, I suspect that on most kAPI cases, the best is to split,
> > having documentation for both.
> >
> >>
> >> BR,
> >> Jani.
> >>
> >>>
> >>> If you see:
> >>>
> >>> def dump_function(self, ln, prototype):
> >>>
> >>> # Prefixes that would be removed
> >>> sub_prefixes = [
> >>> ...
> >>> (r"^extern +", "", 0),
> >>> ...
> >>> }
> >>>
> >>> for search, sub, flags in sub_prefixes:
> >>> prototype = KernRe(search, flags).sub(sub, prototype)
> >>>
> >>>
> >>> we need something similar to that at:
> >>> def dump_enum(self, ln, proto):
> >>>
> >>> alternatively, we could use a simpler approach modifying adding a
> >>> non-group optional match for "extern". E.g:
> >>>
> >>> - r = KernRe(r'enum\s+(\w*)\s*\{(.*)\}')
> >>> + r = KernRe(r'(?:extern\s+)?enum\s+(\w*)\s*\{(.*)\}')
> >>>
> >>> (untested)
>
> Yes, this might work. It might also lead to documenting more than
> was intended. It seems safer just to do the enum declaration and
> variable definition split light Jani suggested.
Yeah, the extra var at the end may be problematic, although it
wouldn't be hard to drop or parse it, like:
/**
* enum system_states - Values used for system_state.
*
* @SYSTEM_BOOTING: %0, no init needed
* @SYSTEM_SCHEDULING: system is ready for scheduling; OK to use RCU
* @SYSTEM_FREEING_INITMEM: system is freeing all of initmem; almost running
* @SYSTEM_RUNNING: system is up and running
* @SYSTEM_HALT: system entered clean system halt state
* @SYSTEM_POWER_OFF: system entered shutdown/clean power off state
* @SYSTEM_RESTART: system entered emergency power off or normal restart
* @SYSTEM_SUSPEND: system entered suspend or hibernate state
*
* @system_state: global var describing the current system state.
*
* Note:
* Ordering of the states must not be changed
* as code checks for <, <=, >, >= STATE.
*/
extern enum system_states {
SYSTEM_BOOTING,
SYSTEM_SCHEDULING,
SYSTEM_FREEING_INITMEM,
SYSTEM_RUNNING,
SYSTEM_HALT,
SYSTEM_POWER_OFF,
SYSTEM_RESTART,
SYSTEM_SUSPEND,
} system_state;
making it create two separate entries: one for the enum, and another one for
the global var - internally calling dump_global() for the var.
Thanks,
Mauro
