[PATCH 00/11] ASoC: use pm_runtime_resume_and_get() when possible

[PATCH 00/11] ASoC: use pm_runtime_resume_and_get() when possible

[Pierre-Louis Bossart]

After a set of SOF-specific changes, this patchset correct problematic
uses of pm_runtime_get_sync() in ASoC, or simplifies the flow with no
functional changes. Two patches for Intel platforms also add a test on
resume success.

Additional changes were initially suggested to completely remove the
use of pm_runtime_get_sync(). These changes were dropped since they
are way too invasive, specifically in cases where the return values
were not tested, which would lead to duplicate pm_runtime_put(). The
remaining uses of pm_runtime_get_sync() cannot really be blindly
modified without context and knowledge of each driver.

Pierre-Louis Bossart (11):
  ASoC: Intel: catpt: use pm_runtime_resume_and_get()
  ASoC: Intel: skylake: skl-pcm: use pm_runtime_resume_and_get()
  ASoC: soc-component: use pm_runtime_resume_and_get()
  ASoC: wcd-mbhc-v2: use pm_runtime_resume_and_get()
  ASoC: wsa881x: use pm_runtime_resume_and_get()
  ASoC: rockchip: i2s_tdm: use pm_runtime_resume_and_get()
  ASoC: fsl: fsl_sai: use pm_runtime_resume_and_get()
  ASoC: img: img-i2s-out: use pm_runtime_resume_and_get()
  ASoC: rockchip: pdm: use pm_runtime_resume_and_get()
  ASoC: tas2552: use pm_runtime_resume_and_get()
  ASoC: ti: davinci-mcasp: use pm_runtime_resume_and_get()

 sound/soc/codecs/tas2552.c            |  2 +-
 sound/soc/codecs/wcd-mbhc-v2.c        | 10 ++++------
 sound/soc/codecs/wsa881x.c            |  6 ++----
 sound/soc/fsl/fsl_sai.c               |  6 ++----
 sound/soc/img/img-i2s-out.c           | 12 ++++--------
 sound/soc/intel/catpt/pcm.c           | 26 ++++++++++++++++++++------
 sound/soc/intel/catpt/sysfs.c         |  4 +++-
 sound/soc/intel/skylake/skl-pcm.c     |  5 ++++-
 sound/soc/rockchip/rockchip_i2s_tdm.c |  6 ++----
 sound/soc/rockchip/rockchip_pdm.c     |  6 ++----
 sound/soc/soc-component.c             |  8 ++++----
 sound/soc/ti/davinci-mcasp.c          |  3 +--
 12 files changed, 49 insertions(+), 45 deletions(-)

-- 
2.34.1

[PATCH 01/11] ASoC: Intel: catpt: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

