[alsa-devel] [PATCH v2] ASoC: Add support for cs42l73 codec
Austin, Brian
Brian.Austin at cirrus.com
Fri Sep 30 21:19:56 CEST 2011
On Sep 30, 2011, at 1:50 PM, Lars-Peter Clausen wrote:
> On 09/30/2011 06:41 PM, Brian Austin wrote:
>> This patch adds support for the Cirrus Logic CS42L73
>> low power stereo codec.
>>
>> This patch has cleared checkpatch.pl with no warnings or errors.
>> Code changes requested were implemented.
>> ASoC API changes requested were implemented.
>>
>> Signed-off-by: Brian Austin <brian.austin at cirrus.com>
>> ---
>> [...]
>> +struct cs42l73_private {
>
> extra space
>
>> [...]
>> +
>> +static const struct snd_soc_dapm_route cs42l73_audio_map[] = {
>> + {"HPOUTA", NULL, "HP Amp Left"},
>> + {"HPOUTB", NULL, "HP Amp Right"},
>> + {"LINEOUTA", NULL, "LO Amp Left"},
>> + {"LINEOUTB", NULL, "LO Amp Right"},
>> + {"SPKOUT", NULL, "SPK Amp"},
>> + {"EAROUT", NULL, "EAR Amp"},
>> + {"SPKLINEOUT", NULL, "SPKLO Amp"},
>> +
>> + {"HP Amp Left", "DAC", "DAC Left"},
>> + {"HP Amp Right", "DAC", "DAC Right"},
>> + {"LO Amp Left", "DAC", "DAC Left"},
>> + {"LO Amp Right", "DAC", "DAC Right"},
>> + {"SPK Amp", "DAC", "DAC Left"},
>> + {"SPKLO Amp", "DAC", "DAC Right"},
>> + {"EAR Amp", "DAC", "DAC Right"},
>
> I suppose this won't work as expected. The control element of the path will
> only have an effect if the sink is a mixer or a mux.
>
>> +
>> + {"PGA Mux Left", NULL, "LINEINA"},
>> + {"PGA Mux Right", NULL, "LINEINB"},
>> + {"PGA Mux Left", NULL, "MIC1"},
>> + {"PGA Mux Right", NULL, "MIC2"},
>
> You probably want connect the inputs of the mux depending on its setting.
>
> e.g. {"PGA Mux Left", "Line A", "LINEINA"},
>
> [...]
>> +
>> +struct cs42l73_mclk_div cs42l73_mclk_coeffs[] = {
>
> static
>
>> [...]
>> +
>> +struct cs42l73_mclkx_div cs42l73_mclkx_coeffs[] = {
>
> static
>
>> [...]
>> +int cs42l73_get_mclkx_coeff(int mclkx)
>
> static
>
>> [...]
>> +int cs42l73_get_mclk_coeff(int mclk, int srate)
>
> static
>
>
>> +
>> +static int cs42l73_set_mclk(struct snd_soc_dai *dai)
>> +{
>> + struct snd_soc_codec *codec = dai->codec;
>> + struct cs42l73_private *priv = snd_soc_codec_get_drvdata(codec);
>> +
>> + int mclkx_coeff;
>> + u32 mclk = 0;
>> + u8 dmmcc = 0;
>> +
>> + /* MCLKX -> MCLK */
>> + mclkx_coeff = cs42l73_get_mclkx_coeff(priv->sysclk);
>> +
>
> here you use priv->sysclk ...
>
>> +
>> +static int cs42l73_set_sysclk(struct snd_soc_dai *dai,
>> + int clk_id, unsigned int freq, int dir)
>> +{
>> + struct snd_soc_codec *codec = dai->codec;
>> + struct cs42l73_private *priv = snd_soc_codec_get_drvdata(codec);
>> +
>> + if (clk_id != CS42L73_CLKID_MCLK1 && clk_id != CS42L73_CLKID_MCLK2) {
>> + dev_err(codec->dev, "Invalid clk_id %u\n", clk_id);
>> + return -EINVAL;
>> + }
>> +
>> + if ((cs42l73_get_mclkx_coeff(freq) < 0)) {
>> + dev_err(codec->dev, "Invalid sysclk %u\n", freq);
>> + return -EINVAL;
>> + }
>> +
>> + if ((cs42l73_set_mclk(dai)) < 0) {
>> + dev_err(codec->dev, "Unable to set MCLK for dai %s\n",
>> + dai->name);
>> + return -EINVAL;
>> + }
>> +
>> + priv->sysclk = freq;
>> + priv->mclksel = clk_id;
>> +
>
> ... but here you only set it after having called cs42l73_set_mclk. Is this
> correct? Maybe pass it in as a parameter to cs42l73_set_mclk
>
>> + return 0;
>> +}
>> +
>> +static int cs42l73_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
>> +{
>> + struct snd_soc_codec *codec = codec_dai->codec;
>> + struct cs42l73_private *priv = snd_soc_codec_get_drvdata(codec);
>> + int id = codec_dai->id;
>> + int inv, format;
>
> unsigned int
>
>> + u8 spc, mmcc;
>> +
>> [...]
>> +
>
>> +static int cs42l73_set_bias_level(struct snd_soc_codec *codec,
>> + enum snd_soc_bias_level level)
>> +{
>> + int ret;
>> +
>> + switch (level) {
>> + case SND_SOC_BIAS_ON:
>> + snd_soc_update_bits(codec, CS42L73_DMMCC, MCLKDIS, 0);
>> + snd_soc_update_bits(codec, CS42L73_PWRCTL1, PDN, 0);
>> + break;
>> +
>> + case SND_SOC_BIAS_PREPARE:
>> + break;
>> +
>> + case SND_SOC_BIAS_STANDBY:
>> + if (codec->dapm.bias_level == SND_SOC_BIAS_OFF) {
>> + ret = snd_soc_cache_sync(codec);
>
> indention
>
>> [...]
>> +
>> +static struct snd_soc_dai_ops cs42l73_ops = {
>
> const
>
>> + .startup = cs42l73_pcm_startup,
>> + .hw_params = cs42l73_pcm_hw_params,
>> + .set_fmt = cs42l73_set_dai_fmt,
>> + .set_sysclk = cs42l73_set_sysclk,
>> + .set_tristate = cs42l73_set_tristate,
>> +};
>> +
>> +struct snd_soc_dai_driver cs42l73_dai[] = {
>
> static
>
>> + {
>> + .name = "cs42l73-xsp",
>> + .id = CS42L73_XSP,
>> + .playback = {
>> + .stream_name = "XSP Playback",
>> + .channels_min = 1,
>> + .channels_max = 2,
>> + .rates = CS42L73_RATES,
>> + .formats = CS42L73_FORMATS,},
>> +
>> + .capture = {
>> + .stream_name = "XSP Capture",
>> + .channels_min = 1,
>> + .channels_max = 2,
>> + .rates = CS42L73_RATES,
>> + .formats = CS42L73_FORMATS,},
>> +
>> + .ops = &cs42l73_ops,
>> + .symmetric_rates = 1,
>> + },
>
> Indention looks a bit odd here
>
>> [...]
>> +static int cs42l73_probe(struct snd_soc_codec *codec)
>> +{
>> + int ret;
>> + unsigned int devid = 0;
>> + struct cs42l73_private *cs42l73 = snd_soc_codec_get_drvdata(codec);
>> +
>> + codec->control_data = cs42l73->control_data;
>> + codec->hw_write = (hw_write_t)i2c_master_send;
>
> You don't need to initialize control_data and hw_write.
> snd_soc_codec_set_cache_io will do this for you. This also means that you can
> remove the control_data field from your driver private data struct.
>
>> +
>> + ret = snd_soc_codec_set_cache_io(codec, 8, 8, cs42l73->control_type);
>
> If you CODEC only has a I2C interface you can use SND_SOC_I2C here directly
> instead of cs42l73->control_type
>
>> [...]
>> +
>> + snd_soc_add_controls(codec, cs42l73_snd_controls,
>> + ARRAY_SIZE(cs42l73_snd_controls));
>
> Just put the controls in your codec driver struct instead of manually adding them.
>
>> +
>> + return ret;
>> +}
>> +
>> +static int cs42l73_remove(struct snd_soc_codec *codec)
>> +{
>> + cs42l73_set_bias_level(codec, SND_SOC_BIAS_OFF);
>> + return 0;
>> +}
>> +
>> +struct snd_soc_codec_driver soc_codec_dev_cs42l73 = {
>
> static
>
>> + .probe = cs42l73_probe,
>> + .remove = cs42l73_remove,
>> + .suspend = cs42l73_suspend,
>> + .resume = cs42l73_resume,
>> + .set_bias_level = cs42l73_set_bias_level,
>> + .reg_cache_size = ARRAY_SIZE(cs42l73_reg),
>> + .reg_cache_default = cs42l73_reg,
>> + .reg_word_size = sizeof(u8),
>> + .dapm_widgets = cs42l73_dapm_widgets,
>> + .num_dapm_widgets = ARRAY_SIZE(cs42l73_dapm_widgets),
>> + .dapm_routes = cs42l73_audio_map,
>> + .num_dapm_routes = ARRAY_SIZE(cs42l73_audio_map),
>> +};
>> +
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
Thanks for the feedback. This is good. The DAPM Routing works with a machine driver, but i will have to
make it work inside the codec I guess.
Thanks,
Brian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3917 bytes
Desc: not available
Url : http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20110930/1f0f0eb6/attachment.p7s
More information about the Alsa-devel
mailing list