On 1.06.2023 12:01, Dmitry Baryshkov wrote: > On 30/05/2023 13:20, Konrad Dybcio wrote: >> Introduce qcom_icc_rpm_set_bus_rate() in preparation for handling RPM >> clock resources within the interconnect framework. This lets us greatly >> simplify all of the code handling, as setting the rate comes down to: >> >> u32 rate_khz = max(clk.sleep_rate, clk.active_rate, clk_a.active_rate) >> write_to_rpm(clock.description, rate_khz); >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> >> --- >> drivers/interconnect/qcom/icc-rpm.h | 15 +++++++++++++++ >> drivers/interconnect/qcom/smd-rpm.c | 37 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 52 insertions(+) >> >> diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h >> index 9b4ea4e39b9f..aec192321411 100644 >> --- a/drivers/interconnect/qcom/icc-rpm.h >> +++ b/drivers/interconnect/qcom/icc-rpm.h >> @@ -22,6 +22,18 @@ enum qcom_icc_type { >> QCOM_ICC_QNOC, >> }; >> +/** >> + * struct rpm_clk_resource - RPM bus clock resource >> + * @resource_type: RPM resource type of the clock resource >> + * @clock_id: index of the clock resource of a specific resource type >> + * @branch: whether the resource represents a branch clock >> +*/ >> +struct rpm_clk_resource { >> + u32 resource_type; >> + u32 clock_id; >> + bool branch; >> +}; >> + >> #define NUM_BUS_CLKS 2 >> /** >> @@ -47,6 +59,7 @@ struct qcom_icc_provider { >> int qos_offset; >> u64 bus_clk_rate[NUM_BUS_CLKS]; >> struct clk_bulk_data bus_clks[NUM_BUS_CLKS]; >> + const struct rpm_clk_resource *bus_clk_desc; >> struct clk_bulk_data *intf_clks; >> bool keep_alive; >> bool is_on; >> @@ -104,6 +117,7 @@ struct qcom_icc_desc { >> struct qcom_icc_node * const *nodes; >> size_t num_nodes; >> const char * const *bus_clocks; >> + const struct rpm_clk_resource *bus_clk_desc; >> const char * const *intf_clocks; >> size_t num_intf_clocks; >> bool keep_alive; >> @@ -125,5 +139,6 @@ int qnoc_remove(struct platform_device *pdev); >> bool qcom_icc_rpm_smd_available(void); >> int qcom_icc_rpm_smd_send(int ctx, int rsc_type, int id, u32 val); >> +int qcom_icc_rpm_set_bus_rate(const struct rpm_clk_resource *clk, u32 active_rate, u32 sleep_rate); >> #endif >> diff --git a/drivers/interconnect/qcom/smd-rpm.c b/drivers/interconnect/qcom/smd-rpm.c >> index b0183262ba66..6c51e346b326 100644 >> --- a/drivers/interconnect/qcom/smd-rpm.c >> +++ b/drivers/interconnect/qcom/smd-rpm.c >> @@ -16,6 +16,7 @@ >> #include "icc-rpm.h" >> #define RPM_KEY_BW 0x00007762 >> +#define QCOM_RPM_SMD_KEY_RATE 0x007a484b > > Nit: can we move all RPM keys to some common header? They're quite scattered across subsystems so I think it'd be a separate topic.. Also I think we could think about migrating to stringified raw values like we have in RPMh - they keys are LE 4-char, non-NULL-terminated strings (e.g. this one is 'KHz') > >> static struct qcom_smd_rpm *icc_smd_rpm; >> @@ -44,6 +45,38 @@ int qcom_icc_rpm_smd_send(int ctx, int rsc_type, int id, u32 val) >> } >> EXPORT_SYMBOL_GPL(qcom_icc_rpm_smd_send); >> +int qcom_icc_rpm_set_bus_rate(const struct rpm_clk_resource *clk, u32 active_rate, u32 sleep_rate) >> +{ >> + struct clk_smd_rpm_req req = { >> + .key = cpu_to_le32(QCOM_RPM_SMD_KEY_RATE), >> + .nbytes = cpu_to_le32(sizeof(u32)), >> + }; >> + int ret; >> + >> + /* Branch clocks are only on/off */ >> + if (clk->branch) { >> + active_rate = !!active_rate; >> + sleep_rate = !!sleep_rate; >> + } >> + >> + req.value = cpu_to_le32(active_rate); >> + ret = qcom_rpm_smd_write(icc_smd_rpm, >> + QCOM_SMD_RPM_ACTIVE_STATE, >> + clk->resource_type, >> + clk->clock_id, >> + &req, sizeof(req)); >> + if (ret) >> + return ret; >> + >> + req.value = cpu_to_le32(sleep_rate); >> + return qcom_rpm_smd_write(icc_smd_rpm, >> + QCOM_SMD_RPM_SLEEP_STATE, >> + clk->resource_type, >> + clk->clock_id, >> + &req, sizeof(req)); >> +} >> +EXPORT_SYMBOL_GPL(qcom_icc_rpm_set_bus_rate); >> + >> static int qcom_icc_rpm_smd_remove(struct platform_device *pdev) >> { >> icc_smd_rpm = NULL; >> @@ -60,6 +93,10 @@ static int qcom_icc_rpm_smd_probe(struct platform_device *pdev) >> return -ENODEV; >> } >> + /* We need the clock driver to kick things off first to avoid ugly races */ >> + if (!qcom_smd_rpm_scaling_available()) >> + return -EPROBE_DEFER; > > > This should not be a part of this commit. "eeh", it makes no functional difference, as if this patch is the HEAD, we're still consuming the clocks via CCF, which guarantees this is always true.. I can move it if you wish. Konrad > >> + >> return 0; >> } >> >