Skip to contentSkip to conversation

ASoC: compress: Only call free for components which have been opened

PATCH In review Charles Keepax 3 participants 12 messages Apr 24, 2018 – Apr 26, 2018

Subsystems (3)
Sound ASoC Sound - compressed audio
lore
Share & tools
lore

Charles Keepax

author
#0 lore raw

PATCH 1/3 ASoC: compress: Only call free for components which have been opened

The core should only call free on a component if said component has already had open called on it. This is not presently the case and most compressed drivers in the kernel assume it will be. This causes null pointer dereferences in the drivers as they attempt clean up for stuff that was never put in place.

This is fixed by aborting calling open callbacks once a failure is encountered and then during clean up only iterating through the component list to that point.

This is a fairly quick fix to the issue, to allow backporting. There is more refactoring to follow to tidy the code up a little.

Fixes 9e7e3738ab0e ("ASoC: snd_soc_component_driver has snd_compr_ops")
Signed-off-by Charles Keepax <[email protected]>
---
 sound/soc/soc-compress.c | 52 ++++++++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 24 deletions(-)
sound/soc/soc-compress.c +28 -24
index 82402688bd8e..948505f74229 100644
@@ -33,7 +33,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 

@@ -68,16 +68,15 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 		    !component->driver->compr_ops->open)
 			continue;
 
-		__ret = component->driver->compr_ops->open(cstream);
-		if (__ret < 0) {
+		ret = component->driver->compr_ops->open(cstream);
+		if (ret < 0) {
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, __ret);
-			ret = __ret;
+				component->name, ret);
+			goto machine_err;
 		}
 	}
-	if (ret < 0)
-		goto machine_err;
+	component = NULL;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
 		ret = rtd->dai_link->compr_ops->startup(cstream);

@@ -97,17 +96,20 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 
 machine_err:
 	for_each_rtdcom(rtd, rtdcom) {
-		component = rtdcom->component;
+		struct snd_soc_component *err_comp = rtdcom->component;
+
+		if (err_comp == component)
+			break;
 
 		/* ignore duplication for now */
-		if (platform && (component == &platform->component))
+		if (platform && (err_comp == &platform->component))
 			continue;
 
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
+		if (!err_comp->driver->compr_ops ||
+		    !err_comp->driver->compr_ops->free)
 			continue;
 
-		component->driver->compr_ops->free(cstream);
+		err_comp->driver->compr_ops->free(cstream);
 	}
 
 	if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)

@@ -132,7 +134,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	struct snd_soc_dpcm *dpcm;
 	struct snd_soc_dapm_widget_list *list;
 	int stream;
-	int ret = 0, __ret;
+	int ret;
 
 	if (cstream->direction == SND_COMPRESS_PLAYBACK)
 		stream = SNDRV_PCM_STREAM_PLAYBACK;

@@ -172,16 +174,15 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		    !component->driver->compr_ops->open)
 			continue;
 
-		__ret = component->driver->compr_ops->open(cstream);
-		if (__ret < 0) {
+		ret = component->driver->compr_ops->open(cstream);
+		if (ret < 0) {
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, __ret);
-			ret = __ret;
+				component->name, ret);
+			goto machine_err;
 		}
 	}
-	if (ret < 0)
-		goto machine_err;
+	component = NULL;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) {
 		ret = fe->dai_link->compr_ops->startup(cstream);

@@ -236,17 +237,20 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		fe->dai_link->compr_ops->shutdown(cstream);
 machine_err:
 	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
+		struct snd_soc_component *err_comp = rtdcom->component;
+
+		if (err_comp == component)
+			break;
 
 		/* ignore duplication for now */
-		if (platform && (component == &platform->component))
+		if (platform && (err_comp == &platform->component))
 			continue;
 
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
+		if (!err_comp->driver->compr_ops ||
+		    !err_comp->driver->compr_ops->free)
 			continue;
 
-		component->driver->compr_ops->free(cstream);
+		err_comp->driver->compr_ops->free(cstream);
 	}
 
 	if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)
Signature · 1 line
2.11.0
author
#1 lore raw

PATCH 2/3 ASoC: Remove platform code now everything is componentised

As all drivers have been moved over to the new generic component code remove the now unused platform specific code.

Signed-off-by Charles Keepax <[email protected]>
---
 include/sound/soc.h      | 103 +--------------------
 sound/soc/soc-compress.c | 227 ++---------------------------------------------
 sound/soc/soc-core.c     | 214 --------------------------------------------
 sound/soc/soc-devres.c   |  35 --------
 sound/soc/soc-io.c       |  21 -----
 sound/soc/soc-pcm.c      | 119 +------------------------
 6 files changed, 12 insertions(+), 707 deletions(-)
include/sound/soc.h +3 -100
index ad266d7e9553..9ea99e5d3c8e 100644
@@ -401,9 +401,7 @@ struct snd_soc_ops;
 struct snd_soc_pcm_runtime;
 struct snd_soc_dai;
 struct snd_soc_dai_driver;
-struct snd_soc_platform;
 struct snd_soc_dai_link;
-struct snd_soc_platform_driver;
 struct snd_soc_codec;
 struct snd_soc_codec_driver;
 struct snd_soc_component;

@@ -455,15 +453,6 @@ static inline int snd_soc_resume(struct device *dev)
 }
 #endif
 int snd_soc_poweroff(struct device *dev);
-int snd_soc_register_platform(struct device *dev,
-		const struct snd_soc_platform_driver *platform_drv);
-int devm_snd_soc_register_platform(struct device *dev,
-		const struct snd_soc_platform_driver *platform_drv);
-void snd_soc_unregister_platform(struct device *dev);
-int snd_soc_add_platform(struct device *dev, struct snd_soc_platform *platform,
-		const struct snd_soc_platform_driver *platform_drv);
-void snd_soc_remove_platform(struct snd_soc_platform *platform);
-struct snd_soc_platform *snd_soc_lookup_platform(struct device *dev);
 int snd_soc_register_codec(struct device *dev,
 		const struct snd_soc_codec_driver *codec_drv,
 		struct snd_soc_dai_driver *dai_drv, int num_dai);

@@ -485,10 +474,6 @@ struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
 int snd_soc_cache_init(struct snd_soc_codec *codec);
 int snd_soc_cache_exit(struct snd_soc_codec *codec);
 
