diff mbox

[RFC,1/4] ASoc: Use ref_count for soc DAI & component

Message ID 1455538772-24926-2-git-send-email-vaibhav.agarwal@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Vaibhav Agarwal Feb. 15, 2016, 12:19 p.m. UTC
This is preperation to allow dynamic DAI link insertion &
removal.

Currently, DAI links are added/removed once during soc-card
instatiate & removal. Thus, irrespective of usage count by multiple
DAI links, DAIs and components are probed or removed only once
while maintaining 'probed' flag.
However, in case of dynamic DAI link insertion/removal we need to
ensure DAI/components are not unnecessarily probed multiple & not
removed mistakenly while in use by any other existing DAI link.
Thus, ref_count is used to maintain their usage count.

Signed-off-by: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
---
 include/sound/soc-dai.h |  1 +
 include/sound/soc.h     |  2 ++
 sound/soc/soc-core.c    | 47 ++++++++++++++++++++++++++++++-----------------
 3 files changed, 33 insertions(+), 17 deletions(-)

Comments

Mark Brown Feb. 15, 2016, 1:59 p.m. UTC | #1
On Mon, Feb 15, 2016 at 05:49:29PM +0530, Vaibhav Agarwal wrote:
> This is preperation to allow dynamic DAI link insertion &
> removal.

Which there is no proposed user of here...

> Currently, DAI links are added/removed once during soc-card
> instatiate & removal. Thus, irrespective of usage count by multiple
> DAI links, DAIs and components are probed or removed only once
> while maintaining 'probed' flag.
> However, in case of dynamic DAI link insertion/removal we need to
> ensure DAI/components are not unnecessarily probed multiple & not
> removed mistakenly while in use by any other existing DAI link.
> Thus, ref_count is used to maintain their usage count.

Please use normal changelog formatting - either consistent line lengths
or blank lines between paragraphs depending on what you're trying to
accomplish here.

