[alsa-devel] [PATCH v3 0/5] ASoC: intel: fix to sst driver series
This series fixes comments observed while applying the sst series last week.
changes in v3: - fix the makefile artifact not required
changes 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/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 +++++------- 4 files changed, 26 insertions(+), 14 deletions(-)
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 subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul 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); }
/**
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 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; }
copypaste error on function sst_get_num_channel caused the comment to be wrong, so fix it here
Signed-off-by: Vinod Koul 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 */
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 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) {
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 jarkko.nikula@linux.intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/sst-firmware.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
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 */
participants (2)
-
Mark Brown -
Vinod Koul