[alsa-devel] [PATCH] ASoC: add SSM2604 codec driver
Mark Brown
broonie at opensource.wolfsonmicro.com
Sat Aug 7 14:44:16 CEST 2010
On Sat, Aug 07, 2010 at 01:54:07AM -0400, Mike Frysinger wrote:
> From: Cliff Cai <cliff.cai at analog.com>
>
> Signed-off-by: Cliff Cai <cliff.cai at analog.com>
> Signed-off-by: Mike Frysinger <vapier at gentoo.org>
This needs to be updated to the multi-component branch. There's a few
other issues as well.
> +#define SSM2604_VERSION "0.1"
Drop this, just track the version through git and kernel releases.
> +static const struct soc_enum ssm2604_enum[] = {
> + SOC_ENUM_SINGLE(SSM2604_APDIGI, 1, 4, ssm2604_deemph),
> +};
Use individual variables for enums rather than stuffing them all in an
array. The arrays are just hard to read and error prone...
> +SOC_ENUM("Capture Source", ssm2604_enum[0]),
> +
> +SOC_ENUM("Playback De-emphasis", ssm2604_enum[1]),
...are you sure this code runs correctly? I see only one element in the
array. It's worth checking this in your other drivers as well.
> + /* The DAI has shared clocks so if we already have a playback or
> + * capture going then constrain this substream to match it.
> + * TODO: the ssm2604 allows pairs of non-matching PB/REC rates
> + */
> + if (ssm2604->master_substream) {
Set the symmetric_rates flag on the DAI rather than open coding this.
> + /* deactivate */
> + if (!codec->active)
> + snd_soc_write(codec, SSM2604_ACTIVE, 0);
This probably wants to be managed by DAPM (possibly a supply widget for
the DAC and ADC).
> +#define SSM2604_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_32000 |\
> + SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |\
> + SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000)
SNDRV_PCM_RATE_8000_96000
> + snd_soc_write(codec, SSM2604_APANA, APANA_SELECT_DAC);
This could use a little explanation?
> +struct ssm2604_setup_data {
> + int i2c_bus;
> + unsigned short i2c_address;
> +};
This is definitely not needed, even with the current code.
More information about the Alsa-devel
mailing list