| Message ID | 87bjt3h5lc.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: add Renesas MSIOF sound driver | expand |
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);
Hi Mukesh Thank you for reviewing > > 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. Hmm.. indeed. I couldn't understand what I wanted to tell... And I noticed that the code/patch itself is not wrong but strange. I will update around here in v3 Thank you for your help !! Best regards --- Kuninori Morimoto
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);
Audio clock generator (= ADG) can be used standalone, but current driver will be error in such use case. Makes it as not error. 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. Because of this fixup, current rsnd_adg_get_clkout() function name will be strange. Rename get -> init. 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(-)