-int snd_soc_platform_read(struct snd_soc_platform *platform,
-					unsigned int reg);
-int snd_soc_platform_write(struct snd_soc_platform *platform,
-					unsigned int reg, unsigned int val);
 int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num);
 #ifdef CONFIG_SND_SOC_COMPRESS
 int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num);

@@ -628,8 +613,6 @@ int snd_soc_add_component_controls(struct snd_soc_component *component,
 	const struct snd_kcontrol_new *controls, unsigned int num_controls);
 int snd_soc_add_codec_controls(struct snd_soc_codec *codec,
 	const struct snd_kcontrol_new *controls, unsigned int num_controls);
-int snd_soc_add_platform_controls(struct snd_soc_platform *platform,
-	const struct snd_kcontrol_new *controls, unsigned int num_controls);
 int snd_soc_add_card_controls(struct snd_soc_card *soc_card,
 	const struct snd_kcontrol_new *controls, int num_controls);
 int snd_soc_add_dai_controls(struct snd_soc_dai *dai,

@@ -996,39 +979,12 @@ struct snd_soc_codec_driver {
 	bool ignore_pmdown_time;  /* Doesn't benefit from pmdown delay */
 };
 
-/* SoC platform interface */
-struct snd_soc_platform_driver {
-
-	int (*probe)(struct snd_soc_platform *);
-	int (*remove)(struct snd_soc_platform *);
-	struct snd_soc_component_driver component_driver;
-
-	/* pcm creation and destruction */
-	int (*pcm_new)(struct snd_soc_pcm_runtime *);
-	void (*pcm_free)(struct snd_pcm *);
-
-	/* platform stream pcm ops */
-	const struct snd_pcm_ops *ops;
-
-	/* platform stream compress ops */
-	const struct snd_compr_ops *compr_ops;
-};
-
 struct snd_soc_dai_link_component {
 	const char *name;
 	struct device_node *of_node;
 	const char *dai_name;
 };
 
-struct snd_soc_platform {
-	struct device *dev;
-	const struct snd_soc_platform_driver *driver;
-
-	struct list_head list;
-
-	struct snd_soc_component component;
-};
-
 struct snd_soc_dai_link {
 	/* config - must be set by machine driver */
 	const char *name;			/* Codec name */

@@ -1277,7 +1233,6 @@ struct snd_soc_pcm_runtime {
 	struct snd_pcm *pcm;
 	struct snd_compr *compr;
 	struct snd_soc_codec *codec;
-	struct snd_soc_platform *platform; /* will be removed */
 	struct snd_soc_dai *codec_dai;
 	struct snd_soc_dai *cpu_dai;
 

@@ -1359,19 +1314,6 @@ static inline struct snd_soc_codec *snd_soc_component_to_codec(
 }
 
 /**
- * snd_soc_component_to_platform() - Casts a component to the platform it is embedded in
- * @component: The component to cast to a platform
- *
- * This function must only be used on components that are known to be platforms.
- * Otherwise the behavior is undefined.
- */
-static inline struct snd_soc_platform *snd_soc_component_to_platform(
-	struct snd_soc_component *component)
-{
-	return container_of(component, struct snd_soc_platform, component);
-}
-
-/**
  * snd_soc_dapm_to_component() - Casts a DAPM context to the component it is
  *  embedded in
  * @dapm: The DAPM context to cast to the component

@@ -1400,20 +1342,6 @@ static inline struct snd_soc_codec *snd_soc_dapm_to_codec(
 }
 
 /**
- * snd_soc_dapm_to_platform() - Casts a DAPM context to the platform it is
- *  embedded in
- * @dapm: The DAPM context to cast to the platform.
- *
- * This function must only be used on DAPM contexts that are known to be part of
- * a platform (e.g. in a platform driver). Otherwise the behavior is undefined.
- */
-static inline struct snd_soc_platform *snd_soc_dapm_to_platform(
-	struct snd_soc_dapm_context *dapm)
-{
-	return snd_soc_component_to_platform(snd_soc_dapm_to_component(dapm));
-}
-
-/**
  * snd_soc_component_get_dapm() - Returns the DAPM context associated with a
  *  component
  * @component: The component for which to get the DAPM context

@@ -1673,17 +1601,6 @@ static inline void *snd_soc_codec_get_drvdata(struct snd_soc_codec *codec)
 	return snd_soc_component_get_drvdata(&codec->component);
 }
 
-static inline void snd_soc_platform_set_drvdata(struct snd_soc_platform *platform,
-		void *data)
-{
-	snd_soc_component_set_drvdata(&platform->component, data);
-}
-
-static inline void *snd_soc_platform_get_drvdata(struct snd_soc_platform *platform)
-{
-	return snd_soc_component_get_drvdata(&platform->component);
-}
-
 static inline void snd_soc_initialize_card_lists(struct snd_soc_card *card)
 {
 	INIT_LIST_HEAD(&card->widgets);

@@ -1746,9 +1663,9 @@ static inline bool snd_soc_codec_is_active(struct snd_soc_codec *codec)
  * @kcontrol: The control for which to get the component
  *
  * Note: This function will work correctly if the control has been registered
- * for a component. Either with snd_soc_add_codec_controls() or
- * snd_soc_add_platform_controls() or via  table based setup for either a
- * CODEC, a platform or component driver. Otherwise the behavior is undefined.
+ * for a component. With snd_soc_add_codec_controls() or via table based
+ * setup for either a CODEC or component driver. Otherwise the behavior is
+ * undefined.
  */
 static inline struct snd_soc_component *snd_soc_kcontrol_component(
 	struct snd_kcontrol *kcontrol)

@@ -1770,20 +1687,6 @@ static inline struct snd_soc_codec *snd_soc_kcontrol_codec(
 	return snd_soc_component_to_codec(snd_soc_kcontrol_component(kcontrol));
 }
 
-/**
- * snd_soc_kcontrol_platform() - Returns the platform that registered the control
- * @kcontrol: The control for which to get the platform
- *
- * Note: This function will only work correctly if the control has been
- * registered with snd_soc_add_platform_controls() or via table based setup of
- * a snd_soc_platform_driver. Otherwise the behavior is undefined.
- */
-static inline struct snd_soc_platform *snd_soc_kcontrol_platform(
-	struct snd_kcontrol *kcontrol)
-{
-	return snd_soc_component_to_platform(snd_soc_kcontrol_component(kcontrol));
-}
-
 int snd_soc_util_init(void);
 void snd_soc_util_exit(void);
sound/soc/soc-compress.c +6 -221

548 lines not on this page · raw message

sound/soc/soc-core.c +0 -214

256 lines not on this page · raw message

sound/soc/soc-devres.c +0 -35
index a57921eeee81..7ac745df1412 100644
@@ -52,41 +52,6 @@ int devm_snd_soc_register_component(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_snd_soc_register_component);
 
-static void devm_platform_release(struct device *dev, void *res)
-{
-	snd_soc_unregister_platform(*(struct device **)res);
-}
-
-/**
- * devm_snd_soc_register_platform - resource managed platform registration
- * @dev: Device used to manage platform
- * @platform_drv: platform to register
- *
- * Register a platform driver with automatic unregistration when the device is
- * unregistered.
- */
-int devm_snd_soc_register_platform(struct device *dev,
-			const struct snd_soc_platform_driver *platform_drv)
-{
-	struct device **ptr;
-	int ret;
-
-	ptr = devres_alloc(devm_platform_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr)
-		return -ENOMEM;
-
-	ret = snd_soc_register_platform(dev, platform_drv);
-	if (ret == 0) {
-		*ptr = dev;
-		devres_add(dev, ptr);
-	} else {
-		devres_free(ptr);
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(devm_snd_soc_register_platform);
-
 static void devm_card_release(struct device *dev, void *res)
 {
 	snd_soc_unregister_card(*(struct snd_soc_card **)res);
sound/soc/soc-io.c +0 -21
index d36a192fbece..c92a04bac3c5 100644
@@ -267,24 +267,3 @@ int snd_soc_test_bits(struct snd_soc_codec *codec, unsigned int reg,
 	return snd_soc_component_test_bits(&codec->component, reg, mask, value);
 }
 EXPORT_SYMBOL_GPL(snd_soc_test_bits);
-
-int snd_soc_platform_read(struct snd_soc_platform *platform,
-					unsigned int reg)
-{
-	unsigned int val;
-	int ret;
-
-	ret = snd_soc_component_read(&platform->component, reg, &val);
-	if (ret < 0)
-		return -1;
-
-	return val;
-}
-EXPORT_SYMBOL_GPL(snd_soc_platform_read);
-
-int snd_soc_platform_write(struct snd_soc_platform *platform,
-					 unsigned int reg, unsigned int val)
-{
-	return snd_soc_component_write(&platform->component, reg, val);
-}
-EXPORT_SYMBOL_GPL(snd_soc_platform_write);
sound/soc/soc-pcm.c +3 -116

290 lines not on this page · raw message

Signature · 1 line
2.11.0
Acked-by on v1 2/3
#2 lore raw

Re: [PATCH 2/3] ASoC: Remove platform code now everything is componentised

Quoted 7 lines · Charles Keepax

As all drivers have been moved over to the new generic component code remove the now unused platform specific code.

Signed-off-by Charles Keepax <[email protected]>
---
 include/sound/soc.h      | 103 +--------------------
 sound/soc/soc-compress.c | 227 ++---------------------------------------------

For compress:

Acked-by Vinod Koul <[email protected]>
Signature · 1 line
~Vinod
author
#3 lore raw

PATCH 3/3 ASoC: compress: Add helper functions for component open/free

In reply to Charles Keepax

There are 2 loops calling open and 4 loops calling free for all the components on a DAI link. Factor out these loops into helper functions to make the code a little clearer.

Signed-off-by Charles Keepax <[email protected]>

---

I am looking at refactoring some of the other callbacks as well, but there is quite a lot to think about. So thought I would get these out for people to look at whilst I work out what makes sense for the rest.

Thanks,
Charles

 sound/soc/soc-compress.c | 141 +++++++++++++++++++++--------------------------
 1 file changed, 62 insertions(+), 79 deletions(-)
sound/soc/soc-compress.c +62 -79
index abc00c6cc2d7..ba56f87f96d4 100644
@@ -26,26 +26,14 @@
 #include <sound/initval.h>
 #include <sound/soc-dpcm.h>
 
-static int soc_compr_open(struct snd_compr_stream *cstream)
+static int soc_compr_components_open(struct snd_compr_stream *cstream,
+				     struct snd_soc_component **last)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
-	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	int ret;
 
-	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
-
-	if (cpu_dai->driver->cops && cpu_dai->driver->cops->startup) {
-		ret = cpu_dai->driver->cops->startup(cstream, cpu_dai);
-		if (ret < 0) {
-			dev_err(cpu_dai->dev,
-				"Compress ASoC: can't open interface %s: %d\n",
-				cpu_dai->name, ret);
-			goto out;
-		}
-	}
-
 	for_each_rtdcom(rtd, rtdcom) {
 		component = rtdcom->component;
 

@@ -58,10 +46,61 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
 				component->name, ret);
-			goto machine_err;
+
+			*last = component;
+			return ret;
+		}
+	}
+
+	*last = NULL;
+	return 0;
+}
+
+static int soc_compr_components_free(struct snd_compr_stream *cstream,
+				     struct snd_soc_component *last)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_component *component;
+	struct snd_soc_rtdcom_list *rtdcom;
+
+	for_each_rtdcom(rtd, rtdcom) {
+		component = rtdcom->component;
+
+		if (component == last)
+			break;
+
+		if (!component->driver->compr_ops ||
+		    !component->driver->compr_ops->free)
+			continue;
+
+		component->driver->compr_ops->free(cstream);
+	}
+
+	return 0;
+}
+
+static int soc_compr_open(struct snd_compr_stream *cstream)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_component *component;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	int ret;
+
+	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
+
+	if (cpu_dai->driver->cops && cpu_dai->driver->cops->startup) {
+		ret = cpu_dai->driver->cops->startup(cstream, cpu_dai);
+		if (ret < 0) {
+			dev_err(cpu_dai->dev,
+				"Compress ASoC: can't open interface %s: %d\n",
+				cpu_dai->name, ret);
+			goto out;
 		}
 	}
-	component = NULL;
+
+	ret = soc_compr_components_open(cstream, &component);
+	if (ret < 0)
+		goto machine_err;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
 		ret = rtd->dai_link->compr_ops->startup(cstream);

@@ -80,18 +119,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	return 0;
 
 machine_err:
-	for_each_rtdcom(rtd, rtdcom) {
-		struct snd_soc_component *err_comp = rtdcom->component;
-
-		if (err_comp == component)
-			break;
-
-		if (!err_comp->driver->compr_ops ||
-		    !err_comp->driver->compr_ops->free)
-			continue;
-
-		err_comp->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, component);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -106,7 +134,6 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	struct snd_pcm_substream *fe_substream =
 		 fe->pcm->streams[cstream->direction].substream;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	struct snd_soc_dpcm *dpcm;
 	struct snd_soc_dapm_widget_list *list;

@@ -130,22 +157,9 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		}
 	}
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->open)
-			continue;
-
-		ret = component->driver->compr_ops->open(cstream);
-		if (ret < 0) {
-			dev_err(component->dev,
-				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, ret);
-			goto machine_err;
-		}
-	}
-	component = NULL;
+	ret = soc_compr_components_open(cstream, &component);
+	if (ret < 0)
+		goto machine_err;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) {
 		ret = fe->dai_link->compr_ops->startup(cstream);

@@ -199,18 +213,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown)
 		fe->dai_link->compr_ops->shutdown(cstream);
 machine_err:
