[PATCH 0/5] Minor default jack pop performance updates

[PATCH 0/5] Minor default jack pop performance updates

[Charles Keepax]

Some small updates to the driver defaults to ensure a good pop
performance on jack insert and removal.

Thanks,
Charles

Charles Keepax (5):
  dt-bindings: ASoC: cirrus,cs42l43: Update a couple of default values
  ASoC: cs42l43: Lower default type detect time
  ASoC: cs42l43: Enable bias sense by default
  ASoC: cs42l43: Move headset bias sense enable earlier in process
  ASoC: cs42l43: Extend timeout on bias sense timeout

 .../bindings/sound/cirrus,cs42l43.yaml        |  4 +-
 sound/soc/codecs/cs42l43-jack.c               | 38 +++++++++----------
 2 files changed, 21 insertions(+), 21 deletions(-)

-- 
2.39.2

[PATCH 1/5] dt-bindings: ASoC: cirrus,cs42l43: Update a couple of default values

[Charles Keepax]

The bias sense is being enabled by default in the driver, and the
default detect time is being dropped slightly. Update the binding
document to match.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml
index 7a6de938b11d1..4fa22fa70ace5 100644
--- a/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml
+++ b/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml
@@ -83,7 +83,7 @@ properties:
       Current at which the headset micbias sense clamp will engage, 0 to
       disable.
     enum: [ 0, 14, 23, 41, 50, 60, 68, 86, 95 ]
-    default: 0
+    default: 14
 
   cirrus,bias-ramp-ms:
     description:
@@ -97,7 +97,7 @@ properties:
       Time in microseconds the type detection will run for. Long values will
       cause more audible effects, but give more accurate detection.
     enum: [ 20, 100, 1000, 10000, 50000, 75000, 100000, 200000 ]
-    default: 10000
+    default: 1000
 
   cirrus,button-automute:
     type: boolean
-- 
2.39.2

Re: [PATCH 1/5] dt-bindings: ASoC: cirrus,cs42l43: Update a couple of default values

[Rob Herring]

On Tue, Sep 19, 2023 at 11:31:12AM +0100, Charles Keepax wrote:
> The bias sense is being enabled by default in the driver, and the
> default detect time is being dropped slightly. Update the binding
> document to match.

That's not really a compatible change. If I wrote my DT expecting bias 
sense was disabled by default then the OS changes behavior, my 
platform behavior would change. Maybe that doesn't matter here? IDK. 
It's on you if this breaks anyone, so:

Acked-by: Rob Herring <robh@kernel.org>

> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>  Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml
> index 7a6de938b11d1..4fa22fa70ace5 100644
> --- a/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml
> +++ b/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml
> @@ -83,7 +83,7 @@ properties:
>        Current at which the headset micbias sense clamp will engage, 0 to
>        disable.
>      enum: [ 0, 14, 23, 41, 50, 60, 68, 86, 95 ]
> -    default: 0
> +    default: 14
>  
>    cirrus,bias-ramp-ms:
>      description:
> @@ -97,7 +97,7 @@ properties:
>        Time in microseconds the type detection will run for. Long values will
>        cause more audible effects, but give more accurate detection.
>      enum: [ 20, 100, 1000, 10000, 50000, 75000, 100000, 200000 ]
> -    default: 10000
> +    default: 1000
>  
>    cirrus,button-automute:
>      type: boolean
> -- 
> 2.39.2
> 

Re: [PATCH 1/5] dt-bindings: ASoC: cirrus,cs42l43: Update a couple of default values

[Charles Keepax]

On Tue, Sep 19, 2023 at 02:23:02PM -0500, Rob Herring wrote:
> On Tue, Sep 19, 2023 at 11:31:12AM +0100, Charles Keepax wrote:
> > The bias sense is being enabled by default in the driver, and the
> > default detect time is being dropped slightly. Update the binding
> > document to match.
> 
> That's not really a compatible change. If I wrote my DT expecting bias 
> sense was disabled by default then the OS changes behavior, my 
> platform behavior would change. Maybe that doesn't matter here? IDK. 
> It's on you if this breaks anyone, so:
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 

Yeah I appreciate that, but this should be fine, the part is very
very new. The only systems using the part are still in development.

