Re: [PATCH v3 4/7] clk: ingenic: tcu: Use contextual data instead of global variable

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Oct 29, 2025 at 05:33:33PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Pass the driver-specific data via the syscore struct and use it in the
> syscore ops.

Some of these things in drivers/clk/ are also platform_device drivers
(though not this one) and use generic power management, e.g.,

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/qcom/q6sstop-qcs404.c?id=v6.17#n209

I have no idea if that's desirable or practical here, but using the
platform_device model instead of syscore could have advantages in
terms of modeling device dependencies and ordering.

> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v3:
> - adjust for API changes and update commit message
> 
>  drivers/clk/ingenic/tcu.c | 63 +++++++++++++++++++--------------------
>  1 file changed, 30 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c
> index bc6a51da2072..8c6337d8e831 100644
> --- a/drivers/clk/ingenic/tcu.c
> +++ b/drivers/clk/ingenic/tcu.c
> @@ -53,9 +53,9 @@ struct ingenic_tcu {
>  	struct clk *clk;
>  
>  	struct clk_hw_onecell_data *clocks;
> -};
>  
> -static struct ingenic_tcu *ingenic_tcu;
> +	struct syscore syscore;
> +};
>  
>  static inline struct ingenic_tcu_clk *to_tcu_clk(struct clk_hw *hw)
>  {
> @@ -332,6 +332,29 @@ static const struct of_device_id __maybe_unused ingenic_tcu_of_match[] __initcon
>  	{ /* sentinel */ }
>  };
>  
> +static int __maybe_unused tcu_pm_suspend(void *data)
> +{
> +	struct ingenic_tcu *tcu = data;
> +
> +	if (tcu->clk)
> +		clk_disable(tcu->clk);
> +
> +	return 0;
> +}
> +
> +static void __maybe_unused tcu_pm_resume(void *data)
> +{
> +	struct ingenic_tcu *tcu = data;
> +
> +	if (tcu->clk)
> +		clk_enable(tcu->clk);
> +}
> +
> +static const struct syscore_ops tcu_pm_ops __maybe_unused = {
> +	.suspend = tcu_pm_suspend,
> +	.resume = tcu_pm_resume,
> +};
> +
>  static int __init ingenic_tcu_probe(struct device_node *np)
>  {
>  	const struct of_device_id *id = of_match_node(ingenic_tcu_of_match, np);
> @@ -430,7 +453,11 @@ static int __init ingenic_tcu_probe(struct device_node *np)
>  		goto err_unregister_ost_clock;
>  	}
>  
> -	ingenic_tcu = tcu;
> +	if (IS_ENABLED(CONFIG_PM_SLEEP)) {
> +		tcu->syscore.ops = &tcu_pm_ops;
> +		tcu->syscore.data = tcu;
> +		register_syscore(&tcu->syscore);
> +	}
>  
>  	return 0;
>  
> @@ -455,42 +482,12 @@ static int __init ingenic_tcu_probe(struct device_node *np)
>  	return ret;
>  }
>  
> -static int __maybe_unused tcu_pm_suspend(void *data)
> -{
> -	struct ingenic_tcu *tcu = ingenic_tcu;
> -
> -	if (tcu->clk)
> -		clk_disable(tcu->clk);
> -
> -	return 0;
> -}
> -
> -static void __maybe_unused tcu_pm_resume(void *data)
> -{
> -	struct ingenic_tcu *tcu = ingenic_tcu;
> -
> -	if (tcu->clk)
> -		clk_enable(tcu->clk);
> -}
> -
> -static const struct syscore_ops __maybe_unused tcu_pm_ops = {
> -	.suspend = tcu_pm_suspend,
> -	.resume = tcu_pm_resume,
> -};
> -
> -static struct syscore __maybe_unused tcu_pm = {
> -	.ops = &tcu_pm_ops,
> -};
> -
>  static void __init ingenic_tcu_init(struct device_node *np)
>  {
>  	int ret = ingenic_tcu_probe(np);
>  
>  	if (ret)
>  		pr_crit("Failed to initialize TCU clocks: %d\n", ret);
> -
> -	if (IS_ENABLED(CONFIG_PM_SLEEP))
> -		register_syscore(&tcu_pm);
>  }
>  
>  CLK_OF_DECLARE_DRIVER(jz4740_cgu, "ingenic,jz4740-tcu", ingenic_tcu_init);
> -- 
> 2.51.0
>