Re: [alsa-devel] [RFC][PATCH 20/26] alsa: ASoC: Add JZ4740 codec driver

Lars-Peter Clausen
2010-06-03T16:59:06Z

Liam Girdwood wrote:
> On Wed, 2010-06-02 at 21:12 +0200, Lars-Peter Clausen wrote:
>
>> This patch adds support for the JZ4740 internal codec.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
>> Cc: Liam Girdwood <lrg@slimlogic.co.uk>
>> Cc: alsa-devel@alsa-project.org
>> ---
>> sound/soc/codecs/Kconfig | 4 +
>> sound/soc/codecs/Makefile | 2 +
>> sound/soc/codecs/jz4740-codec.c | 502 +++++++++++++++++++++++++++++++++++++++
>> sound/soc/codecs/jz4740-codec.h | 20 ++
>> 4 files changed, 528 insertions(+), 0 deletions(-)
>> create mode 100644 sound/soc/codecs/jz4740-codec.c
>> create mode 100644 sound/soc/codecs/jz4740-codec.h
>>
>
> no need for code in file name here.
>
>
>> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
>> index 31ac553..b8008df 100644
>> --- a/sound/soc/codecs/Kconfig
>> +++ b/sound/soc/codecs/Kconfig
>> @@ -23,6 +23,7 @@ config SND_SOC_ALL_CODECS
>> select SND_SOC_AK4671 if I2C
>> select SND_SOC_CQ0093VC if MFD_DAVINCI_VOICECODEC
>> select SND_SOC_CS4270 if I2C
>> + select SND_SOC_JZ4740 if SOC_JZ4740
>> select SND_SOC_MAX9877 if I2C
>> select SND_SOC_DA7210 if I2C
>> select SND_SOC_PCM3008
>> @@ -269,6 +270,9 @@ config SND_SOC_WM9712
>> config SND_SOC_WM9713
>> tristate
>>
>> +config SND_SOC_JZ4740_CODEC
>> + tristate
>> +
>> # Amp
>> config SND_SOC_MAX9877
>> tristate
>> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
>> index 91429ea..4c7ee31 100644
>> --- a/sound/soc/codecs/Makefile
>> +++ b/sound/soc/codecs/Makefile
>> @@ -56,6 +56,7 @@ snd-soc-wm9705-objs := wm9705.o
>> snd-soc-wm9712-objs := wm9712.o
>> snd-soc-wm9713-objs := wm9713.o
>> snd-soc-wm-hubs-objs := wm_hubs.o
>> +snd-soc-jz4740-codec-objs := jz4740-codec.o
>>
>>
>
> Please use the same format here
>
>
>> # Amp
>> snd-soc-max9877-objs := max9877.o
>> @@ -121,6 +122,7 @@ obj-$(CONFIG_SND_SOC_WM9705) += snd-soc-wm9705.o
>> obj-$(CONFIG_SND_SOC_WM9712) += snd-soc-wm9712.o
>> obj-$(CONFIG_SND_SOC_WM9713) += snd-soc-wm9713.o
>> obj-$(CONFIG_SND_SOC_WM_HUBS) += snd-soc-wm-hubs.o
>> +obj-$(CONFIG_SND_SOC_JZ4740_CODEC) += snd-soc-jz4740-codec.o
>>
>>
>
> ditto.
>
Ok, I agree, but the Kconfig symbol should keep the "CODEC" in it,
otherwise it would clash with the JZ4740 ASoC platform support.
>
>> # Amp
>> obj-$(CONFIG_SND_SOC_MAX9877) += snd-soc-max9877.o
>> diff --git a/sound/soc/codecs/jz4740-codec.c b/sound/soc/codecs/jz4740-codec.c
>> new file mode 100644
>> index 0000000..6e4b741
>> --- /dev/null
>> +++ b/sound/soc/codecs/jz4740-codec.c
>> @@ -0,0 +1,502 @@
>> + [...]
>> +static const struct snd_kcontrol_new jz4740_codec_controls[] = {
>> + SOC_SINGLE("Master Playback Volume", JZ4740_REG_CODEC_2,
>> + JZ4740_CODEC_2_HEADPHONE_VOLUME_OFFSET, 3, 0),
>> + SOC_SINGLE("Capture Volume", JZ4740_REG_CODEC_2,
>> + JZ4740_CODEC_2_INPUT_VOLUME_OFFSET, 31, 0),
>>
>
> Is this the master capture volume ?
>
Hm, yes.
>
>> + SOC_SINGLE("Master Playback Switch", JZ4740_REG_CODEC_1,
>> + JZ4740_CODEC_1_HEADPHONE_DISABLE_OFFSET, 1, 1),
>> + SOC_SINGLE("Mic Capture Volume", JZ4740_REG_CODEC_2,
>> + JZ4740_CODEC_2_MIC_BOOST_GAIN_OFFSET, 3, 0),
>> +};
>> +
>> +static const struct snd_kcontrol_new jz4740_codec_output_controls[] = {
>> + SOC_DAPM_SINGLE("Bypass Switch", JZ4740_REG_CODEC_1,
>> + JZ4740_CODEC_1_SW1_ENABLE_OFFSET, 1, 0),
>> + SOC_DAPM_SINGLE("DAC Switch", JZ4740_REG_CODEC_1,
>> + JZ4740_CODEC_1_SW2_ENABLE_OFFSET, 1, 0),
>> +};
>> +
>> +static const struct snd_kcontrol_new jz4740_codec_input_controls[] = {
>> + SOC_DAPM_SINGLE("Line Capture Switch", JZ4740_REG_CODEC_1,
>> + JZ4740_CODEC_1_LINE_ENABLE_OFFSET, 1, 0),
>> + SOC_DAPM_SINGLE("Mic Capture Switch", JZ4740_REG_CODEC_1,
>> + JZ4740_CODEC_1_MIC_ENABLE_OFFSET, 1, 0),
>> +};
>> +
>> +static const struct snd_soc_dapm_widget jz4740_codec_dapm_widgets[] = {
>> + SND_SOC_DAPM_ADC("ADC", "Capture", JZ4740_REG_CODEC_1,
>> + JZ4740_CODEC_1_ADC_ENABLE_OFFSET, 0),
>> + SND_SOC_DAPM_DAC("DAC", "Playback", JZ4740_REG_CODEC_1,
>> + JZ4740_CODEC_1_DAC_ENABLE_OFFSET, 0),
>> +
>> + SND_SOC_DAPM_MIXER("Output Mixer", JZ4740_REG_CODEC_1,
>> + JZ4740_CODEC_1_HEADPHONE_POWER_DOWN_OFFSET, 1,
>> + jz4740_codec_output_controls,
>> + ARRAY_SIZE(jz4740_codec_output_controls)),
>> +
>> + SND_SOC_DAPM_MIXER_NAMED_CTL("Input Mixer", SND_SOC_NOPM, 0, 0,
>> + jz4740_codec_input_controls,
>> + ARRAY_SIZE(jz4740_codec_input_controls)),
>> + SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
>> +
>> + SND_SOC_DAPM_OUTPUT("LOUT"),
>> + SND_SOC_DAPM_OUTPUT("ROUT"),
>> +
>> + SND_SOC_DAPM_INPUT("MIC"),
>> + SND_SOC_DAPM_INPUT("LIN"),
>> + SND_SOC_DAPM_INPUT("RIN"),
>> +};
>> +
>> +static const struct snd_soc_dapm_route jz4740_codec_dapm_routes[] = {
>> +
>> + {"Line Input", NULL, "LIN"},
>> + {"Line Input", NULL, "RIN"},
>> +
>> + {"Input Mixer", "Line Capture Switch", "Line Input"},
>> + {"Input Mixer", "Mic Capture Switch", "MIC"},
>> +
>> + {"ADC", NULL, "Input Mixer"},
>> +
>> + {"Output Mixer", "Bypass Switch", "Input Mixer"},
>> + {"Output Mixer", "DAC Switch", "DAC"},
>> +
>> + {"LOUT", NULL, "Output Mixer"},
>> + {"ROUT", NULL, "Output Mixer"},
>> +};
>> +
>> +static int jz4740_codec_hw_params(struct snd_pcm_substream *substream,
>> + struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
>> +{
>> + uint32_t val;
>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> + struct snd_soc_device *socdev = rtd->socdev;
>> + struct snd_soc_codec *codec = socdev->card->codec;
>> +
>> + switch (params_format(params)) {
>> + case SNDRV_PCM_FORMAT_S8:
>> + case SNDRV_PCM_FORMAT_S16_LE:
>> + case SNDRV_PCM_FORMAT_S18_3LE:
>> + break;
>> + default:
>> + return -EINVAL;
>> + break;
>> + }
>>
>
> The PCM format check is not required here as core checks this.
>
Ok.
>> + [...]
>
> Thanks
>
> Liam
>

Thanks for reviewing
- Lars