[alsa-devel] [PATCH] ASoC: Add max9867 codec driver

[Mark Brown]

On Thu, Feb 11, 2016 at 10:32:19AM -0800, anish kumar wrote:

A few small issues but otherwise this looks good:

> +	SOC_SINGLE("DAC Attenuation", MAX9867_DACLEVEL, 0, 15, 1),
> +	SOC_DOUBLE("ADC Left Gain", MAX9867_ADCLEVEL, 4, 0, 15, 1),
> +	SOC_DOUBLE("ADC Right Gain", MAX9867_ADCLEVEL, 0, 0, 15, 1),

All volume controls should be called "... Volume" regardless of physical
implementation as per ControlNames.txt in order to help userspace figure
out how to handle them.  Please also try to provide a mapping to dB via
TLV.

> +	SOC_SINGLE("Volume Smoothing", MAX9867_MODECONFIG, 6, 1, 0),

On/off switches should have names ending "...Switch" again for similar
reasons to the ones behind Volume.

> +static int max9867_resume(struct device *dev)
> +{
> +	struct max9867_priv *max9867 = dev_get_drvdata(dev);
> +
> +	msleep(20);
> +	regmap_update_bits(max9867->regmap, MAX9867_PWRMAN,
> +		MAX9867_SHTDOWN_MASK, MAX9867_SHTDOWN_MASK);
> +	return 0;

This is very strange - we have a sleep before we ever interact with the
device.  A sleep in a register write sequence would make sense but this
doesn't seem like it should.

> +static int max9867_probe(struct snd_soc_codec *codec)
> +{
> +	struct max9867_priv *max9867 = snd_soc_codec_get_drvdata(codec);
> +
> +	max9867->codec = codec;
> +	codec->control_data = max9867->regmap;
> +	return 0;
> +}

The core will figure this out, you don't need to explicitly do it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20160215/3d242358/attachment.sig>