> -	if (!dai->probed && dai->driver->probe_order == order) {
> -		if (dai->driver->probe) {
> -			ret = dai->driver->probe(dai);
> -			if (ret < 0) {
> -				dev_err(dai->dev,
> -					"ASoC: failed to probe DAI %s: %d\n",
> -					dai->name, ret);
> -				return ret;
> +	if (dai->driver->probe_order == order) {
> +		if (!dai->probed) {
> +			if (dai->driver->probe) {
> +				ret = dai->driver->probe(dai);
> +				if (ret < 0) {
> +					dev_err(dai->dev,
> +						"ASoC: failed to probe DAI %s: %d\n",
> +						dai->name, ret);
> +					return ret;
> +				}
>  			}
> -		}
>  
> -		dai->probed = 1;
> +			dai->probed = 1;
> +		}
> +		dai->ref_count++;
>  	}

I don't understand this at all - why the complete reindent and change of
condition, don't we just increase the refcount when we flag as probed
(and why do we still have the probed flag after this)?
Vaibhav Agarwal Feb. 16, 2016, 1:27 p.m. UTC | #2
On 15 February 2016 at 19:29, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Feb 15, 2016 at 05:49:29PM +0530, Vaibhav Agarwal wrote:
>> This is preperation to allow dynamic DAI link insertion &
>> removal.
>
> Which there is no proposed user of here...
>
>> Currently, DAI links are added/removed once during soc-card
>> instatiate & removal. Thus, irrespective of usage count by multiple
>> DAI links, DAIs and components are probed or removed only once
>> while maintaining 'probed' flag.
>> However, in case of dynamic DAI link insertion/removal we need to
>> ensure DAI/components are not unnecessarily probed multiple & not
>> removed mistakenly while in use by any other existing DAI link.
>> Thus, ref_count is used to maintain their usage count.
>
> Please use normal changelog formatting - either consistent line lengths
> or blank lines between paragraphs depending on what you're trying to
> accomplish here.
Sure, I'll take care while submitting future patches.
>
>> -     if (!dai->probed && dai->driver->probe_order == order) {
>> -             if (dai->driver->probe) {
>> -                     ret = dai->driver->probe(dai);
>> -                     if (ret < 0) {
>> -                             dev_err(dai->dev,
>> -                                     "ASoC: failed to probe DAI %s: %d\n",
>> -                                     dai->name, ret);
>> -                             return ret;
>> +     if (dai->driver->probe_order == order) {
>> +             if (!dai->probed) {
>> +                     if (dai->driver->probe) {
>> +                             ret = dai->driver->probe(dai);
>> +                             if (ret < 0) {
>> +                                     dev_err(dai->dev,
>> +                                             "ASoC: failed to probe DAI %s: %d\n",
>> +                                             dai->name, ret);
>> +                                     return ret;
>> +                             }
>>                       }
>> -             }
>>
>> -             dai->probed = 1;
>> +                     dai->probed = 1;
>> +             }
>> +             dai->ref_count++;
>>       }
>
> I don't understand this at all - why the complete reindent and change of
> condition, don't we just increase the refcount when we flag as probed
> (and why do we still have the probed flag after this)?
yes, non-zero check would also solve the purpose of probed flag.
will make this change in next patchset.
diff mbox

Patch

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 964b7de..03c2c7a 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -270,6 +270,7 @@  struct snd_soc_dai {
 	unsigned int symmetric_samplebits:1;
 	unsigned int active;
 	unsigned char probed:1;
+	int ref_count;
 
 	struct snd_soc_dapm_widget *playback_widget;
 	struct snd_soc_dapm_widget *capture_widget;
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 7afb72c..3dda0c4 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -822,6 +822,8 @@  struct snd_soc_component {
 	struct dentry *debugfs_root;
 #endif
 
+	int ref_count;
+
 	/*
 	* DO NOT use any of the fields below in drivers, they are temporary and
 	* are going to be removed again soon. If you use them in driver code the
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 790ee2b..2b83814 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1060,6 +1060,11 @@  static void soc_remove_component(struct snd_soc_component *component)
 	if (!component->card)
 		return;
 
+	component->ref_count--;
+
+	if (component->ref_count)
+		return;
+
 	/* This is a HACK and will be removed soon */
 	if (component->codec)
 		list_del(&component->codec->card_list);
@@ -1080,14 +1085,17 @@  static void soc_remove_dai(struct snd_soc_dai *dai, int order)
 
 	if (dai && dai->probed &&
 			dai->driver->remove_order == order) {
-		if (dai->driver->remove) {
-			err = dai->driver->remove(dai);
-			if (err < 0)
-				dev_err(dai->dev,
-					"ASoC: failed to remove %s: %d\n",
-					dai->name, err);
+		dai->ref_count--;
+		if (!dai->ref_count) {
+			if (dai->driver->remove) {
+				err = dai->driver->remove(dai);
+				if (err < 0)
+					dev_err(dai->dev,
+						"ASoC: failed to remove %s: %d\n",
+						dai->name, err);
+			}
+			dai->probed = 0;
 		}
-		dai->probed = 0;
 	}
 }
 
@@ -1367,6 +1375,7 @@  static int soc_probe_component(struct snd_soc_card *card,
 				card->name, component->card->name);
 			return -ENODEV;
 		}
+		component->ref_count++;
 		return 0;
 	}
 
@@ -1436,6 +1445,7 @@  static int soc_probe_component(struct snd_soc_card *card,
 	if (component->codec)
 		list_add(&component->codec->card_list, &card->codec_dev_list);
 
+	component->ref_count++;
 	return 0;
 
 err_probe:
@@ -1523,18 +1533,21 @@  static int soc_probe_dai(struct snd_soc_dai *dai, int order)
 {
 	int ret;
 
-	if (!dai->probed && dai->driver->probe_order == order) {
-		if (dai->driver->probe) {
-			ret = dai->driver->probe(dai);
-			if (ret < 0) {
-				dev_err(dai->dev,
-					"ASoC: failed to probe DAI %s: %d\n",
-					dai->name, ret);
-				return ret;
+	if (dai->driver->probe_order == order) {
+		if (!dai->probed) {
+			if (dai->driver->probe) {
+				ret = dai->driver->probe(dai);
+				if (ret < 0) {
+					dev_err(dai->dev,
+						"ASoC: failed to probe DAI %s: %d\n",
+						dai->name, ret);
+					return ret;
+				}
 			}
-		}
 
-		dai->probed = 1;
+			dai->probed = 1;
+		}
+		dai->ref_count++;
 	}
 
 	return 0;