During the review of the patches from unm.edu, one of the patterns
I noticed is the amount of patches trying to fix pm_runtime_get_sync()
calls.
On contrary of the common sense that a foo_get() function will
only increment the usage on success, pm_runtime_get_sync()
increments it unconditionally.
Due to that, there are bugs on lots of places, that ended being
gradually fixed, but, still there are a few places on media where
this is still broken.
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added a new method to does a pm_runtime get, which increments
the usage count only on success.
This series replace all places where the old pm_runtime_get_sync()
is called, using pm_runtime_resume_and_get() instead.
This should help to avoid future mistakes like that, as people
tend to use the existing drivers as examples for newer ones.
compile-tested only.
Mauro Carvalho Chehab (78):
media: atmel: properly get pm_runtime
media: marvel-ccic: fix some issues when getting pm_runtime
media: mdk-mdp: fix pm_runtime_get_sync() usage count
media: rcar_fdp1: fix usage count
media: mdk-mdp: fix pm_runtime_get_sync() usage count
media: renesas-ceu: fix pm_runtime_get_sync() usage count
media: s5p: fix pm_runtime_get_sync() usage count
media: am437x:: fix pm_runtime_get_sync() usage count
media: sh_vou: fix pm_runtime_get_sync() usage count
media: sti/hva: use pm_runtime_resume_and_get()
staging: media: rkvdec: fix pm_runtime_get_sync() usage count
staging: media: atomisp_fops: use pm_runtime_resume_and_get()
staging: media: hantro_drv: use pm_runtime_resume_and_get()
staging: media: imx7-mipi-csis: use pm_runtime_resume_and_get()
staging: media: ipu3: use pm_runtime_resume_and_get()
staging: media: cedrus_video: use pm_runtime_resume_and_get()
staging: media: vde: use pm_runtime_resume_and_get()
staging: media: csi: use pm_runtime_resume_and_get()
staging: media: vi: use pm_runtime_resume_and_get()
media: mtk-vcodec: fix pm_runtime_get_sync() usage count
media: s5p-jpeg: fix pm_runtime_get_sync() usage count
media: delta-v4l2: fix pm_runtime_get_sync() usage count
media: sun8i_rotate: fix pm_runtime_get_sync() usage count
media: i2c: ak7375: use pm_runtime_resume_and_get()
media: i2c: ccs-core: use pm_runtime_resume_and_get()
media: i2c: dw9714: use pm_runtime_resume_and_get()
media: i2c: dw9768: use pm_runtime_resume_and_get()
media: i2c: dw9807-vcm: use pm_runtime_resume_and_get()
media: i2c: hi556: use pm_runtime_resume_and_get()
media: i2c: imx214: use pm_runtime_resume_and_get()
media: i2c: imx219: use pm_runtime_resume_and_get()
media: i2c: imx258: use pm_runtime_resume_and_get()
media: i2c: imx274: use pm_runtime_resume_and_get()
media: i2c: imx290: use pm_runtime_resume_and_get()
media: i2c: imx319: use pm_runtime_resume_and_get()
media: i2c: imx334: use pm_runtime_resume_and_get()
media: i2c: imx355: use pm_runtime_resume_and_get()
media: i2c: mt9m001: use pm_runtime_resume_and_get()
media: i2c: ov02a10: use pm_runtime_resume_and_get()
media: i2c: ov13858: use pm_runtime_resume_and_get()
media: i2c: ov2659: use pm_runtime_resume_and_get()
media: i2c: ov2685: use pm_runtime_resume_and_get()
media: i2c: ov2740: use pm_runtime_resume_and_get()
media: i2c: ov5647: use pm_runtime_resume_and_get()
media: i2c: ov5648: use pm_runtime_resume_and_get()
media: i2c: ov5670: use pm_runtime_resume_and_get()
media: i2c: ov5675: use pm_runtime_resume_and_get()
media: i2c: ov5695: use pm_runtime_resume_and_get()
media: i2c: ov7740: use pm_runtime_resume_and_get()
media: i2c: ov8856: use pm_runtime_resume_and_get()
media: i2c: ov8865: use pm_runtime_resume_and_get()
media: i2c: ov9734: use pm_runtime_resume_and_get()
media: i2c: tvp5150: use pm_runtime_resume_and_get()
media: i2c: video-i2c: use pm_runtime_resume_and_get()
media: ipu3: use pm_runtime_resume_and_get()
media: coda: use pm_runtime_resume_and_get()
media: exynos4-is: use pm_runtime_resume_and_get()
media: exynos-gsc: use pm_runtime_resume_and_get()
media: mtk-jpeg: use pm_runtime_resume_and_get()
media: camss-csid: use pm_runtime_resume_and_get()
media: camss-csiphy: use pm_runtime_resume_and_get()
media: camss-ispif: use pm_runtime_resume_and_get()
media: camss-vfe: use pm_runtime_resume_and_get()
media: core: use pm_runtime_resume_and_get()
media: pm_helpers: use pm_runtime_resume_and_get()
media: vdec: use pm_runtime_resume_and_get()
media: venc: use pm_runtime_resume_and_get()
media: rcar-fcp: use pm_runtime_resume_and_get()
media: rcar-vin: use pm_runtime_resume_and_get()
media: rga-buf: use pm_runtime_resume_and_get()
media: rkisp1-capture: use pm_runtime_resume_and_get()
media: s3c-camif: use pm_runtime_resume_and_get()
media: s5p-mfc: use pm_runtime_resume_and_get()
media: bdisp-v4l2: use pm_runtime_resume_and_get()
media: stm32: use pm_runtime_resume_and_get()
media: sun4i_v4l2: use pm_runtime_resume_and_get()
media: ti-vpe: use pm_runtime_resume_and_get()
media: vsp1: use pm_runtime_resume_and_get()
drivers/media/cec/platform/s5p/s5p_cec.c | 5 +++-
drivers/media/i2c/ak7375.c | 10 +------
drivers/media/i2c/ccs/ccs-core.c | 11 ++++----
drivers/media/i2c/dw9714.c | 10 +------
drivers/media/i2c/dw9768.c | 10 +------
drivers/media/i2c/dw9807-vcm.c | 10 +------
drivers/media/i2c/hi556.c | 3 +--
drivers/media/i2c/imx214.c | 6 ++---
drivers/media/i2c/imx219.c | 6 ++---
drivers/media/i2c/imx258.c | 6 ++---
drivers/media/i2c/imx274.c | 3 +--
drivers/media/i2c/imx290.c | 6 ++---
drivers/media/i2c/imx319.c | 6 ++---
drivers/media/i2c/imx334.c | 5 ++--
drivers/media/i2c/imx355.c | 6 ++---
drivers/media/i2c/mt9m001.c | 7 ++---
drivers/media/i2c/ov02a10.c | 6 ++---
drivers/media/i2c/ov13858.c | 6 ++---
drivers/media/i2c/ov2659.c | 6 ++---
drivers/media/i2c/ov2685.c | 7 +++--
drivers/media/i2c/ov2740.c | 6 ++---
drivers/media/i2c/ov5647.c | 9 ++++---
drivers/media/i2c/ov5648.c | 6 ++---
drivers/media/i2c/ov5670.c | 6 ++---
drivers/media/i2c/ov5675.c | 3 +--
drivers/media/i2c/ov5695.c | 6 ++---
drivers/media/i2c/ov7740.c | 8 +++---
drivers/media/i2c/ov8856.c | 3 +--
drivers/media/i2c/ov8865.c | 6 ++---
drivers/media/i2c/ov9734.c | 3 +--
drivers/media/i2c/tvp5150.c | 16 +++---------
drivers/media/i2c/video-i2c.c | 14 ++++------
drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 3 +--
drivers/media/platform/am437x/am437x-vpfe.c | 10 ++++---
drivers/media/platform/atmel/atmel-isc-base.c | 26 ++++++++++++++-----
drivers/media/platform/atmel/atmel-isi.c | 19 +++++++++++---
drivers/media/platform/coda/coda-common.c | 2 +-
drivers/media/platform/exynos-gsc/gsc-core.c | 3 +--
drivers/media/platform/exynos-gsc/gsc-m2m.c | 2 +-
.../media/platform/exynos4-is/fimc-capture.c | 6 ++---
drivers/media/platform/exynos4-is/fimc-is.c | 3 ++-
.../platform/exynos4-is/fimc-isp-video.c | 3 +--
drivers/media/platform/exynos4-is/fimc-isp.c | 7 +++--
drivers/media/platform/exynos4-is/fimc-lite.c | 5 ++--
drivers/media/platform/exynos4-is/fimc-m2m.c | 2 +-
drivers/media/platform/exynos4-is/media-dev.c | 8 +++---
drivers/media/platform/exynos4-is/mipi-csis.c | 5 ++--
.../media/platform/marvell-ccic/mcam-core.c | 9 +++++--
.../media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 +--
drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 6 ++---
.../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 4 +--
.../media/platform/qcom/camss/camss-csid.c | 6 ++---
.../media/platform/qcom/camss/camss-csiphy.c | 6 ++---
.../media/platform/qcom/camss/camss-ispif.c | 6 ++---
drivers/media/platform/qcom/camss/camss-vfe.c | 5 ++--
drivers/media/platform/qcom/venus/core.c | 19 +++++++-------
.../media/platform/qcom/venus/pm_helpers.c | 10 +++----
drivers/media/platform/qcom/venus/vdec.c | 4 +--
drivers/media/platform/qcom/venus/venc.c | 5 ++--
drivers/media/platform/rcar-fcp.c | 6 ++---
drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +-
drivers/media/platform/rcar-vin/rcar-dma.c | 6 ++---
drivers/media/platform/rcar-vin/rcar-v4l2.c | 6 ++---
drivers/media/platform/rcar_fdp1.c | 12 +++++++--
drivers/media/platform/renesas-ceu.c | 5 +++-
drivers/media/platform/rockchip/rga/rga-buf.c | 3 +--
drivers/media/platform/rockchip/rga/rga.c | 4 ++-
.../platform/rockchip/rkisp1/rkisp1-capture.c | 3 +--
.../media/platform/s3c-camif/camif-capture.c | 5 ++--
drivers/media/platform/s3c-camif/camif-core.c | 5 ++--
drivers/media/platform/s5p-jpeg/jpeg-core.c | 2 +-
drivers/media/platform/s5p-mfc/s5p_mfc_pm.c | 6 ++---
drivers/media/platform/sh_vou.c | 6 ++++-
drivers/media/platform/sti/bdisp/bdisp-v4l2.c | 7 ++---
drivers/media/platform/sti/delta/delta-v4l2.c | 4 +--
drivers/media/platform/sti/hva/hva-hw.c | 17 ++++++------
drivers/media/platform/stm32/stm32-dcmi.c | 5 ++--
.../platform/sunxi/sun4i-csi/sun4i_v4l2.c | 7 +++--
.../sunxi/sun8i-rotate/sun8i_rotate.c | 2 +-
drivers/media/platform/ti-vpe/cal-video.c | 4 ++-
drivers/media/platform/ti-vpe/cal.c | 8 +++---
drivers/media/platform/ti-vpe/vpe.c | 4 +--
drivers/media/platform/vsp1/vsp1_drv.c | 6 ++---
.../staging/media/atomisp/pci/atomisp_fops.c | 6 ++---
drivers/staging/media/hantro/hantro_drv.c | 2 +-
drivers/staging/media/imx/imx7-mipi-csis.c | 7 +++--
drivers/staging/media/ipu3/ipu3.c | 3 +--
drivers/staging/media/rkvdec/rkvdec.c | 2 +-
.../staging/media/sunxi/cedrus/cedrus_video.c | 6 ++---
drivers/staging/media/tegra-vde/vde.c | 16 +++++++-----
drivers/staging/media/tegra-video/csi.c | 3 +--
drivers/staging/media/tegra-video/vi.c | 3 +--
92 files changed, 270 insertions(+), 322 deletions(-)
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/s3c-camif/camif-capture.c | 5 ++---
drivers/media/platform/s3c-camif/camif-core.c | 5 +++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index 9ca49af29542..01fa08065ebc 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -547,16 +547,15 @@ static int s3c_camif_open(struct file *file)
if (ret < 0)
goto unlock;
- ret = pm_runtime_get_sync(camif->dev);
+ ret = pm_runtime_resume_and_get(camif->dev);
if (ret < 0)
- goto err_pm;
+ goto unlock;
ret = sensor_set_power(camif, 1);
if (!ret)
goto unlock;
pm_runtime_put(camif->dev);
-err_pm:
v4l2_fh_release(file);
unlock:
mutex_unlock(&camif->lock);
diff --git a/drivers/media/platform/s3c-camif/camif-core.c b/drivers/media/platform/s3c-camif/camif-core.c
index 4c3c00d59c92..e1d51fd3e700 100644
--- a/drivers/media/platform/s3c-camif/camif-core.c
+++ b/drivers/media/platform/s3c-camif/camif-core.c
@@ -460,9 +460,9 @@ static int s3c_camif_probe(struct platform_device *pdev)
pm_runtime_enable(dev);
- ret = pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
if (ret < 0)
- goto err_pm;
+ goto err_disable;
ret = camif_media_dev_init(camif);
if (ret < 0)
@@ -502,6 +502,7 @@ static int s3c_camif_probe(struct platform_device *pdev)
camif_unregister_media_entities(camif);
err_pm:
pm_runtime_put(dev);
+err_disable:
pm_runtime_disable(dev);
camif_clk_put(camif);
err_clk:
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/hi556.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/i2c/hi556.c b/drivers/media/i2c/hi556.c
index 6f05c1138e3b..627ccfa34835 100644
--- a/drivers/media/i2c/hi556.c
+++ b/drivers/media/i2c/hi556.c
@@ -813,9 +813,8 @@ static int hi556_set_stream(struct v4l2_subdev *sd, int enable)
mutex_lock(&hi556->mutex);
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
mutex_unlock(&hi556->mutex);
return ret;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
index 4785faddf630..ed6ec41b9c2d 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
@@ -206,9 +206,9 @@ static int sun4i_csi_open(struct file *file)
if (ret)
return ret;
- ret = pm_runtime_get_sync(csi->dev);
+ ret = pm_runtime_resume_and_get(csi->dev);
if (ret < 0)
- goto err_pm_put;
+ goto err_unlock;
ret = v4l2_pipeline_pm_get(&csi->vdev.entity);
if (ret)
@@ -225,8 +225,7 @@ static int sun4i_csi_open(struct file *file)
err_pipeline_pm_put:
v4l2_pipeline_pm_put(&csi->vdev.entity);
-err_pm_put:
- pm_runtime_put(csi->dev);
+err_unlock:
mutex_unlock(&csi->lock);
return ret;
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/imx290.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 6319a42057d2..06020e648a97 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -764,11 +764,9 @@ static int imx290_set_stream(struct v4l2_subdev *sd, int enable)
int ret = 0;
if (enable) {
- ret = pm_runtime_get_sync(imx290->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(imx290->dev);
+ ret = pm_runtime_resume_and_get(imx290->dev);
+ if (ret < 0)
goto unlock_and_return;
- }
ret = imx290_start_streaming(imx290);
if (ret) {
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index 88a23bce569d..a89c7b206eef 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -920,7 +920,7 @@ static void mtk_jpeg_enc_device_run(void *priv)
src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
- ret = pm_runtime_get_sync(jpeg->dev);
+ ret = pm_runtime_resume_and_get(jpeg->dev);
if (ret < 0)
goto enc_end;
@@ -973,7 +973,7 @@ static void mtk_jpeg_dec_device_run(void *priv)
return;
}
- ret = pm_runtime_get_sync(jpeg->dev);
+ ret = pm_runtime_resume_and_get(jpeg->dev);
if (ret < 0)
goto dec_end;
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/dw9807-vcm.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/media/i2c/dw9807-vcm.c b/drivers/media/i2c/dw9807-vcm.c
index 438a44b76da8..95e06f13bc9e 100644
--- a/drivers/media/i2c/dw9807-vcm.c
+++ b/drivers/media/i2c/dw9807-vcm.c
@@ -130,15 +130,7 @@ static const struct v4l2_ctrl_ops dw9807_vcm_ctrl_ops = {
static int dw9807_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
{
- int rval;
-
- rval = pm_runtime_get_sync(sd->dev);
- if (rval < 0) {
- pm_runtime_put_noidle(sd->dev);
- return rval;
- }
-
- return 0;
+ return pm_runtime_resume_and_get(sd->dev);
}
static int dw9807_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/qcom/camss/camss-csid.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index cc11fbfdae13..d2a7f2a64f26 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -156,11 +156,9 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
int ret;
if (on) {
- ret = pm_runtime_get_sync(dev);
- if (ret < 0) {
- pm_runtime_put_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret < 0)
return ret;
- }
ret = regulator_enable(csid->vdda);
if (ret < 0) {
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/imx355.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
index ccedcd4c520a..93f13a04439a 100644
--- a/drivers/media/i2c/imx355.c
+++ b/drivers/media/i2c/imx355.c
@@ -1442,11 +1442,9 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
}
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
+ if (ret < 0)
goto err_unlock;
- }
/*
* Apply default & customized values
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/imx319.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c
index 38540323a156..4e0a8c9d271f 100644
--- a/drivers/media/i2c/imx319.c
+++ b/drivers/media/i2c/imx319.c
@@ -2141,11 +2141,9 @@ static int imx319_set_stream(struct v4l2_subdev *sd, int enable)
}
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
+ if (ret < 0)
goto err_unlock;
- }
/*
* Apply default & customized values
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov5670.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index dee7df8dd100..182f271f118f 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -2347,11 +2347,9 @@ static int ov5670_set_stream(struct v4l2_subdev *sd, int enable)
goto unlock_and_return;
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
+ if (ret < 0)
goto unlock_and_return;
- }
ret = ov5670_start_streaming(ov5670);
if (ret)
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov2685.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov2685.c b/drivers/media/i2c/ov2685.c
index 49a2dcedb347..2f3836dd8eed 100644
--- a/drivers/media/i2c/ov2685.c
+++ b/drivers/media/i2c/ov2685.c
@@ -456,11 +456,10 @@ static int ov2685_s_stream(struct v4l2_subdev *sd, int on)
goto unlock_and_return;
if (on) {
- ret = pm_runtime_get_sync(&ov2685->client->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
+ ret = pm_runtime_resume_and_get(&ov2685->client->dev);
+ if (ret < 0)
goto unlock_and_return;
- }
+
ret = __v4l2_ctrl_handler_setup(&ov2685->ctrl_handler);
if (ret) {
pm_runtime_put(&client->dev);
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/sti/bdisp/bdisp-v4l2.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
index 060ca85f64d5..85288da9d2ae 100644
--- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
+++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
@@ -499,7 +499,7 @@ static int bdisp_start_streaming(struct vb2_queue *q, unsigned int count)
{
struct bdisp_ctx *ctx = q->drv_priv;
struct vb2_v4l2_buffer *buf;
- int ret = pm_runtime_get_sync(ctx->bdisp_dev->dev);
+ int ret = pm_runtime_resume_and_get(ctx->bdisp_dev->dev);
if (ret < 0) {
dev_err(ctx->bdisp_dev->dev, "failed to set runtime PM\n");
@@ -1364,10 +1364,10 @@ static int bdisp_probe(struct platform_device *pdev)
/* Power management */
pm_runtime_enable(dev);
- ret = pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
if (ret < 0) {
dev_err(dev, "failed to set PM\n");
- goto err_pm;
+ goto err_remove;
}
/* Filters */
@@ -1395,6 +1395,7 @@ static int bdisp_probe(struct platform_device *pdev)
bdisp_hw_free_filters(bdisp->dev);
err_pm:
pm_runtime_put(dev);
+err_remove:
bdisp_debugfs_remove(bdisp);
v4l2_device_unregister(&bdisp->v4l2_dev);
err_clk:
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/mt9m001.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index 3b0ba8ed5233..57e15a291ebd 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -217,9 +217,9 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
goto done;
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
if (ret < 0)
- goto put_unlock;
+ goto unlock;
ret = mt9m001_apply_selection(sd);
if (ret)
@@ -247,6 +247,7 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
put_unlock:
pm_runtime_put(&client->dev);
+unlock:
mutex_unlock(&mt9m001->mutex);
return ret;
@@ -834,7 +835,7 @@ static int mt9m001_remove(struct i2c_client *client)
{
struct mt9m001 *mt9m001 = to_mt9m001(client);
- pm_runtime_get_sync(&client->dev);
+ pm_runtime_resume_and_get(&client->dev);
v4l2_async_unregister_subdev(&mt9m001->subdev);
media_entity_cleanup(&mt9m001->subdev.entity);
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/qcom/venus/vdec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index ddb7cd39424e..347e533ea673 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -568,7 +568,7 @@ static int vdec_pm_get(struct venus_inst *inst)
int ret;
mutex_lock(&core->pm_lock);
- ret = pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
mutex_unlock(&core->pm_lock);
return ret < 0 ? ret : 0;
@@ -601,7 +601,7 @@ static int vdec_pm_get_put(struct venus_inst *inst)
mutex_lock(&core->pm_lock);
if (pm_runtime_suspended(dev)) {
- ret = pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
if (ret < 0)
goto error;
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/rcar-fcp.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/rcar-fcp.c b/drivers/media/platform/rcar-fcp.c
index 5c03318ae07b..de76af58013c 100644
--- a/drivers/media/platform/rcar-fcp.c
+++ b/drivers/media/platform/rcar-fcp.c
@@ -101,11 +101,9 @@ int rcar_fcp_enable(struct rcar_fcp_device *fcp)
if (!fcp)
return 0;
- ret = pm_runtime_get_sync(fcp->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(fcp->dev);
+ ret = pm_runtime_resume_and_get(fcp->dev);
+ if (ret < 0)
return ret;
- }
return 0;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/video-i2c.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
index 0465832a4090..5cde7da698bd 100644
--- a/drivers/media/i2c/video-i2c.c
+++ b/drivers/media/i2c/video-i2c.c
@@ -286,11 +286,9 @@ static int amg88xx_read(struct device *dev, enum hwmon_sensor_types type,
__le16 buf;
int tmp;
- tmp = pm_runtime_get_sync(regmap_get_device(data->regmap));
- if (tmp < 0) {
- pm_runtime_put_noidle(regmap_get_device(data->regmap));
+ tmp = pm_runtime_resume_and_get(regmap_get_device(data->regmap));
+ if (tmp < 0)
return tmp;
- }
tmp = regmap_bulk_read(data->regmap, AMG88XX_REG_TTHL, &buf, 2);
pm_runtime_mark_last_busy(regmap_get_device(data->regmap));
@@ -512,11 +510,9 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
if (data->kthread_vid_cap)
return 0;
- 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 error_del_list;
- }
ret = data->chip->setup(data);
if (ret)
@@ -893,7 +889,7 @@ static int video_i2c_remove(struct i2c_client *client)
{
struct video_i2c_data *data = i2c_get_clientdata(client);
- pm_runtime_get_sync(&client->dev);
+ pm_runtime_resume_and_get(&client->dev);
pm_runtime_disable(&client->dev);
pm_runtime_set_suspended(&client->dev);
pm_runtime_put_noidle(&client->dev);
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/qcom/camss/camss-vfe.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 15695fd466c4..cf743e61f798 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -584,9 +584,9 @@ static int vfe_get(struct vfe_device *vfe)
if (ret < 0)
goto error_pm_domain;
- ret = pm_runtime_get_sync(vfe->camss->dev);
+ ret = pm_runtime_resume_and_get(vfe->camss->dev);
if (ret < 0)
- goto error_pm_runtime_get;
+ goto error_domain_off;
ret = vfe_set_clock_rates(vfe);
if (ret < 0)
@@ -620,6 +620,7 @@ static int vfe_get(struct vfe_device *vfe)
error_pm_runtime_get:
pm_runtime_put_sync(vfe->camss->dev);
+error_domain_off:
vfe->ops->pm_domain_off(vfe);
error_pm_domain:
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov02a10.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov02a10.c b/drivers/media/i2c/ov02a10.c
index c47b1d45d8fd..a1d7314b20a9 100644
--- a/drivers/media/i2c/ov02a10.c
+++ b/drivers/media/i2c/ov02a10.c
@@ -540,11 +540,9 @@ static int ov02a10_s_stream(struct v4l2_subdev *sd, int on)
}
if (on) {
- ret = pm_runtime_get_sync(&client->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
+ if (ret < 0)
goto unlock_and_return;
- }
ret = __ov02a10_start_stream(ov02a10);
if (ret) {
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +-
drivers/media/platform/rcar-vin/rcar-dma.c | 6 ++----
drivers/media/platform/rcar-vin/rcar-v4l2.c | 6 ++----
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index e06cd512aba2..85574765a11a 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -408,7 +408,7 @@ static void rcsi2_enter_standby(struct rcar_csi2 *priv)
static void rcsi2_exit_standby(struct rcar_csi2 *priv)
{
- pm_runtime_get_sync(priv->dev);
+ pm_runtime_resume_and_get(priv->dev);
reset_control_deassert(priv->rstc);
}
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index f30dafbdf61c..f5f722ab1d4e 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1458,11 +1458,9 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
u32 vnmc;
int ret;
- ret = pm_runtime_get_sync(vin->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(vin->dev);
+ ret = pm_runtime_resume_and_get(vin->dev);
+ if (ret < 0)
return ret;
- }
/* Make register writes take effect immediately. */
vnmc = rvin_read(vin, VNMC_REG);
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 457a65bf6b66..b1e9f86caa5c 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -870,11 +870,9 @@ static int rvin_open(struct file *file)
struct rvin_dev *vin = video_drvdata(file);
int ret;
- ret = pm_runtime_get_sync(vin->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(vin->dev);
+ ret = pm_runtime_resume_and_get(vin->dev);
+ if (ret < 0)
return ret;
- }
ret = mutex_lock_interruptible(&vin->lock);
if (ret)
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/imx334.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 047aa7658d21..a5a03bb4a6ae 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -717,9 +717,9 @@ static int imx334_set_stream(struct v4l2_subdev *sd, int enable)
}
if (enable) {
- ret = pm_runtime_get_sync(imx334->dev);
+ ret = pm_runtime_resume_and_get(imx334->dev);
if (ret)
- goto error_power_off;
+ goto error_unlock;
ret = imx334_start_streaming(imx334);
if (ret)
@@ -737,6 +737,7 @@ static int imx334_set_stream(struct v4l2_subdev *sd, int enable)
error_power_off:
pm_runtime_put(imx334->dev);
+error_unlock:
mutex_unlock(&imx334->mutex);
return ret;
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/qcom/camss/camss-csiphy.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index b3c3bf19e522..8e18b8e668cf 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -197,11 +197,9 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
if (on) {
int ret;
- ret = pm_runtime_get_sync(dev);
- if (ret < 0) {
- pm_runtime_put_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret < 0)
return ret;
- }
ret = csiphy_set_clock_rates(csiphy);
if (ret < 0) {
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/qcom/venus/venc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 4a7291f934b6..8dd49d4f124c 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -1205,9 +1205,9 @@ static int venc_open(struct file *file)
venus_helper_init_instance(inst);
- ret = pm_runtime_get_sync(core->dev_enc);
+ ret = pm_runtime_resume_and_get(core->dev_enc);
if (ret < 0)
- goto err_put_sync;
+ goto err_free;
ret = venc_ctrl_init(inst);
if (ret)
@@ -1252,6 +1252,7 @@ static int venc_open(struct file *file)
venc_ctrl_deinit(inst);
err_put_sync:
pm_runtime_put_sync(core->dev_enc);
+err_free:
kfree(inst);
return ret;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/vsp1/vsp1_drv.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index aa66e4f5f3f3..c2bdb6629657 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -561,11 +561,9 @@ int vsp1_device_get(struct vsp1_device *vsp1)
{
int ret;
- ret = pm_runtime_get_sync(vsp1->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(vsp1->dev);
+ ret = pm_runtime_resume_and_get(vsp1->dev);
+ if (ret < 0)
return ret;
- }
return 0;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov9734.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov9734.c b/drivers/media/i2c/ov9734.c
index b7309a551cae..ba156683c533 100644
--- a/drivers/media/i2c/ov9734.c
+++ b/drivers/media/i2c/ov9734.c
@@ -644,9 +644,8 @@ static int ov9734_set_stream(struct v4l2_subdev *sd, int enable)
}
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
mutex_unlock(&ov9734->mutex);
return ret;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov2659.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index 42f64175a6df..a3c8eae68486 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -1186,11 +1186,9 @@ static int ov2659_s_stream(struct v4l2_subdev *sd, int on)
goto unlock;
}
- ret = pm_runtime_get_sync(&client->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
+ if (ret < 0)
goto unlock;
- }
ret = ov2659_init(sd, 0);
if (!ret)
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov7740.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index 47a9003d29d6..849a1ce61cf7 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -624,11 +624,9 @@ static int ov7740_set_stream(struct v4l2_subdev *sd, int enable)
}
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
+ if (ret < 0)
goto err_unlock;
- }
ret = ov7740_start_streaming(ov7740);
if (ret)
@@ -1165,7 +1163,7 @@ static int ov7740_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(sd);
ov7740_free_controls(ov7740);
- pm_runtime_get_sync(&client->dev);
+ pm_runtime_resume_and_get(&client->dev);
pm_runtime_disable(&client->dev);
pm_runtime_set_suspended(&client->dev);
pm_runtime_put_noidle(&client->dev);
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov13858.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 4a2885ff0cbe..9598c0b19603 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -1472,11 +1472,9 @@ static int ov13858_set_stream(struct v4l2_subdev *sd, int enable)
}
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
+ if (ret < 0)
goto err_unlock;
- }
/*
* Apply default & customized values
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov5675.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c
index dea32859459a..e7e297a23960 100644
--- a/drivers/media/i2c/ov5675.c
+++ b/drivers/media/i2c/ov5675.c
@@ -863,9 +863,8 @@ static int ov5675_set_stream(struct v4l2_subdev *sd, int enable)
mutex_lock(&ov5675->mutex);
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
mutex_unlock(&ov5675->mutex);
return ret;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/tvp5150.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index e26e3f544054..374a9da75e4d 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1448,11 +1448,9 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, int enable)
TVP5150_MISC_CTL_CLOCK_OE;
if (enable) {
- ret = pm_runtime_get_sync(sd->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(sd->dev);
+ ret = pm_runtime_resume_and_get(sd->dev);
+ if (ret < 0)
return ret;
- }
tvp5150_enable(sd);
@@ -1675,15 +1673,7 @@ static int tvp5150_registered(struct v4l2_subdev *sd)
static int tvp5150_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
{
- int ret;
-
- ret = pm_runtime_get_sync(sd->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(sd->dev);
- return ret;
- }
-
- return 0;
+ return pm_runtime_resume_and_get(sd->dev);
}
static int tvp5150_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/qcom/venus/pm_helpers.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index c7e1ebec47ee..9e32ec866af7 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -486,7 +486,7 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
int ret;
if (coreid_mask & VIDC_CORE_ID_1) {
- ret = pm_runtime_get_sync(core->pmdomains[1]);
+ ret = pm_runtime_resume_and_get(core->pmdomains[1]);
if (ret < 0)
return ret;
@@ -504,7 +504,7 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
}
if (coreid_mask & VIDC_CORE_ID_2) {
- ret = pm_runtime_get_sync(core->pmdomains[2]);
+ ret = pm_runtime_resume_and_get(core->pmdomains[2]);
if (ret < 0)
return ret;
@@ -990,11 +990,9 @@ static int core_power_v4(struct venus_core *core, int on)
if (on == POWER_ON) {
if (pmctrl) {
- ret = pm_runtime_get_sync(pmctrl);
- if (ret < 0) {
- pm_runtime_put_noidle(pmctrl);
+ ret = pm_runtime_resume_and_get(pmctrl);
+ if (ret < 0)
return ret;
- }
}
ret = core_resets_reset(core);
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov5647.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 1cefa15729ce..38faa74755e3 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -882,20 +882,20 @@ static int ov5647_s_stream(struct v4l2_subdev *sd, int enable)
}
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
if (ret < 0)
goto error_unlock;
ret = ov5647_stream_on(sd);
if (ret < 0) {
dev_err(&client->dev, "stream start failed: %d\n", ret);
- goto error_unlock;
+ goto error_pm;
}
} else {
ret = ov5647_stream_off(sd);
if (ret < 0) {
dev_err(&client->dev, "stream stop failed: %d\n", ret);
- goto error_unlock;
+ goto error_pm;
}
pm_runtime_put(&client->dev);
}
@@ -905,8 +905,9 @@ static int ov5647_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
+error_pm:
+ pm_runtime_put(&client->dev);
error_unlock:
- pm_runtime_put(&client->dev);
mutex_unlock(&sensor->lock);
return ret;
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov5648.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov5648.c b/drivers/media/i2c/ov5648.c
index 3ecb4a3e8773..07e64ff0be3f 100644
--- a/drivers/media/i2c/ov5648.c
+++ b/drivers/media/i2c/ov5648.c
@@ -2132,11 +2132,9 @@ static int ov5648_s_stream(struct v4l2_subdev *subdev, int enable)
int ret;
if (enable) {
- ret = pm_runtime_get_sync(sensor->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(sensor->dev);
+ ret = pm_runtime_resume_and_get(sensor->dev);
+ if (ret < 0)
return ret;
- }
}
mutex_lock(&sensor->mutex);
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov8856.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
index e3af3ea277af..2875f8e4ddcb 100644
--- a/drivers/media/i2c/ov8856.c
+++ b/drivers/media/i2c/ov8856.c
@@ -1340,9 +1340,8 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
mutex_lock(&ov8856->mutex);
if (enable) {
- ret = pm_runtime_get_sync(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
mutex_unlock(&ov8856->mutex);
return ret;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov5695.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
index 09bee57a241d..469d941813c6 100644
--- a/drivers/media/i2c/ov5695.c
+++ b/drivers/media/i2c/ov5695.c
@@ -946,11 +946,9 @@ static int ov5695_s_stream(struct v4l2_subdev *sd, int on)
goto unlock_and_return;
if (on) {
- ret = pm_runtime_get_sync(&client->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&client->dev);
+ ret = pm_runtime_resume_and_get(&client->dev);
+ if (ret < 0)
goto unlock_and_return;
- }
ret = __ov5695_start_stream(ov5695);
if (ret) {
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index 5f6c9d1623e4..3730376897d9 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -1003,9 +1003,8 @@ rkisp1_vb2_start_streaming(struct vb2_queue *queue, unsigned int count)
if (ret)
goto err_pipeline_stop;
- ret = pm_runtime_get_sync(cap->rkisp1->dev);
+ ret = pm_runtime_resume_and_get(cap->rkisp1->dev);
if (ret < 0) {
- pm_runtime_put_noidle(cap->rkisp1->dev);
dev_err(cap->rkisp1->dev, "power up failed %d\n", ret);
goto err_destroy_dummy;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/rockchip/rga/rga-buf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
index bf9a75b75083..81508ed5abf3 100644
--- a/drivers/media/platform/rockchip/rga/rga-buf.c
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -79,9 +79,8 @@ static int rga_buf_start_streaming(struct vb2_queue *q, unsigned int count)
struct rockchip_rga *rga = ctx->rga;
int ret;
- ret = pm_runtime_get_sync(rga->dev);
+ ret = pm_runtime_resume_and_get(rga->dev);
if (ret < 0) {
- pm_runtime_put_noidle(rga->dev);
rga_buf_return_buffers(q, VB2_BUF_STATE_QUEUED);
return ret;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/coda/coda-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index bd666c858fa1..fea1c136a42c 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -2660,7 +2660,7 @@ static int coda_open(struct file *file)
ctx->use_vdoa = false;
/* Power up and upload firmware if necessary */
- ret = pm_runtime_get_sync(dev->dev);
+ ret = pm_runtime_resume_and_get(dev->dev);
if (ret < 0) {
v4l2_err(&dev->v4l2_dev, "failed to power up: %d\n", ret);
goto err_pm_get;
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/ti-vpe/cal-video.c | 4 +++-
drivers/media/platform/ti-vpe/cal.c | 8 +++++---
drivers/media/platform/ti-vpe/vpe.c | 4 +---
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/cal-video.c b/drivers/media/platform/ti-vpe/cal-video.c
index 7b7436a355ee..15fb5360cf13 100644
--- a/drivers/media/platform/ti-vpe/cal-video.c
+++ b/drivers/media/platform/ti-vpe/cal-video.c
@@ -700,7 +700,9 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
- pm_runtime_get_sync(ctx->cal->dev);
+ ret = pm_runtime_resume_and_get(ctx->cal->dev);
+ if (ret < 0)
+ goto error_pipeline;
cal_ctx_set_dma_addr(ctx, addr);
cal_ctx_start(ctx);
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 2e2bef91b2b0..76fe7a8b33f6 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1024,7 +1024,7 @@ static int cal_probe(struct platform_device *pdev)
/* Read the revision and hardware info to verify hardware access. */
pm_runtime_enable(&pdev->dev);
- ret = pm_runtime_get_sync(&pdev->dev);
+ ret = pm_runtime_resume_and_get(&pdev->dev);
if (ret)
goto error_pm_runtime;
@@ -1098,10 +1098,11 @@ static int cal_remove(struct platform_device *pdev)
{
struct cal_dev *cal = platform_get_drvdata(pdev);
unsigned int i;
+ int ret;
cal_dbg(1, cal, "Removing %s\n", CAL_MODULE_NAME);
- pm_runtime_get_sync(&pdev->dev);
+ ret = pm_runtime_resume_and_get(&pdev->dev);
cal_media_unregister(cal);
@@ -1115,7 +1116,8 @@ static int cal_remove(struct platform_device *pdev)
for (i = 0; i < cal->data->num_csi2_phy; i++)
cal_camerarx_destroy(cal->phy[i]);
- pm_runtime_put_sync(&pdev->dev);
+ if (ret >= 0)
+ pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
return 0;
diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
index 10251b787674..07cb2c140295 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -2471,10 +2471,8 @@ static int vpe_runtime_get(struct platform_device *pdev)
dev_dbg(&pdev->dev, "vpe_runtime_get\n");
- r = pm_runtime_get_sync(&pdev->dev);
+ r = pm_runtime_resume_and_get(&pdev->dev);
WARN_ON(r < 0);
- if (r)
- pm_runtime_put_noidle(&pdev->dev);
return r < 0 ? r : 0;
}
--
2.30.2
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.
Use the new API, in order to cleanup the error check logic.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/i2c/ov8865.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index 9ecf180635ee..3bf6ee4898a9 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -2497,11 +2497,9 @@ static int ov8865_s_stream(struct v4l2_subdev *subdev, int enable)
int ret;
if (enable) {
- ret = pm_runtime_get_sync(sensor->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(sensor->dev);
+ ret = pm_runtime_resume_and_get(sensor->dev);
+ if (ret < 0)
return ret;
- }
}
mutex_lock(&sensor->mutex);
--
2.30.2
Hi Mauro,
On Sat, Apr 24, 2021 at 08:44:48AM +0200, Mauro Carvalho Chehab wrote:
> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> added pm_runtime_resume_and_get() in order to automatically handle
> dev->power.usage_count decrement on errors.
>
> Use the new API, in order to cleanup the error check logic.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Thanks
Acked-by: Jacopo Mondi <[email protected]>
I should re-work the error handling sequence there on top of this
patch as right now it's not the best, that 'done' label bothers me...
anyway, for later.
> ---
> drivers/media/i2c/mt9m001.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> index 3b0ba8ed5233..57e15a291ebd 100644
> --- a/drivers/media/i2c/mt9m001.c
> +++ b/drivers/media/i2c/mt9m001.c
> @@ -217,9 +217,9 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
> goto done;
>
> if (enable) {
> - ret = pm_runtime_get_sync(&client->dev);
> + ret = pm_runtime_resume_and_get(&client->dev);
> if (ret < 0)
> - goto put_unlock;
> + goto unlock;
>
> ret = mt9m001_apply_selection(sd);
> if (ret)
> @@ -247,6 +247,7 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
>
> put_unlock:
> pm_runtime_put(&client->dev);
> +unlock:
> mutex_unlock(&mt9m001->mutex);
>
> return ret;
> @@ -834,7 +835,7 @@ static int mt9m001_remove(struct i2c_client *client)
> {
> struct mt9m001 *mt9m001 = to_mt9m001(client);
>
> - pm_runtime_get_sync(&client->dev);
> + pm_runtime_resume_and_get(&client->dev);
>
> v4l2_async_unregister_subdev(&mt9m001->subdev);
> media_entity_cleanup(&mt9m001->subdev.entity);
> --
> 2.30.2
>
Hi Mauro,
On Sat, Apr 24, 2021 at 08:44:54AM +0200, Mauro Carvalho Chehab wrote:
> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> added pm_runtime_resume_and_get() in order to automatically handle
> dev->power.usage_count decrement on errors.
>
> Use the new API, in order to cleanup the error check logic.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Looks good, thanks!
Acked-by: Jacopo Mondi <[email protected]>
> ---
> drivers/media/i2c/ov5647.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> index 1cefa15729ce..38faa74755e3 100644
> --- a/drivers/media/i2c/ov5647.c
> +++ b/drivers/media/i2c/ov5647.c
> @@ -882,20 +882,20 @@ static int ov5647_s_stream(struct v4l2_subdev *sd, int enable)
> }
>
> if (enable) {
> - ret = pm_runtime_get_sync(&client->dev);
> + ret = pm_runtime_resume_and_get(&client->dev);
> if (ret < 0)
> goto error_unlock;
>
> ret = ov5647_stream_on(sd);
> if (ret < 0) {
> dev_err(&client->dev, "stream start failed: %d\n", ret);
> - goto error_unlock;
> + goto error_pm;
> }
> } else {
> ret = ov5647_stream_off(sd);
> if (ret < 0) {
> dev_err(&client->dev, "stream stop failed: %d\n", ret);
> - goto error_unlock;
> + goto error_pm;
> }
> pm_runtime_put(&client->dev);
> }
> @@ -905,8 +905,9 @@ static int ov5647_s_stream(struct v4l2_subdev *sd, int enable)
>
> return 0;
>
> +error_pm:
> + pm_runtime_put(&client->dev);
> error_unlock:
> - pm_runtime_put(&client->dev);
> mutex_unlock(&sensor->lock);
>
> return ret;
> --
> 2.30.2
>
Hi Mauro,
Thanks for your work.
On 2021-04-24 08:45:19 +0200, Mauro Carvalho Chehab wrote:
> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> added pm_runtime_resume_and_get() in order to automatically handle
> dev->power.usage_count decrement on errors.
>
> Use the new API, in order to cleanup the error check logic.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Reviewed-by: Niklas S?derlund <[email protected]>
> ---
> drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +-
> drivers/media/platform/rcar-vin/rcar-dma.c | 6 ++----
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 6 ++----
> 3 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index e06cd512aba2..85574765a11a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -408,7 +408,7 @@ static void rcsi2_enter_standby(struct rcar_csi2 *priv)
>
> static void rcsi2_exit_standby(struct rcar_csi2 *priv)
> {
> - pm_runtime_get_sync(priv->dev);
> + pm_runtime_resume_and_get(priv->dev);
> reset_control_deassert(priv->rstc);
> }
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index f30dafbdf61c..f5f722ab1d4e 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1458,11 +1458,9 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
> u32 vnmc;
> int ret;
>
> - ret = pm_runtime_get_sync(vin->dev);
> - if (ret < 0) {
> - pm_runtime_put_noidle(vin->dev);
> + ret = pm_runtime_resume_and_get(vin->dev);
> + if (ret < 0)
> return ret;
> - }
>
> /* Make register writes take effect immediately. */
> vnmc = rvin_read(vin, VNMC_REG);
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 457a65bf6b66..b1e9f86caa5c 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -870,11 +870,9 @@ static int rvin_open(struct file *file)
> struct rvin_dev *vin = video_drvdata(file);
> int ret;
>
> - ret = pm_runtime_get_sync(vin->dev);
> - if (ret < 0) {
> - pm_runtime_put_noidle(vin->dev);
> + ret = pm_runtime_resume_and_get(vin->dev);
> + if (ret < 0)
> return ret;
> - }
>
> ret = mutex_lock_interruptible(&vin->lock);
> if (ret)
> --
> 2.30.2
>
--
Regards,
Niklas S?derlund
Hi Mauro,
On Sat, Apr 24, 2021 at 8:46 AM Mauro Carvalho Chehab
<[email protected]> wrote:
> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> added pm_runtime_resume_and_get() in order to automatically handle
> dev->power.usage_count decrement on errors.
>
> Use the new API, in order to cleanup the error check logic.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Thanks for your patch!
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -408,7 +408,7 @@ static void rcsi2_enter_standby(struct rcar_csi2 *priv)
>
> static void rcsi2_exit_standby(struct rcar_csi2 *priv)
> {
> - pm_runtime_get_sync(priv->dev);
> + pm_runtime_resume_and_get(priv->dev);
I believe this part is incorrect: on failure[*], the refcount will now
be decremented, and in a subsequent call to rcsi2_enter_standby(), the
refcount will be decremented again due to the call to pm_runtime_put().
[*] On e.g. R-Car SoCs, this can never fail. This is the reason why
many R-Car (and SuperH) drivers never check the result of
pm_runtime_get_sync(). So the refcount "imbalances" were actually
introduced by the various "clean up" patches to add return value
checking, which now need another round of fixing...
> reset_control_deassert(priv->rstc);
> }
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1458,11 +1458,9 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
> u32 vnmc;
> int ret;
>
> - ret = pm_runtime_get_sync(vin->dev);
> - if (ret < 0) {
> - pm_runtime_put_noidle(vin->dev);
> + ret = pm_runtime_resume_and_get(vin->dev);
> + if (ret < 0)
> return ret;
> - }
This change (and the change below) is correct, as the logic before/after
is equivalent.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Sat, Apr 24, 2021 at 10:24:54AM +0200, Jacopo Mondi wrote:
> Hi Mauro,
>
> On Sat, Apr 24, 2021 at 08:44:48AM +0200, Mauro Carvalho Chehab wrote:
> > Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> > added pm_runtime_resume_and_get() in order to automatically handle
> > dev->power.usage_count decrement on errors.
> >
> > Use the new API, in order to cleanup the error check logic.
> >
> > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
>
> Thanks
> Acked-by: Jacopo Mondi <[email protected]>
>
> I should re-work the error handling sequence there on top of this
> patch as right now it's not the best, that 'done' label bothers me...
> anyway, for later.
>
> > ---
> > drivers/media/i2c/mt9m001.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > index 3b0ba8ed5233..57e15a291ebd 100644
> > --- a/drivers/media/i2c/mt9m001.c
> > +++ b/drivers/media/i2c/mt9m001.c
> > @@ -217,9 +217,9 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
> > goto done;
> >
> > if (enable) {
> > - ret = pm_runtime_get_sync(&client->dev);
> > + ret = pm_runtime_resume_and_get(&client->dev);
> > if (ret < 0)
> > - goto put_unlock;
> > + goto unlock;
> >
> > ret = mt9m001_apply_selection(sd);
> > if (ret)
> > @@ -247,6 +247,7 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
> >
> > put_unlock:
> > pm_runtime_put(&client->dev);
> > +unlock:
> > mutex_unlock(&mt9m001->mutex);
> >
> > return ret;
> > @@ -834,7 +835,7 @@ static int mt9m001_remove(struct i2c_client *client)
> > {
> > struct mt9m001 *mt9m001 = to_mt9m001(client);
> >
> > - pm_runtime_get_sync(&client->dev);
> > + pm_runtime_resume_and_get(&client->dev);
> >
> > v4l2_async_unregister_subdev(&mt9m001->subdev);
> > media_entity_cleanup(&mt9m001->subdev.entity);
I couldn't help looking at one more now that you got feedback on this
one.
Here you have the same problem as the one I reported earlier, in that
the usage count could end up negative on resume failure due to the later
put_noidle() call in remove().
Also note that you're adding more lines than you're removing.
I'd say this kind of mass-conversion is of questionable worth as
pm_runtime_resume_and_get() isn't necessarily an improvement (even if it
may have its use in some places).
Johan
Hi Mauro,
I love your patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on next-20210423]
[cannot apply to rockchip/for-next tegra/for-next v5.12-rc8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/media-use-pm_runtime_resume_and_get-instead-of-pm_runtime_get_sync/20210424-145029
base: git://linuxtv.org/media_tree.git master
config: nios2-randconfig-s031-20210424 (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/ddf22a45db8814ab855d3535a125e17f5e58f94b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Mauro-Carvalho-Chehab/media-use-pm_runtime_resume_and_get-instead-of-pm_runtime_get_sync/20210424-145029
git checkout ddf22a45db8814ab855d3535a125e17f5e58f94b
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=nios2
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c: In function 'sun4i_csi_open':
>> drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c:215:3: error: label 'err_pm_put' used but not defined
215 | goto err_pm_put;
| ^~~~
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for SERIAL_CORE_CONSOLE
Depends on TTY && HAS_IOMEM
Selected by
- EARLY_PRINTK
vim +/err_pm_put +215 drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
577bbf23b758848 Maxime Ripard 2019-08-22 199
577bbf23b758848 Maxime Ripard 2019-08-22 200 static int sun4i_csi_open(struct file *file)
577bbf23b758848 Maxime Ripard 2019-08-22 201 {
577bbf23b758848 Maxime Ripard 2019-08-22 202 struct sun4i_csi *csi = video_drvdata(file);
577bbf23b758848 Maxime Ripard 2019-08-22 203 int ret;
577bbf23b758848 Maxime Ripard 2019-08-22 204
577bbf23b758848 Maxime Ripard 2019-08-22 205 ret = mutex_lock_interruptible(&csi->lock);
577bbf23b758848 Maxime Ripard 2019-08-22 206 if (ret)
577bbf23b758848 Maxime Ripard 2019-08-22 207 return ret;
577bbf23b758848 Maxime Ripard 2019-08-22 208
ddf22a45db8814a Mauro Carvalho Chehab 2021-04-24 209 ret = pm_runtime_resume_and_get(csi->dev);
577bbf23b758848 Maxime Ripard 2019-08-22 210 if (ret < 0)
ddf22a45db8814a Mauro Carvalho Chehab 2021-04-24 211 goto err_unlock;
577bbf23b758848 Maxime Ripard 2019-08-22 212
8fd390b89cc8ca7 Ezequiel Garcia 2020-01-24 213 ret = v4l2_pipeline_pm_get(&csi->vdev.entity);
577bbf23b758848 Maxime Ripard 2019-08-22 214 if (ret)
577bbf23b758848 Maxime Ripard 2019-08-22 @215 goto err_pm_put;
577bbf23b758848 Maxime Ripard 2019-08-22 216
577bbf23b758848 Maxime Ripard 2019-08-22 217 ret = v4l2_fh_open(file);
577bbf23b758848 Maxime Ripard 2019-08-22 218 if (ret)
577bbf23b758848 Maxime Ripard 2019-08-22 219 goto err_pipeline_pm_put;
577bbf23b758848 Maxime Ripard 2019-08-22 220
577bbf23b758848 Maxime Ripard 2019-08-22 221 mutex_unlock(&csi->lock);
577bbf23b758848 Maxime Ripard 2019-08-22 222
577bbf23b758848 Maxime Ripard 2019-08-22 223 return 0;
577bbf23b758848 Maxime Ripard 2019-08-22 224
577bbf23b758848 Maxime Ripard 2019-08-22 225 err_pipeline_pm_put:
8fd390b89cc8ca7 Ezequiel Garcia 2020-01-24 226 v4l2_pipeline_pm_put(&csi->vdev.entity);
577bbf23b758848 Maxime Ripard 2019-08-22 227
ddf22a45db8814a Mauro Carvalho Chehab 2021-04-24 228 err_unlock:
577bbf23b758848 Maxime Ripard 2019-08-22 229 mutex_unlock(&csi->lock);
577bbf23b758848 Maxime Ripard 2019-08-22 230
577bbf23b758848 Maxime Ripard 2019-08-22 231 return ret;
577bbf23b758848 Maxime Ripard 2019-08-22 232 }
577bbf23b758848 Maxime Ripard 2019-08-22 233
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi Mauro,
On 24.04.2021 08:45, Mauro Carvalho Chehab wrote:
> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> added pm_runtime_resume_and_get() in order to automatically handle
> dev->power.usage_count decrement on errors.
>
> Use the new API, in order to cleanup the error check logic.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> drivers/media/platform/s3c-camif/camif-capture.c | 5 ++---
> drivers/media/platform/s3c-camif/camif-core.c | 5 +++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
> index 9ca49af29542..01fa08065ebc 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -547,16 +547,15 @@ static int s3c_camif_open(struct file *file)
> if (ret < 0)
> goto unlock;
>
> - ret = pm_runtime_get_sync(camif->dev);
> + ret = pm_runtime_resume_and_get(camif->dev);
> if (ret < 0)
> - goto err_pm;
> + goto unlock;
I think we don't need that label change, fh still needs to be released
in case of an error.
> ret = sensor_set_power(camif, 1);
> if (!ret)
> goto unlock;
>
> pm_runtime_put(camif->dev);
> -err_pm:
> v4l2_fh_release(file);
> unlock:
> mutex_unlock(&camif->lock);
--
Regards,
Sylwester
Hi Mauro,
On Sat, 2021-04-24 at 08:45 +0200, Mauro Carvalho Chehab wrote:
> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> added pm_runtime_resume_and_get() in order to automatically handle
> dev->power.usage_count decrement on errors.
>
> Use the new API, in order to cleanup the error check logic.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> drivers/media/platform/coda/coda-common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index bd666c858fa1..fea1c136a42c 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -2660,7 +2660,7 @@ static int coda_open(struct file *file)
> ctx->use_vdoa = false;
>
> /* Power up and upload firmware if necessary */
> - ret = pm_runtime_get_sync(dev->dev);
> + ret = pm_runtime_resume_and_get(dev->dev);
> if (ret < 0) {
> v4l2_err(&dev->v4l2_dev, "failed to power up: %d\n", ret);
> goto err_pm_get;
There's a pm_runtime_put_sync() at the err_pm_get below, so the label
needs to be moved down a line:
@@ -2705,8 +2705,8 @@ static int coda_open(struct file *file)
clk_disable_unprepare(dev->clk_ahb);
err_clk_ahb:
clk_disable_unprepare(dev->clk_per);
-err_pm_get:
pm_runtime_put_sync(dev->dev);
+err_pm_get:
v4l2_fh_del(&ctx->fh);
v4l2_fh_exit(&ctx->fh);
err_coda_name_init:
regards
Philipp
Hi Mauro,
Thanks for the patch.
On Sat, 2021-04-24 at 08:44 +0200, Mauro Carvalho Chehab wrote:
> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
> deal with usage counter")
> added pm_runtime_resume_and_get() in order to automatically handle
> dev->power.usage_count decrement on errors.
>
> Use the new API, in order to cleanup the error check logic.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> drivers/media/i2c/imx334.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> index 047aa7658d21..a5a03bb4a6ae 100644
> --- a/drivers/media/i2c/imx334.c
> +++ b/drivers/media/i2c/imx334.c
> @@ -717,9 +717,9 @@ static int imx334_set_stream(struct v4l2_subdev
> *sd, int enable)
> }
>
> if (enable) {
> - ret = pm_runtime_get_sync(imx334->dev);
> + ret = pm_runtime_resume_and_get(imx334->dev);
> if (ret)
> - goto error_power_off;
> + goto error_unlock;
>
> ret = imx334_start_streaming(imx334);
> if (ret)
> @@ -737,6 +737,7 @@ static int imx334_set_stream(struct v4l2_subdev
> *sd, int enable)
>
> error_power_off:
> pm_runtime_put(imx334->dev);
> +error_unlock:
> mutex_unlock(&imx334->mutex);
>
> return ret;
Looks good to me.
Reviewed-by: Daniele Alessandrelli <[email protected]>
Em Sat, 24 Apr 2021 11:12:31 +0200
Geert Uytterhoeven <[email protected]> escreveu:
> Hi Mauro,
>
> On Sat, Apr 24, 2021 at 8:46 AM Mauro Carvalho Chehab
> <[email protected]> wrote:
> > Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> > added pm_runtime_resume_and_get() in order to automatically handle
> > dev->power.usage_count decrement on errors.
> >
> > Use the new API, in order to cleanup the error check logic.
> >
> > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
>
> Thanks for your patch!
>
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -408,7 +408,7 @@ static void rcsi2_enter_standby(struct rcar_csi2 *priv)
> >
> > static void rcsi2_exit_standby(struct rcar_csi2 *priv)
> > {
> > - pm_runtime_get_sync(priv->dev);
> > + pm_runtime_resume_and_get(priv->dev);
>
> I believe this part is incorrect: on failure[*], the refcount will now
> be decremented, and in a subsequent call to rcsi2_enter_standby(), the
> refcount will be decremented again due to the call to pm_runtime_put().
I see your point.
> [*] On e.g. R-Car SoCs, this can never fail. This is the reason why
> many R-Car (and SuperH) drivers never check the result of
> pm_runtime_get_sync(). So the refcount "imbalances" were actually
> introduced by the various "clean up" patches to add return value
> checking, which now need another round of fixing...
It sounds dangerous to assume that. I'm not a PM expert, but, at least
looking at drivers/base/power/runtime.c, there are two situations where the
core itself will return an error, even if the PM callback never return
errors[1], and more could be added in the future:
if (dev->power.runtime_error)
retval = -EINVAL;
else if (dev->power.disable_depth > 0)
retval = -EACCES;
[1] see rpm_resume() function
IMO, the right thing here would be to check it at resume time,
and to handle it properly.
>
> > reset_control_deassert(priv->rstc);
> > }
>
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -1458,11 +1458,9 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
> > u32 vnmc;
> > int ret;
> >
> > - ret = pm_runtime_get_sync(vin->dev);
> > - if (ret < 0) {
> > - pm_runtime_put_noidle(vin->dev);
> > + ret = pm_runtime_resume_and_get(vin->dev);
> > + if (ret < 0)
> > return ret;
> > - }
>
> This change (and the change below) is correct, as the logic before/after
> is equivalent.
>
> Gr{oetje,eeting}s,
>
> Geert
>
Thanks,
Mauro
Em Sat, 24 Apr 2021 12:00:46 +0200
Johan Hovold <[email protected]> escreveu:
> On Sat, Apr 24, 2021 at 10:24:54AM +0200, Jacopo Mondi wrote:
> > Hi Mauro,
> >
> > On Sat, Apr 24, 2021 at 08:44:48AM +0200, Mauro Carvalho Chehab wrote:
> > > Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> > > added pm_runtime_resume_and_get() in order to automatically handle
> > > dev->power.usage_count decrement on errors.
> > >
> > > Use the new API, in order to cleanup the error check logic.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> >
> > Thanks
> > Acked-by: Jacopo Mondi <[email protected]>
> >
> > I should re-work the error handling sequence there on top of this
> > patch as right now it's not the best, that 'done' label bothers me...
> > anyway, for later.
> >
> > > ---
> > > drivers/media/i2c/mt9m001.c | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > > index 3b0ba8ed5233..57e15a291ebd 100644
> > > --- a/drivers/media/i2c/mt9m001.c
> > > +++ b/drivers/media/i2c/mt9m001.c
> > > @@ -217,9 +217,9 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
> > > goto done;
> > >
> > > if (enable) {
> > > - ret = pm_runtime_get_sync(&client->dev);
> > > + ret = pm_runtime_resume_and_get(&client->dev);
> > > if (ret < 0)
> > > - goto put_unlock;
> > > + goto unlock;
> > >
> > > ret = mt9m001_apply_selection(sd);
> > > if (ret)
> > > @@ -247,6 +247,7 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
> > >
> > > put_unlock:
> > > pm_runtime_put(&client->dev);
> > > +unlock:
> > > mutex_unlock(&mt9m001->mutex);
> > >
> > > return ret;
> > > @@ -834,7 +835,7 @@ static int mt9m001_remove(struct i2c_client *client)
> > > {
> > > struct mt9m001 *mt9m001 = to_mt9m001(client);
> > >
> > > - pm_runtime_get_sync(&client->dev);
> > > + pm_runtime_resume_and_get(&client->dev);
> > >
> > > v4l2_async_unregister_subdev(&mt9m001->subdev);
> > > media_entity_cleanup(&mt9m001->subdev.entity);
>
> I couldn't help looking at one more now that you got feedback on this
> one.
>
> Here you have the same problem as the one I reported earlier, in that
> the usage count could end up negative on resume failure due to the later
> put_noidle() call in remove().
I'll double-check this at the entire series. Different sensor
drivers are handling this on different ways, which sounds
bad, as they are meant to be independent on the media bridge
driver.
> Also note that you're adding more lines than you're removing.
Ok, but the end goal is not really reducing the number of lines,
but to have the code following the same pattern, and to avoid
cut-and-paste errors when new drivers are written.
The mt9m001 is one of the oldest sensor drivers, written a long time
before the PM runtime core, written for the soc_camera driver, back
in 2008. The port to use PM runtime isn't old:
commit 8fcfc491c6ca5887bb341b3a622cca3ed8e3c9f0
Author: Akinobu Mita <[email protected]>
Date: Tue Jan 8 12:51:44 2019 -0200
media: mt9m001: switch s_power callback to runtime PM
It was part of an attempt to recover the soc_camera sensor drives
from staging.
Yet, the logic on this driver seems to be different than the
one used on more modern I2C sensors. So, better to re-check
everything.
> I'd say this kind of mass-conversion is of questionable worth as
> pm_runtime_resume_and_get() isn't necessarily an improvement (even if it
> may have its use in some places).
The main problem is that other parts of the driver's core APIs
assume that get object methods will only increment the usage
counter if no errors. The pm_runtime_get_sync() is an exception.
Its name doesn't help at all: A function like that should, IMHO,
be called, instead:
pm_runtime_inc_usage_count_and_try_to_resume().
Or something similar, in order to make clearer that it always
increment the usage count, no matter what. If possible, all drivers
should get rid of it too (or alternatively add comments warning
people that keeping the usage_count incremented is desired on the
very specific places where it is really needed), as it is risky
to use something that has a different usage_count increement behavior
than other more usual *_get() functions.
With regards to mass-fixing it, I've seen several patches seen
to media fixing bugs due to the bad usage_count decrement logic.
So, the best is to solve them all at once, and stop using
pm_runtime_get_sync() inside the subsystem.
Thanks,
Mauro
On Mon, Apr 26, 2021 at 04:38:40PM +0200, Mauro Carvalho Chehab wrote:
> Em Sat, 24 Apr 2021 12:00:46 +0200
> Johan Hovold <[email protected]> escreveu:
>
> > On Sat, Apr 24, 2021 at 10:24:54AM +0200, Jacopo Mondi wrote:
> > > Hi Mauro,
> > >
> > > On Sat, Apr 24, 2021 at 08:44:48AM +0200, Mauro Carvalho Chehab wrote:
> > > > Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> > > > added pm_runtime_resume_and_get() in order to automatically handle
> > > > dev->power.usage_count decrement on errors.
> > > >
> > > > Use the new API, in order to cleanup the error check logic.
> > > >
> > > > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> > I'd say this kind of mass-conversion is of questionable worth as
> > pm_runtime_resume_and_get() isn't necessarily an improvement (even if it
> > may have its use in some places).
>
> The main problem is that other parts of the driver's core APIs
> assume that get object methods will only increment the usage
> counter if no errors. The pm_runtime_get_sync() is an exception.
>
> Its name doesn't help at all: A function like that should, IMHO,
> be called, instead:
>
> pm_runtime_inc_usage_count_and_try_to_resume().
>
> Or something similar, in order to make clearer that it always
> increment the usage count, no matter what. If possible, all drivers
> should get rid of it too (or alternatively add comments warning
> people that keeping the usage_count incremented is desired on the
> very specific places where it is really needed), as it is risky
> to use something that has a different usage_count increement behavior
> than other more usual *_get() functions.
pm_runtime_get_sync() has worked this way since it was merged 12 years
ago, and for someone who's used to this interface this is not such a big
deal as you seem to think. Sure, you need to remember to put the usage
counter on errors, but that's it (and the other side of that is that you
don't need to worry about error handling where it doesn't matter).
Also note all the pm_runtime_get functions *always* increment the usage
count even if an async resume may later fail so there is consistency
here.
And regarding naming, the new pm_resume_and_get() looks completely out
of place to me since it uses a different naming scheme than the other
helpers (including the ones that are used to balance the new call).
> With regards to mass-fixing it, I've seen several patches seen
> to media fixing bugs due to the bad usage_count decrement logic.
> So, the best is to solve them all at once, and stop using
> pm_runtime_get_sync() inside the subsystem.
Sure, having the script kiddies patch drivers without understanding what
they're are really doing is bound to introduce bugs unless it can be
caught in review.
You're call, but converting functioning drivers where the authors knew
what they were doing just because you're not used to the API and risk
introducing new bugs in the process isn't necessarily a good idea.
Especially since the pm_runtime_get_sync() will continue to be used
elsewhere, and possibly even in media in cases where you don't need to
check for errors (e.g. remove()).
Johan
Em Tue, 27 Apr 2021 14:23:20 +0200
Johan Hovold <[email protected]> escreveu:
> On Mon, Apr 26, 2021 at 04:38:40PM +0200, Mauro Carvalho Chehab wrote:
> > Em Sat, 24 Apr 2021 12:00:46 +0200
> > Johan Hovold <[email protected]> escreveu:
> >
> > > On Sat, Apr 24, 2021 at 10:24:54AM +0200, Jacopo Mondi wrote:
> > > > Hi Mauro,
> > > >
> > > > On Sat, Apr 24, 2021 at 08:44:48AM +0200, Mauro Carvalho Chehab wrote:
> > > > > Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> > > > > added pm_runtime_resume_and_get() in order to automatically handle
> > > > > dev->power.usage_count decrement on errors.
> > > > >
> > > > > Use the new API, in order to cleanup the error check logic.
> > > > >
> > > > > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
>
> > > I'd say this kind of mass-conversion is of questionable worth as
> > > pm_runtime_resume_and_get() isn't necessarily an improvement (even if it
> > > may have its use in some places).
> >
> > The main problem is that other parts of the driver's core APIs
> > assume that get object methods will only increment the usage
> > counter if no errors. The pm_runtime_get_sync() is an exception.
> >
> > Its name doesn't help at all: A function like that should, IMHO,
> > be called, instead:
> >
> > pm_runtime_inc_usage_count_and_try_to_resume().
> >
> > Or something similar, in order to make clearer that it always
> > increment the usage count, no matter what. If possible, all drivers
> > should get rid of it too (or alternatively add comments warning
> > people that keeping the usage_count incremented is desired on the
> > very specific places where it is really needed), as it is risky
> > to use something that has a different usage_count increement behavior
> > than other more usual *_get() functions.
>
> pm_runtime_get_sync() has worked this way since it was merged 12 years
> ago, and for someone who's used to this interface this is not such a big
> deal as you seem to think. Sure, you need to remember to put the usage
> counter on errors, but that's it (and the other side of that is that you
> don't need to worry about error handling where it doesn't matter).
Before we have those at PM subsystem, the media had its own way to
set/disable power for their sub-devices. The PCI and USB drivers
still use it, instead of pm_runtime, mostly due to historic reasons.
So, basically, its usage at the media subsystem is restricted to
drivers for embedded systems. The vast majority of drivers supporting
PM runtime are the I2C camera drivers. The camera drivers can be used
interchangeable. So, in practice, the same bridge driver can work
with a lot of different camera models, depending on the hardware
vendors' personal preferences and the desired max resolution.
So, in thesis, all such drivers should behave exactly the same
with regards to PM.
However, on most existing drivers, the pm_runtime was added a
couple of years ago, and by people that are not too familiar
with the PM subsystem.
That probably explains why there were/are several places that
do things like this[1]:
ret = pm_runtime_get_sync(dev);
if (ret < 0)
return ret;
without taking care of calling a pm_runtime_put*() function.
[1] besides the 13 patches made by UCN addressing it on
existing code, I discovered the same pattern on a
couple of other drivers with current upstream code.
That shows a pattern: several media developers are not familiar
with the usage_count behavior for pm_runtime_get functions.
So, doing this work is not only helping to make the PM support
more uniform, but it is also helping to solve existing issues.
> Also note all the pm_runtime_get functions *always* increment the usage
> count even if an async resume may later fail so there is consistency
> here.
>
> And regarding naming, the new pm_resume_and_get() looks completely out
> of place to me since it uses a different naming scheme than the other
> helpers (including the ones that are used to balance the new call).
>
> > With regards to mass-fixing it, I've seen several patches seen
> > to media fixing bugs due to the bad usage_count decrement logic.
> > So, the best is to solve them all at once, and stop using
> > pm_runtime_get_sync() inside the subsystem.
>
> Sure, having the script kiddies patch drivers without understanding what
> they're are really doing is bound to introduce bugs unless it can be
> caught in review.
Yes, but as I pointed, the current code has bugs already.
> You're call, but converting functioning drivers where the authors knew
> what they were doing just because you're not used to the API and risk
> introducing new bugs in the process isn't necessarily a good idea.
The problem is that the above assumption is not necessarily true:
based on the number of drivers that pm_runtime_get_sync() weren't
decrementing usage_count on errors, several driver authors were not
familiar enough with the PM runtime behavior by the time the drivers
were written or converted to use the PM runtime, instead of the
media .s_power()/.s_stream() callbacks.
> Especially since the pm_runtime_get_sync() will continue to be used
> elsewhere, and possibly even in media in cases where you don't need to
> check for errors (e.g. remove()).
Talking about the remove(), I'm not sure if just ignoring errors
there would do the right thing. I mean, if pm_runtime_get_sync()
fails, probably any attempts to disable clocks and other things
that depend on PM runtime will also (silently) fail.
This may put the device on an unknown PM and keep clock lines enabled
after its removal.
Thanks,
Mauro
On Wed, Apr 28, 2021 at 10:31:48AM +0200, Mauro Carvalho Chehab wrote:
> Em Tue, 27 Apr 2021 14:23:20 +0200
> Johan Hovold <[email protected]> escreveu:
> > pm_runtime_get_sync() has worked this way since it was merged 12 years
> > ago, and for someone who's used to this interface this is not such a big
> > deal as you seem to think. Sure, you need to remember to put the usage
> > counter on errors, but that's it (and the other side of that is that you
> > don't need to worry about error handling where it doesn't matter).
>
> Before we have those at PM subsystem, the media had its own way to
> set/disable power for their sub-devices. The PCI and USB drivers
> still use it, instead of pm_runtime, mostly due to historic reasons.
>
> So, basically, its usage at the media subsystem is restricted to
> drivers for embedded systems. The vast majority of drivers supporting
> PM runtime are the I2C camera drivers. The camera drivers can be used
> interchangeable. So, in practice, the same bridge driver can work
> with a lot of different camera models, depending on the hardware
> vendors' personal preferences and the desired max resolution.
>
> So, in thesis, all such drivers should behave exactly the same
> with regards to PM.
>
> However, on most existing drivers, the pm_runtime was added a
> couple of years ago, and by people that are not too familiar
> with the PM subsystem.
>
> That probably explains why there were/are several places that
> do things like this[1]:
>
> ret = pm_runtime_get_sync(dev);
> if (ret < 0)
> return ret;
>
> without taking care of calling a pm_runtime_put*() function.
>
> [1] besides the 13 patches made by UCN addressing it on
> existing code, I discovered the same pattern on a
> couple of other drivers with current upstream code.
>
> That shows a pattern: several media developers are not familiar
> with the usage_count behavior for pm_runtime_get functions.
>
> So, doing this work is not only helping to make the PM support
> more uniform, but it is also helping to solve existing issues.
Sure, I don't doubt that there are issues with the current code too.
> > You're call, but converting functioning drivers where the authors knew
> > what they were doing just because you're not used to the API and risk
> > introducing new bugs in the process isn't necessarily a good idea.
>
> The problem is that the above assumption is not necessarily true:
> based on the number of drivers that pm_runtime_get_sync() weren't
> decrementing usage_count on errors, several driver authors were not
> familiar enough with the PM runtime behavior by the time the drivers
> were written or converted to use the PM runtime, instead of the
> media .s_power()/.s_stream() callbacks.
That may very well be the case. My point is just that this work needs to
be done carefully and by people familiar with the code (and runtime pm)
or you risk introducing new issues.
I really don't want the bot-warning-suppression crew to start with this
for example.
> > Especially since the pm_runtime_get_sync() will continue to be used
> > elsewhere, and possibly even in media in cases where you don't need to
> > check for errors (e.g. remove()).
>
> Talking about the remove(), I'm not sure if just ignoring errors
> there would do the right thing. I mean, if pm_runtime_get_sync()
> fails, probably any attempts to disable clocks and other things
> that depend on PM runtime will also (silently) fail.
>
> This may put the device on an unknown PM and keep clock lines enabled
> after its removal.
Right, a resume failure is a pretty big issue and it's not really clear
how to to even handle that generally. But at remove() time you don't
have much choice but to go on and release resource anyway.
So unless actually implementing some error handling too, using
pm_runtime_sync_get() without checking for errors is still preferred
over pm_runtime_resume_and_get(). That is
pm_runtime_get_sync();
/* cleanup */
pm_runtime_disable()
pm_runtime_put_noidle();
is better than:
ret = pm_runtime_resume_and_get();
/* cleanup */
pm_runtime_disable();
if (ret == 0)
pm_runtime_put_noidle();
unless you also start doing something ret.
Johan
There was a Smatch check for these bugs. This was a good source of
recurring Reported-by tags for me. ;) Thanks for doing this.
regards,
dan carpenter
Em Wed, 28 Apr 2021 12:05:26 +0200
Johan Hovold <[email protected]> escreveu:
> On Wed, Apr 28, 2021 at 10:31:48AM +0200, Mauro Carvalho Chehab wrote:
> > Em Tue, 27 Apr 2021 14:23:20 +0200
> > Johan Hovold <[email protected]> escreveu:
>
> > > You're call, but converting functioning drivers where the authors knew
> > > what they were doing just because you're not used to the API and risk
> > > introducing new bugs in the process isn't necessarily a good idea.
> >
> > The problem is that the above assumption is not necessarily true:
> > based on the number of drivers that pm_runtime_get_sync() weren't
> > decrementing usage_count on errors, several driver authors were not
> > familiar enough with the PM runtime behavior by the time the drivers
> > were written or converted to use the PM runtime, instead of the
> > media .s_power()/.s_stream() callbacks.
>
> That may very well be the case. My point is just that this work needs to
> be done carefully and by people familiar with the code (and runtime pm)
> or you risk introducing new issues.
Yeah, that's for sure.
> I really don't want the bot-warning-suppression crew to start with this
> for example.
>
> > > Especially since the pm_runtime_get_sync() will continue to be used
> > > elsewhere, and possibly even in media in cases where you don't need to
> > > check for errors (e.g. remove()).
> >
> > Talking about the remove(), I'm not sure if just ignoring errors
> > there would do the right thing. I mean, if pm_runtime_get_sync()
> > fails, probably any attempts to disable clocks and other things
> > that depend on PM runtime will also (silently) fail.
> >
> > This may put the device on an unknown PM and keep clock lines enabled
> > after its removal.
>
> Right, a resume failure is a pretty big issue and it's not really clear
> how to to even handle that generally. But at remove() time you don't
> have much choice but to go on and release resource anyway.
>
> So unless actually implementing some error handling too, using
> pm_runtime_sync_get() without checking for errors is still preferred
> over pm_runtime_resume_and_get(). That is
>
> pm_runtime_get_sync();
> /* cleanup */
> pm_runtime_disable()
> pm_runtime_put_noidle();
>
> is better than:
>
> ret = pm_runtime_resume_and_get();
> /* cleanup */
> pm_runtime_disable();
> if (ret == 0)
> pm_runtime_put_noidle();
>
> unless you also start doing something ret.
Perhaps the best would be to use, instead:
pm_runtime_get_noresume();
/* cleanup */
pm_runtime_disable()
pm_runtime_put_noidle();
pm_runtime_set_suspended();
I mean, at least for my eyes, it doesn't make sense to do a PM
resume during driver's removal/unbind time.
Regards,
Mauro
>
> Johan
Thanks,
Mauro
On Wed, Apr 28, 2021 at 01:28:53PM +0200, Mauro Carvalho Chehab wrote:
> Em Wed, 28 Apr 2021 12:05:26 +0200
> Johan Hovold <[email protected]> escreveu:
> > Right, a resume failure is a pretty big issue and it's not really clear
> > how to to even handle that generally. But at remove() time you don't
> > have much choice but to go on and release resource anyway.
> >
> > So unless actually implementing some error handling too, using
> > pm_runtime_sync_get() without checking for errors is still preferred
> > over pm_runtime_resume_and_get(). That is
> >
> > pm_runtime_get_sync();
> > /* cleanup */
> > pm_runtime_disable()
> > pm_runtime_put_noidle();
> >
> > is better than:
> >
> > ret = pm_runtime_resume_and_get();
> > /* cleanup */
> > pm_runtime_disable();
> > if (ret == 0)
> > pm_runtime_put_noidle();
> >
> > unless you also start doing something ret.
>
> Perhaps the best would be to use, instead:
>
> pm_runtime_get_noresume();
> /* cleanup */
> pm_runtime_disable()
> pm_runtime_put_noidle();
> pm_runtime_set_suspended();
>
> I mean, at least for my eyes, it doesn't make sense to do a PM
> resume during driver's removal/unbind time.
The cleanup bit above would depend on the device being resumed so using
pm_runtime_get_noresume() makes little sense.
Some drivers disable clocks etc explicitly at remove instead of relying
on pm runtime for that and then they'd use the above scheme (plus
explicit pm_runtime_set_suspended()).
Johan
On Sat, 2021-04-24 at 08:45 +0200, Mauro Carvalho Chehab wrote:
> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> added pm_runtime_resume_and_get() in order to automatically handle
> dev->power.usage_count decrement on errors.
>
> Use the new API, in order to cleanup the error check logic.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>
Thanks,
> ---
> drivers/media/platform/rockchip/rga/rga-buf.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
> index bf9a75b75083..81508ed5abf3 100644
> --- a/drivers/media/platform/rockchip/rga/rga-buf.c
> +++ b/drivers/media/platform/rockchip/rga/rga-buf.c
> @@ -79,9 +79,8 @@ static int rga_buf_start_streaming(struct vb2_queue *q, unsigned int count)
> struct rockchip_rga *rga = ctx->rga;
> int ret;
>
> - ret = pm_runtime_get_sync(rga->dev);
> + ret = pm_runtime_resume_and_get(rga->dev);
> if (ret < 0) {
> - pm_runtime_put_noidle(rga->dev);
> rga_buf_return_buffers(q, VB2_BUF_STATE_QUEUED);
> return ret;
> }