Re: [PATCH] clk: zynqmp: pll: Remove some dead code

[Christophe JAILLET <christophe.jaillet@wanadoo.fr>]

Le 03/05/2021 à 06:56, Rajan Vaja a écrit :
> Hi,
>
> Thanks for the patch.
>
>> -----Original Message-----
>> From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> Sent: 01 May 2021 04:55 PM
>> To: mturquette@baylibre.com; sboyd@kernel.org; Michal Simek
>> <michals@xilinx.com>; quanyang.wang@windriver.com; Rajan Vaja
>> <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>; Tejas Patel
>> <tejasp@xlnx.xilinx.com>; Shubhrajyoti Datta <shubhraj@xilinx.com>
>> Cc: linux-clk@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; kernel-janitors@vger.kernel.org; Christophe JAILLET
>> <christophe.jaillet@wanadoo.fr>
>> Subject: [PATCH] clk: zynqmp: pll: Remove some dead code
>>
>> 'clk_hw_set_rate_range()' does not return any error code and 'ret' is
>> known to be 0 at this point, so this message can never be displayed.
>>
>> Remove it.
>>
>> Fixes: 3fde0e16d016 ("drivers: clk: Add ZynqMP clock driver")
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> HOWEVER, the message is about 'clk_set_rate_range()', not
>> 'clk_hw_set_rate_range()'. So the message is maybe correct and the
>> 'xxx_rate_range()' function incorrect.
>> ---
>>   drivers/clk/zynqmp/pll.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/clk/zynqmp/pll.c b/drivers/clk/zynqmp/pll.c
>> index abe6afbf3407..af11e9400058 100644
>> --- a/drivers/clk/zynqmp/pll.c
>> +++ b/drivers/clk/zynqmp/pll.c
>> @@ -331,8 +331,6 @@ struct clk_hw *zynqmp_clk_register_pll(const char *name,
>> u32 clk_id,
>>   	}
>>
>>   	clk_hw_set_rate_range(hw, PS_PLL_VCO_MIN, PS_PLL_VCO_MAX);
>> -	if (ret < 0)
>> -		pr_err("%s:ERROR clk_set_rate_range failed %d\n", name, ret);
> [Rajan] Instead of removing, can we get return value of clk_hw_set_rate_range() and
> print in case of an error.

Hi,

if it was possible, it is what I would have proposed because it looks 
'logical'.

However, 'clk_hw_set_rate_range()' returns void.
Hence my comment about 'clk_hw_set_rate_range' being the correct 
function to call or not. (i.e. is the comment right and 
'clk_hw_set_rate_range' wrong?)

CJ



>>   	return hw;
>>   }
>> --
>> 2.30.2