[PATCH v2 0/5] Add CS35L41 shared boost feature
Valve's Steam Deck uses CS35L41 in shared boost mode, where both speakers share the boost circuit. Add this support in the shared lib, but for now, shared boost is not supported in HDA systems as would require BIOS changes. Based on David Rhodes shared boost patches.
Also, fix boost config overwriting in IRQ found in the review and do a small refactor of the code.
Changes from V1: - Fix Documentation patch subject - New patch for External boost without VSPK Documentation - New patch to fix boost IRQ overwriting issue - New patch to refactor IRQ release error code - reinit_completion on pcm_startup - fix DRE switch overwriting - return IRQ_HANDLED in PLL_LOCK case
Lucas Tanure (5): ASoC: cs35l41: Only disable internal boost ASoC: cs35l41: Refactor error release code ALSA: cs35l41: Add shared boost feature ASoC: cs35l41: Document CS35l41 external boost without VSPK ASoC: cs35l41: Document CS35l41 shared boost
.../bindings/sound/cirrus,cs35l41.yaml | 12 +- include/sound/cs35l41.h | 10 +- sound/pci/hda/cs35l41_hda.c | 6 +- sound/soc/codecs/cs35l41-lib.c | 56 +++++++- sound/soc/codecs/cs35l41.c | 125 +++++++++--------- sound/soc/codecs/cs35l41.h | 1 + 6 files changed, 139 insertions(+), 71 deletions(-)
In error situations, only the internal boost case should be disabled and re-enabled. Also, for other boost cases re-enabling the boost to the default internal boost config is incorrect.
Fixes: 6450ef559056 ("ASoC: cs35l41: CS35L41 Boosted Smart Amplifier") Signed-off-by: Lucas Tanure lucas.tanure@collabora.com --- sound/soc/codecs/cs35l41.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c index c223d83e02cf..f2b5032daa6a 100644 --- a/sound/soc/codecs/cs35l41.c +++ b/sound/soc/codecs/cs35l41.c @@ -356,6 +356,19 @@ static const struct snd_kcontrol_new cs35l41_aud_controls[] = { WM_ADSP_FW_CONTROL("DSP1", 0), };
+static void cs35l41_boost_enable(struct cs35l41_private *cs35l41, unsigned int enable) +{ + switch (cs35l41->hw_cfg.bst_type) { + case CS35L41_INT_BOOST: + enable = enable ? CS35L41_BST_EN_DEFAULT : CS35L41_BST_DIS_FET_OFF; + regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL2, CS35L41_BST_EN_MASK, + enable << CS35L41_BST_EN_SHIFT); + break; + default: + break; + } +} + static irqreturn_t cs35l41_irq(int irq, void *data) { struct cs35l41_private *cs35l41 = data; @@ -431,8 +444,7 @@ static irqreturn_t cs35l41_irq(int irq, void *data)
if (status[0] & CS35L41_BST_OVP_ERR) { dev_crit_ratelimited(cs35l41->dev, "VBST Over Voltage error\n"); - regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL2, - CS35L41_BST_EN_MASK, 0); + cs35l41_boost_enable(cs35l41, 0); regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, CS35L41_BST_OVP_ERR); regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); @@ -441,16 +453,13 @@ static irqreturn_t cs35l41_irq(int irq, void *data) CS35L41_BST_OVP_ERR_RLS); regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, CS35L41_BST_OVP_ERR_RLS, 0); - regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL2, - CS35L41_BST_EN_MASK, - CS35L41_BST_EN_DEFAULT << CS35L41_BST_EN_SHIFT); + cs35l41_boost_enable(cs35l41, 1); ret = IRQ_HANDLED; }
if (status[0] & CS35L41_BST_DCM_UVP_ERR) { dev_crit_ratelimited(cs35l41->dev, "DCM VBST Under Voltage Error\n"); - regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL2, - CS35L41_BST_EN_MASK, 0); + cs35l41_boost_enable(cs35l41, 0); regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, CS35L41_BST_DCM_UVP_ERR); regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); @@ -459,16 +468,13 @@ static irqreturn_t cs35l41_irq(int irq, void *data) CS35L41_BST_UVP_ERR_RLS); regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, CS35L41_BST_UVP_ERR_RLS, 0); - regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL2, - CS35L41_BST_EN_MASK, - CS35L41_BST_EN_DEFAULT << CS35L41_BST_EN_SHIFT); + cs35l41_boost_enable(cs35l41, 1); ret = IRQ_HANDLED; }
if (status[0] & CS35L41_BST_SHORT_ERR) { dev_crit_ratelimited(cs35l41->dev, "LBST error: powering off!\n"); - regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL2, - CS35L41_BST_EN_MASK, 0); + cs35l41_boost_enable(cs35l41, 0); regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, CS35L41_BST_SHORT_ERR); regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); @@ -477,9 +483,7 @@ static irqreturn_t cs35l41_irq(int irq, void *data) CS35L41_BST_SHORT_ERR_RLS); regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, CS35L41_BST_SHORT_ERR_RLS, 0); - regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL2, - CS35L41_BST_EN_MASK, - CS35L41_BST_EN_DEFAULT << CS35L41_BST_EN_SHIFT); + cs35l41_boost_enable(cs35l41, 1); ret = IRQ_HANDLED; }
On Tue, Feb 07, 2023 at 04:25:22PM +0000, Lucas Tanure wrote:
In error situations, only the internal boost case should be disabled and re-enabled. Also, for other boost cases re-enabling the boost to the default internal boost config is incorrect.
Fixes: 6450ef559056 ("ASoC: cs35l41: CS35L41 Boosted Smart Amplifier") Signed-off-by: Lucas Tanure lucas.tanure@collabora.com
Would quite like David to review as well, but looks good to me.
Acked-by: Charles Keepax ckeepax@opensource.cirrus.com
Thanks, Charles
Add cs35l41_error_release function to handle error release sequences.
Signed-off-by: Lucas Tanure lucas.tanure@collabora.com --- sound/soc/codecs/cs35l41.c | 64 ++++++++++---------------------------- 1 file changed, 16 insertions(+), 48 deletions(-)
diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c index f2b5032daa6a..c006364e5335 100644 --- a/sound/soc/codecs/cs35l41.c +++ b/sound/soc/codecs/cs35l41.c @@ -369,6 +369,16 @@ static void cs35l41_boost_enable(struct cs35l41_private *cs35l41, unsigned int e } }
+ +static void cs35l41_error_release(struct cs35l41_private *cs35l41, unsigned int irq_err_bit, + unsigned int rel_err_bit) +{ + regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, irq_err_bit); + regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); + regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, rel_err_bit, rel_err_bit); + regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, rel_err_bit, 0); +} + static irqreturn_t cs35l41_irq(int irq, void *data) { struct cs35l41_private *cs35l41 = data; @@ -405,54 +415,26 @@ static irqreturn_t cs35l41_irq(int irq, void *data) */ if (status[0] & CS35L41_AMP_SHORT_ERR) { dev_crit_ratelimited(cs35l41->dev, "Amp short error\n"); - regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, - CS35L41_AMP_SHORT_ERR); - regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_AMP_SHORT_ERR_RLS, - CS35L41_AMP_SHORT_ERR_RLS); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_AMP_SHORT_ERR_RLS, 0); + cs35l41_error_release(cs35l41, CS35L41_AMP_SHORT_ERR, CS35L41_AMP_SHORT_ERR_RLS); ret = IRQ_HANDLED; }
if (status[0] & CS35L41_TEMP_WARN) { dev_crit_ratelimited(cs35l41->dev, "Over temperature warning\n"); - regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, - CS35L41_TEMP_WARN); - regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_TEMP_WARN_ERR_RLS, - CS35L41_TEMP_WARN_ERR_RLS); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_TEMP_WARN_ERR_RLS, 0); + cs35l41_error_release(cs35l41, CS35L41_TEMP_WARN, CS35L41_TEMP_WARN_ERR_RLS); ret = IRQ_HANDLED; }
if (status[0] & CS35L41_TEMP_ERR) { dev_crit_ratelimited(cs35l41->dev, "Over temperature error\n"); - regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, - CS35L41_TEMP_ERR); - regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_TEMP_ERR_RLS, - CS35L41_TEMP_ERR_RLS); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_TEMP_ERR_RLS, 0); + cs35l41_error_release(cs35l41, CS35L41_TEMP_ERR, CS35L41_TEMP_ERR_RLS); ret = IRQ_HANDLED; }
if (status[0] & CS35L41_BST_OVP_ERR) { dev_crit_ratelimited(cs35l41->dev, "VBST Over Voltage error\n"); cs35l41_boost_enable(cs35l41, 0); - regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, - CS35L41_BST_OVP_ERR); - regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_BST_OVP_ERR_RLS, - CS35L41_BST_OVP_ERR_RLS); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_BST_OVP_ERR_RLS, 0); + cs35l41_error_release(cs35l41, CS35L41_BST_OVP_ERR, CS35L41_BST_OVP_ERR_RLS); cs35l41_boost_enable(cs35l41, 1); ret = IRQ_HANDLED; } @@ -460,14 +442,7 @@ static irqreturn_t cs35l41_irq(int irq, void *data) if (status[0] & CS35L41_BST_DCM_UVP_ERR) { dev_crit_ratelimited(cs35l41->dev, "DCM VBST Under Voltage Error\n"); cs35l41_boost_enable(cs35l41, 0); - regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, - CS35L41_BST_DCM_UVP_ERR); - regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_BST_UVP_ERR_RLS, - CS35L41_BST_UVP_ERR_RLS); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_BST_UVP_ERR_RLS, 0); + cs35l41_error_release(cs35l41, CS35L41_BST_DCM_UVP_ERR, CS35L41_BST_UVP_ERR_RLS); cs35l41_boost_enable(cs35l41, 1); ret = IRQ_HANDLED; } @@ -475,14 +450,7 @@ static irqreturn_t cs35l41_irq(int irq, void *data) if (status[0] & CS35L41_BST_SHORT_ERR) { dev_crit_ratelimited(cs35l41->dev, "LBST error: powering off!\n"); cs35l41_boost_enable(cs35l41, 0); - regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS1, - CS35L41_BST_SHORT_ERR); - regmap_write(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, 0); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_BST_SHORT_ERR_RLS, - CS35L41_BST_SHORT_ERR_RLS); - regmap_update_bits(cs35l41->regmap, CS35L41_PROTECT_REL_ERR_IGN, - CS35L41_BST_SHORT_ERR_RLS, 0); + cs35l41_error_release(cs35l41, CS35L41_BST_SHORT_ERR, CS35L41_BST_SHORT_ERR_RLS); cs35l41_boost_enable(cs35l41, 1); ret = IRQ_HANDLED; }
On Tue, Feb 07, 2023 at 04:25:23PM +0000, Lucas Tanure wrote:
Add cs35l41_error_release function to handle error release sequences.
Signed-off-by: Lucas Tanure lucas.tanure@collabora.com
Acked-by: Charles Keepax ckeepax@opensource.cirrus.com
Thanks, Charles
Shared boost allows two amplifiers to share a single boost circuit by communicating on the MDSYNC bus. The passive amplifier does not control the boost and receives data from the active amplifier.
Shared Boost is not supported in HDA Systems. Based on David Rhodes shared boost patches.
Signed-off-by: Lucas Tanure lucas.tanure@collabora.com --- include/sound/cs35l41.h | 10 +++++- sound/pci/hda/cs35l41_hda.c | 6 ++-- sound/soc/codecs/cs35l41-lib.c | 56 +++++++++++++++++++++++++++++++++- sound/soc/codecs/cs35l41.c | 27 ++++++++++++++-- sound/soc/codecs/cs35l41.h | 1 + 5 files changed, 93 insertions(+), 7 deletions(-)
diff --git a/include/sound/cs35l41.h b/include/sound/cs35l41.h index 9ac5918269a5..a034ebe03a0e 100644 --- a/include/sound/cs35l41.h +++ b/include/sound/cs35l41.h @@ -11,6 +11,7 @@ #define __CS35L41_H
#include <linux/regmap.h> +#include <linux/completion.h> #include <linux/firmware/cirrus/cs_dsp.h>
#define CS35L41_FIRSTREG 0x00000000 @@ -677,6 +678,7 @@
#define CS35L36_PUP_DONE_IRQ_UNMASK 0x5F #define CS35L36_PUP_DONE_IRQ_MASK 0xBF +#define CS35L41_SYNC_EN_MASK BIT(8)
#define CS35L41_AMP_SHORT_ERR 0x80000000 #define CS35L41_BST_SHORT_ERR 0x0100 @@ -686,6 +688,7 @@ #define CS35L41_BST_DCM_UVP_ERR 0x80 #define CS35L41_OTP_BOOT_DONE 0x02 #define CS35L41_PLL_UNLOCK 0x10 +#define CS35L41_PLL_LOCK BIT(1) #define CS35L41_OTP_BOOT_ERR 0x80000000
#define CS35L41_AMP_SHORT_ERR_RLS 0x02 @@ -705,6 +708,8 @@ #define CS35L41_INT1_MASK_DEFAULT 0x7FFCFE3F #define CS35L41_INT1_UNMASK_PUP 0xFEFFFFFF #define CS35L41_INT1_UNMASK_PDN 0xFF7FFFFF +#define CS35L41_INT3_PLL_LOCK_SHIFT 1 +#define CS35L41_INT3_PLL_LOCK_MASK BIT(CS35L41_INT3_PLL_LOCK_SHIFT)
#define CS35L41_GPIO_DIR_MASK 0x80000000 #define CS35L41_GPIO_DIR_SHIFT 31 @@ -743,6 +748,8 @@ enum cs35l41_boost_type { CS35L41_INT_BOOST, CS35L41_EXT_BOOST, CS35L41_EXT_BOOST_NO_VSPK_SWITCH, + CS35L41_SHD_BOOST_ACTV, + CS35L41_SHD_BOOST_PASS, };
enum cs35l41_clk_ids { @@ -891,6 +898,7 @@ int cs35l41_exit_hibernate(struct device *dev, struct regmap *regmap); int cs35l41_init_boost(struct device *dev, struct regmap *regmap, struct cs35l41_hw_cfg *hw_cfg); bool cs35l41_safe_reset(struct regmap *regmap, enum cs35l41_boost_type b_type); -int cs35l41_global_enable(struct regmap *regmap, enum cs35l41_boost_type b_type, int enable); +int cs35l41_global_enable(struct regmap *regmap, enum cs35l41_boost_type b_type, int enable, + struct completion *pll_lock);
#endif /* __CS35L41_H */ diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index f7815ee24f83..38c0079ef303 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -515,13 +515,13 @@ static void cs35l41_hda_playback_hook(struct device *dev, int action) break; case HDA_GEN_PCM_ACT_PREPARE: mutex_lock(&cs35l41->fw_mutex); - ret = cs35l41_global_enable(reg, cs35l41->hw_cfg.bst_type, 1); + ret = cs35l41_global_enable(reg, cs35l41->hw_cfg.bst_type, 1, NULL); mutex_unlock(&cs35l41->fw_mutex); break; case HDA_GEN_PCM_ACT_CLEANUP: mutex_lock(&cs35l41->fw_mutex); regmap_multi_reg_write(reg, cs35l41_hda_mute, ARRAY_SIZE(cs35l41_hda_mute)); - ret = cs35l41_global_enable(reg, cs35l41->hw_cfg.bst_type, 0); + ret = cs35l41_global_enable(reg, cs35l41->hw_cfg.bst_type, 0, NULL); mutex_unlock(&cs35l41->fw_mutex); break; case HDA_GEN_PCM_ACT_CLOSE: @@ -673,7 +673,7 @@ static int cs35l41_runtime_suspend(struct device *dev) if (cs35l41->playback_started) { regmap_multi_reg_write(cs35l41->regmap, cs35l41_hda_mute, ARRAY_SIZE(cs35l41_hda_mute)); - cs35l41_global_enable(cs35l41->regmap, cs35l41->hw_cfg.bst_type, 0); + cs35l41_global_enable(cs35l41->regmap, cs35l41->hw_cfg.bst_type, 0, NULL); regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL2, CS35L41_AMP_EN_MASK, 0 << CS35L41_AMP_EN_SHIFT); if (cs35l41->hw_cfg.bst_type == CS35L41_EXT_BOOST) diff --git a/sound/soc/codecs/cs35l41-lib.c b/sound/soc/codecs/cs35l41-lib.c index 04be71435491..138eb352551a 100644 --- a/sound/soc/codecs/cs35l41-lib.c +++ b/sound/soc/codecs/cs35l41-lib.c @@ -1114,12 +1114,31 @@ static const struct reg_sequence cs35l41_reset_to_safe[] = { { 0x00000040, 0x00000033 }, };
+static const struct reg_sequence cs35l41_actv_seq[] = { + /* SYNC_BST_CTL_RX_EN = 0; SYNC_BST_CTL_TX_EN = 1 */ + {CS35L41_MDSYNC_EN, 0x00001000}, + /* BST_CTL_SEL = CLASSH */ + {CS35L41_BSTCVRT_VCTRL2, 0x00000001}, +}; + +static const struct reg_sequence cs35l41_pass_seq[] = { + /* SYNC_BST_CTL_RX_EN = 1; SYNC_BST_CTL_TX_EN = 0 */ + {CS35L41_MDSYNC_EN, 0x00002000}, + /* BST_EN = 0 */ + {CS35L41_PWR_CTRL2, 0x00003300}, + /* BST_CTL_SEL = MDSYNC */ + {CS35L41_BSTCVRT_VCTRL2, 0x00000002}, +}; + int cs35l41_init_boost(struct device *dev, struct regmap *regmap, struct cs35l41_hw_cfg *hw_cfg) { int ret;
switch (hw_cfg->bst_type) { + case CS35L41_SHD_BOOST_ACTV: + regmap_multi_reg_write(regmap, cs35l41_actv_seq, ARRAY_SIZE(cs35l41_actv_seq)); + fallthrough; case CS35L41_INT_BOOST: ret = cs35l41_boost_config(dev, regmap, hw_cfg->bst_ind, hw_cfg->bst_cap, hw_cfg->bst_ipk); @@ -1138,6 +1157,9 @@ int cs35l41_init_boost(struct device *dev, struct regmap *regmap, ret = regmap_update_bits(regmap, CS35L41_PWR_CTRL2, CS35L41_BST_EN_MASK, CS35L41_BST_DIS_FET_OFF << CS35L41_BST_EN_SHIFT); break; + case CS35L41_SHD_BOOST_PASS: + regmap_multi_reg_write(regmap, cs35l41_pass_seq, ARRAY_SIZE(cs35l41_pass_seq)); + break; default: dev_err(dev, "Boost type %d not supported\n", hw_cfg->bst_type); ret = -EINVAL; @@ -1165,11 +1187,43 @@ bool cs35l41_safe_reset(struct regmap *regmap, enum cs35l41_boost_type b_type) } EXPORT_SYMBOL_GPL(cs35l41_safe_reset);
-int cs35l41_global_enable(struct regmap *regmap, enum cs35l41_boost_type b_type, int enable) +int cs35l41_global_enable(struct regmap *regmap, enum cs35l41_boost_type b_type, int enable, + struct completion *pll_lock) { int ret; + unsigned int gpio1;
switch (b_type) { + case CS35L41_SHD_BOOST_ACTV: + case CS35L41_SHD_BOOST_PASS: + regmap_update_bits(regmap, CS35L41_PWR_CTRL3, CS35L41_SYNC_EN_MASK, 0); + + gpio1 = enable ? CS35L41_GPIO1_MDSYNC : CS35L41_GPIO1_HIZ; + regmap_update_bits(regmap, CS35L41_GPIO_PAD_CONTROL, CS35L41_GPIO1_CTRL_MASK, + gpio1 << CS35L41_GPIO1_CTRL_SHIFT); + + ret = regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK, + enable << CS35L41_GLOBAL_EN_SHIFT); + usleep_range(3000, 3100); + if (!enable) + break; + + if (!pll_lock) + return -EINVAL; + + ret = wait_for_completion_timeout(pll_lock, msecs_to_jiffies(1000)); + if (ret == 0) { + ret = -ETIMEDOUT; + } else { + regmap_update_bits(regmap, CS35L41_PWR_CTRL3, CS35L41_SYNC_EN_MASK, 0); + regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK, + 0 << CS35L41_GLOBAL_EN_SHIFT); + usleep_range(3000, 3100); + regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK, + 1 << CS35L41_GLOBAL_EN_SHIFT); + usleep_range(3000, 3100); + } + break; case CS35L41_INT_BOOST: ret = regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK, enable << CS35L41_GLOBAL_EN_SHIFT); diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c index c006364e5335..1624510d09c0 100644 --- a/sound/soc/codecs/cs35l41.c +++ b/sound/soc/codecs/cs35l41.c @@ -360,6 +360,7 @@ static void cs35l41_boost_enable(struct cs35l41_private *cs35l41, unsigned int e { switch (cs35l41->hw_cfg.bst_type) { case CS35L41_INT_BOOST: + case CS35L41_SHD_BOOST_ACTV: enable = enable ? CS35L41_BST_EN_DEFAULT : CS35L41_BST_DIS_FET_OFF; regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL2, CS35L41_BST_EN_MASK, enable << CS35L41_BST_EN_SHIFT); @@ -455,6 +456,12 @@ static irqreturn_t cs35l41_irq(int irq, void *data) ret = IRQ_HANDLED; }
+ if (status[2] & CS35L41_PLL_LOCK) { + regmap_write(cs35l41->regmap, CS35L41_IRQ1_STATUS3, CS35L41_PLL_LOCK); + complete(&cs35l41->pll_lock); + ret = IRQ_HANDLED; + } + done: pm_runtime_mark_last_busy(cs35l41->dev); pm_runtime_put_autosuspend(cs35l41->dev); @@ -492,10 +499,12 @@ static int cs35l41_main_amp_event(struct snd_soc_dapm_widget *w, cs35l41_pup_patch, ARRAY_SIZE(cs35l41_pup_patch));
- cs35l41_global_enable(cs35l41->regmap, cs35l41->hw_cfg.bst_type, 1); + cs35l41_global_enable(cs35l41->regmap, cs35l41->hw_cfg.bst_type, 1, + &cs35l41->pll_lock); break; case SND_SOC_DAPM_POST_PMD: - cs35l41_global_enable(cs35l41->regmap, cs35l41->hw_cfg.bst_type, 0); + cs35l41_global_enable(cs35l41->regmap, cs35l41->hw_cfg.bst_type, 0, + &cs35l41->pll_lock);
ret = regmap_read_poll_timeout(cs35l41->regmap, CS35L41_IRQ1_STATUS1, val, val & CS35L41_PDN_DONE_MASK, @@ -802,6 +811,10 @@ static const struct snd_pcm_hw_constraint_list cs35l41_constraints = { static int cs35l41_pcm_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { + struct cs35l41_private *cs35l41 = snd_soc_component_get_drvdata(dai->component); + + reinit_completion(&cs35l41->pll_lock); + if (substream->runtime) return snd_pcm_hw_constraint_list(substream->runtime, 0, SNDRV_PCM_HW_PARAM_RATE, @@ -1252,6 +1265,10 @@ int cs35l41_probe(struct cs35l41_private *cs35l41, const struct cs35l41_hw_cfg * /* Set interrupt masks for critical errors */ regmap_write(cs35l41->regmap, CS35L41_IRQ1_MASK1, CS35L41_INT1_MASK_DEFAULT); + if (cs35l41->hw_cfg.bst_type == CS35L41_SHD_BOOST_PASS || + cs35l41->hw_cfg.bst_type == CS35L41_SHD_BOOST_ACTV) + regmap_update_bits(cs35l41->regmap, CS35L41_IRQ1_MASK3, CS35L41_INT3_PLL_LOCK_MASK, + 0 << CS35L41_INT3_PLL_LOCK_SHIFT);
ret = devm_request_threaded_irq(cs35l41->dev, cs35l41->irq, NULL, cs35l41_irq, IRQF_ONESHOT | IRQF_SHARED | irq_pol, @@ -1275,6 +1292,8 @@ int cs35l41_probe(struct cs35l41_private *cs35l41, const struct cs35l41_hw_cfg * if (ret < 0) goto err;
+ init_completion(&cs35l41->pll_lock); + pm_runtime_set_autosuspend_delay(cs35l41->dev, 3000); pm_runtime_use_autosuspend(cs35l41->dev); pm_runtime_mark_last_busy(cs35l41->dev); @@ -1317,6 +1336,10 @@ void cs35l41_remove(struct cs35l41_private *cs35l41) pm_runtime_disable(cs35l41->dev);
regmap_write(cs35l41->regmap, CS35L41_IRQ1_MASK1, 0xFFFFFFFF); + if (cs35l41->hw_cfg.bst_type == CS35L41_SHD_BOOST_PASS || + cs35l41->hw_cfg.bst_type == CS35L41_SHD_BOOST_ACTV) + regmap_update_bits(cs35l41->regmap, CS35L41_IRQ1_MASK3, CS35L41_INT3_PLL_LOCK_MASK, + 1 << CS35L41_INT3_PLL_LOCK_SHIFT); kfree(cs35l41->dsp.system_name); wm_adsp2_remove(&cs35l41->dsp); cs35l41_safe_reset(cs35l41->regmap, cs35l41->hw_cfg.bst_type); diff --git a/sound/soc/codecs/cs35l41.h b/sound/soc/codecs/cs35l41.h index c85cbc1dd333..34d967d4372b 100644 --- a/sound/soc/codecs/cs35l41.h +++ b/sound/soc/codecs/cs35l41.h @@ -33,6 +33,7 @@ struct cs35l41_private { int irq; /* GPIO for /RST */ struct gpio_desc *reset_gpio; + struct completion pll_lock; };
int cs35l41_probe(struct cs35l41_private *cs35l41, const struct cs35l41_hw_cfg *hw_cfg);
On Tue, Feb 07, 2023 at 04:25:24PM +0000, Lucas Tanure wrote:
Shared boost allows two amplifiers to share a single boost circuit by communicating on the MDSYNC bus. The passive amplifier does not control the boost and receives data from the active amplifier.
Shared Boost is not supported in HDA Systems. Based on David Rhodes shared boost patches.
Signed-off-by: Lucas Tanure lucas.tanure@collabora.com
-int cs35l41_global_enable(struct regmap *regmap, enum cs35l41_boost_type b_type, int enable) +int cs35l41_global_enable(struct regmap *regmap, enum cs35l41_boost_type b_type, int enable,
struct completion *pll_lock){ int ret;
unsigned int gpio1;
switch (b_type) {
case CS35L41_SHD_BOOST_ACTV:
case CS35L41_SHD_BOOST_PASS:
regmap_update_bits(regmap, CS35L41_PWR_CTRL3, CS35L41_SYNC_EN_MASK, 0);gpio1 = enable ? CS35L41_GPIO1_MDSYNC : CS35L41_GPIO1_HIZ;regmap_update_bits(regmap, CS35L41_GPIO_PAD_CONTROL, CS35L41_GPIO1_CTRL_MASK,gpio1 << CS35L41_GPIO1_CTRL_SHIFT);ret = regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK,enable << CS35L41_GLOBAL_EN_SHIFT);usleep_range(3000, 3100);if (!enable)break;if (!pll_lock)return -EINVAL;ret = wait_for_completion_timeout(pll_lock, msecs_to_jiffies(1000));if (ret == 0) {ret = -ETIMEDOUT;} else {regmap_update_bits(regmap, CS35L41_PWR_CTRL3, CS35L41_SYNC_EN_MASK, 0);regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK,0 << CS35L41_GLOBAL_EN_SHIFT);usleep_range(3000, 3100);regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK,1 << CS35L41_GLOBAL_EN_SHIFT);usleep_range(3000, 3100);}
This approach also makes me nervous, I was somewhat imagining the usage of regmap_multi_reg_write for this sequence was because it was very important that no other register writes could interleave in between these writes. But I don't know, so it could also have just been a random design choice. So we probably need David to confirm if that was the reason for the original code here.
Thanks, Charles
On 2/8/23 10:09 AM, Charles Keepax ckeepax@opensource.cirrus.com wrote:
On Tue, Feb 07, 2023 at 04:25:24PM +0000, Lucas Tanure wrote:
Shared boost allows two amplifiers to share a single boost circuit by communicating on the MDSYNC bus. The passive amplifier does not control the boost and receives data from the active amplifier.
Shared Boost is not supported in HDA Systems. Based on David Rhodes shared boost patches.
Signed-off-by: Lucas Tanure lucas.tanure@collabora.com
-int cs35l41_global_enable(struct regmap *regmap, enum cs35l41_boost_type b_type, int enable) +int cs35l41_global_enable(struct regmap *regmap, enum cs35l41_boost_type b_type, int enable,
struct completion *pll_lock){ int ret;
unsigned int gpio1;
switch (b_type) {
case CS35L41_SHD_BOOST_ACTV:
case CS35L41_SHD_BOOST_PASS:
regmap_update_bits(regmap, CS35L41_PWR_CTRL3, CS35L41_SYNC_EN_MASK, 0);gpio1 = enable ? CS35L41_GPIO1_MDSYNC : CS35L41_GPIO1_HIZ;regmap_update_bits(regmap, CS35L41_GPIO_PAD_CONTROL, CS35L41_GPIO1_CTRL_MASK,gpio1 << CS35L41_GPIO1_CTRL_SHIFT);ret = regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK,enable << CS35L41_GLOBAL_EN_SHIFT);usleep_range(3000, 3100);if (!enable)break;if (!pll_lock)return -EINVAL;ret = wait_for_completion_timeout(pll_lock, msecs_to_jiffies(1000));if (ret == 0) {ret = -ETIMEDOUT;} else {regmap_update_bits(regmap, CS35L41_PWR_CTRL3, CS35L41_SYNC_EN_MASK, 0);
Its wrong here. Should be enabling it not disable. I will send v3.
regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK,0 << CS35L41_GLOBAL_EN_SHIFT);usleep_range(3000, 3100);regmap_update_bits(regmap, CS35L41_PWR_CTRL1, CS35L41_GLOBAL_EN_MASK,1 << CS35L41_GLOBAL_EN_SHIFT);usleep_range(3000, 3100);}This approach also makes me nervous, I was somewhat imagining the usage of regmap_multi_reg_write for this sequence was because it was very important that no other register writes could interleave in between these writes. But I don't know, so it could also have just been a random design choice. So we probably need David to confirm if that was the reason for the original code here.
Thanks, Charles
External Boost without GPIO1 as VSPK switch is no longer supported, but there is laptop models using this feature.
Signed-off-by: Lucas Tanure lucas.tanure@collabora.com --- Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml index 18fb471aa891..8465623bbd96 100644 --- a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml +++ b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml @@ -85,11 +85,13 @@ properties: boost-cap-microfarad. External Boost must have GPIO1 as GPIO output. GPIO1 will be set high to enable boost voltage. + External Boost without GPIO1 as VSPK switch is no longer supported. 0 = Internal Boost 1 = External Boost + 2 = External Boost without VPSK switch (Do not use in new systems) $ref: /schemas/types.yaml#/definitions/uint32 minimum: 0 - maximum: 1 + maximum: 2
cirrus,gpio1-polarity-invert: description:
On 07/02/2023 17:25, Lucas Tanure wrote:
External Boost without GPIO1 as VSPK switch is no longer supported, but there is laptop models using this feature.
No, because:
1. We did not finish discussion 2. Subject prefix is still not correct. 3. Please wrap commit message according to Linux coding style / submission process (neither too early nor over the limit): https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/subm...
Best regards, Krzysztof
Describe the properties used for shared boost configuration. Based on David Rhodes shared boost patches.
Signed-off-by: Lucas Tanure lucas.tanure@collabora.com --- .../devicetree/bindings/sound/cirrus,cs35l41.yaml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml index 8465623bbd96..54f769159ce4 100644 --- a/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml +++ b/Documentation/devicetree/bindings/sound/cirrus,cs35l41.yaml @@ -86,12 +86,20 @@ properties: External Boost must have GPIO1 as GPIO output. GPIO1 will be set high to enable boost voltage. External Boost without GPIO1 as VSPK switch is no longer supported. + Shared boost allows two amplifiers to share a single boost circuit by + communicating on the MDSYNC bus. The passive amplifier does not control + the boost and receives data from the active amplifier. GPIO1 should be + configured for Sync when shared boost is used. Shared boost is not + compatible with External boost. Active amplifier requires + boost-peak-milliamp, boost-ind-nanohenry and boost-cap-microfarad. 0 = Internal Boost 1 = External Boost 2 = External Boost without VPSK switch (Do not use in new systems) + 3 = Shared Boost Active + 4 = Shared Boost Passive $ref: /schemas/types.yaml#/definitions/uint32 minimum: 0 - maximum: 2 + maximum: 4
cirrus,gpio1-polarity-invert: description:
participants (4)
-
Charles Keepax -
Krzysztof Kozlowski -
Lucas Tanure -
lucas.tanure＠collabora.com