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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHgNfTwGM8tEsp5SfYp2XmqN2qC557kGyuDwCJfcoxpc7wGSOQ@mail.gmail.com>
Date: Fri, 18 Oct 2024 22:15:29 +1000
From: James Calligeros <jcalligeros99@...il.com>
To: Mark Brown <broonie@...nel.org>
Cc: Martin Povišer <povik+lin@...ebit.org>, 
	David Rhodes <david.rhodes@...rus.com>, Richard Fitzgerald <rf@...nsource.cirrus.com>, 
	Liam Girdwood <lgirdwood@...il.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, asahi@...ts.linux.dev, 
	linux-sound@...r.kernel.org, patches@...nsource.cirrus.com, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Hector Martin <marcan@...can.st>
Subject: Re: [PATCH 2/3] ASoC: cs42l84: Add new codec driver

Hi Mark,

On Wed, Oct 16, 2024 at 10:23 PM Mark Brown <broonie@...nel.org> 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