[PATCH 1/8] ASoC: cs42l42: PLL must be running when changing MCLK_SRC_SEL
Both SCLK and PLL clocks must be running to drive the glitch-free mux behind MCLK_SRC_SEL and complete the switchover.
This patch moves the writing of MCLK_SRC_SEL to when the PLL is started and stopped, so that it only transitions while the PLL is running. The unconditional write MCLK_SRC_SEL=0 in cs42l42_mute_stream() is safe because if the PLL is not running MCLK_SRC_SEL is already 0.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com Fixes: 43fc357199f9 ("ASoC: cs42l42: Set clock source for both ways of stream") --- sound/soc/codecs/cs42l42.c | 25 ++++++++++++++++++------- sound/soc/codecs/cs42l42.h | 1 + 2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index a00dc3c65549..c96549fe6ab2 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -619,6 +619,8 @@ static int cs42l42_pll_config(struct snd_soc_component *component)
for (i = 0; i < ARRAY_SIZE(pll_ratio_table); i++) { if (pll_ratio_table[i].sclk == clk) { + cs42l42->pll_config = i; + /* Configure the internal sample rate */ snd_soc_component_update_bits(component, CS42L42_MCLK_CTL, CS42L42_INTERNAL_FS_MASK, @@ -627,14 +629,9 @@ static int cs42l42_pll_config(struct snd_soc_component *component) (pll_ratio_table[i].mclk_int != 24000000)) << CS42L42_INTERNAL_FS_SHIFT); - /* Set the MCLK src (PLL or SCLK) and the divide - * ratio - */ + snd_soc_component_update_bits(component, CS42L42_MCLK_SRC_SEL, - CS42L42_MCLK_SRC_SEL_MASK | CS42L42_MCLKDIV_MASK, - (pll_ratio_table[i].mclk_src_sel - << CS42L42_MCLK_SRC_SEL_SHIFT) | (pll_ratio_table[i].mclk_div << CS42L42_MCLKDIV_SHIFT)); /* Set up the LRCLK */ @@ -892,13 +889,21 @@ static int cs42l42_mute_stream(struct snd_soc_dai *dai, int mute, int stream) */ regmap_multi_reg_write(cs42l42->regmap, cs42l42_to_osc_seq, ARRAY_SIZE(cs42l42_to_osc_seq)); + + /* Must disconnect PLL before stopping it */ + snd_soc_component_update_bits(component, + CS42L42_MCLK_SRC_SEL, + CS42L42_MCLK_SRC_SEL_MASK, + 0); + usleep_range(100, 200); + snd_soc_component_update_bits(component, CS42L42_PLL_CTL1, CS42L42_PLL_START_MASK, 0); } } else { if (!cs42l42->stream_use) { /* SCLK must be running before codec unmute */ - if ((cs42l42->bclk < 11289600) && (cs42l42->sclk < 11289600)) { + if (pll_ratio_table[cs42l42->pll_config].mclk_src_sel) { snd_soc_component_update_bits(component, CS42L42_PLL_CTL1, CS42L42_PLL_START_MASK, 1);
@@ -919,6 +924,12 @@ static int cs42l42_mute_stream(struct snd_soc_dai *dai, int mute, int stream) CS42L42_PLL_LOCK_TIMEOUT_US); if (ret < 0) dev_warn(component->dev, "PLL failed to lock: %d\n", ret); + + /* PLL must be running to drive glitchless switch logic */ + snd_soc_component_update_bits(component, + CS42L42_MCLK_SRC_SEL, + CS42L42_MCLK_SRC_SEL_MASK, + CS42L42_MCLK_SRC_SEL_MASK); }
/* Mark SCLK as present, turn off internal oscillator */ diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h index 206b3c81d3e0..b92c17be7f58 100644 --- a/sound/soc/codecs/cs42l42.h +++ b/sound/soc/codecs/cs42l42.h @@ -775,6 +775,7 @@ struct cs42l42_private { struct gpio_desc *reset_gpio; struct completion pdn_done; struct snd_soc_jack *jack; + int pll_config; int bclk; u32 sclk; u32 srate;
An I2S frame starts on the falling edge of LRCLK so ASP_STP must be 0.
At the same time, move other format settings in the same register from cs42l42_pll_config() to cs42l42_set_dai_fmt() where you'd expect to find them, and merge into a single write.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com Fixes: 2c394ca79604 ("ASoC: Add support for CS42L42 codec") --- sound/soc/codecs/cs42l42.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index c96549fe6ab2..02486329a570 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -667,15 +667,6 @@ static int cs42l42_pll_config(struct snd_soc_component *component) CS42L42_FSYNC_PULSE_WIDTH_MASK, CS42L42_FRAC1_VAL(fsync - 1) << CS42L42_FSYNC_PULSE_WIDTH_SHIFT); - snd_soc_component_update_bits(component, - CS42L42_ASP_FRM_CFG, - CS42L42_ASP_5050_MASK, - CS42L42_ASP_5050_MASK); - /* Set the frame delay to 1.0 SCLK clocks */ - snd_soc_component_update_bits(component, CS42L42_ASP_FRM_CFG, - CS42L42_ASP_FSD_MASK, - CS42L42_ASP_FSD_1_0 << - CS42L42_ASP_FSD_SHIFT); /* Set the sample rates (96k or lower) */ snd_soc_component_update_bits(component, CS42L42_FS_RATE_EN, CS42L42_FS_EN_MASK, @@ -775,6 +766,18 @@ static int cs42l42_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) /* interface format */ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { case SND_SOC_DAIFMT_I2S: + /* + * 5050 mode, frame starts on falling edge of LRCLK, + * frame delayed by 1.0 SCLKs + */ + snd_soc_component_update_bits(component, + CS42L42_ASP_FRM_CFG, + CS42L42_ASP_STP_MASK | + CS42L42_ASP_5050_MASK | + CS42L42_ASP_FSD_MASK, + CS42L42_ASP_5050_MASK | + (CS42L42_ASP_FSD_1_0 << + CS42L42_ASP_FSD_SHIFT)); break; default: return -EINVAL;
The lowest valid SCLK corresponds to 44.1 kHz at 16-bit. Sample rates less than this would produce SCLK below the minimum when using a normal I2S frame. A constraint must be applied to prevent this.
The constraint is not applied if the machine driver sets SCLK, to allow setups where the host generates additional bits per LRCLK phase to increase the SCLK frequency. In these cases the machine driver would always have to inform this driver of the actual SCLK, and it must select a legal SCLK.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 02486329a570..29e0c8dc8466 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -807,6 +807,25 @@ static int cs42l42_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) return 0; }
+static int cs42l42_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) +{ + struct snd_soc_component *component = dai->component; + struct cs42l42_private *cs42l42 = snd_soc_component_get_drvdata(component); + + /* + * Sample rates < 44.1 kHz would produce an out-of-range SCLK with + * a standard I2S frame. If the machine driver sets SCLK it must be + * legal. + */ + if (cs42l42->sclk) + return 0; + + /* Machine driver has not set a SCLK, limit bottom end to 44.1 kHz */ + return snd_pcm_hw_constraint_minmax(substream->runtime, + SNDRV_PCM_HW_PARAM_RATE, + 44100, 192000); +} + static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) @@ -966,8 +985,8 @@ static int cs42l42_mute_stream(struct snd_soc_dai *dai, int mute, int stream) SNDRV_PCM_FMTBIT_S24_LE |\ SNDRV_PCM_FMTBIT_S32_LE )
- static const struct snd_soc_dai_ops cs42l42_ops = { + .startup = cs42l42_dai_startup, .hw_params = cs42l42_pcm_hw_params, .set_fmt = cs42l42_set_dai_fmt, .set_sysclk = cs42l42_set_sysclk,
I2S always has two LRCLK phases and both CH1 and CH2 of the RX must be enabled (corresponding to the low and high phases of LRCLK.) The selection of the valid data channels is done by setting the DAC CHA_SEL and CHB_SEL. CHA_SEL is always the first (left) channel, CHB_SEL depends on the number of active channels.
Previously for mono ASP CH2 was not enabled, the result was playing mono data would not produce any audio output.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com Fixes: 621d65f3b868 ("ASoC: cs42l42: Provide finer control on playback path") --- sound/soc/codecs/cs42l42.c | 15 +++++++++++++-- sound/soc/codecs/cs42l42.h | 2 ++ 2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 29e0c8dc8466..99c022be94a6 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -460,8 +460,8 @@ static const struct snd_soc_dapm_widget cs42l42_dapm_widgets[] = { SND_SOC_DAPM_OUTPUT("HP"), SND_SOC_DAPM_DAC("DAC", NULL, CS42L42_PWR_CTL1, CS42L42_HP_PDN_SHIFT, 1), SND_SOC_DAPM_MIXER("MIXER", CS42L42_PWR_CTL1, CS42L42_MIXER_PDN_SHIFT, 1, NULL, 0), - SND_SOC_DAPM_AIF_IN("SDIN1", NULL, 0, CS42L42_ASP_RX_DAI0_EN, CS42L42_ASP_RX0_CH1_SHIFT, 0), - SND_SOC_DAPM_AIF_IN("SDIN2", NULL, 1, CS42L42_ASP_RX_DAI0_EN, CS42L42_ASP_RX0_CH2_SHIFT, 0), + SND_SOC_DAPM_AIF_IN("SDIN1", NULL, 0, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_AIF_IN("SDIN2", NULL, 1, SND_SOC_NOPM, 0, 0),
/* Playback Requirements */ SND_SOC_DAPM_SUPPLY("ASP DAI0", CS42L42_PWR_CTL1, CS42L42_ASP_DAI_PDN_SHIFT, 1, NULL, 0), @@ -866,6 +866,17 @@ static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream, snd_soc_component_update_bits(component, CS42L42_ASP_RX_DAI0_CH2_AP_RES, CS42L42_ASP_RX_CH_AP_MASK | CS42L42_ASP_RX_CH_RES_MASK, val); + + /* Channel B comes from the last active channel */ + snd_soc_component_update_bits(component, CS42L42_SP_RX_CH_SEL, + CS42L42_SP_RX_CHB_SEL_MASK, + (channels - 1) << CS42L42_SP_RX_CHB_SEL_SHIFT); + + /* Both LRCLK slots must be enabled */ + snd_soc_component_update_bits(component, CS42L42_ASP_RX_DAI0_EN, + CS42L42_ASP_RX0_CH_EN_MASK, + BIT(CS42L42_ASP_RX0_CH1_SHIFT) | + BIT(CS42L42_ASP_RX0_CH2_SHIFT)); break; default: break; diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h index b92c17be7f58..8734f6828f3e 100644 --- a/sound/soc/codecs/cs42l42.h +++ b/sound/soc/codecs/cs42l42.h @@ -653,6 +653,8 @@
/* Page 0x25 Audio Port Registers */ #define CS42L42_SP_RX_CH_SEL (CS42L42_PAGE_25 + 0x01) +#define CS42L42_SP_RX_CHB_SEL_SHIFT 2 +#define CS42L42_SP_RX_CHB_SEL_MASK (3 << CS42L42_SP_RX_CHB_SEL_SHIFT)
#define CS42L42_SP_RX_ISOC_CTL (CS42L42_PAGE_25 + 0x02) #define CS42L42_SP_RX_RSYNC_SHIFT 6
44.1kHz 16-bit standard I2S gives a SCLK of 1.4112 MHz. Add a PLL configuration for this.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 99c022be94a6..6895f2fe9eb0 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -586,6 +586,7 @@ struct cs42l42_pll_params { * Table 4-5 from the Datasheet */ static const struct cs42l42_pll_params pll_ratio_table[] = { + { 1411200, 0, 1, 0x00, 0x80, 0x000000, 0x03, 0x10, 11289600, 128, 2}, { 1536000, 0, 1, 0x00, 0x7D, 0x000000, 0x03, 0x10, 12000000, 125, 2}, { 2304000, 0, 1, 0x00, 0x55, 0xC00000, 0x02, 0x10, 12288000, 85, 2}, { 2400000, 0, 1, 0x00, 0x50, 0x000000, 0x03, 0x10, 12000000, 80, 2},
If the machine driver calls snd_set_sysclk() with an unsupported SCLK frequency, return an error instead of letting hw_params() fail.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 6895f2fe9eb0..b2ee51443a22 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -891,10 +891,23 @@ static int cs42l42_set_sysclk(struct snd_soc_dai *dai, { struct snd_soc_component *component = dai->component; struct cs42l42_private *cs42l42 = snd_soc_component_get_drvdata(component); + int i;
- cs42l42->sclk = freq; + if (freq == 0) { + cs42l42->sclk = 0; + return 0; + }
- return 0; + for (i = 0; i < ARRAY_SIZE(pll_ratio_table); i++) { + if (pll_ratio_table[i].sclk == freq) { + cs42l42->sclk = freq; + return 0; + } + } + + dev_err(component->dev, "SCLK %u not supported\n", freq); + + return -EINVAL; }
static int cs42l42_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
If the machine driver doesn't call snd_soc_dai_set_sysclk() the SCLK is assumed to be sample_rate * sample_bits * 2 (that is, the rate necessary for a standard I2S frame).
But 24-bit samples can be sent in either a 24-bit slot or a 32-bit slot. If the PLL is configured for a 24-bit slot, but a 32-bit slot is used, cs42l42 will be overclocked.
Ultimately it is the machine driver's responsibilty to call snd_soc_dai_set_sysclk() if SLK will be different from the standard I2S rate. However, it is convenient to assume 32-bit slots to allow this common case without needing special machine driver support. The machine driver then only has to set SCLK if the slots are 24-bit, but if it fails to do this cs42l42 won't be overclocked.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index b2ee51443a22..3677ed4670d0 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -844,6 +844,13 @@ static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream, if (channels == 1) cs42l42->bclk *= 2;
+ /* + * Assume 24-bit samples are in 32-bit slots, to prevent SCLK being + * more than assumed (which would result in overclocking). + */ + if (params_width(params) == 24) + cs42l42->bclk = (cs42l42->bclk / 3) * 4; + switch(substream->stream) { case SNDRV_PCM_STREAM_CAPTURE: if (channels == 2) {
Add the current authors of this module.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 3677ed4670d0..fb1e4c33e27d 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -2127,4 +2127,7 @@ MODULE_DESCRIPTION("ASoC CS42L42 driver"); MODULE_AUTHOR("James Schulman, Cirrus Logic Inc, james.schulman@cirrus.com"); MODULE_AUTHOR("Brian Austin, Cirrus Logic Inc, brian.austin@cirrus.com"); MODULE_AUTHOR("Michael White, Cirrus Logic Inc, michael.white@cirrus.com"); +MODULE_AUTHOR("Lucas Tanure tanureal@opensource.cirrus.com"); +MODULE_AUTHOR("Richard Fitzgerald rf@opensource.cirrus.com"); +MODULE_AUTHOR("Vitaly Rodionov vitalyr@opensource.cirrus.com"); MODULE_LICENSE("GPL");
On Thu, Aug 05, 2021 at 05:11:04PM +0100, Richard Fitzgerald wrote:
Both SCLK and PLL clocks must be running to drive the glitch-free mux behind MCLK_SRC_SEL and complete the switchover.
Please provide a cover letter for serieses, it helps give an overview of what's going on and is useful for tooling.
On Thu, 5 Aug 2021 17:11:04 +0100, Richard Fitzgerald wrote:
Both SCLK and PLL clocks must be running to drive the glitch-free mux behind MCLK_SRC_SEL and complete the switchover.
This patch moves the writing of MCLK_SRC_SEL to when the PLL is started and stopped, so that it only transitions while the PLL is running. The unconditional write MCLK_SRC_SEL=0 in cs42l42_mute_stream() is safe because if the PLL is not running MCLK_SRC_SEL is already 0.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/8] ASoC: cs42l42: PLL must be running when changing MCLK_SRC_SEL commit: f1040e86f83b0f7d5f45724500a6a441731ff4b7 [2/8] ASoC: cs42l42: Fix LRCLK frame start edge commit: 0c2f2ad4f16a58879463d0979a54293f8f296d6f [3/8] ASoC: cs42l42: Constrain sample rate to prevent illegal SCLK commit: 3a5d89a9c6fe306d35dce4496abbb464c1454da0 [4/8] ASoC: cs42l42: Fix mono playback commit: e5ada3f6787a4d6234adc6f2f3ae35c6d5b71ba0 [5/8] ASoC: cs42l42: Add PLL configuration for 44.1kHz/16-bit (no commit info) [6/8] ASoC: cs42l42: Validate dai_set_sysclk() frequency (no commit info) [7/8] ASoC: cs42l42: Assume 24-bit samples are in 32-bit slots (no commit info) [8/8] ASoC: cs42l42: Update module authors (no commit info)
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
On Thu, 5 Aug 2021 17:11:04 +0100, Richard Fitzgerald wrote:
Both SCLK and PLL clocks must be running to drive the glitch-free mux behind MCLK_SRC_SEL and complete the switchover.
This patch moves the writing of MCLK_SRC_SEL to when the PLL is started and stopped, so that it only transitions while the PLL is running. The unconditional write MCLK_SRC_SEL=0 in cs42l42_mute_stream() is safe because if the PLL is not running MCLK_SRC_SEL is already 0.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/8] ASoC: cs42l42: PLL must be running when changing MCLK_SRC_SEL commit: f1040e86f83b0f7d5f45724500a6a441731ff4b7 [2/8] ASoC: cs42l42: Fix LRCLK frame start edge commit: 0c2f2ad4f16a58879463d0979a54293f8f296d6f [3/8] ASoC: cs42l42: Constrain sample rate to prevent illegal SCLK commit: 3a5d89a9c6fe306d35dce4496abbb464c1454da0 [4/8] ASoC: cs42l42: Fix mono playback commit: e5ada3f6787a4d6234adc6f2f3ae35c6d5b71ba0 [5/8] ASoC: cs42l42: Add PLL configuration for 44.1kHz/16-bit commit: b962bae81fa40fcce7662edcb1e426fa37d32abb [6/8] ASoC: cs42l42: Validate dai_set_sysclk() frequency commit: 24cdbb79bbfe690f7fe87507dd0489670eddb5b0 [7/8] ASoC: cs42l42: Assume 24-bit samples are in 32-bit slots commit: c76d572c1ec82e305c97219e28952966958bc31a [8/8] ASoC: cs42l42: Update module authors commit: e2f6867299ac85ce227eee18be11ce2c4a568447
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Mark Brown -
Richard Fitzgerald