[alsa-devel] [PATCH v2 0/5] ASoC: Intel: geminilake: Adds da7219+max98537 machine driver support

[alsa-devel] [PATCH v2 0/5] ASoC: Intel: geminilake: Adds da7219+max98537 machine driver support

[Naveen Manohar]

Patch adds required changes in bxt machine & corresponding driver
entry in machine to use MAX98357A codec as speaker on SSP1 and
DA7219 codec as headset on SSP2 for GLK board. 

Naveen Manohar (5):
  ASoC: Intel: broxton: reduce machine name for bxt_da7219_max98357a
  ASoC: Intel: Headset button support in broxton machine driver
  ASoC: Intel: Add Geminilake Dialog Maxim machine driver support
  ASoC: Intel: glk: Add DAI links for Multi-Playback
  ASoC: Intel: Add Geminilake Dialog+Maxim machine driver entry

 sound/soc/intel/boards/bxt_da7219_max98357a.c | 152 ++++++++++++++++++++++----
 sound/soc/intel/skylake/skl.c                 |   9 +-
 2 files changed, 141 insertions(+), 20 deletions(-)

-- 
v1->v2: Unified dai-link usage between bxt & glk mach, corrected jack CTRL
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH v2 1/5] ASoC: Intel: broxton: reduce machine name for bxt_da7219_max98357a

[Naveen Manohar]

Use truncated names in bxt id table and bxt_da7219_max98357a machine
as platform device id table expects names to be less then 20chars.

Signed-off-by: Naveen Manohar <naveen.m@intel.com>
---
 sound/soc/intel/boards/bxt_da7219_max98357a.c | 4 ++--
 sound/soc/intel/skylake/skl.c                 | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c
index 668c093..ce021a3 100644
--- a/sound/soc/intel/boards/bxt_da7219_max98357a.c
+++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c
@@ -586,7 +586,7 @@ static int broxton_audio_probe(struct platform_device *pdev)
 static struct platform_driver broxton_audio = {
 	.probe = broxton_audio_probe,
 	.driver = {
-		.name = "bxt_da7219_max98357a_i2s",
+		.name = "bxt_da7219_max98357a",
 		.pm = &snd_soc_pm_ops,
 	},
 };
@@ -599,4 +599,4 @@ MODULE_AUTHOR("Rohit Ainapure <rohit.m.ainapure@intel.com>");
 MODULE_AUTHOR("Harsha Priya <harshapriya.n@intel.com>");
 MODULE_AUTHOR("Conrad Cooke <conrad.cooke@intel.com>");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:bxt_da7219_max98357a_i2s");
+MODULE_ALIAS("platform:bxt_da7219_max98357a");
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index abf3247..d50c2ee 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -1086,7 +1086,7 @@ static struct snd_soc_acpi_mach sst_bxtp_devdata[] = {
 	},
 	{
 		.id = "DLGS7219",
-		.drv_name = "bxt_da7219_max98357a_i2s",
+		.drv_name = "bxt_da7219_max98357a",
 		.fw_filename = "intel/dsp_fw_bxtn.bin",
 		.machine_quirk = snd_soc_acpi_codec_list,
 		.quirk_data = &bxt_codecs,
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH v2 2/5] ASoC: Intel: Headset button support in broxton machine driver

[Naveen Manohar]

Patch adds headset button support for broxton machine driver
(bxt_da7219_max98357a).

Signed-off-by: Naveen Manohar <naveen.m@intel.com>
---
 sound/soc/intel/boards/bxt_da7219_max98357a.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c
