Re: [patch 06/15] timers: Update kernel-doc for various functions

[Randy Dunlap]

On 11/21/22 12:43, Steven Rostedt wrote:
> On Tue, 15 Nov 2022 21:28:43 +0100 (CET)
> Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> 
>> The kernel-doc of timer related functions is partially uncomprehensible
>> word salad. Rewrite it to make it useful.
>>
>> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> ---
>>  kernel/time/timer.c |  131 ++++++++++++++++++++++++++++++----------------------
>>  1 file changed, 77 insertions(+), 54 deletions(-)
>>
>> --- a/kernel/time/timer.c
>> +++ b/kernel/time/timer.c
>> @@ -1121,14 +1121,16 @@ static inline int
>>  }
>>  
>>  /**
>> - * mod_timer_pending - modify a pending timer's timeout
>> - * @timer: the pending timer to be modified
>> - * @expires: new timeout in jiffies
>> + * mod_timer_pending - Modify a pending timer's timeout
>> + * @timer:	The pending timer to be modified
>> + * @expires:	New timeout in jiffies
>>   *
>> - * mod_timer_pending() is the same for pending timers as mod_timer(),
>> - * but will not re-activate and modify already deleted timers.
>> + * mod_timer_pending() is the same for pending timers as mod_timer(), but
>> + * will not activate inactive timers.
>>   *
>> - * It is useful for unserialized use of timers.
>> + * Return:
>> + * * %0 - The timer was inactive and not modified
>> + * * %1 - The timer was active and requeued to expire at @expires
> 
> I didn't know of the '%' option in kernel-doc.
> 
> Looking it up, I see it's for constants. Although it's missing in the
> examples for return values:
> 
>   Documentation/doc-guide/kernel-doc.rst:
> 
> ```
> Return values
> ~~~~~~~~~~~~~
> 
> The return value, if any, should be described in a dedicated section
> named ``Return``.
> 
> .. note::
> 
>   #) The multi-line descriptive text you provide does *not* recognize
>      line breaks, so if you try to format some text nicely, as in::
> 
>         * Return:
>         * 0 - OK
>         * -EINVAL - invalid argument
>         * -ENOMEM - out of memory
> 
>      this will all run together and produce::
> 
>         Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory
> 
>      So, in order to produce the desired line breaks, you need to use a
>      ReST list, e. g.::
> 
>       * Return:
>       * * 0             - OK to runtime suspend the device
>       * * -EBUSY        - Device should not be runtime suspended
> ```
> 
> Should this be updated?

Sure. Do you want to do it?

-- 
~Randy