Re: [PATCH v4 2/3] clk: qcom: add IPQ9574 interconnect clocks support

[Krzysztof Kozlowski]

On 27/03/2024 09:18, 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>
> ---
> v4: Use clk_hw instead of indices
>     Do icc register in qcom_cc_probe() call stream
>     Add icc clock info to qcom_cc_desc structure
> v3: Use indexed identifiers here to avoid confusion
>     Fix error messages and move to common.c
> v2: Move DTS to separate patch
>     Update commit log
>     Auto select CONFIG_INTERCONNECT & CONFIG_INTERCONNECT_CLK to fix build error
> ---
>  drivers/clk/qcom/Kconfig       |  2 ++
>  drivers/clk/qcom/common.c      | 34 ++++++++++++++++++++-
>  drivers/clk/qcom/common.h      |  4 ++-
>  drivers/clk/qcom/gcc-ipq9574.c | 54 ++++++++++++++++++++++++++++++++++
>  4 files changed, 92 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 8ab08e7b5b6c..af73a0b396eb 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -243,6 +243,8 @@ config IPQ_GCC_8074
>  
>  config IPQ_GCC_9574
>  	tristate "IPQ9574 Global Clock Controller"
> +	select INTERCONNECT
> +	select INTERCONNECT_CLK
>  	help
>  	  Support for global clock controller on ipq9574 devices.
>  	  Say Y if you want to use peripheral devices such as UART, SPI,
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 75f09e6e057e..523d30d0ccbc 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -8,6 +8,8 @@
>  #include <linux/regmap.h>
>  #include <linux/platform_device.h>
>  #include <linux/clk-provider.h>
> +#include <linux/interconnect-clk.h>
> +#include <linux/interconnect-provider.h>
>  #include <linux/reset-controller.h>
>  #include <linux/of.h>
>  
> @@ -234,6 +236,36 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
>  	return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
>  }
>  
> +static int qcom_cc_icc_register(struct device *dev,
> +				const struct qcom_cc_desc *desc)
> +{
> +#if IS_ENABLED(CONFIG_INTERCONNECT_CLK)
> +	struct icc_provider *provider;
> +	struct icc_clk_data *icd;
> +	int i;
> +
> +	if (!desc->icc_hws)
> +		return 0;
> +
> +	icd = devm_kcalloc(dev, desc->num_icc_hws, sizeof(*icd), GFP_KERNEL);
> +	if (!icd)
> +		return dev_err_probe(dev, -ENOMEM, "malloc for icc clock data failed\n");

This is still not correct code.

No driver code prints errors on allocation failure.

Best regards,
Krzysztof