Re: [PATCH v2 2/6] clk/qcom: add initial clock driver for ipq9574

[Caleb Connolly <caleb.connolly@linaro.org>]

On 1/30/25 05:37, Varadarajan Narayanan wrote:
> Add initial set of clocks and resets for enabling U-Boot on ipq9574
> based RDP platforms.

What is RDP?
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v2: Combined driver file and makefile/kconfig changes into one patch
> ---
>   drivers/clk/qcom/Kconfig         |   8 +++
>   drivers/clk/qcom/Makefile        |   1 +
>   drivers/clk/qcom/clock-ipq9574.c | 100 +++++++++++++++++++++++++++++++
>   drivers/clk/qcom/clock-qcom.h    |   1 +
>   4 files changed, 110 insertions(+)
>   create mode 100644 drivers/clk/qcom/clock-ipq9574.c
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index cb867acc48..3ea01f3c96 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -31,6 +31,14 @@ config CLK_QCOM_IPQ4019
>   	  on the Snapdragon IPQ4019 SoC. This driver supports the clocks
>   	  and resets exposed by the GCC hardware block.
>   
> +config CLK_QCOM_IPQ9574
> +	bool "Qualcomm IPQ9574 GCC"
> +	select CLK_QCOM
> +	help
> +	  Say Y here to enable support for the Global Clock Controller
> +	  on the Snapdragon IPQ9574 SoC. This driver supports the clocks
> +	  and resets exposed by the GCC hardware block.
> +
>   config CLK_QCOM_QCM2290
>   	bool "Qualcomm QCM2290 GCC"
>   	select CLK_QCOM
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 1bc0f15005..e13fc8c107 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_QCOM_SDM845) += clock-sdm845.o
>   obj-$(CONFIG_CLK_QCOM_APQ8016) += clock-apq8016.o
>   obj-$(CONFIG_CLK_QCOM_APQ8096) += clock-apq8096.o
>   obj-$(CONFIG_CLK_QCOM_IPQ4019) += clock-ipq4019.o
> +obj-$(CONFIG_CLK_QCOM_IPQ9574) += clock-ipq9574.o
>   obj-$(CONFIG_CLK_QCOM_QCM2290) += clock-qcm2290.o
>   obj-$(CONFIG_CLK_QCOM_QCS404) += clock-qcs404.o
>   obj-$(CONFIG_CLK_QCOM_SA8775P) += clock-sa8775p.o
> diff --git a/drivers/clk/qcom/clock-ipq9574.c b/drivers/clk/qcom/clock-ipq9574.c
> new file mode 100644
> index 0000000000..06f2d2dd45
> --- /dev/null
> +++ b/drivers/clk/qcom/clock-ipq9574.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Clock drivers for Qualcomm ipq9574
> + *
> + * (C) Copyright 2024 Linaro Ltd.
> + */
> +
> +#include <linux/types.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <linux/delay.h>
> +#include <asm/io.h>
> +#include <linux/bug.h>
> +#include <linux/bitops.h>
> +#include <dt-bindings/clock/qcom,ipq9574-gcc.h>
> +#include <dt-bindings/reset/qcom,ipq9574-gcc.h>
> +
> +#include "clock-qcom.h"
> +
> +#define GCC_BLSP1_AHB_CBCR			0x1004
> +#define GCC_BLSP1_UART3_APPS_CMD_RCGR		0x402C
> +#define GCC_BLSP1_UART3_APPS_CBCR		0x4054
> +
> +#define GCC_SDCC1_APPS_CBCR			0x3302C
> +#define GCC_SDCC1_AHB_CBCR			0x33034
> +#define GCC_SDCC1_APPS_CMD_RCGR			0x33004
> +#define GCC_SDCC1_ICE_CORE_CBCR			0x33030
> +
> +static ulong ipq9574_set_rate(struct clk *clk, ulong rate)
> +{
> +	struct msm_clk_priv *priv = dev_get_priv(clk->dev);
> +
> +	switch (clk->id) {
> +	case GCC_BLSP1_UART3_APPS_CLK:
> +		clk_rcg_set_rate_mnd(priv->base, GCC_BLSP1_UART3_APPS_CMD_RCGR,
> +				     0, 144, 15625, CFG_CLK_SRC_GPLL0, 16);
> +		return rate;
> +	case GCC_SDCC1_APPS_CLK:
> +		clk_rcg_set_rate_mnd(priv->base, GCC_SDCC1_APPS_CMD_RCGR,
> +				     11, 0, 0, CFG_CLK_SRC_GPLL2, 16);
> +		return rate;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ipq9574_enable(struct clk *clk)
> +{
> +	struct msm_clk_priv *priv = dev_get_priv(clk->dev);
> +
> +	//printk("--> %s: %d\n", __func__, clk->id);

Remove or demote to debug, with gate_clk you can include the clock name 
as well.
> +	switch (clk->id) {
> +	case GCC_BLSP1_UART3_APPS_CLK:
> +		clk_enable_cbc(priv->base + GCC_BLSP1_UART3_APPS_CBCR);
> +		break;
> +	case GCC_BLSP1_AHB_CLK:
> +		clk_enable_cbc(priv->base + GCC_BLSP1_AHB_CBCR);
> +		break;
> +	case GCC_SDCC1_AHB_CLK:
> +		clk_enable_cbc(priv->base + GCC_SDCC1_AHB_CBCR);
> +		break;
> +	case GCC_SDCC1_APPS_CLK:
> +		clk_enable_cbc(priv->base + GCC_SDCC1_APPS_CBCR);
> +		break;
> +	case GCC_SDCC1_ICE_CORE_CLK:
> +		clk_enable_cbc(priv->base + GCC_SDCC1_ICE_CORE_CBCR);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

Please use the gate_clk API for these, refer to clock-sdm845.c for 
reference.

With that:

Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>

Kind regards,
> +
> +	return 0;
> +}
> +
> +static const struct qcom_reset_map ipq9574_gcc_resets[] = {
> +	[GCC_SDCC_BCR] = { 0x33000 },
> +};
> +
> +static struct msm_clk_data ipq9574_gcc_data = {
> +	.resets = ipq9574_gcc_resets,
> +	.num_resets = ARRAY_SIZE(ipq9574_gcc_resets),
> +	.enable = ipq9574_enable,
> +	.set_rate = ipq9574_set_rate,
> +};
> +
> +static const struct udevice_id gcc_ipq9574_of_match[] = {
> +	{
> +		.compatible = "qcom,ipq9574-gcc",
> +		.data = (ulong)&ipq9574_gcc_data,
> +	},
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(gcc_ipq9574) = {
> +	.name		= "gcc_ipq9574",
> +	.id		= UCLASS_NOP,
> +	.of_match	= gcc_ipq9574_of_match,
> +	.bind		= qcom_cc_bind,
> +	.flags		= DM_FLAG_PRE_RELOC | DM_FLAG_DEFAULT_PD_CTRL_OFF,
> +};
> diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h
> index ff336dea39..e038dc421e 100644
> --- a/drivers/clk/qcom/clock-qcom.h
> +++ b/drivers/clk/qcom/clock-qcom.h
> @@ -11,6 +11,7 @@
>   #define CFG_CLK_SRC_CXO   (0 << 8)
>   #define CFG_CLK_SRC_GPLL0 (1 << 8)
>   #define CFG_CLK_SRC_GPLL0_AUX2 (2 << 8)
> +#define CFG_CLK_SRC_GPLL2 (2 << 8)
>   #define CFG_CLK_SRC_GPLL9 (2 << 8)
>   #define CFG_CLK_SRC_GPLL0_ODD (3 << 8)
>   #define CFG_CLK_SRC_GPLL6 (4 << 8)

-- 
Caleb (they/them)