RE: [PATCh v3 11/14] ASoC: rsnd: src: Add SRC reset and clock support for RZ/G3E

[John Madieu]

Hi Kuninori,

Thanks for your review.

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Sent: Friday, April 3, 2026 3:39 AM
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Subject: Re: [PATCh v3 11/14] ASoC: rsnd: src: Add SRC reset and clock
> support for RZ/G3E
> 
> 
> Hi John
> 
> > The RZ/G3E SoC requires explicit SCU (Sampling Rate Converter Unit)
> > reset and clock management unlike previous R-Car generations:
> >
> > - scu_clk: SCU module clock
> > - scu_clkx2: SCU double-rate clock
> > - scu_supply_clk: SCU supply clock
> >
> > Without these clocks enabled, the SRC module cannot operate on RZ/G3E.
> > Add support for the shared SCU reset controller used by the SRC
> > modules on the Renesas RZ/G3E SoC. All SRC instances are gated by the
> same "scu"
> > reset line.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> > ---
> (snip)
> > diff --git a/sound/soc/renesas/rcar/rsnd.h
> > b/sound/soc/renesas/rcar/rsnd.h index 2c5738926093..8700b39b535e
> > 100644
> > --- a/sound/soc/renesas/rcar/rsnd.h
> > +++ b/sound/soc/renesas/rcar/rsnd.h
> > @@ -632,6 +632,13 @@ struct rsnd_priv {
> >  	struct clk *audmapp_clk;
> >  	struct reset_control *audmapp_rstc;
> >
> > +	/*
> > +	 * Below values will be filled in rsnd_src_probe()
> > +	 */
> > +	struct clk *clk_scu;
> > +	struct clk *clk_scu_x2;
> > +	struct clk *clk_scu_supply;
> 
> It is SRC specific.
> Please move it to rsnd_src instead of rsnd_priv.

Agreed. However, since rsnd_src is a per-SRC instance structure,
I'll rather have these variables static in src.c, as the clocks
are shared across all SRC instances but used only in that file.
I hope this is fine for you ?

> 
> > @@ -711,8 +720,9 @@ struct rsnd_mod *rsnd_src_mod_get(struct rsnd_priv
> > *priv, int id)
> >
> >  int rsnd_src_probe(struct rsnd_priv *priv)  {
> > -	struct device_node *node;
> >  	struct device *dev = rsnd_priv_to_dev(priv);
> > +	struct reset_control *rstc;
> > +	struct device_node *node;
> 
> Very nitpick. No need to move *node :)
> 

Noted. I'll keep the original declaration.

Regards,
John

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