[alsa-devel] [PATCH] ASoC: ssm2602: Fix ADC powerup sequencing
From: Philipp Zabel p.zabel@pengutronix.de
According to the ssm2603 data sheet (control register sequencing), the digital core should be activated only after all necessary bits in the power register are enabled, and a delay determined by the decoupling capacitor on the VMID pin has passed. If the digital core is activated too early, or even before the ADC is powered up, audible artifacts appear at the beginning of the recorded signal.
The digital core is also needed for playback, so when recording starts it may already be enabled. This means we cannot get the power sequence correct when we want to be able to start recording after playback.
As a workaround put the MIC mute switch into the DAPM routes. This way we can keep the recording disabled until the MIC Bias has settled and thus get rid of audible artifacts.
The delay we have to wait depends on a board specific capacitor connected to the VMID pins, so make the delay configurable in the device tree.
Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- .../devicetree/bindings/sound/adi,ssm2602.txt | 7 +++++ sound/soc/codecs/ssm2602.c | 30 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt index 3b3302fe399b..9334d48c0462 100644 --- a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt +++ b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt @@ -11,9 +11,16 @@ Required properties: - reg : the I2C address of the device for I2C, the chip select number for SPI.
+Optional properties: + + - startup-delay-us : delay between powering on and activating the digital + core, determined by the decoupling capacitor C on the + VMID pin: delay [µs] = C [µF] * 25000 / 3.5 + Example:
ssm2602: ssm2602@1a { compatible = "adi,ssm2602"; reg = <0x1a>; + startup-delay-us = <34000>; /* 4.7 µF * 25000 / 3.5 -> ~34 ms */ }; diff --git a/sound/soc/codecs/ssm2602.c b/sound/soc/codecs/ssm2602.c index 501a4e73b185..1b99f5d44f8d 100644 --- a/sound/soc/codecs/ssm2602.c +++ b/sound/soc/codecs/ssm2602.c @@ -26,9 +26,11 @@ * 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
+#include <linux/delay.h> #include <linux/module.h> #include <linux/regmap.h> #include <linux/slab.h> +#include <linux/of.h>
#include <sound/pcm.h> #include <sound/pcm_params.h> @@ -46,6 +48,7 @@ struct ssm2602_priv {
enum ssm2602_type type; unsigned int clk_out_pwr; + u32 startup_delay_us; };
/* @@ -111,7 +114,6 @@ SOC_SINGLE_TLV("Sidetone Playback Volume", SSM2602_APANA, 6, 3, 1,
SOC_SINGLE("Mic Boost (+20dB)", SSM2602_APANA, 0, 1, 0), SOC_SINGLE("Mic Boost2 (+20dB)", SSM2602_APANA, 8, 1, 0), -SOC_SINGLE("Mic Switch", SSM2602_APANA, 1, 1, 1), };
/* Output Mixer */ @@ -121,10 +123,26 @@ SOC_DAPM_SINGLE("HiFi Playback Switch", SSM2602_APANA, 4, 1, 0), SOC_DAPM_SINGLE("Mic Sidetone Switch", SSM2602_APANA, 5, 1, 0), };
+static const struct snd_kcontrol_new mic_ctl = + SOC_DAPM_SINGLE("Switch", SSM2602_APANA, 1, 1, 1); + /* Input mux */ static const struct snd_kcontrol_new ssm2602_input_mux_controls = SOC_DAPM_ENUM("Input Select", ssm2602_enum[0]);
+static int ssm2602_mic_switch_event(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm); + struct ssm2602_priv *ssm2602 = snd_soc_codec_get_drvdata(codec); + + if (ssm2602->startup_delay_us) + usleep_range(ssm2602->startup_delay_us, + ssm2602->startup_delay_us * 2); + + return 0; +} + static const struct snd_soc_dapm_widget ssm260x_dapm_widgets[] = { SND_SOC_DAPM_DAC("DAC", "HiFi Playback", SSM2602_PWR, 3, 1), SND_SOC_DAPM_ADC("ADC", "HiFi Capture", SSM2602_PWR, 2, 1), @@ -146,6 +164,9 @@ SND_SOC_DAPM_MIXER("Output Mixer", SSM2602_PWR, 4, 1, SND_SOC_DAPM_MUX("Input Mux", SND_SOC_NOPM, 0, 0, &ssm2602_input_mux_controls), SND_SOC_DAPM_MICBIAS("Mic Bias", SSM2602_PWR, 1, 1),
+SND_SOC_DAPM_SWITCH_E("Mic Switch", SSM2602_APANA, 1, 1, &mic_ctl, + ssm2602_mic_switch_event, SND_SOC_DAPM_PRE_PMU), + SND_SOC_DAPM_OUTPUT("LHPOUT"), SND_SOC_DAPM_OUTPUT("RHPOUT"), SND_SOC_DAPM_INPUT("MICIN"), @@ -178,9 +199,11 @@ static const struct snd_soc_dapm_route ssm2602_routes[] = { {"LHPOUT", NULL, "Output Mixer"},
{"Input Mux", "Line", "Line Input"}, - {"Input Mux", "Mic", "Mic Bias"}, + {"Input Mux", "Mic", "Mic Switch"}, {"ADC", NULL, "Input Mux"},
+ {"Mic Switch", NULL, "Mic Bias"}, + {"Mic Bias", NULL, "MICIN"}, };
@@ -645,6 +668,9 @@ int ssm2602_probe(struct device *dev, enum ssm2602_type type, if (ssm2602 == NULL) return -ENOMEM;
+ of_property_read_u32(dev->of_node, "startup-delay-us", + &ssm2602->startup_delay_us); + dev_set_drvdata(dev, ssm2602); ssm2602->type = type; ssm2602->regmap = regmap;
On Thu, May 17, 2018 at 8:30 AM, Marco Felsch m.felsch@pengutronix.de wrote:
From: Philipp Zabel p.zabel@pengutronix.de
According to the ssm2603 data sheet (control register sequencing), the digital core should be activated only after all necessary bits in the power register are enabled, and a delay determined by the decoupling capacitor on the VMID pin has passed. If the digital core is activated too early, or even before the ADC is powered up, audible artifacts appear at the beginning of the recorded signal.
The digital core is also needed for playback, so when recording starts it may already be enabled. This means we cannot get the power sequence correct when we want to be able to start recording after playback.
As a workaround put the MIC mute switch into the DAPM routes. This way we can keep the recording disabled until the MIC Bias has settled and thus get rid of audible artifacts.
The delay we have to wait depends on a board specific capacitor connected to the VMID pins, so make the delay configurable in the device tree.
Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Marco Felsch m.felsch@pengutronix.de
.../devicetree/bindings/sound/adi,ssm2602.txt | 7 +++++ sound/soc/codecs/ssm2602.c | 30 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt index 3b3302fe399b..9334d48c0462 100644 --- a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt +++ b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt @@ -11,9 +11,16 @@ Required properties:
- reg : the I2C address of the device for I2C, the chip select number for SPI.
+Optional properties:
- startup-delay-us : delay between powering on and activating the digital
core, determined by the decoupling capacitor C on theVMID pin: delay [µs] = C [µF] * 25000 / 3.5
We already have similarly defined property. Please reuse that. See mmc pwrseq binding.
Also, please split bindings to separate patch.
Rob
Hi Rob,
On Thu, 2018-05-17 at 11:14 -0500, Rob Herring wrote:
On Thu, May 17, 2018 at 8:30 AM, Marco Felsch m.felsch@pengutronix.de wrote:
From: Philipp Zabel p.zabel@pengutronix.de
According to the ssm2603 data sheet (control register sequencing), the digital core should be activated only after all necessary bits in the power register are enabled, and a delay determined by the decoupling capacitor on the VMID pin has passed. If the digital core is activated too early, or even before the ADC is powered up, audible artifacts appear at the beginning of the recorded signal.
The digital core is also needed for playback, so when recording starts it may already be enabled. This means we cannot get the power sequence correct when we want to be able to start recording after playback.
As a workaround put the MIC mute switch into the DAPM routes. This way we can keep the recording disabled until the MIC Bias has settled and thus get rid of audible artifacts.
The delay we have to wait depends on a board specific capacitor connected to the VMID pins, so make the delay configurable in the device tree.
Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Marco Felsch m.felsch@pengutronix.de
.../devicetree/bindings/sound/adi,ssm2602.txt | 7 +++++ sound/soc/codecs/ssm2602.c | 30 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt index 3b3302fe399b..9334d48c0462 100644 --- a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt +++ b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt @@ -11,9 +11,16 @@ Required properties:
- reg : the I2C address of the device for I2C, the chip select number for SPI.
+Optional properties:
- startup-delay-us : delay between powering on and activating the digital
core, determined by the decoupling capacitor C on theVMID pin: delay [µs] = C [µF] * 25000 / 3.5We already have similarly defined property. Please reuse that. See mmc pwrseq binding.
Do you mean 'post-power-on-delay-ms' from 'mmc-pwseq-simple'? It is documented as:
- post-power-on-delay-ms : Delay in ms after powering the card and de-asserting the reset-gpios (if any)
The startup delay here is not after powering the whole IC or deasserting resets and before it can be used, but after powering up a specific part of the codec (the ADC) and before unmuting the MIC input to the digital core during start of decoding. With this in mind, do you still think the property should be called the same as the mmc full-chip poweron delay? If so, would it be acceptable to use post-power-on-delay-us to keep the microsecond resolution?
thanks Philipp
On Fri, May 18, 2018 at 12:46:49PM +0200, Philipp Zabel wrote:
Hi Rob,
On Thu, 2018-05-17 at 11:14 -0500, Rob Herring wrote:
On Thu, May 17, 2018 at 8:30 AM, Marco Felsch m.felsch@pengutronix.de wrote:
From: Philipp Zabel p.zabel@pengutronix.de
According to the ssm2603 data sheet (control register sequencing), the digital core should be activated only after all necessary bits in the power register are enabled, and a delay determined by the decoupling capacitor on the VMID pin has passed. If the digital core is activated too early, or even before the ADC is powered up, audible artifacts appear at the beginning of the recorded signal.
The digital core is also needed for playback, so when recording starts it may already be enabled. This means we cannot get the power sequence correct when we want to be able to start recording after playback.
As a workaround put the MIC mute switch into the DAPM routes. This way we can keep the recording disabled until the MIC Bias has settled and thus get rid of audible artifacts.
The delay we have to wait depends on a board specific capacitor connected to the VMID pins, so make the delay configurable in the device tree.
Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Marco Felsch m.felsch@pengutronix.de
.../devicetree/bindings/sound/adi,ssm2602.txt | 7 +++++ sound/soc/codecs/ssm2602.c | 30 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt index 3b3302fe399b..9334d48c0462 100644 --- a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt +++ b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt @@ -11,9 +11,16 @@ Required properties:
- reg : the I2C address of the device for I2C, the chip select number for SPI.
+Optional properties:
- startup-delay-us : delay between powering on and activating the digital
core, determined by the decoupling capacitor C on theVMID pin: delay [µs] = C [µF] * 25000 / 3.5We already have similarly defined property. Please reuse that. See mmc pwrseq binding.
Do you mean 'post-power-on-delay-ms' from 'mmc-pwseq-simple'? It is documented as:
- post-power-on-delay-ms : Delay in ms after powering the card and
de-asserting the reset-gpios (if any)
The startup delay here is not after powering the whole IC or deasserting resets and before it can be used, but after powering up a specific part of the codec (the ADC) and before unmuting the MIC input to the digital core during start of decoding. With this in mind, do you still think the property should be called the same as the mmc full-chip poweron delay? If so, would it be acceptable to use post-power-on-delay-us to keep the microsecond resolution?
Okay, then I'd suggest something a bit more specific. Perhaps "pre-unmute-delay-us" and document in a common location.
Rob
Hi Rob,
On 18-05-23 11:53, Rob Herring wrote:
On Fri, May 18, 2018 at 12:46:49PM +0200, Philipp Zabel wrote:
Hi Rob,
On Thu, 2018-05-17 at 11:14 -0500, Rob Herring wrote:
On Thu, May 17, 2018 at 8:30 AM, Marco Felsch m.felsch@pengutronix.de wrote:
From: Philipp Zabel p.zabel@pengutronix.de
According to the ssm2603 data sheet (control register sequencing), the digital core should be activated only after all necessary bits in the power register are enabled, and a delay determined by the decoupling capacitor on the VMID pin has passed. If the digital core is activated too early, or even before the ADC is powered up, audible artifacts appear at the beginning of the recorded signal.
The digital core is also needed for playback, so when recording starts it may already be enabled. This means we cannot get the power sequence correct when we want to be able to start recording after playback.
As a workaround put the MIC mute switch into the DAPM routes. This way we can keep the recording disabled until the MIC Bias has settled and thus get rid of audible artifacts.
The delay we have to wait depends on a board specific capacitor connected to the VMID pins, so make the delay configurable in the device tree.
Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Marco Felsch m.felsch@pengutronix.de
.../devicetree/bindings/sound/adi,ssm2602.txt | 7 +++++ sound/soc/codecs/ssm2602.c | 30 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt index 3b3302fe399b..9334d48c0462 100644 --- a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt +++ b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt @@ -11,9 +11,16 @@ Required properties:
- reg : the I2C address of the device for I2C, the chip select number for SPI.
+Optional properties:
- startup-delay-us : delay between powering on and activating the digital
core, determined by the decoupling capacitor C on theVMID pin: delay [µs] = C [µF] * 25000 / 3.5We already have similarly defined property. Please reuse that. See mmc pwrseq binding.
Do you mean 'post-power-on-delay-ms' from 'mmc-pwseq-simple'? It is documented as:
- post-power-on-delay-ms : Delay in ms after powering the card and
de-asserting the reset-gpios (if any)
The startup delay here is not after powering the whole IC or deasserting resets and before it can be used, but after powering up a specific part of the codec (the ADC) and before unmuting the MIC input to the digital core during start of decoding. With this in mind, do you still think the property should be called the same as the mmc full-chip poweron delay? If so, would it be acceptable to use post-power-on-delay-us to keep the microsecond resolution?
Okay, then I'd suggest something a bit more specific. Perhaps "pre-unmute-delay-us" and document in a common location.
Rob
The delay is not just for the line-in/mic path it is also for the out path. More technical, it is needed to charge the decouple capacity which provides the voltage bias for the analog input/output frontends.
I found the: "ti,charge-period (sound/ti,tas5086.txt)" binding which represents nearly the same but it is not common. One opportunity would be to introduce a common "charge-period-us" binding and change the ti binding the common binding later.
Would that be okay?
Marco
On Fri, May 25, 2018 at 11:47:24AM +0200, Marco Felsch wrote:
I found the: "ti,charge-period (sound/ti,tas5086.txt)" binding which represents nearly the same but it is not common. One opportunity would be to introduce a common "charge-period-us" binding and change the ti binding the common binding later.
Are you sure what you have is a charge period and not some stabalization time or something?
Hi Mark,
On 18-05-25 11:26, Mark Brown wrote:
On Fri, May 25, 2018 at 11:47:24AM +0200, Marco Felsch wrote:
I found the: "ti,charge-period (sound/ti,tas5086.txt)" binding which represents nearly the same but it is not common. One opportunity would be to introduce a common "charge-period-us" binding and change the ti binding the common binding later.
Are you sure what you have is a charge period and not some stabalization time or something?
According to the data sheet: 8<----- As described in the Digital Core Clock section of the Theory of Operation, insert enough delay time to charge the VMID decoupling capacitor before setting the active bit [Register R9, Bit D0]. 8<----- I think yes, it is a charge period.
Also the formula for the delay time (t = C × 25,000/3.5) depends only on the capacity size.
Marco
On Fri, May 25, 2018 at 01:42:53PM +0200, Marco Felsch wrote:
Also the formula for the delay time (t = C × 25,000/3.5) depends only on the capacity size.
Why not just have the user specify the capacitance of the capacitor on the rail which they can directly read from the schematic rather than forcing them to do the calcualtion? That seems a bit clearer and more user friendly (plus if someone decides the spec was wrong it's easier to roll out fixes).
On Fri, 2018-05-25 at 15:52 +0100, Mark Brown wrote:
On Fri, May 25, 2018 at 01:42:53PM +0200, Marco Felsch wrote:
Also the formula for the delay time (t = C × 25,000/3.5) depends only on the capacity size.
Why not just have the user specify the capacitance of the capacitor on the rail which they can directly read from the schematic rather than forcing them to do the calcualtion? That seems a bit clearer and more user friendly (plus if someone decides the spec was wrong it's easier to roll out fixes).
The exact capacitance may not be known or vary above the nominal value because of cheap components, and the formula from the datasheet is just a guideline.
I'd expect the usual method to set this delay to be semi-empirical: "start from the value calculated from datasheet and schematics and then increase until no more audio artifacts on a representative sample of boards". I think it is be better to specify a delay that works than a bogus capacitance value that happens to correspond to a delay that works.
regards Philipp
On Fri, May 25, 2018 at 05:18:09PM +0200, Philipp Zabel wrote:
On Fri, 2018-05-25 at 15:52 +0100, Mark Brown wrote:
On Fri, May 25, 2018 at 01:42:53PM +0200, Marco Felsch wrote:
Also the formula for the delay time (t = C × 25,000/3.5) depends only on the capacity size.
Why not just have the user specify the capacitance of the capacitor on the rail which they can directly read from the schematic rather than forcing them to do the calcualtion? That seems a bit clearer and more user friendly (plus if someone decides the spec was wrong it's easier to roll out fixes).
The exact capacitance may not be known or vary above the nominal value because of cheap components, and the formula from the datasheet is just a guideline.
That variability is going to apply just as much to the charge time calculations/measurements as it is to the initial capacitance value - the results are going to be very much garbage in, garbage out.
I'd expect the usual method to set this delay to be semi-empirical: "start from the value calculated from datasheet and schematics and then increase until no more audio artifacts on a representative sample of boards". I think it is be better to specify a delay that works than a bogus capacitance value that happens to correspond to a delay that works.
If this is varying so drastically per board/system that it's relevant then we're already into problematic territory. For most devices we just have a number for the part, not something that varies so wildly that each system needs to configure it.
Hi Mark,
On Fri, 2018-05-25 at 18:24 +0100, Mark Brown wrote:
On Fri, May 25, 2018 at 05:18:09PM +0200, Philipp Zabel wrote:
On Fri, 2018-05-25 at 15:52 +0100, Mark Brown wrote:
On Fri, May 25, 2018 at 01:42:53PM +0200, Marco Felsch wrote:
Also the formula for the delay time (t = C × 25,000/3.5) depends only on the capacity size.
Why not just have the user specify the capacitance of the capacitor on the rail which they can directly read from the schematic rather than forcing them to do the calcualtion? That seems a bit clearer and more user friendly (plus if someone decides the spec was wrong it's easier to roll out fixes).
The exact capacitance may not be known or vary above the nominal value because of cheap components, and the formula from the datasheet is just a guideline.
That variability is going to apply just as much to the charge time calculations/measurements as it is to the initial capacitance value - the results are going to be very much garbage in, garbage out.
True.
I'd expect the usual method to set this delay to be semi-empirical: "start from the value calculated from datasheet and schematics and then increase until no more audio artifacts on a representative sample of boards". I think it is be better to specify a delay that works than a bogus capacitance value that happens to correspond to a delay that works.
If this is varying so drastically per board/system that it's relevant then we're already into problematic territory. For most devices we just have a number for the part, not something that varies so wildly that each system needs to configure it.
I'm not arguing this should be configured per individual device, just per board DT.
It's just that my experience of one datapoint consists of a board where I had to increase the delay to about three times the delay calculated from the nominal capacitance according to the datasheet before audible artifacts disappeared reliably on multiple devices. Putting the extended delay into the device tree with a comment explaining its empirical nature seemed more straightforward than putting a bogus capacitance value, that differs from the schematics, and that I have never measured.
Also, by using the capacitance we are guaranteed to end up with a codec specific property name (adi,vmid-decoupling-capacitance ?) as opposed to the possibility of defining a common delay property that could be used for different codecs.
regards Philipp
On Tue, Jun 05, 2018 at 11:58:51AM +0200, Philipp Zabel wrote:
On Fri, 2018-05-25 at 18:24 +0100, Mark Brown wrote:
If this is varying so drastically per board/system that it's relevant then we're already into problematic territory. For most devices we just have a number for the part, not something that varies so wildly that each system needs to configure it.
I'm not arguing this should be configured per individual device, just per board DT.
Even per board is a very surprising amount of variability TBH.
It's just that my experience of one datapoint consists of a board where I had to increase the delay to about three times the delay calculated from the nominal capacitance according to the datasheet before audible artifacts disappeared reliably on multiple devices. Putting the extended delay into the device tree with a comment explaining its empirical nature seemed more straightforward than putting a bogus capacitance value, that differs from the schematics, and that I have never measured.
The thing that's surprising me here is that there's even a board to board variance that's so bad that it needs to be configured like this - usually these things are just hard coded into the driver so they work for everyone rather than needing per-board tuning. It seems entirely possible that the formula quoted in the datasheet is just nonsense in general and that the driver would be better off using your number for all systems for example. If we do have to hard code something in the DT it seems better to hard code something that is at least coming from a spec rather than a measured number that needs to come from an average over a batch of boards since at least we can revise how we handle the number without needing to do something that shows up in the ABI.
Also, by using the capacitance we are guaranteed to end up with a codec specific property name (adi,vmid-decoupling-capacitance ?) as opposed to the possibility of defining a common delay property that could be used for different codecs.
That's actually something I want to avoid since the delays tend to need to come in the middle of write sequences which makes a general property a lot more complicated, and obviously some devices have multiple delays too.
On Fri, May 25, 2018 at 4:47 AM, Marco Felsch m.felsch@pengutronix.de wrote:
Hi Rob,
On 18-05-23 11:53, Rob Herring wrote:
On Fri, May 18, 2018 at 12:46:49PM +0200, Philipp Zabel wrote:
Hi Rob,
On Thu, 2018-05-17 at 11:14 -0500, Rob Herring wrote:
On Thu, May 17, 2018 at 8:30 AM, Marco Felsch m.felsch@pengutronix.de wrote:
From: Philipp Zabel p.zabel@pengutronix.de
According to the ssm2603 data sheet (control register sequencing), the digital core should be activated only after all necessary bits in the power register are enabled, and a delay determined by the decoupling capacitor on the VMID pin has passed. If the digital core is activated too early, or even before the ADC is powered up, audible artifacts appear at the beginning of the recorded signal.
The digital core is also needed for playback, so when recording starts it may already be enabled. This means we cannot get the power sequence correct when we want to be able to start recording after playback.
As a workaround put the MIC mute switch into the DAPM routes. This way we can keep the recording disabled until the MIC Bias has settled and thus get rid of audible artifacts.
The delay we have to wait depends on a board specific capacitor connected to the VMID pins, so make the delay configurable in the device tree.
Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Marco Felsch m.felsch@pengutronix.de
.../devicetree/bindings/sound/adi,ssm2602.txt | 7 +++++ sound/soc/codecs/ssm2602.c | 30 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt index 3b3302fe399b..9334d48c0462 100644 --- a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt +++ b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt @@ -11,9 +11,16 @@ Required properties:
- reg : the I2C address of the device for I2C, the chip select number for SPI.
+Optional properties:
- startup-delay-us : delay between powering on and activating the digital
core, determined by the decoupling capacitor C on theVMID pin: delay [µs] = C [µF] * 25000 / 3.5We already have similarly defined property. Please reuse that. See mmc pwrseq binding.
Do you mean 'post-power-on-delay-ms' from 'mmc-pwseq-simple'? It is documented as:
- post-power-on-delay-ms : Delay in ms after powering the card and de-asserting the reset-gpios (if any)
The startup delay here is not after powering the whole IC or deasserting resets and before it can be used, but after powering up a specific part of the codec (the ADC) and before unmuting the MIC input to the digital core during start of decoding. With this in mind, do you still think the property should be called the same as the mmc full-chip poweron delay? If so, would it be acceptable to use post-power-on-delay-us to keep the microsecond resolution?
Okay, then I'd suggest something a bit more specific. Perhaps "pre-unmute-delay-us" and document in a common location.
Rob
The delay is not just for the line-in/mic path it is also for the out path. More technical, it is needed to charge the decouple capacity which provides the voltage bias for the analog input/output frontends.
I found the: "ti,charge-period (sound/ti,tas5086.txt)" binding which represents nearly the same but it is not common. One opportunity would be to introduce a common "charge-period-us" binding and change the ti binding the common binding later.
Would that be okay?
Sure, sounds fine to me.
Rob
participants (5)
-
Marco Felsch -
Mark Brown -
Philipp Zabel -
Rob Herring -
Rob Herring