-	for_each_rtdcom(fe, rtdcom) {
-		struct snd_soc_component *err_comp = rtdcom->component;
-
-		if (err_comp == component)
-			break;
-
-		if (!err_comp->driver->compr_ops ||
-		    !err_comp->driver->compr_ops->free)
-			continue;
-
-		err_comp->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, component);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -252,8 +255,6 @@ static void close_delayed_work(struct work_struct *work)
 static int soc_compr_free(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	int stream;

@@ -278,15 +279,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->shutdown)
 		rtd->dai_link->compr_ops->shutdown(cstream);
 
-	for_each_rtdcom(rtd, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
-			continue;
-
-		component->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, NULL);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -316,8 +309,6 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	struct snd_soc_dpcm *dpcm;
 	int stream, ret;

@@ -355,15 +346,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown)
 		fe->dai_link->compr_ops->shutdown(cstream);
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
-			continue;
-
-		component->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, NULL);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);
Signature · 1 line
2.11.0
Acked-by on v1 3/3
#4 lore raw

Re: [PATCH 3/3] ASoC: compress: Add helper functions for component open/free

Quoted 3 lines · Charles Keepax

There are 2 loops calling open and 4 loops calling free for all the components on a DAI link. Factor out these loops into helper functions to make the code a little clearer.

