RE: [PATCH 14/22] ASoC: rsnd: adg: Add per-SSI ADG and SSIF supply clock management

[John Madieu]

Hi Kuninori,

Thank you for the review.

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Sent: Monday, March 23, 2026 2:40 AM
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH 14/22] ASoC: rsnd: adg: Add per-SSI ADG and SSIF
> supply clock management
> 
> 
> 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>
> > ---
> 
> In this patch, it adds RZ/G3E specific params, and use it on common
> function without checking whether it is R-Car or RZ, or whether it has
> param or not.
> Is it keep compatible on R-Car ?

Yes - all clocks are acquired using devm_clk_get_optional(), which returns
NULL when the clock is not present in the device tree. The clk framework
treats NULL as no-op for clk_{prepare|enable|unprepared|disable}, so R-Car
platforms are completely unaffected.

This is the same pattern as the audmac-pp clock/reset and per-SSI reset
control handling in other patches.

Regards,
John

> 
> 
> >  sound/soc/renesas/rcar/adg.c  | 99
> > ++++++++++++++++++++++++++++++++++-
> >  sound/soc/renesas/rcar/rsnd.h |  2 +
> >  sound/soc/renesas/rcar/ssi.c  | 18 +++++++
> >  3 files changed, 118 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/renesas/rcar/adg.c
> > b/sound/soc/renesas/rcar/adg.c index cbb5c4432a2d..131a60689f6d 100644
> > --- a/sound/soc/renesas/rcar/adg.c
> > +++ b/sound/soc/renesas/rcar/adg.c
> > @@ -19,6 +19,9 @@
> >  #define CLKOUT3	3
> >  #define CLKOUTMAX 4
> >
> > +/* Maximum SSI count for per-SSI clocks */
> > +#define ADG_SSI_MAX	10
> > +
> >  #define BRGCKR_31	(1 << 31)
> >  #define BRRx_MASK(x) (0x3FF & x)
> >
> > @@ -34,6 +37,9 @@ struct rsnd_adg {
> >  	struct clk *adg;
> >  	struct clk *clkin[CLKINMAX];
> >  	struct clk *clkout[CLKOUTMAX];
> > +	/* RZ/G3E: per-SSI ADG clocks (adg.ssi.0 through adg.ssi.9) */
> > +	struct clk *clk_adg_ssi[ADG_SSI_MAX];
> > +	struct clk *clk_ssif_supply;
> >  	struct clk *null_clk;
> >  	struct clk_onecell_data onecell;
> >  	struct rsnd_mod mod;
> > @@ -341,10 +347,58 @@ int rsnd_adg_clk_query(struct rsnd_priv *priv,
> unsigned int rate)
> >  	return -EIO;
> >  }
> >
> > +/*
> > + * RZ/G3E: Prepare SSI clocks - call from hw_params (can sleep)  */
> > +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);
> > +}
> > +
> >  int rsnd_adg_ssi_clk_stop(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);
> > +
> >  	rsnd_adg_set_ssi_clk(ssi_mod, 0);
> >
> > +	/* RZ/G3E: only disable here, unprepare is done in hw_free */
> > +	clk_disable(adg->clk_adg_ssi[id]);
> > +	clk_disable(adg->clk_ssif_supply);
> > +
> >  	return 0;
> >  }
> >
> > @@ -354,7 +408,8 @@ int rsnd_adg_ssi_clk_try_start(struct rsnd_mod
> *ssi_mod, unsigned int rate)
> >  	struct rsnd_adg *adg = rsnd_priv_to_adg(priv);
> >  	struct device *dev = rsnd_priv_to_dev(priv);
> >  	struct rsnd_mod *adg_mod = rsnd_mod_get(adg);
> > -	int data;
> > +	int id = rsnd_mod_id(ssi_mod);
> > +	int ret, data;
> >  	u32 ckr = 0;
> >
> >  	data = rsnd_adg_clk_query(priv, rate); @@ -376,6 +431,18 @@ int
> > rsnd_adg_ssi_clk_try_start(struct rsnd_mod *ssi_mod, unsigned int rate)
> >  		(ckr) ?	adg->brg_rate[ADG_HZ_48] :
> >  			adg->brg_rate[ADG_HZ_441]);
> >
> > +	/*
> > +	 * RZ/G3E: enable per-SSI and supply clocks
> > +	 * Prepare was done in hw_params
> > +	 */
> > +	ret = clk_enable(adg->clk_adg_ssi[id]);
> > +	if (ret)
> > +		dev_warn(dev, "Cannot enable adg.ssi.%d ADG clock\n", id);
> > +
> > +	ret = clk_enable(adg->clk_ssif_supply);
> > +	if (ret)
> > +		dev_warn(dev, "Cannot enable SSIF supply clock\n");
> > +
> >  	return 0;
> >  }
> >
> > @@ -769,6 +836,31 @@ void rsnd_adg_clk_dbg_info(struct rsnd_priv
> > *priv, struct seq_file *m)  #define rsnd_adg_clk_dbg_info(priv, m)
> > #endif
> >
> > +static int rsnd_adg_get_ssi_clks(struct rsnd_priv *priv) {
> > +	struct rsnd_adg *adg = rsnd_priv_to_adg(priv);
> > +	struct device *dev = rsnd_priv_to_dev(priv);
> > +	char name[16];
> > +	int i;
> > +
> > +	/* SSIF supply clock */
> > +	adg->clk_ssif_supply = devm_clk_get_optional(dev, "ssif_supply");
> > +	if (IS_ERR(adg->clk_ssif_supply))
> > +		return dev_err_probe(dev, PTR_ERR(adg->clk_ssif_supply),
> > +				     "failed to get ssif_supply clock\n");
> > +
> > +	/* Per-SSI ADG clocks */
> > +	for (i = 0; i < ADG_SSI_MAX; i++) {
> > +		snprintf(name, sizeof(name), "adg.ssi.%d", i);
> > +		adg->clk_adg_ssi[i] = devm_clk_get_optional(dev, name);
> > +		if (IS_ERR(adg->clk_adg_ssi[i]))
> > +			return dev_err_probe(dev, PTR_ERR(adg->clk_adg_ssi[i]),
> > +					     "failed to get %s clock\n", name);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  int rsnd_adg_probe(struct rsnd_priv *priv)  {
> >  	struct reset_control *rstc;
> > @@ -800,6 +892,11 @@ int rsnd_adg_probe(struct rsnd_priv *priv)
> >  	if (ret)
> >  		return ret;
> >
> > +	/* RZ/G3E-specific: per-SSI ADG and SSIF supply clocks */
> > +	ret = rsnd_adg_get_ssi_clks(priv);
> > +	if (ret)
> > +		return ret;
> > +
> >  	ret = rsnd_adg_clk_enable(priv);
> >  	if (ret)
> >  		return ret;
> > diff --git a/sound/soc/renesas/rcar/rsnd.h
> > b/sound/soc/renesas/rcar/rsnd.h index da377bca45a9..6bde304f93a8
> > 100644
> > --- a/sound/soc/renesas/rcar/rsnd.h
> > +++ b/sound/soc/renesas/rcar/rsnd.h
> > @@ -612,6 +612,8 @@ void __iomem *rsnd_gen_get_base_addr(struct
> rsnd_priv *priv, int reg_id);
> >   *	R-Car ADG
> >   */
> >  int rsnd_adg_clk_query(struct rsnd_priv *priv, unsigned int rate);
> > +int rsnd_adg_ssi_clk_prepare(struct rsnd_mod *ssi_mod); void
> > +rsnd_adg_ssi_clk_unprepare(struct rsnd_mod *ssi_mod);
> >  int rsnd_adg_ssi_clk_stop(struct rsnd_mod *ssi_mod);  int
> > rsnd_adg_ssi_clk_try_start(struct rsnd_mod *ssi_mod, unsigned int
> > rate);  int rsnd_adg_probe(struct rsnd_priv *priv); diff --git
> > a/sound/soc/renesas/rcar/ssi.c b/sound/soc/renesas/rcar/ssi.c index
> > e25a4dfae90c..e0eb48f8977b 100644
> > --- a/sound/soc/renesas/rcar/ssi.c
> > +++ b/sound/soc/renesas/rcar/ssi.c
> > @@ -544,6 +544,7 @@ static int rsnd_ssi_hw_params(struct rsnd_mod
> > *mod,  {
> >  	struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
> >  	unsigned int fmt_width =
> > snd_pcm_format_width(params_format(params));
> > +	int ret;
> >
> >  	if (fmt_width > rdai->chan_width) {
> >  		struct rsnd_priv *priv = rsnd_io_to_priv(io); @@ -553,6
> +554,21 @@
> > static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
> >  		return -EINVAL;
> >  	}
> >
> > +	/* RZ/G3E: prepare clocks here (can sleep) */
> > +	ret = rsnd_adg_ssi_clk_prepare(mod);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +static int rsnd_ssi_hw_free(struct rsnd_mod *mod,
> > +			    struct rsnd_dai_stream *io,
> > +			    struct snd_pcm_substream *substream) {
> > +	/* RZ/G3E: unprepare clocks here (can sleep) */
> > +	rsnd_adg_ssi_clk_unprepare(mod);
> > +
> >  	return 0;
> >  }
> >
> > @@ -965,6 +981,7 @@ static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
> >  	.pointer	= rsnd_ssi_pio_pointer,
> >  	.pcm_new	= rsnd_ssi_pcm_new,
> >  	.hw_params	= rsnd_ssi_hw_params,
> > +	.hw_free	= rsnd_ssi_hw_free,
> >  	.get_status	= rsnd_ssi_get_status,
> >  };
> >
> > @@ -1079,6 +1096,7 @@ static struct rsnd_mod_ops rsnd_ssi_dma_ops = {
> >  	.pcm_new	= rsnd_ssi_pcm_new,
> >  	.fallback	= rsnd_ssi_fallback,
> >  	.hw_params	= rsnd_ssi_hw_params,
> > +	.hw_free	= rsnd_ssi_hw_free,
> >  	.get_status	= rsnd_ssi_get_status,
> >  	DEBUG_INFO
> >  };
> > --
> > 2.25.1
> >
> 
> 
> 
> 
> Thank you for your help !!
> 
> Best regards
> ---
> Kuninori Morimoto