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
From: Amadeusz Sławiński <[email protected]>
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 <[email protected]>
---
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
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 <[email protected]>
---
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
From: Amadeusz Sławiński <[email protected]>
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 <[email protected]>
---
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
From: Amadeusz Sławiński <[email protected]>
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 <[email protected]>
---
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
From: Amadeusz Sławiński <[email protected]>
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 <[email protected]>
---
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
From: Amadeusz Sławiński <[email protected]>
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 <[email protected]>
---
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
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 <[email protected]>
>
> 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(-)
>
On 2019-08-27 16:17, Amadeusz Sławiński wrote:
> From: Amadeusz Sławiński <[email protected]>
>
> 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 <[email protected]>
> ---
> 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",
>