Re: [alsa-devel] [PATCH 1/2] ASoC: Intel: boards: Add CML m/c using RT1011 and RT5682

[Kai Vehmanen <kai.vehmanen@linux.intel.com>]

Hi,

On Tue, 24 Sep 2019, sathya.prakash.m.r@intel.com wrote:

> +       /* need to enable ASRC function for 24MHz mclk rate */                                                                                                                                         
> +       rt5682_sel_asrc_clk_src(component, RT5682_DA_STEREO1_FILTER |                                                                                                                                  
> +                                       RT5682_AD_STEREO1_FILTER,                                                                                                                                      
> +                                       RT5682_CLK_SEL_I2S1_ASRC);    

please align to opening parenthesis to align with rest of the file.

> +static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = {
> +	/*speaker*/
> +	{"TL Ext Spk", NULL, "TL SPO"},
> +	{"TR Ext Spk", NULL, "TR SPO"},
> +	{"WL Ext Spk", NULL, "WL SPO"},
> +	{"WR Ext Spk", NULL, "WR SPO"},
> +
> +	/* HP jack connectors - unknown if we have jack detection */
> +	{ "Headphone Jack", NULL, "HPOL" },
> +	{ "Headphone Jack", NULL, "HPOR" },

like other reviewer commented, there is a mix of spacing that should be 
fixed. OTOH, there are similar mixed styles in existing machine drivers, 
so that's probably where it's coming from, but better to still fix in new 
code.

> +	/* DMIC */
> +	{"DMic", NULL, "SoC DMIC"},

Here's another instance of above.

> +	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
> +		component = pcm->codec_dai->component;
> +		snprintf(jack_name, sizeof(jack_name),
> +			 "HDMI/DP, pcm=%d Jack", pcm->device);
> +		ret = snd_soc_card_jack_new(card, jack_name,
> +					    SND_JACK_AVOUT, &hdmi_jack[i],
> +					    NULL, 0);
> +		if (ret)
> +			return ret;

The HDMI jack registration part will need to be updated once my
series to support snd-hda-codec-hdmi goes in:

https://mailman.alsa-project.org/pipermail/alsa-devel/2019-September/155456.html

... but your patch is ok as it is. I'll send a patch to update this 
machine driver as well if your patch goes in first. There's no harm unless 
you explicitly enable the new HDMI codec driver in Kconfig.

Br, Kai
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel