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

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

[Lee Jones]

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

[PATCH 1/8] backlight: lms501kf03: Remove unused const variables

[Lee Jones]

W=1 kernel build reports:

 drivers/video/backlight/lms501kf03.c:96:28: warning: ‘seq_sleep_in’ defined but not used [-Wunused-const-variable=]
 96 | static const unsigned char seq_sleep_in[] = {
 | ^~~~~~~~~~~~
 drivers/video/backlight/lms501kf03.c:92:28: warning: ‘seq_up_dn’ defined but not used [-Wunused-const-variable=]
 92 | static const unsigned char seq_up_dn[] = {
 | ^~~~~~~~~

Either 'seq_sleep_in' nor 'seq_up_dn' have been used since the
driver first landed in 2013.

Cc: <stable@vger.kernel.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/video/backlight/lms501kf03.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/video/backlight/lms501kf03.c b/drivers/video/backlight/lms501kf03.c
index 8ae32e3573c1a..c1bd02bb8b2ee 100644
--- a/drivers/video/backlight/lms501kf03.c
+++ b/drivers/video/backlight/lms501kf03.c
@@ -89,14 +89,6 @@ static const unsigned char seq_rgb_gamma[] = {
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 };
 
-static const unsigned char seq_up_dn[] = {
-	0x36, 0x10,
-};
-
-static const unsigned char seq_sleep_in[] = {
-	0x10,
-};
-
 static const unsigned char seq_sleep_out[] = {
 	0x11,
 };
-- 
2.25.1

Re: [PATCH 1/8] backlight: lms501kf03: Remove unused const variables

[Daniel Thompson]

On Wed, Jun 24, 2020 at 03:57:14PM +0100, Lee Jones wrote:
> W=1 kernel build reports:
> 
>  drivers/video/backlight/lms501kf03.c:96:28: warning: ‘seq_sleep_in’ defined but not used [-Wunused-const-variable=]
>  96 | static const unsigned char seq_sleep_in[] = {
>  | ^~~~~~~~~~~~
>  drivers/video/backlight/lms501kf03.c:92:28: warning: ‘seq_up_dn’ defined but not used [-Wunused-const-variable=]
>  92 | static const unsigned char seq_up_dn[] = {
>  | ^~~~~~~~~
> 
> Either 'seq_sleep_in' nor 'seq_up_dn' have been used since the
> driver first landed in 2013.
> 
> Cc: <stable@vger.kernel.org>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
>  drivers/video/backlight/lms501kf03.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/video/backlight/lms501kf03.c b/drivers/video/backlight/lms501kf03.c
> index 8ae32e3573c1a..c1bd02bb8b2ee 100644
> --- a/drivers/video/backlight/lms501kf03.c
> +++ b/drivers/video/backlight/lms501kf03.c
> @@ -89,14 +89,6 @@ static const unsigned char seq_rgb_gamma[] = {
>  	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>  };
>  
> -static const unsigned char seq_up_dn[] = {
> -	0x36, 0x10,
> -};
> -
> -static const unsigned char seq_sleep_in[] = {
> -	0x10,
> -};
> -
>  static const unsigned char seq_sleep_out[] = {
>  	0x11,
>  };
> -- 
> 2.25.1
> 

[PATCH 2/8] backlight: lcd: Add missing kerneldoc entry for 'struct device parent'

[Lee Jones]

This has been missing since the conversion to 'struct device' in 2007.

Cc: <stable@vger.kernel.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Jamey Hicks <jamey.hicks@hp.com>
Cc: Andrew Zabolotny <zap@homelink.ru>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/video/backlight/lcd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index 78b0333586258..db56e465aaff3 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -179,6 +179,7 @@ ATTRIBUTE_GROUPS(lcd_device);
  * lcd_device_register - register a new object of lcd_device class.
  * @name: the name of the new object(must be the same as the name of the
  *   respective framebuffer device).
+ * @parent: pointer to the parent's struct device .
  * @devdata: an optional pointer to be stored in the device. The
  *   methods may retrieve it by using lcd_get_data(ld).
  * @ops: the lcd operations structure.
-- 
2.25.1

Re: [PATCH 2/8] backlight: lcd: Add missing kerneldoc entry for 'struct device parent'

[Daniel Thompson]

On Wed, Jun 24, 2020 at 03:57:15PM +0100, Lee Jones wrote:
> This has been missing since the conversion to 'struct device' in 2007.
> 
> Cc: <stable@vger.kernel.org>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Jamey Hicks <jamey.hicks@hp.com>
> Cc: Andrew Zabolotny <zap@homelink.ru>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
>  drivers/video/backlight/lcd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
> index 78b0333586258..db56e465aaff3 100644
> --- a/drivers/video/backlight/lcd.c
> +++ b/drivers/video/backlight/lcd.c
> @@ -179,6 +179,7 @@ ATTRIBUTE_GROUPS(lcd_device);
>   * lcd_device_register - register a new object of lcd_device class.
>   * @name: the name of the new object(must be the same as the name of the
>   *   respective framebuffer device).
> + * @parent: pointer to the parent's struct device .
>   * @devdata: an optional pointer to be stored in the device. The
>   *   methods may retrieve it by using lcd_get_data(ld).
>   * @ops: the lcd operations structure.
> -- 
> 2.25.1
> 

[PATCH 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

[Lee Jones]

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
+ * @x: pointer to the read/write buffer pair
  */
 #define CHECK_FREQ_REG(s, x)	\
 	do {			\
-- 
2.25.1

Re: [PATCH 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

[Daniel Thompson]

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
> 

Re: [PATCH 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

[Lee Jones]

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

I figured that using the official description would be better than
making something up.  However if you think it's better to KISS, then I
can change it.

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

Re: [PATCH 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

[Daniel Thompson]

On Thu, Jun 25, 2020 at 11:33:34AM +0100, Lee Jones wrote:
> 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 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.
> 
> I figured that using the official description would be better than
> making something up.  However if you think it's better to KISS, then I
> can change it.

Yes, I'd strongly prefer KISS here.

I know it is an "I am the world" argument[1] but I found using such a
dogmatically accurate description out of context to be very confusing
and therefore I don't think such a comment improves readability.


Daniel.


[1]: See #3 from http://www.leany.com/logic/Adams.html

Re: [PATCH 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

[Lee Jones]

On Fri, 26 Jun 2020, Daniel Thompson wrote:

> On Thu, Jun 25, 2020 at 11:33:34AM +0100, Lee Jones wrote:
> > 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 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.
> > 
> > I figured that using the official description would be better than
> > making something up.  However if you think it's better to KISS, then I
> > can change it.
> 
> Yes, I'd strongly prefer KISS here.
> 
> I know it is an "I am the world" argument[1] but I found using such a
> dogmatically accurate description out of context to be very confusing
> and therefore I don't think such a comment improves readability.
> 
> [1]: See #3 from http://www.leany.com/logic/Adams.html

It's fine, you are the world, I get it. ;)

Do you even like Country music?

Will fix!

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

Re: [PATCH 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

[Lee Jones]

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

[PATCH 4/8] backlight: ili922x: Remove invalid use of kerneldoc syntax

[Lee Jones]

Kerneldoc is for documenting function arguments and return values.

Prevents warnings like:

 drivers/video/backlight/ili922x.c:127: warning: cannot understand function prototype: 'int ili922x_id = 1; '
 drivers/video/backlight/ili922x.c:136: warning: cannot understand function prototype: 'struct ili922x '

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c
index 8cb4b9d3c3bba..cd41433b87aeb 100644
--- a/drivers/video/backlight/ili922x.c
+++ b/drivers/video/backlight/ili922x.c
@@ -123,7 +123,7 @@
 
 #define set_tx_byte(b)		(tx_invert ? ~(b) : b)
 
-/**
+/*
  * ili922x_id - id as set by manufacturer
  */
 static int ili922x_id = 1;
@@ -132,7 +132,7 @@ module_param(ili922x_id, int, 0);
 static int tx_invert;
 module_param(tx_invert, int, 0);
 
-/**
+/*
  * driver's private structure
  */
 struct ili922x {
-- 
2.25.1

Re: [PATCH 4/8] backlight: ili922x: Remove invalid use of kerneldoc syntax

[Daniel Thompson]

On Wed, Jun 24, 2020 at 03:57:17PM +0100, Lee Jones wrote:
> Kerneldoc is for documenting function arguments and return values.
> 
> Prevents warnings like:
> 
>  drivers/video/backlight/ili922x.c:127: warning: cannot understand function prototype: 'int ili922x_id = 1; '
>  drivers/video/backlight/ili922x.c:136: warning: cannot understand function prototype: 'struct ili922x '
> 
> 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>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
>  drivers/video/backlight/ili922x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c
> index 8cb4b9d3c3bba..cd41433b87aeb 100644
> --- a/drivers/video/backlight/ili922x.c
> +++ b/drivers/video/backlight/ili922x.c
> @@ -123,7 +123,7 @@
>  
>  #define set_tx_byte(b)		(tx_invert ? ~(b) : b)
>  
> -/**
> +/*
>   * ili922x_id - id as set by manufacturer
>   */
>  static int ili922x_id = 1;
> @@ -132,7 +132,7 @@ module_param(ili922x_id, int, 0);
>  static int tx_invert;
>  module_param(tx_invert, int, 0);
>  
> -/**
> +/*
>   * driver's private structure
>   */
>  struct ili922x {
> -- 
> 2.25.1
> 

[PATCH 5/8] backlight: ili922x: Add missing kerneldoc description for ili922x_reg_dump()'s arg

[Lee Jones]

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
  */
 static void ili922x_reg_dump(struct spi_device *spi)
 {
-- 
2.25.1

Re: [PATCH 5/8] backlight: ili922x: Add missing kerneldoc description for ili922x_reg_dump()'s arg

[Daniel Thompson]

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.


Daniel.

>   */
>  static void ili922x_reg_dump(struct spi_device *spi)
>  {
> -- 
> 2.25.1
> 

Re: [PATCH 5/8] backlight: ili922x: Add missing kerneldoc description for ili922x_reg_dump()'s arg

[Lee Jones]

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

[PATCH 6/8] backlight: backlight: Supply description for function args in existing Kerneldocs

[Lee Jones]

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

Prevents warnings like:

 drivers/video/backlight/backlight.c:329: warning: Function parameter or member 'reason' not described in 'backlight_force_update'
 drivers/video/backlight/backlight.c:354: warning: Function parameter or member 'props' not described in 'backlight_device_register'

Cc: <stable@vger.kernel.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Jamey Hicks <jamey.hicks@hp.com>
Cc: Andrew Zabolotny <zap@homelink.ru>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/video/backlight/backlight.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 92d80aa0c0ef1..744ba58488e01 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -320,6 +320,7 @@ ATTRIBUTE_GROUPS(bl_device);
  * backlight_force_update - tell the backlight subsystem that hardware state
  *   has changed
  * @bd: the backlight device to update
+ * @reason: reason for update
  *
  * Updates the internal state of the backlight in response to a hardware event,
  * and generate a uevent to notify userspace
@@ -344,6 +345,7 @@ EXPORT_SYMBOL(backlight_force_update);
  * @devdata: an optional pointer to be stored for private driver use. The
  *   methods may retrieve it by using bl_get_data(bd).
  * @ops: the backlight operations structure.
+ * @props: pointer to backlight's properties structure.
  *
  * Creates and registers new backlight device. Returns either an
  * ERR_PTR() or a pointer to the newly allocated device.
-- 
2.25.1

Re: [PATCH 6/8] backlight: backlight: Supply description for function args in existing Kerneldocs

[Daniel Thompson]

On Wed, Jun 24, 2020 at 03:57:19PM +0100, Lee Jones wrote:
> Kerneldoc syntax is used, but not complete.  Descriptions required.
> 
> Prevents warnings like:
> 
>  drivers/video/backlight/backlight.c:329: warning: Function parameter or member 'reason' not described in 'backlight_force_update'
>  drivers/video/backlight/backlight.c:354: warning: Function parameter or member 'props' not described in 'backlight_device_register'
> 
> Cc: <stable@vger.kernel.org>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Jamey Hicks <jamey.hicks@hp.com>
> Cc: Andrew Zabolotny <zap@homelink.ru>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
>  drivers/video/backlight/backlight.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 92d80aa0c0ef1..744ba58488e01 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -320,6 +320,7 @@ ATTRIBUTE_GROUPS(bl_device);
>   * backlight_force_update - tell the backlight subsystem that hardware state
>   *   has changed
>   * @bd: the backlight device to update
> + * @reason: reason for update
>   *
>   * Updates the internal state of the backlight in response to a hardware event,
>   * and generate a uevent to notify userspace
> @@ -344,6 +345,7 @@ EXPORT_SYMBOL(backlight_force_update);
>   * @devdata: an optional pointer to be stored for private driver use. The
>   *   methods may retrieve it by using bl_get_data(bd).
>   * @ops: the backlight operations structure.
> + * @props: pointer to backlight's properties structure.
>   *
>   * Creates and registers new backlight device. Returns either an
>   * ERR_PTR() or a pointer to the newly allocated device.
> -- 
> 2.25.1
> 

[PATCH 7/8] backlight: lm3630a_bl: Remove invalid checks for unsigned int < 0

[Lee Jones]

unsigned ints 'sources' and 'bank' cannot be less than LM3630A_SINK_0 (0)
and LM3630A_BANK_0 (0) respecitively, so change the logic to only check
for thier two possible valid values.

Fixes W=1 warnings:

 drivers/video/backlight/lm3630a_bl.c: In function ‘lm3630a_parse_led_sources’:
 drivers/video/backlight/lm3630a_bl.c:394:18: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
 394 | if (sources[i] < LM3630A_SINK_0 || sources[i] > LM3630A_SINK_1)
 | ^
 drivers/video/backlight/lm3630a_bl.c: In function ‘lm3630a_parse_bank’:
 drivers/video/backlight/lm3630a_bl.c:415:11: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
 415 | if (bank < LM3630A_BANK_0 || bank > LM3630A_BANK_1)
 | ^

Cc: <stable@vger.kernel.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Daniel Jeong <gshark.jeong@gmail.com>
Cc: LDD MLP <ldd-mlp@list.ti.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/video/backlight/lm3630a_bl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
index ee320883b7108..e88a2b0e59046 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -391,7 +391,7 @@ static int lm3630a_parse_led_sources(struct fwnode_handle *node,
 		return ret;
 
 	for (i = 0; i < num_sources; i++) {
-		if (sources[i] < LM3630A_SINK_0 || sources[i] > LM3630A_SINK_1)
+		if (sources[i] != LM3630A_SINK_0 && sources[i] != LM3630A_SINK_1)
 			return -EINVAL;
 
 		ret |= BIT(sources[i]);
@@ -412,7 +412,7 @@ static int lm3630a_parse_bank(struct lm3630a_platform_data *pdata,
 	if (ret)
 		return ret;
 
-	if (bank < LM3630A_BANK_0 || bank > LM3630A_BANK_1)
+	if (bank != LM3630A_BANK_0 && bank != LM3630A_BANK_1)
 		return -EINVAL;
 
 	led_sources = lm3630a_parse_led_sources(node, BIT(bank));
-- 
2.25.1

Re: [PATCH 7/8] backlight: lm3630a_bl: Remove invalid checks for unsigned int < 0

[Daniel Thompson]

On Wed, Jun 24, 2020 at 03:57:20PM +0100, Lee Jones wrote:
> unsigned ints 'sources' and 'bank' cannot be less than LM3630A_SINK_0 (0)
> and LM3630A_BANK_0 (0) respecitively, so change the logic to only check
> for thier two possible valid values.
> 
> Fixes W=1 warnings:
> 
>  drivers/video/backlight/lm3630a_bl.c: In function ‘lm3630a_parse_led_sources’:
>  drivers/video/backlight/lm3630a_bl.c:394:18: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>  394 | if (sources[i] < LM3630A_SINK_0 || sources[i] > LM3630A_SINK_1)
>  | ^
>  drivers/video/backlight/lm3630a_bl.c: In function ‘lm3630a_parse_bank’:
>  drivers/video/backlight/lm3630a_bl.c:415:11: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>  415 | if (bank < LM3630A_BANK_0 || bank > LM3630A_BANK_1)
>  | ^
> 
> Cc: <stable@vger.kernel.org>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Daniel Jeong <gshark.jeong@gmail.com>
> Cc: LDD MLP <ldd-mlp@list.ti.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
>  drivers/video/backlight/lm3630a_bl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> index ee320883b7108..e88a2b0e59046 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -391,7 +391,7 @@ static int lm3630a_parse_led_sources(struct fwnode_handle *node,
>  		return ret;
>  
>  	for (i = 0; i < num_sources; i++) {
> -		if (sources[i] < LM3630A_SINK_0 || sources[i] > LM3630A_SINK_1)
> +		if (sources[i] != LM3630A_SINK_0 && sources[i] != LM3630A_SINK_1)
>  			return -EINVAL;
>  
>  		ret |= BIT(sources[i]);
> @@ -412,7 +412,7 @@ static int lm3630a_parse_bank(struct lm3630a_platform_data *pdata,
>  	if (ret)
>  		return ret;
>  
> -	if (bank < LM3630A_BANK_0 || bank > LM3630A_BANK_1)
> +	if (bank != LM3630A_BANK_0 && bank != LM3630A_BANK_1)
>  		return -EINVAL;
>  
>  	led_sources = lm3630a_parse_led_sources(node, BIT(bank));
> -- 
> 2.25.1
> 

[PATCH 8/8] backlight: qcom-wled: Remove unused configs for LED3 and LED4

[Lee Jones]

Fixes W=1 warnings:

 drivers/video/backlight/qcom-wled.c:1294:34: warning: ‘wled4_string_cfg’ defined but not used [-Wunused-const-variable=]
 1294 | static const struct wled_var_cfg wled4_string_cfg = {
 | ^~~~~~~~~~~~~~~~
 drivers/video/backlight/qcom-wled.c:1290:34: warning: ‘wled3_string_cfg’ defined but not used [-Wunused-const-variable=]
 1290 | static const struct wled_var_cfg wled3_string_cfg = {
 | ^~~~~~~~~~~~~~~~

Cc: <stable@vger.kernel.org>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: linux-arm-msm@vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/video/backlight/qcom-wled.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index 4c8c34b994414..c25c31199952c 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -1287,14 +1287,6 @@ static const struct wled_var_cfg wled4_string_i_limit_cfg = {
 	.size = ARRAY_SIZE(wled4_string_i_limit_values),
 };
 
-static const struct wled_var_cfg wled3_string_cfg = {
-	.size = 8,
-};
-
-static const struct wled_var_cfg wled4_string_cfg = {
-	.size = 16,
-};
-
 static const struct wled_var_cfg wled5_mod_sel_cfg = {
 	.size = 2,
 };
-- 
2.25.1

Re: [PATCH 8/8] backlight: qcom-wled: Remove unused configs for LED3 and LED4

[Daniel Thompson]

On Wed, Jun 24, 2020 at 03:57:21PM +0100, Lee Jones wrote:
> Fixes W=1 warnings:
> 
>  drivers/video/backlight/qcom-wled.c:1294:34: warning: ‘wled4_string_cfg’ defined but not used [-Wunused-const-variable=]
>  1294 | static const struct wled_var_cfg wled4_string_cfg = {
>  | ^~~~~~~~~~~~~~~~
>  drivers/video/backlight/qcom-wled.c:1290:34: warning: ‘wled3_string_cfg’ defined but not used [-Wunused-const-variable=]
>  1290 | static const struct wled_var_cfg wled3_string_cfg = {
>  | ^~~~~~~~~~~~~~~~
> 
> Cc: <stable@vger.kernel.org>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: linux-arm-msm@vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
>  drivers/video/backlight/qcom-wled.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
> index 4c8c34b994414..c25c31199952c 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
> @@ -1287,14 +1287,6 @@ static const struct wled_var_cfg wled4_string_i_limit_cfg = {
>  	.size = ARRAY_SIZE(wled4_string_i_limit_values),
>  };
>  
> -static const struct wled_var_cfg wled3_string_cfg = {
> -	.size = 8,
> -};
> -
> -static const struct wled_var_cfg wled4_string_cfg = {
> -	.size = 16,
> -};
> -
>  static const struct wled_var_cfg wled5_mod_sel_cfg = {
>  	.size = 2,
>  };
> -- 
> 2.25.1
> 

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

[Sam Ravnborg]

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

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

[Lee Jones]

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'?

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

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

[Sam Ravnborg]

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

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

[Lee Jones]

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

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

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

[Sam Ravnborg]

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

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

[Lee Jones]

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