[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <eb39cef0-7e1c-f0dc-12fa-6a5a746d17d1@linaro.org>
Date: Wed, 14 Jun 2023 19:40:06 +0200
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Stephan Gerhold <stephan@...hold.net>
Cc: Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Georgi Djakov <djakov@...nel.org>,
Leo Yan <leo.yan@...aro.org>,
Evan Green <evgreen@...omium.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Marijn Suijten <marijn.suijten@...ainline.org>,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-clk@...r.kernel.org, linux-pm@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v5 08/22] interconnect: qcom: smd-rpm: Add rpmcc handling
skeleton code
On 14.06.2023 16:47, Stephan Gerhold wrote:
> On Wed, Jun 14, 2023 at 12:22:19PM +0200, 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@...aro.org>
>> ---
>> drivers/interconnect/qcom/icc-rpm.h | 15 +++++++++++++++
>> drivers/interconnect/qcom/smd-rpm.c | 21 +++++++++++++++++++++
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
>> index 9ec90e13bfbd..d857fb1efb75 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 {
>> unsigned 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, int rsc_type, u32 rate);
>>
>> #endif
>> diff --git a/drivers/interconnect/qcom/smd-rpm.c b/drivers/interconnect/qcom/smd-rpm.c
>> index b0183262ba66..b06374340eeb 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
>>
>> static struct qcom_smd_rpm *icc_smd_rpm;
>>
>> @@ -44,6 +45,26 @@ 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, int rsc_type, u32 rate)
>> +{
>> + struct clk_smd_rpm_req req = {
>> + .key = cpu_to_le32(QCOM_RPM_SMD_KEY_RATE),
>> + .nbytes = cpu_to_le32(sizeof(u32)),
>> + };
>> +
>> + /* Branch clocks are only on/off */
>> + if (clk->branch)
>> + rate = !!rate;
>> +
>> + req.value = cpu_to_le32(rate);
>> + return qcom_rpm_smd_write(icc_smd_rpm,
>> + rsc_type,
>> + clk->resource_type,
>
> Sorry to have more minor comments but as you can see here the resource
> type is taken from the rpm_clk_resource. The parameter that you are
> describing as "rsc_type" is actually the "ctx" in the other function. :')
Meh I fixed it too fast.. thanks
Konrad
>
> If you fix this feel free to add my:
>
> Reviewed-by: Stephan Gerhold <stephan@...hold.net>
>
> Thanks,
> Stephan
Powered by blists - more mailing lists