Re: [PATCH v2 1/7] clk: qcom: Add shared RPMH clock driver for SM8550/SM8650/SA8775P

On 8/12/26 03:11, Balaji Selvanathan wrote:
> sa8775p was being handled by the generic clock stub, which just
> no-ops every request. The RPMH_CXO_CLK (bi_tcxo_div2) it exposes is
> identical on SM8550, SM8650 and SA8775P, so rather than bury it in the
> sa8775p GCC driver, add a dedicated standalone RPMH clock driver shared
> by all three SoCs. It reports the fixed CXO rate and accepts no-op
> enable/set_rate requests.
> 
> Monaco-evk also uses the same RPMH clock driver as sa8775p (it's
> rpmhcc DTS node has compatible "qcom,sa8775p-rpmh-clk").
> 
> Drop the sm8550/sm8650/sa8775p rpmh-clk compatibles from the generic
> clock stub now that they are handled by the real driver.
> 
> Signed-off-by: Balaji Selvanathan <[email protected]>
> ---
>   drivers/clk/clk-stub.c        |  3 ---
>   drivers/clk/qcom/Kconfig      | 12 +++++++++
>   drivers/clk/qcom/Makefile     |  1 +
>   drivers/clk/qcom/clock-rpmh.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 74 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/clk-stub.c b/drivers/clk/clk-stub.c
> index 4a6c71016da..85e54841386 100644
> --- a/drivers/clk/clk-stub.c
> +++ b/drivers/clk/clk-stub.c
> @@ -51,7 +51,6 @@ static struct clk_ops stub_clk_ops = {
>   static const struct udevice_id stub_clk_ids[] = {
>   	{ .compatible = "qcom,qcs615-rpmh-clk" },
>   	{ .compatible = "qcom,rpmcc" },
> -	{ .compatible = "qcom,sa8775p-rpmh-clk" },
>   	{ .compatible = "qcom,sc7180-rpmh-clk" },
>   	{ .compatible = "qcom,sc7280-rpmh-clk" },
>   	{ .compatible = "qcom,sdm670-rpmh-clk" },
> @@ -59,8 +58,6 @@ static const struct udevice_id stub_clk_ids[] = {
>   	{ .compatible = "qcom,sm6350-rpmh-clk" },
>   	{ .compatible = "qcom,sm8150-rpmh-clk" },
>   	{ .compatible = "qcom,sm8250-rpmh-clk" },
> -	{ .compatible = "qcom,sm8550-rpmh-clk" },
> -	{ .compatible = "qcom,sm8650-rpmh-clk" },
>   	{ }
>   };
>   
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 9ad233c83ac..8596b93d4c4 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -87,9 +87,19 @@ config CLK_QCOM_QCS8300
>   	  on the Snapdragon QCS8300 SoC. This driver supports the clocks
>   	  and resets exposed by the GCC hardware block.
>   
> +config CLK_QCOM_RPMH
> +	bool "Qualcomm RPMh clock driver"
> +	depends on CLK
> +	help
> +	  Say Y here to enable support for the RPMh-managed clocks found on
> +	  SoCs such as SM8550, SM8650 and SA8775P. These clocks are owned by
> +	  the always-on subsystem; the driver only reports the fixed CXO rate
> +	  (bi_tcxo_div2) and accepts no-op enable/set_rate requests.
> +
>   config CLK_QCOM_SA8775P
>   	bool "Qualcomm SA8775 GCC"
>   	select CLK_QCOM
> +	select CLK_QCOM_RPMH
>   	help
>   	  Say Y here to enable support for the Global Clock Controller
>   	  on the Snapdragon SA8775 SoC. This driver supports the clocks
> @@ -153,6 +163,7 @@ config CLK_QCOM_SM8250
>   config CLK_QCOM_SM8550
>   	bool "Qualcomm SM8550 GCC"
>   	select CLK_QCOM
> +	select CLK_QCOM_RPMH
>   	help
>   	  Say Y here to enable support for the Global Clock Controller
>   	  on the Snapdragon SM8550 SoC. This driver supports the clocks
> @@ -161,6 +172,7 @@ config CLK_QCOM_SM8550
>   config CLK_QCOM_SM8650
>   	bool "Qualcomm SM8650 GCC"
>   	select CLK_QCOM
> +	select CLK_QCOM_RPMH
>   	help
>   	  Say Y here to enable support for the Global Clock Controller
>   	  on the Snapdragon SM8650 SoC. This driver supports the clocks
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index c0d95a6300e..f5630ae0f9c 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_CLK_QCOM_QCM2290) += clock-qcm2290.o
>   obj-$(CONFIG_CLK_QCOM_QCS404) += clock-qcs404.o
>   obj-$(CONFIG_CLK_QCOM_QCS8300) += clock-qcs8300.o
>   obj-$(CONFIG_CLK_QCOM_QCS615) += clock-qcs615.o
> +obj-$(CONFIG_CLK_QCOM_RPMH) += clock-rpmh.o
>   obj-$(CONFIG_CLK_QCOM_SA8775P) += clock-sa8775p.o
>   obj-$(CONFIG_CLK_QCOM_SC7280) += clock-sc7280.o
>   obj-$(CONFIG_CLK_QCOM_SM6115) += clock-sm6115.o
> diff --git a/drivers/clk/qcom/clock-rpmh.c b/drivers/clk/qcom/clock-rpmh.c
> new file mode 100644
> index 00000000000..69f2a751441
> --- /dev/null
> +++ b/drivers/clk/qcom/clock-rpmh.c
> @@ -0,0 +1,61 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * RPMh clock driver for Qualcomm SoCs
> + *
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <dt-bindings/clock/qcom,rpmh.h>
> +
> +/* On-board TCXO */
> +#define TCXO_RATE	38400000
> +
> +/* bi_tcxo_div2 divided after RPMh output */
> +#define TCXO_DIV2_RATE	(TCXO_RATE / 2)
> +
> +static ulong qcom_rpmh_clk_set_rate(struct clk *clk, ulong rate)
> +{
> +	return (clk->rate = rate);
> +}
> +
> +static ulong qcom_rpmh_clk_get_rate(struct clk *clk)
> +{
> +	ulong cxo_rate = dev_get_driver_data(clk->dev);
> +
> +	switch (clk->id) {
> +	case RPMH_CXO_CLK:
> +		return cxo_rate;
> +	default:
> +		return clk->rate;
> +	}
> +}
> +
> +static int qcom_rpmh_clk_nop(struct clk *clk)
> +{
> +	return 0;
> +}
> +
> +static struct clk_ops qcom_rpmh_clk_ops = {
> +	.set_rate = qcom_rpmh_clk_set_rate,
> +	.get_rate = qcom_rpmh_clk_get_rate,
> +	.enable = qcom_rpmh_clk_nop,
> +	.disable = qcom_rpmh_clk_nop,
> +};
> +
> +static const struct udevice_id qcom_rpmh_clk_ids[] = {
> +	{ .compatible = "qcom,sa8775p-rpmh-clk", .data = TCXO_DIV2_RATE },
> +	{ .compatible = "qcom,sm8550-rpmh-clk", .data = TCXO_DIV2_RATE },
> +	{ .compatible = "qcom,sm8650-rpmh-clk", .data = TCXO_DIV2_RATE },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(qcom_rpmh_clk) = {
> +	.name		= "qcom_rpmh_clk",
> +	.id		= UCLASS_CLK,
> +	.of_match	= qcom_rpmh_clk_ids,
> +	.ops		= &qcom_rpmh_clk_ops,
> +	.flags		= DM_FLAG_DEFAULT_PD_CTRL_OFF,
> +};
> 

This looks better !

Reviewed-by: Neil Armstrong <[email protected]>

Thanks,
Neil