linux-kernel - Re: [PATCH v2] clk: Show clock rate and error value when fail to set clock rate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <55360C44.4020107@samsung.com>
Date:	Tue, 21 Apr 2015 10:37:24 +0200
From:	Sylwester Nawrocki <s.nawrocki@...sung.com>
To:	Chanwoo Choi <cw00.choi@...sung.com>
Cc:	mturquette@...aro.org, sboyd@...eaurora.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] clk: Show clock rate and error value when fail to set
 clock rate

Hi Chanwoo,

On 20/04/15 13:08, Chanwoo Choi wrote:
> This patch shows the current clock rate nad error value when clk_set_rate()
> fail to set the clock rate because log messgae means only the clock rate.
> 
> Cc: Mike Turquette <mturquette@...aro.org>
> Cc: Stephen Boyd <sboyd@...eaurora.org>
> Cc: Sylwester Nawrocki <s.nawrocki@...sung.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.com>
> ---
> Changes from v1:
> - Add error value of clk-set_rate() with current clock rate
> 
>  drivers/clk/clk-conf.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
> index aad4796..2b6375f 100644
> --- a/drivers/clk/clk-conf.c
> +++ b/drivers/clk/clk-conf.c
> @@ -107,8 +107,9 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
>  
>  			rc = clk_set_rate(clk, rate);
>  			if (rc < 0)
> -				pr_err("clk: couldn't set %s clock rate: %d\n",
> -				       __clk_get_name(clk), rc);
> +				pr_err("clk: couldn't set %s rate to %ld (%d)\n",
> +				       __clk_get_name(clk), clk_get_rate(clk),
> +				       rc);

Apologies for my previous misleading comment, with this patch
you log current clock rate as the rate which was attempted to 
be set. And these two will likely be different when clk_set_rate()
failed. So it should rather be:

pr_err("clk: couldn't set %s clk rate to %ld (%d)\n",
       __clk_get_name(clk), rate, rc);

If you really need to log current rate, then it could be changed to:

pr_err("clk: couldn't set %s clk rate to %ld (%d), current rate: %ld\n",
       __clk_get_name(clk), rate, rc, clk_get_rate(clk));

But I doubt we need to log all these parameters. The clock rate
changes could be logged with the ftrace functionality added 
recently to the clk core.

>  			clk_put(clk);
>  		}
>  		index++;

-- 
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/