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

[John Madieu]

Hi Kuninori,

Thanks for your review.

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Sent: Friday, April 3, 2026 3:33 AM
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Subject: Re: [PATCh v3 10/14] 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>
> > ---
> (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.
> 

This solves both issues (raised by Mark) cleanly.
I'll move the clk_prepare/unprepared for the per-SSI ADG clocks
and SSIF supply clock into rsnd_adg_clk_control(), which already
manages other ADG clocks. Prepare happens once at probe (and resume),
unprepare at remove (and suspend), and the trigger path only needs
clk_enable/disable() which is atomic-safe.

This eliminates the hw_params prepare leak concern entirely.

Regards,
John

> Thank you for your help !!
> 
> Best regards
> ---
> Kuninori Morimoto