[alsa-devel] [PATCH 1/3] ASoC: es8316: judge PCM rate at later timing
This patch change the judge timing about playing/capturing PCM rate.
Original code set constraints list of PCM rate limits at set_sysclk. This strategy works well if system is using fixed rate clock.
But some boards and SoC (such as RockPro64 and RockChip I2S) has connected SoC MCLK out to ES8316 MCLK in. In this case, SoC side I2S will choose suitable frequency of MCLK such as fs * mclk-fs when user starts playing or capturing.
Bad scenario as follows (mclk-fs = 256): - Initialize sysclk by correct value (Ex. 12.288MHz) - ES8316 set constraints of PCM rate by sysclk 48kHz (1/256), 32kHz (1/384), 30.720kHz (1/400), 24kHz (1/512), 16kHz (1/768), 12kHz (1/1024) - Play 48kHz sound, it's acceptable - Sysclk is not changed
- Play 32kHz sound, it's acceptable - Set sysclk by 8.192MHz (= fs * mclk-fs = 32k * 256) - ES8316 set constraints of PCM rate by sysclk 32kHz (1/256), 21.33kHz (1/384), 20.48kHz (1/400), 16kHz (1/512), 10.66kHz (1/768), 8kHz (1/1024)
- Play 48kHz again, but it's NOT acceptable because constraints list does not allow 48kHz
Root cause of this strange behavior is changing constraints list at set_sysclk timing. It seems that is too early to determine. So this patch does not use constraints list and check PCM rate limit more later timing at hw_params.
Signed-off-by: Katsuhiro Suzuki katsuhiro@katsuster.net --- sound/soc/codecs/es8316.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-)
diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c index ed2959dbe1fb..229808fa627c 100644 --- a/sound/soc/codecs/es8316.c +++ b/sound/soc/codecs/es8316.c @@ -363,27 +363,12 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai *codec_dai, { struct snd_soc_component *component = codec_dai->component; struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component); - int i; - int count = 0;
es8316->sysclk = freq;
if (freq == 0) return 0;
- /* Limit supported sample rates to ones that can be autodetected - * by the codec running in slave mode. - */ - for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) { - const unsigned int ratio = supported_mclk_lrck_ratios[i]; - - if (freq % ratio == 0) - es8316->allowed_rates[count++] = freq / ratio; - } - - es8316->sysclk_constraints.list = es8316->allowed_rates; - es8316->sysclk_constraints.count = count; - return 0; }
@@ -449,13 +434,6 @@ static int es8316_pcm_startup(struct snd_pcm_substream *substream, return -EINVAL; }
- /* The set of sample rates that can be supported depends on the - * MCLK supplied to the CODEC. - */ - snd_pcm_hw_constraint_list(substream->runtime, 0, - SNDRV_PCM_HW_PARAM_RATE, - &es8316->sysclk_constraints); - return 0; }
@@ -466,12 +444,27 @@ static int es8316_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_soc_component *component = dai->component; struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component); u8 wordlen = 0; + int i;
if (!es8316->sysclk) { dev_err(component->dev, "No MCLK configured\n"); return -EINVAL; }
+ /* Limit supported sample rates to ones that can be autodetected + * by the codec running in slave mode. + */ + for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) { + const unsigned int ratio = supported_mclk_lrck_ratios[i]; + + if (es8316->sysclk % ratio != 0) + continue; + if (es8316->sysclk / ratio == params_rate(params)) + break; + } + if (i == NR_SUPPORTED_MCLK_LRCK_RATIOS) + return -EINVAL; + switch (params_format(params)) { case SNDRV_PCM_FORMAT_S16_LE: wordlen = ES8316_SERDATA2_LEN_16;
This patch introduce clock property for MCLK master freq control. Driver will set rate of MCLK master if set_sysclk is called and changing sysclk by board driver.
Signed-off-by: Katsuhiro Suzuki katsuhiro@katsuster.net --- sound/soc/codecs/es8316.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c index 229808fa627c..9ed564eac202 100644 --- a/sound/soc/codecs/es8316.c +++ b/sound/soc/codecs/es8316.c @@ -9,6 +9,7 @@
#include <linux/module.h> #include <linux/acpi.h> +#include <linux/clk.h> #include <linux/delay.h> #include <linux/i2c.h> #include <linux/mod_devicetable.h> @@ -33,6 +34,7 @@ static const unsigned int supported_mclk_lrck_ratios[] = {
struct es8316_priv { struct mutex lock; + struct clk *mclk; struct regmap *regmap; struct snd_soc_component *component; struct snd_soc_jack *jack; @@ -363,12 +365,19 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai *codec_dai, { struct snd_soc_component *component = codec_dai->component; struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component); + int ret;
es8316->sysclk = freq;
if (freq == 0) return 0;
+ if (es8316->mclk) { + ret = clk_set_rate(es8316->mclk, freq); + if (ret) + return ret; + } + return 0; }
@@ -693,9 +702,20 @@ static int es8316_set_jack(struct snd_soc_component *component, static int es8316_probe(struct snd_soc_component *component) { struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component); + int ret;
es8316->component = component;
+ es8316->mclk = devm_clk_get(component->dev, "mclk"); + if (PTR_ERR(es8316->mclk) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + if (es8316->mclk) { + ret = clk_prepare_enable(es8316->mclk); + if (ret) + return ret; + } + /* Reset codec and enable current state machine */ snd_soc_component_write(component, ES8316_RESET, 0x3f); usleep_range(5000, 5500);
On Fri, Aug 30, 2019 at 02:32:04AM +0900, Katsuhiro Suzuki wrote:
- es8316->mclk = devm_clk_get(component->dev, "mclk");
- if (PTR_ERR(es8316->mclk) == -EPROBE_DEFER)
return -EPROBE_DEFER;
If we don't get a clock it'd be nice to at least log that in case there's something wrong with the clock driver so that people have more of a hint as to why things might be breaking.
- if (es8316->mclk) {
ret = clk_prepare_enable(es8316->mclk);
if (ret)
return ret;
- }
There's nothing that disables the clock on remove.
Otherwise this looks good.
Hello Mark,
On 2019/08/30 20:18, Mark Brown wrote:
On Fri, Aug 30, 2019 at 02:32:04AM +0900, Katsuhiro Suzuki wrote:
- es8316->mclk = devm_clk_get(component->dev, "mclk");
- if (PTR_ERR(es8316->mclk) == -EPROBE_DEFER)
return -EPROBE_DEFER;
If we don't get a clock it'd be nice to at least log that in case there's something wrong with the clock driver so that people have more of a hint as to why things might be breaking.
OK, to change more user friendly.
- if (es8316->mclk) {
ret = clk_prepare_enable(es8316->mclk);
if (ret)
return ret;
- }
There's nothing that disables the clock on remove.
Otherwise this looks good.
Thank you for reviewing. I'll fix it and send V2 patch set.
Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Best Regards, Katsuhiro Suzuki
This patch adds missing DT-bindings document for Everest ES8316.
Signed-off-by: Katsuhiro Suzuki katsuhiro@katsuster.net --- .../bindings/sound/everest,es8316.txt | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.txt
diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.txt b/Documentation/devicetree/bindings/sound/everest,es8316.txt new file mode 100644 index 000000000000..aefcff9c48a2 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/everest,es8316.txt @@ -0,0 +1,20 @@ +Everest ES8316 audio CODEC + +This device supports both I2C and SPI. + +Required properties: + + - compatible : should be "everest,es8316" + - reg : the I2C address of the device for I2C + - clocks : a list of phandle, should contain entries for clock-names + - clock-names : should include as follows: + "mclk" : master clock (MCLK) of the device + +Example: + +es8316: codec@11 { + compatible = "everest,es8316"; + reg = <0x11>; + clocks = <&clks 10>; + clock-names = "mclk"; +};
participants (2)
-
Katsuhiro Suzuki
-
Mark Brown