Re: [PATCH 2/3] ASoC: cs42l84: Add new codec driver

[James Calligeros <jcalligeros99@xxxxxxxxx>]

Hi Mark,

On Wed, Oct 16, 2024 at 10:23 PM Mark Brown <broonie@xxxxxxxxxx> wrote:
> > +static const struct regmap_config cs42l84_regmap = {
> > +     .reg_bits = 16,
> > +     .val_bits = 8,
> > +
> > +     .volatile_reg = cs42l84_volatile_register,
> > +
> > +     .max_register = 0xffff,
>
> Does that register actually exist?

It is likely that the highest register is the ID register, which would match
CS42L42. Setting that here does not break anything.

> > +static int cs42l84_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
> > +{
> > +     struct snd_soc_component *component = dai->component;
> > +     struct cs42l84_private *cs42l84 = snd_soc_component_get_drvdata(component);
> > +     unsigned int regval;
> > +     int ret;
>
> > +                             ret = regmap_read_poll_timeout(cs42l84->regmap,
> > +                                                            CS42L84_PLL_LOCK_STATUS,
> > +                                                            regval,
> > +                                                            (regval & CS42L84_PLL_LOCK_STATUS_LOCKED),
> > +                                                            CS42L84_PLL_LOCK_POLL_US,
> > +                                                            CS42L84_PLL_LOCK_TIMEOUT_US);
> > +                             if (ret < 0)
> > +                                     dev_warn(component->dev, "PLL failed to lock: %d\n", ret);
>
> This is too heavyweight for a mute operation, do you really need one?

Unfortunately it seems that way. This was carried over directly from CS42L42,
and as the comment in the function states, the internal power supply
will discharge
itself and crash the chip if the PLL is set up before any of the interfaces
are fully enabled. This is corroborrated by the CS42L42 datasheet. I've spent
some time today trying to work around this, but trying to sequence and enable
the PLL anywhere more sensible either causes the chip to crash or causes
audible clock jitter artefacts.

Regards,
James