Acked-by Vinod Koul <[email protected]>
Signature · 1 line
~Vinod
maintainer
#5 lore raw

Applied "ASoC: compress: Add helper functions for component open/free" to the asoc tree

In reply to Charles Keepax

The patch

ASoC: compress: Add helper functions for component open/free

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 1e57b82891ade3fd71c030077901808a6e5376ab Mon Sep 17 00:00:00 2001

From: Charles Keepax <[email protected]>
Date: Tue, 24 Apr 2018 16:39:03 +0100
Subject: [PATCH] ASoC: compress: Add helper functions for component open/free

There are 2 loops calling open and 4 loops calling free for all the components on a DAI link. Factor out these loops into helper functions to make the code a little clearer.

Signed-off-by Charles Keepax <[email protected]>
Acked-by Vinod Koul <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/soc-compress.c | 141 +++++++++++++++++----------------------
 1 file changed, 62 insertions(+), 79 deletions(-)
sound/soc/soc-compress.c +62 -79
index abc00c6cc2d7..ba56f87f96d4 100644
@@ -26,26 +26,14 @@
 #include <sound/initval.h>
 #include <sound/soc-dpcm.h>
 
-static int soc_compr_open(struct snd_compr_stream *cstream)
+static int soc_compr_components_open(struct snd_compr_stream *cstream,
+				     struct snd_soc_component **last)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
-	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	int ret;
 
-	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
-
-	if (cpu_dai->driver->cops && cpu_dai->driver->cops->startup) {
-		ret = cpu_dai->driver->cops->startup(cstream, cpu_dai);
-		if (ret < 0) {
-			dev_err(cpu_dai->dev,
-				"Compress ASoC: can't open interface %s: %d\n",
-				cpu_dai->name, ret);
-			goto out;
-		}
-	}
-
 	for_each_rtdcom(rtd, rtdcom) {
 		component = rtdcom->component;
 

@@ -58,10 +46,61 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
 				component->name, ret);
-			goto machine_err;
+
+			*last = component;
+			return ret;
+		}
+	}
+
+	*last = NULL;
+	return 0;
+}
+
+static int soc_compr_components_free(struct snd_compr_stream *cstream,
+				     struct snd_soc_component *last)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_component *component;
+	struct snd_soc_rtdcom_list *rtdcom;
+
+	for_each_rtdcom(rtd, rtdcom) {
+		component = rtdcom->component;
+
+		if (component == last)
+			break;
+
+		if (!component->driver->compr_ops ||
+		    !component->driver->compr_ops->free)
+			continue;
+
+		component->driver->compr_ops->free(cstream);
+	}
+
+	return 0;
+}
+
+static int soc_compr_open(struct snd_compr_stream *cstream)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_component *component;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	int ret;
+
+	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
+
+	if (cpu_dai->driver->cops && cpu_dai->driver->cops->startup) {
+		ret = cpu_dai->driver->cops->startup(cstream, cpu_dai);
+		if (ret < 0) {
+			dev_err(cpu_dai->dev,
+				"Compress ASoC: can't open interface %s: %d\n",
+				cpu_dai->name, ret);
+			goto out;
 		}
 	}
-	component = NULL;
+
+	ret = soc_compr_components_open(cstream, &component);
+	if (ret < 0)
+		goto machine_err;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
 		ret = rtd->dai_link->compr_ops->startup(cstream);

@@ -80,18 +119,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	return 0;
 
 machine_err:
-	for_each_rtdcom(rtd, rtdcom) {
-		struct snd_soc_component *err_comp = rtdcom->component;
-
-		if (err_comp == component)
-			break;
-
-		if (!err_comp->driver->compr_ops ||
-		    !err_comp->driver->compr_ops->free)
-			continue;
-
-		err_comp->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, component);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -106,7 +134,6 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	struct snd_pcm_substream *fe_substream =
 		 fe->pcm->streams[cstream->direction].substream;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	struct snd_soc_dpcm *dpcm;
 	struct snd_soc_dapm_widget_list *list;

