Re: [PATCH 1/3] clk: rockchip: Add new clock-type for the ddrclk

[Sascha Hauer <s.hauer@pengutronix.de>]

Hi Alexander,

On Thu, Apr 10, 2025 at 03:07:43PM +0300, Alexander Shiyan wrote:
> Changing the DDR clock frequency requires special care, this patch
> adds the DDR clock driver code from the Linux kernel repository.
> While we're here, let's also update drivers/clk/rockchip/Makefile
> to make it more readable.

This patch currently breaks build on ARMv7 Rockchip SoCs. It fails with
undefined reference to __arm_smccc_smc.

Looking at upstream Linux it seems that this driver is only used on
RK3399, the SoC we just removed support for.

Where do we go from here? I could fix this patch, but as it's unused I
could equally well drop it without loosing anything.

Sascha

> 
> Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
> ---
>  drivers/clk/rockchip/Makefile       |  29 ++++--
>  drivers/clk/rockchip/clk-ddr.c      | 131 ++++++++++++++++++++++++++++
>  drivers/clk/rockchip/clk-pll.c      |   6 +-
>  drivers/clk/rockchip/clk.c          |   7 ++
>  drivers/clk/rockchip/clk.h          |   1 +
>  include/soc/rockchip/rockchip_sip.h |  23 +++++
>  6 files changed, 188 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/clk/rockchip/clk-ddr.c
>  create mode 100644 include/soc/rockchip/rockchip_sip.h
> 
> diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile
> index f01014da0c..8d752d11b9 100644
> --- a/drivers/clk/rockchip/Makefile
> +++ b/drivers/clk/rockchip/Makefile
> @@ -1,8 +1,21 @@
> -# SPDX-License-Identifier: GPL-2.0-only
> -obj-y += clk-cpu.o clk-pll.o clk.o clk-muxgrf.o clk-mmc-phase.o clk-inverter.o
> -obj-$(CONFIG_RESET_CONTROLLER) += softrst.o
> -obj-$(CONFIG_ARCH_RK3188) += clk-rk3188.o
> -obj-$(CONFIG_ARCH_RK3288) += clk-rk3288.o
> -obj-$(CONFIG_ARCH_RK3399) += clk-rk3399.o
> -obj-$(CONFIG_ARCH_RK3568) += clk-rk3568.o
> -obj-$(CONFIG_ARCH_RK3588) += clk-rk3588.o rst-rk3588.o
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Rockchip Clock specific Makefile
> +#
> +
> +obj-y += clk-rockchip.o
> +
> +clk-rockchip-y += clk.o
> +clk-rockchip-y += clk-pll.o
> +clk-rockchip-y += clk-cpu.o
> +clk-rockchip-y += clk-inverter.o
> +clk-rockchip-y += clk-mmc-phase.o
> +clk-rockchip-y += clk-muxgrf.o
> +clk-rockchip-y += clk-ddr.o
> +
> +clk-rockchip-$(CONFIG_RESET_CONTROLLER) += softrst.o
> +
> +obj-$(CONFIG_ARCH_RK3188)	+= clk-rk3188.o
> +obj-$(CONFIG_ARCH_RK3288)	+= clk-rk3288.o
> +obj-$(CONFIG_ARCH_RK3568)	+= clk-rk3568.o
> +obj-$(CONFIG_ARCH_RK3588)	+= clk-rk3588.o rst-rk3588.o
> diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
> new file mode 100644
> index 0000000000..7b10f1533b
> --- /dev/null
> +++ b/drivers/clk/rockchip/clk-ddr.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
> + * Author: Lin Huang <hl@rock-chips.com>
> + */
> +
> +#include <common.h>
> +#include <linux/arm-smccc.h>
> +#include <soc/rockchip/rockchip_sip.h>
> +
> +#include "clk.h"
> +
> +struct rockchip_ddrclk {
> +	struct clk_hw	hw;
> +	void __iomem	*reg_base;
> +	int		mux_offset;
> +	int		mux_shift;
> +	int		mux_width;
> +	int		div_shift;
> +	int		div_width;
> +	int		ddr_flag;
> +};
> +
> +#define to_rockchip_ddrclk_hw(hw) container_of(hw, struct rockchip_ddrclk, hw)
> +
> +static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
> +					unsigned long prate)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, drate, 0,
> +		      ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
> +		      0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
> +static unsigned long
> +rockchip_ddrclk_sip_recalc_rate(struct clk_hw *hw,
> +				unsigned long parent_rate)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
> +		      ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
> +		      0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
> +static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
> +					   unsigned long rate,
> +					   unsigned long *prate)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, rate, 0,
> +		      ROCKCHIP_SIP_CONFIG_DRAM_ROUND_RATE,
> +		      0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
> +static int rockchip_ddrclk_get_parent(struct clk_hw *hw)
> +{
> +	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> +	u32 val;
> +
> +	val = readl(ddrclk->reg_base +
> +			ddrclk->mux_offset) >> ddrclk->mux_shift;
> +	val &= GENMASK(ddrclk->mux_width - 1, 0);
> +
> +	return val;
> +}
> +
> +static const struct clk_ops rockchip_ddrclk_sip_ops = {
> +	.recalc_rate = rockchip_ddrclk_sip_recalc_rate,
> +	.set_rate = rockchip_ddrclk_sip_set_rate,
> +	.round_rate = rockchip_ddrclk_sip_round_rate,
> +	.get_parent = rockchip_ddrclk_get_parent,
> +};
> +
> +struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
> +					 const char *const *parent_names,
> +					 u8 num_parents, int mux_offset,
> +					 int mux_shift, int mux_width,
> +					 int div_shift, int div_width,
> +					 int ddr_flag, void __iomem *reg_base,
> +					 spinlock_t *lock)
> +{
> +	struct rockchip_ddrclk *ddrclk;
> +	struct clk_init_data init;
> +	struct clk *clk;
> +
> +	ddrclk = kzalloc(sizeof(*ddrclk), GFP_KERNEL);
> +	if (!ddrclk)
> +		return ERR_PTR(-ENOMEM);
> +
> +	init.name = name;
> +	init.parent_names = parent_names;
> +	init.num_parents = num_parents;
> +
> +	init.flags = flags;
> +	init.flags |= CLK_SET_RATE_NO_REPARENT;
> +
> +	switch (ddr_flag) {
> +	case ROCKCHIP_DDRCLK_SIP:
> +		init.ops = &rockchip_ddrclk_sip_ops;
> +		break;
> +	default:
> +		pr_err("%s: unsupported ddrclk type %d\n", __func__, ddr_flag);
> +		kfree(ddrclk);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	ddrclk->reg_base = reg_base;
> +	ddrclk->hw.init = &init;
> +	ddrclk->mux_offset = mux_offset;
> +	ddrclk->mux_shift = mux_shift;
> +	ddrclk->mux_width = mux_width;
> +	ddrclk->div_shift = div_shift;
> +	ddrclk->div_width = div_width;
> +	ddrclk->ddr_flag = ddr_flag;
> +
> +	clk = clk_register(NULL, &ddrclk->hw);
> +	if (IS_ERR(clk))
> +		kfree(ddrclk);
> +
> +	return clk;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_clk_register_ddrclk);
> diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
> index b4152b03b1..2931c09ad5 100644
> --- a/drivers/clk/rockchip/clk-pll.c
> +++ b/drivers/clk/rockchip/clk-pll.c
> @@ -913,7 +913,10 @@ static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned
>  	}
>  	rate64 = rate64 >> cur.s;
>  
> -	return (unsigned long)rate64;
> +	if (pll->type == pll_rk3588_ddr)
> +		return (unsigned long)rate64 * 2;
> +	else
> +		return (unsigned long)rate64;
>  }
>  
>  static int rockchip_rk3588_pll_set_params(struct rockchip_clk_pll *pll,
> @@ -1169,6 +1172,7 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx,
>  		break;
>  	case pll_rk3588:
>  	case pll_rk3588_core:
> +	case pll_rk3588_ddr:
>  		if (!pll->rate_table)
>  			init.ops = &rockchip_rk3588_pll_clk_norate_ops;
>  		else
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index a154495efd..127b79e2c8 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -497,6 +497,13 @@ void rockchip_clk_register_branches(struct rockchip_clk_provider *ctx,
>  				list->gate_flags, flags, &ctx->lock);
>  			break;
>  		case branch_ddrclk:
> +			clk = rockchip_clk_register_ddrclk(
> +				list->name, list->flags,
> +				list->parent_names, list->num_parents,
> +				list->muxdiv_offset, list->mux_shift,
> +				list->mux_width, list->div_shift,
> +				list->div_width, list->div_flags,
> +				ctx->reg_base, &ctx->lock);
>  			break;
>  		}
>  
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index a451229326..6665f8ac90 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -267,6 +267,7 @@ enum rockchip_pll_type {
>  	pll_rk3399,
>  	pll_rk3588,
>  	pll_rk3588_core,
> +	pll_rk3588_ddr,
>  };
>  
>  #define RK3036_PLL_RATE(_rate, _refdiv, _fbdiv, _postdiv1,	\
> diff --git a/include/soc/rockchip/rockchip_sip.h b/include/soc/rockchip/rockchip_sip.h
> new file mode 100644
> index 0000000000..501ad1fedb
> --- /dev/null
> +++ b/include/soc/rockchip/rockchip_sip.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
> + * Author: Lin Huang <hl@rock-chips.com>
> + */
> +#ifndef __SOC_ROCKCHIP_SIP_H
> +#define __SOC_ROCKCHIP_SIP_H
> +
> +#define ROCKCHIP_SIP_SUSPEND_MODE		0x82000003
> +#define ROCKCHIP_SLEEP_PD_CONFIG		0xff
> +
> +#define ROCKCHIP_SIP_DRAM_FREQ			0x82000008
> +#define ROCKCHIP_SIP_CONFIG_DRAM_INIT		0x00
> +#define ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE	0x01
> +#define ROCKCHIP_SIP_CONFIG_DRAM_ROUND_RATE	0x02
> +#define ROCKCHIP_SIP_CONFIG_DRAM_SET_AT_SR	0x03
> +#define ROCKCHIP_SIP_CONFIG_DRAM_GET_BW		0x04
> +#define ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE	0x05
> +#define ROCKCHIP_SIP_CONFIG_DRAM_CLR_IRQ	0x06
> +#define ROCKCHIP_SIP_CONFIG_DRAM_SET_PARAM	0x07
> +#define ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD	0x08
> +
> +#endif
> -- 
> 2.39.1
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |