[PATCH 00/12] ASoC: cs42l42: Series of bugfixes and improvements
Various bugfixes and improvements to the cs42l42 codec driver.
Clocking bugfixes: - Remove any use of MCLK > 12.288 MHz because it isn't possible to switch between the low and high frequency ranges on-the-fly
- Prevent the PLL configuration being written while the PLL is running - Correctly set the MCLK for the sample rate converters
ASP bugfixes: - Ensure the TX (capture) ASP is properly configured when recording mono audio.
Other bugfixes: - Prevent NULL pointer deref if there is an interrupt before soc component probe
Code cleanup: - Remove the unused runtime suspend/resume functions. - Remove some code made redundant by an earlier patch
Improvement: - Add ALSA control for HP path attenuation. Previously this was always set to -6dB with no way for the user to configure it.
- Add ALSA control to disable the soft volume ramp at the start of audio.
Richard Fitzgerald (12): ASoC: cs42l42: Use PLL for SCLK > 12.188MHz ASoC: cs42l42: Don't claim to support 192k ASoC: cs42l42: Always configure both ASP TX channels ASoC: cs42l42: Don't reconfigure the PLL while it is running ASoC: cs42l42: Set correct SRC MCLK ASoC: cs42l42: Mark OSC_SWITCH_STATUS register volatile ASoC: cs42l42: Allow time for HP/ADC to power-up after enable ASoC: cs42l42: Prevent NULL pointer deref in interrupt handler ASoC: cs42l42: Remove runtime_suspend/runtime_resume callbacks ASoC: cs42l42: Remove redundant pll_divout member ASoC: cs42l42: Add HP Volume Scale control ASoC: cs42l42: Add control for audio slow-start switch
sound/soc/codecs/cs42l42.c | 281 ++++++++++++++++++++++++--------------------- sound/soc/codecs/cs42l42.h | 9 +- 2 files changed, 156 insertions(+), 134 deletions(-)
It isn't possible to switch MCLK between 12MHz and 24MHz rate groups on-the-fly - this can only be done when cs42l42 is powered-down.
All "normal" SCLK rates use an MCLK in the 12MHz group, so change the configs for SCLK > 12.288 MHz to use the PLL to generate an MCLK in the 12MHz group.
As this means MCLK_DIV is always 0 it can be removed from the pll configuration setup.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index fb1e4c33e27d..3c1609865440 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -569,7 +569,6 @@ static const struct reg_sequence cs42l42_to_osc_seq[] = {
struct cs42l42_pll_params { u32 sclk; - u8 mclk_div; u8 mclk_src_sel; u8 sclk_prediv; u8 pll_div_int; @@ -586,24 +585,24 @@ 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}, - { 2822400, 0, 1, 0x00, 0x40, 0x000000, 0x03, 0x10, 11289600, 128, 1}, - { 3000000, 0, 1, 0x00, 0x40, 0x000000, 0x03, 0x10, 12000000, 128, 1}, - { 3072000, 0, 1, 0x00, 0x3E, 0x800000, 0x03, 0x10, 12000000, 125, 1}, - { 4000000, 0, 1, 0x00, 0x30, 0x800000, 0x03, 0x10, 12000000, 96, 1}, - { 4096000, 0, 1, 0x00, 0x2E, 0xE00000, 0x03, 0x10, 12000000, 94, 1}, - { 5644800, 0, 1, 0x01, 0x40, 0x000000, 0x03, 0x10, 11289600, 128, 1}, - { 6000000, 0, 1, 0x01, 0x40, 0x000000, 0x03, 0x10, 12000000, 128, 1}, - { 6144000, 0, 1, 0x01, 0x3E, 0x800000, 0x03, 0x10, 12000000, 125, 1}, - { 11289600, 0, 0, 0, 0, 0, 0, 0, 11289600, 0, 1}, - { 12000000, 0, 0, 0, 0, 0, 0, 0, 12000000, 0, 1}, - { 12288000, 0, 0, 0, 0, 0, 0, 0, 12288000, 0, 1}, - { 22579200, 1, 0, 0, 0, 0, 0, 0, 22579200, 0, 1}, - { 24000000, 1, 0, 0, 0, 0, 0, 0, 24000000, 0, 1}, - { 24576000, 1, 0, 0, 0, 0, 0, 0, 24576000, 0, 1} + { 1411200, 1, 0x00, 0x80, 0x000000, 0x03, 0x10, 11289600, 128, 2}, + { 1536000, 1, 0x00, 0x7D, 0x000000, 0x03, 0x10, 12000000, 125, 2}, + { 2304000, 1, 0x00, 0x55, 0xC00000, 0x02, 0x10, 12288000, 85, 2}, + { 2400000, 1, 0x00, 0x50, 0x000000, 0x03, 0x10, 12000000, 80, 2}, + { 2822400, 1, 0x00, 0x40, 0x000000, 0x03, 0x10, 11289600, 128, 1}, + { 3000000, 1, 0x00, 0x40, 0x000000, 0x03, 0x10, 12000000, 128, 1}, + { 3072000, 1, 0x00, 0x3E, 0x800000, 0x03, 0x10, 12000000, 125, 1}, + { 4000000, 1, 0x00, 0x30, 0x800000, 0x03, 0x10, 12000000, 96, 1}, + { 4096000, 1, 0x00, 0x2E, 0xE00000, 0x03, 0x10, 12000000, 94, 1}, + { 5644800, 1, 0x01, 0x40, 0x000000, 0x03, 0x10, 11289600, 128, 1}, + { 6000000, 1, 0x01, 0x40, 0x000000, 0x03, 0x10, 12000000, 128, 1}, + { 6144000, 1, 0x01, 0x3E, 0x800000, 0x03, 0x10, 12000000, 125, 1}, + { 11289600, 0, 0, 0, 0, 0, 0, 11289600, 0, 1}, + { 12000000, 0, 0, 0, 0, 0, 0, 12000000, 0, 1}, + { 12288000, 0, 0, 0, 0, 0, 0, 12288000, 0, 1}, + { 22579200, 1, 0x03, 0x40, 0x000000, 0x03, 0x10, 11289600, 128, 1}, + { 24000000, 1, 0x03, 0x40, 0x000000, 0x03, 0x10, 12000000, 128, 1}, + { 24576000, 1, 0x03, 0x40, 0x000000, 0x03, 0x10, 12288000, 128, 1} };
static int cs42l42_pll_config(struct snd_soc_component *component) @@ -631,10 +630,6 @@ static int cs42l42_pll_config(struct snd_soc_component *component) 24000000)) << CS42L42_INTERNAL_FS_SHIFT);
- snd_soc_component_update_bits(component, CS42L42_MCLK_SRC_SEL, - CS42L42_MCLKDIV_MASK, - (pll_ratio_table[i].mclk_div << - CS42L42_MCLKDIV_SHIFT)); /* Set up the LRCLK */ fsync = clk / cs42l42->srate; if (((fsync * cs42l42->srate) != clk)
The driver currently only supports configuring for sample rates <= 96k and it isn't possible to setup a configuration that will support all sample rates up to 192k.
For sample rates up to 96k MCLK is in the 12MHz group. However, although 192k only requires an I2S clock in the 12MHz group, the cs42l42 audio path is not natively 192k so the audio must be resampled. But for 192k the SRC requires a 24MHz MCLK.
It is not possible to switch MCLK between 12MHz and 24MHz groups on-the-fly. The 12MHz group supports all sample rates up to 96k.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 3c1609865440..a43cdfb6223b 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -819,7 +819,7 @@ static int cs42l42_dai_startup(struct snd_pcm_substream *substream, struct snd_s /* 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); + 44100, 96000); }
static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream, @@ -1026,14 +1026,14 @@ static struct snd_soc_dai_driver cs42l42_dai = { .stream_name = "Playback", .channels_min = 1, .channels_max = 2, - .rates = SNDRV_PCM_RATE_8000_192000, + .rates = SNDRV_PCM_RATE_8000_96000, .formats = CS42L42_FORMATS, }, .capture = { .stream_name = "Capture", .channels_min = 1, .channels_max = 2, - .rates = SNDRV_PCM_RATE_8000_192000, + .rates = SNDRV_PCM_RATE_8000_96000, .formats = CS42L42_FORMATS, }, .symmetric_rate = 1,
An I2S frame always has two slots (left and right) even when sending mono. The right channel (channel 2) of ASP TX will always have the same bit width as the left channel and will always be on the high phase of LRCLK.
The previous implementation always passed the field masks for both channels to snd_soc_component_update_bits() but for mono the written value only contained the settings for channel 1. The result was that for mono channel 2 was set to 8-bit (which is an invalid configuration) with both channels on the low phase of LRCLK.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com Fixes: 585e7079de0e ("ASoC: cs42l42: Add Capture Support") --- sound/soc/codecs/cs42l42.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index a43cdfb6223b..5dc3a30272a4 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -848,11 +848,10 @@ static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream,
switch(substream->stream) { case SNDRV_PCM_STREAM_CAPTURE: - if (channels == 2) { - val |= CS42L42_ASP_TX_CH2_AP_MASK; - val |= width << CS42L42_ASP_TX_CH2_RES_SHIFT; - } - val |= width << CS42L42_ASP_TX_CH1_RES_SHIFT; + /* channel 2 on high LRCLK */ + val = CS42L42_ASP_TX_CH2_AP_MASK | + (width << CS42L42_ASP_TX_CH2_RES_SHIFT) | + (width << CS42L42_ASP_TX_CH1_RES_SHIFT);
snd_soc_component_update_bits(component, CS42L42_ASP_TX_CH_AP_RES, CS42L42_ASP_TX_CH1_AP_MASK | CS42L42_ASP_TX_CH2_AP_MASK |
cs42l42_pcm_hw_params() must only configure the PLL if this is the first stream to become active, otherwise it will be overwriting the registers while the PLL is running.
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 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 5dc3a30272a4..1893d3694570 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -884,7 +884,11 @@ static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream, break; }
- return cs42l42_pll_config(component); + /* Configure the PLL if this is the first active stream */ + if (!cs42l42->stream_use) + return cs42l42_pll_config(component); + else + return 0; }
static int cs42l42_set_sysclk(struct snd_soc_dai *dai,
On Tue, Aug 10, 2021 at 04:37:51PM +0100, Richard Fitzgerald wrote:
cs42l42_pcm_hw_params() must only configure the PLL if this is the first stream to become active, otherwise it will be overwriting the registers while the PLL is running.
Shouldn't the PLL code be noticing problematic attempts to reconfigure the PLL while it's active rather than the individual callers?
On 10/08/2021 16:49, Mark Brown wrote:
On Tue, Aug 10, 2021 at 04:37:51PM +0100, Richard Fitzgerald wrote:
cs42l42_pcm_hw_params() must only configure the PLL if this is the first stream to become active, otherwise it will be overwriting the registers while the PLL is running.
Shouldn't the PLL code be noticing problematic attempts to reconfigure the PLL while it's active rather than the individual callers?
It's wrong for a hw_params() for one stream to try to configure the PLL when the other stream has already called hw_params(), configured the PLL and started it. E.g. if you started a PLAYBACK, configured and started everything, then got another hw_params() for the CAPTURE.
cs42l42_pll_config() could check whether it is already running and skip configuration in that case, but that seems to me a rather opaque implementation. In my opinion this doesn't really fall into the case of ignoring-bad-stuff-to-be-helpful (like free() accepting a NULL).
hw_params() deals with both playback and capture streams so it makes sense to me that it should contain logic to ensure it isn't stomping on the other stream it already set up, rather than have everything it calls figure out whether it shouldn't have done that.
On Tue, Aug 10, 2021 at 05:27:45PM +0100, Richard Fitzgerald wrote:
On 10/08/2021 16:49, Mark Brown wrote:
Shouldn't the PLL code be noticing problematic attempts to reconfigure the PLL while it's active rather than the individual callers?
It's wrong for a hw_params() for one stream to try to configure the PLL when the other stream has already called hw_params(), configured the PLL and started it. E.g. if you started a PLAYBACK, configured and started everything, then got another hw_params() for the CAPTURE.
cs42l42_pll_config() could check whether it is already running and skip configuration in that case, but that seems to me a rather opaque implementation. In my opinion this doesn't really fall into the case of ignoring-bad-stuff-to-be-helpful (like free() accepting a NULL).
This doesn't treat the situation as an error though, it just ignores it, and there's nothing to stop _pll_config() generating a warning if that makes sense.
On 11/08/2021 12:56, Mark Brown wrote:
On Tue, Aug 10, 2021 at 05:27:45PM +0100, Richard Fitzgerald wrote:
On 10/08/2021 16:49, Mark Brown wrote:
Shouldn't the PLL code be noticing problematic attempts to reconfigure the PLL while it's active rather than the individual callers?
It's wrong for a hw_params() for one stream to try to configure the PLL when the other stream has already called hw_params(), configured the PLL and started it. E.g. if you started a PLAYBACK, configured and started everything, then got another hw_params() for the CAPTURE.
cs42l42_pll_config() could check whether it is already running and skip configuration in that case, but that seems to me a rather opaque implementation. In my opinion this doesn't really fall into the case of ignoring-bad-stuff-to-be-helpful (like free() accepting a NULL).
This doesn't treat the situation as an error though, it just ignores it, and there's nothing to stop _pll_config() generating a warning if that makes sense.
It isn't an error. hw_params() will be called for both substreams (PLAYBACK and CAPTURE) and if one is already running we mustn't reconfigure the things we already configured. The DAI is marked symmetric so both substreams will always produce the same I2C BCLK.
As in:
hw_params() substream=PLAYBACK configure PLL prepare() substream=PLAYBACK PLL is started hw_params() substream=CAPTURE PLAYBACK substream already running so don't rewrite PLL registers
Some of the PLL configurations start with a "safe" configuration and then switch over to the running configuration once the PLL is stable. Calling pll_config() again would rewrite back to the startup config, which would change the clock output.
It's ok if neither stream is started, since the PLL isn't started. This is needed anyway because it is legal for hw_params() to be called several times to change parameters without starting a stream.
On Wed, Aug 11, 2021 at 01:21:24PM +0100, Richard Fitzgerald wrote:
On 11/08/2021 12:56, Mark Brown wrote:
On Tue, Aug 10, 2021 at 05:27:45PM +0100, Richard Fitzgerald wrote:
cs42l42_pll_config() could check whether it is already running and skip configuration in that case, but that seems to me a rather opaque implementation. In my opinion this doesn't really fall into the case of ignoring-bad-stuff-to-be-helpful (like free() accepting a NULL).
This doesn't treat the situation as an error though, it just ignores it, and there's nothing to stop _pll_config() generating a warning if that makes sense.
It isn't an error. hw_params() will be called for both substreams (PLAYBACK and CAPTURE) and if one is already running we mustn't reconfigure the things we already configured. The DAI is marked symmetric so both substreams will always produce the same I2C BCLK.
If it's a noop reconfiguration then there's a case for saying that _pll_config() should just silently do nothing anyway regardless of issues with reconfiguring, though you might also want to warn dpeending on other expectations. If it's not a noop reconfiguration then presumably the new configuration not taking effect might mean that other things aren't going to see the clocks they expect. Either way if a reconfiguration gets introduced via a path other than hw_params(), either now or later, having the check in the _pll_config() would catch it.
On 10/08/2021 16:49, Mark Brown wrote:
On Tue, Aug 10, 2021 at 04:37:51PM +0100, Richard Fitzgerald wrote:
cs42l42_pcm_hw_params() must only configure the PLL if this is the first stream to become active, otherwise it will be overwriting the registers while the PLL is running.
Shouldn't the PLL code be noticing problematic attempts to reconfigure the PLL while it's active rather than the individual callers?
I'll push a new version with cs42l42_pll_config() doing the check.
According to the datasheet the SRC MCLK must be as near as possible to (125 * sample rate). This means it should be ~6MHz for rates up to 48k and ~12MHz for rates above that. As per datasheet table 4-21.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 60 ++++++++++++++++++++++++++++++---------------- sound/soc/codecs/cs42l42.h | 1 + 2 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 1893d3694570..14fd70c56891 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -663,22 +663,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); - /* Set the sample rates (96k or lower) */ - snd_soc_component_update_bits(component, CS42L42_FS_RATE_EN, - CS42L42_FS_EN_MASK, - (CS42L42_FS_EN_IASRC_96K | - CS42L42_FS_EN_OASRC_96K) << - CS42L42_FS_EN_SHIFT); - /* Set the input/output internal MCLK clock ~12 MHz */ - snd_soc_component_update_bits(component, CS42L42_IN_ASRC_CLK, - CS42L42_CLK_IASRC_SEL_MASK, - CS42L42_CLK_IASRC_SEL_12 << - CS42L42_CLK_IASRC_SEL_SHIFT); - snd_soc_component_update_bits(component, - CS42L42_OUT_ASRC_CLK, - CS42L42_CLK_OASRC_SEL_MASK, - CS42L42_CLK_OASRC_SEL_12 << - CS42L42_CLK_OASRC_SEL_SHIFT); if (pll_ratio_table[i].mclk_src_sel == 0) { /* Pass the clock straight through */ snd_soc_component_update_bits(component, @@ -741,6 +725,34 @@ static int cs42l42_pll_config(struct snd_soc_component *component) return -EINVAL; }
+static void cs42l42_src_config(struct snd_soc_component *component, unsigned int sample_rate) +{ + unsigned int fs; + + /* SRC MCLK must be as close as possible to 125 * sample rate */ + if (sample_rate <= 48000) + fs = CS42L42_CLK_IASRC_SEL_6; + else + fs = CS42L42_CLK_IASRC_SEL_12; + + /* Set the sample rates (96k or lower) */ + snd_soc_component_update_bits(component, + CS42L42_FS_RATE_EN, + CS42L42_FS_EN_MASK, + (CS42L42_FS_EN_IASRC_96K | + CS42L42_FS_EN_OASRC_96K) << + CS42L42_FS_EN_SHIFT); + + snd_soc_component_update_bits(component, + CS42L42_IN_ASRC_CLK, + CS42L42_CLK_IASRC_SEL_MASK, + fs << CS42L42_CLK_IASRC_SEL_SHIFT); + snd_soc_component_update_bits(component, + CS42L42_OUT_ASRC_CLK, + CS42L42_CLK_OASRC_SEL_MASK, + fs << CS42L42_CLK_OASRC_SEL_SHIFT); +} + static int cs42l42_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) { struct snd_soc_component *component = codec_dai->component; @@ -831,6 +843,7 @@ static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream, unsigned int channels = params_channels(params); unsigned int width = (params_width(params) / 8) - 1; unsigned int val = 0; + int ret;
cs42l42->srate = params_rate(params); cs42l42->bclk = snd_soc_params_to_bclk(params); @@ -884,11 +897,16 @@ static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream, break; }
- /* Configure the PLL if this is the first active stream */ - if (!cs42l42->stream_use) - return cs42l42_pll_config(component); - else - return 0; + /* Configure clocking only if this is the first active stream */ + if (!cs42l42->stream_use) { + ret = cs42l42_pll_config(component); + if (ret) + return ret; + + cs42l42_src_config(component, params_rate(params)); + } + + return 0; }
static int cs42l42_set_sysclk(struct snd_soc_dai *dai, diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h index 8734f6828f3e..addb6560c649 100644 --- a/sound/soc/codecs/cs42l42.h +++ b/sound/soc/codecs/cs42l42.h @@ -288,6 +288,7 @@ #define CS42L42_IN_ASRC_CLK (CS42L42_PAGE_12 + 0x0A) #define CS42L42_CLK_IASRC_SEL_SHIFT 0 #define CS42L42_CLK_IASRC_SEL_MASK (1 << CS42L42_CLK_IASRC_SEL_SHIFT) +#define CS42L42_CLK_IASRC_SEL_6 0 #define CS42L42_CLK_IASRC_SEL_12 1
#define CS42L42_OUT_ASRC_CLK (CS42L42_PAGE_12 + 0x0B)
OSC_SWITCH_STATUS is a volatile register indicating the current state of the clock switch logic.
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 14fd70c56891..dd677055a3c1 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -351,6 +351,7 @@ static bool cs42l42_volatile_register(struct device *dev, unsigned int reg) case CS42L42_DEVID_CD: case CS42L42_DEVID_E: case CS42L42_MCLK_STATUS: + case CS42L42_OSC_SWITCH_STATUS: case CS42L42_TRSENSE_STATUS: case CS42L42_HS_DET_STATUS: case CS42L42_ADC_OVFL_STATUS:
After enabling the HP or ADC by writing the corresponding PDN=0, it takes around 20 milliseconds for it to power up and the midrail supply to be stable. Add this wait into a DAPM widget callback.
If HP and ADC are both powering up in a DAPM sequence, there's no need to do the wait twice. The widget will perform one wait in the POST_PMU if there was a PRE_PMU for one or both.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 31 +++++++++++++++++++++++++++++-- sound/soc/codecs/cs42l42.h | 2 ++ 2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index dd677055a3c1..0b63dba07b6d 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -456,10 +456,36 @@ static const struct snd_kcontrol_new cs42l42_snd_controls[] = { 0x3f, 1, mixer_tlv) };
+static int cs42l42_hp_adc_ev(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm); + struct cs42l42_private *cs42l42 = snd_soc_component_get_drvdata(component); + + switch (event) { + case SND_SOC_DAPM_PRE_PMU: + cs42l42->hp_adc_up_pending = true; + break; + case SND_SOC_DAPM_POST_PMU: + /* Only need one delay if HP and ADC are both powering-up */ + if (cs42l42->hp_adc_up_pending) { + usleep_range(CS42L42_HP_ADC_EN_TIME_US, + CS42L42_HP_ADC_EN_TIME_US + 1000); + cs42l42->hp_adc_up_pending = false; + } + break; + default: + break; + } + + return 0; +} + static const struct snd_soc_dapm_widget cs42l42_dapm_widgets[] = { /* Playback Path */ SND_SOC_DAPM_OUTPUT("HP"), - SND_SOC_DAPM_DAC("DAC", NULL, CS42L42_PWR_CTL1, CS42L42_HP_PDN_SHIFT, 1), + SND_SOC_DAPM_DAC_E("DAC", NULL, CS42L42_PWR_CTL1, CS42L42_HP_PDN_SHIFT, 1, + cs42l42_hp_adc_ev, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU), SND_SOC_DAPM_MIXER("MIXER", CS42L42_PWR_CTL1, CS42L42_MIXER_PDN_SHIFT, 1, NULL, 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), @@ -469,7 +495,8 @@ static const struct snd_soc_dapm_widget cs42l42_dapm_widgets[] = {
/* Capture Path */ SND_SOC_DAPM_INPUT("HS"), - SND_SOC_DAPM_ADC("ADC", NULL, CS42L42_PWR_CTL1, CS42L42_ADC_PDN_SHIFT, 1), + SND_SOC_DAPM_ADC_E("ADC", NULL, CS42L42_PWR_CTL1, CS42L42_ADC_PDN_SHIFT, 1, + cs42l42_hp_adc_ev, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU), SND_SOC_DAPM_AIF_OUT("SDOUT1", NULL, 0, CS42L42_ASP_TX_CH_EN, CS42L42_ASP_TX0_CH1_SHIFT, 0), SND_SOC_DAPM_AIF_OUT("SDOUT2", NULL, 1, CS42L42_ASP_TX_CH_EN, CS42L42_ASP_TX0_CH2_SHIFT, 0),
diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h index addb6560c649..d13749e9d8c5 100644 --- a/sound/soc/codecs/cs42l42.h +++ b/sound/soc/codecs/cs42l42.h @@ -762,6 +762,7 @@ #define CS42L42_CLOCK_SWITCH_DELAY_US 150 #define CS42L42_PLL_LOCK_POLL_US 250 #define CS42L42_PLL_LOCK_TIMEOUT_US 1250 +#define CS42L42_HP_ADC_EN_TIME_US 20000
static const char *const cs42l42_supply_names[CS42L42_NUM_SUPPLIES] = { "VA", @@ -795,6 +796,7 @@ struct cs42l42_private { u8 hs_bias_ramp_time; u8 hs_bias_sense_en; u8 stream_use; + bool hp_adc_up_pending; };
#endif /* __CS42L42_H__ */
The interrupt handling code was getting the struct device* from a struct snd_soc_component* stored in struct cs42l42_private. If the interrupt was asserted before ASoC calls component_probe() the snd_soc_component* will be NULL.
The stored snd_soc_component* is not actually used for anything other than indirectly getting the struct device*. Remove it, and store the struct device* in struct cs42l42_private.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 26 ++++++++------------------ sound/soc/codecs/cs42l42.h | 2 +- 2 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 0b63dba07b6d..b7a1231add2d 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -554,17 +554,7 @@ static int cs42l42_set_jack(struct snd_soc_component *component, struct snd_soc_ return 0; }
-static int cs42l42_component_probe(struct snd_soc_component *component) -{ - struct cs42l42_private *cs42l42 = snd_soc_component_get_drvdata(component); - - cs42l42->component = component; - - return 0; -} - static const struct snd_soc_component_driver soc_component_dev_cs42l42 = { - .probe = cs42l42_component_probe, .set_jack = cs42l42_set_jack, .dapm_widgets = cs42l42_dapm_widgets, .num_dapm_widgets = ARRAY_SIZE(cs42l42_dapm_widgets), @@ -1416,19 +1406,19 @@ static int cs42l42_handle_button_press(struct cs42l42_private *cs42l42) switch (bias_level) { case 1: /* Function C button press */ bias_level = SND_JACK_BTN_2; - dev_dbg(cs42l42->component->dev, "Function C button press\n"); + dev_dbg(cs42l42->dev, "Function C button press\n"); break; case 2: /* Function B button press */ bias_level = SND_JACK_BTN_1; - dev_dbg(cs42l42->component->dev, "Function B button press\n"); + dev_dbg(cs42l42->dev, "Function B button press\n"); break; case 3: /* Function D button press */ bias_level = SND_JACK_BTN_3; - dev_dbg(cs42l42->component->dev, "Function D button press\n"); + dev_dbg(cs42l42->dev, "Function D button press\n"); break; case 4: /* Function A button press */ bias_level = SND_JACK_BTN_0; - dev_dbg(cs42l42->component->dev, "Function A button press\n"); + dev_dbg(cs42l42->dev, "Function A button press\n"); break; default: bias_level = 0; @@ -1502,7 +1492,6 @@ static const struct cs42l42_irq_params irq_params_table[] = { static irqreturn_t cs42l42_irq_thread(int irq, void *data) { struct cs42l42_private *cs42l42 = (struct cs42l42_private *)data; - struct snd_soc_component *component = cs42l42->component; unsigned int stickies[12]; unsigned int masks[12]; unsigned int current_plug_status; @@ -1549,7 +1538,7 @@ static irqreturn_t cs42l42_irq_thread(int irq, void *data) default: break; } - dev_dbg(component->dev, "Auto detect done (%d)\n", cs42l42->hs_type); + dev_dbg(cs42l42->dev, "Auto detect done (%d)\n", cs42l42->hs_type); } }
@@ -1583,7 +1572,7 @@ static irqreturn_t cs42l42_irq_thread(int irq, void *data) SND_JACK_BTN_0 | SND_JACK_BTN_1 | SND_JACK_BTN_2 | SND_JACK_BTN_3);
- dev_dbg(component->dev, "Unplug event\n"); + dev_dbg(cs42l42->dev, "Unplug event\n"); } break;
@@ -1599,7 +1588,7 @@ static irqreturn_t cs42l42_irq_thread(int irq, void *data) CS42L42_M_HSBIAS_HIZ_MASK)) {
if (current_button_status & CS42L42_M_DETECT_TF_MASK) { - dev_dbg(component->dev, "Button released\n"); + dev_dbg(cs42l42->dev, "Button released\n"); report = 0; } else if (current_button_status & CS42L42_M_DETECT_FT_MASK) { report = cs42l42_handle_button_press(cs42l42); @@ -1953,6 +1942,7 @@ static int cs42l42_i2c_probe(struct i2c_client *i2c_client, if (!cs42l42) return -ENOMEM;
+ cs42l42->dev = &i2c_client->dev; i2c_set_clientdata(i2c_client, cs42l42);
cs42l42->regmap = devm_regmap_init_i2c(i2c_client, &cs42l42_regmap); diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h index d13749e9d8c5..a1e6d443b489 100644 --- a/sound/soc/codecs/cs42l42.h +++ b/sound/soc/codecs/cs42l42.h @@ -774,7 +774,7 @@ static const char *const cs42l42_supply_names[CS42L42_NUM_SUPPLIES] = {
struct cs42l42_private { struct regmap *regmap; - struct snd_soc_component *component; + struct device *dev; struct regulator_bulk_data supplies[CS42L42_NUM_SUPPLIES]; struct gpio_desc *reset_gpio; struct completion pdn_done;
The driver has runtime_suspend and runtime_resume callbacks, but pm_runtime is never enabled so these functions won't be called, and the runtime_suspend would cause jack detect to stop working.
These functions are unused - delete them.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 44 -------------------------------------------- 1 file changed, 44 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index b7a1231add2d..93a8fa290cb6 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -25,7 +25,6 @@ #include <linux/regulator/consumer.h> #include <linux/gpio/consumer.h> #include <linux/of_device.h> -#include <linux/pm_runtime.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -2067,19 +2066,6 @@ static int cs42l42_i2c_remove(struct i2c_client *i2c_client) struct cs42l42_private *cs42l42 = i2c_get_clientdata(i2c_client);
devm_free_irq(&i2c_client->dev, i2c_client->irq, cs42l42); - pm_runtime_suspend(&i2c_client->dev); - pm_runtime_disable(&i2c_client->dev); - - return 0; -} - -#ifdef CONFIG_PM -static int cs42l42_runtime_suspend(struct device *dev) -{ - struct cs42l42_private *cs42l42 = dev_get_drvdata(dev); - - regcache_cache_only(cs42l42->regmap, true); - regcache_mark_dirty(cs42l42->regmap);
/* Hold down reset */ gpiod_set_value_cansleep(cs42l42->reset_gpio, 0); @@ -2091,35 +2077,6 @@ static int cs42l42_runtime_suspend(struct device *dev) return 0; }
-static int cs42l42_runtime_resume(struct device *dev) -{ - struct cs42l42_private *cs42l42 = dev_get_drvdata(dev); - int ret; - - /* Enable power */ - ret = regulator_bulk_enable(ARRAY_SIZE(cs42l42->supplies), - cs42l42->supplies); - if (ret != 0) { - dev_err(dev, "Failed to enable supplies: %d\n", - ret); - return ret; - } - - gpiod_set_value_cansleep(cs42l42->reset_gpio, 1); - usleep_range(CS42L42_BOOT_TIME_US, CS42L42_BOOT_TIME_US * 2); - - regcache_cache_only(cs42l42->regmap, false); - regcache_sync(cs42l42->regmap); - - return 0; -} -#endif - -static const struct dev_pm_ops cs42l42_runtime_pm = { - SET_RUNTIME_PM_OPS(cs42l42_runtime_suspend, cs42l42_runtime_resume, - NULL) -}; - #ifdef CONFIG_OF static const struct of_device_id cs42l42_of_match[] = { { .compatible = "cirrus,cs42l42", }, @@ -2146,7 +2103,6 @@ MODULE_DEVICE_TABLE(i2c, cs42l42_id); static struct i2c_driver cs42l42_i2c_driver = { .driver = { .name = "cs42l42", - .pm = &cs42l42_runtime_pm, .of_match_table = of_match_ptr(cs42l42_of_match), .acpi_match_table = ACPI_PTR(cs42l42_acpi_match), },
Now that struct cs42l42_private has pll_config, the current PLL configuration can be looked up directly in pll_ratio_table. This makes the pll_divout member of cs42l42_private redundant since it was only a copy of the value from pll_ratio_table.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 9 +++------ sound/soc/codecs/cs42l42.h | 1 - 2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 93a8fa290cb6..5c1a587af89e 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -725,10 +725,6 @@ static int cs42l42_pll_config(struct snd_soc_component *component) CS42L42_PLL_DIVOUT_MASK, (pll_ratio_table[i].pll_divout * pll_ratio_table[i].n) << CS42L42_PLL_DIVOUT_SHIFT); - if (pll_ratio_table[i].n != 1) - cs42l42->pll_divout = pll_ratio_table[i].pll_divout; - else - cs42l42->pll_divout = 0; snd_soc_component_update_bits(component, CS42L42_PLL_CAL_RATIO, CS42L42_PLL_CAL_RATIO_MASK, @@ -994,12 +990,13 @@ static int cs42l42_mute_stream(struct snd_soc_dai *dai, int mute, int stream) snd_soc_component_update_bits(component, CS42L42_PLL_CTL1, CS42L42_PLL_START_MASK, 1);
- if (cs42l42->pll_divout) { + if (pll_ratio_table[cs42l42->pll_config].n > 1) { usleep_range(CS42L42_PLL_DIVOUT_TIME_US, CS42L42_PLL_DIVOUT_TIME_US * 2); + regval = pll_ratio_table[cs42l42->pll_config].pll_divout; snd_soc_component_update_bits(component, CS42L42_PLL_CTL3, CS42L42_PLL_DIVOUT_MASK, - cs42l42->pll_divout << + regval << CS42L42_PLL_DIVOUT_SHIFT); }
diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h index a1e6d443b489..b10796d755ae 100644 --- a/sound/soc/codecs/cs42l42.h +++ b/sound/soc/codecs/cs42l42.h @@ -783,7 +783,6 @@ struct cs42l42_private { int bclk; u32 sclk; u32 srate; - u8 pll_divout; u8 plug_state; u8 hs_type; u8 ts_inv;
cs42l42 has a configurable scaling of the maximum volume. Add an ALSA control for this. Note that the datasheet name is "full scale volume" but this conflicts with ALSA naming convention so the control is named "HP Volume Scale".
Before this change the FULL_SCALE_VOLUME was set based on the value in RLA_STAT, but as there isn't any load detection result this always set the scaling to -6dB instead of the default 0dB.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index 5c1a587af89e..b2632fdef9a0 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -425,6 +425,14 @@ static SOC_ENUM_SINGLE_DECL(cs42l42_wnf3_freq_enum, CS42L42_ADC_WNF_HPF_CTL, CS42L42_ADC_WNF_CF_SHIFT, cs42l42_wnf3_freq_text);
+static const char * const cs42l42_full_scale_vol_text[] = { + "0dB", "-6dB" +}; + +static SOC_ENUM_SINGLE_DECL(cs42l42_full_scale_vol_enum, CS42L42_HP_CTL, + CS42L42_HP_FULL_SCALE_VOL_SHIFT, + cs42l42_full_scale_vol_text); + static const struct snd_kcontrol_new cs42l42_snd_controls[] = { /* ADC Volume and Filter Controls */ SOC_SINGLE("ADC Notch Switch", CS42L42_ADC_CTL, @@ -450,6 +458,7 @@ static const struct snd_kcontrol_new cs42l42_snd_controls[] = { CS42L42_DACB_INV_SHIFT, true, false), SOC_SINGLE("DAC HPF Switch", CS42L42_DAC_CTL2, CS42L42_DAC_HPF_EN_SHIFT, true, false), + SOC_ENUM("HP Volume Scale", cs42l42_full_scale_vol_enum), SOC_DOUBLE_R_TLV("Mixer Volume", CS42L42_MIXER_CHA_VOL, CS42L42_MIXER_CHB_VOL, CS42L42_MIXER_CH_VOL_SHIFT, 0x3f, 1, mixer_tlv) @@ -951,7 +960,6 @@ static int cs42l42_mute_stream(struct snd_soc_dai *dai, int mute, int stream) struct snd_soc_component *component = dai->component; struct cs42l42_private *cs42l42 = snd_soc_component_get_drvdata(component); unsigned int regval; - u8 fullScaleVol; int ret;
if (mute) { @@ -1023,20 +1031,11 @@ static int cs42l42_mute_stream(struct snd_soc_dai *dai, int mute, int stream) cs42l42->stream_use |= 1 << stream;
if (stream == SNDRV_PCM_STREAM_PLAYBACK) { - /* Read the headphone load */ - regval = snd_soc_component_read(component, CS42L42_LOAD_DET_RCSTAT); - if (((regval & CS42L42_RLA_STAT_MASK) >> CS42L42_RLA_STAT_SHIFT) == - CS42L42_RLA_STAT_15_OHM) { - fullScaleVol = CS42L42_HP_FULL_SCALE_VOL_MASK; - } else { - fullScaleVol = 0; - } - - /* Un-mute the headphone, set the full scale volume flag */ + /* Un-mute the headphone */ snd_soc_component_update_bits(component, CS42L42_HP_CTL, CS42L42_HP_ANA_AMUTE_MASK | - CS42L42_HP_ANA_BMUTE_MASK | - CS42L42_HP_FULL_SCALE_VOL_MASK, fullScaleVol); + CS42L42_HP_ANA_BMUTE_MASK, + 0); } }
This adds an ALSA control so that the slow-start audio ramp feature can be disabled. This is useful for high-definition audio applications.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/cs42l42.c | 33 ++++++++++++++++++++++++++++++++- sound/soc/codecs/cs42l42.h | 3 +++ 2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c index b2632fdef9a0..ae9190720380 100644 --- a/sound/soc/codecs/cs42l42.c +++ b/sound/soc/codecs/cs42l42.c @@ -43,6 +43,7 @@ static const struct reg_default cs42l42_reg_defaults[] = { { CS42L42_MCLK_STATUS, 0x02 }, { CS42L42_MCLK_CTL, 0x02 }, { CS42L42_SFTRAMP_RATE, 0xA4 }, + { CS42L42_SLOW_START_ENABLE, 0x70 }, { CS42L42_I2C_DEBOUNCE, 0x88 }, { CS42L42_I2C_STRETCH, 0x03 }, { CS42L42_I2C_TIMEOUT, 0xB7 }, @@ -198,6 +199,7 @@ static bool cs42l42_readable_register(struct device *dev, unsigned int reg) case CS42L42_MCLK_STATUS: case CS42L42_MCLK_CTL: case CS42L42_SFTRAMP_RATE: + case CS42L42_SLOW_START_ENABLE: case CS42L42_I2C_DEBOUNCE: case CS42L42_I2C_STRETCH: case CS42L42_I2C_TIMEOUT: @@ -408,6 +410,30 @@ static const struct regmap_config cs42l42_regmap = { static DECLARE_TLV_DB_SCALE(adc_tlv, -9700, 100, true); static DECLARE_TLV_DB_SCALE(mixer_tlv, -6300, 100, true);
+static int cs42l42_slow_start_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); + u8 val; + + /* all bits of SLOW_START_EN must be 1 to enable */ + switch (ucontrol->value.integer.value[0]) { + case 0: + val = 0; + break; + case 1: + val = CS42L42_SLOW_START_EN_MASK; + break; + default: + return -EINVAL; + } + + snd_soc_component_update_bits(component, CS42L42_SLOW_START_ENABLE, + CS42L42_SLOW_START_EN_MASK, val); + + return 0; +} + static const char * const cs42l42_hpf_freq_text[] = { "1.86Hz", "120Hz", "235Hz", "466Hz" }; @@ -461,7 +487,12 @@ static const struct snd_kcontrol_new cs42l42_snd_controls[] = { SOC_ENUM("HP Volume Scale", cs42l42_full_scale_vol_enum), SOC_DOUBLE_R_TLV("Mixer Volume", CS42L42_MIXER_CHA_VOL, CS42L42_MIXER_CHB_VOL, CS42L42_MIXER_CH_VOL_SHIFT, - 0x3f, 1, mixer_tlv) + 0x3f, 1, mixer_tlv), + + /* Global */ + SOC_SINGLE_EXT("Slow Start Switch", CS42L42_SLOW_START_ENABLE, + CS42L42_SLOW_START_EN_SHIFT, true, false, + snd_soc_get_volsw, cs42l42_slow_start_put), };
static int cs42l42_hp_adc_ev(struct snd_soc_dapm_widget *w, diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h index b10796d755ae..85ba1d846072 100644 --- a/sound/soc/codecs/cs42l42.h +++ b/sound/soc/codecs/cs42l42.h @@ -62,6 +62,9 @@ #define CS42L42_INTERNAL_FS_MASK (1 << CS42L42_INTERNAL_FS_SHIFT)
#define CS42L42_SFTRAMP_RATE (CS42L42_PAGE_10 + 0x0A) +#define CS42L42_SLOW_START_ENABLE (CS42L42_PAGE_10 + 0x0B) +#define CS42L42_SLOW_START_EN_MASK GENMASK(6, 4) +#define CS42L42_SLOW_START_EN_SHIFT 4 #define CS42L42_I2C_DEBOUNCE (CS42L42_PAGE_10 + 0x0E) #define CS42L42_I2C_STRETCH (CS42L42_PAGE_10 + 0x0F) #define CS42L42_I2C_TIMEOUT (CS42L42_PAGE_10 + 0x10)
participants (2)
-
Mark Brown -
Richard Fitzgerald