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.