Re: [PATCH] ASoC: soc-pcm: Revert "call snd_soc_component_open/close() once"

[Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>]

Hi Kai, Pierre-Louis

Thank you for your test / review

> ASoC component open/close and snd_soc_component_module_get/put are
> called independently for each component-substream pair, so the logic
> in the reverted patch was not sufficient and led to PCM playback and
> module unload errors.
> 
> Fixes: dd03907bf129 ("ASoC: soc-pcm: call snd_soc_component_open/close() once")
> Signed-off-by: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx>
> ---

OK, some component (SOF, and maybe DPCM too ?) want to be opened for each substreams.
If so, indeed current code is not enough for it.

But, unfortunately I don't want spaghetti error handling code again.
I think we can solve it if we can *count* open / module ?
Can you please test this patch ?

--- 8< --- 8< --- 8< --- 8< --- 8< --- 8< --- 8< --- 8< --- 8< ---
diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 1866ecc8e94b..4e78925858c0 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -181,10 +181,11 @@ struct snd_soc_component {
 	const char *debugfs_prefix;
 #endif
 
+	u8 opened;
+	u8 module;
+
 	/* bit field */
 	unsigned int suspended:1; /* is in suspend PM state */
-	unsigned int opened:1;
-	unsigned int module:1;
 };
 
 #define for_each_component_dais(component, dai)\
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index ee00c09df5e7..a2526a1ffcbe 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -297,14 +297,16 @@ EXPORT_SYMBOL_GPL(snd_soc_component_set_jack);
 int snd_soc_component_module_get(struct snd_soc_component *component,
 				 int upon_open)
 {
-	if (component->module)
-		return 0;
+	if (unlikely(component->module == 0xff)) {
+		dev_warn(component->dev, "too many module get (%s)\n", component->name);
+		return -EBUSY;
+	}
 
 	if (component->driver->module_get_upon_open == !!upon_open &&
 	    !try_module_get(component->dev->driver->owner))
 		return -ENODEV;
 
-	component->module = 1;
+	component->module++;
 
 	return 0;
 }
@@ -316,7 +318,7 @@ void snd_soc_component_module_put(struct snd_soc_component *component,
 	    component->driver->module_get_upon_open == !!upon_open)
 		module_put(component->dev->driver->owner);
 
-	component->module = 0;
+	component->module--;
 }
 
 int snd_soc_component_open(struct snd_soc_component *component,
@@ -324,12 +326,15 @@ int snd_soc_component_open(struct snd_soc_component *component,
 {
 	int ret = 0;
 
-	if (!component->opened &&
-	    component->driver->open)
+	if (unlikely(component->opened == 0xff)) {
+		dev_warn(component->dev, "too many open (%s)\n", component->name);
+		return -EBUSY;
+	}
+
+	if (component->driver->open)
 		ret = component->driver->open(component, substream);
 
-	if (ret == 0)
-		component->opened = 1;
+	component->opened++;
 
 	return ret;
 }
@@ -343,7 +348,7 @@ int snd_soc_component_close(struct snd_soc_component *component,
 	    component->driver->close)
 		ret = component->driver->close(component, substream);
 
-	component->opened = 0;
+	component->opened--;
 
 	return ret;
 }


Thank you for your help !!
Best regards
---
Kuninori Morimoto