Re: [PATCH 3/5] clk: qcom: cbf-msm8996: scale CBF clock according to the CPUfreq

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

On 12/01/2023 17:00, Konrad Dybcio wrote:
> 
> 
> On 11.01.2023 20:57, Dmitry Baryshkov wrote:
>> Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth)
>> according to CPU frequencies.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
> TODO: assess if we should be doing this manually or rely on SPDM..

Manually, see the msm-3.18's devfreq-cpu and m4m-cpufreq maps.

SPDM uses CBF as an input not as an output.

> 
> Konrad
>>   drivers/clk/qcom/clk-cbf-8996.c | 141 +++++++++++++++++++++++++++++++-
>>   1 file changed, 140 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c
>> index bdd4f8b48a7e..76db623b0f92 100644
>> --- a/drivers/clk/qcom/clk-cbf-8996.c
>> +++ b/drivers/clk/qcom/clk-cbf-8996.c
>> @@ -5,6 +5,7 @@
>>   #include <linux/bitfield.h>
>>   #include <linux/clk.h>
>>   #include <linux/clk-provider.h>
>> +#include <linux/interconnect-provider.h>
>>   #include <linux/of.h>
>>   #include <linux/module.h>
>>   #include <linux/platform_device.h>
>> @@ -225,6 +226,133 @@ static const struct regmap_config cbf_msm8996_regmap_config = {
>>   	.val_format_endian	= REGMAP_ENDIAN_LITTLE,
>>   };
>>   
>> +#ifdef CONFIG_INTERCONNECT
>> +struct qcom_msm8996_cbf_icc_provider {
>> +	struct icc_provider provider;
>> +	struct clk *clk;
>> +};
>> +
>> +#define to_qcom_cbf_provider(_provider) \
>> +	container_of(_provider, struct qcom_msm8996_cbf_icc_provider, provider)
>> +
>> +enum {
>> +	CBF_MASTER_NODE = 2000,
>> +	CBF_SLAVE_NODE
>> +};
>> +
>> +#define CBF_NUM_NODES 2
>> +
>> +static int qcom_msm8996_cbf_set(struct icc_node *src, struct icc_node *dst)
>> +{
>> +	struct qcom_msm8996_cbf_icc_provider *qp;
>> +
>> +	qp = to_qcom_cbf_provider(src->provider);
>> +
>> +	return clk_set_rate(qp->clk, icc_units_to_bps(dst->peak_bw));
>> +}
>> +
>> +static int qcom_msm8996_cbf_icc_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
>> +{
>> +	struct qcom_msm8996_cbf_icc_provider *qp;
>> +
>> +	qp = to_qcom_cbf_provider(node->provider);
>> +	*peak = clk_get_rate(qp->clk) / 1000ULL;
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev, struct clk_hw *cbf_hw)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct qcom_msm8996_cbf_icc_provider *qp;
>> +	struct icc_provider *provider;
>> +	struct icc_onecell_data *data;
>> +	struct icc_node *node;
>> +	struct clk *clk;
>> +	int ret;
>> +
>> +	clk = devm_clk_hw_get_clk(dev, cbf_hw, "cbf");
>> +	if (IS_ERR(clk))
>> +		return PTR_ERR(clk);
>> +
>> +	data = devm_kzalloc(dev, struct_size(data, nodes, CBF_NUM_NODES), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	data->num_nodes = CBF_NUM_NODES;
>> +
>> +	qp = devm_kzalloc(dev, sizeof(*qp), GFP_KERNEL);
>> +	if (!qp)
>> +		return -ENOMEM;
>> +
>> +	qp->clk = clk;
>> +
>> +	provider = &qp->provider;
>> +	provider->dev = dev;
>> +	provider->get_bw = qcom_msm8996_cbf_icc_get_bw;
>> +	provider->set = qcom_msm8996_cbf_set;
>> +	provider->aggregate = icc_std_aggregate;
>> +	provider->xlate = of_icc_xlate_onecell;
>> +	INIT_LIST_HEAD(&provider->nodes);
>> +	provider->data = data;
>> +
>> +	ret = icc_provider_add(provider);
>> +	if (ret) {
>> +		dev_err(dev, "error adding interconnect provider\n");
>> +		return ret;
>> +	}
>> +
>> +	node = icc_node_create(CBF_MASTER_NODE);
>> +	if (IS_ERR(node)) {
>> +		ret = PTR_ERR(node);
>> +		goto err;
>> +	}
>> +
>> +	node->name = "cbf_master";
>> +	icc_node_add(node, provider);
>> +	icc_link_create(node, CBF_SLAVE_NODE);
>> +	data->nodes[0] = node;
>> +
>> +	node = icc_node_create(CBF_SLAVE_NODE);
>> +	if (IS_ERR(node)) {
>> +		ret = PTR_ERR(node);
>> +		goto err;
>> +	}
>> +
>> +	node->name = "cbf_slave";
>> +	icc_node_add(node, provider);
>> +	data->nodes[1] = node;
>> +
>> +	platform_set_drvdata(pdev, provider);
>> +
>> +	return 0;
>> +
>> +err:
>> +	icc_nodes_remove(provider);
>> +	icc_provider_del(provider);
>> +
>> +	return ret;
>> +}
>> +
>> +static int qcom_msm8996_cbf_icc_remove(struct platform_device *pdev)
>> +{
>> +	struct icc_provider *provider = platform_get_drvdata(pdev);
>> +
>> +	icc_nodes_remove(provider);
>> +	icc_provider_del(provider);
>> +
>> +	return 0;
>> +}
>> +#else
>> +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev)
>> +{
>> +	dev_warn(&pdev->dev, "interconnects support is disabled, CBF clock is fixed\n");
>> +
>> +	return 0;
>> +}
>> +#define qcom_msm8996_cbf_icc_remove(pdev) (0)
>> +#endif
>> +
>>   static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
>>   {
>>   	void __iomem *base;
>> @@ -284,7 +412,16 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> -	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw);
>> +	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return qcom_msm8996_cbf_icc_register(pdev, &cbf_mux.clkr.hw);
>> +}
>> +
>> +static int qcom_msm8996_cbf_remove(struct platform_device *pdev)
>> +{
>> +	return qcom_msm8996_cbf_icc_remove(pdev);
>>   }
>>   
>>   static const struct of_device_id qcom_msm8996_cbf_match_table[] = {
>> @@ -295,9 +432,11 @@ MODULE_DEVICE_TABLE(of, qcom_msm8996_cbf_match_table);
>>   
>>   static struct platform_driver qcom_msm8996_cbf_driver = {
>>   	.probe = qcom_msm8996_cbf_probe,
>> +	.remove = qcom_msm8996_cbf_remove,
>>   	.driver = {
>>   		.name = "qcom-msm8996-cbf",
>>   		.of_match_table = qcom_msm8996_cbf_match_table,
>> +		.sync_state = icc_sync_state,
>>   	},
>>   };
>>   

-- 
With best wishes
Dmitry