All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple
@ 2021-12-16 11:57 Cezary Rojewski
  2021-12-16 11:57 ` [PATCH 1/5] ASoC: Intel: catpt: Test dmaengine_submit() result before moving on Cezary Rojewski
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-16 11:57 UTC (permalink / raw)
  To: alsa-devel; +Cc: Cezary Rojewski, broonie, tiwai

Set is made of one fix for dma-transfer so that result of
dmaengine_submit() is tested before moving on, and few cleanups:

- two non-impactful, where catpt_component_open() layout gets improved
  slightly as well as relocation of couple of locals found in
  PCM-functions so that they look more cohesive
- no need to expose catpt-driver board-matching information globally.
  Most fields are not by it and it's the sole user of haswell_machines
  table. By having them locally it is clear what is actually being used

Cezary Rojewski (5):
  ASoC: Intel: catpt: Test dmaengine_submit() result before moving on
  ASoC: Intel: catpt: Reduce size of catpt_component_open()
  ASoC: Intel: catpt: Streamline locals declaration for PCM-functions
  ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  ASoC: Intel: Drop legacy HSW/BDW board-match information

 include/sound/soc-acpi-intel-match.h          |  1 -
 sound/soc/intel/Kconfig                       |  2 +-
 sound/soc/intel/catpt/device.c                | 33 +++++++++++++++--
 sound/soc/intel/catpt/dsp.c                   | 14 ++++++-
 sound/soc/intel/catpt/pcm.c                   | 37 +++++++++----------
 .../common/soc-acpi-intel-hsw-bdw-match.c     | 16 --------
 6 files changed, 61 insertions(+), 42 deletions(-)

-- 
2.25.1


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

* [PATCH 1/5] ASoC: Intel: catpt: Test dmaengine_submit() result before moving on
  2021-12-16 11:57 [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple Cezary Rojewski
@ 2021-12-16 11:57 ` Cezary Rojewski
  2021-12-16 11:57 ` [PATCH 2/5] ASoC: Intel: catpt: Reduce size of catpt_component_open() Cezary Rojewski
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-16 11:57 UTC (permalink / raw)
  To: alsa-devel; +Cc: Kevin Tian, Cezary Rojewski, broonie, Dave Jiang, tiwai

After calling dmaengine_submit(), the submitted transfer descriptor
belongs to the DMA engine. Pointer to that descriptor may no longer be
valid after the call and should be tested before awaiting transfer
completion.

Reported-by: Kevin Tian <kevin.tian@intel.com>
Suggested-by: Dave Jiang <dave.jiang@intel.com>
Fixes: 4fac9b31d0b9 ("ASoC: Intel: Add catpt base members")
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/dsp.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/catpt/dsp.c b/sound/soc/intel/catpt/dsp.c
index 9c5fd18f2600..346bec000306 100644
--- a/sound/soc/intel/catpt/dsp.c
+++ b/sound/soc/intel/catpt/dsp.c
@@ -65,6 +65,7 @@ static int catpt_dma_memcpy(struct catpt_dev *cdev, struct dma_chan *chan,
 {
 	struct dma_async_tx_descriptor *desc;
 	enum dma_status status;
+	int ret;
 
 	desc = dmaengine_prep_dma_memcpy(chan, dst_addr, src_addr, size,
 					 DMA_CTRL_ACK);
@@ -77,13 +78,22 @@ static int catpt_dma_memcpy(struct catpt_dev *cdev, struct dma_chan *chan,
 	catpt_updatel_shim(cdev, HMDC,
 			   CATPT_HMDC_HDDA(CATPT_DMA_DEVID, chan->chan_id),
 			   CATPT_HMDC_HDDA(CATPT_DMA_DEVID, chan->chan_id));
-	dmaengine_submit(desc);
+
+	ret = dma_submit_error(dmaengine_submit(desc));
+	if (ret) {
+		dev_err(cdev->dev, "submit tx failed: %d\n", ret);
+		goto clear_hdda;
+	}
+
 	status = dma_wait_for_async_tx(desc);
+	ret = (status == DMA_COMPLETE) ? 0 : -EPROTO;
+
+clear_hdda:
 	/* regardless of status, disable access to HOST memory in demand mode */
 	catpt_updatel_shim(cdev, HMDC,
 			   CATPT_HMDC_HDDA(CATPT_DMA_DEVID, chan->chan_id), 0);
 
-	return (status == DMA_COMPLETE) ? 0 : -EPROTO;
+	return ret;
 }
 
 int catpt_dma_memcpy_todsp(struct catpt_dev *cdev, struct dma_chan *chan,
-- 
2.25.1


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

* [PATCH 2/5] ASoC: Intel: catpt: Reduce size of catpt_component_open()
  2021-12-16 11:57 [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple Cezary Rojewski
  2021-12-16 11:57 ` [PATCH 1/5] ASoC: Intel: catpt: Test dmaengine_submit() result before moving on Cezary Rojewski
@ 2021-12-16 11:57 ` Cezary Rojewski
  2021-12-16 11:57 ` [PATCH 3/5] ASoC: Intel: catpt: Streamline locals declaration for PCM-functions Cezary Rojewski
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-16 11:57 UTC (permalink / raw)
  To: alsa-devel; +Cc: Cezary Rojewski, broonie, tiwai

