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.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
