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

[Randy Dunlap <rdunlap@infradead.org>]

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


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


Thanks.
-- 
~Randy