mbox series

[0/8] Fix a bunch of W=1 warnings in Backlight

Message ID 20200624145721.2590327-1-lee.jones@linaro.org
Headers show
Series Fix a bunch of W=1 warnings in Backlight | expand

Message

Lee Jones June 24, 2020, 2:57 p.m. UTC 
Attempting to clean-up W=1 kernel builds, which are currently
overwhelmingly riddled with niggly little warnings.

Lee Jones (8):
  backlight: lms501kf03: Remove unused const variables
  backlight: lcd: Add missing kerneldoc entry for 'struct device parent'
  backlight: ili922x: Add missing kerneldoc descriptions for
    CHECK_FREQ_REG() args
  backlight: ili922x: Remove invalid use of kerneldoc syntax
  backlight: ili922x: Add missing kerneldoc description for
    ili922x_reg_dump()'s arg
  backlight: backlight: Supply description for function args in existing
    Kerneldocs
  backlight: lm3630a_bl: Remove invalid checks for unsigned int < 0
  backlight: qcom-wled: Remove unused configs for LED3 and LED4

 drivers/video/backlight/backlight.c  | 2 ++
 drivers/video/backlight/ili922x.c    | 8 ++++++--
 drivers/video/backlight/lcd.c        | 1 +
 drivers/video/backlight/lm3630a_bl.c | 4 ++--
 drivers/video/backlight/lms501kf03.c | 8 --------
 drivers/video/backlight/qcom-wled.c  | 8 --------
 6 files changed, 11 insertions(+), 20 deletions(-)

-- 
2.25.1

Comments

Sam Ravnborg June 24, 2020, 3:32 p.m. UTC | #1 
Hi Lee.

On Wed, Jun 24, 2020 at 03:57:13PM +0100, Lee Jones wrote:
> Attempting to clean-up W=1 kernel builds, which are currently

> overwhelmingly riddled with niggly little warnings.

> 

> Lee Jones (8):

>   backlight: lms501kf03: Remove unused const variables

>   backlight: lcd: Add missing kerneldoc entry for 'struct device parent'



>   backlight: ili922x: Add missing kerneldoc descriptions for

>     CHECK_FREQ_REG() args

>   backlight: ili922x: Remove invalid use of kerneldoc syntax

>   backlight: ili922x: Add missing kerneldoc description for

>     ili922x_reg_dump()'s arg

I wonder why these warnings show up as nothing pulls in this .c file.
Anyway I would suggest to drop using kerneldoc syntax for single drivers
like this - and the benefit here is low.
Now they are typed, otherwise this ahd been fine in a single patch.

>   backlight: backlight: Supply description for function args in existing

>     Kerneldocs

>   backlight: lm3630a_bl: Remove invalid checks for unsigned int < 0

>   backlight: qcom-wled: Remove unused configs for LED3 and LED4


The other fixes looks good.
They are all:
Acked-by: Sam Ravnborg <sam@ravnborg.org>


	Sam

>  drivers/video/backlight/backlight.c  | 2 ++

>  drivers/video/backlight/ili922x.c    | 8 ++++++--

>  drivers/video/backlight/lcd.c        | 1 +

>  drivers/video/backlight/lm3630a_bl.c | 4 ++--

>  drivers/video/backlight/lms501kf03.c | 8 --------

>  drivers/video/backlight/qcom-wled.c  | 8 --------

>  6 files changed, 11 insertions(+), 20 deletions(-)

> 

> -- 

> 2.25.1

> 

> _______________________________________________

> dri-devel mailing list

> dri-devel@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sam Ravnborg June 24, 2020, 4:24 p.m. UTC | #2 
Hi Lee.

On Wed, Jun 24, 2020 at 04:43:21PM +0100, Lee Jones wrote:
> On Wed, 24 Jun 2020, Sam Ravnborg wrote:

> 

> > Hi Lee.

> > 

> > On Wed, Jun 24, 2020 at 03:57:13PM +0100, Lee Jones wrote:

> > > Attempting to clean-up W=1 kernel builds, which are currently

> > > overwhelmingly riddled with niggly little warnings.