Thanks,
Charles

[PATCH 2/5] ASoC: cs42l43: Lower default type detect time

[Charles Keepax]

The current default is a little excessive, reduce the pop on insertion
by reducing the time a little. The new value of 1000uS is still pretty
conservative.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/codecs/cs42l43-jack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c
index 92e37bc1df9dc..7008e50eded96 100644
--- a/sound/soc/codecs/cs42l43-jack.c
+++ b/sound/soc/codecs/cs42l43-jack.c
@@ -110,7 +110,7 @@ int cs42l43_set_jack(struct snd_soc_component *component,
 		priv->buttons[3] = 735;
 	}
 
-	ret = cs42l43_find_index(priv, "cirrus,detect-us", 10000, &priv->detect_us,
+	ret = cs42l43_find_index(priv, "cirrus,detect-us", 1000, &priv->detect_us,
 				 cs42l43_accdet_us, ARRAY_SIZE(cs42l43_accdet_us));
 	if (ret < 0)
 		goto error;
-- 
2.39.2

[PATCH 3/5] ASoC: cs42l43: Enable bias sense by default

[Charles Keepax]

Improve the default pop performance on jack removal by enabling bias
sense on the least sensitive level by default.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/codecs/cs42l43-jack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c
index 7008e50eded96..7bd7cc1779506 100644
--- a/sound/soc/codecs/cs42l43-jack.c
+++ b/sound/soc/codecs/cs42l43-jack.c
@@ -127,7 +127,7 @@ int cs42l43_set_jack(struct snd_soc_component *component,
 
 	hs2 |= ret << CS42L43_HSBIAS_RAMP_SHIFT;
 
-	ret = cs42l43_find_index(priv, "cirrus,bias-sense-microamp", 0,
+	ret = cs42l43_find_index(priv, "cirrus,bias-sense-microamp", 14,
 				 &priv->bias_sense_ua, cs42l43_accdet_bias_sense,
 				 ARRAY_SIZE(cs42l43_accdet_bias_sense));
 	if (ret < 0)
-- 
2.39.2

[PATCH 4/5] ASoC: cs42l43: Move headset bias sense enable earlier in process

[Charles Keepax]

Currently the bias sense is enabled along with the button detect, but
this has two problems. Firstly, the detections themselves arn't covered
by the bias sense, potentially resulting in pops and secondly, the
sequence of enabling/disabling looks like:

enable bias
enable bias sense
disable bias sense
disable bias

When the bias sense is disabled but the bias is still on the clamp is
removed and a pop results. Fix both of these issues by moving the bias
sense enable/disable to be along with the bias itself. With a resulting
sequence of:

enable bias sense
enable bias
disable bias
disable bias sense

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/codecs/cs42l43-jack.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c
index 7bd7cc1779506..66923cf2fdaff 100644
--- a/sound/soc/codecs/cs42l43-jack.c
+++ b/sound/soc/codecs/cs42l43-jack.c
@@ -250,6 +250,15 @@ static void cs42l43_start_hs_bias(struct cs42l43_codec *priv, bool force_high)
 	if (!force_high && priv->bias_low)
 		val = 0x2 << CS42L43_HSBIAS_MODE_SHIFT;
 
+	if (priv->bias_sense_ua) {
+		regmap_update_bits(cs42l43->regmap,
+				   CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL,
+				   CS42L43_HSBIAS_SENSE_EN_MASK |
+				   CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK,
+				   CS42L43_HSBIAS_SENSE_EN_MASK |
+				   CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK);
+	}
+
 	regmap_update_bits(cs42l43->regmap, CS42L43_MIC_DETECT_CONTROL_1,
 			   CS42L43_HSBIAS_MODE_MASK, val);
 
@@ -267,6 +276,13 @@ static void cs42l43_stop_hs_bias(struct cs42l43_codec *priv)
 
 	regmap_update_bits(cs42l43->regmap, CS42L43_HS2,
 			   CS42L43_HS_CLAMP_DISABLE_MASK, 0);
+
+	if (priv->bias_sense_ua) {
+		regmap_update_bits(cs42l43->regmap,
+				   CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL,
+				   CS42L43_HSBIAS_SENSE_EN_MASK |
+				   CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK, 0);
+	}
 }
 
 irqreturn_t cs42l43_bias_detect_clamp(int irq, void *data)
@@ -318,15 +334,6 @@ static void cs42l43_start_button_detect(struct cs42l43_codec *priv)
 	regmap_update_bits(cs42l43->regmap, CS42L43_MIC_DETECT_CONTROL_1,
 			   CS42L43_BUTTON_DETECT_MODE_MASK |
 			   CS42L43_MIC_LVL_DET_DISABLE_MASK, val);
-
-	if (priv->bias_sense_ua) {
-		regmap_update_bits(cs42l43->regmap,
-				   CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL,
-				   CS42L43_HSBIAS_SENSE_EN_MASK |
-				   CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK,
-				   CS42L43_HSBIAS_SENSE_EN_MASK |
-				   CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK);
-	}
 }
 
 static void cs42l43_stop_button_detect(struct cs42l43_codec *priv)
@@ -335,13 +342,6 @@ static void cs42l43_stop_button_detect(struct cs42l43_codec *priv)
 
 	dev_dbg(priv->dev, "Stop button detect\n");
 
-	if (priv->bias_sense_ua) {
-		regmap_update_bits(cs42l43->regmap,
-				   CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL,
-				   CS42L43_HSBIAS_SENSE_EN_MASK |
-				   CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK, 0);
-	}
-
 	regmap_update_bits(cs42l43->regmap, CS42L43_MIC_DETECT_CONTROL_1,
 			   CS42L43_BUTTON_DETECT_MODE_MASK |
 			   CS42L43_MIC_LVL_DET_DISABLE_MASK,
-- 
2.39.2

[PATCH 5/5] ASoC: cs42l43: Extend timeout on bias sense timeout

[Charles Keepax]

For very slow removals the current bias sense timeout is sometimes too
short and unclamps the mic bias before the jack removal is properly
detected by the tip detect, causing a pop. As bias sense should be
tuned to deliver very few false positives, increase the timeout fairly
dramatically to cover all but the most exaggerated removals.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/codecs/cs42l43-jack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c
index 66923cf2fdaff..861f9ee671cdf 100644
--- a/sound/soc/codecs/cs42l43-jack.c
+++ b/sound/soc/codecs/cs42l43-jack.c
@@ -290,7 +290,7 @@ irqreturn_t cs42l43_bias_detect_clamp(int irq, void *data)
 	struct cs42l43_codec *priv = data;
 
 	queue_delayed_work(system_wq, &priv->bias_sense_timeout,
-			   msecs_to_jiffies(250));
+			   msecs_to_jiffies(1000));
 
 	return IRQ_HANDLED;
 }