@@ -130,22 +157,9 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		}
 	}
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->open)
-			continue;
-
-		ret = component->driver->compr_ops->open(cstream);
-		if (ret < 0) {
-			dev_err(component->dev,
-				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, ret);
-			goto machine_err;
-		}
-	}
-	component = NULL;
+	ret = soc_compr_components_open(cstream, &component);
+	if (ret < 0)
+		goto machine_err;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) {
 		ret = fe->dai_link->compr_ops->startup(cstream);

@@ -199,18 +213,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown)
 		fe->dai_link->compr_ops->shutdown(cstream);
 machine_err:
-	for_each_rtdcom(fe, rtdcom) {
-		struct snd_soc_component *err_comp = rtdcom->component;
-
-		if (err_comp == component)
-			break;
-
-		if (!err_comp->driver->compr_ops ||
-		    !err_comp->driver->compr_ops->free)
-			continue;
-
-		err_comp->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, component);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -252,8 +255,6 @@ static void close_delayed_work(struct work_struct *work)
 static int soc_compr_free(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	int stream;

@@ -278,15 +279,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->shutdown)
 		rtd->dai_link->compr_ops->shutdown(cstream);
 
-	for_each_rtdcom(rtd, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
-			continue;
-
-		component->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, NULL);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -316,8 +309,6 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	struct snd_soc_dpcm *dpcm;
 	int stream, ret;

@@ -355,15 +346,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown)
 		fe->dai_link->compr_ops->shutdown(cstream);
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
-			continue;
-
-		component->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, NULL);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);
Signature · 1 line
2.17.0
maintainer
#6 lore raw

Applied "ASoC: compress: Add helper functions for component open/free" to the asoc tree

In reply to Charles Keepax

The patch

ASoC: compress: Add helper functions for component open/free

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 1e57b82891ade3fd71c030077901808a6e5376ab Mon Sep 17 00:00:00 2001

From: Charles Keepax <[email protected]>
Date: Tue, 24 Apr 2018 16:39:03 +0100
Subject: [PATCH] ASoC: compress: Add helper functions for component open/free

There are 2 loops calling open and 4 loops calling free for all the components on a DAI link. Factor out these loops into helper functions to make the code a little clearer.

Signed-off-by Charles Keepax <[email protected]>
Acked-by Vinod Koul <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/soc-compress.c | 141 +++++++++++++++++----------------------
 1 file changed, 62 insertions(+), 79 deletions(-)
sound/soc/soc-compress.c +62 -79
index abc00c6cc2d7..ba56f87f96d4 100644
@@ -26,26 +26,14 @@
 #include <sound/initval.h>
 #include <sound/soc-dpcm.h>
 
-static int soc_compr_open(struct snd_compr_stream *cstream)
+static int soc_compr_components_open(struct snd_compr_stream *cstream,
+				     struct snd_soc_component **last)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
-	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	int ret;
 
-	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
-
-	if (cpu_dai->driver->cops && cpu_dai->driver->cops->startup) {
-		ret = cpu_dai->driver->cops->startup(cstream, cpu_dai);
-		if (ret < 0) {
-			dev_err(cpu_dai->dev,
-				"Compress ASoC: can't open interface %s: %d\n",
-				cpu_dai->name, ret);
-			goto out;
-		}
-	}
-
 	for_each_rtdcom(rtd, rtdcom) {
 		component = rtdcom->component;
 

@@ -58,10 +46,61 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
 				component->name, ret);
-			goto machine_err;
+
+			*last = component;
+			return ret;
+		}
+	}
+
+	*last = NULL;
+	return 0;
+}
+
+static int soc_compr_components_free(struct snd_compr_stream *cstream,
+				     struct snd_soc_component *last)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_component *component;
+	struct snd_soc_rtdcom_list *rtdcom;
+
+	for_each_rtdcom(rtd, rtdcom) {
+		component = rtdcom->component;
+
+		if (component == last)
+			break;
+
+		if (!component->driver->compr_ops ||
+		    !component->driver->compr_ops->free)
+			continue;
+
+		component->driver->compr_ops->free(cstream);
+	}
+
+	return 0;
+}
+
+static int soc_compr_open(struct snd_compr_stream *cstream)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_component *component;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	int ret;
+
+	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
+
+	if (cpu_dai->driver->cops && cpu_dai->driver->cops->startup) {
+		ret = cpu_dai->driver->cops->startup(cstream, cpu_dai);
+		if (ret < 0) {
+			dev_err(cpu_dai->dev,
+				"Compress ASoC: can't open interface %s: %d\n",
+				cpu_dai->name, ret);
+			goto out;
 		}
 	}
-	component = NULL;
+
+	ret = soc_compr_components_open(cstream, &component);
+	if (ret < 0)
+		goto machine_err;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
 		ret = rtd->dai_link->compr_ops->startup(cstream);

@@ -80,18 +119,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	return 0;
 
 machine_err:
-	for_each_rtdcom(rtd, rtdcom) {
-		struct snd_soc_component *err_comp = rtdcom->component;
-
-		if (err_comp == component)
-			break;
-
-		if (!err_comp->driver->compr_ops ||
-		    !err_comp->driver->compr_ops->free)
-			continue;
-
-		err_comp->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, component);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -106,7 +134,6 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	struct snd_pcm_substream *fe_substream =
 		 fe->pcm->streams[cstream->direction].substream;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	struct snd_soc_dpcm *dpcm;
 	struct snd_soc_dapm_widget_list *list;

@@ -130,22 +157,9 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		}
 	}
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->open)
-			continue;
-
-		ret = component->driver->compr_ops->open(cstream);
-		if (ret < 0) {
-			dev_err(component->dev,
-				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, ret);
-			goto machine_err;
-		}
-	}
-	component = NULL;
+	ret = soc_compr_components_open(cstream, &component);
+	if (ret < 0)
+		goto machine_err;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) {
 		ret = fe->dai_link->compr_ops->startup(cstream);

@@ -199,18 +213,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown)
 		fe->dai_link->compr_ops->shutdown(cstream);
 machine_err:
-	for_each_rtdcom(fe, rtdcom) {
-		struct snd_soc_component *err_comp = rtdcom->component;
-
-		if (err_comp == component)
-			break;
-
-		if (!err_comp->driver->compr_ops ||
-		    !err_comp->driver->compr_ops->free)
-			continue;
-
-		err_comp->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, component);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -252,8 +255,6 @@ static void close_delayed_work(struct work_struct *work)
 static int soc_compr_free(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	int stream;

@@ -278,15 +279,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->shutdown)
 		rtd->dai_link->compr_ops->shutdown(cstream);
 
