Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6c104724-1095-6307-1dbb-aefbb55dc5b1@infradead.org>
Date:   Mon, 21 Nov 2022 16:09:43 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [patch 06/15] timers: Update kernel-doc for various functions
To:     Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Linus Torvalds <torvalds@linuxfoundation.org>,
        Anna-Maria Behnsen <anna-maria@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Andrew Morton <akpm@linux-foundation.org>,
        Julia Lawall <Julia.Lawall@inria.fr>,
        Arnd Bergmann <arnd@arndb.de>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Marc Zyngier <maz@kernel.org>,
        Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        linux-bluetooth@vger.kernel.org,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        Jonathan Corbet <corbet@lwn.net>
References: <20221115195802.415956561@linutronix.de>
 <20221115202117.323694948@linutronix.de>
 <20221121154358.36856ca6@gandalf.local.home>
Content-Language: en-US
From:   Randy Dunlap <rdunlap@infradead.org>
In-Reply-To: <20221121154358.36856ca6@gandalf.local.home>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 11/21/22 12:43, Steven Rostedt wrote:
> On Tue, 15 Nov 2022 21:28:43 +0100 (CET)
> Thomas Gleixner <tglx@linutronix.de> 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@linutronix.de>
>> ---
>>  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