Re: [PATCh v3 10/14] ASoC: rsnd: adg: Add per-SSI ADG and SSIF supply clock management

[Kuninori Morimoto]

Hi John

> RZ/G3E's ADG module requires explicit clock management for SSI audio
> interfaces that differs from R-Car Gen2/Gen3/Gen4:
> 
>  - Per-SSI ADG clocks (adg.ssi.N) for each SSI module
>  - A shared SSIF supply clock for the SSI subsystem
> 
> These clocks are acquired using optional APIs, making them transparent
> to platforms that do not require them.
> 
> Additionally, since rsnd_adg_ssi_clk_try_start() is called from the
> trigger path (atomic context), clk_prepare_enable() cannot be used
> directly as clk_prepare() may sleep. Split clock handling into:
> 
>  - hw_params: clk_prepare() - sleepable context
>  - trigger (start): clk_enable() - atomic safe
>  - trigger (stop): clk_disable() - atomic safe
>  - hw_free: clk_unprepare() - sleepable context
> 
> Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> ---
(snip)
> +int rsnd_adg_ssi_clk_prepare(struct rsnd_mod *ssi_mod)
> +{
> +	struct rsnd_priv *priv = rsnd_mod_to_priv(ssi_mod);
> +	struct rsnd_adg *adg = rsnd_priv_to_adg(priv);
> +	struct device *dev = rsnd_priv_to_dev(priv);
> +	int id = rsnd_mod_id(ssi_mod);
> +	int ret;
> +
> +	ret = clk_prepare(adg->clk_adg_ssi[id]);
> +	if (ret) {
> +		dev_err(dev, "Cannot prepare adg.ssi.%d ADG clock\n", id);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare(adg->clk_ssif_supply);
> +	if (ret) {
> +		dev_err(dev, "Cannot prepare SSIF supply clock\n");
> +		clk_unprepare(adg->clk_adg_ssi[id]);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * RZ/G3E: Unprepare SSI clocks - call from hw_free (can sleep)
> + */
> +void rsnd_adg_ssi_clk_unprepare(struct rsnd_mod *ssi_mod)
> +{
> +	struct rsnd_priv *priv = rsnd_mod_to_priv(ssi_mod);
> +	struct rsnd_adg *adg = rsnd_priv_to_adg(priv);
> +	int id = rsnd_mod_id(ssi_mod);
> +
> +	clk_unprepare(adg->clk_adg_ssi[id]);
> +	clk_unprepare(adg->clk_ssif_supply);
> +}

Can't we done clk_{un}prepare() at rsnd_adg_clk_control() ?
It is the function that ADG is calling clk_{un}prepare().
Performing similar processes in multiple locations makes maintenance
difficult.

Thank you for your help !!

Best regards
---
Kuninori Morimoto