-	for_each_rtdcom(rtd, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
-			continue;
-
-		component->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, NULL);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -316,8 +309,6 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	struct snd_soc_dpcm *dpcm;
 	int stream, ret;

@@ -355,15 +346,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown)
 		fe->dai_link->compr_ops->shutdown(cstream);
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
-			continue;
-
-		component->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, NULL);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);
Signature · 1 line
2.17.0
maintainer
#7 lore raw

Applied "ASoC: compress: Add helper functions for component open/free" to the asoc tree

In reply to Charles Keepax

The patch

ASoC: compress: Add helper functions for component open/free

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 1e57b82891ade3fd71c030077901808a6e5376ab Mon Sep 17 00:00:00 2001

From: Charles Keepax <[email protected]>
Date: Tue, 24 Apr 2018 16:39:03 +0100
Subject: [PATCH] ASoC: compress: Add helper functions for component open/free

There are 2 loops calling open and 4 loops calling free for all the components on a DAI link. Factor out these loops into helper functions to make the code a little clearer.

Signed-off-by Charles Keepax <[email protected]>
Acked-by Vinod Koul <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/soc-compress.c | 141 +++++++++++++++++----------------------
 1 file changed, 62 insertions(+), 79 deletions(-)
sound/soc/soc-compress.c +62 -79
index abc00c6cc2d7..ba56f87f96d4 100644
@@ -26,26 +26,14 @@
 #include <sound/initval.h>
 #include <sound/soc-dpcm.h>
 
-static int soc_compr_open(struct snd_compr_stream *cstream)
+static int soc_compr_components_open(struct snd_compr_stream *cstream,
+				     struct snd_soc_component **last)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
-	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	int ret;
 
-	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
-
-	if (cpu_dai->driver->cops && cpu_dai->driver->cops->startup) {
-		ret = cpu_dai->driver->cops->startup(cstream, cpu_dai);
-		if (ret < 0) {
-			dev_err(cpu_dai->dev,
-				"Compress ASoC: can't open interface %s: %d\n",
-				cpu_dai->name, ret);
-			goto out;
-		}
-	}
-
 	for_each_rtdcom(rtd, rtdcom) {
 		component = rtdcom->component;
 

@@ -58,10 +46,61 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
 				component->name, ret);
-			goto machine_err;
+
+			*last = component;
+			return ret;
+		}
+	}
+
+	*last = NULL;
+	return 0;
+}
+
+static int soc_compr_components_free(struct snd_compr_stream *cstream,
+				     struct snd_soc_component *last)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_component *component;
+	struct snd_soc_rtdcom_list *rtdcom;
+
+	for_each_rtdcom(rtd, rtdcom) {
+		component = rtdcom->component;
+
+		if (component == last)
+			break;
+
+		if (!component->driver->compr_ops ||
+		    !component->driver->compr_ops->free)
+			continue;
+
+		component->driver->compr_ops->free(cstream);
+	}
+
+	return 0;
+}
+
+static int soc_compr_open(struct snd_compr_stream *cstream)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_component *component;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	int ret;
+
+	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
+
+	if (cpu_dai->driver->cops && cpu_dai->driver->cops->startup) {
+		ret = cpu_dai->driver->cops->startup(cstream, cpu_dai);
+		if (ret < 0) {
+			dev_err(cpu_dai->dev,
+				"Compress ASoC: can't open interface %s: %d\n",
+				cpu_dai->name, ret);
+			goto out;
 		}
 	}
-	component = NULL;
+
+	ret = soc_compr_components_open(cstream, &component);
+	if (ret < 0)
+		goto machine_err;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
 		ret = rtd->dai_link->compr_ops->startup(cstream);

@@ -80,18 +119,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	return 0;
 
 machine_err:
-	for_each_rtdcom(rtd, rtdcom) {
-		struct snd_soc_component *err_comp = rtdcom->component;
-
-		if (err_comp == component)
-			break;
-
-		if (!err_comp->driver->compr_ops ||
-		    !err_comp->driver->compr_ops->free)
-			continue;
-
-		err_comp->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, component);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -106,7 +134,6 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	struct snd_pcm_substream *fe_substream =
 		 fe->pcm->streams[cstream->direction].substream;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	struct snd_soc_dpcm *dpcm;
 	struct snd_soc_dapm_widget_list *list;

@@ -130,22 +157,9 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		}
 	}
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->open)
-			continue;
-
-		ret = component->driver->compr_ops->open(cstream);
-		if (ret < 0) {
-			dev_err(component->dev,
-				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, ret);
-			goto machine_err;
-		}
-	}
-	component = NULL;
+	ret = soc_compr_components_open(cstream, &component);
+	if (ret < 0)
+		goto machine_err;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) {
 		ret = fe->dai_link->compr_ops->startup(cstream);

@@ -199,18 +213,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown)
 		fe->dai_link->compr_ops->shutdown(cstream);
 machine_err:
-	for_each_rtdcom(fe, rtdcom) {
-		struct snd_soc_component *err_comp = rtdcom->component;
-
-		if (err_comp == component)
-			break;
-
-		if (!err_comp->driver->compr_ops ||
-		    !err_comp->driver->compr_ops->free)
-			continue;
-
-		err_comp->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, component);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -252,8 +255,6 @@ static void close_delayed_work(struct work_struct *work)
 static int soc_compr_free(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	int stream;

@@ -278,15 +279,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->shutdown)
 		rtd->dai_link->compr_ops->shutdown(cstream);
 
-	for_each_rtdcom(rtd, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
-			continue;
-
-		component->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, NULL);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);

@@ -316,8 +309,6 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	struct snd_soc_dpcm *dpcm;
 	int stream, ret;

@@ -355,15 +346,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->shutdown)
 		fe->dai_link->compr_ops->shutdown(cstream);
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
-			continue;
-
-		component->driver->compr_ops->free(cstream);
-	}
+	soc_compr_components_free(cstream, NULL);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);
Signature · 1 line
2.17.0
Acked-by on v1 1/3
#8 lore raw
In reply to Charles Keepax
Quoted 9 lines · Charles Keepax

The core should only call free on a component if said component has already had open called on it. This is not presently the case and most compressed drivers in the kernel assume it will be. This causes null pointer dereferences in the drivers as they attempt clean up for stuff that was never put in place.

