Re: [PATCH v9 6/7] clk: clock-wizard: Remove the hardcoding of the clock outputs

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Shubhrajyoti,

Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> wrote on Thu, 18 Feb
2021 10:19:50 +0530:

> The number of output clocks are configurable in the hardware.
> Currently the driver registers the maximum number of outputs.
> Fix the same by registering only the outputs that are there.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v4:
> Assign output in this patch
> 
>  drivers/clk/clk-xlnx-clock-wizard.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> index ed3b0ef..d403a74 100644
> --- a/drivers/clk/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> @@ -473,6 +473,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  	unsigned long rate;
>  	const char *clk_name;
>  	struct clk_wzrd *clk_wzrd;
> +	int outputs;
>  	struct device_node *np = pdev->dev.of_node;
>  
>  	clk_wzrd = devm_kzalloc(&pdev->dev, sizeof(*clk_wzrd), GFP_KERNEL);
> @@ -541,6 +542,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  		goto err_disable_clk;
>  	}
>  
> +	outputs = of_property_count_strings(np, "clock-output-names");

A check on outputs validity is probably welcome.

Also I usually prefer noutputs or nb_outputs for such variable name,
which implies a number rather than an array, but this is personal taste.

>  	/* register div */
>  	reg = (readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) &
>  			WZRD_DIVCLK_DIVIDE_MASK) >> WZRD_DIVCLK_DIVIDE_SHIFT;
> @@ -562,7 +564,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  	}
>  
>  	/* register div per output */
> -	for (i = WZRD_NUM_OUTPUTS - 1; i >= 0 ; i--) {
> +	for (i = outputs - 1; i >= 0 ; i--) {
>  		const char *clkout_name;
>  
>  		if (of_property_read_string_index(np, "clock-output-names", i,
> @@ -593,7 +595,7 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  		if (IS_ERR(clk_wzrd->clkout[i])) {
>  			int j;
>  
> -			for (j = i + 1; j < WZRD_NUM_OUTPUTS; j++)
> +			for (j = i + 1; j < outputs; j++)
>  				clk_unregister(clk_wzrd->clkout[j]);
>  			dev_err(&pdev->dev,
>  				"unable to register divider clock\n");


Thanks,
Miquèl