-- 
2.39.2

Re: [PATCH 0/5] Minor default jack pop performance updates

[Mark Brown]

On Tue, 19 Sep 2023 11:31:11 +0100, Charles Keepax wrote:
> Some small updates to the driver defaults to ensure a good pop
> performance on jack insert and removal.
> 
> Thanks,
> Charles
> 
> Charles Keepax (5):
>   dt-bindings: ASoC: cirrus,cs42l43: Update a couple of default values
>   ASoC: cs42l43: Lower default type detect time
>   ASoC: cs42l43: Enable bias sense by default
>   ASoC: cs42l43: Move headset bias sense enable earlier in process
>   ASoC: cs42l43: Extend timeout on bias sense timeout
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] dt-bindings: ASoC: cirrus,cs42l43: Update a couple of default values
      commit: aa7627111c689f9dc2458c7dd9c1fbb554502664
[2/5] ASoC: cs42l43: Lower default type detect time
      commit: 686b8f711b990d895d39dee3fab88861917d2dc4
[3/5] ASoC: cs42l43: Enable bias sense by default
      commit: 9c0ccc9f8e3be79ab44f5f8034ef90c367caf06f
[4/5] ASoC: cs42l43: Move headset bias sense enable earlier in process
      commit: 1e4ce0d5c023e8d8663f6b79b98b9f8026776127
[5/5] ASoC: cs42l43: Extend timeout on bias sense timeout
      commit: 6388a0619c83625214e98377c32bcefa4fffb9cd

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