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

[Daniel Thompson <daniel.thompson@linaro.org>]

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

> + *
>   * @id: display's id as set by the manufacturer
>   * @rs: operation type bit, one of:
>   *	  - START_RS_INDEX	set the index register
> @@ -101,14 +100,14 @@
>  	(0x70 | (((id) & 0x01) << 2) | (((rs) & 0x01) << 1) | ((rw) & 0x01))
>
>  /**
> - * CHECK_FREQ_REG(spi_device s, spi_transfer x) - Check the frequency
> + * CHECK_FREQ_REG() - Check the frequency
>   *	for the SPI transfer.

Likewise I think there is no need for "According to the datasheet..." to be
included in the brief description.


Daniel.


>                             According to the datasheet, the controller
>   *	accept higher frequency for the GRAM transfer, but it requires
>   *	lower frequency when the registers are read/written.
>   *	The macro sets the frequency in the spi_transfer structure if
>   *	the frequency exceeds the maximum value.
>   * @s: pointer to an SPI device
> - * @x: pointer to the read/write buffer pair
> + * @x: pointer to the &spi_transfer read/write buffer pair
>   */
>  #define CHECK_FREQ_REG(s, x)	\
>  	do {			\