> > > 

> > > Lee Jones (8):

> > >   backlight: lms501kf03: Remove unused const variables

> > >   backlight: lcd: Add missing kerneldoc entry for 'struct device parent'

> > 

> > 

> > >   backlight: ili922x: Add missing kerneldoc descriptions for

> > >     CHECK_FREQ_REG() args

> > >   backlight: ili922x: Remove invalid use of kerneldoc syntax

> > >   backlight: ili922x: Add missing kerneldoc description for

> > >     ili922x_reg_dump()'s arg

> > I wonder why these warnings show up as nothing pulls in this .c file.

> > Anyway I would suggest to drop using kerneldoc syntax for single drivers

> > like this - and the benefit here is low.

> > Now they are typed, otherwise this ahd been fine in a single patch.

> 

> What do you mean by 'nothing pulls it in'?

There are no .rst files that includes any:
.. kernel-doc:: drivers/video/backlight/ili922x.c

so I do not see how the kernel-doc comments will be used by any
of the generated kernel-docs.

	Sam

> 

> > >   backlight: backlight: Supply description for function args in existing

> > >     Kerneldocs

> > >   backlight: lm3630a_bl: Remove invalid checks for unsigned int < 0

> > >   backlight: qcom-wled: Remove unused configs for LED3 and LED4

> > 

> > The other fixes looks good.

> > They are all:

> > Acked-by: Sam Ravnborg <sam@ravnborg.org>

> 

> Thanks (although this should be Reviewed-by).

> 

> > >  drivers/video/backlight/backlight.c  | 2 ++

> > >  drivers/video/backlight/ili922x.c    | 8 ++++++--

> > >  drivers/video/backlight/lcd.c        | 1 +

> > >  drivers/video/backlight/lm3630a_bl.c | 4 ++--

> > >  drivers/video/backlight/lms501kf03.c | 8 --------

> > >  drivers/video/backlight/qcom-wled.c  | 8 --------

> > >  6 files changed, 11 insertions(+), 20 deletions(-)

> > > 

> 

> -- 

> Lee Jones [李琼斯]

> Senior Technical Lead - Developer Services

> Linaro.org │ Open source software for Arm SoCs

> Follow Linaro: Facebook | Twitter | Blog
Daniel Thompson June 25, 2020, 9:40 a.m. UTC | #3 
On Wed, Jun 24, 2020 at 03:57:16PM +0100, Lee Jones wrote:
> Kerneldoc syntax is used, but not complete.  Descriptions required.

> 

> Prevents warnings like:

> 

>  drivers/video/backlight/ili922x.c:116: warning: Function parameter or member 's' not described in 'CHECK_FREQ_REG'

>  drivers/video/backlight/ili922x.c:116: warning: Function parameter or member 'x' not described in 'CHECK_FREQ_REG'

> 

> Cc: <stable@vger.kernel.org>

> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

> Cc: Software Engineering <sbabic@denx.de>

> Signed-off-by: Lee Jones <lee.jones@linaro.org>

> ---

>  drivers/video/backlight/ili922x.c | 2 ++

>  1 file changed, 2 insertions(+)

> 

> diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c

> index 9c5aa3fbb2842..8cb4b9d3c3bba 100644

> --- a/drivers/video/backlight/ili922x.c

> +++ b/drivers/video/backlight/ili922x.c

> @@ -107,6 +107,8 @@

>   *	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 controller side proxy for an SPI slave device


What's wrong with "a pointer to an SPI device"?

I am aware, having looked it up to find out what the above actually
means, that this is how struct spi_device is described in its own kernel
doc but quoting at that level of detail of both overkill and confusing.


Daniel.


> + * @x: pointer to the read/write buffer pair

>   */

>  #define CHECK_FREQ_REG(s, x)	\

