[alsa-devel] [PATCH 1/2] ASoC: cs35l36: Add support for Cirrus CS35L36 Amplifier

[Mark Brown]

On Fri, Feb 01, 2019 at 01:33:14PM -0600, James Schulman wrote:

> +++ b/sound/soc/codecs/cs35l36.c
> @@ -0,0 +1,1959 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * cs35l36.c -- CS35L36 ALSA SoC audio driver

SPDX headers in C files need to be C++ comments; please make the entire
thing a C++ comment so it looks more intentional.  Headers should still
use /* */

> +static int cs35l36_ldm_sel_put(struct snd_kcontrol *kcontrol,
> +			       struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component =
> +			snd_soc_kcontrol_component(kcontrol);
> +	struct cs35l36_private *cs35l36 =
> +			snd_soc_component_get_drvdata(component);
> +	int val = (ucontrol->value.integer.value[0]) ? CS35L36_NG_AMP_EN_MASK :
> +						       0;
> +
> +	cs35l36->pdata.ldm_mode_sel = val;
> +
> +	regmap_update_bits(cs35l36->regmap, CS35L36_NG_CFG,
> +			   CS35L36_NG_AMP_EN_MASK, val);

If this can be usefully changed at runtime why is it part of the
platform data?  I'd expect control by one of these methods but not both.

> +	SOC_SINGLE("Amp Gain Zero-Cross", CS35L36_AMP_GAIN_CTRL,
> +		CS35L36_AMP_ZC_SHIFT, 1, 0),

This is an on/off switch so should have Switch at the end fo the name so
that userspace tools know how to handle it (the same thing seems to
apply to at least some of the other controls).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20190202/efff0122/attachment.sig>