All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: <broonie@kernel.org>
Cc: <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
	<patches@opensource.cirrus.com>,
	Richard Fitzgerald <rf@opensource.cirrus.com>
Subject: [PATCH] ASoC: cs42l42: Add warnings about DETECT_MODE and PLL_START
Date: Fri, 4 Mar 2022 14:40:15 +0000	[thread overview]
Message-ID: <20220304144015.398656-1-rf@opensource.cirrus.com> (raw)

DETECT_MODE and PLL_START must be zero while HP_PDN and ADC_PDN are
both 1. If this condition is broken it can discharge FILT+ and it
can then take up to 1 second for FILT+ to recharge.

There is no workaround required for this, simply avoiding settings
and sequences that would break the requirement. The driver already
meets the requirement.

But it is not obvious from reading the code that this requirement
exists, or what is ensuring it is met. So it would not currently be
obvious to someone changing the code that there is certain special
behaviour that must be maintained.

To avoid accidental breakage in the future:

- Add comments into the register definitions to warn about this so
  that anyone changing the code around DETECT_MODE and PLL_START is
  aware of this requirement.

- Add a comment where PLL_START is written to 1 to highlight the
  requirement and why it is satisfied.

- Add a comment in cs42l42_setup_hs_type_detect() when DETECT_MODE is
  initialized.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 sound/soc/codecs/cs42l42.c | 13 ++++++++++++-
 sound/soc/codecs/cs42l42.h |  9 ++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c
