Re: [PATCH] backlight: ili922x: fix W=1 kernel-doc warnings

[Randy Dunlap <rdunlap@infradead.org>]

On 12/6/23 05:25, Lee Jones wrote:
> On Wed, 06 Dec 2023, Daniel Thompson wrote:
> 
>> On Tue, Dec 05, 2023 at 02:56:38PM -0800, Randy Dunlap wrote:
>>> Fix kernel-doc warnings found when using "W=1".
>>>
>>> ili922x.c:85: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
>>> ili922x.c:85: warning: missing initial short description on line:
>>>  * START_BYTE(id, rs, rw)
>>> ili922x.c:91: warning: contents before sections
>>> ili922x.c:118: warning: expecting prototype for CHECK_FREQ_REG(spi_device s, spi_transfer x)(). Prototype was for CHECK_FREQ_REG() instead
>>>
>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>> Cc: Lee Jones <lee@kernel.org>
>>> Cc: Daniel Thompson <daniel.thompson@linaro.org>
>>> Cc: Jingoo Han <jingoohan1@gmail.com>
>>> Cc: Helge Deller <deller@gmx.de>
>>> Cc: linux-fbdev@vger.kernel.org
>>> ---
>>>  drivers/video/backlight/ili922x.c |    9 ++++-----
>>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff -- a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c
>>> --- a/drivers/video/backlight/ili922x.c
>>> +++ b/drivers/video/backlight/ili922x.c
>>> @@ -82,13 +82,12 @@
>>>  #define START_RW_READ		1
>>>
>>>  /**
>>> - * START_BYTE(id, rs, rw)
>>> - *
>>> - * Set the start byte according to the required operation.
>>> + * START_BYTE() - Set the start byte according to the required operation.
>>>   * The start byte is defined as:
>>>   *   ----------------------------------
>>>   *  | 0 | 1 | 1 | 1 | 0 | ID | RS | RW |
>>>   *   ----------------------------------
>>
>> I'm not sure we want "The start byte is defined as" in the brief
>> description. Needs a blank line between the brief and full description
>> (or hoist the argument descriptions up to match the idiomatic
>> form for a kernel-doc comment in the docs if you prefer).
> 
> I'd consider dropping the kernel-docness of this header entirely.
> Kerneldoc is designed for documenting exported (or at least externally
> available) functions and data structures, with allowances for static
> functions in the name of consistency or in cases of excessive
> complication.  I've fixed A LOT of kernel-doc headers in my time and I
> can't say I remember coming across MACROs being documented this way
> before now.
> 

I've seen several macros that are documented, but I am happy to just drop
the kernel-doc for these local macros.  I'll send a patch for that.

Thanks.
-- 
~Randy