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

Randy Dunlap
2025-09-06T17:13:27Z

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".

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.

thanks.
--
~Randy