index db6ef6cdce15..c8409d50e934 100644
--- a/sound/soc/codecs/cs42l42.c
+++ b/sound/soc/codecs/cs42l42.c
@@ -1012,7 +1012,14 @@ static int cs42l42_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
 		}
 	} else {
 		if (!cs42l42->stream_use) {
-			/* SCLK must be running before codec unmute */
+			/* SCLK must be running before codec unmute.
+			 *
+			 * PLL must not be started with ADC and HP both off
+			 * otherwise the FILT+ supply will not charge properly.
+			 * DAPM widgets power-up before stream unmute so at least
+			 * one of the "DAC" or "ADC" widgets will already have
+			 * powered-up.
+			 */
 			if (pll_ratio_table[cs42l42->pll_config].mclk_src_sel) {
 				snd_soc_component_update_bits(component, CS42L42_PLL_CTL1,
 							      CS42L42_PLL_START_MASK, 1);
@@ -1830,6 +1837,10 @@ static void cs42l42_setup_hs_type_detect(struct cs42l42_private *cs42l42)
 
 	cs42l42->hs_type = CS42L42_PLUG_INVALID;
 
+	/*
+	 * DETECT_MODE must always be 0 with ADC and HP both off otherwise the
+	 * FILT+ supply will not charge properly.
+	 */
 	regmap_update_bits(cs42l42->regmap, CS42L42_MISC_DET_CTL,
 			   CS42L42_DETECT_MODE_MASK, 0);
 
diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h
index 244b24d1f5e9..60d3bdf5d7c9 100644
--- a/sound/soc/codecs/cs42l42.h
+++ b/sound/soc/codecs/cs42l42.h
@@ -491,7 +491,10 @@
 #define CS42L42_TS_UNPLUG		0
 #define CS42L42_TS_TRANS		1
 
-/* Page 0x15 Fractional-N PLL Registers */
+/*
+ * NOTE: PLL_START must be 0 while both ADC_PDN=1 and HP_PDN=1.
+ * Otherwise it will prevent FILT+ from charging properly.
+ */
 #define CS42L42_PLL_CTL1		(CS42L42_PAGE_15 + 0x01)
 #define CS42L42_PLL_START_SHIFT		0
 #define CS42L42_PLL_START_MASK		(1 << CS42L42_PLL_START_SHIFT)
@@ -574,6 +577,10 @@
 #define CS42L42_TIP_SENSE_CTRL_MASK		(3 << \
 					CS42L42_TIP_SENSE_CTRL_SHIFT)
 
+/*
+ * NOTE: DETECT_MODE must be 0 while both ADC_PDN=1 and HP_PDN=1.
+ * Otherwise it will prevent FILT+ from charging properly.
+ */
 #define CS42L42_MISC_DET_CTL		(CS42L42_PAGE_1B + 0x74)
 #define CS42L42_PDN_MIC_LVL_DET_SHIFT	0
 #define CS42L42_PDN_MIC_LVL_DET_MASK	(1 << CS42L42_PDN_MIC_LVL_DET_SHIFT)
-- 
2.30.2


WARNING: multiple messages have this Message-ID (diff)
From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: <broonie@kernel.org>
Cc: patches@opensource.cirrus.com, alsa-devel@alsa-project.org,
	Richard Fitzgerald <rf@opensource.cirrus.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] ASoC: cs42l42: Add warnings about DETECT_MODE and PLL_START
Date: Fri, 4 Mar 2022 14:40:15 +0000	[thread overview]
Message-ID: <20220304144015.398656-1-rf@opensource.cirrus.com> (raw)

DETECT_MODE and PLL_START must be zero while HP_PDN and ADC_PDN are
both 1. If this condition is broken it can discharge FILT+ and it
can then take up to 1 second for FILT+ to recharge.

There is no workaround required for this, simply avoiding settings
and sequences that would break the requirement. The driver already
meets the requirement.

But it is not obvious from reading the code that this requirement
exists, or what is ensuring it is met. So it would not currently be
obvious to someone changing the code that there is certain special
behaviour that must be maintained.

To avoid accidental breakage in the future:

- Add comments into the register definitions to warn about this so
  that anyone changing the code around DETECT_MODE and PLL_START is
  aware of this requirement.

- Add a comment where PLL_START is written to 1 to highlight the
  requirement and why it is satisfied.

- Add a comment in cs42l42_setup_hs_type_detect() when DETECT_MODE is
  initialized.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 sound/soc/codecs/cs42l42.c | 13 ++++++++++++-
 sound/soc/codecs/cs42l42.h |  9 ++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c
index db6ef6cdce15..c8409d50e934 100644
--- a/sound/soc/codecs/cs42l42.c
+++ b/sound/soc/codecs/cs42l42.c
@@ -1012,7 +1012,14 @@ static int cs42l42_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
 		}
 	} else {
 		if (!cs42l42->stream_use) {
-			/* SCLK must be running before codec unmute */
+			/* SCLK must be running before codec unmute.
+			 *
+			 * PLL must not be started with ADC and HP both off
+			 * otherwise the FILT+ supply will not charge properly.
+			 * DAPM widgets power-up before stream unmute so at least
+			 * one of the "DAC" or "ADC" widgets will already have
+			 * powered-up.
+			 */
 			if (pll_ratio_table[cs42l42->pll_config].mclk_src_sel) {
 				snd_soc_component_update_bits(component, CS42L42_PLL_CTL1,
 							      CS42L42_PLL_START_MASK, 1);
@@ -1830,6 +1837,10 @@ static void cs42l42_setup_hs_type_detect(struct cs42l42_private *cs42l42)
 
 	cs42l42->hs_type = CS42L42_PLUG_INVALID;
 
+	/*
+	 * DETECT_MODE must always be 0 with ADC and HP both off otherwise the
+	 * FILT+ supply will not charge properly.
+	 */
 	regmap_update_bits(cs42l42->regmap, CS42L42_MISC_DET_CTL,
 			   CS42L42_DETECT_MODE_MASK, 0);
 
diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h
index 244b24d1f5e9..60d3bdf5d7c9 100644
--- a/sound/soc/codecs/cs42l42.h
+++ b/sound/soc/codecs/cs42l42.h
@@ -491,7 +491,10 @@
 #define CS42L42_TS_UNPLUG		0
 #define CS42L42_TS_TRANS		1
 
-/* Page 0x15 Fractional-N PLL Registers */
+/*
+ * NOTE: PLL_START must be 0 while both ADC_PDN=1 and HP_PDN=1.
+ * Otherwise it will prevent FILT+ from charging properly.
+ */
 #define CS42L42_PLL_CTL1		(CS42L42_PAGE_15 + 0x01)
 #define CS42L42_PLL_START_SHIFT		0
 #define CS42L42_PLL_START_MASK		(1 << CS42L42_PLL_START_SHIFT)
@@ -574,6 +577,10 @@
 #define CS42L42_TIP_SENSE_CTRL_MASK		(3 << \
 					CS42L42_TIP_SENSE_CTRL_SHIFT)
 
+/*
+ * NOTE: DETECT_MODE must be 0 while both ADC_PDN=1 and HP_PDN=1.
+ * Otherwise it will prevent FILT+ from charging properly.
+ */
 #define CS42L42_MISC_DET_CTL		(CS42L42_PAGE_1B + 0x74)
 #define CS42L42_PDN_MIC_LVL_DET_SHIFT	0
 #define CS42L42_PDN_MIC_LVL_DET_MASK	(1 << CS42L42_PDN_MIC_LVL_DET_SHIFT)
-- 
2.30.2


             reply	other threads:[~2022-03-04 14:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04 14:40 Richard Fitzgerald [this message]
2022-03-04 14:40 ` [PATCH] ASoC: cs42l42: Add warnings about DETECT_MODE and PLL_START Richard Fitzgerald
2022-03-07 20:39 ` Mark Brown
2022-03-07 20:39   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220304144015.398656-1-rf@opensource.cirrus.com \
    --to=rf@opensource.cirrus.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.