index ce021a3..2eff1f0 100644
--- a/sound/soc/intel/boards/bxt_da7219_max98357a.c
+++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c
@@ -16,6 +16,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/input.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <sound/core.h>
@@ -168,6 +169,7 @@ static int broxton_ssp_fixup(struct snd_soc_pcm_runtime *rtd,
 static int broxton_da7219_codec_init(struct snd_soc_pcm_runtime *rtd)
 {
 	int ret;
+	struct snd_soc_jack *jack;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	struct snd_soc_component *component = rtd->codec_dai->component;
 
@@ -192,7 +194,12 @@ static int broxton_da7219_codec_init(struct snd_soc_pcm_runtime *rtd)
 		return ret;
 	}
 
-	da7219_aad_jack_det(component, &broxton_headset);
+	jack = &broxton_headset;
+	snd_jack_set_key(jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
+	snd_jack_set_key(jack->jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
+	snd_jack_set_key(jack->jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
+	snd_jack_set_key(jack->jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
+	da7219_aad_jack_det(component, jack);
 
 	snd_soc_dapm_ignore_suspend(&rtd->card->dapm, "SoC DMIC");
 
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH v2 3/5] ASoC: Intel: Add Geminilake Dialog Maxim machine driver support

[Naveen Manohar]

Patch adds required changes in bxt machine to use MAX98357A codec
as speaker on SSP1 & DA7219 codec as headset on SSP2 on GLK board.

Signed-off-by: Naveen Manohar <naveen.m@intel.com>
---
 sound/soc/intel/boards/bxt_da7219_max98357a.c | 124 ++++++++++++++++++++++----
 1 file changed, 108 insertions(+), 16 deletions(-)

diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c
index 2eff1f0..6c23aaa 100644
--- a/sound/soc/intel/boards/bxt_da7219_max98357a.c
+++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c
@@ -16,6 +16,7 @@
  * GNU General Public License for more details.
  */
 
+#include <asm/cpu_device_id.h>
 #include <linux/input.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -33,6 +34,7 @@
 #define DUAL_CHANNEL		2
 #define QUAD_CHANNEL		4
 
+static struct snd_soc_card *audio_card;
 static struct snd_soc_jack broxton_headset;
 static struct snd_soc_jack broxton_hdmi[3];
 
@@ -103,7 +105,7 @@ static const struct snd_soc_dapm_widget broxton_widgets[] = {
 			platform_clock_control,	SND_SOC_DAPM_POST_PMD|SND_SOC_DAPM_PRE_PMU),
 };
 
-static const struct snd_soc_dapm_route broxton_map[] = {
+static const struct snd_soc_dapm_route audio_map[] = {
 	/* HP jack connectors - unknown if we have jack detection */
 	{"Headphone Jack", NULL, "HPL"},
 	{"Headphone Jack", NULL, "HPR"},
@@ -118,15 +120,6 @@ static const struct snd_soc_dapm_route broxton_map[] = {
 	{"DMic", NULL, "SoC DMIC"},
 
 	/* CODEC BE connections */
-	{"HiFi Playback", NULL, "ssp5 Tx"},
-	{"ssp5 Tx", NULL, "codec0_out"},
-
-	{"Playback", NULL, "ssp1 Tx"},
-	{"ssp1 Tx", NULL, "codec1_out"},
-
-	{"codec0_in", NULL, "ssp1 Rx"},
-	{"ssp1 Rx", NULL, "Capture"},
-
 	{"HDMI1", NULL, "hif5-0 Output"},
 	{"HDMI2", NULL, "hif6-0 Output"},
 	{"HDMI2", NULL, "hif7-0 Output"},
@@ -146,6 +139,28 @@ static const struct snd_soc_dapm_route broxton_map[] = {
 	{ "Headset Mic", NULL, "Platform Clock" },
 };
 
+static const struct snd_soc_dapm_route broxton_map[] = {
+	{"HiFi Playback", NULL, "ssp5 Tx"},
+	{"ssp5 Tx", NULL, "codec0_out"},
+
+	{"Playback", NULL, "ssp1 Tx"},
+	{"ssp1 Tx", NULL, "codec1_out"},
+
+	{"codec0_in", NULL, "ssp1 Rx"},
+	{"ssp1 Rx", NULL, "Capture"},
+};
+
+static const struct snd_soc_dapm_route gemini_map[] = {
+	{"HiFi Playback", NULL, "ssp1 Tx"},
+	{"ssp1 Tx", NULL, "codec0_out"},
+
+	{"Playback", NULL, "ssp2 Tx"},
+	{"ssp2 Tx", NULL, "codec1_out"},
+
+	{"codec0_in", NULL, "ssp2 Rx"},
+	{"ssp2 Rx", NULL, "Capture"},
+};
+
 static int broxton_ssp_fixup(struct snd_soc_pcm_runtime *rtd,
 			struct snd_pcm_hw_params *params)
 {
@@ -524,6 +539,18 @@ static struct snd_soc_dai_link broxton_dais[] = {
 	},
 };
 
+static int is_geminilake(void)
+{
+	static const struct x86_cpu_id cpu_ids[] = {
+		{ X86_VENDOR_INTEL, 6, 0x7A },	/* Geminilake CPU_ID */
+		{}
+	};
+
+	if (x86_match_cpu(cpu_ids))
+		return true;
+	return false;
+}
+
 #define NAME_SIZE	32
 static int bxt_card_late_probe(struct snd_soc_card *card)
 {
@@ -533,6 +560,13 @@ static int bxt_card_late_probe(struct snd_soc_card *card)
 	int err, i = 0;
 	char jack_name[NAME_SIZE];
 
+	if (is_geminilake())
+		snd_soc_dapm_add_routes(&card->dapm, gemini_map,
+				ARRAY_SIZE(gemini_map));
+	else
+		snd_soc_dapm_add_routes(&card->dapm, broxton_map,
+				ARRAY_SIZE(broxton_map));
+
 	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
 		component = pcm->codec_dai->component;
 		snprintf(jack_name, sizeof(jack_name),
@@ -559,7 +593,7 @@ static int bxt_card_late_probe(struct snd_soc_card *card)
 }
 
 /* broxton audio machine driver for SPT + da7219 */
-static struct snd_soc_card broxton_audio_card = {
+static struct snd_soc_card bxt_audio_card_da7219_m98357a = {
 	.name = "bxtda7219max",
 	.owner = THIS_MODULE,
 	.dai_link = broxton_dais,
@@ -568,15 +602,35 @@ static struct snd_soc_card broxton_audio_card = {
 	.num_controls = ARRAY_SIZE(broxton_controls),
 	.dapm_widgets = broxton_widgets,
 	.num_dapm_widgets = ARRAY_SIZE(broxton_widgets),
-	.dapm_routes = broxton_map,
-	.num_dapm_routes = ARRAY_SIZE(broxton_map),
+	.dapm_routes = audio_map,
+	.num_dapm_routes = ARRAY_SIZE(audio_map),
 	.fully_routed = true,
 	.late_probe = bxt_card_late_probe,
 };
 
+/* geminilake audio machine driver for SPT + DA7219 */
+static struct snd_soc_card glk_audio_card_da7219_m98357a = {
+	.name = "glkda7219max",
+	.owner = THIS_MODULE,
+	.dai_link = broxton_dais,
+	.num_links = ARRAY_SIZE(broxton_dais),
+	.controls = broxton_controls,
+	.num_controls = ARRAY_SIZE(broxton_controls),
+	.dapm_widgets = broxton_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(broxton_widgets),
+	.dapm_routes = audio_map,
+	.num_dapm_routes = ARRAY_SIZE(audio_map),
+	.fully_routed = true,
+	.late_probe = bxt_card_late_probe,
+};
+
+static char glk_spk_dai_name[10];
+static char glk_hs_dai_name[10];
+
 static int broxton_audio_probe(struct platform_device *pdev)
 {
 	struct bxt_card_private *ctx;
+	int dai_index = 8;
 
 	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
 	if (!ctx)
@@ -584,18 +638,54 @@ static int broxton_audio_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&ctx->hdmi_pcm_list);
 
-	broxton_audio_card.dev = &pdev->dev;
-	snd_soc_card_set_drvdata(&broxton_audio_card, ctx);
+	audio_card =
+		(struct snd_soc_card *)pdev->id_entry->driver_data;
+
+	audio_card->dev = &pdev->dev;
+	snd_soc_card_set_drvdata(audio_card, ctx);
+
+	if (is_geminilake()) {
+		/* fixup name & cpu_dai_name for SPK */
+		snprintf(glk_spk_dai_name,
+			sizeof(glk_spk_dai_name), "%s", "SSP1-Codec");
+		broxton_dais[dai_index].name = glk_spk_dai_name;
+		snprintf(glk_spk_dai_name,
+			sizeof(glk_spk_dai_name), "%s", "SSP1 Pin");
+		broxton_dais[dai_index].cpu_dai_name = glk_spk_dai_name;
+		/* fixup name & cpu_dai_name for HS*/
+		dai_index++;
+		snprintf(glk_hs_dai_name,
+			sizeof(glk_hs_dai_name), "%s", "SSP2-Codec");
+		broxton_dais[dai_index].name = glk_hs_dai_name;
+		snprintf(glk_hs_dai_name,
+			sizeof(glk_hs_dai_name), "%s", "SSP2 Pin");
+		broxton_dais[dai_index].cpu_dai_name = glk_hs_dai_name;
+	}
 
-	return devm_snd_soc_register_card(&pdev->dev, &broxton_audio_card);
+	return devm_snd_soc_register_card(&pdev->dev, audio_card);
 }
 
+static const struct platform_device_id bxt_board_ids[] = {
+	{
+		.name = "bxt_da7219_max98357a",
+		.driver_data =
+			(kernel_ulong_t)&bxt_audio_card_da7219_m98357a,
+	},
+	{
+		.name = "glk_da7219_max98357a",
+		.driver_data =
+			(kernel_ulong_t)&glk_audio_card_da7219_m98357a,
+	},
+	{ }
+};
+
 static struct platform_driver broxton_audio = {
 	.probe = broxton_audio_probe,
 	.driver = {
 		.name = "bxt_da7219_max98357a",
 		.pm = &snd_soc_pm_ops,
 	},
+	.id_table = bxt_board_ids,
 };
 module_platform_driver(broxton_audio)
 
@@ -605,5 +695,7 @@ MODULE_AUTHOR("Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>");
 MODULE_AUTHOR("Rohit Ainapure <rohit.m.ainapure@intel.com>");
 MODULE_AUTHOR("Harsha Priya <harshapriya.n@intel.com>");
 MODULE_AUTHOR("Conrad Cooke <conrad.cooke@intel.com>");
+MODULE_AUTHOR("Naveen Manohar <naveen.m@intel.com>");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:bxt_da7219_max98357a");
+MODULE_ALIAS("platform:glk_da7219_max98357a");
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH v2 4/5] ASoC: Intel: glk: Add DAI links for Multi-Playback

[Naveen Manohar]

Add new FE DAI link to support: Parallel playback on 2 ports simultaneously

Signed-off-by: Naveen Manohar <naveen.m@intel.com>
---
 sound/soc/intel/boards/bxt_da7219_max98357a.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c
index 6c23aaa..860f94a 100644
--- a/sound/soc/intel/boards/bxt_da7219_max98357a.c
+++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c
@@ -51,6 +51,7 @@ struct bxt_card_private {
 enum {
 	BXT_DPCM_AUDIO_PB = 0,
 	BXT_DPCM_AUDIO_CP,
+	BXT_DPCM_AUDIO_HS_PB,
 	BXT_DPCM_AUDIO_REF_CP,
 	BXT_DPCM_AUDIO_DMIC_CP,
 	BXT_DPCM_AUDIO_HDMI1_PB,
@@ -391,6 +392,20 @@ static struct snd_soc_dai_link broxton_dais[] = {
 		.dpcm_capture = 1,
 		.ops = &broxton_da7219_fe_ops,
 	},
+	[BXT_DPCM_AUDIO_HS_PB] = {
+		.name = "Bxt Audio Headset Playback",
+		.stream_name = "Headset Playback",
+		.cpu_dai_name = "System Pin2",
+		.platform_name = "0000:00:0e.0",
+		.dynamic = 1,
+		.codec_name = "snd-soc-dummy",
+		.codec_dai_name = "snd-soc-dummy-dai",
+		.nonatomic = 1,
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.dpcm_playback = 1,
+		.ops = &broxton_da7219_fe_ops,
+	},
 	[BXT_DPCM_AUDIO_REF_CP] =
 	{
 		.name = "Bxt Audio Reference cap",
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH v2 5/5] ASoC: Intel: Add Geminilake Dialog+Maxim machine driver entry

[Naveen Manohar]

This patch adds da7219_max98357a machine driver entry into
machine table

Signed-off-by: Naveen Manohar <naveen.m@intel.com>
---
 sound/soc/intel/skylake/skl.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index d50c2ee..96eeb1d 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -1155,6 +1155,13 @@ static struct snd_soc_acpi_mach sst_glk_devdata[] = {
 		.drv_name = "glk_alc298s_i2s",
 		.fw_filename = "intel/dsp_fw_glk.bin",
 	},
+	{
+		.id = "DLGS7219",
+		.drv_name = "glk_da7219_max98357a",
+		.fw_filename = "intel/dsp_fw_glk.bin",
+		.machine_quirk = snd_soc_acpi_codec_list,
+		.quirk_data = &bxt_codecs,
+	},
 	{}
 };
 
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2 3/5] ASoC: Intel: Add Geminilake Dialog Maxim machine driver support

[Pierre-Louis Bossart]

  geminilake audio machine driver for SPT + DA7219 */
> +static struct snd_soc_card glk_audio_card_da7219_m98357a = {
> +	.name = "glkda7219max",
> +	.owner = THIS_MODULE,
> +	.dai_link = broxton_dais,
> +	.num_links = ARRAY_SIZE(broxton_dais),
> +	.controls = broxton_controls,
> +	.num_controls = ARRAY_SIZE(broxton_controls),
> +	.dapm_widgets = broxton_widgets,
> +	.num_dapm_widgets = ARRAY_SIZE(broxton_widgets),
> +	.dapm_routes = audio_map,
> +	.num_dapm_routes = ARRAY_SIZE(audio_map),
> +	.fully_routed = true,
> +	.late_probe = bxt_card_late_probe,
> +};
> +
> +static char glk_spk_dai_name[10];
> +static char glk_hs_dai_name[10];

Off-by-one? "SSPx-Codec" would be 11 chars if you include null termination.

> +
>   static int broxton_audio_probe(struct platform_device *pdev)
>   {
>   	struct bxt_card_private *ctx;
> +	int dai_index = 8;

maybe better to do an explicit search that hard-code values?
This will actually not work if you have an additional FE for headset on 
GLK, or you are assuming a dependency on a patch added later in the 
series - not good.

>   
>   	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
>   	if (!ctx)
> @@ -584,18 +638,54 @@ static int broxton_audio_probe(struct platform_device *pdev)
>   
>   	INIT_LIST_HEAD(&ctx->hdmi_pcm_list);
>   
> -	broxton_audio_card.dev = &pdev->dev;
> -	snd_soc_card_set_drvdata(&broxton_audio_card, ctx);
> +	audio_card =
> +		(struct snd_soc_card *)pdev->id_entry->driver_data;
> +
> +	audio_card->dev = &pdev->dev;
> +	snd_soc_card_set_drvdata(audio_card, ctx);
> +
> +	if (is_geminilake()) {
> +		/* fixup name & cpu_dai_name for SPK */
> +		snprintf(glk_spk_dai_name,
> +			sizeof(glk_spk_dai_name), "%s", "SSP1-Codec");
> +		broxton_dais[dai_index].name = glk_spk_dai_name;
> +		snprintf(glk_spk_dai_name,
> +			sizeof(glk_spk_dai_name), "%s", "SSP1 Pin");
> +		broxton_dais[dai_index].cpu_dai_name = glk_spk_dai_name;

does this work? You have the dai_name and cpu_dai_name pointing to the 
same string?

> +		/* fixup name & cpu_dai_name for HS*/
> +		dai_index++;
> +		snprintf(glk_hs_dai_name,
> +			sizeof(glk_hs_dai_name), "%s", "SSP2-Codec");
> +		broxton_dais[dai_index].name = glk_hs_dai_name;
> +		snprintf(glk_hs_dai_name,
> +			sizeof(glk_hs_dai_name), "%s", "SSP2 Pin");
> +		broxton_dais[dai_index].cpu_dai_name = glk_hs_dai_name;

same here?

> +	}
>   
> -	return devm_snd_soc_register_card(&pdev->dev, &broxton_audio_card);
> +	return devm_snd_soc_register_card(&pdev->dev, audio_card);
>   }
>   
> +static const struct platform_device_id bxt_board_ids[] = {
> +	{
> +		.name = "bxt_da7219_max98357a",
> +		.driver_data =
> +			(kernel_ulong_t)&bxt_audio_card_da7219_m98357a,
> +	},
> +	{
> +		.name = "glk_da7219_max98357a",
> +		.driver_data =
> +			(kernel_ulong_t)&glk_audio_card_da7219_m98357a,
> +	},
> +	{ }
> +};
> +
>   static struct platform_driver broxton_audio = {
>   	.probe = broxton_audio_probe,
>   	.driver = {
>   		.name = "bxt_da7219_max98357a",
>   		.pm = &snd_soc_pm_ops,
>   	},
> +	.id_table = bxt_board_ids,
>   };
>   module_platform_driver(broxton_audio)
>   
> @@ -605,5 +695,7 @@ MODULE_AUTHOR("Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>");
>   MODULE_AUTHOR("Rohit Ainapure <rohit.m.ainapure@intel.com>");
>   MODULE_AUTHOR("Harsha Priya <harshapriya.n@intel.com>");
>   MODULE_AUTHOR("Conrad Cooke <conrad.cooke@intel.com>");
> +MODULE_AUTHOR("Naveen Manohar <naveen.m@intel.com>");
>   MODULE_LICENSE("GPL v2");
>   MODULE_ALIAS("platform:bxt_da7219_max98357a");
> +MODULE_ALIAS("platform:glk_da7219_max98357a");
> 

Re: [PATCH v2 3/5] ASoC: Intel: Add Geminilake Dialog Maxim machine driver support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 689 bytes --]

On Wed, Apr 11, 2018 at 11:20:10AM -0500, Pierre-Louis Bossart wrote:

> > +static char glk_spk_dai_name[10];
> > +static char glk_hs_dai_name[10];

> Off-by-one? "SSPx-Codec" would be 11 chars if you include null termination.

Or avoid needing global statics entirely...

> >   static int broxton_audio_probe(struct platform_device *pdev)
> >   {
> >   	struct bxt_card_private *ctx;
> > +	int dai_index = 8;

> maybe better to do an explicit search that hard-code values?
> This will actually not work if you have an additional FE for headset on GLK,
> or you are assuming a dependency on a patch added later in the series - not
> good.

Definitely, this is just a recipe for fragility.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 3/5] ASoC: Intel: Add Geminilake Dialog Maxim machine driver support

[Pierre-Louis Bossart]

On 4/11/18 11:34 AM, Mark Brown wrote:
> On Wed, Apr 11, 2018 at 11:20:10AM -0500, Pierre-Louis Bossart wrote:
> 
>>> +static char glk_spk_dai_name[10];
>>> +static char glk_hs_dai_name[10];
> 
>> Off-by-one? "SSPx-Codec" would be 11 chars if you include null termination.
> 
> Or avoid needing global statics entirely...
> 
>>>    static int broxton_audio_probe(struct platform_device *pdev)
>>>    {
>>>    	struct bxt_card_private *ctx;
>>> +	int dai_index = 8;
> 
>> maybe better to do an explicit search that hard-code values?
>> This will actually not work if you have an additional FE for headset on GLK,
>> or you are assuming a dependency on a patch added later in the series - not
>> good.
> 
> Definitely, this is just a recipe for fragility.

Which reminds me btw that we talked about adding a helper to dynamically 
change fields in a dailink (codec/cpu/dai names) instead of doing this 
in multiple machine drivers, e.g.

snd_soc_fixup_dailink(bxt_dailink, device_name, CPU_DAI_NAME, "Pin1");
snd_soc_fixup_dailink(bxt_dailink, device_name, DAI_NAME, "SSP5-Codec");
snd_soc_fixup_dailink(bxt_dailink, device_name, CODEC_NAME, hid-name);

Re: [PATCH v2 3/5] ASoC: Intel: Add Geminilake Dialog Maxim machine driver support

[Naveen M]

On Wed, Apr 11, 2018 at 12:06:52PM -0500, Pierre-Louis Bossart wrote:
> On 4/11/18 11:34 AM, Mark Brown wrote:
> >On Wed, Apr 11, 2018 at 11:20:10AM -0500, Pierre-Louis Bossart wrote:
> >
> >>>+static char glk_spk_dai_name[10];
> >>>+static char glk_hs_dai_name[10];
> >
> >>Off-by-one? "SSPx-Codec" would be 11 chars if you include null termination.
> >
> >Or avoid needing global statics entirely...
> >
> >>>   static int broxton_audio_probe(struct platform_device *pdev)
> >>>   {
> >>>   	struct bxt_card_private *ctx;
> >>>+	int dai_index = 8;
> >
> >>maybe better to do an explicit search that hard-code values?
> >>This will actually not work if you have an additional FE for headset on GLK,
> >>or you are assuming a dependency on a patch added later in the series - not
> >>good.
> >
> >Definitely, this is just a recipe for fragility.
> 
> Which reminds me btw that we talked about adding a helper to dynamically
> change fields in a dailink (codec/cpu/dai names) instead of doing this in
> multiple machine drivers, e.g.
> 
> snd_soc_fixup_dailink(bxt_dailink, device_name, CPU_DAI_NAME, "Pin1");
> snd_soc_fixup_dailink(bxt_dailink, device_name, DAI_NAME, "SSP5-Codec");
> snd_soc_fixup_dailink(bxt_dailink, device_name, CODEC_NAME, hid-name);
Thanks I Got it, will modify the patch and update.
--