| Message ID | 71a9fed5f762a71248b8ac73c0a15af82f3ce1e2.1619867987.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | clk: zynqmp: pll: Remove some dead code | expand |
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. > > return hw; > } > -- > 2.30.2
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
Hi Chris, > -----Original Message----- > From: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > Sent: 03 May 2021 11:20 AM > To: Rajan Vaja <RAJANV@xilinx.com>; mturquette@baylibre.com; > sboyd@kernel.org; Michal Simek <michals@xilinx.com>; > quanyang.wang@windriver.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 > Subject: Re: [PATCH] clk: zynqmp: pll: Remove some dead code > > > 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?) [Rajan] Thanks for the clarification. Then, it looks fine. > > CJ > > > > >> return hw; > >> } > >> -- > >> 2.30.2
On Sat, 01 May 2021 13:24:32 +0200, Christophe JAILLET wrote: > '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> Reviewed-by: Michael Tretter <m.tretter@pengutronix.de> > --- > 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. Thanks. The function is correct, as this is a clock provider and should use the clk_hw. Removing the message is correct. Michael > --- > 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); > > return hw; > } > -- > 2.30.2 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
Quoting Christophe JAILLET (2021-05-01 04:24:32) > '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> > --- Applied to clk-next
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); return hw; }
'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(-)