From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lars-Peter Clausen Subject: Re: [PATCH V4 1/5] sound: asoc: Adding support for STA529 Audio Codec Date: Mon, 06 Jun 2011 08:23:49 +0200 Message-ID: <4DEC7275.7010603@metafoo.de> References: <1307339856-30656-1-git-send-email-rajeev-dlh.kumar@st.com> <1307339856-30656-2-git-send-email-rajeev-dlh.kumar@st.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mailhost.informatik.uni-hamburg.de (mailhost.informatik.uni-hamburg.de [134.100.9.70]) by alsa0.perex.cz (Postfix) with ESMTP id D5FF72433F for ; Mon, 6 Jun 2011 08:25:01 +0200 (CEST) In-Reply-To: <1307339856-30656-2-git-send-email-rajeev-dlh.kumar@st.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: alsa-devel-bounces@alsa-project.org Errors-To: alsa-devel-bounces@alsa-project.org To: Rajeev Kumar Cc: tiwai@suse.de, alsa-devel@alsa-project.org, broonie@opensource.wolfsonmicro.com, lrg@slimlogic.co.uk List-Id: alsa-devel@alsa-project.org On 06/06/2011 07:57 AM, Rajeev Kumar wrote: > This patch adds the support for STA529 audio codec. > Details of the audio codec can be seen here: > http://www.st.com/internet/imag_video/product/159187.jsp > > Signed-off-by: Rajeev Kumar > --- > sound/soc/codecs/Kconfig | 5 + > sound/soc/codecs/Makefile | 2 + > sound/soc/codecs/sta529.c | 374 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 381 insertions(+), 0 deletions(-) > create mode 100644 sound/soc/codecs/sta529.c > > [...] > + > +struct sta529 { > + unsigned int sysclk; sysclk is unused > + enum snd_soc_control_type control_type; > + void *control_data; control_data is unused > +}; > + > +static const char *pwm_mode_text[] = { "binary", "headphone", "ternary", > + "phase-shift"}; > +static const char *interface_mode_text[] = { "slave", "master"}; > + > +static const struct soc_enum pwm_src_enum = > +SOC_ENUM_SINGLE(STA529_FFXCFG1, 4, 4, pwm_mode_text); > + > +static const struct soc_enum mode_src_enum = > +SOC_ENUM_SINGLE(STA529_P2SCFG0, 0, 2, interface_mode_text); > + > +static const struct snd_kcontrol_new sta529_new_snd_controls[] = { > + SOC_ENUM("PWM Select", pwm_src_enum), > + SOC_ENUM("MODE Select", mode_src_enum), The mode should be configured by the dai_drivers set_fmt callback, and not by a control. > +}; > + > +static const DECLARE_TLV_DB_SCALE(out_gain_tlv, -9150, 50, 0); > +static const DECLARE_TLV_DB_SCALE(master_vol_tlv, -12750, 50, 0); > + > +static const struct snd_kcontrol_new sta529_snd_controls[] = { > + SOC_DOUBLE_R_TLV("Digital Playback Volume", STA529_LVOL, STA529_RVOL, 0, > + 127, 0, out_gain_tlv), > + SOC_SINGLE_TLV("Master Playback Volume", STA529_MVOL, 0, 127, 1, > + master_vol_tlv), > +}; > + > +static int sta529_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct snd_soc_codec *codec = rtd->codec; > + int pdata = 0; > + > + switch (params_format(params)) { > + case SNDRV_PCM_FORMAT_S16_LE: > + pdata = 1; > + break; > + case SNDRV_PCM_FORMAT_S24_LE: > + pdata = 2; > + break; > + case SNDRV_PCM_FORMAT_S32_LE: > + pdata = 3; > + break; > + } > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) > + snd_soc_update_bits(codec, STA529_S2PCFG1, 0xC0, (pdata << 6)); > + else > + snd_soc_update_bits(codec, STA529_P2SCFG1, 0xC0, (pdata << 6)); > + > + return 0; > +} > + > +static int sta529_set_dai_fmt(struct snd_soc_dai *codec_dai, u32 fmt) > +{ > + struct snd_soc_codec *codec = codec_dai->codec; > + u8 mode = 0; > + > + /* interface format */ > + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { > + case SND_SOC_DAIFMT_LEFT_J: > + mode = LEFT_J_DATA_FORMAT; > + break; > + case SND_SOC_DAIFMT_I2S: > + mode = I2S_DATA_FORMAT; > + break; > + case SND_SOC_DAIFMT_RIGHT_J: > + mode = RIGHT_J_DATA_FORMAT; > + break; > + default: > + return -EINVAL; > + } > + mode |= 0x20; > + snd_soc_update_bits(codec, STA529_S2PCFG0, 0xE0, mode); What about the P2SCFG0 register, I would suspect, that the same data needs to be written to it. > + > + return 0; > +} > + > +static int sta529_mute(struct snd_soc_dai *dai, int mute) > +{ > + struct snd_soc_codec *codec = dai->codec; > + > + u8 mute_reg = snd_soc_read(codec, STA529_FFXCFG0) & ~CODEC_MUTE_VAL; > + > + if (mute) > + mute_reg |= CODEC_MUTE_VAL; > + > + snd_soc_update_bits(codec, STA529_FFXCFG0, 0x80, 00); I guess, it should be mute_reg instead of 00 > + > + return 0; > +} > + > +static int > +sta529_set_bias_level(struct snd_soc_codec *codec, > + enum snd_soc_bias_level level) > +{ > + switch (level) { > + case SND_SOC_BIAS_ON: > + case SND_SOC_BIAS_PREPARE: > + snd_soc_update_bits(codec, STA529_FFXCFG0, POWER_CNTLMSAK, > + POWER_UP); > + snd_soc_update_bits(codec, STA529_MISC, 1, 0x01); > + break; > + case SND_SOC_BIAS_STANDBY: > + case SND_SOC_BIAS_OFF: > + snd_soc_update_bits(codec, STA529_FFXCFG0, POWER_CNTLMSAK, > + POWER_STDBY); > + /* Making FFX output to zero */ > + snd_soc_update_bits(codec, STA529_FFXCFG0, FFX_MASK, > + FFX_OFF); > + snd_soc_update_bits(codec, STA529_MISC, 1, 0x00); > + > + break; > + } > + > + /* > + * store the label for powers down audio subsystem for suspend.This is > + * used by soc core layer > + */ > + codec->dapm.bias_level = level; > + return 0; > + > +} > + > +static struct snd_soc_dai_ops sta529_dai_ops = { Can be const > + .hw_params = sta529_hw_params, > + .set_fmt = sta529_set_dai_fmt, > + .digital_mute = sta529_mute, > +}; > + > [..] > + > +static int sta529_probe(struct snd_soc_codec *codec) > +{ > + struct sta529 *sta529 = snd_soc_codec_get_drvdata(codec); > + int ret; > + > + ret = snd_soc_codec_set_cache_io(codec, 8, 8, sta529->control_type); If your codec only supports I2C, you can pass SND_SOC_I2C here directly and get rid of the control_type field. > + if (ret < 0) { > + dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret); > + return ret; > + } > + sta529_set_bias_level(codec, SND_SOC_BIAS_STANDBY); > + > + snd_soc_add_controls(codec, sta529_snd_controls, > + ARRAY_SIZE(sta529_snd_controls)); > + > + snd_soc_add_controls(codec, sta529_new_snd_controls, > + ARRAY_SIZE(sta529_new_snd_controls)); You should use table based controls setup. i.e assign the control table to the 'controls' field of your codec_driver. > + return 0; > +} > + > +/* power down chip */ > +static int sta529_remove(struct snd_soc_codec *codec) > +{ > + sta529_set_bias_level(codec, SND_SOC_BIAS_OFF); > + > + return 0; > +} > + > +static int sta529_suspend(struct snd_soc_codec *codec, pm_message_t state) > +{ > + sta529_set_bias_level(codec, SND_SOC_BIAS_OFF); > + > + return 0; > +} > + > +static int sta529_resume(struct snd_soc_codec *codec) > +{ > + snd_soc_cache_sync(codec); > + sta529_set_bias_level(codec, SND_SOC_BIAS_STANDBY); > + sta529_set_bias_level(codec, codec->dapm.suspend_bias_level); > + > + return 0; > +} > + > +struct snd_soc_codec_driver soc_codec_dev_sta529 = { > + .probe = sta529_probe, > + .remove = sta529_remove, > + .set_bias_level = sta529_set_bias_level, > + .suspend = sta529_suspend, > + .resume = sta529_resume, > + .reg_cache_size = STA529_CACHEREGNUM, > + .reg_word_size = sizeof(u8), > + .reg_cache_default = sta529_reg, > + > +}; > + > +static __devinit int sta529_i2c_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) > +{ > + struct sta529 *sta529; > + int ret; > + > + if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) > + return -EINVAL; > + > + sta529 = kzalloc(sizeof(struct sta529), GFP_KERNEL); > + if (sta529 == NULL) > + return -ENOMEM; > + > + i2c_set_clientdata(i2c, sta529); > + sta529->control_data = i2c; > + sta529->control_type = SND_SOC_I2C; > + > + ret = snd_soc_register_codec(&i2c->dev, > + &soc_codec_dev_sta529, &sta529_dai, 1); > + if (ret < 0) > + kfree(sta529); > + return ret; > +} > + > +static int sta529_i2c_remove(struct i2c_client *client) __devexit > +{ > + snd_soc_unregister_codec(&client->dev); > + kfree(i2c_get_clientdata(client)); > + return 0; > +} > + > [...] Your driver doesn't use any DAPM. You should at least define input and output pins and their routing, but I would expect that there is more that can be done, like dynamicly powering unused sections of the codec down, like the DAC or ADC.