1. Patchew
  2. linux
  3. View series

[PATCH] clk: renesas: r9a06g032: fix kerneldoc warning

Ralph Siemsen posted 1 patch 2 years, 5 months ago 
drivers/clk/renesas/r9a06g032-clocks.c | 64 ++++++++++++++------------
1 file changed, 34 insertions(+), 30 deletions(-)
[PATCH] clk: renesas: r9a06g032: fix kerneldoc warning
Posted by Ralph Siemsen 2 years, 5 months ago 
This fixes the following W=1 warning during build:
> drivers/clk/renesas/r9a06g032-clocks.c:119: warning: Function parameter or member 'dual' not described in 'r9a06g032_clkdesc'

Added documentation for member 'dual'. Also added names for the other
structures in the same union, with documentation. Adjusted names of
members within the 'div' structure to avoid duplication.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202309101314.kTRoxND5-lkp@intel.com/
Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
---
Second post, as the Subject: line accidentally got messed up previously.

 drivers/clk/renesas/r9a06g032-clocks.c | 64 ++++++++++++++------------
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
index 55db63c7041a..61296c81f9b5 100644
--- a/drivers/clk/renesas/r9a06g032-clocks.c
+++ b/drivers/clk/renesas/r9a06g032-clocks.c
@@ -102,19 +102,22 @@ enum gate_type {
  * @source:    the ID+1 of the parent clock element.
  *             Root clock uses ID of ~0 (PARENT_ID);
  * @gate:      clock enable/disable
- * @div_min:   smallest permitted clock divider
- * @div_max:   largest permitted clock divider
- * @reg:       clock divider register offset, in 32-bit words
- * @div_table: optional list of fixed clock divider values;
+ * @div:       substructure for clock divider
+ * @div.min:   smallest permitted clock divider
+ * @div.max:   largest permitted clock divider
+ * @div.reg:   clock divider register offset, in 32-bit words
+ * @div.table: optional list of fixed clock divider values;
  *             must be in ascending order, zero for unused
- * @div:       divisor for fixed-factor clock
- * @mul:       multiplier for fixed-factor clock
- * @group:     UART group, 0=UART0/1/2, 1=UART3/4/5/6/7
- * @sel:       select either g1/r1 or g2/r2 as clock source
- * @g1:        1st source gate (clock enable/disable)
- * @r1:        1st source reset (module reset)
- * @g2:        2nd source gate (clock enable/disable)
- * @r2:        2nd source reset (module reset)
+ * @ffc:       substructure for fixed-factor clocks
+ * @ffc.div:   divisor for fixed-factor clock
+ * @ffc.mul:   multiplier for fixed-factor clock
+ * @dual:      substructure for dual clock gates
+ * @dual.group: UART group, 0=UART0/1/2, 1=UART3/4/5/6/7
+ * @dual.sel:  select either g1/r1 or g2/r2 as clock source
+ * @dual.g1:   1st source gate (clock enable/disable)
+ * @dual.r1:   1st source reset (module reset)
+ * @dual.g2:   2nd source gate (clock enable/disable)
+ * @dual.r2:   2nd source reset (module reset)
  *
  * Describes a single element in the clock tree hierarchy.
  * As there are quite a large number of clock elements, this
@@ -131,13 +134,13 @@ struct r9a06g032_clkdesc {
 		struct r9a06g032_gate gate;
 		/* type = K_DIV  */
 		struct {
-			unsigned int div_min:10, div_max:10, reg:10;
-			u16 div_table[4];
-		};
+			unsigned int min:10, max:10, reg:10;
+			u16 table[4];
+		} div;
 		/* type = K_FFC */
 		struct {
 			u16 div, mul;
-		};
+		} ffc;
 		/* type = K_DUALGATE */
 		struct {
 			uint16_t group:1;
@@ -178,26 +181,26 @@ struct r9a06g032_clkdesc {
 	.type = K_FFC, \
 	.index = R9A06G032_##_idx, \
 	.name = _n, \
-	.div = _div, \
-	.mul = _mul \
+	.ffc.div = _div, \
+	.ffc.mul = _mul \
 }
 #define D_FFC(_idx, _n, _src, _div) { \
 	.type = K_FFC, \
 	.index = R9A06G032_##_idx, \
 	.source = 1 + R9A06G032_##_src, \
 	.name = _n, \
-	.div = _div, \
-	.mul = 1 \
+	.ffc.div = _div, \
+	.ffc.mul = 1 \
 }
 #define D_DIV(_idx, _n, _src, _reg, _min, _max, ...) { \
 	.type = K_DIV, \
 	.index = R9A06G032_##_idx, \
 	.source = 1 + R9A06G032_##_src, \
 	.name = _n, \
