[PATCH 00/11] ASoC: use pm_runtime_resume_and_get() when possible
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(-)
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);
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
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;
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
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); }
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); }
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);
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);@@ -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);
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; /*
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; /*
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) {
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);
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);
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);
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);
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)
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)
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
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
participants (5)
-
Cezary Rojewski -
Mark Brown -
Pierre-Louis Bossart -
Péter Ujfalusi -
Srinivas Kandagatla