Re: [alsa-devel] [PATCH v2] ASoC: Intel: Boards: Add CNL RT274 I2S machine driver

+static int cnl_rt274_clock_control(struct snd_soc_dapm_widget *w,

• 		   struct snd_kcontrol *k, int event)
  

+{

• struct snd_soc_dapm_context *dapm = w->dapm;
• struct snd_soc_card *card = dapm->card;
• struct snd_soc_dai *codec_dai =

• snd_soc_card_get_codec_dai(card, RT274_CODEC_DAI);
  

• int ret, ratio = 100;
• if (!codec_dai)

• return -EINVAL;
  

• /* Codec needs clock for Jack detection and button press */
• ret = snd_soc_dai_set_sysclk(codec_dai, RT274_SCLK_S_PLL2,

• 		     CNL_FREQ_OUT, SND_SOC_CLOCK_IN);
  

• if (ret < 0) {

• dev_err(codec_dai->dev, "set codec sysclk failed: %d\n", ret);
  

• return ret;
  

• }
• if (SND_SOC_DAPM_EVENT_ON(event)) {

• ret = snd_soc_dai_set_bclk_ratio(codec_dai, ratio);
  

• if (ret) {
  

• 	dev_err(codec_dai->dev,
  

• 		"set bclk ratio failed: %d\n", ret);
  

• 	return ret;
  

• }
  

• ret = snd_soc_dai_set_pll(codec_dai, 0, RT274_PLL2_S_BCLK,
  

• 			  CNL_BE_FIXUP_RATE * ratio,
  

• 			  CNL_FREQ_OUT);
  

• if (ret) {
  

• 	dev_err(codec_dai->dev,
  

• 		"enable PLL2 failed: %d\n", ret);
  

• 	return ret;
  

• }
  

• }
• return 0;

+}

it's not clear to me why you need a clock control? You are not changing anything that really depends on DAPM events, to e.g. take the MCLK down and use a local clock, so could this be moved to hw_params?

+static const struct snd_soc_dapm_route cnl_map[] = {

• {"Headphone Jack", NULL, "HPO Pin"},
• {"MIC", NULL, "Mic Jack"},
• {"DMic", NULL, "SoC DMIC"},
• {"DMIC01 Rx", NULL, "Capture"},
• {"dmic01_hifi", NULL, "DMIC01 Rx"},
• {"AIF1 Playback", NULL, "ssp0 Tx"},
• {"ssp0 Tx", NULL, "codec1_out"},
• {"ssp0 Tx", NULL, "codec0_out"},

I get the routes to connect firmware widgets to codec ones, but why do we need SSP0 TX-> codec1_out? shouldn't this be part of the topology?

• {"ssp0 Rx", NULL, "AIF1 Capture"},
• {"codec0_in", NULL, "ssp0 Rx"},
• {"Headphone Jack", NULL, "Platform Clock"},
• {"Mic Jack", NULL, "Platform Clock"},

+};

+static struct snd_soc_jack_pin cnl_headset_pins[] = {

• {

• .pin = "Mic Jack",
  

• .mask = SND_JACK_MICROPHONE,
  

• },
• {

• .pin = "Headphone Jack",
  

• .mask = SND_JACK_HEADPHONE,
  

• },

+};

+static struct snd_soc_jack cnl_headset;

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

• struct snd_soc_card *card = runtime->card;
• struct snd_soc_dai *codec_dai = runtime->codec_dai;
• struct snd_soc_component *component = codec_dai->component;
• int ret;
• ret = snd_soc_card_jack_new(runtime->card, "Headset",

• SND_JACK_HEADSET, &cnl_headset,
  

• cnl_headset_pins, ARRAY_SIZE(cnl_headset_pins));
  

• if (ret)

• return ret;
  

• ret = snd_soc_component_set_jack(component, &cnl_headset, NULL);
• if (ret)

• return ret;
  

• /* TDM 4 slots 24 bit, set Rx & Tx bitmask to 4 active slots */
• ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xf, 0xf, 4, 24);

what are the 4 slots used for?