On Wed, Oct 16, 2024 at 08:41:01PM +1000, James Calligeros wrote:
> The CS42L84 is a codec from Cirrus Logic found in Apple Silicon Macs.
> The chip continues Apple's long tradition of compelling vendors to
> spin out bespoke SKUs that are based on existing IP but made subtly
> incompatible with the publicly available part. CS42L84 is very similar
> to CS42L42, but has a different regmap.
> sound/soc/codecs/Makefile | 2 +
> sound/soc/codecs/cs42l84.c | 1081 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> sound/soc/codecs/cs42l84.h | 210 ++++++++++++++++++++++++++++++++
Something's wrong with your formatting here...
> @@ -0,0 +1,1081 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * cs42l84.c -- CS42L84 ALSA SoC audio driver
> + *
Please make the entire comment a C++ one so it's more clearly
intentional.
> +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?
> + .cache_type = REGCACHE_RBTREE,
Unless you've got a particular reason to use something else new drivers
should use _MAPLE.
> +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?
> + case 0b00: /* open */
> + dev_dbg(cs42l84->dev, "Detected open circuit on HS4\n");
> + fallthrough;
> + case 0b11: /* shorted */
> + default:
> + snd_soc_jack_report(cs42l84->jack, SND_JACK_HEADPHONE,
> + SND_JACK_HEADSET);
> + cs42l84->hs_type = SND_JACK_HEADPHONE;
> + dev_dbg(cs42l84->dev, "Detected bare headphone (no mic)\n");
> + }
For coding style we should have a break at the end of the case.
> + default:
> + if (cs42l84->plug_state != CS42L84_TRANS)
> + cs42l84->plug_state = CS42L84_TRANS;
> + }
Here too.
Attachment:
signature.asc
Description: PGP signature
