[08/20] interconnect: qcom: smd-rpm: Add rpmcc handling skeleton code
Commit Message
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@linaro.org>
---
drivers/interconnect/qcom/icc-rpm.h | 15 +++++++++++++++
drivers/interconnect/qcom/smd-rpm.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
Comments
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@linaro.org>
> ---
> 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?
>
> 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.
> +
> return 0;
> }
>
>
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@linaro.org>
>> ---
>> 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;
>> }
>>
>
@@ -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
@@ -16,6 +16,7 @@
#include "icc-rpm.h"
#define RPM_KEY_BW 0x00007762
+#define QCOM_RPM_SMD_KEY_RATE 0x007a484b
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;
+
return 0;
}