[PATCH 0/6] Small fixes

[PATCH 0/6] Small fixes

[Amadeusz Sławiński]

Series of small fixes:
* fixes few issues found during checking code with static checkers
* fix few prints
* define function in header, like it should be
* release topology when done with it

Amadeusz Sławiński (6):
  ASoC: Intel: Skylake: Use correct function to access iomem space
  ASoC: Intel: Fix use of potentially uninitialized variable
  ASoC: dapm: Expose snd_soc_dapm_new_control_unlocked properly
  ASoC: Intel: Skylake: Print module type instead of id
  ASoC: Intel: Skylake: Release topology when we are done with it
  ASoC: Intel: NHLT: Fix debug print format

 include/sound/soc-dapm.h               |  3 +++
 sound/soc/intel/common/sst-ipc.c       |  2 ++
 sound/soc/intel/skylake/skl-debug.c    |  2 +-
 sound/soc/intel/skylake/skl-messages.c |  5 +++--
 sound/soc/intel/skylake/skl-nhlt.c     |  2 +-
 sound/soc/intel/skylake/skl-topology.c | 20 ++++++++++----------
 sound/soc/intel/skylake/skl.h          |  1 -
 sound/soc/soc-topology.c               |  6 ------
 8 files changed, 20 insertions(+), 21 deletions(-)

-- 
2.17.1

[PATCH 1/6] ASoC: Intel: Skylake: Use correct function to access iomem space

[Amadeusz Sławiński]

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

For copying from __iomem, we should use __ioread32_copy.

reported by sparse:
sound/soc/intel/skylake/skl-debug.c:437:34: warning: incorrect type in argument 1 (different address spaces)
sound/soc/intel/skylake/skl-debug.c:437:34:    expected void [noderef] <asn:2> *to
sound/soc/intel/skylake/skl-debug.c:437:34:    got unsigned char *
sound/soc/intel/skylake/skl-debug.c:437:51: warning: incorrect type in argument 2 (different address spaces)
sound/soc/intel/skylake/skl-debug.c:437:51:    expected void const *from
sound/soc/intel/skylake/skl-debug.c:437:51:    got void [noderef] <asn:2> *[assigned] fw_reg_addr

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@intel.com>
---
 sound/soc/intel/skylake/skl-debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-debug.c b/sound/soc/intel/skylake/skl-debug.c
