diff mbox series

clk: Delete redundant logic.

Message ID tencent_713186AE6290AC7B8037FD247F5BF04C0308@qq.com (mailing list archive)
State Changes Requested, archived
Headers show
Series clk: Delete redundant logic. | expand

Commit Message

jiping huang Aug. 26, 2024, 8:29 a.m. UTC 
In fact, the local variable "best_parent_rate" saved at the function
entrance is not used later on.

Signed-off-by: jiping huang <huangjiping95@qq.com>

Comments

Vladimir Zapolskiy Aug. 26, 2024, 9:40 a.m. UTC | #1 
On 8/26/24 11:29, jiping huang wrote:
> In fact, the local variable "best_parent_rate" saved at the function
> entrance is not used later on.

It's non-obvious that the stored value is not used later on in the function,
and likely there are use-cases, when it is used.

However the very least the commit message is obviously wrong stating that the
local variable is not used later on, since the local variable is used later on.

You need a more solid justification for the change.

> Signed-off-by: jiping huang <huangjiping95@qq.com>
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 8cca52be993f..d076939c42ab 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2295,11 +2295,6 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core,
>   	if (IS_ERR_OR_NULL(core))
>   		return NULL;
>   
> -	/* save parent rate, if it exists */
> -	parent = old_parent = core->parent;
> -	if (parent)
> -		best_parent_rate = parent->rate;
> -
>   	clk_core_get_boundaries(core, &min_rate, &max_rate);
>   
>   	/* find the closest rate and parent clk/rate */

--
Best wishes,
Vladimir
jiping huang Aug. 26, 2024, 2:45 p.m. UTC | #2 
I'm very sorry, my submitted description may not have expressed it clearly. 
  Actually, it is, the local variable 'best_crent_rate' is only used in line 2355 \
for the judgment 'best_crent_rate!=parent ->rate'. However, if the \
"if (clk_core_can_round (core))" branch condition in line 2306 is true, \
the value of the local variable "best_crent_rate" will be updated by \
"best_crent_rate=req.best_crent_rate;" in line 2319, otherwise it will \
be directly returned in the "else if" branch in line 2325 and the "else" branch \
in line 2329. 
  In summary, it is unnecessary to store the "parent ->rate" value in "best_crent_rate" in line 2301.
kernel test robot Aug. 27, 2024, 10:37 a.m. UTC | #3 
Hi jiping,

kernel test robot noticed the following build warnings:

[auto build test WARNING on clk/clk-next]
[also build test WARNING on linus/master v6.11-rc5 next-20240827]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/jiping-huang/clk-Delete-redundant-logic/20240826-170900
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/tencent_713186AE6290AC7B8037FD247F5BF04C0308%40qq.com
patch subject: [PATCH] clk: Delete redundant logic.
config: i386-buildonly-randconfig-005-20240827 (https://download.01.org/0day-ci/archive/20240827/202408271846.xZq9AEQa-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240827/202408271846.xZq9AEQa-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408271846.xZq9AEQa-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/clk/clk.c:2320:14: warning: variable 'parent' is uninitialized when used here [-Wuninitialized]
    2320 |         } else if (!parent || !(core->flags & CLK_SET_RATE_PARENT)) {
         |                     ^~~~~~
   drivers/clk/clk.c:2286:38: note: initialize the variable 'parent' to silence this warning
    2286 |         struct clk_core *old_parent, *parent;
         |                                             ^
         |                                              = NULL
>> drivers/clk/clk.c:2332:16: warning: variable 'old_parent' is uninitialized when used here [-Wuninitialized]
    2332 |         if (parent != old_parent &&
         |                       ^~~~~~~~~~
   drivers/clk/clk.c:2286:29: note: initialize the variable 'old_parent' to silence this warning
    2286 |         struct clk_core *old_parent, *parent;
         |                                    ^
         |                                     = NULL
   2 warnings generated.


vim +/parent +2320 drivers/clk/clk.c

b2476490ef11134 Mike Turquette  2012-03-15  2277  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2278  /*
4dff95dc9477a34 Stephen Boyd    2015-04-30  2279   * calculate the new rates returning the topmost clock that has to be
4dff95dc9477a34 Stephen Boyd    2015-04-30  2280   * changed.
4dff95dc9477a34 Stephen Boyd    2015-04-30  2281   */
4dff95dc9477a34 Stephen Boyd    2015-04-30  2282  static struct clk_core *clk_calc_new_rates(struct clk_core *core,
4dff95dc9477a34 Stephen Boyd    2015-04-30  2283  					   unsigned long rate)
4dff95dc9477a34 Stephen Boyd    2015-04-30  2284  {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2285  	struct clk_core *top = core;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2286  	struct clk_core *old_parent, *parent;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2287  	unsigned long best_parent_rate = 0;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2288  	unsigned long new_rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2289  	unsigned long min_rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2290  	unsigned long max_rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2291  	int p_index = 0;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2292  	long ret;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2293  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2294  	/* sanity */
4dff95dc9477a34 Stephen Boyd    2015-04-30  2295  	if (IS_ERR_OR_NULL(core))
4dff95dc9477a34 Stephen Boyd    2015-04-30  2296  		return NULL;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2297  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2298  	clk_core_get_boundaries(core, &min_rate, &max_rate);
71472c0c06cf9a3 James Hogan     2013-07-29  2299  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2300  	/* find the closest rate and parent clk/rate */
0f6cc2b8e94da54 Jerome Brunet   2017-12-01  2301  	if (clk_core_can_round(core)) {
0817b62cc037a56 Boris Brezillon 2015-07-07  2302  		struct clk_rate_request req;
0817b62cc037a56 Boris Brezillon 2015-07-07  2303  
718af795d3fd786 Maxime Ripard   2022-08-16  2304  		clk_core_init_rate_req(core, &req, rate);
0f6cc2b8e94da54 Jerome Brunet   2017-12-01  2305  
49e62e0d96baf72 Maxime Ripard   2022-10-26  2306  		trace_clk_rate_request_start(&req);
49e62e0d96baf72 Maxime Ripard   2022-10-26  2307  
0f6cc2b8e94da54 Jerome Brunet   2017-12-01  2308  		ret = clk_core_determine_round_nolock(core, &req);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2309  		if (ret < 0)
4dff95dc9477a34 Stephen Boyd    2015-04-30  2310  			return NULL;
035a61c314eb3da Tomeu Vizoso    2015-01-23  2311  
49e62e0d96baf72 Maxime Ripard   2022-10-26  2312  		trace_clk_rate_request_done(&req);
49e62e0d96baf72 Maxime Ripard   2022-10-26  2313  
0817b62cc037a56 Boris Brezillon 2015-07-07  2314  		best_parent_rate = req.best_parent_rate;
0817b62cc037a56 Boris Brezillon 2015-07-07  2315  		new_rate = req.rate;
0817b62cc037a56 Boris Brezillon 2015-07-07  2316  		parent = req.best_parent_hw ? req.best_parent_hw->core : NULL;
1c8e600440c7f50 Tomeu Vizoso    2015-01-23  2317  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2318  		if (new_rate < min_rate || new_rate > max_rate)
4dff95dc9477a34 Stephen Boyd    2015-04-30  2319  			return NULL;
4dff95dc9477a34 Stephen Boyd    2015-04-30 @2320  	} else if (!parent || !(core->flags & CLK_SET_RATE_PARENT)) {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2321  		/* pass-through clock without adjustable parent */
4dff95dc9477a34 Stephen Boyd    2015-04-30  2322  		core->new_rate = core->rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2323  		return NULL;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2324  	} else {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2325  		/* pass-through clock with adjustable parent */
4dff95dc9477a34 Stephen Boyd    2015-04-30  2326  		top = clk_calc_new_rates(parent, rate);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2327  		new_rate = parent->new_rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2328  		goto out;
1c8e600440c7f50 Tomeu Vizoso    2015-01-23  2329  	}
035a61c314eb3da Tomeu Vizoso    2015-01-23  2330  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2331  	/* some clocks must be gated to change parent */
4dff95dc9477a34 Stephen Boyd    2015-04-30 @2332  	if (parent != old_parent &&
4dff95dc9477a34 Stephen Boyd    2015-04-30  2333  	    (core->flags & CLK_SET_PARENT_GATE) && core->prepare_count) {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2334  		pr_debug("%s: %s not gated but wants to reparent\n",
4dff95dc9477a34 Stephen Boyd    2015-04-30  2335  			 __func__, core->name);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2336  		return NULL;
035a61c314eb3da Tomeu Vizoso    2015-01-23  2337  	}
b2476490ef11134 Mike Turquette  2012-03-15  2338  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2339  	/* try finding the new parent index */
4dff95dc9477a34 Stephen Boyd    2015-04-30  2340  	if (parent && core->num_parents > 1) {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2341  		p_index = clk_fetch_parent_index(core, parent);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2342  		if (p_index < 0) {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2343  			pr_debug("%s: clk %s can not be parent of clk %s\n",
4dff95dc9477a34 Stephen Boyd    2015-04-30  2344  				 __func__, parent->name, core->name);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2345  			return NULL;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2346  		}
4dff95dc9477a34 Stephen Boyd    2015-04-30  2347  	}
b2476490ef11134 Mike Turquette  2012-03-15  2348  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2349  	if ((core->flags & CLK_SET_RATE_PARENT) && parent &&
4dff95dc9477a34 Stephen Boyd    2015-04-30  2350  	    best_parent_rate != parent->rate)
4dff95dc9477a34 Stephen Boyd    2015-04-30  2351  		top = clk_calc_new_rates(parent, best_parent_rate);
035a61c314eb3da Tomeu Vizoso    2015-01-23  2352  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2353  out:
4dff95dc9477a34 Stephen Boyd    2015-04-30  2354  	clk_calc_subtree(core, new_rate, parent, p_index);
b2476490ef11134 Mike Turquette  2012-03-15  2355  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2356  	return top;
b2476490ef11134 Mike Turquette  2012-03-15  2357  }
b2476490ef11134 Mike Turquette  2012-03-15  2358
Dan Carpenter Aug. 29, 2024, 9:52 a.m. UTC | #4 
Hi jiping,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/jiping-huang/clk-Delete-redundant-logic/20240826-170900
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/tencent_713186AE6290AC7B8037FD247F5BF04C0308%40qq.com
patch subject: [PATCH] clk: Delete redundant logic.
config: x86_64-randconfig-161-20240827 (https://download.01.org/0day-ci/archive/20240829/202408291052.4ngp4ieh-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202408291052.4ngp4ieh-lkp@intel.com/

New smatch warnings:
drivers/clk/clk.c:2320 clk_calc_new_rates() error: uninitialized symbol 'parent'.
drivers/clk/clk.c:2332 clk_calc_new_rates() error: uninitialized symbol 'old_parent'.

vim +/parent +2320 drivers/clk/clk.c

4dff95dc9477a34 Stephen Boyd    2015-04-30  2282  static struct clk_core *clk_calc_new_rates(struct clk_core *core,
4dff95dc9477a34 Stephen Boyd    2015-04-30  2283  					   unsigned long rate)
4dff95dc9477a34 Stephen Boyd    2015-04-30  2284  {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2285  	struct clk_core *top = core;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2286  	struct clk_core *old_parent, *parent;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2287  	unsigned long best_parent_rate = 0;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2288  	unsigned long new_rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2289  	unsigned long min_rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2290  	unsigned long max_rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2291  	int p_index = 0;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2292  	long ret;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2293  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2294  	/* sanity */
4dff95dc9477a34 Stephen Boyd    2015-04-30  2295  	if (IS_ERR_OR_NULL(core))
4dff95dc9477a34 Stephen Boyd    2015-04-30  2296  		return NULL;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2297  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2298  	clk_core_get_boundaries(core, &min_rate, &max_rate);
71472c0c06cf9a3 James Hogan     2013-07-29  2299  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2300  	/* find the closest rate and parent clk/rate */
0f6cc2b8e94da54 Jerome Brunet   2017-12-01  2301  	if (clk_core_can_round(core)) {
0817b62cc037a56 Boris Brezillon 2015-07-07  2302  		struct clk_rate_request req;
0817b62cc037a56 Boris Brezillon 2015-07-07  2303  
718af795d3fd786 Maxime Ripard   2022-08-16  2304  		clk_core_init_rate_req(core, &req, rate);
0f6cc2b8e94da54 Jerome Brunet   2017-12-01  2305  
49e62e0d96baf72 Maxime Ripard   2022-10-26  2306  		trace_clk_rate_request_start(&req);
49e62e0d96baf72 Maxime Ripard   2022-10-26  2307  
0f6cc2b8e94da54 Jerome Brunet   2017-12-01  2308  		ret = clk_core_determine_round_nolock(core, &req);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2309  		if (ret < 0)
4dff95dc9477a34 Stephen Boyd    2015-04-30  2310  			return NULL;
035a61c314eb3da Tomeu Vizoso    2015-01-23  2311  
49e62e0d96baf72 Maxime Ripard   2022-10-26  2312  		trace_clk_rate_request_done(&req);
49e62e0d96baf72 Maxime Ripard   2022-10-26  2313  
0817b62cc037a56 Boris Brezillon 2015-07-07  2314  		best_parent_rate = req.best_parent_rate;
0817b62cc037a56 Boris Brezillon 2015-07-07  2315  		new_rate = req.rate;
0817b62cc037a56 Boris Brezillon 2015-07-07  2316  		parent = req.best_parent_hw ? req.best_parent_hw->core : NULL;
1c8e600440c7f50 Tomeu Vizoso    2015-01-23  2317  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2318  		if (new_rate < min_rate || new_rate > max_rate)
4dff95dc9477a34 Stephen Boyd    2015-04-30  2319  			return NULL;
4dff95dc9477a34 Stephen Boyd    2015-04-30 @2320  	} else if (!parent || !(core->flags & CLK_SET_RATE_PARENT)) {
                                                                    ^^^^^^
parent isn't initialized.

4dff95dc9477a34 Stephen Boyd    2015-04-30  2321  		/* pass-through clock without adjustable parent */
4dff95dc9477a34 Stephen Boyd    2015-04-30  2322  		core->new_rate = core->rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2323  		return NULL;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2324  	} else {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2325  		/* pass-through clock with adjustable parent */
4dff95dc9477a34 Stephen Boyd    2015-04-30  2326  		top = clk_calc_new_rates(parent, rate);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2327  		new_rate = parent->new_rate;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2328  		goto out;
1c8e600440c7f50 Tomeu Vizoso    2015-01-23  2329  	}
035a61c314eb3da Tomeu Vizoso    2015-01-23  2330  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2331  	/* some clocks must be gated to change parent */
4dff95dc9477a34 Stephen Boyd    2015-04-30 @2332  	if (parent != old_parent &&
                                                                      ^^^^^^^^^^
old_parent is never set.

4dff95dc9477a34 Stephen Boyd    2015-04-30  2333  	    (core->flags & CLK_SET_PARENT_GATE) && core->prepare_count) {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2334  		pr_debug("%s: %s not gated but wants to reparent\n",
4dff95dc9477a34 Stephen Boyd    2015-04-30  2335  			 __func__, core->name);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2336  		return NULL;
035a61c314eb3da Tomeu Vizoso    2015-01-23  2337  	}
b2476490ef11134 Mike Turquette  2012-03-15  2338  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2339  	/* try finding the new parent index */
4dff95dc9477a34 Stephen Boyd    2015-04-30  2340  	if (parent && core->num_parents > 1) {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2341  		p_index = clk_fetch_parent_index(core, parent);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2342  		if (p_index < 0) {
4dff95dc9477a34 Stephen Boyd    2015-04-30  2343  			pr_debug("%s: clk %s can not be parent of clk %s\n",
4dff95dc9477a34 Stephen Boyd    2015-04-30  2344  				 __func__, parent->name, core->name);
4dff95dc9477a34 Stephen Boyd    2015-04-30  2345  			return NULL;
4dff95dc9477a34 Stephen Boyd    2015-04-30  2346  		}
4dff95dc9477a34 Stephen Boyd    2015-04-30  2347  	}
b2476490ef11134 Mike Turquette  2012-03-15  2348  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2349  	if ((core->flags & CLK_SET_RATE_PARENT) && parent &&
4dff95dc9477a34 Stephen Boyd    2015-04-30  2350  	    best_parent_rate != parent->rate)
4dff95dc9477a34 Stephen Boyd    2015-04-30  2351  		top = clk_calc_new_rates(parent, best_parent_rate);
035a61c314eb3da Tomeu Vizoso    2015-01-23  2352  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2353  out:
4dff95dc9477a34 Stephen Boyd    2015-04-30  2354  	clk_calc_subtree(core, new_rate, parent, p_index);
b2476490ef11134 Mike Turquette  2012-03-15  2355  
4dff95dc9477a34 Stephen Boyd    2015-04-30  2356  	return top;
b2476490ef11134 Mike Turquette  2012-03-15  2357  }
diff mbox series

Patch

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8cca52be993f..d076939c42ab 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2295,11 +2295,6 @@  static struct clk_core *clk_calc_new_rates(struct clk_core *core,
 	if (IS_ERR_OR_NULL(core))
 		return NULL;
 
-	/* save parent rate, if it exists */
-	parent = old_parent = core->parent;
-	if (parent)
-		best_parent_rate = parent->rate;
-
 	clk_core_get_boundaries(core, &min_rate, &max_rate);
 
 	/* find the closest rate and parent clk/rate */