This is fixed by aborting calling open callbacks once a failure is encountered and then during clean up only iterating through the component list to that point.

Acked-by Vinod Koul <[email protected]>
Signature · 1 line
~Vinod
maintainer
#9 lore raw

Applied "ASoC: compress: Only call free for components which have been opened" to the asoc tree

In reply to Charles Keepax

The patch

ASoC: compress: Only call free for components which have been opened

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 572e6c8dd174bc6fc7ba5d9b6935e9ec8d2660f5 Mon Sep 17 00:00:00 2001

From: Charles Keepax <[email protected]>
Date: Tue, 24 Apr 2018 16:39:01 +0100
Subject: [PATCH] ASoC: compress: Only call free for components which have been
opened

The core should only call free on a component if said component has already had open called on it. This is not presently the case and most compressed drivers in the kernel assume it will be. This causes null pointer dereferences in the drivers as they attempt clean up for stuff that was never put in place.

This is fixed by aborting calling open callbacks once a failure is encountered and then during clean up only iterating through the component list to that point.

This is a fairly quick fix to the issue, to allow backporting. There is more refactoring to follow to tidy the code up a little.

Fixes 9e7e3738ab0e ("ASoC: snd_soc_component_driver has snd_compr_ops")
Signed-off-by Charles Keepax <[email protected]>
Acked-by Vinod Koul <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/soc-compress.c | 52 +++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 24 deletions(-)
sound/soc/soc-compress.c +28 -24
index 82402688bd8e..948505f74229 100644
@@ -33,7 +33,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 

@@ -68,16 +68,15 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 		    !component->driver->compr_ops->open)
 			continue;
 
-		__ret = component->driver->compr_ops->open(cstream);
-		if (__ret < 0) {
+		ret = component->driver->compr_ops->open(cstream);
+		if (ret < 0) {
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, __ret);
-			ret = __ret;
+				component->name, ret);
+			goto machine_err;
 		}
 	}
-	if (ret < 0)
-		goto machine_err;
+	component = NULL;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
 		ret = rtd->dai_link->compr_ops->startup(cstream);

@@ -97,17 +96,20 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 
 machine_err:
 	for_each_rtdcom(rtd, rtdcom) {
-		component = rtdcom->component;
+		struct snd_soc_component *err_comp = rtdcom->component;
+
+		if (err_comp == component)
+			break;
 
 		/* ignore duplication for now */
-		if (platform && (component == &platform->component))
+		if (platform && (err_comp == &platform->component))
 			continue;
 
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
+		if (!err_comp->driver->compr_ops ||
+		    !err_comp->driver->compr_ops->free)
 			continue;
 
-		component->driver->compr_ops->free(cstream);
+		err_comp->driver->compr_ops->free(cstream);
 	}
 
 	if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)

@@ -132,7 +134,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	struct snd_soc_dpcm *dpcm;
 	struct snd_soc_dapm_widget_list *list;
 	int stream;
-	int ret = 0, __ret;
+	int ret;
 
 	if (cstream->direction == SND_COMPRESS_PLAYBACK)
 		stream = SNDRV_PCM_STREAM_PLAYBACK;

@@ -172,16 +174,15 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		    !component->driver->compr_ops->open)
 			continue;
 
-		__ret = component->driver->compr_ops->open(cstream);
-		if (__ret < 0) {
+		ret = component->driver->compr_ops->open(cstream);
+		if (ret < 0) {
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, __ret);
-			ret = __ret;
+				component->name, ret);
+			goto machine_err;
 		}
 	}
-	if (ret < 0)
-		goto machine_err;
+	component = NULL;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) {
 		ret = fe->dai_link->compr_ops->startup(cstream);

@@ -236,17 +237,20 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		fe->dai_link->compr_ops->shutdown(cstream);
 machine_err:
 	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
+		struct snd_soc_component *err_comp = rtdcom->component;
+
+		if (err_comp == component)
+			break;
 
 		/* ignore duplication for now */
-		if (platform && (component == &platform->component))
+		if (platform && (err_comp == &platform->component))
 			continue;
 
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
+		if (!err_comp->driver->compr_ops ||
+		    !err_comp->driver->compr_ops->free)
 			continue;
 
-		component->driver->compr_ops->free(cstream);
+		err_comp->driver->compr_ops->free(cstream);
 	}
 
 	if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)
Signature · 1 line
2.17.0
maintainer
#10 lore raw

Applied "ASoC: compress: Only call free for components which have been opened" to the asoc tree

In reply to Charles Keepax

The patch

ASoC: compress: Only call free for components which have been opened

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 572e6c8dd174bc6fc7ba5d9b6935e9ec8d2660f5 Mon Sep 17 00:00:00 2001

From: Charles Keepax <[email protected]>
Date: Tue, 24 Apr 2018 16:39:01 +0100
Subject: [PATCH] ASoC: compress: Only call free for components which have been
opened

The core should only call free on a component if said component has already had open called on it. This is not presently the case and most compressed drivers in the kernel assume it will be. This causes null pointer dereferences in the drivers as they attempt clean up for stuff that was never put in place.

This is fixed by aborting calling open callbacks once a failure is encountered and then during clean up only iterating through the component list to that point.

This is a fairly quick fix to the issue, to allow backporting. There is more refactoring to follow to tidy the code up a little.

Fixes 9e7e3738ab0e ("ASoC: snd_soc_component_driver has snd_compr_ops")
Signed-off-by Charles Keepax <[email protected]>
Acked-by Vinod Koul <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/soc-compress.c | 52 +++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 24 deletions(-)
sound/soc/soc-compress.c +28 -24
index 82402688bd8e..948505f74229 100644
@@ -33,7 +33,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 

@@ -68,16 +68,15 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 		    !component->driver->compr_ops->open)
 			continue;
 
-		__ret = component->driver->compr_ops->open(cstream);
-		if (__ret < 0) {
+		ret = component->driver->compr_ops->open(cstream);
+		if (ret < 0) {
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, __ret);
-			ret = __ret;
+				component->name, ret);
+			goto machine_err;
 		}
 	}
-	if (ret < 0)
-		goto machine_err;
+	component = NULL;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
 		ret = rtd->dai_link->compr_ops->startup(cstream);

