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

Vinod Koul vinod.koul at intel.com
Tue May 6 18:58:20 CEST 2014


On Tue, May 06, 2014 at 05:54:53PM +0200, Lars-Peter Clausen wrote:
> 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.
which one ../platform-libs/controls_v2.h is part of this patch.

> 
> >+
> >+/* 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!
Am all ears :)

> 
> >+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.
again the driver would need to store the pointers to cards (we multi codec
systems) and multi dais. Somehow I dont feel this might be worth thr trouble.
Earlier we always had card->codec pointing to _one_ codec but with multi-codec
systems that is not the case, so we need ot lookup, but yes am not sure if above
is best way or something else..

> 
> 
> >+
> >+/* 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
Ah that shouldnt have slipped, will fix.

> 
> >+ */
> [...]
> >+
> >+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.
ok

> 
> >+	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.
ok

> 
> >+	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.
I havent looked thos changes up, will this mark as nc asll the unrouted pins?

> 
> >+
> >+	/* 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.
ah yes with DPCM we dont need this

> 
> >+
> >+	/* 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
devm_ yes
> 
> >+	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().
yes!

-- 
~Vinod


More information about the Alsa-devel mailing list