Re: [PATCH 08/20] interconnect: qcom: smd-rpm: Add rpmcc handling skeleton code

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

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;
>   }
>   
> 

-- 
With best wishes
Dmitry