On 25.03.2024 11:20 AM, Varadarajan Narayanan wrote:
> Unlike MSM platforms that manage NoC related clocks and scaling
> from RPM, IPQ SoCs dont involve RPM in managing NoC related
> clocks and there is no NoC scaling.
>
> However, there is a requirement to enable some NoC interface
> clocks for accessing the peripheral controllers present on
> these NoCs. Though exposing these as normal clocks would work,
> having a minimalistic interconnect driver to handle these clocks
> would make it consistent with other Qualcomm platforms resulting
> in common code paths. This is similar to msm8996-cbf's usage of
> icc-clk framework.
>
> Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx>
> ---
[...]
>
> +
> +static struct icc_clk_data *icc_ipq9574;
> +
What does this help achieve?
[...]
> +static int noc_clks[] = {
We could probably use indexed identifiers here to avoid confusion:
[ICC_BINDING_NAME] = CLK_BINDING_NAME
> static int gcc_ipq9574_probe(struct platform_device *pdev)
> {
> - return qcom_cc_probe(pdev, &gcc_ipq9574_desc);
> + int ret = qcom_cc_probe(pdev, &gcc_ipq9574_desc);
> + struct icc_provider *provider;
> + struct icc_clk_data *icd;
> + int i;
> +
> + if (ret)
I'd personally prefer if you left ret uninitialized and assigned it
above the if-statement.
> + return dev_err_probe(&pdev->dev, ret, "%s failed\n", __func__);
Please avoid the use of __func__ throughout your change and write
a more useful error message.
> +
> + icd = devm_kmalloc(&pdev->dev, ARRAY_SIZE(noc_clks) * sizeof(*icd),
> + GFP_KERNEL);
devm_kcalloc
> +
> + if (IS_ERR_OR_NULL(icd))
> + return dev_err_probe(&pdev->dev, PTR_ERR(icd),
> + "%s malloc failed\n", __func__);
ditto
> +
> + icc_ipq9574 = icd;
> +
> + for (i = 0; i < ARRAY_SIZE(noc_clks); i++, icd++) {
> + icd->clk = gcc_ipq9574_clks[noc_clks[i]]->hw.clk;
> + if (IS_ERR_OR_NULL(icd->clk)) {
> + dev_err(&pdev->dev, "%s: %d clock not found\n",
> + __func__, noc_clks[i]);
> + return -ENOENT;
return dev_err_probe
> + }
> + icd->name = clk_hw_get_name(&gcc_ipq9574_clks[noc_clks[i]]->hw);
> + }
> +
> + provider = icc_clk_register(&pdev->dev, IPQ_APPS_ID,
> + ARRAY_SIZE(noc_clks), icc_ipq9574);
> + if (IS_ERR_OR_NULL(provider))
> + return dev_err_probe(&pdev->dev, PTR_ERR(provider),
> + "%s: icc_clk_register failed\n", __func__);
ditto
On a second thought, since I'm assuming you're going to expand this to other
IPQ SoCs, it might be useful to factor this out into drivers/clk/qcom/common.c
Konrad