>  	do {			\

> -- 

> 2.25.1

>
Sam Ravnborg June 25, 2020, 6:57 p.m. UTC | #4 
Hi Lee.

On Thu, Jun 25, 2020 at 09:03:37AM +0100, Lee Jones wrote:
> On Wed, 24 Jun 2020, Sam Ravnborg wrote:

> 

> > Hi Lee.

> > 

> > On Wed, Jun 24, 2020 at 04:43:21PM +0100, Lee Jones wrote:

> > > On Wed, 24 Jun 2020, Sam Ravnborg wrote:

> > > 

> > > > Hi Lee.

> > > > 

> > > > On Wed, Jun 24, 2020 at 03:57:13PM +0100, Lee Jones wrote:

> > > > > Attempting to clean-up W=1 kernel builds, which are currently

> > > > > overwhelmingly riddled with niggly little warnings.

> > > > > 

> > > > > Lee Jones (8):

> > > > >   backlight: lms501kf03: Remove unused const variables

> > > > >   backlight: lcd: Add missing kerneldoc entry for 'struct device parent'

> > > > 

> > > > 

> > > > >   backlight: ili922x: Add missing kerneldoc descriptions for

> > > > >     CHECK_FREQ_REG() args

> > > > >   backlight: ili922x: Remove invalid use of kerneldoc syntax

> > > > >   backlight: ili922x: Add missing kerneldoc description for

> > > > >     ili922x_reg_dump()'s arg

> > > > I wonder why these warnings show up as nothing pulls in this .c file.

> > > > Anyway I would suggest to drop using kerneldoc syntax for single drivers

> > > > like this - and the benefit here is low.

> > > > Now they are typed, otherwise this ahd been fine in a single patch.

> > > 

> > > What do you mean by 'nothing pulls it in'?

> > There are no .rst files that includes any:

> > .. kernel-doc:: drivers/video/backlight/ili922x.c

> > 

> > so I do not see how the kernel-doc comments will be used by any

> > of the generated kernel-docs.

> 

> Looks like a common problem (if it is actually a problem):

> 

>  $ ./scripts/find-unused-docs.sh . | wc -l

>  1476

> 

> The role of this patch-set is not to eradicate unused kerneldoc

> headers, but to ensure they are formatted correctly.  W=1 builds

> currently complain of ill formatted kerneldocs, which is currently

> littering the build-log and masking some more important issues (which

> I'm also trying to fix en route).


My point is that I do not see why we should maintain correct kernel-doc
style comments for files that are not used to to generate kernel-doc.
It would serve us better to drop the kernel-doc style comments.
But thats just my opinion, feel free to ignore.

I digged a little and can see we run kernel-doc on all .c files
when we specify W=1 - which was a suprise to me.
That explains why I had not seen said warnings in my regular make
htmldocs runs.

	Sam

> 

> > > > >   backlight: backlight: Supply description for function args in existing

> > > > >     Kerneldocs

> > > > >   backlight: lm3630a_bl: Remove invalid checks for unsigned int < 0

> > > > >   backlight: qcom-wled: Remove unused configs for LED3 and LED4

> > > > 

> > > > The other fixes looks good.

> > > > They are all:

> > > > Acked-by: Sam Ravnborg <sam@ravnborg.org>

> > > 

> > > Thanks (although this should be Reviewed-by).

> > > 

> > > > >  drivers/video/backlight/backlight.c  | 2 ++

> > > > >  drivers/video/backlight/ili922x.c    | 8 ++++++--

> > > > >  drivers/video/backlight/lcd.c        | 1 +

> > > > >  drivers/video/backlight/lm3630a_bl.c | 4 ++--

> > > > >  drivers/video/backlight/lms501kf03.c | 8 --------

> > > > >  drivers/video/backlight/qcom-wled.c  | 8 --------

> > > > >  6 files changed, 11 insertions(+), 20 deletions(-)

> > > > > 

> > > 

> 

> -- 

> Lee Jones [李琼斯]

> Senior Technical Lead - Developer Services

> Linaro.org │ Open source software for Arm SoCs

> Follow Linaro: Facebook | Twitter | Blog
Lee Jones July 6, 2020, 7:12 a.m. UTC | #5 
On Thu, 25 Jun 2020, Daniel Thompson wrote:

> On Wed, Jun 24, 2020 at 03:57:16PM +0100, Lee Jones wrote:

> > Kerneldoc syntax is used, but not complete.  Descriptions required.

> > 

> > Prevents warnings like:

> > 

> >  drivers/video/backlight/ili922x.c:116: warning: Function parameter or member 's' not described in 'CHECK_FREQ_REG'

> >  drivers/video/backlight/ili922x.c:116: warning: Function parameter or member 'x' not described in 'CHECK_FREQ_REG'

> > 

> > Cc: <stable@vger.kernel.org>

> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

> > Cc: Software Engineering <sbabic@denx.de>

> > Signed-off-by: Lee Jones <lee.jones@linaro.org>

> > ---

> >  drivers/video/backlight/ili922x.c | 2 ++

> >  1 file changed, 2 insertions(+)

> > 

> > diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c

> > index 9c5aa3fbb2842..8cb4b9d3c3bba 100644

> > --- a/drivers/video/backlight/ili922x.c

> > +++ b/drivers/video/backlight/ili922x.c

> > @@ -107,6 +107,8 @@

> >   *	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 controller side proxy for an SPI slave device

> 

> What's wrong with "a pointer to an SPI device"?


I've fixed this and applied the patch.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Lee Jones July 6, 2020, 7:13 a.m. UTC | #6 
On Thu, 25 Jun 2020, Daniel Thompson wrote:

> On Wed, Jun 24, 2020 at 03:57:18PM +0100, Lee Jones wrote:

> > Kerneldoc syntax is used, but not complete.  Descriptions required.

> > 

> > Prevents warnings like:

> > 

> >  drivers/video/backlight/ili922x.c:298: warning: Function parameter or member 'spi' not described in 'ili922x_reg_dump'

> > 

> > Cc: <stable@vger.kernel.org>

> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

> > Cc: Software Engineering <sbabic@denx.de>

> > Signed-off-by: Lee Jones <lee.jones@linaro.org>

> > ---

> >  drivers/video/backlight/ili922x.c | 2 ++

> >  1 file changed, 2 insertions(+)

> > 

> > diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c

> > index cd41433b87aeb..26193f38234e7 100644

> > --- a/drivers/video/backlight/ili922x.c

> > +++ b/drivers/video/backlight/ili922x.c

> > @@ -295,6 +295,8 @@ static int ili922x_write(struct spi_device *spi, u8 reg, u16 value)

> >  #ifdef DEBUG

> >  /**

> >   * ili922x_reg_dump - dump all registers

> > + *

> > + * @spi: pointer to the controller side proxy for an SPI slave device

> 

> Similar to previous... and I also noticed that there are several other

> existing @spi descriptions in this file and it would be good to make

> them consistent.


I've fixed this and applied the patch.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Lee Jones July 6, 2020, 7:13 a.m. UTC | #7 
On Wed, 24 Jun 2020, Lee Jones wrote:

> Attempting to clean-up W=1 kernel builds, which are currently

> overwhelmingly riddled with niggly little warnings.

> 

> Lee Jones (8):

>   backlight: lms501kf03: Remove unused const variables

>   backlight: lcd: Add missing kerneldoc entry for 'struct device parent'

>   backlight: ili922x: Add missing kerneldoc descriptions for

>     CHECK_FREQ_REG() args

>   backlight: ili922x: Remove invalid use of kerneldoc syntax

>   backlight: ili922x: Add missing kerneldoc description for

>     ili922x_reg_dump()'s arg

>   backlight: backlight: Supply description for function args in existing

>     Kerneldocs

>   backlight: lm3630a_bl: Remove invalid checks for unsigned int < 0

>   backlight: qcom-wled: Remove unused configs for LED3 and LED4

> 

>  drivers/video/backlight/backlight.c  | 2 ++

>  drivers/video/backlight/ili922x.c    | 8 ++++++--

>  drivers/video/backlight/lcd.c        | 1 +

>  drivers/video/backlight/lm3630a_bl.c | 4 ++--

>  drivers/video/backlight/lms501kf03.c | 8 --------

>  drivers/video/backlight/qcom-wled.c  | 8 --------

>  6 files changed, 11 insertions(+), 20 deletions(-)


All applied to Backlight.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog