From: Stephen Boyd <sboyd@kernel.org>
To: Li Zhengyu <lizhengyu3@huawei.com>, quic_tdas@quicinc.com
Cc: agross@kernel.org, bjorn.andersson@linaro.org,
linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] clk: qcom: clk-rpmh: Fix if statement to match comment
Date: Fri, 10 Jun 2022 12:58:54 -0700 [thread overview]
Message-ID: <20220610195856.A2D7EC3411C@smtp.kernel.org> (raw)
In-Reply-To: <20220531094539.252642-1-lizhengyu3@huawei.com>
Quoting Li Zhengyu (2022-05-31 02:45:39)
> (c->state) is u32, (enable) is bool. It returns false when
> (c->state) > 1 and (enable) is true. Convert (c->state) to bool.
>
> Signed-off-by: Li Zhengyu <lizhengyu3@huawei.com>
Nice catch! It looks like it fixes an optimization, where we don't want
to run through and check has_state_changed() if this clk is already
enabled or disabled. But how does this ever happen? The clk framework
already reference counts prepare/unprepare, so how can we get into this
function when the condition would be true, after this patch?
I think we can simply remove the if condition entirely. Do you agree?
> ---
> drivers/clk/qcom/clk-rpmh.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> index aed907982344..851e127432a9 100644
> --- a/drivers/clk/qcom/clk-rpmh.c
> +++ b/drivers/clk/qcom/clk-rpmh.c
> @@ -196,7 +196,7 @@ static int clk_rpmh_aggregate_state_send_command(struct clk_rpmh *c,
> int ret;
>
> /* Nothing required to be done if already off or on */
> - if (enable == c->state)
> + if (enable == !!c->state)
> return 0;
>
> c->state = enable ? c->valid_state_mask : 0;
next prev parent reply other threads:[~2022-06-10 19:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-31 9:45 [PATCH] clk: qcom: clk-rpmh: Fix if statement to match comment Li Zhengyu
2022-06-10 19:58 ` Stephen Boyd [this message]
2022-06-13 3:50 ` lizhengyu (E)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220610195856.A2D7EC3411C@smtp.kernel.org \
--to=sboyd@kernel.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lizhengyu3@huawei.com \
--cc=quic_tdas@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.