With some improved if-logy, function's size can be reduced slightly.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/pcm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/catpt/pcm.c b/sound/soc/intel/catpt/pcm.c
index ebb27daeb1c7..16146c693c08 100644
--- a/sound/soc/intel/catpt/pcm.c
+++ b/sound/soc/intel/catpt/pcm.c
@@ -595,9 +595,8 @@ static int catpt_component_open(struct snd_soc_component *component,
 {
 	struct snd_soc_pcm_runtime *rtm = substream->private_data;
 
-	if (rtm->dai_link->no_pcm)
-		return 0;
-	snd_soc_set_runtime_hwparams(substream, &catpt_pcm_hardware);
+	if (!rtm->dai_link->no_pcm)
+		snd_soc_set_runtime_hwparams(substream, &catpt_pcm_hardware);
 	return 0;
 }
 
-- 
2.25.1


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

* [PATCH 3/5] ASoC: Intel: catpt: Streamline locals declaration for PCM-functions
  2021-12-16 11:57 [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple Cezary Rojewski
  2021-12-16 11:57 ` [PATCH 1/5] ASoC: Intel: catpt: Test dmaengine_submit() result before moving on Cezary Rojewski
  2021-12-16 11:57 ` [PATCH 2/5] ASoC: Intel: catpt: Reduce size of catpt_component_open() Cezary Rojewski
@ 2021-12-16 11:57 ` Cezary Rojewski
  2021-12-16 11:57 ` [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency Cezary Rojewski
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-16 11:57 UTC (permalink / raw)
  To: alsa-devel; +Cc: Cezary Rojewski, broonie, tiwai

Group all the catpt_xxx structs together in PCM related functions so
they look more cohesive.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/pcm.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/soc/intel/catpt/pcm.c b/sound/soc/intel/catpt/pcm.c
index 16146c693c08..939a9b801dec 100644
--- a/sound/soc/intel/catpt/pcm.c
+++ b/sound/soc/intel/catpt/pcm.c
@@ -259,9 +259,9 @@ static enum catpt_channel_config catpt_get_channel_config(u32 num_channels)
 static int catpt_dai_startup(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
-	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	struct catpt_stream_template *template;
 	struct catpt_stream_runtime *stream;
+	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	struct resource *res;
 	int ret;
 
@@ -306,8 +306,8 @@ static int catpt_dai_startup(struct snd_pcm_substream *substream,
 static void catpt_dai_shutdown(struct snd_pcm_substream *substream,
 			       struct snd_soc_dai *dai)
 {
-	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	struct catpt_stream_runtime *stream;
+	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 
 	stream = snd_soc_dai_get_dma_data(dai, substream);
 
@@ -329,9 +329,9 @@ static int catpt_set_dspvol(struct catpt_dev *cdev, u8 stream_id, long *ctlvol);
 static int catpt_dai_apply_usettings(struct snd_soc_dai *dai,
 				     struct catpt_stream_runtime *stream)
 {
-	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	struct snd_soc_component *component = dai->component;
 	struct snd_kcontrol *pos;
+	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	const char *name;
 	int ret;
 	u32 id = stream->info.stream_hw_id;
@@ -374,12 +374,12 @@ static int catpt_dai_hw_params(struct snd_pcm_substream *substream,
 			       struct snd_pcm_hw_params *params,
 			       struct snd_soc_dai *dai)
 {
-	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
+	struct snd_pcm_runtime *rtm = substream->runtime;
+	struct snd_dma_buffer *dmab;
 	struct catpt_stream_runtime *stream;
 	struct catpt_audio_format afmt;
 	struct catpt_ring_info rinfo;
-	struct snd_pcm_runtime *rtm = substream->runtime;
-	struct snd_dma_buffer *dmab;
+	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	int ret;
 
 	stream = snd_soc_dai_get_dma_data(dai, substream);
@@ -427,8 +427,8 @@ static int catpt_dai_hw_params(struct snd_pcm_substream *substream,
 static int catpt_dai_hw_free(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
-	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	struct catpt_stream_runtime *stream;
+	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 
 	stream = snd_soc_dai_get_dma_data(dai, substream);
 	if (!stream->allocated)
@@ -444,8 +444,8 @@ static int catpt_dai_hw_free(struct snd_pcm_substream *substream,
 static int catpt_dai_prepare(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
-	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	struct catpt_stream_runtime *stream;
+	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	int ret;
 
 	stream = snd_soc_dai_get_dma_data(dai, substream);
@@ -467,9 +467,9 @@ static int catpt_dai_prepare(struct snd_pcm_substream *substream,
 static int catpt_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 			     struct snd_soc_dai *dai)
 {
-	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
-	struct catpt_stream_runtime *stream;
 	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct catpt_stream_runtime *stream;
+	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	snd_pcm_uframes_t pos;
 	int ret;
 
@@ -604,10 +604,10 @@ static snd_pcm_uframes_t
 catpt_component_pointer(struct snd_soc_component *component,
 			struct snd_pcm_substream *substream)
 {
-	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
-	struct catpt_stream_runtime *stream;
 	struct snd_soc_pcm_runtime *rtm = substream->private_data;
 	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtm, 0);
+	struct catpt_stream_runtime *stream;
+	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	u32 pos;
 
 	if (rtm->dai_link->no_pcm)
@@ -632,8 +632,8 @@ static int catpt_dai_pcm_new(struct snd_soc_pcm_runtime *rtm,
 			     struct snd_soc_dai *dai)
 {
 	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtm, 0);
-	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	struct catpt_ssp_device_format devfmt;
+	struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
 	int ret;
 
 	devfmt.iface = dai->driver->id;
@@ -893,8 +893,8 @@ static int catpt_stream_volume_get(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_component *component =
 		snd_soc_kcontrol_component(kcontrol);
-	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	struct catpt_stream_runtime *stream;
+	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	long *ctlvol = (long *)kcontrol->private_value;
 	u32 dspvol;
 	int i;
@@ -925,8 +925,8 @@ static int catpt_stream_volume_put(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_component *component =
 		snd_soc_kcontrol_component(kcontrol);
-	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	struct catpt_stream_runtime *stream;
+	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	long *ctlvol = (long *)kcontrol->private_value;
 	int ret, i;
 
@@ -1001,8 +1001,8 @@ static int catpt_loopback_switch_put(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_component *component =
 		snd_soc_kcontrol_component(kcontrol);
-	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	struct catpt_stream_runtime *stream;
+	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	bool mute;
 	int ret;
 
-- 
2.25.1


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

* [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 11:57 [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple Cezary Rojewski
                   ` (2 preceding siblings ...)
  2021-12-16 11:57 ` [PATCH 3/5] ASoC: Intel: catpt: Streamline locals declaration for PCM-functions Cezary Rojewski
@ 2021-12-16 11:57 ` Cezary Rojewski
  2021-12-16 14:11   ` Pierre-Louis Bossart
  2021-12-17  2:42   ` kernel test robot
  2021-12-16 11:57 ` [PATCH 5/5] ASoC: Intel: Drop legacy HSW/BDW board-match information Cezary Rojewski
  2021-12-21  2:50 ` (subset) [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple Mark Brown
  5 siblings, 2 replies; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-16 11:57 UTC (permalink / raw)
  To: alsa-devel; +Cc: Cezary Rojewski, broonie, tiwai

catpt-driver does not make use of most of the fields found in the
descriptor table and is the sole user of haswell machines list. Move the
tables to local directory and clean them up so it's clear what's
actually used by the solution.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/Kconfig        |  2 +-
 sound/soc/intel/catpt/device.c | 33 ++++++++++++++++++++++++++++++---
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index f3a4a907b29d..0423009a186e 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -23,7 +23,7 @@ config SND_SOC_INTEL_CATPT
 	depends on ACPI || COMPILE_TEST
 	depends on DMADEVICES && SND_DMA_SGBUF
 	select DW_DMAC_CORE
-	select SND_SOC_ACPI_INTEL_MATCH
+	select SND_SOC_ACPI
 	select WANT_DEV_COREDUMP
 	select SND_INTEL_DSP_CONFIG
 	help
diff --git a/sound/soc/intel/catpt/device.c b/sound/soc/intel/catpt/device.c
index 85a34e37316d..81c7cb94b68b 100644
--- a/sound/soc/intel/catpt/device.c
+++ b/sound/soc/intel/catpt/device.c
@@ -22,7 +22,6 @@
 #include <sound/intel-dsp-config.h>
 #include <sound/soc.h>
 #include <sound/soc-acpi.h>
-#include <sound/soc-acpi-intel-match.h>
 #include "core.h"
 #include "registers.h"
 
@@ -313,8 +312,36 @@ static int catpt_acpi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+struct snd_soc_acpi_mach lpt_machines[] = {
+	{
+		.id = "INT33CA",
+		.drv_name = "haswell-audio",
+	},
+	{}
+};
+
+struct snd_soc_acpi_mach wpt_machines[] = {
+	{
+		.id = "INT343A",
+		.drv_name = "broadwell-audio",
+	},
+	{
+		.id = "10EC5650",
+		.drv_name = "bdw-rt5650",
+	},
+	{
+		.id = "RT5677CE",
+		.drv_name = "bdw-rt5677",
+	},
+	{
+		.id = "INT33CA",
+		.drv_name = "haswell-audio",
+	},
+	{}
+};
+
 static struct catpt_spec lpt_desc = {
-	.machines = snd_soc_acpi_intel_haswell_machines,
+	.machines = lpt_machines,
 	.core_id = 0x01,
 	.host_dram_offset = 0x000000,
 	.host_iram_offset = 0x080000,
@@ -329,7 +356,7 @@ static struct catpt_spec lpt_desc = {
 };
 
 static struct catpt_spec wpt_desc = {
-	.machines = snd_soc_acpi_intel_broadwell_machines,
+	.machines = wpt_machines,
 	.core_id = 0x02,
 	.host_dram_offset = 0x000000,
 	.host_iram_offset = 0x0A0000,
-- 
2.25.1


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

* [PATCH 5/5] ASoC: Intel: Drop legacy HSW/BDW board-match information
  2021-12-16 11:57 [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple Cezary Rojewski
                   ` (3 preceding siblings ...)
  2021-12-16 11:57 ` [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency Cezary Rojewski
@ 2021-12-16 11:57 ` Cezary Rojewski
  2021-12-21  2:50 ` (subset) [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple Mark Brown
  5 siblings, 0 replies; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-16 11:57 UTC (permalink / raw)
  To: alsa-devel; +Cc: Cezary Rojewski, broonie, tiwai

With board-matching information for legacy solution moved to local
directory, there is no need to expose it globally.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/soc-acpi-intel-match.h             |  1 -
 .../intel/common/soc-acpi-intel-hsw-bdw-match.c  | 16 ----------------
 2 files changed, 17 deletions(-)

diff --git a/include/sound/soc-acpi-intel-match.h b/include/sound/soc-acpi-intel-match.h
index 59551b1f22f3..7b94451ab93a 100644
--- a/include/sound/soc-acpi-intel-match.h
+++ b/include/sound/soc-acpi-intel-match.h
@@ -14,7 +14,6 @@
  * these tables are not constants, some fields can be used for
  * pdata or machine ops
  */
-extern struct snd_soc_acpi_mach snd_soc_acpi_intel_haswell_machines[];
 extern struct snd_soc_acpi_mach snd_soc_acpi_intel_broadwell_machines[];
 extern struct snd_soc_acpi_mach snd_soc_acpi_intel_baytrail_machines[];
 extern struct snd_soc_acpi_mach snd_soc_acpi_intel_cherrytrail_machines[];
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 fe343a95b5ff..25758c23da17 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
@@ -9,44 +9,28 @@
 #include <sound/soc-acpi.h>
 #include <sound/soc-acpi-intel-match.h>
 
-struct snd_soc_acpi_mach snd_soc_acpi_intel_haswell_machines[] = {
-	{
-		.id = "INT33CA",
-		.drv_name = "haswell-audio",
-		.fw_filename = "intel/IntcSST1.bin",
-		.sof_fw_filename = "sof-hsw.ri",
-		.sof_tplg_filename = "sof-hsw.tplg",
-	},
-	{}
-};
-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",
-		.fw_filename =  "intel/IntcSST2.bin",
 		.sof_fw_filename = "sof-bdw.ri",
 		.sof_tplg_filename = "sof-bdw-rt286.tplg",
 	},
 	{
 		.id = "10EC5650",
 		.drv_name = "bdw-rt5650",
-		.fw_filename = "intel/IntcSST2.bin",
 		.sof_fw_filename = "sof-bdw.ri",
 		.sof_tplg_filename = "sof-bdw-rt5650.tplg",
 	},
 	{
 		.id = "RT5677CE",
 		.drv_name = "bdw-rt5677",
-		.fw_filename =  "intel/IntcSST2.bin",
 		.sof_fw_filename = "sof-bdw.ri",
 		.sof_tplg_filename = "sof-bdw-rt5677.tplg",
 	},
 	{
 		.id = "INT33CA",
 		.drv_name = "haswell-audio",
-		.fw_filename = "intel/IntcSST2.bin",
 		.sof_fw_filename = "sof-bdw.ri",
 		.sof_tplg_filename = "sof-bdw-rt5640.tplg",
 	},
-- 
2.25.1


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

* Re: [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 11:57 ` [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency Cezary Rojewski
@ 2021-12-16 14:11   ` Pierre-Louis Bossart
  2021-12-16 14:37     ` Cezary Rojewski
  2021-12-17  2:42   ` kernel test robot
  1 sibling, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2021-12-16 14:11 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel; +Cc: broonie, tiwai


> +struct snd_soc_acpi_mach lpt_machines[] = {
> +	{
> +		.id = "INT33CA",
> +		.drv_name = "haswell-audio",
> +	},
> +	{}
> +};
> +
> +struct snd_soc_acpi_mach wpt_machines[] = {
> +	{
> +		.id = "INT343A",
> +		.drv_name = "broadwell-audio",
> +	},
> +	{
> +		.id = "10EC5650",
> +		.drv_name = "bdw-rt5650",
> +	},
> +	{
> +		.id = "RT5677CE",
> +		.drv_name = "bdw-rt5677",
> +	},
> +	{
> +		.id = "INT33CA",
> +		.drv_name = "haswell-audio",
> +	},
> +	{}
> +};

The intent of soc-acpi files is to establish a match between ACPI _HID
and machine driver, this is now duplicated, and it makes limited sense
to add machine driver dependencies in a platform driver.

Nothing was broken with the existing code.


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

* Re: [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 14:11   ` Pierre-Louis Bossart
@ 2021-12-16 14:37     ` Cezary Rojewski
  2021-12-16 15:13       ` Pierre-Louis Bossart
  0 siblings, 1 reply; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-16 14:37 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel; +Cc: broonie, tiwai

On 2021-12-16 3:11 PM, Pierre-Louis Bossart wrote:
> The intent of soc-acpi files is to establish a match between ACPI _HID
> and machine driver, this is now duplicated, and it makes limited sense
> to add machine driver dependencies in a platform driver.
> 
> Nothing was broken with the existing code.

Hello,

Yes, nothing is broken in the existing code. The intention is different 
- be cohesive about what is actually used by the driver.

PCI-ids table is duplicated already for the Intel audio drivers. And 
it's OK to do so - one knows which ids are covered by given driver and 
how. Here, it's clear that haswell_machines are only used by 
catpt-driver and so are some fields for broadwell_machines. In time I 
believe that we will be able to reduce the number of fields for struct 
snd_soc_acpi_mach i.e. have a single fw_filename and single 
tplg_filename field without some driver-specific duplicates.

About the last, there could be a case where no topology file is 
available for certain configuration and given entry should not be taken 
into account. While catpt-driver does not make use of soc-topology 
feature, that isn't true for other drivers.


Regards,
Czarek

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

* Re: [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 14:37     ` Cezary Rojewski
@ 2021-12-16 15:13       ` Pierre-Louis Bossart
  2021-12-16 15:59         ` Cezary Rojewski
  0 siblings, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2021-12-16 15:13 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel; +Cc: broonie, tiwai



On 12/16/21 8:37 AM, Cezary Rojewski wrote:
> On 2021-12-16 3:11 PM, Pierre-Louis Bossart wrote:
>> The intent of soc-acpi files is to establish a match between ACPI _HID
>> and machine driver, this is now duplicated, and it makes limited sense
>> to add machine driver dependencies in a platform driver.
>>
>> Nothing was broken with the existing code.
> 
> Hello,
> 
> Yes, nothing is broken in the existing code. The intention is different
> - be cohesive about what is actually used by the driver.
> 
> PCI-ids table is duplicated already for the Intel audio drivers. And
> it's OK to do so - one knows which ids are covered by given driver and
> how. Here, it's clear that haswell_machines are only used by
> catpt-driver and so are some fields for broadwell_machines. In time I
> believe that we will be able to reduce the number of fields for struct
> snd_soc_acpi_mach i.e. have a single fw_filename and single
> tplg_filename field without some driver-specific duplicates.
I don't really see the point about the number of fields, this is a
generic descriptor used for I2S/SoundWire devices so mechanically there
are things are are not used in all platforms.

Another example is the quirks field, it's only meant to be used when
there's actually a quirk.

Note that I am planning to remove the sof_fw_filename field since it's
redundant with what is part of the PCI descriptor, but the topology will
remain there: it has to match with the machine driver.

> About the last, there could be a case where no topology file is
> available for certain configuration and given entry should not be taken
> into account. While catpt-driver does not make use of soc-topology
> feature, that isn't true for other drivers.

Again if a feature is not needed/not supported, the field can remain empty.

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

* Re: [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 15:13       ` Pierre-Louis Bossart
@ 2021-12-16 15:59         ` Cezary Rojewski
  2021-12-16 16:29           ` Pierre-Louis Bossart
  0 siblings, 1 reply; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-16 15:59 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel; +Cc: broonie, tiwai

On 2021-12-16 4:13 PM, Pierre-Louis Bossart wrote:

...

> I don't really see the point about the number of fields, this is a
> generic descriptor used for I2S/SoundWire devices so mechanically there
> are things are are not used in all platforms.
> 
> Another example is the quirks field, it's only meant to be used when
> there's actually a quirk.
> 
> Note that I am planning to remove the sof_fw_filename field since it's
> redundant with what is part of the PCI descriptor, but the topology will
> remain there: it has to match with the machine driver.

That's why no new struct is declared. Simply the tables are moved 
locally, and there is nothing wrong with that. Cohesiveness and 
readability outweighs the duplication of ACPI _HID.


After thinking about this again, perhaps in the future, this generic 
descriptor should be split into more specific bits e.g.: like the struct 
pci_driver which wraps struct driver.

i2s_mach = {
	// some i2s-specific fields e.g.:
	.acpi-id = XXX,
	.mach = {
		// some generic fields e.g.:
		.drv_name = "a machine board name",
	},
};


Regards,
Czarek

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

* Re: [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 15:59         ` Cezary Rojewski
@ 2021-12-16 16:29           ` Pierre-Louis Bossart
  2021-12-16 16:50             ` Cezary Rojewski
  0 siblings, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2021-12-16 16:29 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel; +Cc: broonie, tiwai


>> I don't really see the point about the number of fields, this is a
>> generic descriptor used for I2S/SoundWire devices so mechanically there
>> are things are are not used in all platforms.
>>
>> Another example is the quirks field, it's only meant to be used when
>> there's actually a quirk.
>>
>> Note that I am planning to remove the sof_fw_filename field since it's
>> redundant with what is part of the PCI descriptor, but the topology will
>> remain there: it has to match with the machine driver.
> 
> That's why no new struct is declared. Simply the tables are moved
> locally, and there is nothing wrong with that. Cohesiveness and
> readability outweighs the duplication of ACPI _HID.

If I follow your logic, I could move all the tables for glk, cnl, cfl,
cml, icl, jsl, tgl, ehl, adl into the SOF driver. That really doesn't
seem sensible.

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

* Re: [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 16:29           ` Pierre-Louis Bossart
@ 2021-12-16 16:50             ` Cezary Rojewski
  2021-12-16 18:04               ` Pierre-Louis Bossart
  0 siblings, 1 reply; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-16 16:50 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel; +Cc: broonie, tiwai

On 2021-12-16 5:29 PM, Pierre-Louis Bossart wrote:
>>> I don't really see the point about the number of fields, this is a
>>> generic descriptor used for I2S/SoundWire devices so mechanically there
>>> are things are are not used in all platforms.
>>>
>>> Another example is the quirks field, it's only meant to be used when
>>> there's actually a quirk.
>>>
>>> Note that I am planning to remove the sof_fw_filename field since it's
>>> redundant with what is part of the PCI descriptor, but the topology will
>>> remain there: it has to match with the machine driver.
>>
>> That's why no new struct is declared. Simply the tables are moved
>> locally, and there is nothing wrong with that. Cohesiveness and
>> readability outweighs the duplication of ACPI _HID.
> 
> If I follow your logic, I could move all the tables for glk, cnl, cfl,
> cml, icl, jsl, tgl, ehl, adl into the SOF driver. That really doesn't
> seem sensible.

Hmm.. doesn't it really? Are the glk/cnl/cfl/cml/icl/jsl/tgl/ehl/adl 
tables "common" for the Intel audio drivers? There are not used by 
anything except for SOF. It seems reasonable to have them present 
locally too. SOF solution becomes more cohesively organized in such 
case, just like catpt is after this patch.


Regards,
Czarek

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

* Re: [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 16:50             ` Cezary Rojewski
@ 2021-12-16 18:04               ` Pierre-Louis Bossart
  2021-12-17  9:06                 ` Cezary Rojewski
  0 siblings, 1 reply; 16+ messages in thread
From: Pierre-Louis Bossart @ 2021-12-16 18:04 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel; +Cc: broonie, tiwai



>>>> I don't really see the point about the number of fields, this is a
>>>> generic descriptor used for I2S/SoundWire devices so mechanically there
>>>> are things are are not used in all platforms.
>>>>
>>>> Another example is the quirks field, it's only meant to be used when
>>>> there's actually a quirk.
>>>>
>>>> Note that I am planning to remove the sof_fw_filename field since it's
>>>> redundant with what is part of the PCI descriptor, but the topology
>>>> will
>>>> remain there: it has to match with the machine driver.
>>>
>>> That's why no new struct is declared. Simply the tables are moved
>>> locally, and there is nothing wrong with that. Cohesiveness and
>>> readability outweighs the duplication of ACPI _HID.
>>
>> If I follow your logic, I could move all the tables for glk, cnl, cfl,
>> cml, icl, jsl, tgl, ehl, adl into the SOF driver. That really doesn't
>> seem sensible.
> 
> Hmm.. doesn't it really? Are the glk/cnl/cfl/cml/icl/jsl/tgl/ehl/adl
> tables "common" for the Intel audio drivers? There are not used by
> anything except for SOF. It seems reasonable to have them present
> locally too. SOF solution becomes more cohesively organized in such
> case, just like catpt is after this patch.

We could also move the boards/ while we're at it, on the grounds they
are not all used by all drivers.

My take is that unless a new feature is added that warrants moving the
tables around, let's keep the existing code as is.

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

* Re: [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 11:57 ` [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency Cezary Rojewski
  2021-12-16 14:11   ` Pierre-Louis Bossart
@ 2021-12-17  2:42   ` kernel test robot
  1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2021-12-17  2:42 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 15728 bytes --]

Hi Cezary,

I love your patch! Yet something to improve:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on tiwai-sound/for-next linus/master v5.16-rc5 next-20211215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Cezary-Rojewski/ASoC-Intel-catpt-Dma-transfer-fix-and-couple/20211216-195950
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: x86_64-buildonly-randconfig-r003-20211216 (https://download.01.org/0day-ci/archive/20211217/202112171009.7QVDVnji-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/1ad5d0805b19369eabd9476253a94343fe09ce75
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cezary-Rojewski/ASoC-Intel-catpt-Dma-transfer-fix-and-couple/20211216-195950
        git checkout 1ad5d0805b19369eabd9476253a94343fe09ce75
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash sound/soc/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> sound/soc/soc-acpi.c:34:1: error: redefinition of 'snd_soc_acpi_find_machine'
      34 | snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines)
         | ^~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from sound/soc/soc-acpi.c:9:
   include/sound/soc-acpi.h:38:1: note: previous definition of 'snd_soc_acpi_find_machine' was here
      38 | snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines)
         | ^~~~~~~~~~~~~~~~~~~~~~~~~
   sound/soc/soc-acpi.c: In function 'snd_soc_acpi_find_package':
>> sound/soc/soc-acpi.c:64:6: error: implicit declaration of function 'acpi_bus_get_device'; did you mean 'acpi_get_gpe_device'? [-Werror=implicit-function-declaration]
      64 |  if (acpi_bus_get_device(handle, &adev))
         |      ^~~~~~~~~~~~~~~~~~~
         |      acpi_get_gpe_device
>> sound/soc/soc-acpi.c:67:10: error: dereferencing pointer to incomplete type 'struct acpi_device'
      67 |  if (adev->status.present && adev->status.functional) {
         |          ^~
>> sound/soc/soc-acpi.c:83:12: error: implicit declaration of function 'acpi_extract_package' [-Werror=implicit-function-declaration]
      83 |   status = acpi_extract_package(myobj,
         |            ^~~~~~~~~~~~~~~~~~~~
   sound/soc/soc-acpi.c: At top level:
>> sound/soc/soc-acpi.c:98:6: error: redefinition of 'snd_soc_acpi_find_package_from_hid'
      98 | bool snd_soc_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN],
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from sound/soc/soc-acpi.c:9:
   include/sound/soc-acpi.h:44:1: note: previous definition of 'snd_soc_acpi_find_package_from_hid' was here
      44 | snd_soc_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN],
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> sound/soc/soc-acpi.c:112:27: error: redefinition of 'snd_soc_acpi_codec_list'
     112 | struct snd_soc_acpi_mach *snd_soc_acpi_codec_list(void *arg)
         |                           ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from sound/soc/soc-acpi.c:9:
   include/sound/soc-acpi.h:51:41: note: previous definition of 'snd_soc_acpi_codec_list' was here
      51 | static inline struct snd_soc_acpi_mach *snd_soc_acpi_codec_list(void *arg)
         |                                         ^~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/snd_soc_acpi_find_machine +34 sound/soc/soc-acpi.c

cafa39b650ec3ba sound/soc/soc-acpi.c                    Brent Lu             2021-10-30   32  
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12   33  struct snd_soc_acpi_mach *
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12  @34  snd_soc_acpi_find_machine(struct snd_soc_acpi_mach *machines)
95f098014815b33 sound/soc/intel/common/sst-match-acpi.c Vinod Koul           2015-11-05   35  {
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12   36  	struct snd_soc_acpi_mach *mach;
a3e620f8422832a sound/soc/soc-acpi.c                    Keyon Jie            2018-11-16   37  	struct snd_soc_acpi_mach *mach_alt;
95f098014815b33 sound/soc/intel/common/sst-match-acpi.c Vinod Koul           2015-11-05   38  
cafa39b650ec3ba sound/soc/soc-acpi.c                    Brent Lu             2021-10-30   39  	for (mach = machines; mach->id[0] || mach->comp_ids; mach++) {
cafa39b650ec3ba sound/soc/soc-acpi.c                    Brent Lu             2021-10-30   40  		if (snd_soc_acpi_id_present(mach)) {
a3e620f8422832a sound/soc/soc-acpi.c                    Keyon Jie            2018-11-16   41  			if (mach->machine_quirk) {
a3e620f8422832a sound/soc/soc-acpi.c                    Keyon Jie            2018-11-16   42  				mach_alt = mach->machine_quirk(mach);
a3e620f8422832a sound/soc/soc-acpi.c                    Keyon Jie            2018-11-16   43  				if (!mach_alt)
a3e620f8422832a sound/soc/soc-acpi.c                    Keyon Jie            2018-11-16   44  					continue; /* not full match, ignore */
a3e620f8422832a sound/soc/soc-acpi.c                    Keyon Jie            2018-11-16   45  				mach = mach_alt;
a3e620f8422832a sound/soc/soc-acpi.c                    Keyon Jie            2018-11-16   46  			}
a3e620f8422832a sound/soc/soc-acpi.c                    Keyon Jie            2018-11-16   47  
7827d66946ad3af sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15   48  			return mach;
7827d66946ad3af sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15   49  		}
7827d66946ad3af sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15   50  	}
95f098014815b33 sound/soc/intel/common/sst-match-acpi.c Vinod Koul           2015-11-05   51  	return NULL;
95f098014815b33 sound/soc/intel/common/sst-match-acpi.c Vinod Koul           2015-11-05   52  }
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12   53  EXPORT_SYMBOL_GPL(snd_soc_acpi_find_machine);
8ceffd229f0ef13 sound/soc/intel/common/sst-match-acpi.c Vinod Koul           2016-02-08   54  
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12   55  static acpi_status snd_soc_acpi_find_package(acpi_handle handle, u32 level,
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   56  					     void *context, void **ret)
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   57  {
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   58  	struct acpi_device *adev;
59ce3233a538fc2 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2021-04-16   59  	acpi_status status;
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12   60  	struct snd_soc_acpi_package_context *pkg_ctx = context;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   61  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   62  	pkg_ctx->data_valid = false;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   63  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  @64  	if (acpi_bus_get_device(handle, &adev))
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   65  		return AE_OK;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   66  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  @67  	if (adev->status.present && adev->status.functional) {
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   68  		struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   69  		union acpi_object  *myobj = NULL;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   70  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   71  		status = acpi_evaluate_object_typed(handle, pkg_ctx->name,
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   72  						NULL, &buffer,
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   73  						ACPI_TYPE_PACKAGE);
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   74  		if (ACPI_FAILURE(status))
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   75  			return AE_OK;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   76  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   77  		myobj = buffer.pointer;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   78  		if (!myobj || myobj->package.count != pkg_ctx->length) {
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   79  			kfree(buffer.pointer);
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   80  			return AE_OK;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   81  		}
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   82  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  @83  		status = acpi_extract_package(myobj,
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   84  					pkg_ctx->format, pkg_ctx->state);
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   85  		if (ACPI_FAILURE(status)) {
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   86  			kfree(buffer.pointer);
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   87  			return AE_OK;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   88  		}
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   89  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   90  		kfree(buffer.pointer);
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   91  		pkg_ctx->data_valid = true;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   92  		return AE_CTRL_TERMINATE;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   93  	}
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   94  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   95  	return AE_OK;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   96  }
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12   97  
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12  @98  bool snd_soc_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN],
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12   99  				struct snd_soc_acpi_package_context *ctx)
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  100  {
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  101  	acpi_status status;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  102  
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12  103  	status = acpi_get_devices(hid, snd_soc_acpi_find_package, ctx, NULL);
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  104  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  105  	if (ACPI_FAILURE(status) || !ctx->data_valid)
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  106  		return false;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  107  
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  108  	return true;
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  109  }
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12  110  EXPORT_SYMBOL_GPL(snd_soc_acpi_find_package_from_hid);
3421894765a345c sound/soc/intel/common/sst-match-acpi.c Pierre-Louis Bossart 2016-11-12  111  
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12 @112  struct snd_soc_acpi_mach *snd_soc_acpi_codec_list(void *arg)
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  113  {
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12  114  	struct snd_soc_acpi_mach *mach = arg;
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12  115  	struct snd_soc_acpi_codecs *codec_list =
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12  116  		(struct snd_soc_acpi_codecs *) mach->quirk_data;
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  117  	int i;
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  118  
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  119  	if (mach->quirk_data == NULL)
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  120  		return mach;
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  121  
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  122  	for (i = 0; i < codec_list->num_codecs; i++) {
0d5ea120abc020f sound/soc/soc-acpi.c                    Jeremy Cline         2018-01-05  123  		if (!acpi_dev_present(codec_list->codecs[i], NULL, -1))
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  124  			return NULL;
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  125  	}
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  126  
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  127  	return mach;
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  128  }
7feb2f786a46d34 sound/soc/soc-acpi.c                    Pierre-Louis Bossart 2017-10-12  129  EXPORT_SYMBOL_GPL(snd_soc_acpi_codec_list);
54746dabf770eb2 sound/soc/intel/common/sst-match-acpi.c Naveen M             2017-05-15  130  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

* Re: [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency
  2021-12-16 18:04               ` Pierre-Louis Bossart
@ 2021-12-17  9:06                 ` Cezary Rojewski
  0 siblings, 0 replies; 16+ messages in thread
From: Cezary Rojewski @ 2021-12-17  9:06 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel; +Cc: broonie, tiwai

On 2021-12-16 7:04 PM, Pierre-Louis Bossart wrote:

...

>> Hmm.. doesn't it really? Are the glk/cnl/cfl/cml/icl/jsl/tgl/ehl/adl
>> tables "common" for the Intel audio drivers? There are not used by
>> anything except for SOF. It seems reasonable to have them present
>> locally too. SOF solution becomes more cohesively organized in such
>> case, just like catpt is after this patch.
> 
> We could also move the boards/ while we're at it, on the grounds they
> are not all used by all drivers.
> 
> My take is that unless a new feature is added that warrants moving the
> tables around, let's keep the existing code as is.
> 

I agree with the relocation of the boards! In general, improving the 
maintainability is worth it, even if it takes a larger number of patches.

Hmm.. avs-driver related boards are kind of an argument in this 
discussion. I can delay patches 4/5 and 5/5 until they're on the list, 
no problem.


Mark,

Should I re-send the series as 3, or is it fine as is and simply last 
two patches could be omitted?


Regards,
Czarek

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

* Re: (subset) [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple
  2021-12-16 11:57 [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple Cezary Rojewski
                   ` (4 preceding siblings ...)
  2021-12-16 11:57 ` [PATCH 5/5] ASoC: Intel: Drop legacy HSW/BDW board-match information Cezary Rojewski
@ 2021-12-21  2:50 ` Mark Brown
  5 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2021-12-21  2:50 UTC (permalink / raw)
  To: alsa-devel, Cezary Rojewski; +Cc: tiwai

On Thu, 16 Dec 2021 12:57:38 +0100, Cezary Rojewski wrote:
> Set is made of one fix for dma-transfer so that result of
> dmaengine_submit() is tested before moving on, and few cleanups:
> 
> - two non-impactful, where catpt_component_open() layout gets improved
>   slightly as well as relocation of couple of locals found in
>   PCM-functions so that they look more cohesive
> - no need to expose catpt-driver board-matching information globally.
>   Most fields are not by it and it's the sole user of haswell_machines
>   table. By having them locally it is clear what is actually being used
> 
> [...]

Applied to

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

Thanks!

[1/5] ASoC: Intel: catpt: Test dmaengine_submit() result before moving on
      commit: 2a9a72e290d4a4741e673f86b9fba9bfb319786d
[2/5] ASoC: Intel: catpt: Reduce size of catpt_component_open()
      commit: dad492cfd24caf1b62d598555cde279bcca4755e
[3/5] ASoC: Intel: catpt: Streamline locals declaration for PCM-functions
      commit: a62a02986d3990f4b55c2d75610f8fb2074b0870

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] 16+ messages in thread

end of thread, other threads:[~2021-12-21  2:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 11:57 [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple Cezary Rojewski
2021-12-16 11:57 ` [PATCH 1/5] ASoC: Intel: catpt: Test dmaengine_submit() result before moving on Cezary Rojewski
2021-12-16 11:57 ` [PATCH 2/5] ASoC: Intel: catpt: Reduce size of catpt_component_open() Cezary Rojewski
2021-12-16 11:57 ` [PATCH 3/5] ASoC: Intel: catpt: Streamline locals declaration for PCM-functions Cezary Rojewski
2021-12-16 11:57 ` [PATCH 4/5] ASoC: Intel: catpt: Drop SND_SOC_ACPI_INTEL_MATCH dependency Cezary Rojewski
2021-12-16 14:11   ` Pierre-Louis Bossart
2021-12-16 14:37     ` Cezary Rojewski
2021-12-16 15:13       ` Pierre-Louis Bossart
2021-12-16 15:59         ` Cezary Rojewski
2021-12-16 16:29           ` Pierre-Louis Bossart
2021-12-16 16:50             ` Cezary Rojewski
2021-12-16 18:04               ` Pierre-Louis Bossart
2021-12-17  9:06                 ` Cezary Rojewski
2021-12-17  2:42   ` kernel test robot
2021-12-16 11:57 ` [PATCH 5/5] ASoC: Intel: Drop legacy HSW/BDW board-match information Cezary Rojewski
2021-12-21  2:50 ` (subset) [PATCH 0/5] ASoC: Intel: catpt: Dma-transfer fix and couple 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.