Re: [alsa-devel] [RFC 2/4] ASoC: Intel: Add merrifield machine driver

On 05/05/2014 08:01 PM, Vinod Koul wrote: [...]

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <linux/module.h> +#include <linux/init.h> +#include <linux/device.h> +#include <linux/slab.h> +#include <linux/io.h> +#include <linux/async.h> +#include <linux/delay.h> +#include <linux/gpio.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/jack.h> +#include <linux/input.h>

+#include <linux/mfd/wm8994/core.h> +#include <linux/mfd/wm8994/registers.h> +#include <linux/mfd/wm8994/pdata.h> +#include "../../codecs/wm8994.h" +#include "../platform-libs/controls_v2.h"

I don't think that include exists in upstream.

+/* Codec PLL output clk rate */ +#define CODEC_SYSCLK_RATE 24576000 +/* Input clock to codec at MCLK1 PIN */ +#define CODEC_IN_MCLK1_RATE 19200000 +/* Input clock to codec at MCLK2 PIN */ +#define CODEC_IN_MCLK2_RATE 32768 +/* define to select between MCLK1 and MCLK2 input to codec as its clock */ +#define CODEC_IN_MCLK1 1 +#define CODEC_IN_MCLK2 2

+struct mrfld_8958_mc_private {

• struct snd_soc_jack jack;
• int jack_retry;

+};

+static inline struct snd_soc_codec *mrfld_8958_get_codec(struct snd_soc_card *card) +{

• bool found = false;
• struct snd_soc_codec *codec;
• list_for_each_entry(codec, &card->codec_dev_list, card_list) {

• if (!strstr(codec->name, "wm8994-codec")) {
  

• 	pr_debug("codec was %s", codec->name);
  

• 	continue;
  

• } else {
  

• 	found = true;
  

• 	break;
  

• }
  

• }
• if (found == false) {

• pr_warn("%s: cant find codec", __func__);
  

• return NULL;
  

• }
• return codec;

+}

+/* TODO: find better way of doing this */

Yes!

+static struct snd_soc_dai *find_codec_dai(struct snd_soc_card *card, const char *dai_name) +{

• int i;
• for (i = 0; i < card->num_rtd; i++) {

• if (!strcmp(card->rtd[i].codec_dai->name, dai_name))
  

• 	return card->rtd[i].codec_dai;
  

• }
• pr_err("%s: unable to find codec dai\n", __func__);
• /* this should never occur */
• WARN_ON(1);
• return NULL;

+}

The proper way to do this is to implement the init callback for the dai link. There you get a pointer to the codec and the dai and everything else. If you need one for later store them in the private struct of the card driver.

+/* Function to switch the input clock for codec, When audio is in

• • progress input clock to codec will be through MCLK1 which is 19.2MHz
• • while in off state input clock to codec will be through 32KHz through
• • MCLK2
• • card : Sound card structure
• • src : Input clock source to codec

That's not proper kerneldoc

• */

[...]

+static int mrfld_wm8958_set_clk_fmt(struct snd_soc_dai *codec_dai) +{

• unsigned int fmt;
• int ret = 0;
• struct snd_soc_card *card = codec_dai->card;
• pr_err("setting snd_soc_dai_set_tdm_slot\n");
• ret = snd_soc_dai_set_tdm_slot(codec_dai, 0, 0, 4, SNDRV_PCM_FORMAT_S24_LE);

the slot width should be in number of bit clock cycles.

• if (ret < 0) {

• pr_err("can't set codec pcm format %d\n", ret);
  

• return ret;
  

• }
• /* WM8958 slave Mode */
• fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF

• | SND_SOC_DAIFMT_CBS_CFS;
  

• ret = snd_soc_dai_set_fmt(codec_dai, fmt);

Use the dai_fmt field of the dai_link to set this up.

• if (ret < 0) {

• pr_err("can't set codec DAI configuration %d\n", ret);
  

• return ret;
  

• }
• /* FIXME: move this to SYS_CLOCK event handler when codec driver

• * dependency is clean.
  

• */
  

• /* Switch to 19.2MHz MCLK1 input clock for codec */
• ret = mrfld_8958_set_codec_clk(card, CODEC_IN_MCLK1);
• return ret;

+}

[...]