@@ -97,17 +96,20 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 
 machine_err:
 	for_each_rtdcom(rtd, rtdcom) {
-		component = rtdcom->component;
+		struct snd_soc_component *err_comp = rtdcom->component;
+
+		if (err_comp == component)
+			break;
 
 		/* ignore duplication for now */
-		if (platform && (component == &platform->component))
+		if (platform && (err_comp == &platform->component))
 			continue;
 
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
+		if (!err_comp->driver->compr_ops ||
+		    !err_comp->driver->compr_ops->free)
 			continue;
 
-		component->driver->compr_ops->free(cstream);
+		err_comp->driver->compr_ops->free(cstream);
 	}
 
 	if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)

@@ -132,7 +134,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	struct snd_soc_dpcm *dpcm;
 	struct snd_soc_dapm_widget_list *list;
 	int stream;
-	int ret = 0, __ret;
+	int ret;
 
 	if (cstream->direction == SND_COMPRESS_PLAYBACK)
 		stream = SNDRV_PCM_STREAM_PLAYBACK;

@@ -172,16 +174,15 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		    !component->driver->compr_ops->open)
 			continue;
 
-		__ret = component->driver->compr_ops->open(cstream);
-		if (__ret < 0) {
+		ret = component->driver->compr_ops->open(cstream);
+		if (ret < 0) {
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, __ret);
-			ret = __ret;
+				component->name, ret);
+			goto machine_err;
 		}
 	}
-	if (ret < 0)
-		goto machine_err;
+	component = NULL;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) {
 		ret = fe->dai_link->compr_ops->startup(cstream);

@@ -236,17 +237,20 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		fe->dai_link->compr_ops->shutdown(cstream);
 machine_err:
 	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
+		struct snd_soc_component *err_comp = rtdcom->component;
+
+		if (err_comp == component)
+			break;
 
 		/* ignore duplication for now */
-		if (platform && (component == &platform->component))
+		if (platform && (err_comp == &platform->component))
 			continue;
 
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
+		if (!err_comp->driver->compr_ops ||
+		    !err_comp->driver->compr_ops->free)
 			continue;
 
-		component->driver->compr_ops->free(cstream);
+		err_comp->driver->compr_ops->free(cstream);
 	}
 
 	if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)
Signature · 1 line
2.17.0
maintainer
#11 lore raw

Applied "ASoC: compress: Only call free for components which have been opened" to the asoc tree

In reply to Charles Keepax

The patch

ASoC: compress: Only call free for components which have been opened

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 572e6c8dd174bc6fc7ba5d9b6935e9ec8d2660f5 Mon Sep 17 00:00:00 2001

From: Charles Keepax <[email protected]>
Date: Tue, 24 Apr 2018 16:39:01 +0100
Subject: [PATCH] ASoC: compress: Only call free for components which have been
opened

The core should only call free on a component if said component has already had open called on it. This is not presently the case and most compressed drivers in the kernel assume it will be. This causes null pointer dereferences in the drivers as they attempt clean up for stuff that was never put in place.

This is fixed by aborting calling open callbacks once a failure is encountered and then during clean up only iterating through the component list to that point.

This is a fairly quick fix to the issue, to allow backporting. There is more refactoring to follow to tidy the code up a little.

Fixes 9e7e3738ab0e ("ASoC: snd_soc_component_driver has snd_compr_ops")
Signed-off-by Charles Keepax <[email protected]>
Acked-by Vinod Koul <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 sound/soc/soc-compress.c | 52 +++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 24 deletions(-)
sound/soc/soc-compress.c +28 -24
index 82402688bd8e..948505f74229 100644
@@ -33,7 +33,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 

@@ -68,16 +68,15 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 		    !component->driver->compr_ops->open)
 			continue;
 
-		__ret = component->driver->compr_ops->open(cstream);
-		if (__ret < 0) {
+		ret = component->driver->compr_ops->open(cstream);
+		if (ret < 0) {
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, __ret);
-			ret = __ret;
+				component->name, ret);
+			goto machine_err;
 		}
 	}
-	if (ret < 0)
-		goto machine_err;
+	component = NULL;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
 		ret = rtd->dai_link->compr_ops->startup(cstream);

@@ -97,17 +96,20 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 
 machine_err:
 	for_each_rtdcom(rtd, rtdcom) {
-		component = rtdcom->component;
+		struct snd_soc_component *err_comp = rtdcom->component;
+
+		if (err_comp == component)
+			break;
 
 		/* ignore duplication for now */
-		if (platform && (component == &platform->component))
+		if (platform && (err_comp == &platform->component))
 			continue;
 
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
+		if (!err_comp->driver->compr_ops ||
+		    !err_comp->driver->compr_ops->free)
 			continue;
 
-		component->driver->compr_ops->free(cstream);
+		err_comp->driver->compr_ops->free(cstream);
 	}
 
 	if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)

@@ -132,7 +134,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	struct snd_soc_dpcm *dpcm;
 	struct snd_soc_dapm_widget_list *list;
 	int stream;
-	int ret = 0, __ret;
+	int ret;
 
 	if (cstream->direction == SND_COMPRESS_PLAYBACK)
 		stream = SNDRV_PCM_STREAM_PLAYBACK;

@@ -172,16 +174,15 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		    !component->driver->compr_ops->open)
 			continue;
 
-		__ret = component->driver->compr_ops->open(cstream);
-		if (__ret < 0) {
+		ret = component->driver->compr_ops->open(cstream);
+		if (ret < 0) {
 			dev_err(component->dev,
 				"Compress ASoC: can't open platform %s: %d\n",
-				component->name, __ret);
-			ret = __ret;
+				component->name, ret);
+			goto machine_err;
 		}
 	}
-	if (ret < 0)
-		goto machine_err;
+	component = NULL;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) {
 		ret = fe->dai_link->compr_ops->startup(cstream);

@@ -236,17 +237,20 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 		fe->dai_link->compr_ops->shutdown(cstream);
 machine_err:
 	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
+		struct snd_soc_component *err_comp = rtdcom->component;
+
+		if (err_comp == component)
+			break;
 
 		/* ignore duplication for now */
-		if (platform && (component == &platform->component))
+		if (platform && (err_comp == &platform->component))
 			continue;
 
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->free)
+		if (!err_comp->driver->compr_ops ||
+		    !err_comp->driver->compr_ops->free)
 			continue;
 
-		component->driver->compr_ops->free(cstream);
+		err_comp->driver->compr_ops->free(cstream);
 	}
 
 	if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)
Signature · 1 line
2.17.0