-	.reg = _reg, \
-	.div_min = _min, \
-	.div_max = _max, \
-	.div_table = { __VA_ARGS__ } \
+	.div.reg = _reg, \
+	.div.min = _min, \
+	.div.max = _max, \
+	.div.table = { __VA_ARGS__ } \
 }
 #define D_UGATE(_idx, _n, _src, _g, _g1, _r1, _g2, _r2) { \
 	.type = K_DUALGATE, \
@@ -1063,14 +1066,14 @@ r9a06g032_register_div(struct r9a06g032_priv *clocks,
 
 	div->clocks = clocks;
 	div->index = desc->index;
-	div->reg = desc->reg;
+	div->reg = desc->div.reg;
 	div->hw.init = &init;
-	div->min = desc->div_min;
-	div->max = desc->div_max;
+	div->min = desc->div.min;
+	div->max = desc->div.max;
 	/* populate (optional) divider table fixed values */
 	for (i = 0; i < ARRAY_SIZE(div->table) &&
-	     i < ARRAY_SIZE(desc->div_table) && desc->div_table[i]; i++) {
-		div->table[div->table_size++] = desc->div_table[i];
+	     i < ARRAY_SIZE(desc->div.table) && desc->div.table[i]; i++) {
+		div->table[div->table_size++] = desc->div.table[i];
 	}
 
 	clk = clk_register(NULL, &div->hw);
@@ -1333,7 +1336,8 @@ static int __init r9a06g032_clocks_probe(struct platform_device *pdev)
 		case K_FFC:
 			clk = clk_register_fixed_factor(NULL, d->name,
 							parent_name, 0,
-							d->mul, d->div);
+							d->ffc.mul,
+							d->ffc.div);
 			break;
 		case K_GATE:
 			clk = r9a06g032_register_gate(clocks, parent_name, d);
-- 
2.25.1
Re: [PATCH] clk: renesas: r9a06g032: fix kerneldoc warning
Posted by Miquel Raynal 2 years, 5 months ago 
Hi Ralph,

ralph.siemsen@linaro.org wrote on Mon, 11 Sep 2023 13:52:15 -0400:

> This fixes the following W=1 warning during build:
> > drivers/clk/renesas/r9a06g032-clocks.c:119: warning: Function parameter or member 'dual' not described in 'r9a06g032_clkdesc'  
> 
> Added documentation for member 'dual'. Also added names for the other
> structures in the same union, with documentation. Adjusted names of
> members within the 'div' structure to avoid duplication.

It would be better to use the imperative form: s/added/add/,
s/adjusted/adjust/, etc.

I would also split this patch because you are doing two different
actions here: adding "dual" to the kdoc *and* naming other anonymous
structures.

I would use something like this for the first patch:
"
Mention the 'dual' structure in the kdoc. This fixes the following
W=1...
"

And rephrase the commit log for the second patch, something along:
"
Clarify the content of the r9a06g032_clkdesc structure by naming the
remaining anonymous structures defined inside. Renaming each field and
updating the doc then becomes necessary in order to avoid name
duplications and kdoc warnings.
"

> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202309101314.kTRoxND5-lkp@intel.com/
> Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
> ---
> Second post, as the Subject: line accidentally got messed up previously.
> 
>  drivers/clk/renesas/r9a06g032-clocks.c | 64 ++++++++++++++------------
>  1 file changed, 34 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
> index 55db63c7041a..61296c81f9b5 100644
> --- a/drivers/clk/renesas/r9a06g032-clocks.c
> +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> @@ -102,19 +102,22 @@ enum gate_type {
>   * @source:    the ID+1 of the parent clock element.
>   *             Root clock uses ID of ~0 (PARENT_ID);
>   * @gate:      clock enable/disable
> - * @div_min:   smallest permitted clock divider
> - * @div_max:   largest permitted clock divider
> - * @reg:       clock divider register offset, in 32-bit words
> - * @div_table: optional list of fixed clock divider values;
> + * @div:       substructure for clock divider
> + * @div.min:   smallest permitted clock divider
> + * @div.max:   largest permitted clock divider
> + * @div.reg:   clock divider register offset, in 32-bit words
> + * @div.table: optional list of fixed clock divider values;
>   *             must be in ascending order, zero for unused
> - * @div:       divisor for fixed-factor clock
> - * @mul:       multiplier for fixed-factor clock
> - * @group:     UART group, 0=UART0/1/2, 1=UART3/4/5/6/7
> - * @sel:       select either g1/r1 or g2/r2 as clock source
> - * @g1:        1st source gate (clock enable/disable)
> - * @r1:        1st source reset (module reset)
> - * @g2:        2nd source gate (clock enable/disable)
> - * @r2:        2nd source reset (module reset)
> + * @ffc:       substructure for fixed-factor clocks
> + * @ffc.div:   divisor for fixed-factor clock
> + * @ffc.mul:   multiplier for fixed-factor clock
> + * @dual:      substructure for dual clock gates
> + * @dual.group: UART group, 0=UART0/1/2, 1=UART3/4/5/6/7
> + * @dual.sel:  select either g1/r1 or g2/r2 as clock source
> + * @dual.g1:   1st source gate (clock enable/disable)
> + * @dual.r1:   1st source reset (module reset)
> + * @dual.g2:   2nd source gate (clock enable/disable)
> + * @dual.r2:   2nd source reset (module reset)
>   *
>   * Describes a single element in the clock tree hierarchy.
>   * As there are quite a large number of clock elements, this
> @@ -131,13 +134,13 @@ struct r9a06g032_clkdesc {
>  		struct r9a06g032_gate gate;
>  		/* type = K_DIV  */
>  		struct {
> -			unsigned int div_min:10, div_max:10, reg:10;
> -			u16 div_table[4];
> -		};
> +			unsigned int min:10, max:10, reg:10;
> +			u16 table[4];
> +		} div;
>  		/* type = K_FFC */
>  		struct {
>  			u16 div, mul;
> -		};
> +		} ffc;
>  		/* type = K_DUALGATE */
>  		struct {
>  			uint16_t group:1;
> @@ -178,26 +181,26 @@ struct r9a06g032_clkdesc {
>  	.type = K_FFC, \
>  	.index = R9A06G032_##_idx, \
>  	.name = _n, \
> -	.div = _div, \
> -	.mul = _mul \
> +	.ffc.div = _div, \
> +	.ffc.mul = _mul \
>  }
>  #define D_FFC(_idx, _n, _src, _div) { \
>  	.type = K_FFC, \
>  	.index = R9A06G032_##_idx, \
>  	.source = 1 + R9A06G032_##_src, \
>  	.name = _n, \
> -	.div = _div, \
> -	.mul = 1 \
> +	.ffc.div = _div, \
> +	.ffc.mul = 1 \
>  }
>  #define D_DIV(_idx, _n, _src, _reg, _min, _max, ...) { \
>  	.type = K_DIV, \
>  	.index = R9A06G032_##_idx, \
>  	.source = 1 + R9A06G032_##_src, \
>  	.name = _n, \
> -	.reg = _reg, \
> -	.div_min = _min, \
> -	.div_max = _max, \
> -	.div_table = { __VA_ARGS__ } \
> +	.div.reg = _reg, \
> +	.div.min = _min, \
> +	.div.max = _max, \
> +	.div.table = { __VA_ARGS__ } \
>  }
>  #define D_UGATE(_idx, _n, _src, _g, _g1, _r1, _g2, _r2) { \
>  	.type = K_DUALGATE, \
> @@ -1063,14 +1066,14 @@ r9a06g032_register_div(struct r9a06g032_priv *clocks,
>  
>  	div->clocks = clocks;
>  	div->index = desc->index;
> -	div->reg = desc->reg;
> +	div->reg = desc->div.reg;
>  	div->hw.init = &init;
> -	div->min = desc->div_min;
> -	div->max = desc->div_max;
> +	div->min = desc->div.min;
> +	div->max = desc->div.max;
>  	/* populate (optional) divider table fixed values */
>  	for (i = 0; i < ARRAY_SIZE(div->table) &&
> -	     i < ARRAY_SIZE(desc->div_table) && desc->div_table[i]; i++) {
> -		div->table[div->table_size++] = desc->div_table[i];
> +	     i < ARRAY_SIZE(desc->div.table) && desc->div.table[i]; i++) {
> +		div->table[div->table_size++] = desc->div.table[i];
>  	}
>  
>  	clk = clk_register(NULL, &div->hw);
> @@ -1333,7 +1336,8 @@ static int __init r9a06g032_clocks_probe(struct platform_device *pdev)
>  		case K_FFC:
>  			clk = clk_register_fixed_factor(NULL, d->name,
>  							parent_name, 0,
> -							d->mul, d->div);
> +							d->ffc.mul,
> +							d->ffc.div);
>  			break;
>  		case K_GATE:
>  			clk = r9a06g032_register_gate(clocks, parent_name, d);


Thanks,
Miquèl

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.