On Tue 12 Apr 14:45 CDT 2022, Stephen Boyd wrote:
> Set the wake and sleep state for BCM clks here, not just the active
> state, as the active only state is dropped when CPUs go to deep idle.
> This ensures the clk is always on when the driver thinks it is on.
>
> This was found by inspection, and could very well be incorrect if the
> RPMh hardware copies over the active only state to the sleep and wake
> states.
>
Taking another look at this patch and now it makes perfect sense to me.
Sorry for not grasping the problem earlier.
Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
Will you take this in fixes, or do you want me to pick it for 5.19?
> Cc: Alex Elder <elder@xxxxxxxxxx>
> Cc: Taniya Das <quic_tdas@xxxxxxxxxxx>
> Fixes: 04053f4d23a4 ("clk: qcom: clk-rpmh: Add IPA clock support")
> Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> ---
> drivers/clk/qcom/clk-rpmh.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> index aed907982344..29da1ffd10cf 100644
> --- a/drivers/clk/qcom/clk-rpmh.c
> +++ b/drivers/clk/qcom/clk-rpmh.c
> @@ -260,6 +260,7 @@ static int clk_rpmh_bcm_send_cmd(struct clk_rpmh *c, bool enable)
> struct tcs_cmd cmd = { 0 };
> u32 cmd_state;
> int ret = 0;
> + enum rpmh_state state;
>
> mutex_lock(&rpmh_clk_lock);
> if (enable) {
> @@ -274,15 +275,19 @@ static int clk_rpmh_bcm_send_cmd(struct clk_rpmh *c, bool enable)
> cmd.addr = c->res_addr;
> cmd.data = BCM_TCS_CMD(1, enable, 0, cmd_state);
>
> - ret = clk_rpmh_send(c, RPMH_ACTIVE_ONLY_STATE, &cmd, enable);
> - if (ret) {
> - dev_err(c->dev, "set active state of %s failed: (%d)\n",
> - c->res_name, ret);
> - } else {
> - c->last_sent_aggr_state = cmd_state;
> + for (state = RPMH_SLEEP_STATE; state <= RPMH_ACTIVE_ONLY_STATE; state++) {
> + ret = clk_rpmh_send(c, state, &cmd, enable);
Nit. We only need to pass the positive enable on the last iteration here...
Regards,
Bjorn
> + if (ret) {
> + dev_err(c->dev, "set %s state of %s failed: (%d)\n",
> + !state ? "sleep" :
> + state == RPMH_WAKE_ONLY_STATE ?
> + "wake" : "active", c->res_name, ret);
> + goto out;
> + }
> }
> + c->last_sent_aggr_state = cmd_state;
> }
> -
> +out:
> mutex_unlock(&rpmh_clk_lock);
>
> return ret;
>
> base-commit: 3123109284176b1532874591f7c81f3837bbdc17
> --
> https://chromeos.dev
>
