All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] ALSA: hda: New NHLT functions and cleanup
@ 2021-10-15 16:40 Cezary Rojewski
  2021-10-15 16:40 ` [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions Cezary Rojewski
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Cezary Rojewski @ 2021-10-15 16:40 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie

Changes add two crucial functions: endpoint presence-check and
retrieval of endpoint's BLOB (hardware configuration) to NHLT API.

Few cleanups accompany the above:
First, drop device pointer usage in NHLT interface as those function
don't really make use of it. Work is done to align NHLT-struct naming
with other, commonly used ACPI-structs. While cleaning up, don't forget
about "is DMIC in NHLT?" check. No need to check for channel count or
anything DMIC-configuration related, just straight up verify link_type
presence.

Amadeusz Sławiński (5):
  ALSA: hda: Drop device-argument in NHLT functions
  ALSA: hda: Follow ACPI convention in NHLT struct naming
  ALSA: hda: Fill gaps in NHLT endpoint-interface
  ALSA: hda: Simplify DMIC-in-NHLT check
  ASoC: Intel: Skylake: Use NHLT API to search for blob

 include/sound/intel-nhlt.h             |  37 +++++--
 sound/hda/intel-dsp-config.c           |   6 +-
 sound/hda/intel-nhlt.c                 | 127 ++++++++++++++++++++++---
 sound/soc/intel/skylake/skl-nhlt.c     | 108 +--------------------
 sound/soc/intel/skylake/skl-pcm.c      |   3 +
 sound/soc/intel/skylake/skl-topology.c |  27 +++---
 sound/soc/intel/skylake/skl-topology.h |   1 +
 sound/soc/intel/skylake/skl.c          |   5 +-
 sound/soc/intel/skylake/skl.h          |   6 +-
 sound/soc/sof/intel/hda.c              |   6 +-
 10 files changed, 173 insertions(+), 153 deletions(-)

-- 
2.25.1


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

* [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions
  2021-10-15 16:40 [PATCH 0/5] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
@ 2021-10-15 16:40 ` Cezary Rojewski
  2021-10-15 16:42   ` Pierre-Louis Bossart
  2021-10-15 16:40 ` [PATCH 2/5] ALSA: hda: Follow ACPI convention in NHLT struct naming Cezary Rojewski
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Cezary Rojewski @ 2021-10-15 16:40 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

ACPI is device independent, so printing warnings using device functions
is misleading. Replace dev_xxx() with pr_xxx() and remove now
unnecessary argument.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/intel-nhlt.h    |  9 ++++-----
 sound/hda/intel-dsp-config.c  |  4 ++--
 sound/hda/intel-nhlt.c        | 24 +++++++++++++-----------
 sound/soc/intel/skylake/skl.c |  5 ++---
 sound/soc/sof/intel/hda.c     |  4 ++--
 5 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/sound/intel-nhlt.h b/include/sound/intel-nhlt.h
index d0574805865f..4debab7c1996 100644
--- a/include/sound/intel-nhlt.h
+++ b/include/sound/intel-nhlt.h
@@ -126,17 +126,17 @@ enum {
 	NHLT_MIC_ARRAY_VENDOR_DEFINED = 0xf,
 };
 
-struct nhlt_acpi_table *intel_nhlt_init(struct device *dev);
+struct nhlt_acpi_table *intel_nhlt_init(void);
 
 void intel_nhlt_free(struct nhlt_acpi_table *addr);
 
-int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt);
+int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt);
 
 #else
 
 struct nhlt_acpi_table;
 
-static inline struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
+static inline struct nhlt_acpi_table *intel_nhlt_init(void)
 {
 	return NULL;
 }
@@ -145,8 +145,7 @@ static inline void intel_nhlt_free(struct nhlt_acpi_table *addr)
 {
 }
 
-static inline int intel_nhlt_get_dmic_geo(struct device *dev,
-					  struct nhlt_acpi_table *nhlt)
+static inline int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt)
 {
 	return 0;
 }
diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index b9ac9e9e45a4..60cc4735c6ec 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -382,9 +382,9 @@ static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
 	struct nhlt_acpi_table *nhlt;
 	int ret = 0;
 
-	nhlt = intel_nhlt_init(&pci->dev);
+	nhlt = intel_nhlt_init();
 	if (nhlt) {
-		if (intel_nhlt_get_dmic_geo(&pci->dev, nhlt))
+		if (intel_nhlt_get_dmic_geo(nhlt))
 			ret = 1;
 		intel_nhlt_free(nhlt);
 	}
diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index e2237239d922..195d9e193a6c 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -1,10 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0-only
 // Copyright (c) 2015-2019 Intel Corporation
 
+#define pr_fmt(fmt)     "NHLT: " fmt
+
 #include <linux/acpi.h>
 #include <sound/intel-nhlt.h>
 
-struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
+struct nhlt_acpi_table *intel_nhlt_init(void)
 {
 	struct nhlt_acpi_table *nhlt;
 	acpi_status status;
@@ -12,7 +14,7 @@ struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
 	status = acpi_get_table(ACPI_SIG_NHLT, 0,
 				(struct acpi_table_header **)&nhlt);
 	if (ACPI_FAILURE(status)) {
-		dev_warn(dev, "NHLT table not found\n");
+		pr_warn("NHLT table not found\n");
 		return NULL;
 	}
 
@@ -26,7 +28,7 @@ void intel_nhlt_free(struct nhlt_acpi_table *nhlt)
 }
 EXPORT_SYMBOL_GPL(intel_nhlt_free);
 
-int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
+int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt)
 {
 	struct nhlt_endpoint *epnt;
 	struct nhlt_dmic_array_config *cfg;
@@ -40,7 +42,7 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
 		return 0;
 
 	if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
-		dev_warn(dev, "Invalid DMIC description table\n");
+		pr_warn("Invalid DMIC description table\n");
 		return 0;
 	}
 
