Some bus clock should always have a minimum (19.2 MHz) vote cast on
them, otherwise the platform will fall apart, hang and reboot.
Add support for specifying which clocks should be kept alive and
always keep a vote on XO_A to make sure the clock tree doesn't
collapse. This removes the need to keep a maximum vote that was
previously guaranteed by clk_smd_rpm_handoff.
This commit is a combination of existing (not-exactly-upstream) work
by Taniya Das, Shawn Guo and myself.
Co-developed-by: Shawn Guo <shawn.guo@linaro.org>
Co-developed-by: Taniya Das <quic_tdas@quicinc.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/clk/qcom/clk-smd-rpm.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index eb7781e5c8c1..d89918f9ae60 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -45,15 +45,17 @@
}, \
}; \
__DEFINE_CLK_SMD_RPM_AO_PREFIX(_prefix, _name, _active, type, \
- r_id, key, ao_flags)
+ r_id, key, ao_flags, false)
#define __DEFINE_CLK_SMD_RPM_AO_PREFIX(_prefix, _name, _active, \
- type, r_id, key, ao_flags) \
+ type, r_id, key, ao_flags, \
+ _keep_alive) \
static struct clk_smd_rpm clk_smd_rpm_##_prefix##_active = { \
.rpm_res_type = (type), \
.rpm_clk_id = (r_id), \
.active_only = true, \
.rpm_key = (key), \
+ .keep_alive = (_keep_alive), \
.peer = &clk_smd_rpm_##_prefix##_name, \
.rate = INT_MAX, \
.hw.init = &(struct clk_init_data){ \
@@ -170,6 +172,7 @@ struct clk_smd_rpm {
const bool active_only;
bool enabled;
bool branch;
+ bool keep_alive;
struct clk_smd_rpm *peer;
struct clk_hw hw;
unsigned long rate;
@@ -198,11 +201,16 @@ static int clk_smd_rpm_handoff(struct clk_smd_rpm *r)
.value = cpu_to_le32(r->branch ? 1 : INT_MAX),
};
+ /* Set up keepalive clocks with a minimum bus rate */
+ if (r->keep_alive)
+ req.value = cpu_to_le32(19200); /* 19.2 MHz */
+
ret = qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_ACTIVE_STATE,
r->rpm_res_type, r->rpm_clk_id, &req,
sizeof(req));
if (ret)
return ret;
+
ret = qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_SLEEP_STATE,
r->rpm_res_type, r->rpm_clk_id, &req,
sizeof(req));
@@ -438,12 +446,29 @@ static int clk_smd_rpm_is_enabled(struct clk_hw *hw)
return r->enabled;
}
+static int clk_smd_rpm_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
+
+ /*
+ * RPM resolves the rates internally. All we have to do on the kernel
+ * side is ensure that we don't accidentally put down the keepalive
+ * clocks, which could happen if they received a vote below 19.2 MHz.
+ */
+ if (r->keep_alive)
+ req->rate = max(req->rate, 19200000UL);
+
+ return 0;
+}
+
static const struct clk_ops clk_smd_rpm_ops = {
.prepare = clk_smd_rpm_prepare,
.unprepare = clk_smd_rpm_unprepare,
.set_rate = clk_smd_rpm_set_rate,
.round_rate = clk_smd_rpm_round_rate,
.recalc_rate = clk_smd_rpm_recalc_rate,
+ .determine_rate = clk_smd_rpm_determine_rate,
.is_enabled = clk_smd_rpm_is_enabled,
.is_prepared = clk_smd_rpm_is_enabled,
};
--
2.39.2On 08/03/2023 23:35, Konrad Dybcio wrote:
> Some bus clock should always have a minimum (19.2 MHz) vote cast on
> them, otherwise the platform will fall apart, hang and reboot.
>
> Add support for specifying which clocks should be kept alive and
> always keep a vote on XO_A to make sure the clock tree doesn't
> collapse. This removes the need to keep a maximum vote that was
> previously guaranteed by clk_smd_rpm_handoff.
>
> This commit is a combination of existing (not-exactly-upstream) work
> by Taniya Das, Shawn Guo and myself.
>
> Co-developed-by: Shawn Guo <shawn.guo@linaro.org>
> Co-developed-by: Taniya Das <quic_tdas@quicinc.com>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/clk/qcom/clk-smd-rpm.c | 29 +++++++++++++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> index eb7781e5c8c1..d89918f9ae60 100644
> --- a/drivers/clk/qcom/clk-smd-rpm.c
> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> @@ -45,15 +45,17 @@
> }, \
> }; \
> __DEFINE_CLK_SMD_RPM_AO_PREFIX(_prefix, _name, _active, type, \
> - r_id, key, ao_flags)
> + r_id, key, ao_flags, false)
>
> #define __DEFINE_CLK_SMD_RPM_AO_PREFIX(_prefix, _name, _active, \
> - type, r_id, key, ao_flags) \
> + type, r_id, key, ao_flags, \
> + _keep_alive) \
> static struct clk_smd_rpm clk_smd_rpm_##_prefix##_active = { \
> .rpm_res_type = (type), \
> .rpm_clk_id = (r_id), \
> .active_only = true, \
> .rpm_key = (key), \
> + .keep_alive = (_keep_alive), \
> .peer = &clk_smd_rpm_##_prefix##_name, \
> .rate = INT_MAX, \
> .hw.init = &(struct clk_init_data){ \
> @@ -170,6 +172,7 @@ struct clk_smd_rpm {
> const bool active_only;
> bool enabled;
> bool branch;
> + bool keep_alive;
> struct clk_smd_rpm *peer;
> struct clk_hw hw;
> unsigned long rate;
> @@ -198,11 +201,16 @@ static int clk_smd_rpm_handoff(struct clk_smd_rpm *r)
> .value = cpu_to_le32(r->branch ? 1 : INT_MAX),
> };
>
> + /* Set up keepalive clocks with a minimum bus rate */
> + if (r->keep_alive)
> + req.value = cpu_to_le32(19200); /* 19.2 MHz */
Should this be set to cpu_to_le32(max(19200, ...)) ?
> +
> ret = qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_ACTIVE_STATE,
> r->rpm_res_type, r->rpm_clk_id, &req,
> sizeof(req));
> if (ret)
> return ret;
> +
> ret = qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_SLEEP_STATE,
> r->rpm_res_type, r->rpm_clk_id, &req,
> sizeof(req));
> @@ -438,12 +446,29 @@ static int clk_smd_rpm_is_enabled(struct clk_hw *hw)
> return r->enabled;
> }
>
> +static int clk_smd_rpm_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> +
> + /*
> + * RPM resolves the rates internally. All we have to do on the kernel
> + * side is ensure that we don't accidentally put down the keepalive
> + * clocks, which could happen if they received a vote below 19.2 MHz.
> + */
> + if (r->keep_alive)
> + req->rate = max(req->rate, 19200000UL);
> +
> + return 0;
> +}
> +
> static const struct clk_ops clk_smd_rpm_ops = {
> .prepare = clk_smd_rpm_prepare,
> .unprepare = clk_smd_rpm_unprepare,
> .set_rate = clk_smd_rpm_set_rate,
> .round_rate = clk_smd_rpm_round_rate,
> .recalc_rate = clk_smd_rpm_recalc_rate,
> + .determine_rate = clk_smd_rpm_determine_rate,
> .is_enabled = clk_smd_rpm_is_enabled,
> .is_prepared = clk_smd_rpm_is_enabled,
> };
>
--
With best wishes
Dmitry
On 9.03.2023 01:54, Dmitry Baryshkov wrote:
> On 08/03/2023 23:35, Konrad Dybcio wrote:
>> Some bus clock should always have a minimum (19.2 MHz) vote cast on
>> them, otherwise the platform will fall apart, hang and reboot.
>>
>> Add support for specifying which clocks should be kept alive and
>> always keep a vote on XO_A to make sure the clock tree doesn't
>> collapse. This removes the need to keep a maximum vote that was
>> previously guaranteed by clk_smd_rpm_handoff.
>>
>> This commit is a combination of existing (not-exactly-upstream) work
>> by Taniya Das, Shawn Guo and myself.
>>
>> Co-developed-by: Shawn Guo <shawn.guo@linaro.org>
>> Co-developed-by: Taniya Das <quic_tdas@quicinc.com>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>> drivers/clk/qcom/clk-smd-rpm.c | 29 +++++++++++++++++++++++++++--
>> 1 file changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
>> index eb7781e5c8c1..d89918f9ae60 100644
>> --- a/drivers/clk/qcom/clk-smd-rpm.c
>> +++ b/drivers/clk/qcom/clk-smd-rpm.c
>> @@ -45,15 +45,17 @@
>> }, \
>> }; \
>> __DEFINE_CLK_SMD_RPM_AO_PREFIX(_prefix, _name, _active, type, \
>> - r_id, key, ao_flags)
>> + r_id, key, ao_flags, false)
>> #define __DEFINE_CLK_SMD_RPM_AO_PREFIX(_prefix, _name, _active, \
>> - type, r_id, key, ao_flags) \
>> + type, r_id, key, ao_flags, \
>> + _keep_alive) \
>> static struct clk_smd_rpm clk_smd_rpm_##_prefix##_active = { \
>> .rpm_res_type = (type), \
>> .rpm_clk_id = (r_id), \
>> .active_only = true, \
>> .rpm_key = (key), \
>> + .keep_alive = (_keep_alive), \
>> .peer = &clk_smd_rpm_##_prefix##_name, \
>> .rate = INT_MAX, \
>> .hw.init = &(struct clk_init_data){ \
>> @@ -170,6 +172,7 @@ struct clk_smd_rpm {
>> const bool active_only;
>> bool enabled;
>> bool branch;
>> + bool keep_alive;
>> struct clk_smd_rpm *peer;
>> struct clk_hw hw;
>> unsigned long rate;
>> @@ -198,11 +201,16 @@ static int clk_smd_rpm_handoff(struct clk_smd_rpm *r)
>> .value = cpu_to_le32(r->branch ? 1 : INT_MAX),
>> };
>> + /* Set up keepalive clocks with a minimum bus rate */
>> + if (r->keep_alive)
>> + req.value = cpu_to_le32(19200); /* 19.2 MHz */
>
>
> Should this be set to cpu_to_le32(max(19200, ...)) ?
I was debating this. Downstream explicitly sets 19.2 Mhz here and
the only regression I can think of is that we'd throttle a bus that
was left on by the bootloader and is (ab)used by us..
But then, it's only an active vote, and we're voting INT_MAX on the
non-active-only one, so that's a non-issue.
So I think 19.2 here is okay as the bare minimum, whatever stupidity
the eventual interconnect driver may entail..
Konrad
>
>> +
>> ret = qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_ACTIVE_STATE,
>> r->rpm_res_type, r->rpm_clk_id, &req,
>> sizeof(req));
>> if (ret)
>> return ret;
>> +
>> ret = qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_SLEEP_STATE,
>> r->rpm_res_type, r->rpm_clk_id, &req,
>> sizeof(req));
>> @@ -438,12 +446,29 @@ static int clk_smd_rpm_is_enabled(struct clk_hw *hw)
>> return r->enabled;
>> }
>> +static int clk_smd_rpm_determine_rate(struct clk_hw *hw,
>> + struct clk_rate_request *req)
>> +{
>> + struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
>> +
>> + /*
>> + * RPM resolves the rates internally. All we have to do on the kernel
>> + * side is ensure that we don't accidentally put down the keepalive
>> + * clocks, which could happen if they received a vote below 19.2 MHz.
>> + */
>> + if (r->keep_alive)
>> + req->rate = max(req->rate, 19200000UL);
>> +
>> + return 0;
>> +}
>> +
>> static const struct clk_ops clk_smd_rpm_ops = {
>> .prepare = clk_smd_rpm_prepare,
>> .unprepare = clk_smd_rpm_unprepare,
>> .set_rate = clk_smd_rpm_set_rate,
>> .round_rate = clk_smd_rpm_round_rate,
>> .recalc_rate = clk_smd_rpm_recalc_rate,
>> + .determine_rate = clk_smd_rpm_determine_rate,
>> .is_enabled = clk_smd_rpm_is_enabled,
>> .is_prepared = clk_smd_rpm_is_enabled,
>> };
>>
>
© 2016 - 2026 Red Hat, Inc.