| Message ID | 20180528193503.18905-2-daniel@zonque.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Daniel > The simple-card driver currently accepts a clock node in the cpu dai > sub-node and only uses it as an alternative to the > 'system-clock-frequency' property to get the current frequency. > > This patch adds another use of the passed clock node. If mclk-fs is > specified, the clock will be set to the calculated rate (stream rate * > mclk_fs) in hw_params. This allows platforms to pass a tuneable clock > as phandle that will automatically be set to the right rates. > > Signed-off-by: Daniel Mack <daniel@zonque.org> > --- (snip) > if (mclk_fs) { > mclk = params_rate(params) * mclk_fs; > + > + if (dai_props->cpu_dai.clk) > + clk_set_rate(dai_props->cpu_dai.clk, mclk); > + > ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, > SND_SOC_CLOCK_IN); > if (ret && ret != -ENOTSUPP) Having codec is nice balance ? Best regards --- Kuninori Morimoto
On Tuesday, May 29, 2018 03:38 AM, Kuninori Morimoto wrote: >> The simple-card driver currently accepts a clock node in the cpu dai >> sub-node and only uses it as an alternative to the >> 'system-clock-frequency' property to get the current frequency. >> >> This patch adds another use of the passed clock node. If mclk-fs is >> specified, the clock will be set to the calculated rate (stream rate * >> mclk_fs) in hw_params. This allows platforms to pass a tuneable clock >> as phandle that will automatically be set to the right rates. >> >> Signed-off-by: Daniel Mack <daniel@zonque.org> >> --- > (snip) >> if (mclk_fs) { >> mclk = params_rate(params) * mclk_fs; >> + >> + if (dai_props->cpu_dai.clk) >> + clk_set_rate(dai_props->cpu_dai.clk, mclk); >> + >> ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, >> SND_SOC_CLOCK_IN); >> if (ret && ret != -ENOTSUPP) > > Having codec is nice balance ? Ah, yes, why not. Will post a v2. Thanks, Daniel
On Mon, May 28, 2018 at 09:35:01PM +0200, Daniel Mack wrote: > if (mclk_fs) { > mclk = params_rate(params) * mclk_fs; > + > + if (dai_props->cpu_dai.clk) > + clk_set_rate(dai_props->cpu_dai.clk, mclk); > + We're ignoring the return value here.
On Tuesday, May 29, 2018 01:16 PM, Mark Brown wrote: > On Mon, May 28, 2018 at 09:35:01PM +0200, Daniel Mack wrote: > >> if (mclk_fs) { >> mclk = params_rate(params) * mclk_fs; >> + >> + if (dai_props->cpu_dai.clk) >> + clk_set_rate(dai_props->cpu_dai.clk, mclk); >> + > > We're ignoring the return value here. > On purpose actually. Not all clocks might be settable, and in that case, this is a no-op. You think we should bail or warn?
On Tue, May 29, 2018 at 01:17:45PM +0200, Daniel Mack wrote: > On Tuesday, May 29, 2018 01:16 PM, Mark Brown wrote: > > On Mon, May 28, 2018 at 09:35:01PM +0200, Daniel Mack wrote: > > > + if (dai_props->cpu_dai.clk) > > > + clk_set_rate(dai_props->cpu_dai.clk, mclk); > > We're ignoring the return value here. > On purpose actually. Not all clocks might be settable, and in that case, > this is a no-op. You think we should bail or warn? If we need to set the rate and fail to set it then clearly we shouldn't just carry on ignoring the error. You might want some more involved logic there around checking if it's actually a rate change before you error out, and possibly some logic to carry on with whatever the rate is and a reduced set of resulting sample rates.
On Tuesday, May 29, 2018 01:32 PM, Mark Brown wrote: > On Tue, May 29, 2018 at 01:17:45PM +0200, Daniel Mack wrote: >> On Tuesday, May 29, 2018 01:16 PM, Mark Brown wrote: >>> On Mon, May 28, 2018 at 09:35:01PM +0200, Daniel Mack wrote: > >>>> + if (dai_props->cpu_dai.clk) >>>> + clk_set_rate(dai_props->cpu_dai.clk, mclk); > >>> We're ignoring the return value here. > >> On purpose actually. Not all clocks might be settable, and in that case, >> this is a no-op. You think we should bail or warn? > > If we need to set the rate and fail to set it then clearly we shouldn't > just carry on ignoring the error. You might want some more involved > logic there around checking if it's actually a rate change before you > error out, and possibly some logic to carry on with whatever the rate is > and a reduced set of resulting sample rates. > Fair enough. I'll add that error checking at least, that makes sense. Thanks for the feedback! Daniel
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index 17c13e74667d..a4c72d09cd45 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -86,6 +86,11 @@ Optional CPU/CODEC subnodes properties: in dai startup() and disabled with clk_disable_unprepare() in dai shutdown(). + If a clock is specified and a + multiplication factor is given with + mclk-fs, the clock will be set to the + calculated mclk frequency when the + stream starts. - system-clock-direction-out : specifies clock direction as 'out' on initialization. It is useful for some aCPUs with fixed clocks. diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 6959a74a6f49..ca529a6cab06 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -154,6 +154,10 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream, if (mclk_fs) { mclk = params_rate(params) * mclk_fs; + + if (dai_props->cpu_dai.clk) + clk_set_rate(dai_props->cpu_dai.clk, mclk); + ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, SND_SOC_CLOCK_IN); if (ret && ret != -ENOTSUPP)
The simple-card driver currently accepts a clock node in the cpu dai sub-node and only uses it as an alternative to the 'system-clock-frequency' property to get the current frequency. This patch adds another use of the passed clock node. If mclk-fs is specified, the clock will be set to the calculated rate (stream rate * mclk_fs) in hw_params. This allows platforms to pass a tuneable clock as phandle that will automatically be set to the right rates. Signed-off-by: Daniel Mack <daniel@zonque.org> --- Documentation/devicetree/bindings/sound/simple-card.txt | 5 +++++ sound/soc/generic/simple-card.c | 4 ++++ 2 files changed, 9 insertions(+)