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

[Pierre-Louis Bossart]

> +
> +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?