All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update
@ 2022-06-13  9:15 Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 01/17] ASoC: Intel: Rename haswell source file to hsw_rt5640 Cezary Rojewski
                   ` (17 more replies)
  0 siblings, 18 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

A number of patches improving overall quality and readability of
haswell.c and broadwell.c source files found in sound/soc/intel/boards.
Both files are first renamed and only then actual changes are being
incrementally added. The respective names are: hsw_rt5640 and bdw_rt286
to match the pattern found in more recent boards.

Most patches bring no functional change - the more impactful patches at
are placed the end:

Refactor of suspend/resume flow for the bdw_rt286 board by dropping
dev->remove() in favour of card->remove() and adjust jack handling to
reduce code size slightly by implementing card_set_jack().

The last patch is removing of FE DAI ops. Given the existence of
platform FE DAI capabilities (either static declaration or through
topology file), this code is redundant.


Changes in v2:
- fixed wording error in patch 02/17 so it correctly mentions
  'haswell_rt5640', not 'broadwell_rt286'
- decided not to add kernel module names changes to this patchset so the
  review is not complicated unnecessarily. Will send them separately
  instead


Cezary Rojewski (17):
  ASoC: Intel: Rename haswell source file to hsw_rt5640
  ASoC: Intel: hsw_rt5640: Reword prefixes of all driver members
  ASoC: Intel: hsw_rt5640: Reword driver name
  ASoC: Intel: hsw_rt5640: Update code indentation
  ASoC: Intel: hsw_rt5640: Update file comments
  ASoC: Intel: hsw_rt5640: Improve probe() function quality
  ASoC: Intel: hsw_rt5640: Improve hw_params() debug-ability
  ASoC: Intel: Rename broadwell source file to bdw_rt286
  ASoC: Intel: bdw_rt286: Reword prefixes of all driver members
  ASoC: Intel: bdw_rt286: Reword driver name
  ASoC: Intel: bdw_rt286: Update code indentation
  ASoC: Intel: bdw_rt286: Update file comments
  ASoC: Intel: bdw_rt286: Improve probe() function quality
  ASoC: Intel: bdw_rt286: Improve hw_params() debug-ability
  ASoC: Intel: bdw_rt286: Improve codec_link_init() quality
  ASoC: Intel: bdw_rt286: Refactor suspend/resume
  ASoC: Intel: bdw_rt286: Remove FE DAI ops

 sound/soc/intel/boards/Kconfig                |   4 +-
 sound/soc/intel/boards/Makefile               |   4 +-
 sound/soc/intel/boards/bdw_rt286.c            | 257 +++++++++++++
 sound/soc/intel/boards/broadwell.c            | 338 ------------------
 sound/soc/intel/boards/haswell.c              | 202 -----------
 sound/soc/intel/boards/hsw_rt5640.c           | 176 +++++++++
 .../common/soc-acpi-intel-hsw-bdw-match.c     |   6 +-
 7 files changed, 440 insertions(+), 547 deletions(-)
 create mode 100644 sound/soc/intel/boards/bdw_rt286.c
 delete mode 100644 sound/soc/intel/boards/broadwell.c
 delete mode 100644 sound/soc/intel/boards/haswell.c
 create mode 100644 sound/soc/intel/boards/hsw_rt5640.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 01/17] ASoC: Intel: Rename haswell source file to hsw_rt5640
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 02/17] ASoC: Intel: hsw_rt5640: Reword prefixes of all driver members Cezary Rojewski
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Rename source file to drop any ambiguity.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/Makefile                    | 2 +-
 sound/soc/intel/boards/{haswell.c => hsw_rt5640.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename sound/soc/intel/boards/{haswell.c => hsw_rt5640.c} (100%)

diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
index 40c0c3d1c500..e479546a3d4b 100644
--- a/sound/soc/intel/boards/Makefile
+++ b/sound/soc/intel/boards/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
-snd-soc-sst-haswell-objs := haswell.o
+snd-soc-sst-haswell-objs := hsw_rt5640.o
 snd-soc-sst-bdw-rt5650-mach-objs := bdw-rt5650.o
 snd-soc-sst-bdw-rt5677-mach-objs := bdw-rt5677.o
 snd-soc-sst-broadwell-objs := broadwell.o
diff --git a/sound/soc/intel/boards/haswell.c b/sound/soc/intel/boards/hsw_rt5640.c
similarity index 100%
rename from sound/soc/intel/boards/haswell.c
rename to sound/soc/intel/boards/hsw_rt5640.c
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 02/17] ASoC: Intel: hsw_rt5640: Reword prefixes of all driver members
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 01/17] ASoC: Intel: Rename haswell source file to hsw_rt5640 Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 03/17] ASoC: Intel: hsw_rt5640: Reword driver name Cezary Rojewski
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Replace ambiguous 'haswell_rt5640_' prefixes in favour of 'card_',
'link_' and other similar strings to clearly state which object given
member implements behavior for.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/hsw_rt5640.c | 46 ++++++++++++++---------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c
index aa61e101f793..b51ce8d0ca22 100644
--- a/sound/soc/intel/boards/hsw_rt5640.c
+++ b/sound/soc/intel/boards/hsw_rt5640.c
@@ -16,12 +16,12 @@
 #include "../../codecs/rt5640.h"
 
 /* Haswell ULT platforms have a Headphone and Mic jack */
-static const struct snd_soc_dapm_widget haswell_widgets[] = {
+static const struct snd_soc_dapm_widget card_widgets[] = {
 	SND_SOC_DAPM_HP("Headphones", NULL),
 	SND_SOC_DAPM_MIC("Mic", NULL),
 };
 
-static const struct snd_soc_dapm_route haswell_rt5640_map[] = {
+static const struct snd_soc_dapm_route card_routes[] = {
 
 	{"Headphones", NULL, "HPOR"},
 	{"Headphones", NULL, "HPOL"},
@@ -32,7 +32,7 @@ static const struct snd_soc_dapm_route haswell_rt5640_map[] = {
 	{"AIF1 Playback", NULL, "SSP0 CODEC OUT"},
 };
 
-static int haswell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd,
+static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 			struct snd_pcm_hw_params *params)
 {
 	struct snd_interval *rate = hw_param_interval(params,
@@ -49,7 +49,7 @@ static int haswell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd,
 	return 0;
 }
 
-static int haswell_rt5640_hw_params(struct snd_pcm_substream *substream,
+static int codec_link_hw_params(struct snd_pcm_substream *substream,
 	struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
@@ -70,8 +70,8 @@ static int haswell_rt5640_hw_params(struct snd_pcm_substream *substream,
 	return ret;
 }
 
-static const struct snd_soc_ops haswell_rt5640_ops = {
-	.hw_params = haswell_rt5640_hw_params,
+static const struct snd_soc_ops codec_link_ops = {
+	.hw_params = codec_link_hw_params,
 };
 
 SND_SOC_DAILINK_DEF(dummy,
@@ -98,7 +98,7 @@ SND_SOC_DAILINK_DEF(platform,
 SND_SOC_DAILINK_DEF(ssp0_port,
 	    DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
 
-static struct snd_soc_dai_link haswell_rt5640_dais[] = {
+static struct snd_soc_dai_link card_dai_links[] = {
 	/* Front End DAI links */
 	{
 		.name = "System",
@@ -147,8 +147,8 @@ static struct snd_soc_dai_link haswell_rt5640_dais[] = {
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			SND_SOC_DAIFMT_CBC_CFC,
 		.ignore_pmdown_time = 1,
-		.be_hw_params_fixup = haswell_ssp0_fixup,
-		.ops = &haswell_rt5640_ops,
+		.be_hw_params_fixup = codec_link_hw_params_fixup,
+		.ops = &codec_link_ops,
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
 		SND_SOC_DAILINK_REG(ssp0_port, codec, platform),
@@ -156,44 +156,44 @@ static struct snd_soc_dai_link haswell_rt5640_dais[] = {
 };
 
 /* audio machine driver for Haswell Lynxpoint DSP + RT5640 */
-static struct snd_soc_card haswell_rt5640 = {
+static struct snd_soc_card hsw_rt5640_card = {
 	.name = "haswell-rt5640",
 	.owner = THIS_MODULE,
-	.dai_link = haswell_rt5640_dais,
-	.num_links = ARRAY_SIZE(haswell_rt5640_dais),
-	.dapm_widgets = haswell_widgets,
-	.num_dapm_widgets = ARRAY_SIZE(haswell_widgets),
-	.dapm_routes = haswell_rt5640_map,
-	.num_dapm_routes = ARRAY_SIZE(haswell_rt5640_map),
+	.dai_link = card_dai_links,
+	.num_links = ARRAY_SIZE(card_dai_links),
+	.dapm_widgets = card_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(card_widgets),
+	.dapm_routes = card_routes,
+	.num_dapm_routes = ARRAY_SIZE(card_routes),
 	.fully_routed = true,
 };
 
-static int haswell_audio_probe(struct platform_device *pdev)
+static int hsw_rt5640_probe(struct platform_device *pdev)
 {
 	struct snd_soc_acpi_mach *mach;
 	int ret;
 
-	haswell_rt5640.dev = &pdev->dev;
+	hsw_rt5640_card.dev = &pdev->dev;
 
 	/* override platform name, if required */
 	mach = pdev->dev.platform_data;
-	ret = snd_soc_fixup_dai_links_platform_name(&haswell_rt5640,
+	ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card,
 						    mach->mach_params.platform);
 	if (ret)
 		return ret;
 
-	return devm_snd_soc_register_card(&pdev->dev, &haswell_rt5640);
+	return devm_snd_soc_register_card(&pdev->dev, &hsw_rt5640_card);
 }
 
-static struct platform_driver haswell_audio = {
-	.probe = haswell_audio_probe,
+static struct platform_driver hsw_rt5640_driver = {
+	.probe = hsw_rt5640_probe,
 	.driver = {
 		.name = "haswell-audio",
 		.pm = &snd_soc_pm_ops,
 	},
 };
 
-module_platform_driver(haswell_audio)
+module_platform_driver(hsw_rt5640_driver)
 
 /* Module information */
 MODULE_AUTHOR("Liam Girdwood, Xingchao Wang");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 03/17] ASoC: Intel: hsw_rt5640: Reword driver name
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 01/17] ASoC: Intel: Rename haswell source file to hsw_rt5640 Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 02/17] ASoC: Intel: hsw_rt5640: Reword prefixes of all driver members Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 04/17] ASoC: Intel: hsw_rt5640: Update code indentation Cezary Rojewski
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Align with other Intel boards naming convention and let the name
explicitly state which components are being connected.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/hsw_rt5640.c                   | 4 ++--
 sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c
index b51ce8d0ca22..a096453bf1df 100644
--- a/sound/soc/intel/boards/hsw_rt5640.c
+++ b/sound/soc/intel/boards/hsw_rt5640.c
@@ -188,7 +188,7 @@ static int hsw_rt5640_probe(struct platform_device *pdev)
 static struct platform_driver hsw_rt5640_driver = {
 	.probe = hsw_rt5640_probe,
 	.driver = {
-		.name = "haswell-audio",
+		.name = "hsw_rt5640",
 		.pm = &snd_soc_pm_ops,
 	},
 };
@@ -199,4 +199,4 @@ module_platform_driver(hsw_rt5640_driver)
 MODULE_AUTHOR("Liam Girdwood, Xingchao Wang");
 MODULE_DESCRIPTION("Intel SST Audio for Haswell Lynxpoint");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:haswell-audio");
+MODULE_ALIAS("platform:hsw_rt5640");
diff --git a/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c b/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c
index 0441df97b260..4e00f8f6c521 100644
--- a/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c
@@ -12,7 +12,7 @@
 struct snd_soc_acpi_mach snd_soc_acpi_intel_haswell_machines[] = {
 	{
 		.id = "INT33CA",
-		.drv_name = "haswell-audio",
+		.drv_name = "hsw_rt5640",
 		.fw_filename = "intel/IntcSST1.bin",
 		.sof_tplg_filename = "sof-hsw.tplg",
 	},
@@ -41,7 +41,7 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_broadwell_machines[] = {
 	},
 	{
 		.id = "INT33CA",
-		.drv_name = "haswell-audio",
+		.drv_name = "hsw_rt5640",
 		.fw_filename = "intel/IntcSST2.bin",
 		.sof_tplg_filename = "sof-bdw-rt5640.tplg",
 	},
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 04/17] ASoC: Intel: hsw_rt5640: Update code indentation
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (2 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 03/17] ASoC: Intel: hsw_rt5640: Reword driver name Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 05/17] ASoC: Intel: hsw_rt5640: Update file comments Cezary Rojewski
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Make use of 100 character limit and modify indentation so code is easier
to read. While at it, sort includes in alphabetical order.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/hsw_rt5640.c | 67 ++++++++++-------------------
 1 file changed, 22 insertions(+), 45 deletions(-)

diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c
index a096453bf1df..69be5086b98d 100644
--- a/sound/soc/intel/boards/hsw_rt5640.c
+++ b/sound/soc/intel/boards/hsw_rt5640.c
@@ -9,10 +9,9 @@
 #include <linux/platform_device.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
+#include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/soc-acpi.h>
-#include <sound/pcm_params.h>
-
 #include "../../codecs/rt5640.h"
 
 /* Haswell ULT platforms have a Headphone and Mic jack */
@@ -22,7 +21,6 @@ static const struct snd_soc_dapm_widget card_widgets[] = {
 };
 
 static const struct snd_soc_dapm_route card_routes[] = {
-
 	{"Headphones", NULL, "HPOR"},
 	{"Headphones", NULL, "HPOL"},
 	{"IN2P", NULL, "Mic"},
@@ -33,32 +31,28 @@ static const struct snd_soc_dapm_route card_routes[] = {
 };
 
 static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
-			struct snd_pcm_hw_params *params)
+				      struct snd_pcm_hw_params *params)
 {
-	struct snd_interval *rate = hw_param_interval(params,
-			SNDRV_PCM_HW_PARAM_RATE);
-	struct snd_interval *channels = hw_param_interval(params,
-						SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
 
 	/* The ADSP will covert the FE rate to 48k, stereo */
 	rate->min = rate->max = 48000;
 	channels->min = channels->max = 2;
-
 	/* set SSP0 to 16 bit */
 	params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
+
 	return 0;
 }
 
 static int codec_link_hw_params(struct snd_pcm_substream *substream,
-	struct snd_pcm_hw_params *params)
+				struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
 	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
 	int ret;
 
-	ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_MCLK, 12288000,
-		SND_SOC_CLOCK_IN);
-
+	ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_MCLK, 12288000, SND_SOC_CLOCK_IN);
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
 		return ret;
@@ -74,29 +68,15 @@ static const struct snd_soc_ops codec_link_ops = {
 	.hw_params = codec_link_hw_params,
 };
 
-SND_SOC_DAILINK_DEF(dummy,
-	DAILINK_COMP_ARRAY(COMP_DUMMY()));
-
-SND_SOC_DAILINK_DEF(system,
-	DAILINK_COMP_ARRAY(COMP_CPU("System Pin")));
+SND_SOC_DAILINK_DEF(system, DAILINK_COMP_ARRAY(COMP_CPU("System Pin")));
+SND_SOC_DAILINK_DEF(offload0, DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin")));
+SND_SOC_DAILINK_DEF(offload1, DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin")));
+SND_SOC_DAILINK_DEF(loopback, DAILINK_COMP_ARRAY(COMP_CPU("Loopback Pin")));
 
-SND_SOC_DAILINK_DEF(offload0,
-	DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin")));
-
-SND_SOC_DAILINK_DEF(offload1,
-	DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin")));
-
-SND_SOC_DAILINK_DEF(loopback,
-	DAILINK_COMP_ARRAY(COMP_CPU("Loopback Pin")));
-
-SND_SOC_DAILINK_DEF(codec,
-	DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT33CA:00", "rt5640-aif1")));
-
-SND_SOC_DAILINK_DEF(platform,
-	DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audio")));
-
-SND_SOC_DAILINK_DEF(ssp0_port,
-	    DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
+SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY()));
+SND_SOC_DAILINK_DEF(codec, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT33CA:00", "rt5640-aif1")));
+SND_SOC_DAILINK_DEF(platform, DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audio")));
+SND_SOC_DAILINK_DEF(ssp0_port, DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
 
 static struct snd_soc_dai_link card_dai_links[] = {
 	/* Front End DAI links */
@@ -105,7 +85,7 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.stream_name = "System Playback/Capture",
 		.nonatomic = 1,
 		.dynamic = 1,
-		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST },
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
 		SND_SOC_DAILINK_REG(system, dummy, platform),
@@ -115,7 +95,7 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.stream_name = "Offload0 Playback",
 		.nonatomic = 1,
 		.dynamic = 1,
-		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST },
 		.dpcm_playback = 1,
 		SND_SOC_DAILINK_REG(offload0, dummy, platform),
 	},
@@ -124,7 +104,7 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.stream_name = "Offload1 Playback",
 		.nonatomic = 1,
 		.dynamic = 1,
-		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST },
 		.dpcm_playback = 1,
 		SND_SOC_DAILINK_REG(offload1, dummy, platform),
 	},
@@ -133,19 +113,17 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.stream_name = "Loopback",
 		.nonatomic = 1,
 		.dynamic = 1,
-		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST },
 		.dpcm_capture = 1,
 		SND_SOC_DAILINK_REG(loopback, dummy, platform),
 	},
-
 	/* Back End DAI links */
 	{
 		/* SSP0 - Codec */
 		.name = "Codec",
 		.id = 0,
 		.no_pcm = 1,
-		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
-			SND_SOC_DAIFMT_CBC_CFC,
+		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC,
 		.ignore_pmdown_time = 1,
 		.be_hw_params_fixup = codec_link_hw_params_fixup,
 		.ops = &codec_link_ops,
@@ -174,11 +152,10 @@ static int hsw_rt5640_probe(struct platform_device *pdev)
 	int ret;
 
 	hsw_rt5640_card.dev = &pdev->dev;
-
 	/* override platform name, if required */
 	mach = pdev->dev.platform_data;
-	ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card,
-						    mach->mach_params.platform);
+
+	ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card, mach->mach_params.platform);
 	if (ret)
 		return ret;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 05/17] ASoC: Intel: hsw_rt5640: Update file comments
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (3 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 04/17] ASoC: Intel: hsw_rt5640: Update code indentation Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 06/17] ASoC: Intel: hsw_rt5640: Improve probe() function quality Cezary Rojewski
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Drop redundant and update valuable comments within the file to increase
readability. This patch also revisits module information and kconfig
help strings.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/Kconfig      |  2 +-
 sound/soc/intel/boards/hsw_rt5640.c | 16 ++++++----------
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index 4b4c1e1e4808..817b4c04bf6a 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -41,7 +41,7 @@ config SND_SOC_INTEL_SOF_CIRRUS_COMMON
 if SND_SOC_INTEL_CATPT
 
 config SND_SOC_INTEL_HASWELL_MACH
-	tristate "Haswell Lynxpoint"
+	tristate "Haswell with RT5640 I2S codec"
 	depends on I2C
 	depends on I2C_DESIGNWARE_PLATFORM || COMPILE_TEST
 	depends on X86_INTEL_LPSS || COMPILE_TEST
diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c
index 69be5086b98d..0cd788a73694 100644
--- a/sound/soc/intel/boards/hsw_rt5640.c
+++ b/sound/soc/intel/boards/hsw_rt5640.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Intel Haswell Lynxpoint SST Audio
+ * Sound card driver for Intel Haswell Lynx Point with Realtek 5640
  *
  * Copyright (C) 2013, Intel Corporation. All rights reserved.
  */
@@ -14,7 +14,6 @@
 #include <sound/soc-acpi.h>
 #include "../../codecs/rt5640.h"
 
-/* Haswell ULT platforms have a Headphone and Mic jack */
 static const struct snd_soc_dapm_widget card_widgets[] = {
 	SND_SOC_DAPM_HP("Headphones", NULL),
 	SND_SOC_DAPM_MIC("Mic", NULL),
@@ -36,10 +35,10 @@ static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 	struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
 	struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
 
-	/* The ADSP will covert the FE rate to 48k, stereo */
+	/* The ADSP will convert the FE rate to 48k, stereo. */
 	rate->min = rate->max = 48000;
 	channels->min = channels->max = 2;
-	/* set SSP0 to 16 bit */
+	/* Set SSP0 to 16 bit. */
 	params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
 
 	return 0;
@@ -58,7 +57,7 @@ static int codec_link_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	/* set correct codec filter for DAI format and clock config */
+	/* Set correct codec filter for DAI format and clock config. */
 	snd_soc_component_update_bits(codec_dai->component, 0x83, 0xffff, 0x8000);
 
 	return ret;
@@ -133,7 +132,6 @@ static struct snd_soc_dai_link card_dai_links[] = {
 	},
 };
 
-/* audio machine driver for Haswell Lynxpoint DSP + RT5640 */
 static struct snd_soc_card hsw_rt5640_card = {
 	.name = "haswell-rt5640",
 	.owner = THIS_MODULE,
@@ -152,7 +150,6 @@ static int hsw_rt5640_probe(struct platform_device *pdev)
 	int ret;
 
 	hsw_rt5640_card.dev = &pdev->dev;
-	/* override platform name, if required */
 	mach = pdev->dev.platform_data;
 
 	ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card, mach->mach_params.platform);
@@ -172,8 +169,7 @@ static struct platform_driver hsw_rt5640_driver = {
 
 module_platform_driver(hsw_rt5640_driver)
 
-/* Module information */
 MODULE_AUTHOR("Liam Girdwood, Xingchao Wang");
-MODULE_DESCRIPTION("Intel SST Audio for Haswell Lynxpoint");
-MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Sound card driver for Intel Haswell Lynx Point with Realtek 5640");
+MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:hsw_rt5640");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 06/17] ASoC: Intel: hsw_rt5640: Improve probe() function quality
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (4 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 05/17] ASoC: Intel: hsw_rt5640: Update file comments Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 07/17] ASoC: Intel: hsw_rt5640: Improve hw_params() debug-ability Cezary Rojewski
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Declare local 'dev' and make use of it plus dev_get_platdata() to
improve code readability.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/hsw_rt5640.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c
index 0cd788a73694..94f96de0b62a 100644
--- a/sound/soc/intel/boards/hsw_rt5640.c
+++ b/sound/soc/intel/boards/hsw_rt5640.c
@@ -147,16 +147,17 @@ static struct snd_soc_card hsw_rt5640_card = {
 static int hsw_rt5640_probe(struct platform_device *pdev)
 {
 	struct snd_soc_acpi_mach *mach;
+	struct device *dev = &pdev->dev;
 	int ret;
 
-	hsw_rt5640_card.dev = &pdev->dev;
-	mach = pdev->dev.platform_data;
+	hsw_rt5640_card.dev = dev;
+	mach = dev_get_platdata(dev);
 
 	ret = snd_soc_fixup_dai_links_platform_name(&hsw_rt5640_card, mach->mach_params.platform);
 	if (ret)
 		return ret;
 
-	return devm_snd_soc_register_card(&pdev->dev, &hsw_rt5640_card);
+	return devm_snd_soc_register_card(dev, &hsw_rt5640_card);
 }
 
 static struct platform_driver hsw_rt5640_driver = {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 07/17] ASoC: Intel: hsw_rt5640: Improve hw_params() debug-ability
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (5 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 06/17] ASoC: Intel: hsw_rt5640: Improve probe() function quality Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 08/17] ASoC: Intel: Rename broadwell source file to bdw_rt286 Cezary Rojewski
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Print status if setting sysclk fails.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/hsw_rt5640.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c
index 94f96de0b62a..5e2224f7e986 100644
--- a/sound/soc/intel/boards/hsw_rt5640.c
+++ b/sound/soc/intel/boards/hsw_rt5640.c
@@ -53,7 +53,7 @@ static int codec_link_hw_params(struct snd_pcm_substream *substream,
 
 	ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_MCLK, 12288000, SND_SOC_CLOCK_IN);
 	if (ret < 0) {
-		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
+		dev_err(rtd->dev, "set codec sysclk failed: %d\n", ret);
 		return ret;
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 08/17] ASoC: Intel: Rename broadwell source file to bdw_rt286
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (6 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 07/17] ASoC: Intel: hsw_rt5640: Improve hw_params() debug-ability Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 09/17] ASoC: Intel: bdw_rt286: Reword prefixes of all driver members Cezary Rojewski
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Rename source file to drop any ambiguity.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/Makefile                     | 2 +-
 sound/soc/intel/boards/{broadwell.c => bdw_rt286.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename sound/soc/intel/boards/{broadwell.c => bdw_rt286.c} (100%)

diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
index e479546a3d4b..eea1e26acfda 100644
--- a/sound/soc/intel/boards/Makefile
+++ b/sound/soc/intel/boards/Makefile
@@ -2,7 +2,7 @@
 snd-soc-sst-haswell-objs := hsw_rt5640.o
 snd-soc-sst-bdw-rt5650-mach-objs := bdw-rt5650.o
 snd-soc-sst-bdw-rt5677-mach-objs := bdw-rt5677.o
-snd-soc-sst-broadwell-objs := broadwell.o
+snd-soc-sst-broadwell-objs := bdw_rt286.o
 snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o
 snd-soc-sst-bxt-rt298-objs := bxt_rt298.o
 snd-soc-sst-sof-pcm512x-objs := sof_pcm512x.o
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/bdw_rt286.c
similarity index 100%
rename from sound/soc/intel/boards/broadwell.c
rename to sound/soc/intel/boards/bdw_rt286.c
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 09/17] ASoC: Intel: bdw_rt286: Reword prefixes of all driver members
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (7 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 08/17] ASoC: Intel: Rename broadwell source file to bdw_rt286 Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 10/17] ASoC: Intel: bdw_rt286: Reword driver name Cezary Rojewski
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Replace ambiguous 'broadwell_rt286_' prefixes in favour of 'card_',
'link_' and other similar strings to clearly state which object given
member implements behavior for.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/bdw_rt286.c | 96 +++++++++++++++---------------
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
index 48bf3241b3e6..a72c17171765 100644
--- a/sound/soc/intel/boards/bdw_rt286.c
+++ b/sound/soc/intel/boards/bdw_rt286.c
@@ -16,9 +16,9 @@
 
 #include "../../codecs/rt286.h"
 
-static struct snd_soc_jack broadwell_headset;
+static struct snd_soc_jack card_headset;
 /* Headset jack detection DAPM pins */
-static struct snd_soc_jack_pin broadwell_headset_pins[] = {
+static struct snd_soc_jack_pin card_headset_pins[] = {
 	{
 		.pin = "Mic Jack",
 		.mask = SND_JACK_MICROPHONE,
@@ -29,12 +29,12 @@ static struct snd_soc_jack_pin broadwell_headset_pins[] = {
 	},
 };
 
-static const struct snd_kcontrol_new broadwell_controls[] = {
+static const struct snd_kcontrol_new card_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Speaker"),
 	SOC_DAPM_PIN_SWITCH("Headphone Jack"),
 };
 
-static const struct snd_soc_dapm_widget broadwell_widgets[] = {
+static const struct snd_soc_dapm_widget card_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone Jack", NULL),
 	SND_SOC_DAPM_SPK("Speaker", NULL),
 	SND_SOC_DAPM_MIC("Mic Jack", NULL),
@@ -43,7 +43,7 @@ static const struct snd_soc_dapm_widget broadwell_widgets[] = {
 	SND_SOC_DAPM_LINE("Line Jack", NULL),
 };
 
-static const struct snd_soc_dapm_route broadwell_rt286_map[] = {
+static const struct snd_soc_dapm_route card_routes[] = {
 
 	/* speaker */
 	{"Speaker", NULL, "SPOR"},
@@ -65,22 +65,22 @@ static const struct snd_soc_dapm_route broadwell_rt286_map[] = {
 	{"AIF1 Playback", NULL, "SSP0 CODEC OUT"},
 };
 
-static int broadwell_rt286_codec_init(struct snd_soc_pcm_runtime *rtd)
+static int codec_link_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component;
 	int ret = 0;
 	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset",
-		SND_JACK_HEADSET | SND_JACK_BTN_0, &broadwell_headset,
-		broadwell_headset_pins, ARRAY_SIZE(broadwell_headset_pins));
+		SND_JACK_HEADSET | SND_JACK_BTN_0, &card_headset,
+		card_headset_pins, ARRAY_SIZE(card_headset_pins));
 	if (ret)
 		return ret;
 
-	snd_soc_component_set_jack(component, &broadwell_headset, NULL);
+	snd_soc_component_set_jack(component, &card_headset, NULL);
 	return 0;
 }
 
 
-static int broadwell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd,
+static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 			struct snd_pcm_hw_params *params)
 {
 	struct snd_interval *rate = hw_param_interval(params,
@@ -97,7 +97,7 @@ static int broadwell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd,
 	return 0;
 }
 
-static int broadwell_rt286_hw_params(struct snd_pcm_substream *substream,
+static int codec_link_hw_params(struct snd_pcm_substream *substream,
 	struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
@@ -115,8 +115,8 @@ static int broadwell_rt286_hw_params(struct snd_pcm_substream *substream,
 	return ret;
 }
 
-static const struct snd_soc_ops broadwell_rt286_ops = {
-	.hw_params = broadwell_rt286_hw_params,
+static const struct snd_soc_ops codec_link_ops = {
+	.hw_params = codec_link_hw_params,
 };
 
 static const unsigned int channels[] = {
@@ -129,7 +129,7 @@ static const struct snd_pcm_hw_constraint_list constraints_channels = {
 	.mask = 0,
 };
 
-static int broadwell_fe_startup(struct snd_pcm_substream *substream)
+static int bdw_rt286_fe_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
@@ -140,8 +140,8 @@ static int broadwell_fe_startup(struct snd_pcm_substream *substream)
 					  &constraints_channels);
 }
 
-static const struct snd_soc_ops broadwell_fe_ops = {
-	.startup = broadwell_fe_startup,
+static const struct snd_soc_ops bdw_rt286_fe_ops = {
+	.startup = bdw_rt286_fe_startup,
 };
 
 SND_SOC_DAILINK_DEF(system,
@@ -169,7 +169,7 @@ SND_SOC_DAILINK_DEF(ssp0_port,
 	    DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
 
 /* broadwell digital audio interface glue - connects codec <--> CPU */
-static struct snd_soc_dai_link broadwell_rt286_dais[] = {
+static struct snd_soc_dai_link card_dai_links[] = {
 	/* Front End DAI links */
 	{
 		.name = "System PCM",
@@ -177,7 +177,7 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = {
 		.nonatomic = 1,
 		.dynamic = 1,
 		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
-		.ops = &broadwell_fe_ops,
+		.ops = &bdw_rt286_fe_ops,
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
 		SND_SOC_DAILINK_REG(system, dummy, platform),
@@ -215,12 +215,12 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = {
 		.name = "Codec",
 		.id = 0,
 		.no_pcm = 1,
-		.init = broadwell_rt286_codec_init,
+		.init = codec_link_init,
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			SND_SOC_DAIFMT_CBC_CFC,
 		.ignore_pmdown_time = 1,
-		.be_hw_params_fixup = broadwell_ssp0_fixup,
-		.ops = &broadwell_rt286_ops,
+		.be_hw_params_fixup = codec_link_hw_params_fixup,
+		.ops = &codec_link_ops,
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
 		SND_SOC_DAILINK_REG(ssp0_port, codec, platform),
@@ -241,21 +241,21 @@ static void broadwell_disable_jack(struct snd_soc_card *card)
 	}
 }
 
-static int broadwell_suspend(struct snd_soc_card *card)
+static int card_suspend_pre(struct snd_soc_card *card)
 {
 	broadwell_disable_jack(card);
 
 	return 0;
 }
 
-static int broadwell_resume(struct snd_soc_card *card){
+static int card_resume_post(struct snd_soc_card *card){
 	struct snd_soc_component *component;
 
 	for_each_card_components(card, component) {
 		if (!strcmp(component->name, "i2c-INT343A:00")) {
 
 			dev_dbg(component->dev, "enabling jack detect for resume.\n");
-			snd_soc_component_set_jack(component, &broadwell_headset, NULL);
+			snd_soc_component_set_jack(component, &card_headset, NULL);
 			break;
 		}
 	}
@@ -270,48 +270,48 @@ static int broadwell_resume(struct snd_soc_card *card){
 #define DRIVER_NAME NULL /* card name will be used for driver name */
 
 /* broadwell audio machine driver for WPT + RT286S */
-static struct snd_soc_card broadwell_rt286 = {
+static struct snd_soc_card bdw_rt286_card = {
 	.owner = THIS_MODULE,
-	.dai_link = broadwell_rt286_dais,
-	.num_links = ARRAY_SIZE(broadwell_rt286_dais),
-	.controls = broadwell_controls,
-	.num_controls = ARRAY_SIZE(broadwell_controls),
-	.dapm_widgets = broadwell_widgets,
-	.num_dapm_widgets = ARRAY_SIZE(broadwell_widgets),
-	.dapm_routes = broadwell_rt286_map,
-	.num_dapm_routes = ARRAY_SIZE(broadwell_rt286_map),
+	.dai_link = card_dai_links,
+	.num_links = ARRAY_SIZE(card_dai_links),
+	.controls = card_controls,
+	.num_controls = ARRAY_SIZE(card_controls),
+	.dapm_widgets = card_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(card_widgets),
+	.dapm_routes = card_routes,
+	.num_dapm_routes = ARRAY_SIZE(card_routes),
 	.fully_routed = true,
-	.suspend_pre = broadwell_suspend,
-	.resume_post = broadwell_resume,
+	.suspend_pre = card_suspend_pre,
+	.resume_post = card_resume_post,
 };
 
-static int broadwell_audio_probe(struct platform_device *pdev)
+static int bdw_rt286_probe(struct platform_device *pdev)
 {
 	struct snd_soc_acpi_mach *mach;
 	int ret;
 
-	broadwell_rt286.dev = &pdev->dev;
+	bdw_rt286_card.dev = &pdev->dev;
 
 	/* override platform name, if required */
 	mach = pdev->dev.platform_data;
-	ret = snd_soc_fixup_dai_links_platform_name(&broadwell_rt286,
+	ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card,
 						    mach->mach_params.platform);
 	if (ret)
 		return ret;
 
 	/* set card and driver name */
 	if (snd_soc_acpi_sof_parent(&pdev->dev)) {
-		broadwell_rt286.name = SOF_CARD_NAME;
-		broadwell_rt286.driver_name = SOF_DRIVER_NAME;
+		bdw_rt286_card.name = SOF_CARD_NAME;
+		bdw_rt286_card.driver_name = SOF_DRIVER_NAME;
 	} else {
-		broadwell_rt286.name = CARD_NAME;
-		broadwell_rt286.driver_name = DRIVER_NAME;
+		bdw_rt286_card.name = CARD_NAME;
+		bdw_rt286_card.driver_name = DRIVER_NAME;
 	}
 
-	return devm_snd_soc_register_card(&pdev->dev, &broadwell_rt286);
+	return devm_snd_soc_register_card(&pdev->dev, &bdw_rt286_card);
 }
 
-static int broadwell_audio_remove(struct platform_device *pdev)
+static int bdw_rt286_remove(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = platform_get_drvdata(pdev);
 
@@ -320,16 +320,16 @@ static int broadwell_audio_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver broadwell_audio = {
-	.probe = broadwell_audio_probe,
-	.remove = broadwell_audio_remove,
+static struct platform_driver bdw_rt286_driver = {
+	.probe = bdw_rt286_probe,
+	.remove = bdw_rt286_remove,
 	.driver = {
 		.name = "broadwell-audio",
 		.pm = &snd_soc_pm_ops
 	},
 };
 
-module_platform_driver(broadwell_audio)
+module_platform_driver(bdw_rt286_driver)
 
 /* Module information */
 MODULE_AUTHOR("Liam Girdwood, Xingchao Wang");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 10/17] ASoC: Intel: bdw_rt286: Reword driver name
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (8 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 09/17] ASoC: Intel: bdw_rt286: Reword prefixes of all driver members Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 11/17] ASoC: Intel: bdw_rt286: Update code indentation Cezary Rojewski
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Align with other Intel boards naming convention and let the name
explicitly state which components are being connected.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/bdw_rt286.c                    | 4 ++--
 sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
index a72c17171765..da570e162f2a 100644
--- a/sound/soc/intel/boards/bdw_rt286.c
+++ b/sound/soc/intel/boards/bdw_rt286.c
@@ -324,7 +324,7 @@ static struct platform_driver bdw_rt286_driver = {
 	.probe = bdw_rt286_probe,
 	.remove = bdw_rt286_remove,
 	.driver = {
-		.name = "broadwell-audio",
+		.name = "bdw_rt286",
 		.pm = &snd_soc_pm_ops
 	},
 };
@@ -335,4 +335,4 @@ module_platform_driver(bdw_rt286_driver)
 MODULE_AUTHOR("Liam Girdwood, Xingchao Wang");
 MODULE_DESCRIPTION("Intel SST Audio for WPT/Broadwell");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:broadwell-audio");
+MODULE_ALIAS("platform:bdw_rt286");
diff --git a/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c b/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c
index 4e00f8f6c521..cbcb649604e5 100644
--- a/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-hsw-bdw-match.c
@@ -23,7 +23,7 @@ EXPORT_SYMBOL_GPL(snd_soc_acpi_intel_haswell_machines);
 struct snd_soc_acpi_mach snd_soc_acpi_intel_broadwell_machines[] = {
 	{
 		.id = "INT343A",
-		.drv_name = "broadwell-audio",
+		.drv_name = "bdw_rt286",
 		.fw_filename =  "intel/IntcSST2.bin",
 		.sof_tplg_filename = "sof-bdw-rt286.tplg",
 	},
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 11/17] ASoC: Intel: bdw_rt286: Update code indentation
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (9 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 10/17] ASoC: Intel: bdw_rt286: Reword driver name Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 12/17] ASoC: Intel: bdw_rt286: Update file comments Cezary Rojewski
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Make use of 100 character limit and modify indentation so code is easier
to read. While at it, sort includes in alphabetical order.

While at it, rename local variable 'chan' to 'channels' to match
hsw_rt5640 board's equivalent.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/bdw_rt286.c | 87 +++++++++++-------------------
 1 file changed, 32 insertions(+), 55 deletions(-)

diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
index da570e162f2a..a3351eb04d4d 100644
--- a/sound/soc/intel/boards/bdw_rt286.c
+++ b/sound/soc/intel/boards/bdw_rt286.c
@@ -8,12 +8,11 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <sound/core.h>
-#include <sound/pcm.h>
-#include <sound/soc.h>
 #include <sound/jack.h>
+#include <sound/pcm.h>
 #include <sound/pcm_params.h>
+#include <sound/soc.h>
 #include <sound/soc-acpi.h>
-
 #include "../../codecs/rt286.h"
 
 static struct snd_soc_jack card_headset;
@@ -44,7 +43,6 @@ static const struct snd_soc_dapm_widget card_widgets[] = {
 };
 
 static const struct snd_soc_dapm_route card_routes[] = {
-
 	/* speaker */
 	{"Speaker", NULL, "SPOR"},
 	{"Speaker", NULL, "SPOL"},
@@ -69,9 +67,10 @@ static int codec_link_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component;
 	int ret = 0;
-	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset",
-		SND_JACK_HEADSET | SND_JACK_BTN_0, &card_headset,
-		card_headset_pins, ARRAY_SIZE(card_headset_pins));
+
+	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset", SND_JACK_HEADSET | SND_JACK_BTN_0,
+					 &card_headset, card_headset_pins,
+					 ARRAY_SIZE(card_headset_pins));
 	if (ret)
 		return ret;
 
@@ -79,34 +78,29 @@ static int codec_link_init(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
-
 static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
-			struct snd_pcm_hw_params *params)
+				      struct snd_pcm_hw_params *params)
 {
-	struct snd_interval *rate = hw_param_interval(params,
-						      SNDRV_PCM_HW_PARAM_RATE);
-	struct snd_interval *chan = hw_param_interval(params,
-						      SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
 
 	/* The ADSP will covert the FE rate to 48k, stereo */
 	rate->min = rate->max = 48000;
-	chan->min = chan->max = 2;
-
+	channels->min = channels->max = 2;
 	/* set SSP0 to 16 bit */
 	params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
+
 	return 0;
 }
 
 static int codec_link_hw_params(struct snd_pcm_substream *substream,
-	struct snd_pcm_hw_params *params)
+				struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
 	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
 	int ret;
 
-	ret = snd_soc_dai_set_sysclk(codec_dai, RT286_SCLK_S_PLL, 24000000,
-		SND_SOC_CLOCK_IN);
-
+	ret = snd_soc_dai_set_sysclk(codec_dai, RT286_SCLK_S_PLL, 24000000, SND_SOC_CLOCK_IN);
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
 		return ret;
@@ -135,8 +129,7 @@ static int bdw_rt286_fe_startup(struct snd_pcm_substream *substream)
 
 	/* Board supports stereo configuration only */
 	runtime->hw.channels_max = 2;
-	return snd_pcm_hw_constraint_list(runtime, 0,
-					  SNDRV_PCM_HW_PARAM_CHANNELS,
+	return snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
 					  &constraints_channels);
 }
 
@@ -144,29 +137,15 @@ static const struct snd_soc_ops bdw_rt286_fe_ops = {
 	.startup = bdw_rt286_fe_startup,
 };
 
-SND_SOC_DAILINK_DEF(system,
-	DAILINK_COMP_ARRAY(COMP_CPU("System Pin")));
-
-SND_SOC_DAILINK_DEF(offload0,
-	DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin")));
+SND_SOC_DAILINK_DEF(system, DAILINK_COMP_ARRAY(COMP_CPU("System Pin")));
+SND_SOC_DAILINK_DEF(offload0, DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin")));
+SND_SOC_DAILINK_DEF(offload1, DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin")));
+SND_SOC_DAILINK_DEF(loopback, DAILINK_COMP_ARRAY(COMP_CPU("Loopback Pin")));
 
-SND_SOC_DAILINK_DEF(offload1,
-	DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin")));
-
-SND_SOC_DAILINK_DEF(loopback,
-	DAILINK_COMP_ARRAY(COMP_CPU("Loopback Pin")));
-
-SND_SOC_DAILINK_DEF(dummy,
-	DAILINK_COMP_ARRAY(COMP_DUMMY()));
-
-SND_SOC_DAILINK_DEF(platform,
-	DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audio")));
-
-SND_SOC_DAILINK_DEF(codec,
-	DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1")));
-
-SND_SOC_DAILINK_DEF(ssp0_port,
-	    DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
+SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY()));
+SND_SOC_DAILINK_DEF(platform, DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audio")));
+SND_SOC_DAILINK_DEF(codec, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1")));
+SND_SOC_DAILINK_DEF(ssp0_port, DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
 
 /* broadwell digital audio interface glue - connects codec <--> CPU */
 static struct snd_soc_dai_link card_dai_links[] = {
@@ -176,7 +155,7 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.stream_name = "System Playback/Capture",
 		.nonatomic = 1,
 		.dynamic = 1,
-		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST },
 		.ops = &bdw_rt286_fe_ops,
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
@@ -187,7 +166,7 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.stream_name = "Offload0 Playback",
 		.nonatomic = 1,
 		.dynamic = 1,
-		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST },
 		.dpcm_playback = 1,
 		SND_SOC_DAILINK_REG(offload0, dummy, platform),
 	},
@@ -196,7 +175,7 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.stream_name = "Offload1 Playback",
 		.nonatomic = 1,
 		.dynamic = 1,
-		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST },
 		.dpcm_playback = 1,
 		SND_SOC_DAILINK_REG(offload1, dummy, platform),
 	},
@@ -205,7 +184,7 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.stream_name = "Loopback",
 		.nonatomic = 1,
 		.dynamic = 1,
-		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST },
 		.dpcm_capture = 1,
 		SND_SOC_DAILINK_REG(loopback, dummy, platform),
 	},
@@ -216,8 +195,7 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.id = 0,
 		.no_pcm = 1,
 		.init = codec_link_init,
-		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
-			SND_SOC_DAIFMT_CBC_CFC,
+		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC,
 		.ignore_pmdown_time = 1,
 		.be_hw_params_fixup = codec_link_hw_params_fixup,
 		.ops = &codec_link_ops,
@@ -233,7 +211,6 @@ static void broadwell_disable_jack(struct snd_soc_card *card)
 
 	for_each_card_components(card, component) {
 		if (!strcmp(component->name, "i2c-INT343A:00")) {
-
 			dev_dbg(component->dev, "disabling jack detect before going to suspend.\n");
 			snd_soc_component_set_jack(component, NULL, NULL);
 			break;
@@ -248,17 +225,18 @@ static int card_suspend_pre(struct snd_soc_card *card)
 	return 0;
 }
 
-static int card_resume_post(struct snd_soc_card *card){
+static int card_resume_post(struct snd_soc_card *card)
+{
 	struct snd_soc_component *component;
 
 	for_each_card_components(card, component) {
 		if (!strcmp(component->name, "i2c-INT343A:00")) {
-
 			dev_dbg(component->dev, "enabling jack detect for resume.\n");
 			snd_soc_component_set_jack(component, &card_headset, NULL);
 			break;
 		}
 	}
+
 	return 0;
 }
 
@@ -291,11 +269,10 @@ static int bdw_rt286_probe(struct platform_device *pdev)
 	int ret;
 
 	bdw_rt286_card.dev = &pdev->dev;
-
 	/* override platform name, if required */
 	mach = pdev->dev.platform_data;
-	ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card,
-						    mach->mach_params.platform);
+
+	ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card, mach->mach_params.platform);
 	if (ret)
 		return ret;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 12/17] ASoC: Intel: bdw_rt286: Update file comments
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (10 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 11/17] ASoC: Intel: bdw_rt286: Update code indentation Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 13/17] ASoC: Intel: bdw_rt286: Improve probe() function quality Cezary Rojewski
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Drop redundant and update valuable comments within the file to increase
readability. This patch also revisits module information and kconfig
help strings.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/Kconfig     |  2 +-
 sound/soc/intel/boards/bdw_rt286.c | 23 +++++++----------------
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index 817b4c04bf6a..aa12d7e3dd2f 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -85,7 +85,7 @@ config SND_SOC_INTEL_BDW_RT5677_MACH
 	  If unsure select "N".
 
 config SND_SOC_INTEL_BROADWELL_MACH
-	tristate "Broadwell Wildcatpoint"
+	tristate "Broadwell with RT286 I2S codec"
 	depends on I2C
 	depends on I2C_DESIGNWARE_PLATFORM || COMPILE_TEST
 	depends on X86_INTEL_LPSS || COMPILE_TEST
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
index a3351eb04d4d..628c93c75ca1 100644
--- a/sound/soc/intel/boards/bdw_rt286.c
+++ b/sound/soc/intel/boards/bdw_rt286.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Intel Broadwell Wildcatpoint SST Audio
+ * Sound card driver for Intel Broadwell Wildcat Point with Realtek 286
  *
  * Copyright (C) 2013, Intel Corporation. All rights reserved.
  */
@@ -16,7 +16,7 @@
 #include "../../codecs/rt286.h"
 
 static struct snd_soc_jack card_headset;
-/* Headset jack detection DAPM pins */
+
 static struct snd_soc_jack_pin card_headset_pins[] = {
 	{
 		.pin = "Mic Jack",
@@ -43,18 +43,14 @@ static const struct snd_soc_dapm_widget card_widgets[] = {
 };
 
 static const struct snd_soc_dapm_route card_routes[] = {
-	/* speaker */
 	{"Speaker", NULL, "SPOR"},
 	{"Speaker", NULL, "SPOL"},
 
-	/* HP jack connectors - unknown if we have jack deteck */
 	{"Headphone Jack", NULL, "HPO Pin"},
 
-	/* other jacks */
 	{"MIC1", NULL, "Mic Jack"},
 	{"LINE1", NULL, "Line Jack"},
 
-	/* digital mics */
 	{"DMIC1 Pin", NULL, "DMIC1"},
 	{"DMIC2 Pin", NULL, "DMIC2"},
 
@@ -84,10 +80,10 @@ static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 	struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
 	struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
 
-	/* The ADSP will covert the FE rate to 48k, stereo */
+	/* The ADSP will convert the FE rate to 48kHz, stereo. */
 	rate->min = rate->max = 48000;
 	channels->min = channels->max = 2;
-	/* set SSP0 to 16 bit */
+	/* Set SSP0 to 16 bit. */
 	params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
 
 	return 0;
@@ -147,7 +143,6 @@ SND_SOC_DAILINK_DEF(platform, DAILINK_COMP_ARRAY(COMP_PLATFORM("haswell-pcm-audi
 SND_SOC_DAILINK_DEF(codec, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1")));
 SND_SOC_DAILINK_DEF(ssp0_port, DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
 
-/* broadwell digital audio interface glue - connects codec <--> CPU */
 static struct snd_soc_dai_link card_dai_links[] = {
 	/* Front End DAI links */
 	{
@@ -240,14 +235,13 @@ static int card_resume_post(struct snd_soc_card *card)
 	return 0;
 }
 
-/* use space before codec name to simplify card ID, and simplify driver name */
+/* Use space before codec name to simplify card ID, and simplify driver name. */
 #define SOF_CARD_NAME "bdw rt286" /* card name will be 'sof-bdw rt286' */
 #define SOF_DRIVER_NAME "SOF"
 
 #define CARD_NAME "broadwell-rt286"
 #define DRIVER_NAME NULL /* card name will be used for driver name */
 
-/* broadwell audio machine driver for WPT + RT286S */
 static struct snd_soc_card bdw_rt286_card = {
 	.owner = THIS_MODULE,
 	.dai_link = card_dai_links,
@@ -269,14 +263,12 @@ static int bdw_rt286_probe(struct platform_device *pdev)
 	int ret;
 
 	bdw_rt286_card.dev = &pdev->dev;
-	/* override platform name, if required */
 	mach = pdev->dev.platform_data;
 
 	ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card, mach->mach_params.platform);
 	if (ret)
 		return ret;
 
-	/* set card and driver name */
 	if (snd_soc_acpi_sof_parent(&pdev->dev)) {
 		bdw_rt286_card.name = SOF_CARD_NAME;
 		bdw_rt286_card.driver_name = SOF_DRIVER_NAME;
@@ -308,8 +300,7 @@ static struct platform_driver bdw_rt286_driver = {
 
 module_platform_driver(bdw_rt286_driver)
 
-/* Module information */
 MODULE_AUTHOR("Liam Girdwood, Xingchao Wang");
-MODULE_DESCRIPTION("Intel SST Audio for WPT/Broadwell");
-MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Sound card driver for Intel Broadwell Wildcat Point with Realtek 286");
+MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:bdw_rt286");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 13/17] ASoC: Intel: bdw_rt286: Improve probe() function quality
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (11 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 12/17] ASoC: Intel: bdw_rt286: Update file comments Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 14/17] ASoC: Intel: bdw_rt286: Improve hw_params() debug-ability Cezary Rojewski
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Declare local 'dev' and make use of it plus dev_get_platdata() to
improve code readability. Relocate few relevant to the function macros
for the exact same read too.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/bdw_rt286.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
index 628c93c75ca1..5e86f477114a 100644
--- a/sound/soc/intel/boards/bdw_rt286.c
+++ b/sound/soc/intel/boards/bdw_rt286.c
@@ -235,13 +235,6 @@ static int card_resume_post(struct snd_soc_card *card)
 	return 0;
 }
 
-/* Use space before codec name to simplify card ID, and simplify driver name. */
-#define SOF_CARD_NAME "bdw rt286" /* card name will be 'sof-bdw rt286' */
-#define SOF_DRIVER_NAME "SOF"
-
-#define CARD_NAME "broadwell-rt286"
-#define DRIVER_NAME NULL /* card name will be used for driver name */
-
 static struct snd_soc_card bdw_rt286_card = {
 	.owner = THIS_MODULE,
 	.dai_link = card_dai_links,
@@ -257,27 +250,33 @@ static struct snd_soc_card bdw_rt286_card = {
 	.resume_post = card_resume_post,
 };
 
+/* Use space before codec name to simplify card ID, and simplify driver name. */
+#define SOF_CARD_NAME "bdw rt286" /* card name will be 'sof-bdw rt286' */
+#define SOF_DRIVER_NAME "SOF"
+
+#define CARD_NAME "broadwell-rt286"
+
 static int bdw_rt286_probe(struct platform_device *pdev)
 {
 	struct snd_soc_acpi_mach *mach;
+	struct device *dev = &pdev->dev;
 	int ret;
 
-	bdw_rt286_card.dev = &pdev->dev;
-	mach = pdev->dev.platform_data;
+	bdw_rt286_card.dev = dev;
+	mach = dev_get_platdata(dev);
 
 	ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt286_card, mach->mach_params.platform);
 	if (ret)
 		return ret;
 
-	if (snd_soc_acpi_sof_parent(&pdev->dev)) {
+	if (snd_soc_acpi_sof_parent(dev)) {
 		bdw_rt286_card.name = SOF_CARD_NAME;
 		bdw_rt286_card.driver_name = SOF_DRIVER_NAME;
 	} else {
 		bdw_rt286_card.name = CARD_NAME;
-		bdw_rt286_card.driver_name = DRIVER_NAME;
 	}
 
-	return devm_snd_soc_register_card(&pdev->dev, &bdw_rt286_card);
+	return devm_snd_soc_register_card(dev, &bdw_rt286_card);
 }
 
 static int bdw_rt286_remove(struct platform_device *pdev)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 14/17] ASoC: Intel: bdw_rt286: Improve hw_params() debug-ability
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (12 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 13/17] ASoC: Intel: bdw_rt286: Improve probe() function quality Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 15/17] ASoC: Intel: bdw_rt286: Improve codec_link_init() quality Cezary Rojewski
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Print status if setting sysclk fails.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/bdw_rt286.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
index 5e86f477114a..c21f71b477d5 100644
--- a/sound/soc/intel/boards/bdw_rt286.c
+++ b/sound/soc/intel/boards/bdw_rt286.c
@@ -98,7 +98,7 @@ static int codec_link_hw_params(struct snd_pcm_substream *substream,
 
 	ret = snd_soc_dai_set_sysclk(codec_dai, RT286_SCLK_S_PLL, 24000000, SND_SOC_CLOCK_IN);
 	if (ret < 0) {
-		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
+		dev_err(rtd->dev, "set codec sysclk failed: %d\n", ret);
 		return ret;
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 15/17] ASoC: Intel: bdw_rt286: Improve codec_link_init() quality
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (13 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 14/17] ASoC: Intel: bdw_rt286: Improve hw_params() debug-ability Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-13  9:15 ` [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume Cezary Rojewski
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Drop redundant 'ret' assignemnt, stop ignoring set_jack() return value
and reword local 'component' variable to 'codec' to improve readability.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/bdw_rt286.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
index c21f71b477d5..92fddf6061e8 100644
--- a/sound/soc/intel/boards/bdw_rt286.c
+++ b/sound/soc/intel/boards/bdw_rt286.c
@@ -61,8 +61,8 @@ static const struct snd_soc_dapm_route card_routes[] = {
 
 static int codec_link_init(struct snd_soc_pcm_runtime *rtd)
 {
-	struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component;
-	int ret = 0;
+	struct snd_soc_component *codec = asoc_rtd_to_codec(rtd, 0)->component;
+	int ret;
 
 	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset", SND_JACK_HEADSET | SND_JACK_BTN_0,
 					 &card_headset, card_headset_pins,
@@ -70,8 +70,7 @@ static int codec_link_init(struct snd_soc_pcm_runtime *rtd)
 	if (ret)
 		return ret;
 
-	snd_soc_component_set_jack(component, &card_headset, NULL);
-	return 0;
+	return snd_soc_component_set_jack(codec, &card_headset, NULL);
 }
 
 static int codec_link_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (14 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 15/17] ASoC: Intel: bdw_rt286: Improve codec_link_init() quality Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-15  1:27   ` Pierre-Louis Bossart
  2022-06-13  9:15 ` [PATCH v2 17/17] ASoC: Intel: bdw_rt286: Remove FE DAI ops Cezary Rojewski
  2022-06-24 10:59 ` [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Mark Brown
  17 siblings, 1 reply; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

Make use of card->remove() rather than pdev->remove() to unassign jack
during card unbind procedure.

To reduce code size, define unified jack setter in form of
bdw_rt286_set_jack() and invoke it during remove(), suspend_pre() and
resume_port().

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/bdw_rt286.c | 50 +++++++++---------------------
 1 file changed, 14 insertions(+), 36 deletions(-)

diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
index 92fddf6061e8..106a06398858 100644
--- a/sound/soc/intel/boards/bdw_rt286.c
+++ b/sound/soc/intel/boards/bdw_rt286.c
@@ -199,43 +199,33 @@ static struct snd_soc_dai_link card_dai_links[] = {
 	},
 };
 
-static void broadwell_disable_jack(struct snd_soc_card *card)
+static int card_set_jack(struct snd_soc_card *card, struct snd_soc_jack *jack)
 {
-	struct snd_soc_component *component;
-
-	for_each_card_components(card, component) {
-		if (!strcmp(component->name, "i2c-INT343A:00")) {
-			dev_dbg(component->dev, "disabling jack detect before going to suspend.\n");
-			snd_soc_component_set_jack(component, NULL, NULL);
-			break;
-		}
-	}
+	struct snd_soc_dai *codec_dai = snd_soc_card_get_codec_dai(card, "rt286-aif1");
+
+	return snd_soc_component_set_jack(codec_dai->component, jack, NULL);
 }
 
-static int card_suspend_pre(struct snd_soc_card *card)
+static int card_remove(struct snd_soc_card *card)
 {
-	broadwell_disable_jack(card);
+	return card_set_jack(card, NULL);
+}
 
-	return 0;
+static int card_suspend_pre(struct snd_soc_card *card)
+{
+	return card_set_jack(card, NULL);
 }
 
 static int card_resume_post(struct snd_soc_card *card)
 {
-	struct snd_soc_component *component;
-
-	for_each_card_components(card, component) {
-		if (!strcmp(component->name, "i2c-INT343A:00")) {
-			dev_dbg(component->dev, "enabling jack detect for resume.\n");
-			snd_soc_component_set_jack(component, &card_headset, NULL);
-			break;
-		}
-	}
-
-	return 0;
+	return card_set_jack(card, &card_headset);
 }
 
 static struct snd_soc_card bdw_rt286_card = {
 	.owner = THIS_MODULE,
+	.remove = card_remove,
+	.suspend_pre = card_suspend_pre,
+	.resume_post = card_resume_post,
 	.dai_link = card_dai_links,
 	.num_links = ARRAY_SIZE(card_dai_links),
 	.controls = card_controls,
@@ -245,8 +235,6 @@ static struct snd_soc_card bdw_rt286_card = {
 	.dapm_routes = card_routes,
 	.num_dapm_routes = ARRAY_SIZE(card_routes),
 	.fully_routed = true,
-	.suspend_pre = card_suspend_pre,
-	.resume_post = card_resume_post,
 };
 
 /* Use space before codec name to simplify card ID, and simplify driver name. */
@@ -278,18 +266,8 @@ static int bdw_rt286_probe(struct platform_device *pdev)
 	return devm_snd_soc_register_card(dev, &bdw_rt286_card);
 }
 
-static int bdw_rt286_remove(struct platform_device *pdev)
-{
-	struct snd_soc_card *card = platform_get_drvdata(pdev);
-
-	broadwell_disable_jack(card);
-
-	return 0;
-}
-
 static struct platform_driver bdw_rt286_driver = {
 	.probe = bdw_rt286_probe,
-	.remove = bdw_rt286_remove,
 	.driver = {
 		.name = "bdw_rt286",
 		.pm = &snd_soc_pm_ops
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 17/17] ASoC: Intel: bdw_rt286: Remove FE DAI ops
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (15 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume Cezary Rojewski
@ 2022-06-13  9:15 ` Cezary Rojewski
  2022-06-24 10:59 ` [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Mark Brown
  17 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-13  9:15 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Cezary Rojewski, pierre-louis.bossart, tiwai, hdegoede,
	amadeuszx.slawinski

bdw_rt286_fe_ops is redundant as platform FE DAIs already limit the
number of channels available for the endpoint.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/boards/bdw_rt286.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
index 106a06398858..4d30e1811139 100644
--- a/sound/soc/intel/boards/bdw_rt286.c
+++ b/sound/soc/intel/boards/bdw_rt286.c
@@ -108,30 +108,6 @@ static const struct snd_soc_ops codec_link_ops = {
 	.hw_params = codec_link_hw_params,
 };
 
-static const unsigned int channels[] = {
-	2,
-};
-
-static const struct snd_pcm_hw_constraint_list constraints_channels = {
-	.count = ARRAY_SIZE(channels),
-	.list = channels,
-	.mask = 0,
-};
-
-static int bdw_rt286_fe_startup(struct snd_pcm_substream *substream)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-
-	/* Board supports stereo configuration only */
-	runtime->hw.channels_max = 2;
-	return snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
-					  &constraints_channels);
-}
-
-static const struct snd_soc_ops bdw_rt286_fe_ops = {
-	.startup = bdw_rt286_fe_startup,
-};
-
 SND_SOC_DAILINK_DEF(system, DAILINK_COMP_ARRAY(COMP_CPU("System Pin")));
 SND_SOC_DAILINK_DEF(offload0, DAILINK_COMP_ARRAY(COMP_CPU("Offload0 Pin")));
 SND_SOC_DAILINK_DEF(offload1, DAILINK_COMP_ARRAY(COMP_CPU("Offload1 Pin")));
@@ -150,7 +126,6 @@ static struct snd_soc_dai_link card_dai_links[] = {
 		.nonatomic = 1,
 		.dynamic = 1,
 		.trigger = { SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST },
-		.ops = &bdw_rt286_fe_ops,
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
 		SND_SOC_DAILINK_REG(system, dummy, platform),
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume
  2022-06-13  9:15 ` [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume Cezary Rojewski
@ 2022-06-15  1:27   ` Pierre-Louis Bossart
  2022-06-15 12:57     ` Cezary Rojewski
  0 siblings, 1 reply; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-06-15  1:27 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel, broonie; +Cc: hdegoede, amadeuszx.slawinski, tiwai



On 6/13/22 04:15, Cezary Rojewski wrote:
> Make use of card->remove() rather than pdev->remove() to unassign jack
> during card unbind procedure.
> 
> To reduce code size, define unified jack setter in form of
> bdw_rt286_set_jack() and invoke it during remove(), suspend_pre() and
> resume_port().
> 
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

This seems to have rather negative side effects in our modprobe/modprobe
-r tests?

The pattern of disabling the jack in the platform device .remove is
fairly common, I don't recall having seen a machine driver doing this in
the card .remove step. Are you sure this is equivalent?

Reverting this patch removes the kernel oops.

I don't have time to debug further - but this adds to my point of
minimizing risk on legacy code, doesn't it? suspend-resume is difficult
to get right, and easy to break. I have done the latter more often that
the former.

if you want to reproduce the issue, see
https://github.com/thesofproject/linux/pull/3696

and use sof-test:
/root/sof-test/test-case/check-kmod-load-unload.sh -l 1



[  246.570618] BUG: kernel NULL pointer dereference, address:
0000000000000058

[  246.570636] #PF: supervisor read access in kernel mode

[  246.570644] #PF: error_code(0x0000) - not-present page

[  246.570653] PGD 0 P4D 0

[  246.570662] Oops: 0000 [#1] PREEMPT SMP PTI

[  246.570672] CPU: 2 PID: 5409 Comm: rmmod Tainted: G          I
5.19.0-rc1-test-05237-gf04711968c69 #224

[  246.570687] Hardware name: Intel Corporation Broadwell Client
platform/Wilson Beach SDS, BIOS BDW-E2R1.86C.0095.R09.1410300006 10/30/2014

[  246.570700] RIP: 0010:card_set_jack+0x62/0x80 [snd_soc_sst_broadwell]

[  246.570717] Code: 49 39 c4 74 39 8b 93 14 06 00 00 48 8b 83 08 06 00
00 48 c7 c6 8d 1c 4e c0 48 8b 2c d0 48 8b 7d 00 e8 22 7b e5 c6 85 c0 75
c8 <48> 8b 7d 58 5b 4c 89 ee 31 d2 5d 41 5c 41 5d e9 fa d2 ee ff 31 ed

[  246.570736] RSP: 0018:ffffa6b9810d3c90 EFLAGS: 00010246

[  246.570746] RAX: ffffffffc04e33e8 RBX: ffffffffc04e30e0 RCX:
0000000000000000

[  246.570756] RDX: 0000000000000001 RSI: 0000000000000000 RDI:
ffffffffc04e30e0

[  246.570766] RBP: 0000000000000000 R08: 0000003968c0fba0 R09:
0000000000000000

[  246.570775] R10: ffffffffc03c1aaa R11: 0000000000000000 R12:
ffffffffc04e33e8

[  246.570784] R13: 0000000000000000 R14: ffffffffc04e3420 R15:
dead000000000100

[  246.570794] FS:  00007f17697a0740(0000) GS:ffff8e9685a00000(0000)
knlGS:0000000000000000

[  246.570805] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033

[  246.570814] CR2: 0000000000000058 CR3: 0000000132d7c006 CR4:
00000000003706e0

[  246.570823] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000

[  246.570832] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400

[  246.570842] Call Trace:

[  246.570849]  <TASK>

[  246.570858]  snd_soc_card_remove+0x27/0x60 [snd_soc_core]

[  246.570902]  soc_cleanup_card_resources+0x22d/0x260 [snd_soc_core]

[  246.570938]  snd_soc_unregister_card+0xd9/0xe0 [snd_soc_core]

[  246.570970]  devres_release_all+0xb8/0x100

[  246.570988]  device_unbind_cleanup+0xe/0x70

[  246.571001]  device_release_driver_internal+0x1dd/0x230

[  246.571012]  bus_remove_device+0xef/0x160

[  246.571025]  device_del+0x18c/0x3f0

[  246.571037]  platform_device_del.part.0+0x13/0x70

[  246.571048]  platform_device_unregister+0x1c/0x30

[  246.571059]  snd_sof_device_remove+0x57/0xe0 [snd_sof]

[  246.571087]  sof_acpi_remove+0x34/0x40 [snd_sof_acpi]

[  246.571099]  platform_remove+0x1f/0x40

[  246.571109]  device_release_driver_internal+0x1b0/0x230

[  246.571120]  driver_detach+0x47/0x90

[  246.571129]  bus_remove_driver+0x58/0xd0

[  246.571141]  __do_sys_delete_module+0x19f/0x270

[  246.571156]  ? fpregs_assert_state_consistent+0x1e/0x40

[  246.571168]  ? exit_to_user_mode_prepare+0x37/0x120

[  246.571181]  do_syscall_64+0x3b/0x90

[  246.571193]  entry_SYSCALL_64_after_hwframe+0x46/0xb0


> ---
>  sound/soc/intel/boards/bdw_rt286.c | 50 +++++++++---------------------
>  1 file changed, 14 insertions(+), 36 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c
> index 92fddf6061e8..106a06398858 100644
> --- a/sound/soc/intel/boards/bdw_rt286.c
> +++ b/sound/soc/intel/boards/bdw_rt286.c
> @@ -199,43 +199,33 @@ static struct snd_soc_dai_link card_dai_links[] = {
>  	},
>  };
>  
> -static void broadwell_disable_jack(struct snd_soc_card *card)
> +static int card_set_jack(struct snd_soc_card *card, struct snd_soc_jack *jack)
>  {
> -	struct snd_soc_component *component;
> -
> -	for_each_card_components(card, component) {
> -		if (!strcmp(component->name, "i2c-INT343A:00")) {
> -			dev_dbg(component->dev, "disabling jack detect before going to suspend.\n");
> -			snd_soc_component_set_jack(component, NULL, NULL);
> -			break;
> -		}
> -	}
> +	struct snd_soc_dai *codec_dai = snd_soc_card_get_codec_dai(card, "rt286-aif1");
> +
> +	return snd_soc_component_set_jack(codec_dai->component, jack, NULL);
>  }
>  
> -static int card_suspend_pre(struct snd_soc_card *card)
> +static int card_remove(struct snd_soc_card *card)
>  {
> -	broadwell_disable_jack(card);
> +	return card_set_jack(card, NULL);
> +}
>  
> -	return 0;
> +static int card_suspend_pre(struct snd_soc_card *card)
> +{
> +	return card_set_jack(card, NULL);
>  }
>  
>  static int card_resume_post(struct snd_soc_card *card)
>  {
> -	struct snd_soc_component *component;
> -
> -	for_each_card_components(card, component) {
> -		if (!strcmp(component->name, "i2c-INT343A:00")) {
> -			dev_dbg(component->dev, "enabling jack detect for resume.\n");
> -			snd_soc_component_set_jack(component, &card_headset, NULL);
> -			break;
> -		}
> -	}
> -
> -	return 0;
> +	return card_set_jack(card, &card_headset);
>  }
>  
>  static struct snd_soc_card bdw_rt286_card = {
>  	.owner = THIS_MODULE,
> +	.remove = card_remove,
> +	.suspend_pre = card_suspend_pre,
> +	.resume_post = card_resume_post,
>  	.dai_link = card_dai_links,
>  	.num_links = ARRAY_SIZE(card_dai_links),
>  	.controls = card_controls,
> @@ -245,8 +235,6 @@ static struct snd_soc_card bdw_rt286_card = {
>  	.dapm_routes = card_routes,
>  	.num_dapm_routes = ARRAY_SIZE(card_routes),
>  	.fully_routed = true,
> -	.suspend_pre = card_suspend_pre,
> -	.resume_post = card_resume_post,
>  };
>  
>  /* Use space before codec name to simplify card ID, and simplify driver name. */
> @@ -278,18 +266,8 @@ static int bdw_rt286_probe(struct platform_device *pdev)
>  	return devm_snd_soc_register_card(dev, &bdw_rt286_card);
>  }
>  
> -static int bdw_rt286_remove(struct platform_device *pdev)
> -{
> -	struct snd_soc_card *card = platform_get_drvdata(pdev);
> -
> -	broadwell_disable_jack(card);
> -
> -	return 0;
> -}
> -
>  static struct platform_driver bdw_rt286_driver = {
>  	.probe = bdw_rt286_probe,
> -	.remove = bdw_rt286_remove,
>  	.driver = {
>  		.name = "bdw_rt286",
>  		.pm = &snd_soc_pm_ops

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume
  2022-06-15  1:27   ` Pierre-Louis Bossart
@ 2022-06-15 12:57     ` Cezary Rojewski
  2022-06-15 16:25       ` Pierre-Louis Bossart
  2022-06-19 13:58       ` Cezary Rojewski
  0 siblings, 2 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-15 12:57 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel, broonie
  Cc: hdegoede, amadeuszx.slawinski, tiwai

On 2022-06-15 3:27 AM, Pierre-Louis Bossart wrote:

> On 6/13/22 04:15, Cezary Rojewski wrote:
>> Make use of card->remove() rather than pdev->remove() to unassign jack
>> during card unbind procedure.
>>
>> To reduce code size, define unified jack setter in form of
>> bdw_rt286_set_jack() and invoke it during remove(), suspend_pre() and
>> resume_port().
>>
>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
>> Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> 
> This seems to have rather negative side effects in our modprobe/modprobe
> -r tests?
> 
> The pattern of disabling the jack in the platform device .remove is
> fairly common, I don't recall having seen a machine driver doing this in
> the card .remove step. Are you sure this is equivalent?
> 
> Reverting this patch removes the kernel oops.
> 
> I don't have time to debug further - but this adds to my point of
> minimizing risk on legacy code, doesn't it? suspend-resume is difficult
> to get right, and easy to break. I have done the latter more often that
> the former.
> 
> if you want to reproduce the issue, see
> https://github.com/thesofproject/linux/pull/3696
> 
> and use sof-test:
> /root/sof-test/test-case/check-kmod-load-unload.sh -l 1


Thanks for the report. Indeed, the latest "optimization" broke the 
card->remove() path.

Jacks are often initialized during dai_link initialization which is 
completely out of platform_device area. This report made me think 
further - if we assign jack in dai_link->init(), we should be able to 
drop it in dai_link->exit().

Not exactly! ->init() is done once card components are already accounted 
for (available for use) but snd_soc_link_exit() is called during 
snd_soc_remove_pcm_runtime() when card components are available no 
longer - soc_remove_link_components().

TLDR: teardown path is not symmetric with its counterpart, perhaps a 
problem yet to be addressed. I'll see if moving the jack-NULLing to 
codec's DAI ->remove() won't be a better temporary (?) solution than 
reverting to platform_device->remove() usage.


Mark,

Is it fine to leave v2 series as is, just ignoring this single 16/17 
patch? Or should I resend entire series as v3 without this very patch? 
I'd like to address the problem via a separate change.


Regards,
Czarek

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume
  2022-06-15 12:57     ` Cezary Rojewski
@ 2022-06-15 16:25       ` Pierre-Louis Bossart
  2022-06-19 13:04         ` Cezary Rojewski
  2022-06-19 13:58       ` Cezary Rojewski
  1 sibling, 1 reply; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-06-15 16:25 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel, broonie; +Cc: hdegoede, tiwai, amadeuszx.slawinski


> Jacks are often initialized during dai_link initialization which is
> completely out of platform_device area. This report made me think
> further - if we assign jack in dai_link->init(), we should be able to
> drop it in dai_link->exit().
> 
> Not exactly! ->init() is done once card components are already accounted
> for (available for use) but snd_soc_link_exit() is called during
> snd_soc_remove_pcm_runtime() when card components are available no
> longer - soc_remove_link_components().
> 
> TLDR: teardown path is not symmetric with its counterpart, perhaps a
> problem yet to be addressed. I'll see if moving the jack-NULLing to
> codec's DAI ->remove() won't be a better temporary (?) solution than
> reverting to platform_device->remove() usage.

It's a problem that impacted other platforms, see e.g.

static void kabylake_rt5660_codec_exit(struct snd_soc_pcm_runtime *rtd)
{
	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);

	/*
	 * The .exit() can be reached without going through the .init()
	 * so explicitly test if the gpiod is valid
	 */
	if (!IS_ERR_OR_NULL(ctx->gpio_lo_mute))
		gpiod_put(ctx->gpio_lo_mute);
}

I vaguely recall hitting this myself when working with codec properties.
It's worthy of a comment in the ASoC header to make sure this is better
known/shared.

I see in other drivers that the use of component_set_jack() is
symmetrical between .init and .exit, so far we haven't seen any issues
with sof_rt5682.c and others.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume
  2022-06-15 16:25       ` Pierre-Louis Bossart
@ 2022-06-19 13:04         ` Cezary Rojewski
  0 siblings, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-19 13:04 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: hdegoede, alsa-devel, Mark Brown, tiwai, amadeuszx.slawinski

On 2022-06-15 6:25 PM, Pierre-Louis Bossart wrote:
> 
>> Jacks are often initialized during dai_link initialization which is
>> completely out of platform_device area. This report made me think
>> further - if we assign jack in dai_link->init(), we should be able to
>> drop it in dai_link->exit().
>>
>> Not exactly! ->init() is done once card components are already accounted
>> for (available for use) but snd_soc_link_exit() is called during
>> snd_soc_remove_pcm_runtime() when card components are available no
>> longer - soc_remove_link_components().
>>
>> TLDR: teardown path is not symmetric with its counterpart, perhaps a
>> problem yet to be addressed. I'll see if moving the jack-NULLing to
>> codec's DAI ->remove() won't be a better temporary (?) solution than
>> reverting to platform_device->remove() usage.
> 
> It's a problem that impacted other platforms, see e.g.
> 
> static void kabylake_rt5660_codec_exit(struct snd_soc_pcm_runtime *rtd)
> {
> 	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);
> 
> 	/*
> 	 * The .exit() can be reached without going through the .init()
> 	 * so explicitly test if the gpiod is valid
> 	 */
> 	if (!IS_ERR_OR_NULL(ctx->gpio_lo_mute))
> 		gpiod_put(ctx->gpio_lo_mute);
> }
> 
> I vaguely recall hitting this myself when working with codec properties.
> It's worthy of a comment in the ASoC header to make sure this is better
> known/shared.
> 
> I see in other drivers that the use of component_set_jack() is
> symmetrical between .init and .exit, so far we haven't seen any issues
> with sof_rt5682.c and others.


I'll send a separate mail where we can discuss the teardown path. Don't 
believe the problem can be ignored. Even for the bdw_rt286.c usage of 
link->exit() generates:

   rt286 i2c-INT343A:00: ASoC: DAPM unknown pin LDO1

the following is the cause:

soc_remove_component() calls both component->remove() and 
snd_soc_dapm_free() for the component (in that order) so when 
link->exit() finally gets executes DAPM widgets are no longer there.


Regards,
Czarek

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume
  2022-06-15 12:57     ` Cezary Rojewski
  2022-06-15 16:25       ` Pierre-Louis Bossart
@ 2022-06-19 13:58       ` Cezary Rojewski
  1 sibling, 0 replies; 24+ messages in thread
From: Cezary Rojewski @ 2022-06-19 13:58 UTC (permalink / raw)
  To: broonie
  Cc: hdegoede, alsa-devel, Pierre-Louis Bossart, tiwai, amadeuszx.slawinski

On 2022-06-15 2:57 PM, Cezary Rojewski wrote:

> Mark,
> 
> Is it fine to leave v2 series as is, just ignoring this single 16/17 
> patch? Or should I resend entire series as v3 without this very patch? 
> I'd like to address the problem via a separate change.


Please ignore this question. I've opted to use link->exit() just like 
many other boards do and module reloads just fine. A separate thread 
will be opened for the card teardown flow discussion as mentioned by me 
earlier. link->exit() has its own problems too.

I'll send v3 tomorrow morning.


Regards,
Czarek

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update
  2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
                   ` (16 preceding siblings ...)
  2022-06-13  9:15 ` [PATCH v2 17/17] ASoC: Intel: bdw_rt286: Remove FE DAI ops Cezary Rojewski
@ 2022-06-24 10:59 ` Mark Brown
  17 siblings, 0 replies; 24+ messages in thread
From: Mark Brown @ 2022-06-24 10:59 UTC (permalink / raw)
  To: alsa-devel, cezary.rojewski
  Cc: hdegoede, tiwai, pierre-louis.bossart, amadeuszx.slawinski

On Mon, 13 Jun 2022 11:15:29 +0200, Cezary Rojewski wrote:
> A number of patches improving overall quality and readability of
> haswell.c and broadwell.c source files found in sound/soc/intel/boards.
> Both files are first renamed and only then actual changes are being
> incrementally added. The respective names are: hsw_rt5640 and bdw_rt286
> to match the pattern found in more recent boards.
> 
> Most patches bring no functional change - the more impactful patches at
> are placed the end:
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[01/17] ASoC: Intel: Rename haswell source file to hsw_rt5640
        commit: 8b99e24de3fae72ff5ef38832b94b1e41059eeed
[02/17] ASoC: Intel: hsw_rt5640: Reword prefixes of all driver members
        commit: 675002b6ca9132445e340bd106297d584e44fc9a
[03/17] ASoC: Intel: hsw_rt5640: Reword driver name
        commit: a69615e81709da0ff1f035886e8b3faf6125cd22
[04/17] ASoC: Intel: hsw_rt5640: Update code indentation
        commit: 5b66dde4ada531c1a2417d8daf68004067932a19
[05/17] ASoC: Intel: hsw_rt5640: Update file comments
        commit: 2c53debbbf04eb40854fa33813514828fa455783
[06/17] ASoC: Intel: hsw_rt5640: Improve probe() function quality
        commit: 0439f262a9b39734c1440733850969f0342c50c3
[07/17] ASoC: Intel: hsw_rt5640: Improve hw_params() debug-ability
        commit: 6c65908251edc637b53bdeb9e79d918a8d081183
[08/17] ASoC: Intel: Rename broadwell source file to bdw_rt286
        commit: 6d8758f6afd91cced9c6c5571337a5fbc6955bb2
[09/17] ASoC: Intel: bdw_rt286: Reword prefixes of all driver members
        commit: 40b5c9030a87e97c00c84403902481deadd2a57b
[10/17] ASoC: Intel: bdw_rt286: Reword driver name
        commit: 86156bcbca08ee32d04ca56c57ff3fce6fc5fc4b
[11/17] ASoC: Intel: bdw_rt286: Update code indentation
        commit: 9de833d2dcd43c953f7869f27bffd41896adb425
[12/17] ASoC: Intel: bdw_rt286: Update file comments
        commit: 128bb6fb530841348ee4d9b4234b30006c44c803
[13/17] ASoC: Intel: bdw_rt286: Improve probe() function quality
        commit: 9177203c209d9137dce52c7f0bc28e54960e5a41
[14/17] ASoC: Intel: bdw_rt286: Improve hw_params() debug-ability
        commit: 423cc2d0e8506a0ce6e3ef1806a561de1076e033
[15/17] ASoC: Intel: bdw_rt286: Improve codec_link_init() quality
        (no commit info)
[16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume
        (no commit info)
[17/17] ASoC: Intel: bdw_rt286: Remove FE DAI ops
        commit: e7f68863545163ec75b6bc3cc48fe888c28e0ec6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2022-06-24 11:00 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13  9:15 [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 01/17] ASoC: Intel: Rename haswell source file to hsw_rt5640 Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 02/17] ASoC: Intel: hsw_rt5640: Reword prefixes of all driver members Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 03/17] ASoC: Intel: hsw_rt5640: Reword driver name Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 04/17] ASoC: Intel: hsw_rt5640: Update code indentation Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 05/17] ASoC: Intel: hsw_rt5640: Update file comments Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 06/17] ASoC: Intel: hsw_rt5640: Improve probe() function quality Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 07/17] ASoC: Intel: hsw_rt5640: Improve hw_params() debug-ability Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 08/17] ASoC: Intel: Rename broadwell source file to bdw_rt286 Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 09/17] ASoC: Intel: bdw_rt286: Reword prefixes of all driver members Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 10/17] ASoC: Intel: bdw_rt286: Reword driver name Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 11/17] ASoC: Intel: bdw_rt286: Update code indentation Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 12/17] ASoC: Intel: bdw_rt286: Update file comments Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 13/17] ASoC: Intel: bdw_rt286: Improve probe() function quality Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 14/17] ASoC: Intel: bdw_rt286: Improve hw_params() debug-ability Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 15/17] ASoC: Intel: bdw_rt286: Improve codec_link_init() quality Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume Cezary Rojewski
2022-06-15  1:27   ` Pierre-Louis Bossart
2022-06-15 12:57     ` Cezary Rojewski
2022-06-15 16:25       ` Pierre-Louis Bossart
2022-06-19 13:04         ` Cezary Rojewski
2022-06-19 13:58       ` Cezary Rojewski
2022-06-13  9:15 ` [PATCH v2 17/17] ASoC: Intel: bdw_rt286: Remove FE DAI ops Cezary Rojewski
2022-06-24 10:59 ` [PATCH v2 00/17] ASoC: Intel: haswell and broadwell boards update Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.