On Fri, Jun 20, 2014 at 02:14:18PM +0800, Sean Cross wrote:
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + reg = ES8328_DACCONTROL2;
> +
> + else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
> + reg = ES8328_ADCCONTROL5;
> + else
> + return -EINVAL;
Idiomatically this is just a single if/else, if you weren't doing that
then it ought to be a switch but really there's no need to do something
non-idiomatic.
> + /* Master serial port mode */
> + snd_soc_write(codec, ES8328_MASTERMODE,
> + ES8328_MASTERMODE_MCLKDIV2 |
> + ES8328_MASTERMODE_MSC);
It seems a bit unfortunate that set_sysclk() doesn't manage MCLKDIV2 -
if that isn't badly named the driver could easily support both the
current fixed SYSCLK rate and twice that.
> + case SND_SOC_BIAS_PREPARE:
> + /* VREF, VMID=2x50k, digital enabled */
> + snd_soc_write(codec, ES8328_CHIPPOWER, pwr_reg);
> + snd_soc_write(codec, ES8328_CONTROL1,
> + cc1_reg |
> + ES8328_CONTROL1_VMIDSEL_50k |
> + ES8328_CONTROL1_ENREF);
snd_soc_update_bits() as I'm fairly sure I said last time.
> +static int es8328_suspend(struct snd_soc_codec *codec)
> +{
> + es8328_set_bias_level(codec, SND_SOC_BIAS_OFF);
> + return 0;
> +}
Neither this nor set_bias_level() restores the register cache after
suspend - this will mean suspend and resume is broken in systems where
power is removed from the device over suspend.
> + ret = devm_regulator_bulk_get(codec->dev, ARRAY_SIZE(es8328->supplies),
> + es8328->supplies);
> + if (ret) {
> + dev_err(codec->dev, "!!! Unable to get regulators\n");
> + return ret;
> + }
Print the error code and format the messages normally - none of this
!!! stuff please.
Attachment:
signature.asc
Description: Digital signature