+static int mrfld_8958_init(struct snd_soc_pcm_runtime *runtime) +{

• int ret;
• unsigned int fmt;
• struct snd_soc_codec *codec;
• struct snd_soc_card *card = runtime->card;
• struct snd_soc_dai *aif1_dai = find_codec_dai(card, "wm8994-aif1");
• struct mrfld_8958_mc_private *ctx = snd_soc_card_get_drvdata(card);
• if (!aif1_dai)

• return -ENODEV;
  

• pr_debug("Entry %s\n", __func__);
• ret = snd_soc_dai_set_tdm_slot(aif1_dai, 0, 0, 4, SNDRV_PCM_FORMAT_S24_LE);
• if (ret < 0) {

• pr_err("can't set codec pcm format %d\n", ret);
  

• return ret;
  

• }
• /* WM8958 slave Mode */
• fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF

• | SND_SOC_DAIFMT_CBS_CFS;
  

• ret = snd_soc_dai_set_fmt(aif1_dai, fmt);

Same commens as above.

• if (ret < 0) {

• pr_err("can't set codec DAI configuration %d\n", ret);
  

• return ret;
  

• }
• mrfld_8958_set_bias_level(card, &card->dapm, SND_SOC_BIAS_OFF);
• card->dapm.idle_bias_off = true;
• /* these pins are not used in SB config so mark as nc

• *
  

• * LINEOUT1, 2
  

• * IN1R
  

• * DMICDAT2
  

• */
  

• snd_soc_dapm_nc_pin(&card->dapm, "DMIC2DAT");
• snd_soc_dapm_nc_pin(&card->dapm, "LINEOUT1P");
• snd_soc_dapm_nc_pin(&card->dapm, "LINEOUT1N");
• snd_soc_dapm_nc_pin(&card->dapm, "LINEOUT2P");
• snd_soc_dapm_nc_pin(&card->dapm, "LINEOUT2N");
• snd_soc_dapm_nc_pin(&card->dapm, "IN1RN");
• snd_soc_dapm_nc_pin(&card->dapm, "IN1RP");

Set the fully_routed flag on the card and this will happen automatically.

• /* Force enable VMID to avoid cold latency constraints */
• snd_soc_dapm_force_enable_pin(&card->dapm, "VMID");
• snd_soc_dapm_sync(&card->dapm);
• codec = mrfld_8958_get_codec(card);
• if (!codec) {

• pr_err("%s: we didnt find the codec pointer!\n", __func__);
  

• return 0;
  

• }
• ctx->jack_retry = 0;
• ret = snd_soc_jack_new(codec, "Intel MID Audio Jack",

• 	       SND_JACK_HEADSET | SND_JACK_HEADPHONE |
  

• 		SND_JACK_BTN_0 | SND_JACK_BTN_1,
  

• 		&ctx->jack);
  

• if (ret) {

• pr_err("jack creation failed\n");
  

• return ret;
  

• }
• snd_jack_set_key(ctx->jack.jack, SND_JACK_BTN_1, KEY_MEDIA);
• snd_jack_set_key(ctx->jack.jack, SND_JACK_BTN_0, KEY_MEDIA);
• wm8958_mic_detect(codec, &ctx->jack, NULL, NULL, NULL, NULL);
• snd_soc_update_bits(codec, WM8994_AIF1_DAC1_FILTERS_1, WM8994_AIF1DAC1_MUTE, 0);
• snd_soc_update_bits(codec, WM8994_AIF1_DAC2_FILTERS_1, WM8994_AIF1DAC2_MUTE, 0);

The card driver should not be poking in the codec registers.

• /* Micbias1 is always off, so for pm optimizations make sure the micbias1

• * discharge bit is set to floating to avoid discharge in disable state
  

• */
  

• snd_soc_update_bits(codec, WM8958_MICBIAS1, WM8958_MICB1_DISCH, 0);
• return 0;

+}

[...]

+static int snd_mrfld_8958_mc_probe(struct platform_device *pdev) +{

• int ret_val = 0;
• struct mrfld_8958_mc_private *drv;
• pr_debug("Entry %s\n", __func__);
• drv = kzalloc(sizeof(*drv), GFP_ATOMIC);

GFP_KERNEL and maybe devm_kzalloc

• if (!drv) {

• pr_err("allocation failed\n");
  

• return -ENOMEM;
  

• }
• /* register the soc card */
• snd_soc_card_mrfld.dev = &pdev->dev;
• snd_soc_card_set_drvdata(&snd_soc_card_mrfld, drv);
• ret_val = snd_soc_register_card(&snd_soc_card_mrfld);
• if (ret_val) {

• pr_err("snd_soc_register_card failed %d\n", ret_val);
  

• goto unalloc;
  

• }
• platform_set_drvdata(pdev, &snd_soc_card_mrfld);
• pr_info("%s successful\n", __func__);
• return ret_val;

+unalloc:

• kfree(drv);
• return ret_val;

+}

[...]

+static int snd_mrfld_8958_driver_init(void) +{

• pr_info("Merrifield Machine Driver mrfld_wm8958 registerd\n");

That's just noise.

• return platform_driver_register(&snd_mrfld_8958_mc_driver);

+}

+static void snd_mrfld_8958_driver_exit(void) +{

• pr_debug("In %s\n", __func__);
• platform_driver_unregister(&snd_mrfld_8958_mc_driver);

+} +late_initcall(snd_mrfld_8958_driver_init); +module_exit(snd_mrfld_8958_driver_exit);

module_platform_driver().