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

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 12.01.2023 16:29, Dmitry Baryshkov wrote:
> 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.
Thanks, that makes sense, I keep confusing it!

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