[PATCH v2 0/5] ASoC: intel: fix to sst driver series

[Vinod Koul]

This series fixes comments observed while applying the sst series last week.

canges in v2:
- typo fixes
- make wrapper inline
- add newline on err log

Vinod Koul (5):
ASoC: intel: use __iowrite32_copy for 32 bit copy
ASoC: intel: log an error on double free
ASoC: intel: fix the kernldoc comment
ASoC: intel: explain why block not found isn't error always
ASoC: intel: use __iowrite32_copy for 32 bit copy

sound/soc/intel/Kconfig | 2 +-
sound/soc/intel/sst-firmware.c | 9 +++------
sound/soc/intel/sst/sst_drv_interface.c | 4 +++-
sound/soc/intel/sst/sst_ipc.c | 15 +++++++++++++++
sound/soc/intel/sst/sst_loader.c | 12 +++++-------
5 files changed, 27 insertions(+), 15 deletions(-)

[Vinod Koul]

copypaste error on function sst_get_num_channel caused the comment to be
wrong, so fix it here

Signed-off-by: Vinod Koul <***@intel.com>
---
sound/soc/intel/sst/sst_drv_interface.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/intel/sst/sst_drv_interface.c b/sound/soc/intel/sst/sst_drv_interface.c
index 3a5e920..183b1eb 100644
--- a/sound/soc/intel/sst/sst_drv_interface.c
+++ b/sound/soc/intel/sst/sst_drv_interface.c
@@ -94,7 +94,7 @@ int sst_get_sfreq(struct snd_sst_params *str_param)
}

/*
- * sst_get_sfreq - this function returns the frequency of the stream
+ * sst_get_num_channel - get number of channels for the stream
*
* @str_param : stream params
*/

[Vinod Koul]

the stream context should be freed only once on stream cleanup. If we ever
hit a chance that stream context is getting double freed, though not an
cause of panic as memory allocator can deal with this, we should still log
this to help in finding issues and debugging

Signed-off-by: Vinod Koul <***@intel.com>
---
sound/soc/intel/sst/sst_drv_interface.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/intel/sst/sst_drv_interface.c b/sound/soc/intel/sst/sst_drv_interface.c
index aadb0db..3a5e920 100644
--- a/sound/soc/intel/sst/sst_drv_interface.c
+++ b/sound/soc/intel/sst/sst_drv_interface.c
@@ -55,6 +55,8 @@ int free_stream_context(struct intel_sst_drv *ctx, unsigned int str_id)
if (ret)
sst_clean_stream(&ctx->streams[str_id]);
return ret;
+ } else {
+ dev_err(ctx->dev, "we tried to free stream context %d which was freed!!!\n", str_id);
}
return ret;
}

[Vinod Koul]

The driver was using own method to do 32bit copy, turns out we have a kernel
API so use that instead

Tested-by: Subhransu S. Prusty <***@intel.com>
Signed-off-by: Vinod Koul <***@intel.com>
---
sound/soc/intel/sst/sst_loader.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/sst/sst_loader.c b/sound/soc/intel/sst/sst_loader.c
index b6d27c1..00f60c1 100644
--- a/sound/soc/intel/sst/sst_loader.c
+++ b/sound/soc/intel/sst/sst_loader.c
@@ -39,14 +39,12 @@
#include "sst.h"
#include "../sst-dsp.h"

-static void memcpy32_toio(void __iomem *dst, const void *src, int count)
+static inline void memcpy32_toio(void __iomem *dst, const void *src, int count)
{
- int i;
- const u32 *src_32 = src;
- u32 *dst_32 = dst;
-
- for (i = 0; i < count/sizeof(u32); i++)
- writel(*src_32++, dst_32++);
+ /* __iowrite32_copy uses 32-bit count values so divide by 4 for
+ * right count in words
+ */
+ __iowrite32_copy(dst, src, count/4);
}

/**

[Vinod Koul]

The IPC blocking can be error when we don't find block or a short message,
explain that by adding a comment about this scenario

Signed-off-by: Vinod Koul <***@intel.com>
---
sound/soc/intel/sst/sst_ipc.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/sound/soc/intel/sst/sst_ipc.c b/sound/soc/intel/sst/sst_ipc.c
index 41a2b41..2126f5b 100644
--- a/sound/soc/intel/sst/sst_ipc.c
+++ b/sound/soc/intel/sst/sst_ipc.c
@@ -54,6 +54,21 @@ struct sst_block *sst_create_block(struct intel_sst_drv *ctx,
return msg;
}

+/*
+ * while handling the interrupts, we need to check for message status and
+ * then if we are blocking for a message
+ *
+ * here we are unblocking the blocked ones, this is based on id we have
+ * passed and search that for block threads.
+ * We will not find block in two cases
+ * a) when its small message and block in not there, so silently ignore
+ * them
+ * b) when we are actually not able to find the block (bug perhaps)
+ *
+ * Since we have bit of small messages we can spam kernel log with err
+ * print on above so need to keep as debug prints which should be enabled
+ * via dynamic debug while debugging IPC issues
+ */
int sst_wake_up_block(struct intel_sst_drv *ctx, int result,
u32 drv_id, u32 ipc, void *data, u32 size)
{

[Vinod Koul]

The sst-firmware was also using own method to do 32bit copy, turns out we have a
kernel API so use that instead

[For BYT]
Tested-by: Jarkko Nikula <***@linux.intel.com>
Signed-off-by: Vinod Koul <***@intel.com>
---
sound/soc/intel/Kconfig | 2 +-
sound/soc/intel/sst-firmware.c | 9 +++------
2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index c719438..b8a02cb 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -1,6 +1,6 @@
config SND_MFLD_MACHINE
tristate "SOC Machine Audio driver for Intel Medfield MID platform"
- depends on INTEL_SCU_IPC
+ depends on INTEL_SCU_IPC || COMPILE_TEST
select SND_SOC_SN95031
select SND_SST_MFLD_PLATFORM
select SND_SST_IPC
diff --git a/sound/soc/intel/sst-firmware.c b/sound/soc/intel/sst-firmware.c
index 3bb43da..cf3d199 100644
--- a/sound/soc/intel/sst-firmware.c
+++ b/sound/soc/intel/sst-firmware.c
@@ -32,13 +32,10 @@

static void block_module_remove(struct sst_module *module);

-static void sst_memcpy32(volatile void __iomem *dest, void *src, u32 bytes)
+static inline void sst_memcpy32(volatile void __iomem *dest, void *src, u32 bytes)
{
- u32 i;
-
- /* copy one 32 bit word at a time as 64 bit access is not supported */
- for (i = 0; i < bytes; i += 4)
- memcpy_toio(dest + i, src + i, 4);
+ /* __iowrite32_copy use 32bit size values so divide by 4 */
+ __iowrite32_copy((void *)dest, src, bytes/4);
}

/* create new generic firmware object */