index 212370bf704c..3466675f2678 100644
--- a/sound/soc/intel/skylake/skl-debug.c
+++ b/sound/soc/intel/skylake/skl-debug.c
@@ -188,7 +188,7 @@ static ssize_t fw_softreg_read(struct file *file, char __user *user_buf,
 	memset(d->fw_read_buff, 0, FW_REG_BUF);
 
 	if (w0_stat_sz > 0)
-		__iowrite32_copy(d->fw_read_buff, fw_reg_addr, w0_stat_sz >> 2);
+		__ioread32_copy(d->fw_read_buff, fw_reg_addr, w0_stat_sz >> 2);
 
 	for (offset = 0; offset < FW_REG_SIZE; offset += 16) {
 		ret += snprintf(tmp + ret, FW_REG_BUF - ret, "%#.4x: ", offset);
-- 
2.17.1

[PATCH 2/6] ASoC: Intel: Fix use of potentially uninitialized variable

[Amadeusz Sławiński]

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

If ipc->ops.reply_msg_match is NULL, we may end up using uninitialized
mask value.

reported by smatch:
sound/soc/intel/common/sst-ipc.c:266 sst_ipc_reply_find_msg() error: uninitialized symbol 'mask'.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@intel.com>
---
 sound/soc/intel/common/sst-ipc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/intel/common/sst-ipc.c b/sound/soc/intel/common/sst-ipc.c
index 1186a03a88d6..6068bb697e22 100644
--- a/sound/soc/intel/common/sst-ipc.c
+++ b/sound/soc/intel/common/sst-ipc.c
@@ -223,6 +223,8 @@ struct ipc_message *sst_ipc_reply_find_msg(struct sst_generic_ipc *ipc,
 
 	if (ipc->ops.reply_msg_match != NULL)
 		header = ipc->ops.reply_msg_match(header, &mask);
+	else
+		mask = (u64)-1;
 
 	if (list_empty(&ipc->rx_list)) {
 		dev_err(ipc->dev, "error: rx list empty but received 0x%llx\n",
-- 
2.17.1

[PATCH 3/6] ASoC: dapm: Expose snd_soc_dapm_new_control_unlocked properly

[Amadeusz Sławiński]

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

We use snd_soc_dapm_new_control_unlocked for topology and have local
declaration, instead declare it properly in header like already declared
snd_soc_dapm_new_control.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@intel.com>
---
 include/sound/soc-dapm.h | 3 +++
 sound/soc/soc-topology.c | 6 ------
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 2aa73d6dd7be..30d8d0410952 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -404,6 +404,9 @@ int snd_soc_dapm_new_controls(struct snd_soc_dapm_context *dapm,
 struct snd_soc_dapm_widget *snd_soc_dapm_new_control(
 		struct snd_soc_dapm_context *dapm,
 		const struct snd_soc_dapm_widget *widget);
+struct snd_soc_dapm_widget *snd_soc_dapm_new_control_unlocked(
+		struct snd_soc_dapm_context *dapm,
+		const struct snd_soc_dapm_widget *widget);
 int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
 				 struct snd_soc_dai *dai);
 int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card);
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index b8690715abb5..aa9a1fca46fa 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -80,12 +80,6 @@ struct soc_tplg {
 
 static int soc_tplg_process_headers(struct soc_tplg *tplg);
 static void soc_tplg_complete(struct soc_tplg *tplg);
-struct snd_soc_dapm_widget *
-snd_soc_dapm_new_control_unlocked(struct snd_soc_dapm_context *dapm,
-			 const struct snd_soc_dapm_widget *widget);
-struct snd_soc_dapm_widget *
-snd_soc_dapm_new_control(struct snd_soc_dapm_context *dapm,
-			 const struct snd_soc_dapm_widget *widget);
 static void soc_tplg_denum_remove_texts(struct soc_enum *se);
 static void soc_tplg_denum_remove_values(struct soc_enum *se);
 
-- 
2.17.1

[PATCH 4/6] ASoC: Intel: Skylake: Print module type instead of id

[Amadeusz Sławiński]

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

When we are printing module params, we were actually printing module id
instead of type, but debug message was saying that number we get is type.
So print module type, as it is useful when debugging paths, but also
keep printing module id, as it is used in all other logs.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@intel.com>
---
 sound/soc/intel/skylake/skl-messages.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index d43496c5f29e..476ef1897961 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -867,8 +867,9 @@ static int skl_set_module_format(struct skl_dev *skl,
 
 	}
 
-	dev_dbg(skl->dev, "Module type=%d config size: %d bytes\n",
-			module_config->id.module_id, param_size);
+	dev_dbg(skl->dev, "Module type=%d id=%d config size: %d bytes\n",
+			module_config->m_type, module_config->id.module_id,
+			param_size);
 	print_hex_dump_debug("Module params:", DUMP_PREFIX_OFFSET, 8, 4,
 			*param_data, param_size, false);
 	return 0;
-- 
2.17.1

[PATCH 5/6] ASoC: Intel: Skylake: Release topology when we are done with it

[Amadeusz Sławiński]

Currently topology is kept in memory while driver is running. It's
unnecessary, as it's only needed during parsing.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/skylake/skl-topology.c | 20 ++++++++++----------
 sound/soc/intel/skylake/skl.h          |  1 -
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index ae5c75d03fdc..69cd7a81bf2a 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -3579,23 +3579,25 @@ int skl_tplg_init(struct snd_soc_component *component, struct hdac_bus *bus)
 	 * The complete tplg for SKL is loaded as index 0, we don't use
 	 * any other index
 	 */
-	ret = snd_soc_tplg_component_load(component,
-					&skl_tplg_ops, fw, 0);
+	ret = snd_soc_tplg_component_load(component, &skl_tplg_ops, fw, 0);
 	if (ret < 0) {
 		dev_err(bus->dev, "tplg component load failed%d\n", ret);
-		release_firmware(fw);
-		return -EINVAL;
+		goto err;
 	}
 
-	skl->tplg = fw;
 	ret = skl_tplg_create_pipe_widget_list(component);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		dev_err(bus->dev, "tplg create pipe widget list failed%d\n",
+				ret);
+		goto err;
+	}
 
 	list_for_each_entry(ppl, &skl->ppl_list, node)
 		skl_tplg_set_pipe_type(skl, ppl->pipe);
 
-	return 0;
+err:
+	release_firmware(fw);
+	return ret;
 }
 
 void skl_tplg_exit(struct snd_soc_component *component, struct hdac_bus *bus)
@@ -3609,6 +3611,4 @@ void skl_tplg_exit(struct snd_soc_component *component, struct hdac_bus *bus)
 
 	/* clean up topology */
 	snd_soc_tplg_component_remove(component, SND_SOC_TPLG_INDEX_ALL);
-
-	release_firmware(skl->tplg);
 }
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index f8c714153610..2bfbf59277c4 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -75,7 +75,6 @@ struct skl_dev {
 	const char *fw_name;
 	char tplg_name[64];
 	unsigned short pci_id;
-	const struct firmware *tplg;
 
 	int supend_active;
 
-- 
2.17.1

[PATCH 6/6] ASoC: Intel: NHLT: Fix debug print format

[Amadeusz Sławiński]

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

oem_table_id is 8 chars long, so we need to limit it, otherwise it
may print some unprintable characters into dmesg.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@intel.com>
---
 sound/soc/intel/skylake/skl-nhlt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index ab3d23c7bd65..19f328d71f24 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -136,7 +136,7 @@ int skl_nhlt_update_topology_bin(struct skl_dev *skl)
 	struct hdac_bus *bus = skl_to_bus(skl);
 	struct device *dev = bus->dev;
 
-	dev_dbg(dev, "oem_id %.6s, oem_table_id %8s oem_revision %d\n",
+	dev_dbg(dev, "oem_id %.6s, oem_table_id %.8s oem_revision %d\n",
 		nhlt->header.oem_id, nhlt->header.oem_table_id,
 		nhlt->header.oem_revision);
 
-- 
2.17.1

Re: [alsa-devel] [PATCH 0/6] Small fixes

[Pierre-Louis Bossart]

On 8/27/19 9:17 AM, Amadeusz Sławiński wrote:
> Series of small fixes:
> * fixes few issues found during checking code with static checkers
> * fix few prints
> * define function in header, like it should be
> * release topology when done with it
> 
> Amadeusz Sławiński (6):
>    ASoC: Intel: Skylake: Use correct function to access iomem space
>    ASoC: Intel: Fix use of potentially uninitialized variable
>    ASoC: dapm: Expose snd_soc_dapm_new_control_unlocked properly
>    ASoC: Intel: Skylake: Print module type instead of id
>    ASoC: Intel: Skylake: Release topology when we are done with it
>    ASoC: Intel: NHLT: Fix debug print format

LGTM

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> 
>   include/sound/soc-dapm.h               |  3 +++
>   sound/soc/intel/common/sst-ipc.c       |  2 ++
>   sound/soc/intel/skylake/skl-debug.c    |  2 +-
>   sound/soc/intel/skylake/skl-messages.c |  5 +++--
>   sound/soc/intel/skylake/skl-nhlt.c     |  2 +-
>   sound/soc/intel/skylake/skl-topology.c | 20 ++++++++++----------
>   sound/soc/intel/skylake/skl.h          |  1 -
>   sound/soc/soc-topology.c               |  6 ------
>   8 files changed, 20 insertions(+), 21 deletions(-)
> 

Re: [PATCH 2/6] ASoC: Intel: Fix use of potentially uninitialized variable

[Cezary Rojewski]

On 2019-08-27 16:17, Amadeusz Sławiński wrote:
> From: Amadeusz Sławiński <amadeuszx.slawinski@intel.com>
> 
> If ipc->ops.reply_msg_match is NULL, we may end up using uninitialized
> mask value.
> 
> reported by smatch:
> sound/soc/intel/common/sst-ipc.c:266 sst_ipc_reply_find_msg() error: uninitialized symbol 'mask'.
> 
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@intel.com>
> ---
>   sound/soc/intel/common/sst-ipc.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/sound/soc/intel/common/sst-ipc.c b/sound/soc/intel/common/sst-ipc.c
> index 1186a03a88d6..6068bb697e22 100644
> --- a/sound/soc/intel/common/sst-ipc.c
> +++ b/sound/soc/intel/common/sst-ipc.c
> @@ -223,6 +223,8 @@ struct ipc_message *sst_ipc_reply_find_msg(struct sst_generic_ipc *ipc,
>   
>   	if (ipc->ops.reply_msg_match != NULL)
>   		header = ipc->ops.reply_msg_match(header, &mask);
> +	else
> +		mask = (u64)-1;

Please see linux/limits.h and check if this can't be replaced by an 
equivalent found there.

>   
>   	if (list_empty(&ipc->rx_list)) {
>   		dev_err(ipc->dev, "error: rx list empty but received 0x%llx\n",
>