Message-ID: <fwor4n7m2ghviupileec663hf4tbzytjxkfuccorfcqyq3726p@awh3tnugxxpg>
Date: Wed, 22 Jan 2025 14:28:27 +0300
From: Dmitry Rokosov <ddrokosov@...utedevices.com>
To: Jerome Brunet <jbrunet@...libre.com>
CC: Michael Turquette <mturquette@...libre.com>, Stephen Boyd
	<sboyd@...nel.org>, Neil Armstrong <neil.armstrong@...aro.org>, Kevin Hilman
	<khilman@...libre.com>, Martin Blumenstingl
	<martin.blumenstingl@...glemail.com>, <linux-clk@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-amlogic@...ts.infradead.org>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 2/4] clk: amlogic: get regmap with clk_regmap_init

On Mon, Jan 20, 2025 at 06:15:31PM +0100, Jerome Brunet wrote:
> Add clk_regmap_init() and use it with all clock types which derive from
> clk_regmap. This helps initialise clk_regmap clocks without requiring
> tables to keep track of the clock using this type.
> 
> The way it is done couples clk_regmap with the controllers, which is not
> ideal. This is a temporary solution to get rid of the tables. The situation
> will eventually be improved.
> 
> Signed-off-by: Jerome Brunet <jbrunet@...libre.com>

Reviewed-by: Dmitry Rokosov <ddrokosov@...utedevices.com>

