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

STALE2225d

28 messages, 3 authors, 2020-07-06 · open the first message on its own page

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

From: Lee Jones <hidden>
Date: 2020-06-24 15:04:44

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 2/8] backlight: lcd: Add missing kerneldoc entry for 'struct device parent'

From: Lee Jones <hidden>
Date: 2020-06-24 15:04:53

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

Cc: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Jamey Hicks <redacted>
Cc: Andrew Zabolotny <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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 7/8] backlight: lm3630a_bl: Remove invalid checks for unsigned int < 0

From: Lee Jones <hidden>
Date: 2020-06-24 15:04:58

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Daniel Jeong <redacted>
Cc: LDD MLP <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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

From: Lee Jones <hidden>
Date: 2020-06-24 15:05:03

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: <redacted>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-arm-msm@vger.kernel.org
Signed-off-by: Lee Jones <redacted>
---
 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

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

From: Lee Jones <hidden>
Date: 2020-06-24 15:05:07

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Jamey Hicks <redacted>
Cc: Andrew Zabolotny <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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 5/8] backlight: ili922x: Add missing kerneldoc description for ili922x_reg_dump()'s arg

From: Lee Jones <hidden>
Date: 2020-06-24 15:05:10

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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

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

From: Lee Jones <hidden>
Date: 2020-06-24 15:05:15

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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 4/8] backlight: ili922x: Remove invalid use of kerneldoc syntax

From: Lee Jones <hidden>
Date: 2020-06-24 15:05:19

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

From: Lee Jones <hidden>
Date: 2020-06-24 15:05:24

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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 0/8] Fix a bunch of W=1 warnings in Backlight

From: Sam Ravnborg <hidden>
Date: 2020-06-24 15:32:23

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

	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

From: Lee Jones <hidden>
Date: 2020-06-24 15:43:27

On Wed, 24 Jun 2020, Sam Ravnborg wrote:
Hi Lee.

On Wed, Jun 24, 2020 at 03:57:13PM +0100, Lee Jones wrote:
quoted
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'
quoted
  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'?
quoted
  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 <redacted>
Thanks (although this should be Reviewed-by).
quoted
 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

From: Sam Ravnborg <hidden>
Date: 2020-06-24 16:24:53

Hi Lee.

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

On Wed, Jun 24, 2020 at 03:57:13PM +0100, Lee Jones wrote:
quoted
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'
quoted
  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
quoted
quoted
  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 <redacted>
Thanks (although this should be Reviewed-by).
quoted
quoted
 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

From: Lee Jones <hidden>
Date: 2020-06-25 08:03:53

On Wed, 24 Jun 2020, Sam Ravnborg wrote:
Hi Lee.

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

On Wed, Jun 24, 2020 at 03:57:13PM +0100, Lee Jones wrote:
quoted
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'
quoted
  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).
quoted
quoted
quoted
  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 <redacted>
Thanks (although this should be Reviewed-by).
quoted
quoted
 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 1/8] backlight: lms501kf03: Remove unused const variables

From: Daniel Thompson <hidden>
Date: 2020-06-25 09:24:41

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Lee Jones <redacted>
Reviewed-by: Daniel Thompson <redacted>

quoted hunk
---
 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 2/8] backlight: lcd: Add missing kerneldoc entry for 'struct device parent'

From: Daniel Thompson <hidden>
Date: 2020-06-25 09:28:35

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Jamey Hicks <redacted>
Cc: Andrew Zabolotny <redacted>
Signed-off-by: Lee Jones <redacted>
Reviewed-by: Daniel Thompson <redacted>

quoted hunk
---
 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 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

From: Daniel Thompson <hidden>
Date: 2020-06-25 09:40:59

On Wed, Jun 24, 2020 at 03:57:16PM +0100, Lee Jones wrote:
quoted hunk
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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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 4/8] backlight: ili922x: Remove invalid use of kerneldoc syntax

From: Daniel Thompson <hidden>
Date: 2020-06-25 09:41:33

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
Reviewed-by: Daniel Thompson <redacted>

quoted hunk
---
 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 5/8] backlight: ili922x: Add missing kerneldoc description for ili922x_reg_dump()'s arg

From: Daniel Thompson <hidden>
Date: 2020-06-25 09:43:25

On Wed, Jun 24, 2020 at 03:57:18PM +0100, Lee Jones wrote:
quoted hunk
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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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 6/8] backlight: backlight: Supply description for function args in existing Kerneldocs

From: Daniel Thompson <hidden>
Date: 2020-06-25 09:43:51

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Jamey Hicks <redacted>
Cc: Andrew Zabolotny <redacted>
Signed-off-by: Lee Jones <redacted>
Reviewed-by: Daniel Thompson <redacted>

quoted hunk
---
 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 7/8] backlight: lm3630a_bl: Remove invalid checks for unsigned int < 0

From: Daniel Thompson <hidden>
Date: 2020-06-25 09:45:16

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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Daniel Jeong <redacted>
Cc: LDD MLP <redacted>
Signed-off-by: Lee Jones <redacted>
Reviewed-by: Daniel Thompson <redacted>

quoted hunk
---
 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 8/8] backlight: qcom-wled: Remove unused configs for LED3 and LED4

From: Daniel Thompson <hidden>
Date: 2020-06-25 09:46:19

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: <redacted>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: linux-arm-msm@vger.kernel.org
Signed-off-by: Lee Jones <redacted>
Reviewed-by: Daniel Thompson <redacted>

quoted hunk
---
 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 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

From: Lee Jones <hidden>
Date: 2020-06-25 10:33:41

On Thu, 25 Jun 2020, Daniel Thompson wrote:
On Wed, Jun 24, 2020 at 03:57:16PM +0100, Lee Jones wrote:
quoted
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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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 0/8] Fix a bunch of W=1 warnings in Backlight

From: Sam Ravnborg <hidden>
Date: 2020-06-25 18:57:54

Hi Lee.

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

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

On Wed, Jun 24, 2020 at 03:57:13PM +0100, Lee Jones wrote:
quoted
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'
quoted
  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
quoted
quoted
quoted
quoted
  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 <redacted>
Thanks (although this should be Reviewed-by).
quoted
quoted
 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 3/8] backlight: ili922x: Add missing kerneldoc descriptions for CHECK_FREQ_REG() args

From: Daniel Thompson <hidden>
Date: 2020-06-26 09:54:12

On Thu, Jun 25, 2020 at 11:33:34AM +0100, Lee Jones wrote:
On Thu, 25 Jun 2020, Daniel Thompson wrote:
quoted
On Wed, Jun 24, 2020 at 03:57:16PM +0100, Lee Jones wrote:
quoted
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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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

From: Lee Jones <hidden>
Date: 2020-06-26 15:33:56

On Fri, 26 Jun 2020, Daniel Thompson wrote:
On Thu, Jun 25, 2020 at 11:33:34AM +0100, Lee Jones wrote:
quoted
On Thu, 25 Jun 2020, Daniel Thompson wrote:
quoted
On Wed, Jun 24, 2020 at 03:57:16PM +0100, Lee Jones wrote:
quoted
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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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

From: Lee Jones <hidden>
Date: 2020-07-06 07:12:59

On Thu, 25 Jun 2020, Daniel Thompson wrote:
On Wed, Jun 24, 2020 at 03:57:16PM +0100, Lee Jones wrote:
quoted
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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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

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

From: Lee Jones <hidden>
Date: 2020-07-06 07:13:29

On Thu, 25 Jun 2020, Daniel Thompson wrote:
On Wed, Jun 24, 2020 at 03:57:18PM +0100, Lee Jones wrote:
quoted
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: <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Software Engineering <redacted>
Signed-off-by: Lee Jones <redacted>
---
 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

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

From: Lee Jones <hidden>
Date: 2020-07-06 07:13:46

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help