On Thu, Sep 26, 2019 at 09:17:06AM +0200, Nuno Sá wrote: > --- /dev/null > +++ b/sound/soc/codecs/adau7118-hw.c > @@ -0,0 +1,43 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Analog Devices ADAU7118 8 channel PDM-to-I2S/TDM Converter Standalone Hw > + * driver > + * > + * Copyright 2019 Analog Devices Inc. > + */ Please make the entire comment a C++ style one in the .c files so things look more intentional. > + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { > + case SND_SOC_DAIFMT_I2S: > + ret = snd_soc_component_update_bits(dai->component, > + ADAU7118_REG_SPT_CTRL1, > + ADAU7118_DATA_FMT_MASK, > + ADAU7118_DATA_FMT(0)); > + break; > + case SND_SOC_DAIFMT_LEFT_J: > + ret = snd_soc_component_update_bits(dai->component, > + ADAU7118_REG_SPT_CTRL1, > + ADAU7118_DATA_FMT_MASK, > + ADAU7118_DATA_FMT(1)); > + break; > + case SND_SOC_DAIFMT_RIGHT_J: > + st->right_j = true; > + break; Don't we need to set any register values here? > + > + return ret < 0 ? ret : 0; > +} Please don't use the ternery operator like this, it just makes things harder to read - write normal if conditional statements. > + case SND_SOC_BIAS_STANDBY: > + if (snd_soc_component_get_bias_level(component) == > + SND_SOC_BIAS_OFF) { > + if (!st->iovdd) > + return 0; This is broken, the device will always require power so it should always control the regulators. > +static int adau7118_suspend(struct snd_soc_component *component) > +{ > + return snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF); > +} > + > +static int adau7118_resume(struct snd_soc_component *component) > +{ > + return snd_soc_component_force_bias_level(component, > + SND_SOC_BIAS_STANDBY); > +} Let DAPM do this for you, there's no substantial delays on power on so you're probably best just setting idle_bias_off. > +static int adau7118_regulator_setup(struct adau7118_data *st) > +{ > + int ret = 0; > + > + st->iovdd = devm_regulator_get_optional(st->dev, "IOVDD"); > + if (!IS_ERR(st->iovdd)) { Unless the device can operate with supplies physically absent it should not be requesting regulators as optional, this breaks your error handling especially with probe deferral which is a fairly common case.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel