[PATCH 0/4] new fix for broadwell module removing failed issue

[PATCH 0/4] new fix for broadwell module removing failed issue

[Jie Yang]

This patch series are based on broonie/sound.git for-next branch, commit
01a4d5083562fcdeab99d42b0873aabc5fcca018.

The broadwell platform driver had module removing failed issue, and the
two commits("ASoC: Intel: remove unused function hsw_pcm_free_modules()"
and "ASoC: Intel: fix broadwell module removing failed issue") workaround
it.

But at the same time, we shouldn't leave the device as suspended state
after module freed, it is not good to do runtime suspend at driver free
either, so the fix was workaround only and not good enough.

Here revert the old fix, and replace it with new unified procedure for
runtime modules freeing:
suspends firmware ==> frees runtime modules ==> unloads firmware.

With this new procedure, we remove the runtime modules at both runtime
suspend(D0->D3) and module(snd_soc_sst_haswell_pcm) removing, but make
sure we do it only once.

Jie Yang (4):
  Revert "ASoC: Intel: remove unused function hsw_pcm_free_modules()"
  Revert "ASoC: Intel: fix broadwell module removing failed issue"
  ASoC: Intel: check and clear runtime module pointer
  ASoC: Intel: handle haswell pcm suspend including runtime modules
    freeing

 sound/soc/intel/haswell/sst-haswell-pcm.c | 46 +++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 9 deletions(-)

-- 
1.9.1

[PATCH 1/4] Revert "ASoC: Intel: remove unused function hsw_pcm_free_modules()"

[Jie Yang]

This reverts commit 506c148ee5e1bfb836116353305927ca4c21a23e.

We still need this hsw_pcm_free_modules(), we plan to remove the
runtime modules at both fw_unload(D0->D3) and snd_soc_sst_haswell_pcm
module removing.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 sound/soc/intel/haswell/sst-haswell-pcm.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c b/sound/soc/intel/haswell/sst-haswell-pcm.c
index 1557e37..bd96629 100644
--- a/sound/soc/intel/haswell/sst-haswell-pcm.c
+++ b/sound/soc/intel/haswell/sst-haswell-pcm.c
@@ -920,6 +920,21 @@ err:
 	return -ENODEV;
 }
 
+static void hsw_pcm_free_modules(struct hsw_priv_data *pdata)
+{
+	struct sst_hsw *hsw = pdata->hsw;
+	struct hsw_pcm_data *pcm_data;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mod_map); i++) {
+		pcm_data = &pdata->pcm[mod_map[i].dai_id][mod_map[i].stream];
+		sst_hsw_runtime_module_free(pcm_data->runtime);
+	}
+	if (sst_hsw_is_module_loaded(hsw, SST_HSW_MODULE_WAVES)) {
+		sst_hsw_runtime_module_free(pdata->runtime_waves);
+	}
+}
+
 static int hsw_pcm_new(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_pcm *pcm = rtd->pcm;
-- 
1.9.1

[PATCH 2/4] Revert "ASoC: Intel: fix broadwell module removing failed issue"

[Jie Yang]

This reverts commit 01f202c7b4b40025f3ea4721c52e7f78545e3b07.

We shouldn't leave the device as suspended state after module freed,
it is not good to do runtime suspend at driver free, here revert
this fixing, and replace it with the procedure:
suspends firmware ==> frees runtime modules ==> unloads firmware.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 sound/soc/intel/haswell/sst-haswell-pcm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c b/sound/soc/intel/haswell/sst-haswell-pcm.c
index bd96629..23ae040 100644
--- a/sound/soc/intel/haswell/sst-haswell-pcm.c
+++ b/sound/soc/intel/haswell/sst-haswell-pcm.c
@@ -1118,10 +1118,8 @@ static int hsw_pcm_remove(struct snd_soc_platform *platform)
 		snd_soc_platform_get_drvdata(platform);
 	int i;
 
-	/* execute a suspend call to unload all FW resources */
-	if (!pm_runtime_status_suspended(platform->dev))
-		pm_runtime_put_sync_suspend(platform->dev);
 	pm_runtime_disable(platform->dev);
+	hsw_pcm_free_modules(priv_data);
 
 	for (i = 0; i < ARRAY_SIZE(hsw_dais); i++) {
 		if (hsw_dais[i].playback.channels_min)
-- 
1.9.1

[PATCH 3/4] ASoC: Intel: check and clear runtime module pointer

[Jie Yang]

Add check runtime module pointers before freeing them, and clear
them to NULL after freed.

With this implemented, we can avoid NULL pointer dereference or
double free errors.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 sound/soc/intel/haswell/sst-haswell-pcm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c b/sound/soc/intel/haswell/sst-haswell-pcm.c
index 23ae040..f97fa5a 100644
--- a/sound/soc/intel/haswell/sst-haswell-pcm.c
+++ b/sound/soc/intel/haswell/sst-haswell-pcm.c
@@ -928,10 +928,15 @@ static void hsw_pcm_free_modules(struct hsw_priv_data *pdata)
 
 	for (i = 0; i < ARRAY_SIZE(mod_map); i++) {
 		pcm_data = &pdata->pcm[mod_map[i].dai_id][mod_map[i].stream];
-		sst_hsw_runtime_module_free(pcm_data->runtime);
+		if (pcm_data->runtime){
+			sst_hsw_runtime_module_free(pcm_data->runtime);
+			pcm_data->runtime = NULL;
+		}
 	}
-	if (sst_hsw_is_module_loaded(hsw, SST_HSW_MODULE_WAVES)) {
+	if (sst_hsw_is_module_loaded(hsw, SST_HSW_MODULE_WAVES) &&
+				pdata->runtime_waves) {
 		sst_hsw_runtime_module_free(pdata->runtime_waves);
+		pdata->runtime_waves = NULL;
 	}
 }
 
-- 
1.9.1

[PATCH 4/4] ASoC: Intel: handle haswell pcm suspend including runtime modules freeing

[Jie Yang]

It needs free pcm runtime modules before unloading firmware, here
add hsw_pcm_suspend() to handle this procedure:
suspends firmware ==> frees runtime modules ==> unloads firmware.

This fixes the broadwell module unload failed issue.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 sound/soc/intel/haswell/sst-haswell-pcm.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c b/sound/soc/intel/haswell/sst-haswell-pcm.c
index f97fa5a..e593e7a 100644
--- a/sound/soc/intel/haswell/sst-haswell-pcm.c
+++ b/sound/soc/intel/haswell/sst-haswell-pcm.c
@@ -1209,6 +1209,20 @@ static int hsw_pcm_runtime_idle(struct device *dev)
 	return 0;
 }
 
+static int hsw_pcm_suspend(struct device *dev)
+{
+	struct hsw_priv_data *pdata = dev_get_drvdata(dev);
+	struct sst_hsw *hsw = pdata->hsw;
+
+	/* enter D3 state and stall */
+	sst_hsw_dsp_runtime_suspend(hsw);
+	/* free all runtime modules */
+	hsw_pcm_free_modules(pdata);
+	/* put the DSP to sleep, fw unloaded after runtime modules freed */
+	sst_hsw_dsp_runtime_sleep(hsw);
+	return 0;
+}
+
 static int hsw_pcm_runtime_suspend(struct device *dev)
 {
 	struct hsw_priv_data *pdata = dev_get_drvdata(dev);
@@ -1225,8 +1239,7 @@ static int hsw_pcm_runtime_suspend(struct device *dev)
 			return ret;
 		sst_hsw_set_module_enabled_rtd3(hsw, SST_HSW_MODULE_WAVES);
 	}
-	sst_hsw_dsp_runtime_suspend(hsw);
-	sst_hsw_dsp_runtime_sleep(hsw);
+	hsw_pcm_suspend(dev);
 	pdata->pm_state = HSW_PM_STATE_RTD3;
 
 	return 0;
@@ -1366,10 +1379,7 @@ static int hsw_pcm_prepare(struct device *dev)
 			if (err < 0)
 				dev_err(dev, "failed to save context for PCM %d\n", i);
 		}
-		/* enter D3 state and stall */
-		sst_hsw_dsp_runtime_suspend(hsw);
-		/* put the DSP to sleep */
-		sst_hsw_dsp_runtime_sleep(hsw);
+		hsw_pcm_suspend(dev);
 	}
 
 	snd_soc_suspend(pdata->soc_card->dev);
-- 
1.9.1

Re: [PATCH 0/4] new fix for broadwell module removing failed issue

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 280 bytes --]

On Sat, May 30, 2015 at 10:33:55PM +0800, Jie Yang wrote:
> This patch series are based on broonie/sound.git for-next branch, commit
> 01a4d5083562fcdeab99d42b0873aabc5fcca018.

Applied all, thanks.  Please use subject lines matcing the style for the
subsystem, even for reverts.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/4] new fix for broadwell module removing failed issue

[Jie, Yang]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Tuesday, June 02, 2015 10:35 PM
> To: Jie, Yang
> Cc: alsa-devel@alsa-project.org; Girdwood, Liam R
> Subject: Re: [PATCH 0/4] new fix for broadwell module removing failed issue
> 
> On Sat, May 30, 2015 at 10:33:55PM +0800, Jie Yang wrote:
> > This patch series are based on broonie/sound.git for-next branch,
> > commit 01a4d5083562fcdeab99d42b0873aabc5fcca018.
> 
> Applied all, thanks.  Please use subject lines matcing the style for the
> subsystem, even for reverts.
 
Got it, will notice that next time, thanks.

~Keyon