Re: [PATCH v2 4/9] ASoC: renesas: rsnd: allow to use ADG only

Mukesh Kumar Savaliya
2025-04-14T09:04:50Z

On 4/11/2025 6:33 AM, Kuninori Morimoto wrote:
> Audio clock generator (= ADG) can be used standalone, but current driver
> will be error in such use case. Makes it as not error.
will be error ?
Makes it as not error ?

I could not get exact problem here. seems you need to write properly.
> And, current driver registers it as fixed rate clock, but actual clkout
> was handled when SSI start works. Setup clkout setting when it was probed.
> Otherwise it can't be used ADG only.
>
Same here, its not clearly explaining.
> Because of this fixup, current rsnd_adg_get_clkout() function name will be
> strange. Rename get -> init.
>
same here too. Please write in some verbose which says something.
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> sound/soc/renesas/rcar/adg.c | 28 ++++++++++++++++------------
> sound/soc/renesas/rcar/core.c | 7 ++++++-
> 2 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/sound/soc/renesas/rcar/adg.c b/sound/soc/renesas/rcar/adg.c
> index 191f212d338c..db980e4642b8 100644
> --- a/sound/soc/renesas/rcar/adg.c
> +++ b/sound/soc/renesas/rcar/adg.c
> @@ -377,16 +377,9 @@ int rsnd_adg_ssi_clk_try_start(struct rsnd_mod *ssi_mod, unsigned int rate)
> int rsnd_adg_clk_control(struct rsnd_priv *priv, int enable)
> {
> struct rsnd_adg *adg = rsnd_priv_to_adg(priv);
> - struct rsnd_mod *adg_mod = rsnd_mod_get(adg);
> struct clk *clk;
> int ret = 0, i;
>
> - if (enable) {
> - rsnd_mod_bset(adg_mod, BRGCKR, 0x80770000, adg->ckr);
> - rsnd_mod_write(adg_mod, BRRA, adg->brga);
> - rsnd_mod_write(adg_mod, BRRB, adg->brgb);
> - }
> -
> for_each_rsnd_clkin(clk, adg, i) {
> if (enable) {
> ret = clk_prepare_enable(clk);
> @@ -504,13 +497,14 @@ static void rsnd_adg_unregister_clkout(struct rsnd_priv *priv)
> clk_unregister_fixed_rate(clk);
> }
>
> -static int rsnd_adg_get_clkout(struct rsnd_priv *priv)
> +static int rsnd_adg_init_clkout(struct rsnd_priv *priv)
> {
> struct rsnd_adg *adg = priv->adg;
> struct clk *clk;
> struct device *dev = rsnd_priv_to_dev(priv);
> struct device_node *np = dev->of_node;
> struct property *prop;
> + struct rsnd_mod *adg_mod = rsnd_mod_get(adg);
> u32 ckr, brgx, brga, brgb;
> u32 req_rate[ADG_HZ_SIZE] = {};
> uint32_t count = 0;
> @@ -537,7 +531,7 @@ static int rsnd_adg_get_clkout(struct rsnd_priv *priv)
> */
> prop = of_find_property(np, "clock-frequency", NULL);
> if (!prop)
> - goto rsnd_adg_get_clkout_end;
> + goto rsnd_adg_init_clkout_end;
>
> req_size = prop->length / sizeof(u32);
> if (req_size > ADG_HZ_SIZE) {
> @@ -633,7 +627,7 @@ static int rsnd_adg_get_clkout(struct rsnd_priv *priv)
>
> if (!(adg->brg_rate[ADG_HZ_48] && req_Hz[ADG_HZ_48]) &&
> !(adg->brg_rate[ADG_HZ_441] && req_Hz[ADG_HZ_441]))
> - goto rsnd_adg_get_clkout_end;
> + goto rsnd_adg_init_clkout_end;
>
> if (approximate)
> dev_info(dev, "It uses CLK_I as approximate rate");
> @@ -682,11 +676,21 @@ static int rsnd_adg_get_clkout(struct rsnd_priv *priv)
> &adg->onecell);
> }
>
> -rsnd_adg_get_clkout_end:
> +rsnd_adg_init_clkout_end:
> adg->ckr = ckr;
> adg->brga = brga;
> adg->brgb = brgb;
>
> + /*
> + * setup default clkout
> + */
> + if (0 == (req_rate[0] % 8000))
> + ckr = 0x80000000; /* use BRGB output */
> +
> + rsnd_mod_bset(adg_mod, BRGCKR, 0x80770000, adg->ckr | ckr);
> + rsnd_mod_write(adg_mod, BRRA, adg->brga);
> + rsnd_mod_write(adg_mod, BRRB, adg->brgb);
> +
> return 0;
>
> err:
> @@ -764,7 +768,7 @@ int rsnd_adg_probe(struct rsnd_priv *priv)
> if (ret)
> return ret;
>
> - ret = rsnd_adg_get_clkout(priv);
> + ret = rsnd_adg_init_clkout(priv);
> if (ret)
> return ret;
>
> diff --git a/sound/soc/renesas/rcar/core.c b/sound/soc/renesas/rcar/core.c
> index 30afc942d381..4f4ed24cb361 100644
> --- a/sound/soc/renesas/rcar/core.c
> +++ b/sound/soc/renesas/rcar/core.c
> @@ -1482,8 +1482,13 @@ static int rsnd_dai_probe(struct rsnd_priv *priv)
> int dai_i;
>
> nr = rsnd_dai_of_node(priv, &is_graph);
> +
> + /*
> + * There is a case that it is used only for ADG (Sound Clock).
> + * No DAI is not error
> + */
> if (!nr)
> - return -EINVAL;
> + return 0;
>
> rdrv = devm_kcalloc(dev, nr, sizeof(*rdrv), GFP_KERNEL);
> rdai = devm_kcalloc(dev, nr, sizeof(*rdai), GFP_KERNEL);