> ---
>  drivers/clk/meson/Kconfig          |  1 +
>  drivers/clk/meson/clk-cpu-dyndiv.c |  1 +
>  drivers/clk/meson/clk-dualdiv.c    |  2 ++
>  drivers/clk/meson/clk-mpll.c       |  6 +++++
>  drivers/clk/meson/clk-phase.c      | 11 +++++++++
>  drivers/clk/meson/clk-pll.c        |  7 ++++++
>  drivers/clk/meson/clk-regmap.c     | 49 ++++++++++++++++++++++++++++++++++++++
>  drivers/clk/meson/clk-regmap.h     |  4 ++++
>  drivers/clk/meson/sclk-div.c       |  5 ++++
>  drivers/clk/meson/vclk.c           |  2 ++
>  drivers/clk/meson/vid-pll-div.c    |  1 +
>  11 files changed, 89 insertions(+)
> 
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index be2e3a5f83363b07cdcec2601acf15780ff24892..d6e2ddb4eac85ddad44b99a8efa9d33426edbab3 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -5,6 +5,7 @@ menu "Clock support for Amlogic platforms"
>  config COMMON_CLK_MESON_REGMAP
>  	tristate
>  	select REGMAP
> +	select MFD_SYSCON
>  
>  config COMMON_CLK_MESON_DUALDIV
>  	tristate
> diff --git a/drivers/clk/meson/clk-cpu-dyndiv.c b/drivers/clk/meson/clk-cpu-dyndiv.c
> index 6c1f58826e24a2bf7706f0fc98cdcdb44fd60b64..0498fd3af7de7b9a4e72930ac4de11368d686393 100644
> --- a/drivers/clk/meson/clk-cpu-dyndiv.c
> +++ b/drivers/clk/meson/clk-cpu-dyndiv.c
> @@ -61,6 +61,7 @@ static int meson_clk_cpu_dyndiv_set_rate(struct clk_hw *hw, unsigned long rate,
>  };
>  
>  const struct clk_ops meson_clk_cpu_dyndiv_ops = {
> +	.init = clk_regmap_init,
>  	.recalc_rate = meson_clk_cpu_dyndiv_recalc_rate,
>  	.determine_rate = meson_clk_cpu_dyndiv_determine_rate,
>  	.set_rate = meson_clk_cpu_dyndiv_set_rate,
> diff --git a/drivers/clk/meson/clk-dualdiv.c b/drivers/clk/meson/clk-dualdiv.c
> index 913bf25d3771bdffa8db28e9b08dc566d1aac1ff..7c668df1a29f6eb20bde3f23d8701443602d774c 100644
> --- a/drivers/clk/meson/clk-dualdiv.c
> +++ b/drivers/clk/meson/clk-dualdiv.c
> @@ -126,6 +126,7 @@ static int meson_clk_dualdiv_set_rate(struct clk_hw *hw, unsigned long rate,
>  }
>  
>  const struct clk_ops meson_clk_dualdiv_ops = {
> +	.init		= clk_regmap_init,
>  	.recalc_rate	= meson_clk_dualdiv_recalc_rate,
>  	.determine_rate	= meson_clk_dualdiv_determine_rate,
>  	.set_rate	= meson_clk_dualdiv_set_rate,
> @@ -133,6 +134,7 @@ const struct clk_ops meson_clk_dualdiv_ops = {
>  EXPORT_SYMBOL_NS_GPL(meson_clk_dualdiv_ops, CLK_MESON);
>  
>  const struct clk_ops meson_clk_dualdiv_ro_ops = {
> +	.init		= clk_regmap_init,
>  	.recalc_rate	= meson_clk_dualdiv_recalc_rate,
>  };
>  EXPORT_SYMBOL_NS_GPL(meson_clk_dualdiv_ro_ops, CLK_MESON);
> diff --git a/drivers/clk/meson/clk-mpll.c b/drivers/clk/meson/clk-mpll.c
> index aa9abd06ae653585f68043bb1ddc2b0b96feeb6d..166fbcb053cc573ec92489c2bb03f81caffdf64b 100644
> --- a/drivers/clk/meson/clk-mpll.c
> +++ b/drivers/clk/meson/clk-mpll.c
> @@ -128,6 +128,11 @@ static int mpll_init(struct clk_hw *hw)
>  {
>  	struct clk_regmap *clk = to_clk_regmap(hw);
>  	struct meson_clk_mpll_data *mpll = meson_clk_mpll_data(clk);
> +	int ret;
> +
> +	ret = clk_regmap_init(hw);
> +	if (ret)
> +		return ret;
>  
>  	if (mpll->init_count)
>  		regmap_multi_reg_write(clk->map, mpll->init_regs,
> @@ -151,6 +156,7 @@ static int mpll_init(struct clk_hw *hw)
>  }
>  
>  const struct clk_ops meson_clk_mpll_ro_ops = {
> +	.init		= clk_regmap_init,
>  	.recalc_rate	= mpll_recalc_rate,
>  	.determine_rate	= mpll_determine_rate,
>  };
> diff --git a/drivers/clk/meson/clk-phase.c b/drivers/clk/meson/clk-phase.c
> index c1526fbfb6c4ccdd4034df465f70a56e0f9214ac..d05065e56483a218e0eaf402807c8734271d0cf5 100644
> --- a/drivers/clk/meson/clk-phase.c
> +++ b/drivers/clk/meson/clk-phase.c
> @@ -58,6 +58,7 @@ static int meson_clk_phase_set_phase(struct clk_hw *hw, int degrees)
>  }
>  
>  const struct clk_ops meson_clk_phase_ops = {
> +	.init		= clk_regmap_init,
>  	.get_phase	= meson_clk_phase_get_phase,
>  	.set_phase	= meson_clk_phase_set_phase,
>  };
> @@ -83,6 +84,11 @@ static int meson_clk_triphase_sync(struct clk_hw *hw)
>  	struct clk_regmap *clk = to_clk_regmap(hw);
>  	struct meson_clk_triphase_data *tph = meson_clk_triphase_data(clk);
>  	unsigned int val;
> +	int ret;
> +
> +	ret = clk_regmap_init(hw);
> +	if (ret)
> +		return ret;
>  
>  	/* Get phase 0 and sync it to phase 1 and 2 */
>  	val = meson_parm_read(clk->map, &tph->ph0);
> @@ -142,6 +148,11 @@ static int meson_sclk_ws_inv_sync(struct clk_hw *hw)
>  	struct clk_regmap *clk = to_clk_regmap(hw);
>  	struct meson_sclk_ws_inv_data *tph = meson_sclk_ws_inv_data(clk);
>  	unsigned int val;
> +	int ret;
> +
> +	ret = clk_regmap_init(hw);
> +	if (ret)
> +		return ret;
>  
>  	/* Get phase and sync the inverted value to ws */
>  	val = meson_parm_read(clk->map, &tph->ph);
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index 89f0f04a16abad99748e1ff3784e7ed91abd0f88..eaaf021d2f6911a99a722c8b9d8b4e1f1f77d0e6 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -311,6 +311,11 @@ static int meson_clk_pll_init(struct clk_hw *hw)
>  {
>  	struct clk_regmap *clk = to_clk_regmap(hw);
>  	struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
> +	int ret;
> +
> +	ret = clk_regmap_init(hw);
> +	if (ret)
> +		return ret;
>  
>  	/*
>  	 * Keep the clock running, which was already initialized and enabled
> @@ -468,6 +473,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>   * the other ops except set_rate since the rate is fixed.
>   */
>  const struct clk_ops meson_clk_pcie_pll_ops = {
> +	.init		= clk_regmap_init,
>  	.recalc_rate	= meson_clk_pll_recalc_rate,
>  	.determine_rate	= meson_clk_pll_determine_rate,
>  	.is_enabled	= meson_clk_pll_is_enabled,
> @@ -488,6 +494,7 @@ const struct clk_ops meson_clk_pll_ops = {
>  EXPORT_SYMBOL_NS_GPL(meson_clk_pll_ops, CLK_MESON);
>  
>  const struct clk_ops meson_clk_pll_ro_ops = {
> +	.init		= clk_regmap_init,
>  	.recalc_rate	= meson_clk_pll_recalc_rate,
>  	.is_enabled	= meson_clk_pll_is_enabled,
>  };
> diff --git a/drivers/clk/meson/clk-regmap.c b/drivers/clk/meson/clk-regmap.c
> index 07f7e441b9161c47fdb38ec61ed162a2f9d4fe41..d6bce32b087b174482ca896e4a0f3c57b58f155c 100644
> --- a/drivers/clk/meson/clk-regmap.c
> +++ b/drivers/clk/meson/clk-regmap.c
> @@ -4,9 +4,52 @@
>   * Author: Jerome Brunet <jbrunet@...libre.com>
>   */
>  
> +#include <linux/device.h>
>  #include <linux/module.h>
> +#include <linux/mfd/syscon.h>
>  #include "clk-regmap.h"
>  
> +int clk_regmap_init(struct clk_hw *hw)
> +{
> +	struct clk_regmap *clk = to_clk_regmap(hw);
> +	struct device_node *np, *parent_np;
> +	struct device *dev;
> +
> +	/* Allow regmap to be preset as it was historically done */
> +	if (clk->map)
> +		return 0;
> +
> +	/*
> +	 * FIXME: what follows couples the controller implementation
> +	 * and clk_regmap clock type. This situation is not desirable
> +	 * but temporary, until the controller is able to register
> +	 * a hook to initialize a clock type
> +	 */
> +
> +	/* Check the usual dev enabled controller with an basic IO regmap */
> +	dev = clk_hw_get_dev(hw);
> +	if (dev) {
> +		clk->map = dev_get_regmap(dev, NULL);
> +		if (clk->map)
> +			return 0;
> +	}
> +
> +	/* Move on to early and syscon based controllers */
> +	np = clk_hw_get_of_node(hw);
> +	if (np) {
> +		parent_np = of_get_parent(np);
> +		clk->map = syscon_node_to_regmap(parent_np);
> +		of_node_put(parent_np);
> +
> +		if (!IS_ERR_OR_NULL(clk->map))
> +			return 0;
> +	}
> +
> +	/* Bail out if regmap can't be found */
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_NS_GPL(clk_regmap_init, CLK_MESON);
> +
>  static int clk_regmap_gate_endisable(struct clk_hw *hw, int enable)
>  {
>  	struct clk_regmap *clk = to_clk_regmap(hw);
> @@ -45,6 +88,7 @@ static int clk_regmap_gate_is_enabled(struct clk_hw *hw)
>  }
>  
>  const struct clk_ops clk_regmap_gate_ops = {
> +	.init		= clk_regmap_init,
>  	.enable = clk_regmap_gate_enable,
>  	.disable = clk_regmap_gate_disable,
>  	.is_enabled = clk_regmap_gate_is_enabled,
> @@ -52,6 +96,7 @@ const struct clk_ops clk_regmap_gate_ops = {
>  EXPORT_SYMBOL_NS_GPL(clk_regmap_gate_ops, CLK_MESON);
>  
>  const struct clk_ops clk_regmap_gate_ro_ops = {
> +	.init		= clk_regmap_init,
>  	.is_enabled = clk_regmap_gate_is_enabled,
>  };
>  EXPORT_SYMBOL_NS_GPL(clk_regmap_gate_ro_ops, CLK_MESON);
> @@ -121,6 +166,7 @@ static int clk_regmap_div_set_rate(struct clk_hw *hw, unsigned long rate,
>  /* Would prefer clk_regmap_div_ro_ops but clashes with qcom */
>  
>  const struct clk_ops clk_regmap_divider_ops = {
> +	.init = clk_regmap_init,
>  	.recalc_rate = clk_regmap_div_recalc_rate,
>  	.determine_rate = clk_regmap_div_determine_rate,
>  	.set_rate = clk_regmap_div_set_rate,
> @@ -128,6 +174,7 @@ const struct clk_ops clk_regmap_divider_ops = {
>  EXPORT_SYMBOL_NS_GPL(clk_regmap_divider_ops, CLK_MESON);
>  
>  const struct clk_ops clk_regmap_divider_ro_ops = {
> +	.init = clk_regmap_init,
>  	.recalc_rate = clk_regmap_div_recalc_rate,
>  	.determine_rate = clk_regmap_div_determine_rate,
>  };
> @@ -170,6 +217,7 @@ static int clk_regmap_mux_determine_rate(struct clk_hw *hw,
>  }
>  
>  const struct clk_ops clk_regmap_mux_ops = {
> +	.init		= clk_regmap_init,
>  	.get_parent = clk_regmap_mux_get_parent,
>  	.set_parent = clk_regmap_mux_set_parent,
>  	.determine_rate = clk_regmap_mux_determine_rate,
> @@ -177,6 +225,7 @@ const struct clk_ops clk_regmap_mux_ops = {
>  EXPORT_SYMBOL_NS_GPL(clk_regmap_mux_ops, CLK_MESON);
>  
>  const struct clk_ops clk_regmap_mux_ro_ops = {
> +	.init		= clk_regmap_init,
>  	.get_parent = clk_regmap_mux_get_parent,
>  };
>  EXPORT_SYMBOL_NS_GPL(clk_regmap_mux_ro_ops, CLK_MESON);
> diff --git a/drivers/clk/meson/clk-regmap.h b/drivers/clk/meson/clk-regmap.h
> index e365312da54ece08098293b1a831c803415db814..f8cac2df5755b0f894570305604485f54c17ea49 100644
> --- a/drivers/clk/meson/clk-regmap.h
> +++ b/drivers/clk/meson/clk-regmap.h
> @@ -7,6 +7,7 @@
>  #ifndef __CLK_REGMAP_H
>  #define __CLK_REGMAP_H
>  
> +#include <linux/device.h>
>  #include <linux/clk-provider.h>
>  #include <linux/regmap.h>
>  
> @@ -31,6 +32,9 @@ static inline struct clk_regmap *to_clk_regmap(struct clk_hw *hw)
>  	return container_of(hw, struct clk_regmap, hw);
>  }
>  
> +/* clk_regmap init op to get and cache regmap from the controllers */
> +int clk_regmap_init(struct clk_hw *hw);
> +
>  /**
>   * struct clk_regmap_gate_data - regmap backed gate specific data
>   *
> diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
> index ae03b048182f3bbaf4a7e86b605fb4fb37b9cfd5..13536df1760d135cc2d62062e2699ed6842e1e5f 100644
> --- a/drivers/clk/meson/sclk-div.c
> +++ b/drivers/clk/meson/sclk-div.c
> @@ -222,6 +222,11 @@ static int sclk_div_init(struct clk_hw *hw)
>  	struct clk_regmap *clk = to_clk_regmap(hw);
>  	struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk);
>  	unsigned int val;
> +	int ret;
> +
> +	ret = clk_regmap_init(hw);
> +	if (ret)
> +		return ret;
>  
>  	val = meson_parm_read(clk->map, &sclk->div);
>  
> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
> index 36f637d2d01b5d86bf5d85196643ad9d66c6f19b..bcdec8a444f60838c84eb27c4cdf2b8f8574ce71 100644
> --- a/drivers/clk/meson/vclk.c
> +++ b/drivers/clk/meson/vclk.c
> @@ -45,6 +45,7 @@ static int meson_vclk_gate_is_enabled(struct clk_hw *hw)
>  }
>  
>  const struct clk_ops meson_vclk_gate_ops = {
> +	.init = clk_regmap_init,
>  	.enable = meson_vclk_gate_enable,
>  	.disable = meson_vclk_gate_disable,
>  	.is_enabled = meson_vclk_gate_is_enabled,
> @@ -127,6 +128,7 @@ static int meson_vclk_div_is_enabled(struct clk_hw *hw)
>  }
>  
>  const struct clk_ops meson_vclk_div_ops = {
> +	.init = clk_regmap_init,
>  	.recalc_rate = meson_vclk_div_recalc_rate,
>  	.determine_rate = meson_vclk_div_determine_rate,
>  	.set_rate = meson_vclk_div_set_rate,
> diff --git a/drivers/clk/meson/vid-pll-div.c b/drivers/clk/meson/vid-pll-div.c
> index 486cf68fc97a0205ee5139056b1eabc6e743ff35..c28d8b150400308df050e500c2aedc14157c1bcf 100644
> --- a/drivers/clk/meson/vid-pll-div.c
> +++ b/drivers/clk/meson/vid-pll-div.c
> @@ -90,6 +90,7 @@ static unsigned long meson_vid_pll_div_recalc_rate(struct clk_hw *hw,
>  }
>  
>  const struct clk_ops meson_vid_pll_div_ro_ops = {
> +	.init		= clk_regmap_init,
>  	.recalc_rate	= meson_vid_pll_div_recalc_rate,
>  };
>  EXPORT_SYMBOL_NS_GPL(meson_vid_pll_div_ro_ops, CLK_MESON);
> 
> -- 
> 2.45.2
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

-- 
Thank you,
Dmitry

Powered by blists - more mailing lists