The current code does not check for errors and does not release the
reference on errors.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/intel/catpt/pcm.c   | 26 ++++++++++++++++++++------
 sound/soc/intel/catpt/sysfs.c |  4 +++-
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/catpt/pcm.c b/sound/soc/intel/catpt/pcm.c
index a26000cd5cebc..30ca5416c9a3f 100644
--- a/sound/soc/intel/catpt/pcm.c
+++ b/sound/soc/intel/catpt/pcm.c
@@ -667,7 +667,9 @@ static int catpt_dai_pcm_new(struct snd_soc_pcm_runtime *rtm,
 	if (!memcmp(&cdev->devfmt[devfmt.iface], &devfmt, sizeof(devfmt)))
 		return 0;
 
-	pm_runtime_get_sync(cdev->dev);
+	ret = pm_runtime_resume_and_get(cdev->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
 
 	ret = catpt_ipc_set_device_format(cdev, &devfmt);
 
@@ -853,9 +855,12 @@ static int catpt_mixer_volume_get(struct snd_kcontrol *kcontrol,
 		snd_soc_kcontrol_component(kcontrol);
 	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	u32 dspvol;
+	int ret;
 	int i;
 
-	pm_runtime_get_sync(cdev->dev);
+	ret = pm_runtime_resume_and_get(cdev->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
 
 	for (i = 0; i < CATPT_CHANNELS_MAX; i++) {
 		dspvol = catpt_mixer_volume(cdev, &cdev->mixer, i);
@@ -876,7 +881,9 @@ static int catpt_mixer_volume_put(struct snd_kcontrol *kcontrol,
 	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	int ret;
 
-	pm_runtime_get_sync(cdev->dev);
+	ret = pm_runtime_resume_and_get(cdev->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
 
 	ret = catpt_set_dspvol(cdev, cdev->mixer.mixer_hw_id,
 			       ucontrol->value.integer.value);
@@ -897,6 +904,7 @@ static int catpt_stream_volume_get(struct snd_kcontrol *kcontrol,
 	struct catpt_dev *cdev = dev_get_drvdata(component->dev);
 	long *ctlvol = (long *)kcontrol->private_value;
 	u32 dspvol;
+	int ret;
 	int i;
 
 	stream = catpt_stream_find(cdev, pin_id);
@@ -906,7 +914,9 @@ static int catpt_stream_volume_get(struct snd_kcontrol *kcontrol,
 		return 0;
 	}
 
-	pm_runtime_get_sync(cdev->dev);
+	ret = pm_runtime_resume_and_get(cdev->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
 
 	for (i = 0; i < CATPT_CHANNELS_MAX; i++) {
 		dspvol = catpt_stream_volume(cdev, stream, i);
@@ -937,7 +947,9 @@ static int catpt_stream_volume_put(struct snd_kcontrol *kcontrol,
 		return 0;
 	}
 
-	pm_runtime_get_sync(cdev->dev);
+	ret = pm_runtime_resume_and_get(cdev->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
 
 	ret = catpt_set_dspvol(cdev, stream->info.stream_hw_id,
 			       ucontrol->value.integer.value);
@@ -1013,7 +1025,9 @@ static int catpt_loopback_switch_put(struct snd_kcontrol *kcontrol,
 		return 0;
 	}
 
-	pm_runtime_get_sync(cdev->dev);
+	ret = pm_runtime_resume_and_get(cdev->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
 
 	ret = catpt_ipc_mute_loopback(cdev, stream->info.stream_hw_id, mute);
 
diff --git a/sound/soc/intel/catpt/sysfs.c b/sound/soc/intel/catpt/sysfs.c
index 9579e233a15db..1bdbcc04dc712 100644
--- a/sound/soc/intel/catpt/sysfs.c
+++ b/sound/soc/intel/catpt/sysfs.c
@@ -15,7 +15,9 @@ static ssize_t fw_version_show(struct device *dev,
 	struct catpt_fw_version version;
 	int ret;
 
-	pm_runtime_get_sync(cdev->dev);
+	ret = pm_runtime_resume_and_get(cdev->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
 
 	ret = catpt_ipc_get_fw_version(cdev, &version);
 
-- 
2.34.1

Re: [PATCH 01/11] ASoC: Intel: catpt: use pm_runtime_resume_and_get()

[Cezary Rojewski]

On 2022-06-17 12:04 AM, Pierre-Louis Bossart wrote:
> The current code does not check for errors and does not release the
> reference on errors.


Thanks for the fixes.

Acked-by: Cezary Rojewski <cezary.rojewski@intel.com>

[PATCH 02/11] ASoC: Intel: skylake: skl-pcm: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

The current code does not check for errors and does not release the
reference on errors.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 55f310e91b55c..9d72ebd812af9 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1380,7 +1380,10 @@ static int skl_platform_soc_probe(struct snd_soc_component *component)
 	const struct skl_dsp_ops *ops;
 	int ret;
 
-	pm_runtime_get_sync(component->dev);
+	ret = pm_runtime_resume_and_get(component->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
+
 	if (bus->ppcap) {
 		skl->component = component;
 
-- 
2.34.1

Re: [PATCH 02/11] ASoC: Intel: skylake: skl-pcm: use pm_runtime_resume_and_get()

[Cezary Rojewski]

On 2022-06-17 12:04 AM, Pierre-Louis Bossart wrote:
> The current code does not check for errors and does not release the
> reference on errors.


Thanks for the fixes.

Acked-by: Cezary Rojewski <cezary.rojewski@intel.com>

[PATCH 03/11] ASoC: soc-component: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

simplify the flow. No functionality change, except that on -EACCESS
the reference count will be decreased.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/soc-component.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index e12f8244242b9..cb92e002c38bc 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -1213,11 +1213,11 @@ int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd,
 	int i;
 
 	for_each_rtd_components(rtd, i, component) {
-		int ret = pm_runtime_get_sync(component->dev);
-		if (ret < 0 && ret != -EACCES) {
-			pm_runtime_put_noidle(component->dev);
+		int ret = pm_runtime_resume_and_get(component->dev);
+
+		if (ret < 0 && ret != -EACCES)
 			return soc_component_ret(component, ret);
-		}
+
 		/* mark stream if succeeded */
 		soc_component_mark_push(component, stream, pm);
 	}
-- 
2.34.1

Re: [PATCH 03/11] ASoC: soc-component: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

On 6/16/22 17:04, Pierre-Louis Bossart wrote:
> simplify the flow. No functionality change, except that on -EACCESS
> the reference count will be decreased.

This patch turns out to be incorrect and should not be merged.

I missed the fact that the component pm_runtime_put() will decrease the
reference count that is already decreased with
pm_runtime_resume_and_get() when pm_runtime is not enabled. This leads
to warnings:

snd-soc-dummy snd-soc-dummy: Runtime PM usage count underflow!

Unfortunately we missed those warnings during validation, that's not so
good.

pm_runtime_resume_and_get() really needs to be used ONLY when the
get/put are part of the same function and the reference count can be
checked. When the get/put are in different functions, it's asking for
trouble.

Also the check on -EACCES is problematic when the component is handled
by a framework, it's not clear if that can happen or not.

The rest of the patches follow the pattern get_sync/put and don't have a
problem.

Sorry for the noise.

> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
>  sound/soc/soc-component.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
> index e12f8244242b9..cb92e002c38bc 100644
> --- a/sound/soc/soc-component.c
> +++ b/sound/soc/soc-component.c
> @@ -1213,11 +1213,11 @@ int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd,
>  	int i;
>  
>  	for_each_rtd_components(rtd, i, component) {
> -		int ret = pm_runtime_get_sync(component->dev);
> -		if (ret < 0 && ret != -EACCES) {
> -			pm_runtime_put_noidle(component->dev);
> +		int ret = pm_runtime_resume_and_get(component->dev);
> +
> +		if (ret < 0 && ret != -EACCES)
>  			return soc_component_ret(component, ret);
> -		}
> +
>  		/* mark stream if succeeded */
>  		soc_component_mark_push(component, stream, pm);
>  	}

[PATCH 04/11] ASoC: wcd-mbhc-v2: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

simplify the flow. No functionality change, except that on -EACCESS
the reference count will be decreased.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/codecs/wcd-mbhc-v2.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/wcd-mbhc-v2.c b/sound/soc/codecs/wcd-mbhc-v2.c
index 31009283e7d4a..98baef594bf31 100644
--- a/sound/soc/codecs/wcd-mbhc-v2.c
+++ b/sound/soc/codecs/wcd-mbhc-v2.c
@@ -714,12 +714,11 @@ static int wcd_mbhc_initialise(struct wcd_mbhc *mbhc)
 	struct snd_soc_component *component = mbhc->component;
 	int ret;
 
-	ret = pm_runtime_get_sync(component->dev);
+	ret = pm_runtime_resume_and_get(component->dev);
 	if (ret < 0 && ret != -EACCES) {
 		dev_err_ratelimited(component->dev,
-				    "pm_runtime_get_sync failed in %s, ret %d\n",
+				    "pm_runtime_resume_and_get failed in %s, ret %d\n",
 				    __func__, ret);
-		pm_runtime_put_noidle(component->dev);
 		return ret;
 	}
 
@@ -1097,12 +1096,11 @@ static void wcd_correct_swch_plug(struct work_struct *work)
 	mbhc = container_of(work, struct wcd_mbhc, correct_plug_swch);
 	component = mbhc->component;
 
-	ret = pm_runtime_get_sync(component->dev);
+	ret = pm_runtime_resume_and_get(component->dev);
 	if (ret < 0 && ret != -EACCES) {
 		dev_err_ratelimited(component->dev,
-				    "pm_runtime_get_sync failed in %s, ret %d\n",
+				    "pm_runtime_resume_and_get failed in %s, ret %d\n",
 				    __func__, ret);
-		pm_runtime_put_noidle(component->dev);
 		return;
 	}
 	micbias_mv = wcd_mbhc_get_micbias(mbhc);
-- 
2.34.1

Re: [PATCH 04/11] ASoC: wcd-mbhc-v2: use pm_runtime_resume_and_get()

[Srinivas Kandagatla]

On 16/06/2022 15:04, Pierre-Louis Bossart wrote:
> simplify the flow. No functionality change, except that on -EACCESS
> the reference count will be decreased.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
Thanks Pierre,

LGTM,
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

--srini

>   sound/soc/codecs/wcd-mbhc-v2.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/codecs/wcd-mbhc-v2.c b/sound/soc/codecs/wcd-mbhc-v2.c
> index 31009283e7d4a..98baef594bf31 100644
> --- a/sound/soc/codecs/wcd-mbhc-v2.c
> +++ b/sound/soc/codecs/wcd-mbhc-v2.c
> @@ -714,12 +714,11 @@ static int wcd_mbhc_initialise(struct wcd_mbhc *mbhc)
>   	struct snd_soc_component *component = mbhc->component;
>   	int ret;
>   
> -	ret = pm_runtime_get_sync(component->dev);
> +	ret = pm_runtime_resume_and_get(component->dev);
>   	if (ret < 0 && ret != -EACCES) {
>   		dev_err_ratelimited(component->dev,
> -				    "pm_runtime_get_sync failed in %s, ret %d\n",
> +				    "pm_runtime_resume_and_get failed in %s, ret %d\n",
>   				    __func__, ret);
> -		pm_runtime_put_noidle(component->dev);
>   		return ret;
>   	}
>   
> @@ -1097,12 +1096,11 @@ static void wcd_correct_swch_plug(struct work_struct *work)
>   	mbhc = container_of(work, struct wcd_mbhc, correct_plug_swch);
>   	component = mbhc->component;
>   
> -	ret = pm_runtime_get_sync(component->dev);
> +	ret = pm_runtime_resume_and_get(component->dev);
>   	if (ret < 0 && ret != -EACCES) {
>   		dev_err_ratelimited(component->dev,
> -				    "pm_runtime_get_sync failed in %s, ret %d\n",
> +				    "pm_runtime_resume_and_get failed in %s, ret %d\n",
>   				    __func__, ret);
> -		pm_runtime_put_noidle(component->dev);
>   		return;
>   	}
>   	micbias_mv = wcd_mbhc_get_micbias(mbhc);

[PATCH 05/11] ASoC: wsa881x: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

simplify the flow. No functionality change, except that on -EACCESS
the reference count will be decreased.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/codecs/wsa881x.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
index f3a56f3ce4871..dc954b85a9881 100644
--- a/sound/soc/codecs/wsa881x.c
+++ b/sound/soc/codecs/wsa881x.c
@@ -749,11 +749,9 @@ static int wsa881x_put_pa_gain(struct snd_kcontrol *kc,
 	unsigned int mask = (1 << fls(max)) - 1;
 	int val, ret, min_gain, max_gain;
 
-	ret = pm_runtime_get_sync(comp->dev);
-	if (ret < 0 && ret != -EACCES) {
-		pm_runtime_put_noidle(comp->dev);
+	ret = pm_runtime_resume_and_get(comp->dev);
+	if (ret < 0 && ret != -EACCES)
 		return ret;
-	}
 
 	max_gain = (max - ucontrol->value.integer.value[0]) & mask;
 	/*
-- 
2.34.1

Re: [PATCH 05/11] ASoC: wsa881x: use pm_runtime_resume_and_get()

[Srinivas Kandagatla]

On 16/06/2022 15:04, Pierre-Louis Bossart wrote:
> simplify the flow. No functionality change, except that on -EACCESS
> the reference count will be decreased.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
Thanks Pierre,

LGTM,

Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

--srini
>   sound/soc/codecs/wsa881x.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
> index f3a56f3ce4871..dc954b85a9881 100644
> --- a/sound/soc/codecs/wsa881x.c
> +++ b/sound/soc/codecs/wsa881x.c
> @@ -749,11 +749,9 @@ static int wsa881x_put_pa_gain(struct snd_kcontrol *kc,
>   	unsigned int mask = (1 << fls(max)) - 1;
>   	int val, ret, min_gain, max_gain;
>   
> -	ret = pm_runtime_get_sync(comp->dev);
> -	if (ret < 0 && ret != -EACCES) {
> -		pm_runtime_put_noidle(comp->dev);
> +	ret = pm_runtime_resume_and_get(comp->dev);
> +	if (ret < 0 && ret != -EACCES)
>   		return ret;
> -	}
>   
>   	max_gain = (max - ucontrol->value.integer.value[0]) & mask;
>   	/*

[PATCH 06/11] ASoC: rockchip: i2s_tdm: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

simplify the flow. No functionality change, except that on -EACCESS
the reference count will be decreased.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/rockchip/rockchip_i2s_tdm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c
index 48b3ecfa58b46..70542a402477e 100644
--- a/sound/soc/rockchip/rockchip_i2s_tdm.c
+++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
@@ -404,11 +404,9 @@ static int rockchip_i2s_tdm_set_fmt(struct snd_soc_dai *cpu_dai,
 	int ret;
 	bool is_tdm = i2s_tdm->tdm_mode;
 
-	ret = pm_runtime_get_sync(cpu_dai->dev);
-	if (ret < 0 && ret != -EACCES) {
-		pm_runtime_put_noidle(cpu_dai->dev);
+	ret = pm_runtime_resume_and_get(cpu_dai->dev);
+	if (ret < 0 && ret != -EACCES)
 		return ret;
-	}
 
 	mask = I2S_CKR_MSS_MASK;
 	switch (fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) {
-- 
2.34.1

[PATCH 07/11] ASoC: fsl: fsl_sai: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

Simplify the flow.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/fsl/fsl_sai.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 4f5bd9597c746..b6407d4d3e09d 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1141,11 +1141,9 @@ static int fsl_sai_probe(struct platform_device *pdev)
 			goto err_pm_disable;
 	}
 
-	ret = pm_runtime_get_sync(dev);
-	if (ret < 0) {
-		pm_runtime_put_noidle(dev);
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
 		goto err_pm_get_sync;
-	}
 
 	/* Get sai version */
 	ret = fsl_sai_check_version(dev);
-- 
2.34.1

[PATCH 08/11] ASoC: img: img-i2s-out: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

Simplify the flow.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/img/img-i2s-out.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/sound/soc/img/img-i2s-out.c b/sound/soc/img/img-i2s-out.c
index 9ec6fc528e2b4..50a522aca419a 100644
--- a/sound/soc/img/img-i2s-out.c
+++ b/sound/soc/img/img-i2s-out.c
@@ -346,11 +346,9 @@ static int img_i2s_out_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 
 	chan_control_mask = IMG_I2S_OUT_CHAN_CTL_CLKT_MASK;
 
-	ret = pm_runtime_get_sync(i2s->dev);
-	if (ret < 0) {
-		pm_runtime_put_noidle(i2s->dev);
+	ret = pm_runtime_resume_and_get(i2s->dev);
+	if (ret < 0)
 		return ret;
-	}
 
 	img_i2s_out_disable(i2s);
 
@@ -482,11 +480,9 @@ static int img_i2s_out_probe(struct platform_device *pdev)
 		if (ret)
 			goto err_pm_disable;
 	}
-	ret = pm_runtime_get_sync(&pdev->dev);
-	if (ret < 0) {
-		pm_runtime_put_noidle(&pdev->dev);
+	ret = pm_runtime_resume_and_get(&pdev->dev);
+	if (ret < 0)
 		goto err_suspend;
-	}
 
 	reg = IMG_I2S_OUT_CTL_FRM_SIZE_MASK;
 	img_i2s_out_writel(i2s, reg, IMG_I2S_OUT_CTL);
-- 
2.34.1

[PATCH 09/11] ASoC: rockchip: pdm: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

Simplify the flow.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/rockchip/rockchip_pdm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/rockchip/rockchip_pdm.c b/sound/soc/rockchip/rockchip_pdm.c
index 64d9891b6434f..066e29b7879d1 100644
--- a/sound/soc/rockchip/rockchip_pdm.c
+++ b/sound/soc/rockchip/rockchip_pdm.c
@@ -688,11 +688,9 @@ static int rockchip_pdm_resume(struct device *dev)
 	struct rk_pdm_dev *pdm = dev_get_drvdata(dev);
 	int ret;
 
-	ret = pm_runtime_get_sync(dev);
-	if (ret < 0) {
-		pm_runtime_put(dev);
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
 		return ret;
-	}
 
 	ret = regcache_sync(pdm->regmap);
 
-- 
2.34.1

[PATCH 10/11] ASoC: tas2552: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

The use of pm_runtime_get_sync() is buggy with no use of put_noidle() on
error.

Use pm_runtime_resume_and_get() instead.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/codecs/tas2552.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c
index c98a9332dcc0e..da744cfae611c 100644
--- a/sound/soc/codecs/tas2552.c
+++ b/sound/soc/codecs/tas2552.c
@@ -581,7 +581,7 @@ static int tas2552_component_probe(struct snd_soc_component *component)
 
 	gpiod_set_value(tas2552->enable_gpio, 1);
 
-	ret = pm_runtime_get_sync(component->dev);
+	ret = pm_runtime_resume_and_get(component->dev);
 	if (ret < 0) {
 		dev_err(component->dev, "Enabling device failed: %d\n",
 			ret);
-- 
2.34.1

[PATCH 11/11] ASoC: ti: davinci-mcasp: use pm_runtime_resume_and_get()

[Pierre-Louis Bossart]

The use of pm_runtime_get_sync() is buggy with no use of put_noidle on
error.

Use pm_runtime_resume_and_get() instead.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/ti/davinci-mcasp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/ti/davinci-mcasp.c b/sound/soc/ti/davinci-mcasp.c
index e2aab4729f3ab..0201000b619f6 100644
--- a/sound/soc/ti/davinci-mcasp.c
+++ b/sound/soc/ti/davinci-mcasp.c
@@ -2111,8 +2111,7 @@ static int davinci_mcasp_gpio_request(struct gpio_chip *chip, unsigned offset)
 	}
 
 	/* Do not change the PIN yet */
-
-	return pm_runtime_get_sync(mcasp->dev);
+	return pm_runtime_resume_and_get(mcasp->dev);
 }
 
 static void davinci_mcasp_gpio_free(struct gpio_chip *chip, unsigned offset)
-- 
2.34.1

Re: [PATCH 11/11] ASoC: ti: davinci-mcasp: use pm_runtime_resume_and_get()

[Péter Ujfalusi]

Hi Pierre,

On 17/06/2022 01:04, Pierre-Louis Bossart wrote:
> The use of pm_runtime_get_sync() is buggy with no use of put_noidle on
> error.
> 
> Use pm_runtime_resume_and_get() instead.

Thanks for the fix,

Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>

> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
>  sound/soc/ti/davinci-mcasp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/sound/soc/ti/davinci-mcasp.c b/sound/soc/ti/davinci-mcasp.c
> index e2aab4729f3ab..0201000b619f6 100644
> --- a/sound/soc/ti/davinci-mcasp.c
> +++ b/sound/soc/ti/davinci-mcasp.c
> @@ -2111,8 +2111,7 @@ static int davinci_mcasp_gpio_request(struct gpio_chip *chip, unsigned offset)
>  	}
>  
>  	/* Do not change the PIN yet */
> -
> -	return pm_runtime_get_sync(mcasp->dev);
> +	return pm_runtime_resume_and_get(mcasp->dev);
>  }
>  
>  static void davinci_mcasp_gpio_free(struct gpio_chip *chip, unsigned offset)

-- 
Péter

Re: [PATCH 00/11] ASoC: use pm_runtime_resume_and_get() when possible

[Cezary Rojewski]

On 2022-06-17 12:04 AM, Pierre-Louis Bossart wrote:
> After a set of SOF-specific changes, this patchset correct problematic
> uses of pm_runtime_get_sync() in ASoC, or simplifies the flow with no
> functional changes. Two patches for Intel platforms also add a test on
> resume success.
> 
> Additional changes were initially suggested to completely remove the
> use of pm_runtime_get_sync(). These changes were dropped since they
> are way too invasive, specifically in cases where the return values
> were not tested, which would lead to duplicate pm_runtime_put(). The
> remaining uses of pm_runtime_get_sync() cannot really be blindly
> modified without context and knowledge of each driver.
> 
> Pierre-Louis Bossart (11):
>    ASoC: Intel: catpt: use pm_runtime_resume_and_get()
>    ASoC: Intel: skylake: skl-pcm: use pm_runtime_resume_and_get()
>    ASoC: soc-component: use pm_runtime_resume_and_get()
>    ASoC: wcd-mbhc-v2: use pm_runtime_resume_and_get()
>    ASoC: wsa881x: use pm_runtime_resume_and_get()
>    ASoC: rockchip: i2s_tdm: use pm_runtime_resume_and_get()
>    ASoC: fsl: fsl_sai: use pm_runtime_resume_and_get()
>    ASoC: img: img-i2s-out: use pm_runtime_resume_and_get()
>    ASoC: rockchip: pdm: use pm_runtime_resume_and_get()
>    ASoC: tas2552: use pm_runtime_resume_and_get()
>    ASoC: ti: davinci-mcasp: use pm_runtime_resume_and_get()


For the non-acked patches:

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>

Re: (subset) [PATCH 00/11] ASoC: use pm_runtime_resume_and_get() when possible

[Mark Brown]

On Thu, 16 Jun 2022 17:04:16 -0500, Pierre-Louis Bossart wrote:
> After a set of SOF-specific changes, this patchset correct problematic
> uses of pm_runtime_get_sync() in ASoC, or simplifies the flow with no
> functional changes. Two patches for Intel platforms also add a test on
> resume success.
> 
> Additional changes were initially suggested to completely remove the
> use of pm_runtime_get_sync(). These changes were dropped since they
> are way too invasive, specifically in cases where the return values
> were not tested, which would lead to duplicate pm_runtime_put(). The
> remaining uses of pm_runtime_get_sync() cannot really be blindly
> modified without context and knowledge of each driver.
> 
> [...]

Applied to

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

Thanks!

[01/11] ASoC: Intel: catpt: use pm_runtime_resume_and_get()
        commit: 82102a24c930986aedc572f89b437cd9e4d44d7e
[02/11] ASoC: Intel: skylake: skl-pcm: use pm_runtime_resume_and_get()
        commit: 7213170a9515109322f75c08b5268d8e9cdad8e4
[04/11] ASoC: wcd-mbhc-v2: use pm_runtime_resume_and_get()
        commit: ddea4bbf287b6028eaa15a185d0693856956ecf2
[05/11] ASoC: wsa881x: use pm_runtime_resume_and_get()
        commit: 9a1a28610a1c49bf93777d017aa3fe121eef944e
[06/11] ASoC: rockchip: i2s_tdm: use pm_runtime_resume_and_get()
        commit: 8c8a13e83c29472044d733dfb1fced2ccd025d35
[07/11] ASoC: fsl: fsl_sai: use pm_runtime_resume_and_get()
        commit: 37cb8a58013fc6ca2febaed355f6559012699542
[08/11] ASoC: img: img-i2s-out: use pm_runtime_resume_and_get()
        commit: 57d714535051b1baca9ffd92e79fbda1fae3177a
[09/11] ASoC: rockchip: pdm: use pm_runtime_resume_and_get()
        commit: 76a6f4537650e6d2211f34661de35630487c7c64
[10/11] ASoC: tas2552: use pm_runtime_resume_and_get()
        commit: 05b71fb2a5014d2430ff6c5678db021c67afa9ec
[11/11] ASoC: ti: davinci-mcasp: use pm_runtime_resume_and_get()
        commit: cecc81d6a5deb094bdbc6a1d7f2c014ba9b71cf8

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