On 3/15/2021 9:03 PM, Michael Walle wrote:
Am 2021-03-15 16:19, schrieb Sameer Pujar:
On 3/15/2021 5:35 PM, Michael Walle wrote:
Am 2021-03-12 14:46, schrieb Mark Brown:
On Fri, Mar 12, 2021 at 01:30:02PM +0100, Michael Walle wrote:
The card calls set_sysclk(), which eventually ends up in the codec.
The codec therefore, could figure out if it needs to configure the
clock or if it can use its internal FLL.
Is that what you mean?
Yes.
But the set_sysclk() of the codec isn't even called, because the
card itself already tries to call clk_set_rate() on the Codec's
MCLK,
which returns with an error [0].
OK, so I think we need to push this down a level so that the clock
setting is implemented by the core/CODEC rather than by simple-card,
with the helpers being something the CODEC can opt out of.
Sameer, it looks like the proper fix should be to add the clock
support to your codec.
I agree that complicated clock relationships should be handled within
the codec itself, however MCLK rate setting depends on "mclk-fs"
factor and this property is specified as part of
simple-card/audio-graph-card codec subnode. Right now codec, in
general, does not have a way to know this. The set_sysclk() callback
takes rate argument and not the factor.
Why would you need the factor?
Sorry, you are right. I misread the flow. Fixed rates can use
"assigned-clocks" binding and runtime updates can depend on "mclk-fs".
No additional property is required for codec. Something like below
should help.
diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
index 67f0ab817135..7fd41f51f856 100644
--- a/sound/soc/codecs/rt5659.c
+++ b/sound/soc/codecs/rt5659.c
@@ -3426,12 +3426,18 @@ static int rt5659_set_component_sysclk(struct
snd_soc_component *component, int
{
struct rt5659_priv *rt5659 =
snd_soc_component_get_drvdata(component);
unsigned int reg_val = 0;
+ int ret;
if (freq == rt5659->sysclk && clk_id == rt5659->sysclk_src)
return 0;
switch (clk_id) {
case RT5659_SCLK_S_MCLK:
+ ret = clk_set_rate(rt5659->mclk, freq);
+ if (ret)
+ return ret;
reg_val |= RT5659_SCLK_SRC_MCLK;
break;
case RT5659_SCLK_S_PLL1:
