On 07-02-2023 11:48, Charles Keepax wrote:
On Tue, Feb 07, 2023 at 10:40:20AM +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.
Probably would be nice to put at least a note to say based on
David's patches.
ack
+static const struct reg_sequence cs35l41_shd_boost_seq[] = {
+ {CS35L41_PWR_CTRL3, 0x01000110},
This will blat whatever the user set in the DRE switch.
Technically blats the CLASS H enable from the DAPM widget too,
but as that always turns on should be a no-op. Probably should
either not register the DRE switch or have setting it return an
error for these boost modes.
Fixed in v2.
Changed to regmap_update_bits.
+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;
This feels kinda scary, in that you are relying on a 1 to 1
correspondence between this code running and getting a PLL lock
signal. The datasheet is helpfully completely vague on when PLL
locks are triggered.
The PLL enable seems to be set through set_sysclk, which could
be called multiple times, per DAPM power up. Does the PLL
lock only go once global enable has been set? Can't help
but wonder if a reinit_completion should probably go somewhere
to ensure we are getting this lock of the PLL not a past one.
Added a reinit_completion at cs35l41_pcm_startup
@@ -483,6 +483,11 @@ 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);
+ }
+
If you fall into any of the error cases in this IRQ handler above
this, it will blat values you don't want into BST_EN although, to
be fair that does look currently broken for external boost as
well.
Fixed with a new patch in v2 series.
Thanks,
Charles