@@ -55,7 +57,7 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
 
 		/* find max number of channels based on format_configuration */
 		if (fmt_configs->fmt_count) {
-			dev_dbg(dev, "%s: found %d format definitions\n",
+			pr_debug("%s: found %d format definitions\n",
 				__func__, fmt_configs->fmt_count);
 
 			for (i = 0; i < fmt_configs->fmt_count; i++) {
@@ -66,9 +68,9 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
 				if (fmt_ext->fmt.channels > max_ch)
 					max_ch = fmt_ext->fmt.channels;
 			}
-			dev_dbg(dev, "%s: max channels found %d\n", __func__, max_ch);
+			pr_debug("%s: max channels found %d\n", __func__, max_ch);
 		} else {
-			dev_dbg(dev, "%s: No format information found\n", __func__);
+			pr_debug("%s: No format information found\n", __func__);
 		}
 
 		if (cfg->device_config.config_type != NHLT_CONFIG_TYPE_MIC_ARRAY) {
@@ -90,21 +92,21 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
 				dmic_geo = cfg_vendor->nb_mics;
 				break;
 			default:
-				dev_warn(dev, "%s: undefined DMIC array_type 0x%0x\n",
+				pr_warn("%s: undefined DMIC array_type 0x%0x\n",
 					 __func__, cfg->array_type);
 			}
 
 			if (dmic_geo > 0) {
-				dev_dbg(dev, "%s: Array with %d dmics\n", __func__, dmic_geo);
+				pr_debug("%s: Array with %d dmics\n", __func__, dmic_geo);
 			}
 			if (max_ch > dmic_geo) {
-				dev_dbg(dev, "%s: max channels %d exceed dmic number %d\n",
+				pr_debug("%s: max channels %d exceed dmic number %d\n",
 					__func__, max_ch, dmic_geo);
 			}
 		}
 	}
 
-	dev_dbg(dev, "%s: dmic number %d max_ch %d\n",
+	pr_debug("%s: dmic number %d max_ch %d\n",
 		__func__, dmic_geo, max_ch);
 
 	return dmic_geo;
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 5b1a15e39912..4f122616b636 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -517,8 +517,7 @@ static int skl_find_machine(struct skl_dev *skl, void *driver_data)
 	if (pdata) {
 		skl->use_tplg_pcm = pdata->use_tplg_pcm;
 		mach->mach_params.dmic_num =
-			intel_nhlt_get_dmic_geo(&skl->pci->dev,
-						skl->nhlt);
+			intel_nhlt_get_dmic_geo(skl->nhlt);
 	}
 
 	return 0;
@@ -1009,7 +1008,7 @@ static int skl_probe(struct pci_dev *pci,
 
 	device_disable_async_suspend(bus->dev);
 
-	skl->nhlt = intel_nhlt_init(bus->dev);
+	skl->nhlt = intel_nhlt_init();
 
 	if (skl->nhlt == NULL) {
 #if !IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 883d78dd01b5..75c2ee91bf13 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -651,9 +651,9 @@ static int check_nhlt_dmic(struct snd_sof_dev *sdev)
 	struct nhlt_acpi_table *nhlt;
 	int dmic_num;
 
-	nhlt = intel_nhlt_init(sdev->dev);
+	nhlt = intel_nhlt_init();
 	if (nhlt) {
-		dmic_num = intel_nhlt_get_dmic_geo(sdev->dev, nhlt);
+		dmic_num = intel_nhlt_get_dmic_geo(nhlt);
 		intel_nhlt_free(nhlt);
 		if (dmic_num >= 1 && dmic_num <= 4)
 			return dmic_num;
-- 
2.25.1


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

* [PATCH 2/5] ALSA: hda: Follow ACPI convention in NHLT struct naming
  2021-10-15 16:40 [PATCH 0/5] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
  2021-10-15 16:40 ` [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions Cezary Rojewski
@ 2021-10-15 16:40 ` Cezary Rojewski
  2021-10-15 16:40 ` [PATCH 3/5] ALSA: hda: Fill gaps in NHLT endpoint-interface Cezary Rojewski
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 20+ messages in thread
From: Cezary Rojewski @ 2021-10-15 16:40 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

All ACPI table structs except for NHLT ones are defined in postfix way.
Update NHLT structs naming to make code cohesive.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/intel-nhlt.h         | 16 ++++++++--------
 sound/hda/intel-dsp-config.c       |  2 +-
 sound/hda/intel-nhlt.c             |  8 ++++----
 sound/soc/intel/skylake/skl-nhlt.c |  8 ++++----
 sound/soc/intel/skylake/skl.h      |  2 +-
 sound/soc/sof/intel/hda.c          |  2 +-
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/sound/intel-nhlt.h b/include/sound/intel-nhlt.h
index 4debab7c1996..b7eec4b3ca01 100644
--- a/include/sound/intel-nhlt.h
+++ b/include/sound/intel-nhlt.h
@@ -77,7 +77,7 @@ struct nhlt_endpoint {
 	struct nhlt_specific_cfg config;
 } __packed;
 
-struct nhlt_acpi_table {
+struct acpi_table_nhlt {
 	struct acpi_table_header header;
 	u8 endpoint_count;
 	struct nhlt_endpoint desc[];
@@ -126,26 +126,26 @@ enum {
 	NHLT_MIC_ARRAY_VENDOR_DEFINED = 0xf,
 };
 
-struct nhlt_acpi_table *intel_nhlt_init(void);
+struct acpi_table_nhlt *intel_nhlt_init(void);
 
-void intel_nhlt_free(struct nhlt_acpi_table *addr);
+void intel_nhlt_free(struct acpi_table_nhlt *addr);
 
-int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt);
+int intel_nhlt_get_dmic_geo(struct acpi_table_nhlt *nhlt);
 
 #else
 
-struct nhlt_acpi_table;
+struct acpi_table_nhlt;
 
-static inline struct nhlt_acpi_table *intel_nhlt_init(void)
+static inline struct acpi_table_nhlt *intel_nhlt_init(void)
 {
 	return NULL;
 }
 
-static inline void intel_nhlt_free(struct nhlt_acpi_table *addr)
+static inline void intel_nhlt_free(struct acpi_table_nhlt *addr)
 {
 }
 
-static inline int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt)
+static inline int intel_nhlt_get_dmic_geo(struct acpi_table_nhlt *nhlt)
 {
 	return 0;
 }
diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 60cc4735c6ec..4c9d5bc39cbd 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -379,7 +379,7 @@ static const struct config_entry *snd_intel_dsp_find_config
 
 static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
 {
-	struct nhlt_acpi_table *nhlt;
+	struct acpi_table_nhlt *nhlt;
 	int ret = 0;
 
 	nhlt = intel_nhlt_init();
diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index 195d9e193a6c..c41d20e1b349 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -6,9 +6,9 @@
 #include <linux/acpi.h>
 #include <sound/intel-nhlt.h>
 
-struct nhlt_acpi_table *intel_nhlt_init(void)
+struct acpi_table_nhlt *intel_nhlt_init(void)
 {
-	struct nhlt_acpi_table *nhlt;
+	struct acpi_table_nhlt *nhlt;
 	acpi_status status;
 
 	status = acpi_get_table(ACPI_SIG_NHLT, 0,
@@ -22,13 +22,13 @@ struct nhlt_acpi_table *intel_nhlt_init(void)
 }
 EXPORT_SYMBOL_GPL(intel_nhlt_init);
 
-void intel_nhlt_free(struct nhlt_acpi_table *nhlt)
+void intel_nhlt_free(struct acpi_table_nhlt *nhlt)
 {
 	acpi_put_table((struct acpi_table_header *)nhlt);
 }
 EXPORT_SYMBOL_GPL(intel_nhlt_free);
 
-int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt)
+int intel_nhlt_get_dmic_geo(struct acpi_table_nhlt *nhlt)
 {
 	struct nhlt_endpoint *epnt;
 	struct nhlt_dmic_array_config *cfg;
diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index 64226072f0ee..3033c1bf279f 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -88,7 +88,7 @@ struct nhlt_specific_cfg
 	struct hdac_bus *bus = skl_to_bus(skl);
 	struct device *dev = bus->dev;
 	struct nhlt_specific_cfg *sp_config;
-	struct nhlt_acpi_table *nhlt = skl->nhlt;
+	struct acpi_table_nhlt *nhlt = skl->nhlt;
 	u16 bps = (s_fmt == 16) ? 16 : 32;
 	u8 j;
 
@@ -132,7 +132,7 @@ static void skl_nhlt_trim_space(char *trim)
 
 int skl_nhlt_update_topology_bin(struct skl_dev *skl)
 {
-	struct nhlt_acpi_table *nhlt = (struct nhlt_acpi_table *)skl->nhlt;
+	struct acpi_table_nhlt *nhlt = (struct acpi_table_nhlt *)skl->nhlt;
 	struct hdac_bus *bus = skl_to_bus(skl);
 	struct device *dev = bus->dev;
 
@@ -155,7 +155,7 @@ static ssize_t platform_id_show(struct device *dev,
 	struct pci_dev *pci = to_pci_dev(dev);
 	struct hdac_bus *bus = pci_get_drvdata(pci);
 	struct skl_dev *skl = bus_to_skl(bus);
-	struct nhlt_acpi_table *nhlt = (struct nhlt_acpi_table *)skl->nhlt;
+	struct acpi_table_nhlt *nhlt = (struct acpi_table_nhlt *)skl->nhlt;
 	char platform_id[32];
 
 	sprintf(platform_id, "%x-%.6s-%.8s-%d", skl->pci_id,
@@ -335,7 +335,7 @@ static void skl_get_mclk(struct skl_dev *skl, struct skl_ssp_clk *mclk,
 
 void skl_get_clks(struct skl_dev *skl, struct skl_ssp_clk *ssp_clks)
 {
-	struct nhlt_acpi_table *nhlt = (struct nhlt_acpi_table *)skl->nhlt;
+	struct acpi_table_nhlt *nhlt = (struct acpi_table_nhlt *)skl->nhlt;
 	struct nhlt_endpoint *epnt;
 	struct nhlt_fmt *fmt;
 	int i;
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index 33ed274fc0cb..37195aafbf27 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -67,7 +67,7 @@ struct skl_dev {
 	struct snd_soc_component *component;
 	struct snd_soc_dai_driver *dais;
 
-	struct nhlt_acpi_table *nhlt; /* nhlt ptr */
+	struct acpi_table_nhlt *nhlt; /* nhlt ptr */
 
 	struct list_head ppl_list;
 	struct list_head bind_list;
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 75c2ee91bf13..4a8907fe7f94 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -648,7 +648,7 @@ static int hda_init(struct snd_sof_dev *sdev)
 
 static int check_nhlt_dmic(struct snd_sof_dev *sdev)
 {
-	struct nhlt_acpi_table *nhlt;
+	struct acpi_table_nhlt *nhlt;
 	int dmic_num;
 
 	nhlt = intel_nhlt_init();
-- 
2.25.1


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

* [PATCH 3/5] ALSA: hda: Fill gaps in NHLT endpoint-interface
  2021-10-15 16:40 [PATCH 0/5] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
  2021-10-15 16:40 ` [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions Cezary Rojewski
  2021-10-15 16:40 ` [PATCH 2/5] ALSA: hda: Follow ACPI convention in NHLT struct naming Cezary Rojewski
@ 2021-10-15 16:40 ` Cezary Rojewski
  2021-10-16  3:21     ` kernel test robot
  2021-10-15 16:40 ` [PATCH 4/5] ALSA: hda: Simplify DMIC-in-NHLT check Cezary Rojewski
  2021-10-15 16:40 ` [PATCH 5/5] ASoC: Intel: Skylake: Use NHLT API to search for blob Cezary Rojewski
  4 siblings, 1 reply; 20+ messages in thread
From: Cezary Rojewski @ 2021-10-15 16:40 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

Two key operations missings are: endpoint presence-check and retrieval
of matching endpoint hardware configuration (blob). Add operations for
both use cases.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/intel-nhlt.h | 20 ++++++++
 sound/hda/intel-nhlt.c     | 99 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/include/sound/intel-nhlt.h b/include/sound/intel-nhlt.h
index b7eec4b3ca01..2cd3f27b7a72 100644
--- a/include/sound/intel-nhlt.h
+++ b/include/sound/intel-nhlt.h
@@ -132,6 +132,12 @@ void intel_nhlt_free(struct acpi_table_nhlt *addr);
 
 int intel_nhlt_get_dmic_geo(struct acpi_table_nhlt *nhlt);
 
+bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type);
+struct nhlt_specific_cfg *
+intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
+			     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
+			     u8 num_ch, u32 rate, u8 dir, u8 dev_type);
+
 #else
 
 struct acpi_table_nhlt;
@@ -149,6 +155,20 @@ static inline int intel_nhlt_get_dmic_geo(struct acpi_table_nhlt *nhlt)
 {
 	return 0;
 }
+
+bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
+{
+	return false;
+}
+
+struct nhlt_specific_cfg *
+intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
+			     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
+			     u8 num_ch, u32 rate, u8 dir, u8 dev_type)
+{
+	return NULL;
+}
+
 #endif
 
 #endif
diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index c41d20e1b349..d97fe57a7ef2 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -112,3 +112,102 @@ int intel_nhlt_get_dmic_geo(struct acpi_table_nhlt *nhlt)
 	return dmic_geo;
 }
 EXPORT_SYMBOL_GPL(intel_nhlt_get_dmic_geo);
+
+bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
+{
+	struct nhlt_endpoint *epnt;
+	int i;
+
+	if (!nhlt)
+		return false;
+
+	epnt = (struct nhlt_endpoint *)nhlt->desc;
+	for (i = 0; i < nhlt->endpoint_count; i++) {
+		if (epnt->linktype == link_type)
+			return true;
+
+		epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length);
+	}
+	return false;
+}
+EXPORT_SYMBOL(intel_nhlt_has_endpoint_type);
+
+static struct nhlt_specific_cfg *
+nhlt_get_specific_cfg(struct nhlt_fmt *fmt, u8 num_ch, u32 rate, u8 vbps, u8 bps)
+{
+	struct nhlt_fmt_cfg *cfg = fmt->fmt_config;
+	struct wav_fmt *wfmt;
+	u16 _bps, _vbps;
+	int i;
+
+	pr_debug("Endpoint format count=%d\n", fmt->fmt_count);
+
+	for (i = 0; i < fmt->fmt_count; i++) {
+		wfmt = &cfg->fmt_ext.fmt;
+		_bps = wfmt->bits_per_sample;
+		_vbps = cfg->fmt_ext.sample.valid_bits_per_sample;
+
+		pr_debug("Endpoint format: ch=%d fmt=%d/%d rate=%d\n",
+			 wfmt->channels, _vbps, _bps, wfmt->samples_per_sec);
+
+		if (wfmt->channels == num_ch && wfmt->samples_per_sec == rate &&
+		    vbps == _vbps && bps == _bps)
+			return &cfg->config;
+
+		cfg = (struct nhlt_fmt_cfg *)(cfg->config.caps + cfg->config.size);
+	}
+
+	return NULL;
+}
+
+static bool nhlt_check_ep_match(struct nhlt_endpoint *epnt,
+				u32 bus_id, u8 link_type, u8 dir, u8 dev_type)
+{
+	pr_debug("Endpoint: vbus_id=%d link_type=%d dir=%d dev_type = %d\n",
+		 epnt->virtual_bus_id, epnt->linktype,
+		 epnt->direction, epnt->device_type);
+
+	if ((epnt->virtual_bus_id != bus_id) ||
+	    (epnt->linktype != link_type) ||
+	    (epnt->direction != dir))
+		return false;
+
+	/* link of type DMIC bypasses device_type check */
+	return epnt->linktype == NHLT_LINK_DMIC ||
+	       epnt->device_type == dev_type;
+}
+
+struct nhlt_specific_cfg *
+intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
+			     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
+			     u8 num_ch, u32 rate, u8 dir, u8 dev_type)
+{
+	struct nhlt_specific_cfg *cfg;
+	struct nhlt_endpoint *epnt;
+	struct nhlt_fmt *fmt;
+	int i;
+
+	if (!nhlt)
+		return NULL;
+
+	pr_debug("Looking for configuration:\n");
+	pr_debug("  vbus_id=%d link_type=%d dir=%d, dev_type=%d\n",
+		 bus_id, link_type, dir, dev_type);
+	pr_debug("  ch=%d fmt=%d/%d rate=%d\n", num_ch, vbps, bps, rate);
+	pr_debug("Endpoint count=%d\n", nhlt->endpoint_count);
+
+	epnt = (struct nhlt_endpoint *)nhlt->desc;
+	for (i = 0; i < nhlt->endpoint_count; i++) {
+		if (nhlt_check_ep_match(epnt, bus_id, link_type, dir, dev_type)) {
+			fmt = (struct nhlt_fmt *)(epnt->config.caps + epnt->config.size);
+			cfg = nhlt_get_specific_cfg(fmt, num_ch, rate, vbps, bps);
+			if (cfg)
+				return cfg;
+		}
+
+		epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length);
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL(intel_nhlt_get_endpoint_blob);
-- 
2.25.1


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

* [PATCH 4/5] ALSA: hda: Simplify DMIC-in-NHLT check
  2021-10-15 16:40 [PATCH 0/5] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
                   ` (2 preceding siblings ...)
  2021-10-15 16:40 ` [PATCH 3/5] ALSA: hda: Fill gaps in NHLT endpoint-interface Cezary Rojewski
@ 2021-10-15 16:40 ` Cezary Rojewski
  2021-10-16  5:09     ` kernel test robot
  2021-10-15 16:40 ` [PATCH 5/5] ASoC: Intel: Skylake: Use NHLT API to search for blob Cezary Rojewski
  4 siblings, 1 reply; 20+ messages in thread
From: Cezary Rojewski @ 2021-10-15 16:40 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

Only DMIC endpoint presence is relevant, not its configuration.

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

diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
index 4c9d5bc39cbd..e8fc5557436b 100644
--- a/sound/hda/intel-dsp-config.c
+++ b/sound/hda/intel-dsp-config.c
@@ -384,7 +384,7 @@ static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
 
 	nhlt = intel_nhlt_init();
 	if (nhlt) {
-		if (intel_nhlt_get_dmic_geo(nhlt))
+		if (intel_nhlt_has_endpoint_type(nhlt, NHLT_LINK_DMIC))
 			ret = 1;
 		intel_nhlt_free(nhlt);
 	}
-- 
2.25.1


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

* [PATCH 5/5] ASoC: Intel: Skylake: Use NHLT API to search for blob
  2021-10-15 16:40 [PATCH 0/5] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
                   ` (3 preceding siblings ...)
  2021-10-15 16:40 ` [PATCH 4/5] ALSA: hda: Simplify DMIC-in-NHLT check Cezary Rojewski
@ 2021-10-15 16:40 ` Cezary Rojewski
  2021-10-15 16:42   ` Mark Brown
  4 siblings, 1 reply; 20+ messages in thread
From: Cezary Rojewski @ 2021-10-15 16:40 UTC (permalink / raw)
  To: alsa-devel
  Cc: pierre-louis.bossart, Cezary Rojewski, tiwai, hdegoede, broonie,
	Amadeusz Sławiński

From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>

With NHLT enriched with new search functions, remove local code in
favour of them. This also fixes broken behaviour: search should be based
on significant bits count rather than container size.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/skylake/skl-nhlt.c     | 102 -------------------------
 sound/soc/intel/skylake/skl-pcm.c      |   3 +
 sound/soc/intel/skylake/skl-topology.c |  27 ++++---
 sound/soc/intel/skylake/skl-topology.h |   1 +
 sound/soc/intel/skylake/skl.h          |   4 -
 5 files changed, 19 insertions(+), 118 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index 3033c1bf279f..16f70554b2a0 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -13,108 +13,6 @@
 #include "skl.h"
 #include "skl-i2s.h"
 
-static struct nhlt_specific_cfg *skl_get_specific_cfg(
-		struct device *dev, struct nhlt_fmt *fmt,
-		u8 no_ch, u32 rate, u16 bps, u8 linktype)
-{
-	struct nhlt_specific_cfg *sp_config;
-	struct wav_fmt *wfmt;
-	struct nhlt_fmt_cfg *fmt_config = fmt->fmt_config;
-	int i;
-
-	dev_dbg(dev, "Format count =%d\n", fmt->fmt_count);
-
-	for (i = 0; i < fmt->fmt_count; i++) {
-		wfmt = &fmt_config->fmt_ext.fmt;
-		dev_dbg(dev, "ch=%d fmt=%d s_rate=%d\n", wfmt->channels,
-			 wfmt->bits_per_sample, wfmt->samples_per_sec);
-		if (wfmt->channels == no_ch && wfmt->bits_per_sample == bps) {
-			/*
-			 * if link type is dmic ignore rate check as the blob is
-			 * generic for all rates
-			 */
-			sp_config = &fmt_config->config;
-			if (linktype == NHLT_LINK_DMIC)
-				return sp_config;
-
-			if (wfmt->samples_per_sec == rate)
-				return sp_config;
-		}
-
-		fmt_config = (struct nhlt_fmt_cfg *)(fmt_config->config.caps +
-						fmt_config->config.size);
-	}
-
-	return NULL;
-}
-
-static void dump_config(struct device *dev, u32 instance_id, u8 linktype,
-		u8 s_fmt, u8 num_channels, u32 s_rate, u8 dirn, u16 bps)
-{
-	dev_dbg(dev, "Input configuration\n");
-	dev_dbg(dev, "ch=%d fmt=%d s_rate=%d\n", num_channels, s_fmt, s_rate);
-	dev_dbg(dev, "vbus_id=%d link_type=%d\n", instance_id, linktype);
-	dev_dbg(dev, "bits_per_sample=%d\n", bps);
-}
-
-static bool skl_check_ep_match(struct device *dev, struct nhlt_endpoint *epnt,
-		u32 instance_id, u8 link_type, u8 dirn, u8 dev_type)
-{
-	dev_dbg(dev, "vbus_id=%d link_type=%d dir=%d dev_type = %d\n",
-			epnt->virtual_bus_id, epnt->linktype,
-			epnt->direction, epnt->device_type);
-
-	if ((epnt->virtual_bus_id == instance_id) &&
-			(epnt->linktype == link_type) &&
-			(epnt->direction == dirn)) {
-		/* do not check dev_type for DMIC link type */
-		if (epnt->linktype == NHLT_LINK_DMIC)
-			return true;
-
-		if (epnt->device_type == dev_type)
-			return true;
-	}
-
-	return false;
-}
-
-struct nhlt_specific_cfg
-*skl_get_ep_blob(struct skl_dev *skl, u32 instance, u8 link_type,
-			u8 s_fmt, u8 num_ch, u32 s_rate,
-			u8 dirn, u8 dev_type)
-{
-	struct nhlt_fmt *fmt;
-	struct nhlt_endpoint *epnt;
-	struct hdac_bus *bus = skl_to_bus(skl);
-	struct device *dev = bus->dev;
-	struct nhlt_specific_cfg *sp_config;
-	struct acpi_table_nhlt *nhlt = skl->nhlt;
-	u16 bps = (s_fmt == 16) ? 16 : 32;
-	u8 j;
-
-	dump_config(dev, instance, link_type, s_fmt, num_ch, s_rate, dirn, bps);
-
-	epnt = (struct nhlt_endpoint *)nhlt->desc;
-
-	dev_dbg(dev, "endpoint count =%d\n", nhlt->endpoint_count);
-
-	for (j = 0; j < nhlt->endpoint_count; j++) {
-		if (skl_check_ep_match(dev, epnt, instance, link_type,
-						dirn, dev_type)) {
-			fmt = (struct nhlt_fmt *)(epnt->config.caps +
-						 epnt->config.size);
-			sp_config = skl_get_specific_cfg(dev, fmt, num_ch,
-							s_rate, bps, link_type);
-			if (sp_config)
-				return sp_config;
-		}
-
-		epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length);
-	}
-
-	return NULL;
-}
-
 static void skl_nhlt_trim_space(char *trim)
 {
 	char *s = trim;
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 9ecaf6a1e847..34908af50046 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -317,6 +317,7 @@ static int skl_pcm_hw_params(struct snd_pcm_substream *substream,
 	dev_dbg(dai->dev, "dma_id=%d\n", dma_id);
 
 	p_params.s_fmt = snd_pcm_format_width(params_format(params));
+	p_params.s_cont = snd_pcm_format_physical_width(params_format(params));
 	p_params.ch = params_channels(params);
 	p_params.s_freq = params_rate(params);
 	p_params.host_dma_id = dma_id;
@@ -405,6 +406,7 @@ static int skl_be_hw_params(struct snd_pcm_substream *substream,
 	struct skl_pipe_params p_params = {0};
 
 	p_params.s_fmt = snd_pcm_format_width(params_format(params));
+	p_params.s_cont = snd_pcm_format_physical_width(params_format(params));
 	p_params.ch = params_channels(params);
 	p_params.s_freq = params_rate(params);
 	p_params.stream = substream->stream;
@@ -569,6 +571,7 @@ static int skl_link_hw_params(struct snd_pcm_substream *substream,
 		snd_soc_dai_set_tdm_slot(codec_dai, 0, stream_tag, 0, 0);
 
 	p_params.s_fmt = snd_pcm_format_width(params_format(params));
+	p_params.s_cont = snd_pcm_format_physical_width(params_format(params));
 	p_params.ch = params_channels(params);
 	p_params.s_freq = params_rate(params);
 	p_params.stream = substream->stream;
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 89e4231304dd..4a92f046925d 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -285,7 +285,7 @@ static int skl_tplg_update_be_blob(struct snd_soc_dapm_widget *w,
 {
 	struct skl_module_cfg *m_cfg = w->priv;
 	int link_type, dir;
-	u32 ch, s_freq, s_fmt;
+	u32 ch, s_freq, s_fmt, s_cont;
 	struct nhlt_specific_cfg *cfg;
 	u8 dev_type = skl_tplg_be_dev_type(m_cfg->dev_type);
 	int fmt_idx = m_cfg->fmt_idx;
@@ -301,7 +301,8 @@ static int skl_tplg_update_be_blob(struct snd_soc_dapm_widget *w,
 		link_type = NHLT_LINK_DMIC;
 		dir = SNDRV_PCM_STREAM_CAPTURE;
 		s_freq = m_iface->inputs[0].fmt.s_freq;
-		s_fmt = m_iface->inputs[0].fmt.bit_depth;
+		s_fmt = m_iface->inputs[0].fmt.valid_bit_depth;
+		s_cont = m_iface->inputs[0].fmt.bit_depth;
 		ch = m_iface->inputs[0].fmt.channels;
 		break;
 
@@ -310,12 +311,14 @@ static int skl_tplg_update_be_blob(struct snd_soc_dapm_widget *w,
 		if (m_cfg->hw_conn_type == SKL_CONN_SOURCE) {
 			dir = SNDRV_PCM_STREAM_PLAYBACK;
 			s_freq = m_iface->outputs[0].fmt.s_freq;
-			s_fmt = m_iface->outputs[0].fmt.bit_depth;
+			s_fmt = m_iface->outputs[0].fmt.valid_bit_depth;
+			s_cont = m_iface->outputs[0].fmt.bit_depth;
 			ch = m_iface->outputs[0].fmt.channels;
 		} else {
 			dir = SNDRV_PCM_STREAM_CAPTURE;
 			s_freq = m_iface->inputs[0].fmt.s_freq;
-			s_fmt = m_iface->inputs[0].fmt.bit_depth;
+			s_fmt = m_iface->inputs[0].fmt.valid_bit_depth;
+			s_cont = m_iface->inputs[0].fmt.bit_depth;
 			ch = m_iface->inputs[0].fmt.channels;
 		}
 		break;
@@ -325,16 +328,16 @@ static int skl_tplg_update_be_blob(struct snd_soc_dapm_widget *w,
 	}
 
 	/* update the blob based on virtual bus_id and default params */
-	cfg = skl_get_ep_blob(skl, m_cfg->vbus_id, link_type,
-					s_fmt, ch, s_freq, dir, dev_type);
+	cfg = intel_nhlt_get_endpoint_blob(skl->nhlt, m_cfg->vbus_id, link_type,
+					s_fmt, s_cont, ch, s_freq, dir, dev_type);
 	if (cfg) {
 		m_cfg->formats_config[SKL_PARAM_INIT].caps_size = cfg->size;
 		m_cfg->formats_config[SKL_PARAM_INIT].caps = (u32 *)&cfg->caps;
 	} else {
 		dev_err(skl->dev, "Blob NULL for id %x type %d dirn %d\n",
 					m_cfg->vbus_id, link_type, dir);
-		dev_err(skl->dev, "PCM: ch %d, freq %d, fmt %d\n",
-					ch, s_freq, s_fmt);
+		dev_err(skl->dev, "PCM: ch %d, freq %d, fmt %d/%d\n",
+					ch, s_freq, s_fmt, s_cont);
 		return -EIO;
 	}
 
@@ -1849,10 +1852,10 @@ static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai,
 		pipe_fmt = &pipe->configs[pipe->pipe_config_idx].in_fmt;
 
 	/* update the blob based on virtual bus_id*/
-	cfg = skl_get_ep_blob(skl, mconfig->vbus_id, link_type,
-					pipe_fmt->bps, pipe_fmt->channels,
-					pipe_fmt->freq, pipe->direction,
-					dev_type);
+	cfg = intel_nhlt_get_endpoint_blob(skl->nhlt, mconfig->vbus_id, link_type,
+					pipe_fmt->bps, params->s_cont,
+					pipe_fmt->channels, pipe_fmt->freq,
+					pipe->direction, dev_type);
 	if (cfg) {
 		mconfig->formats_config[SKL_PARAM_INIT].caps_size = cfg->size;
 		mconfig->formats_config[SKL_PARAM_INIT].caps = (u32 *)&cfg->caps;
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index f0695b2ac5dd..22963634fbea 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -284,6 +284,7 @@ struct skl_pipe_params {
 	u32 ch;
 	u32 s_freq;
 	u32 s_fmt;
+	u32 s_cont;
 	u8 linktype;
 	snd_pcm_format_t format;
 	int link_index;
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index 37195aafbf27..3b92dfc23731 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -165,10 +165,6 @@ struct skl_dsp_ops {
 int skl_platform_unregister(struct device *dev);
 int skl_platform_register(struct device *dev);
 
-struct nhlt_specific_cfg *skl_get_ep_blob(struct skl_dev *skl, u32 instance,
-					u8 link_type, u8 s_fmt, u8 num_ch,
-					u32 s_rate, u8 dirn, u8 dev_type);
-
 int skl_nhlt_update_topology_bin(struct skl_dev *skl);
 int skl_init_dsp(struct skl_dev *skl);
 int skl_free_dsp(struct skl_dev *skl);
-- 
2.25.1


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

* Re: [PATCH 5/5] ASoC: Intel: Skylake: Use NHLT API to search for blob
  2021-10-15 16:40 ` [PATCH 5/5] ASoC: Intel: Skylake: Use NHLT API to search for blob Cezary Rojewski
@ 2021-10-15 16:42   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-10-15 16:42 UTC (permalink / raw)
  To: Cezary Rojewski
  Cc: pierre-louis.bossart, alsa-devel, tiwai, hdegoede,
	Amadeusz Sławiński

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

On Fri, Oct 15, 2021 at 06:40:47PM +0200, Cezary Rojewski wrote:
> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> 
> With NHLT enriched with new search functions, remove local code in
> favour of them. This also fixes broken behaviour: search should be based
> on significant bits count rather than container size.

Acked-by: Mark Brown <broonie@kernel.org>

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

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

* Re: [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions
  2021-10-15 16:40 ` [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions Cezary Rojewski
@ 2021-10-15 16:42   ` Pierre-Louis Bossart
  2021-10-17  7:52     ` Takashi Iwai
  0 siblings, 1 reply; 20+ messages in thread
From: Pierre-Louis Bossart @ 2021-10-15 16:42 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel
  Cc: hdegoede, broonie, tiwai, Amadeusz Sławiński



On 10/15/21 11:40 AM, Cezary Rojewski wrote:
> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> 
> ACPI is device independent, so printing warnings using device functions
> is misleading. Replace dev_xxx() with pr_xxx() and remove now
> unnecessary argument.

the routines in sound/hda/intel-nhtl.c are called from a specific PCI
device, why would you remove that information?

This makes no sense to me.

> 
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
>  include/sound/intel-nhlt.h    |  9 ++++-----
>  sound/hda/intel-dsp-config.c  |  4 ++--
>  sound/hda/intel-nhlt.c        | 24 +++++++++++++-----------
>  sound/soc/intel/skylake/skl.c |  5 ++---
>  sound/soc/sof/intel/hda.c     |  4 ++--
>  5 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/include/sound/intel-nhlt.h b/include/sound/intel-nhlt.h
> index d0574805865f..4debab7c1996 100644
> --- a/include/sound/intel-nhlt.h
> +++ b/include/sound/intel-nhlt.h
> @@ -126,17 +126,17 @@ enum {
>  	NHLT_MIC_ARRAY_VENDOR_DEFINED = 0xf,
>  };
>  
> -struct nhlt_acpi_table *intel_nhlt_init(struct device *dev);
> +struct nhlt_acpi_table *intel_nhlt_init(void);
>  
>  void intel_nhlt_free(struct nhlt_acpi_table *addr);
>  
> -int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt);
> +int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt);
>  
>  #else
>  
>  struct nhlt_acpi_table;
>  
> -static inline struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
> +static inline struct nhlt_acpi_table *intel_nhlt_init(void)
>  {
>  	return NULL;
>  }
> @@ -145,8 +145,7 @@ static inline void intel_nhlt_free(struct nhlt_acpi_table *addr)
>  {
>  }
>  
> -static inline int intel_nhlt_get_dmic_geo(struct device *dev,
> -					  struct nhlt_acpi_table *nhlt)
> +static inline int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt)
>  {
>  	return 0;
>  }
> diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
> index b9ac9e9e45a4..60cc4735c6ec 100644
> --- a/sound/hda/intel-dsp-config.c
> +++ b/sound/hda/intel-dsp-config.c
> @@ -382,9 +382,9 @@ static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
>  	struct nhlt_acpi_table *nhlt;
>  	int ret = 0;
>  
> -	nhlt = intel_nhlt_init(&pci->dev);
> +	nhlt = intel_nhlt_init();
>  	if (nhlt) {
> -		if (intel_nhlt_get_dmic_geo(&pci->dev, nhlt))
> +		if (intel_nhlt_get_dmic_geo(nhlt))
>  			ret = 1;
>  		intel_nhlt_free(nhlt);
>  	}
> diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
> index e2237239d922..195d9e193a6c 100644
> --- a/sound/hda/intel-nhlt.c
> +++ b/sound/hda/intel-nhlt.c
> @@ -1,10 +1,12 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  // Copyright (c) 2015-2019 Intel Corporation
>  
> +#define pr_fmt(fmt)     "NHLT: " fmt
> +
>  #include <linux/acpi.h>
>  #include <sound/intel-nhlt.h>
>  
> -struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
> +struct nhlt_acpi_table *intel_nhlt_init(void)
>  {
>  	struct nhlt_acpi_table *nhlt;
>  	acpi_status status;
> @@ -12,7 +14,7 @@ struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
>  	status = acpi_get_table(ACPI_SIG_NHLT, 0,
>  				(struct acpi_table_header **)&nhlt);
>  	if (ACPI_FAILURE(status)) {
> -		dev_warn(dev, "NHLT table not found\n");
> +		pr_warn("NHLT table not found\n");
>  		return NULL;
>  	}
>  
> @@ -26,7 +28,7 @@ void intel_nhlt_free(struct nhlt_acpi_table *nhlt)
>  }
>  EXPORT_SYMBOL_GPL(intel_nhlt_free);
>  
> -int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
> +int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt)
>  {
>  	struct nhlt_endpoint *epnt;
>  	struct nhlt_dmic_array_config *cfg;
> @@ -40,7 +42,7 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
>  		return 0;
>  
>  	if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
> -		dev_warn(dev, "Invalid DMIC description table\n");
> +		pr_warn("Invalid DMIC description table\n");
>  		return 0;
>  	}
>  
> @@ -55,7 +57,7 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
>  
>  		/* find max number of channels based on format_configuration */
>  		if (fmt_configs->fmt_count) {
> -			dev_dbg(dev, "%s: found %d format definitions\n",
> +			pr_debug("%s: found %d format definitions\n",
>  				__func__, fmt_configs->fmt_count);
>  
>  			for (i = 0; i < fmt_configs->fmt_count; i++) {
> @@ -66,9 +68,9 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
>  				if (fmt_ext->fmt.channels > max_ch)
>  					max_ch = fmt_ext->fmt.channels;
>  			}
> -			dev_dbg(dev, "%s: max channels found %d\n", __func__, max_ch);
> +			pr_debug("%s: max channels found %d\n", __func__, max_ch);
>  		} else {
> -			dev_dbg(dev, "%s: No format information found\n", __func__);
> +			pr_debug("%s: No format information found\n", __func__);
>  		}
>  
>  		if (cfg->device_config.config_type != NHLT_CONFIG_TYPE_MIC_ARRAY) {
> @@ -90,21 +92,21 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
>  				dmic_geo = cfg_vendor->nb_mics;
>  				break;
>  			default:
> -				dev_warn(dev, "%s: undefined DMIC array_type 0x%0x\n",
> +				pr_warn("%s: undefined DMIC array_type 0x%0x\n",
>  					 __func__, cfg->array_type);
>  			}
>  
>  			if (dmic_geo > 0) {
> -				dev_dbg(dev, "%s: Array with %d dmics\n", __func__, dmic_geo);
> +				pr_debug("%s: Array with %d dmics\n", __func__, dmic_geo);
>  			}
>  			if (max_ch > dmic_geo) {
> -				dev_dbg(dev, "%s: max channels %d exceed dmic number %d\n",
> +				pr_debug("%s: max channels %d exceed dmic number %d\n",
>  					__func__, max_ch, dmic_geo);
>  			}
>  		}
>  	}
>  
> -	dev_dbg(dev, "%s: dmic number %d max_ch %d\n",
> +	pr_debug("%s: dmic number %d max_ch %d\n",
>  		__func__, dmic_geo, max_ch);
>  
>  	return dmic_geo;
> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> index 5b1a15e39912..4f122616b636 100644
> --- a/sound/soc/intel/skylake/skl.c
> +++ b/sound/soc/intel/skylake/skl.c
> @@ -517,8 +517,7 @@ static int skl_find_machine(struct skl_dev *skl, void *driver_data)
>  	if (pdata) {
>  		skl->use_tplg_pcm = pdata->use_tplg_pcm;
>  		mach->mach_params.dmic_num =
> -			intel_nhlt_get_dmic_geo(&skl->pci->dev,
> -						skl->nhlt);
> +			intel_nhlt_get_dmic_geo(skl->nhlt);
>  	}
>  
>  	return 0;
> @@ -1009,7 +1008,7 @@ static int skl_probe(struct pci_dev *pci,
>  
>  	device_disable_async_suspend(bus->dev);
>  
> -	skl->nhlt = intel_nhlt_init(bus->dev);
> +	skl->nhlt = intel_nhlt_init();
>  
>  	if (skl->nhlt == NULL) {
>  #if !IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
> diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
> index 883d78dd01b5..75c2ee91bf13 100644
> --- a/sound/soc/sof/intel/hda.c
> +++ b/sound/soc/sof/intel/hda.c
> @@ -651,9 +651,9 @@ static int check_nhlt_dmic(struct snd_sof_dev *sdev)
>  	struct nhlt_acpi_table *nhlt;
>  	int dmic_num;
>  
> -	nhlt = intel_nhlt_init(sdev->dev);
> +	nhlt = intel_nhlt_init();
>  	if (nhlt) {
> -		dmic_num = intel_nhlt_get_dmic_geo(sdev->dev, nhlt);
> +		dmic_num = intel_nhlt_get_dmic_geo(nhlt);
>  		intel_nhlt_free(nhlt);
>  		if (dmic_num >= 1 && dmic_num <= 4)
>  			return dmic_num;
> 

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

* Re: [PATCH 3/5] ALSA: hda: Fill gaps in NHLT endpoint-interface
  2021-10-15 16:40 ` [PATCH 3/5] ALSA: hda: Fill gaps in NHLT endpoint-interface Cezary Rojewski
  2021-10-16  3:21     ` kernel test robot
@ 2021-10-16  3:21     ` kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2021-10-16  3:21 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel
  Cc: llvm, kbuild-all, pierre-louis.bossart, Cezary Rojewski, tiwai,
	hdegoede, broonie, Amadeusz Sławiński

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

Hi Cezary,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on broonie-sound/for-next]
[also build test WARNING on tiwai-sound/for-next v5.15-rc5 next-20211015]
[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/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: riscv-randconfig-r042-20211016 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6069a6a5049497a32a50a49661c2f4169078bdba)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/418a9f8fd0b93d717ac19a0fed367c60fe2e4352
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cezary-Rojewski/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
        git checkout 418a9f8fd0b93d717ac19a0fed367c60fe2e4352
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv 

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

All warnings (new ones prefixed by >>):

   In file included from sound/hda/intel-dsp-config.c:13:
>> include/sound/intel-nhlt.h:159:6: warning: no previous prototype for function 'intel_nhlt_has_endpoint_type' [-Wmissing-prototypes]
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
        ^
   include/sound/intel-nhlt.h:159:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
   ^
   static 
>> include/sound/intel-nhlt.h:165:1: warning: no previous prototype for function 'intel_nhlt_get_endpoint_blob' [-Wmissing-prototypes]
   intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
   ^
   include/sound/intel-nhlt.h:164:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct nhlt_specific_cfg *
   ^
   static 
   2 warnings generated.


vim +/intel_nhlt_has_endpoint_type +159 include/sound/intel-nhlt.h

   158	
 > 159	bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
   160	{
   161		return false;
   162	}
   163	
   164	struct nhlt_specific_cfg *
 > 165	intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
   166				     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
   167				     u8 num_ch, u32 rate, u8 dir, u8 dev_type)
   168	{
   169		return NULL;
   170	}
   171	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30564 bytes --]

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

* Re: [PATCH 3/5] ALSA: hda: Fill gaps in NHLT endpoint-interface
@ 2021-10-16  3:21     ` kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2021-10-16  3:21 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel
  Cc: Cezary Rojewski, kbuild-all, llvm, pierre-louis.bossart, tiwai,
	hdegoede, broonie, Amadeusz Sławiński

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

Hi Cezary,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on broonie-sound/for-next]
[also build test WARNING on tiwai-sound/for-next v5.15-rc5 next-20211015]
[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/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: riscv-randconfig-r042-20211016 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6069a6a5049497a32a50a49661c2f4169078bdba)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/418a9f8fd0b93d717ac19a0fed367c60fe2e4352
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cezary-Rojewski/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
        git checkout 418a9f8fd0b93d717ac19a0fed367c60fe2e4352
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv 

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

All warnings (new ones prefixed by >>):

   In file included from sound/hda/intel-dsp-config.c:13:
>> include/sound/intel-nhlt.h:159:6: warning: no previous prototype for function 'intel_nhlt_has_endpoint_type' [-Wmissing-prototypes]
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
        ^
   include/sound/intel-nhlt.h:159:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
   ^
   static 
>> include/sound/intel-nhlt.h:165:1: warning: no previous prototype for function 'intel_nhlt_get_endpoint_blob' [-Wmissing-prototypes]
   intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
   ^
   include/sound/intel-nhlt.h:164:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct nhlt_specific_cfg *
   ^
   static 
   2 warnings generated.


vim +/intel_nhlt_has_endpoint_type +159 include/sound/intel-nhlt.h

   158	
 > 159	bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
   160	{
   161		return false;
   162	}
   163	
   164	struct nhlt_specific_cfg *
 > 165	intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
   166				     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
   167				     u8 num_ch, u32 rate, u8 dir, u8 dev_type)
   168	{
   169		return NULL;
   170	}
   171	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30564 bytes --]

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

* Re: [PATCH 3/5] ALSA: hda: Fill gaps in NHLT endpoint-interface
@ 2021-10-16  3:21     ` kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2021-10-16  3:21 UTC (permalink / raw)
  To: kbuild-all

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

Hi Cezary,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on broonie-sound/for-next]
[also build test WARNING on tiwai-sound/for-next v5.15-rc5 next-20211015]
[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/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: riscv-randconfig-r042-20211016 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6069a6a5049497a32a50a49661c2f4169078bdba)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/418a9f8fd0b93d717ac19a0fed367c60fe2e4352
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cezary-Rojewski/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
        git checkout 418a9f8fd0b93d717ac19a0fed367c60fe2e4352
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv 

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

All warnings (new ones prefixed by >>):

   In file included from sound/hda/intel-dsp-config.c:13:
>> include/sound/intel-nhlt.h:159:6: warning: no previous prototype for function 'intel_nhlt_has_endpoint_type' [-Wmissing-prototypes]
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
        ^
   include/sound/intel-nhlt.h:159:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
   ^
   static 
>> include/sound/intel-nhlt.h:165:1: warning: no previous prototype for function 'intel_nhlt_get_endpoint_blob' [-Wmissing-prototypes]
   intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
   ^
   include/sound/intel-nhlt.h:164:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct nhlt_specific_cfg *
   ^
   static 
   2 warnings generated.


vim +/intel_nhlt_has_endpoint_type +159 include/sound/intel-nhlt.h

   158	
 > 159	bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
   160	{
   161		return false;
   162	}
   163	
   164	struct nhlt_specific_cfg *
 > 165	intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
   166				     u32 bus_id, u8 link_type, u8 vbps, u8 bps,
   167				     u8 num_ch, u32 rate, u8 dir, u8 dev_type)
   168	{
   169		return NULL;
   170	}
   171	

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

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30564 bytes --]

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

* Re: [PATCH 4/5] ALSA: hda: Simplify DMIC-in-NHLT check
  2021-10-15 16:40 ` [PATCH 4/5] ALSA: hda: Simplify DMIC-in-NHLT check Cezary Rojewski
  2021-10-16  5:09     ` kernel test robot
@ 2021-10-16  5:09     ` kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2021-10-16  5:09 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel
  Cc: llvm, kbuild-all, pierre-louis.bossart, Cezary Rojewski, tiwai,
	hdegoede, broonie, Amadeusz Sławiński

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

Hi Cezary,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on tiwai-sound/for-next v5.15-rc5 next-20211015]
[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/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: riscv-randconfig-r042-20211016 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6069a6a5049497a32a50a49661c2f4169078bdba)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/2d319eabf52e504c98ef5c0fdd812347b41edd00
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cezary-Rojewski/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
        git checkout 2d319eabf52e504c98ef5c0fdd812347b41edd00
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash sound/hda/

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 >>):

   In file included from sound/hda/intel-dsp-config.c:13:
   include/sound/intel-nhlt.h:159:6: warning: no previous prototype for function 'intel_nhlt_has_endpoint_type' [-Wmissing-prototypes]
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
        ^
   include/sound/intel-nhlt.h:159:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
   ^
   static 
   include/sound/intel-nhlt.h:165:1: warning: no previous prototype for function 'intel_nhlt_get_endpoint_blob' [-Wmissing-prototypes]
   intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
   ^
   include/sound/intel-nhlt.h:164:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct nhlt_specific_cfg *
   ^
   static 
>> sound/hda/intel-dsp-config.c:387:42: error: use of undeclared identifier 'NHLT_LINK_DMIC'
                   if (intel_nhlt_has_endpoint_type(nhlt, NHLT_LINK_DMIC))
                                                          ^
>> sound/hda/intel-dsp-config.c:387:42: error: use of undeclared identifier 'NHLT_LINK_DMIC'
>> sound/hda/intel-dsp-config.c:387:42: error: use of undeclared identifier 'NHLT_LINK_DMIC'
   2 warnings and 3 errors generated.


vim +/NHLT_LINK_DMIC +387 sound/hda/intel-dsp-config.c

   379	
   380	static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
   381	{
   382		struct acpi_table_nhlt *nhlt;
   383		int ret = 0;
   384	
   385		nhlt = intel_nhlt_init();
   386		if (nhlt) {
 > 387			if (intel_nhlt_has_endpoint_type(nhlt, NHLT_LINK_DMIC))
   388				ret = 1;
   389			intel_nhlt_free(nhlt);
   390		}
   391		return ret;
   392	}
   393	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30564 bytes --]

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

* Re: [PATCH 4/5] ALSA: hda: Simplify DMIC-in-NHLT check
@ 2021-10-16  5:09     ` kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2021-10-16  5:09 UTC (permalink / raw)
  To: Cezary Rojewski, alsa-devel
  Cc: Cezary Rojewski, kbuild-all, llvm, pierre-louis.bossart, tiwai,
	hdegoede, broonie, Amadeusz Sławiński

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

Hi Cezary,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on tiwai-sound/for-next v5.15-rc5 next-20211015]
[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/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: riscv-randconfig-r042-20211016 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6069a6a5049497a32a50a49661c2f4169078bdba)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/2d319eabf52e504c98ef5c0fdd812347b41edd00
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cezary-Rojewski/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
        git checkout 2d319eabf52e504c98ef5c0fdd812347b41edd00
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash sound/hda/

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 >>):

   In file included from sound/hda/intel-dsp-config.c:13:
   include/sound/intel-nhlt.h:159:6: warning: no previous prototype for function 'intel_nhlt_has_endpoint_type' [-Wmissing-prototypes]
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
        ^
   include/sound/intel-nhlt.h:159:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
   ^
   static 
   include/sound/intel-nhlt.h:165:1: warning: no previous prototype for function 'intel_nhlt_get_endpoint_blob' [-Wmissing-prototypes]
   intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
   ^
   include/sound/intel-nhlt.h:164:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct nhlt_specific_cfg *
   ^
   static 
>> sound/hda/intel-dsp-config.c:387:42: error: use of undeclared identifier 'NHLT_LINK_DMIC'
                   if (intel_nhlt_has_endpoint_type(nhlt, NHLT_LINK_DMIC))
                                                          ^
>> sound/hda/intel-dsp-config.c:387:42: error: use of undeclared identifier 'NHLT_LINK_DMIC'
>> sound/hda/intel-dsp-config.c:387:42: error: use of undeclared identifier 'NHLT_LINK_DMIC'
   2 warnings and 3 errors generated.


vim +/NHLT_LINK_DMIC +387 sound/hda/intel-dsp-config.c

   379	
   380	static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
   381	{
   382		struct acpi_table_nhlt *nhlt;
   383		int ret = 0;
   384	
   385		nhlt = intel_nhlt_init();
   386		if (nhlt) {
 > 387			if (intel_nhlt_has_endpoint_type(nhlt, NHLT_LINK_DMIC))
   388				ret = 1;
   389			intel_nhlt_free(nhlt);
   390		}
   391		return ret;
   392	}
   393	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30564 bytes --]

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

* Re: [PATCH 4/5] ALSA: hda: Simplify DMIC-in-NHLT check
@ 2021-10-16  5:09     ` kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2021-10-16  5:09 UTC (permalink / raw)
  To: kbuild-all

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

Hi Cezary,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on tiwai-sound/for-next v5.15-rc5 next-20211015]
[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/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: riscv-randconfig-r042-20211016 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6069a6a5049497a32a50a49661c2f4169078bdba)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/2d319eabf52e504c98ef5c0fdd812347b41edd00
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cezary-Rojewski/ALSA-hda-New-NHLT-functions-and-cleanup/20211016-004154
        git checkout 2d319eabf52e504c98ef5c0fdd812347b41edd00
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash sound/hda/

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 >>):

   In file included from sound/hda/intel-dsp-config.c:13:
   include/sound/intel-nhlt.h:159:6: warning: no previous prototype for function 'intel_nhlt_has_endpoint_type' [-Wmissing-prototypes]
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
        ^
   include/sound/intel-nhlt.h:159:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool intel_nhlt_has_endpoint_type(struct acpi_table_nhlt *nhlt, u8 link_type)
   ^
   static 
   include/sound/intel-nhlt.h:165:1: warning: no previous prototype for function 'intel_nhlt_get_endpoint_blob' [-Wmissing-prototypes]
   intel_nhlt_get_endpoint_blob(struct acpi_table_nhlt *nhlt,
   ^
   include/sound/intel-nhlt.h:164:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct nhlt_specific_cfg *
   ^
   static 
>> sound/hda/intel-dsp-config.c:387:42: error: use of undeclared identifier 'NHLT_LINK_DMIC'
                   if (intel_nhlt_has_endpoint_type(nhlt, NHLT_LINK_DMIC))
                                                          ^
>> sound/hda/intel-dsp-config.c:387:42: error: use of undeclared identifier 'NHLT_LINK_DMIC'
>> sound/hda/intel-dsp-config.c:387:42: error: use of undeclared identifier 'NHLT_LINK_DMIC'
   2 warnings and 3 errors generated.


vim +/NHLT_LINK_DMIC +387 sound/hda/intel-dsp-config.c

   379	
   380	static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
   381	{
   382		struct acpi_table_nhlt *nhlt;
   383		int ret = 0;
   384	
   385		nhlt = intel_nhlt_init();
   386		if (nhlt) {
 > 387			if (intel_nhlt_has_endpoint_type(nhlt, NHLT_LINK_DMIC))
   388				ret = 1;
   389			intel_nhlt_free(nhlt);
   390		}
   391		return ret;
   392	}
   393	

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

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30564 bytes --]

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

* Re: [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions
  2021-10-15 16:42   ` Pierre-Louis Bossart
@ 2021-10-17  7:52     ` Takashi Iwai
  2021-10-18  8:08       ` Cezary Rojewski
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2021-10-17  7:52 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Cezary Rojewski, tiwai, alsa-devel, hdegoede, broonie,
	Amadeusz SX2awiX4ski

On Fri, 15 Oct 2021 18:42:33 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 10/15/21 11:40 AM, Cezary Rojewski wrote:
> > From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> > 
> > ACPI is device independent, so printing warnings using device functions
> > is misleading. Replace dev_xxx() with pr_xxx() and remove now
> > unnecessary argument.
> 
> the routines in sound/hda/intel-nhtl.c are called from a specific PCI
> device, why would you remove that information?
> 
> This makes no sense to me.

Right, otherwise this change would confuse user, too; they'll be
clueless about who triggers it.

It's OK to change to pr_*(), but then it should have more information
that can be easily identified and understood what user should do.


thanks,

Takashi

> 
> > 
> > Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> > Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> > ---
> >  include/sound/intel-nhlt.h    |  9 ++++-----
> >  sound/hda/intel-dsp-config.c  |  4 ++--
> >  sound/hda/intel-nhlt.c        | 24 +++++++++++++-----------
> >  sound/soc/intel/skylake/skl.c |  5 ++---
> >  sound/soc/sof/intel/hda.c     |  4 ++--
> >  5 files changed, 23 insertions(+), 23 deletions(-)
> > 
> > diff --git a/include/sound/intel-nhlt.h b/include/sound/intel-nhlt.h
> > index d0574805865f..4debab7c1996 100644
> > --- a/include/sound/intel-nhlt.h
> > +++ b/include/sound/intel-nhlt.h
> > @@ -126,17 +126,17 @@ enum {
> >  	NHLT_MIC_ARRAY_VENDOR_DEFINED = 0xf,
> >  };
> >  
> > -struct nhlt_acpi_table *intel_nhlt_init(struct device *dev);
> > +struct nhlt_acpi_table *intel_nhlt_init(void);
> >  
> >  void intel_nhlt_free(struct nhlt_acpi_table *addr);
> >  
> > -int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt);
> > +int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt);
> >  
> >  #else
> >  
> >  struct nhlt_acpi_table;
> >  
> > -static inline struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
> > +static inline struct nhlt_acpi_table *intel_nhlt_init(void)
> >  {
> >  	return NULL;
> >  }
> > @@ -145,8 +145,7 @@ static inline void intel_nhlt_free(struct nhlt_acpi_table *addr)
> >  {
> >  }
> >  
> > -static inline int intel_nhlt_get_dmic_geo(struct device *dev,
> > -					  struct nhlt_acpi_table *nhlt)
> > +static inline int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt)
> >  {
> >  	return 0;
> >  }
> > diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c
> > index b9ac9e9e45a4..60cc4735c6ec 100644
> > --- a/sound/hda/intel-dsp-config.c
> > +++ b/sound/hda/intel-dsp-config.c
> > @@ -382,9 +382,9 @@ static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
> >  	struct nhlt_acpi_table *nhlt;
> >  	int ret = 0;
> >  
> > -	nhlt = intel_nhlt_init(&pci->dev);
> > +	nhlt = intel_nhlt_init();
> >  	if (nhlt) {
> > -		if (intel_nhlt_get_dmic_geo(&pci->dev, nhlt))
> > +		if (intel_nhlt_get_dmic_geo(nhlt))
> >  			ret = 1;
> >  		intel_nhlt_free(nhlt);
> >  	}
> > diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
> > index e2237239d922..195d9e193a6c 100644
> > --- a/sound/hda/intel-nhlt.c
> > +++ b/sound/hda/intel-nhlt.c
> > @@ -1,10 +1,12 @@
> >  // SPDX-License-Identifier: GPL-2.0-only
> >  // Copyright (c) 2015-2019 Intel Corporation
> >  
> > +#define pr_fmt(fmt)     "NHLT: " fmt
> > +
> >  #include <linux/acpi.h>
> >  #include <sound/intel-nhlt.h>
> >  
> > -struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
> > +struct nhlt_acpi_table *intel_nhlt_init(void)
> >  {
> >  	struct nhlt_acpi_table *nhlt;
> >  	acpi_status status;
> > @@ -12,7 +14,7 @@ struct nhlt_acpi_table *intel_nhlt_init(struct device *dev)
> >  	status = acpi_get_table(ACPI_SIG_NHLT, 0,
> >  				(struct acpi_table_header **)&nhlt);
> >  	if (ACPI_FAILURE(status)) {
> > -		dev_warn(dev, "NHLT table not found\n");
> > +		pr_warn("NHLT table not found\n");
> >  		return NULL;
> >  	}
> >  
> > @@ -26,7 +28,7 @@ void intel_nhlt_free(struct nhlt_acpi_table *nhlt)
> >  }
> >  EXPORT_SYMBOL_GPL(intel_nhlt_free);
> >  
> > -int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
> > +int intel_nhlt_get_dmic_geo(struct nhlt_acpi_table *nhlt)
> >  {
> >  	struct nhlt_endpoint *epnt;
> >  	struct nhlt_dmic_array_config *cfg;
> > @@ -40,7 +42,7 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
> >  		return 0;
> >  
> >  	if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
> > -		dev_warn(dev, "Invalid DMIC description table\n");
> > +		pr_warn("Invalid DMIC description table\n");
> >  		return 0;
> >  	}
> >  
> > @@ -55,7 +57,7 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
> >  
> >  		/* find max number of channels based on format_configuration */
> >  		if (fmt_configs->fmt_count) {
> > -			dev_dbg(dev, "%s: found %d format definitions\n",
> > +			pr_debug("%s: found %d format definitions\n",
> >  				__func__, fmt_configs->fmt_count);
> >  
> >  			for (i = 0; i < fmt_configs->fmt_count; i++) {
> > @@ -66,9 +68,9 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
> >  				if (fmt_ext->fmt.channels > max_ch)
> >  					max_ch = fmt_ext->fmt.channels;
> >  			}
> > -			dev_dbg(dev, "%s: max channels found %d\n", __func__, max_ch);
> > +			pr_debug("%s: max channels found %d\n", __func__, max_ch);
> >  		} else {
> > -			dev_dbg(dev, "%s: No format information found\n", __func__);
> > +			pr_debug("%s: No format information found\n", __func__);
> >  		}
> >  
> >  		if (cfg->device_config.config_type != NHLT_CONFIG_TYPE_MIC_ARRAY) {
> > @@ -90,21 +92,21 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
> >  				dmic_geo = cfg_vendor->nb_mics;
> >  				break;
> >  			default:
> > -				dev_warn(dev, "%s: undefined DMIC array_type 0x%0x\n",
> > +				pr_warn("%s: undefined DMIC array_type 0x%0x\n",
> >  					 __func__, cfg->array_type);
> >  			}
> >  
> >  			if (dmic_geo > 0) {
> > -				dev_dbg(dev, "%s: Array with %d dmics\n", __func__, dmic_geo);
> > +				pr_debug("%s: Array with %d dmics\n", __func__, dmic_geo);
> >  			}
> >  			if (max_ch > dmic_geo) {
> > -				dev_dbg(dev, "%s: max channels %d exceed dmic number %d\n",
> > +				pr_debug("%s: max channels %d exceed dmic number %d\n",
> >  					__func__, max_ch, dmic_geo);
> >  			}
> >  		}
> >  	}
> >  
> > -	dev_dbg(dev, "%s: dmic number %d max_ch %d\n",
> > +	pr_debug("%s: dmic number %d max_ch %d\n",
> >  		__func__, dmic_geo, max_ch);
> >  
> >  	return dmic_geo;
> > diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> > index 5b1a15e39912..4f122616b636 100644
> > --- a/sound/soc/intel/skylake/skl.c
> > +++ b/sound/soc/intel/skylake/skl.c
> > @@ -517,8 +517,7 @@ static int skl_find_machine(struct skl_dev *skl, void *driver_data)
> >  	if (pdata) {
> >  		skl->use_tplg_pcm = pdata->use_tplg_pcm;
> >  		mach->mach_params.dmic_num =
> > -			intel_nhlt_get_dmic_geo(&skl->pci->dev,
> > -						skl->nhlt);
> > +			intel_nhlt_get_dmic_geo(skl->nhlt);
> >  	}
> >  
> >  	return 0;
> > @@ -1009,7 +1008,7 @@ static int skl_probe(struct pci_dev *pci,
> >  
> >  	device_disable_async_suspend(bus->dev);
> >  
> > -	skl->nhlt = intel_nhlt_init(bus->dev);
> > +	skl->nhlt = intel_nhlt_init();
> >  
> >  	if (skl->nhlt == NULL) {
> >  #if !IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
> > diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
> > index 883d78dd01b5..75c2ee91bf13 100644
> > --- a/sound/soc/sof/intel/hda.c
> > +++ b/sound/soc/sof/intel/hda.c
> > @@ -651,9 +651,9 @@ static int check_nhlt_dmic(struct snd_sof_dev *sdev)
> >  	struct nhlt_acpi_table *nhlt;
> >  	int dmic_num;
> >  
> > -	nhlt = intel_nhlt_init(sdev->dev);
> > +	nhlt = intel_nhlt_init();
> >  	if (nhlt) {
> > -		dmic_num = intel_nhlt_get_dmic_geo(sdev->dev, nhlt);
> > +		dmic_num = intel_nhlt_get_dmic_geo(nhlt);
> >  		intel_nhlt_free(nhlt);
> >  		if (dmic_num >= 1 && dmic_num <= 4)
> >  			return dmic_num;
> > 
> 

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

* Re: [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions
  2021-10-17  7:52     ` Takashi Iwai
@ 2021-10-18  8:08       ` Cezary Rojewski
  2021-10-18  8:25         ` Takashi Iwai
  0 siblings, 1 reply; 20+ messages in thread
From: Cezary Rojewski @ 2021-10-18  8:08 UTC (permalink / raw)
  To: Takashi Iwai, Pierre-Louis Bossart
  Cc: alsa-devel, tiwai, hdegoede, broonie, Amadeusz SX2awiX4ski

On 2021-10-17 9:52 AM, Takashi Iwai wrote:
> On Fri, 15 Oct 2021 18:42:33 +0200,
> Pierre-Louis Bossart wrote:
>> On 10/15/21 11:40 AM, Cezary Rojewski wrote:
>>> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>>>
>>> ACPI is device independent, so printing warnings using device functions
>>> is misleading. Replace dev_xxx() with pr_xxx() and remove now
>>> unnecessary argument.
>>
>> the routines in sound/hda/intel-nhtl.c are called from a specific PCI
>> device, why would you remove that information?
>>
>> This makes no sense to me.
> 
> Right, otherwise this change would confuse user, too; they'll be
> clueless about who triggers it.
> 
> It's OK to change to pr_*(), but then it should have more information
> that can be easily identified and understood what user should do.

Isn't the answer as to 'who' used it obvious, though? NHLT is used for 
I2S and DMIC endpoints only, so the question is 'limited' in the first 
place. And then, you cannot have several Intel ADSP drivers running 
simultaneously.

Also, logs found ACPI-table interface are device-less so this patch 
makes NHLT interface look more ACPI-generic alike.


Regards,
Czarek

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

* Re: [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions
  2021-10-18  8:08       ` Cezary Rojewski
@ 2021-10-18  8:25         ` Takashi Iwai
  2021-10-18  9:07           ` Cezary Rojewski
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2021-10-18  8:25 UTC (permalink / raw)
  To: Cezary Rojewski
  Cc: alsa-devel, tiwai, Pierre-Louis Bossart, hdegoede, broonie,
	Amadeusz SX2awiX4ski

On Mon, 18 Oct 2021 10:08:30 +0200,
Cezary Rojewski wrote:
> 
> On 2021-10-17 9:52 AM, Takashi Iwai wrote:
> > On Fri, 15 Oct 2021 18:42:33 +0200,
> > Pierre-Louis Bossart wrote:
> >> On 10/15/21 11:40 AM, Cezary Rojewski wrote:
> >>> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> >>>
> >>> ACPI is device independent, so printing warnings using device functions
> >>> is misleading. Replace dev_xxx() with pr_xxx() and remove now
> >>> unnecessary argument.
> >>
> >> the routines in sound/hda/intel-nhtl.c are called from a specific PCI
> >> device, why would you remove that information?
> >>
> >> This makes no sense to me.
> >
> > Right, otherwise this change would confuse user, too; they'll be
> > clueless about who triggers it.
> >
> > It's OK to change to pr_*(), but then it should have more information
> > that can be easily identified and understood what user should do.
> 
> Isn't the answer as to 'who' used it obvious, though? NHLT is used for
> I2S and DMIC endpoints only, so the question is 'limited' in the first
> place. And then, you cannot have several Intel ADSP drivers running
> simultaneously.

Well, it's not about you or devs -- those must know which driver is
relevant very well, of course.  Instead, the problem is for *all*
others who read the message.

IOW, which user would know and think "hey, it's a NHLT thingy that
must be ASoC xxx driver that spewed" only from the text snippet "NHLT
table not found"?  That's way too much expectation.  Some more
guidance is needed in the error message.  The dev_*() variant gave at
least the device names that can help guessing the relevant driver
easily.

> Also, logs found ACPI-table interface are device-less so this patch
> makes NHLT interface look more ACPI-generic alike.

The conversion itself is no problem, but the lost information is the
problem.


Takashi

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

* Re: [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions
  2021-10-18  8:25         ` Takashi Iwai
@ 2021-10-18  9:07           ` Cezary Rojewski
  2021-10-18 12:01             ` Takashi Iwai
  0 siblings, 1 reply; 20+ messages in thread
From: Cezary Rojewski @ 2021-10-18  9:07 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, tiwai, Pierre-Louis Bossart, hdegoede, broonie,
	Amadeusz SX2awiX4ski



On 2021-10-18 10:25 AM, Takashi Iwai wrote:
> On Mon, 18 Oct 2021 10:08:30 +0200,
> Cezary Rojewski wrote:
>>
>> On 2021-10-17 9:52 AM, Takashi Iwai wrote:
>>> On Fri, 15 Oct 2021 18:42:33 +0200,
>>> Pierre-Louis Bossart wrote:
>>>> On 10/15/21 11:40 AM, Cezary Rojewski wrote:
>>>>> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>>>>>
>>>>> ACPI is device independent, so printing warnings using device functions
>>>>> is misleading. Replace dev_xxx() with pr_xxx() and remove now
>>>>> unnecessary argument.
>>>>
>>>> the routines in sound/hda/intel-nhtl.c are called from a specific PCI
>>>> device, why would you remove that information?
>>>>
>>>> This makes no sense to me.
>>>
>>> Right, otherwise this change would confuse user, too; they'll be
>>> clueless about who triggers it.
>>>
>>> It's OK to change to pr_*(), but then it should have more information
>>> that can be easily identified and understood what user should do.
>>
>> Isn't the answer as to 'who' used it obvious, though? NHLT is used for
>> I2S and DMIC endpoints only, so the question is 'limited' in the first
>> place. And then, you cannot have several Intel ADSP drivers running
>> simultaneously.
> 
> Well, it's not about you or devs -- those must know which driver is
> relevant very well, of course.  Instead, the problem is for *all*
> others who read the message.
> 
> IOW, which user would know and think "hey, it's a NHLT thingy that
> must be ASoC xxx driver that spewed" only from the text snippet "NHLT
> table not found"?  That's way too much expectation.  Some more
> guidance is needed in the error message.  The dev_*() variant gave at
> least the device names that can help guessing the relevant driver
> easily.
> 
>> Also, logs found ACPI-table interface are device-less so this patch
>> makes NHLT interface look more ACPI-generic alike.
> 
> The conversion itself is no problem, but the lost information is the
> problem.

Well, most invocations that end in error will cause stack of error-logs 
(dev_xxx()) in the parent driver. NHLT reads: Non-HDAudio-Link-Table. 
The name itself points reader towards sound/hda already. I understand 
that not everyone will google "NHLT" and such but then there are ton of 
kernel messages which won't guide reader by the hand anyway.

"NHLT table not found" is configuration problem, not a driver problem. 
Mixing BIOS/config problems with driver ones leads to incorrect 
conclusions what we want to avoid.


Regards,
Czarek

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

* Re: [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions
  2021-10-18  9:07           ` Cezary Rojewski
@ 2021-10-18 12:01             ` Takashi Iwai
  2021-10-18 12:18               ` Cezary Rojewski
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2021-10-18 12:01 UTC (permalink / raw)
  To: Cezary Rojewski
  Cc: alsa-devel, tiwai, Pierre-Louis Bossart, hdegoede, broonie,
	Amadeusz SX2awiX4ski

On Mon, 18 Oct 2021 11:07:15 +0200,
Cezary Rojewski wrote:
> 
> 
> 
> On 2021-10-18 10:25 AM, Takashi Iwai wrote:
> > On Mon, 18 Oct 2021 10:08:30 +0200,
> > Cezary Rojewski wrote:
> >>
> >> On 2021-10-17 9:52 AM, Takashi Iwai wrote:
> >>> On Fri, 15 Oct 2021 18:42:33 +0200,
> >>> Pierre-Louis Bossart wrote:
> >>>> On 10/15/21 11:40 AM, Cezary Rojewski wrote:
> >>>>> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> >>>>>
> >>>>> ACPI is device independent, so printing warnings using device functions
> >>>>> is misleading. Replace dev_xxx() with pr_xxx() and remove now
> >>>>> unnecessary argument.
> >>>>
> >>>> the routines in sound/hda/intel-nhtl.c are called from a specific PCI
> >>>> device, why would you remove that information?
> >>>>
> >>>> This makes no sense to me.
> >>>
> >>> Right, otherwise this change would confuse user, too; they'll be
> >>> clueless about who triggers it.
> >>>
> >>> It's OK to change to pr_*(), but then it should have more information
> >>> that can be easily identified and understood what user should do.
> >>
> >> Isn't the answer as to 'who' used it obvious, though? NHLT is used for
> >> I2S and DMIC endpoints only, so the question is 'limited' in the first
> >> place. And then, you cannot have several Intel ADSP drivers running
> >> simultaneously.
> >
> > Well, it's not about you or devs -- those must know which driver is
> > relevant very well, of course.  Instead, the problem is for *all*
> > others who read the message.
> >
> > IOW, which user would know and think "hey, it's a NHLT thingy that
> > must be ASoC xxx driver that spewed" only from the text snippet "NHLT
> > table not found"?  That's way too much expectation.  Some more
> > guidance is needed in the error message.  The dev_*() variant gave at
> > least the device names that can help guessing the relevant driver
> > easily.
> >
> >> Also, logs found ACPI-table interface are device-less so this patch
> >> makes NHLT interface look more ACPI-generic alike.
> >
> > The conversion itself is no problem, but the lost information is the
> > problem.
> 
> Well, most invocations that end in error will cause stack of
> error-logs (dev_xxx()) in the parent driver. NHLT reads:
> Non-HDAudio-Link-Table. The name itself points reader towards
> sound/hda already.

But then how dropping the device information would improve things at
all?  It rather cuts off the link between that message and the later
one.

> I understand that not everyone will google "NHLT"
> and such but then there are ton of kernel messages which won't guide
> reader by the hand anyway.

This can't be an excuse, we don't have to follow that anti pattern :)

> "NHLT table not found" is configuration problem, not a driver
> problem. Mixing BIOS/config problems with driver ones leads to
> incorrect conclusions what we want to avoid.

The configuration itself makes sense only if it's in actual use,
i.e. the problem is always tied with the driver.

So, if any, you can put more information around the message mentioning
that it's the configuration problem and what to do for users,
e.g. check your BIOS, etc.  *That* would be the improvement.


Takashi

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

* Re: [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions
  2021-10-18 12:01             ` Takashi Iwai
@ 2021-10-18 12:18               ` Cezary Rojewski
  0 siblings, 0 replies; 20+ messages in thread
From: Cezary Rojewski @ 2021-10-18 12:18 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, tiwai, Pierre-Louis Bossart, hdegoede, broonie,
	Amadeusz SX2awiX4ski

On 2021-10-18 2:01 PM, Takashi Iwai wrote:
> On Mon, 18 Oct 2021 11:07:15 +0200,
> Cezary Rojewski wrote:

...

>> Well, most invocations that end in error will cause stack of
>> error-logs (dev_xxx()) in the parent driver. NHLT reads:
>> Non-HDAudio-Link-Table. The name itself points reader towards
>> sound/hda already.
> 
> But then how dropping the device information would improve things at
> all?  It rather cuts off the link between that message and the later
> one.
> 
>> I understand that not everyone will google "NHLT"
>> and such but then there are ton of kernel messages which won't guide
>> reader by the hand anyway.
> 
> This can't be an excuse, we don't have to follow that anti pattern :)
> 
>> "NHLT table not found" is configuration problem, not a driver
>> problem. Mixing BIOS/config problems with driver ones leads to
>> incorrect conclusions what we want to avoid.
> 
> The configuration itself makes sense only if it's in actual use,
> i.e. the problem is always tied with the driver.
> 
> So, if any, you can put more information around the message mentioning
> that it's the configuration problem and what to do for users,
> e.g. check your BIOS, etc.  *That* would be the improvement.

Ok, I'll think about better solution. For now I'll just drop this patch 
from the series and send v2.

Thanks for the discussion!

Czarek

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

end of thread, other threads:[~2021-10-18 12:19 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 16:40 [PATCH 0/5] ALSA: hda: New NHLT functions and cleanup Cezary Rojewski
2021-10-15 16:40 ` [PATCH 1/5] ALSA: hda: Drop device-argument in NHLT functions Cezary Rojewski
2021-10-15 16:42   ` Pierre-Louis Bossart
2021-10-17  7:52     ` Takashi Iwai
2021-10-18  8:08       ` Cezary Rojewski
2021-10-18  8:25         ` Takashi Iwai
2021-10-18  9:07           ` Cezary Rojewski
2021-10-18 12:01             ` Takashi Iwai
2021-10-18 12:18               ` Cezary Rojewski
2021-10-15 16:40 ` [PATCH 2/5] ALSA: hda: Follow ACPI convention in NHLT struct naming Cezary Rojewski
2021-10-15 16:40 ` [PATCH 3/5] ALSA: hda: Fill gaps in NHLT endpoint-interface Cezary Rojewski
2021-10-16  3:21   ` kernel test robot
2021-10-16  3:21     ` kernel test robot
2021-10-16  3:21     ` kernel test robot
2021-10-15 16:40 ` [PATCH 4/5] ALSA: hda: Simplify DMIC-in-NHLT check Cezary Rojewski
2021-10-16  5:09   ` kernel test robot
2021-10-16  5:09     ` kernel test robot
2021-10-16  5:09     ` kernel test robot
2021-10-15 16:40 ` [PATCH 5/5] ASoC: Intel: Skylake: Use NHLT API to search for blob Cezary Rojewski
2021-10-15 16:42   ` 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.