* [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync()
@ 2021-04-24 6:44 Mauro Carvalho Chehab
2021-04-24 6:44 ` [PATCH 04/78] media: rcar_fdp1: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-24 6:44 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Lad, Prabhakar,
Paul J. Murphy, Alexandre Torgue, Andrzej Hajda,
Andrzej Pietrasiewicz, Andy Gross, Benoit Parrot, Bingbu Cao,
Bjorn Andersson, Chen-Yu Tsai, Chiranjeevi Rapolu,
Dafna Hirschfeld, Dan Scally, Daniele Alessandrelli,
Dave Stevenson, Dmitry Osipenko, Dongchun Zhu, Ezequiel Garcia,
Fabio Estevam, Heiko Stuebner, Helen Koike, Hyungwoo Yang,
Jacek Anaszewski, Jacob Chen, Jacopo Mondi, Jernej Skrabec,
Jonathan Hunter, Krzysztof Kozlowski, Leon Luo,
Manivannan Sadhasivam, Marek Szyprowski, Matt Ranostay,
Matthias Brugger, Mauro Carvalho Chehab, Maxime Coquelin,
Maxime Ripard, NXP Linux Team, Paul Kocialkowski,
Pengutronix Kernel Team, Philipp Zabel, Ricardo Ribalda,
Robert Foss, Rui Miguel Silva, Sakari Ailus, Sascha Hauer,
Shawn Guo, Shawn Tu, Shunqian Zheng, Sowjanya Komatineni,
Stanimir Varbanov, Steve Longerbeam, Sylwester Nawrocki,
Sylwester Nawrocki, Thierry Reding, Tianshu Qiu, Todor Tomov,
Wenyou Yang, Yong Zhi, devel, linux-arm-kernel, linux-arm-msm,
linux-kernel, linux-media, linux-mediatek, linux-renesas-soc,
linux-rockchip, linux-samsung-soc, linux-stm32, linux-tegra
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 04/78] media: rcar_fdp1: fix pm_runtime_get_sync() usage count
2021-04-24 6:44 [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Mauro Carvalho Chehab
@ 2021-04-24 6:44 ` Mauro Carvalho Chehab
2021-04-24 6:44 ` [PATCH 06/78] media: renesas-ceu: " Mauro Carvalho Chehab
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-24 6:44 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Kieran Bingham,
Mauro Carvalho Chehab, linux-kernel, linux-media,
linux-renesas-soc
The pm_runtime_get_sync() internally increments the
dev->power.usage_count without decrementing it, even on errors.
replace it by the new pm_runtime_resume_and_get(), introduced by:
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
in order to properly decrement the usage counter and avoid memory
leaks.
Also, right now, the driver is ignoring any troubles when
trying to do PM resume. So, add the proper error handling
for the code.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/media/platform/rcar_fdp1.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c
index 01c1fbb97bf6..c32d237af618 100644
--- a/drivers/media/platform/rcar_fdp1.c
+++ b/drivers/media/platform/rcar_fdp1.c
@@ -2140,7 +2140,13 @@ static int fdp1_open(struct file *file)
}
/* Perform any power management required */
- pm_runtime_get_sync(fdp1->dev);
+ ret = pm_runtime_resume_and_get(fdp1->dev);
+ if (ret < 0) {
+ v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
+ v4l2_ctrl_handler_free(&ctx->hdl);
+ kfree(ctx);
+ goto done;
+ }
v4l2_fh_add(&ctx->fh);
@@ -2351,7 +2357,9 @@ static int fdp1_probe(struct platform_device *pdev)
/* Power up the cells to read HW */
pm_runtime_enable(&pdev->dev);
- pm_runtime_get_sync(fdp1->dev);
+ ret = pm_runtime_resume_and_get(fdp1->dev);
+ if (ret < 0)
+ return ret;
hw_version = fdp1_read(fdp1, FD1_IP_INTDATA);
switch (hw_version) {
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 06/78] media: renesas-ceu: fix pm_runtime_get_sync() usage count
2021-04-24 6:44 [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Mauro Carvalho Chehab
2021-04-24 6:44 ` [PATCH 04/78] media: rcar_fdp1: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
@ 2021-04-24 6:44 ` Mauro Carvalho Chehab
2021-04-24 8:11 ` Jacopo Mondi
2021-04-24 6:45 ` [PATCH 68/78] media: rcar-fcp: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-24 6:44 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Jacopo Mondi,
Mauro Carvalho Chehab, linux-kernel, linux-media,
linux-renesas-soc
The pm_runtime_get_sync() internally increments the
dev->power.usage_count without decrementing it, even on errors.
replace it by the new pm_runtime_resume_and_get(), introduced by:
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
in order to properly decrement the usage counter and avoid memory
leaks.
While here, check if the PM runtime was caught at open time.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/media/platform/renesas-ceu.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
index cd137101d41e..965a7259e707 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -1099,7 +1099,10 @@ static int ceu_open(struct file *file)
mutex_lock(&ceudev->mlock);
/* Causes soft-reset and sensor power on on first open */
- pm_runtime_get_sync(ceudev->dev);
+ ret = pm_runtime_resume_and_get(ceudev->dev);
+ if (ret < 0)
+ return ret;
+
mutex_unlock(&ceudev->mlock);
return 0;
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 68/78] media: rcar-fcp: use pm_runtime_resume_and_get()
2021-04-24 6:44 [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Mauro Carvalho Chehab
2021-04-24 6:44 ` [PATCH 04/78] media: rcar_fdp1: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-04-24 6:44 ` [PATCH 06/78] media: renesas-ceu: " Mauro Carvalho Chehab
@ 2021-04-24 6:45 ` Mauro Carvalho Chehab
2021-04-24 6:45 ` [PATCH 69/78] media: rcar-vin: " Mauro Carvalho Chehab
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-24 6:45 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Laurent Pinchart,
Mauro Carvalho Chehab, linux-kernel, linux-media,
linux-renesas-soc
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 <mchehab+huawei@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 69/78] media: rcar-vin: use pm_runtime_resume_and_get()
2021-04-24 6:44 [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Mauro Carvalho Chehab
` (2 preceding siblings ...)
2021-04-24 6:45 ` [PATCH 68/78] media: rcar-fcp: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
@ 2021-04-24 6:45 ` Mauro Carvalho Chehab
2021-04-24 8:33 ` Niklas Söderlund
2021-04-24 9:12 ` Geert Uytterhoeven
2021-04-24 6:45 ` [PATCH 78/78] media: vsp1: " Mauro Carvalho Chehab
2021-04-28 10:13 ` [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Dan Carpenter
5 siblings, 2 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-24 6:45 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab,
Niklas Söderlund, Mauro Carvalho Chehab, linux-kernel,
linux-media, linux-renesas-soc
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 <mchehab+huawei@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 78/78] media: vsp1: use pm_runtime_resume_and_get()
2021-04-24 6:44 [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Mauro Carvalho Chehab
` (3 preceding siblings ...)
2021-04-24 6:45 ` [PATCH 69/78] media: rcar-vin: " Mauro Carvalho Chehab
@ 2021-04-24 6:45 ` Mauro Carvalho Chehab
2021-04-28 10:13 ` [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Dan Carpenter
5 siblings, 0 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-24 6:45 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Kieran Bingham,
Laurent Pinchart, Mauro Carvalho Chehab, linux-kernel,
linux-media, linux-renesas-soc
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 <mchehab+huawei@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 06/78] media: renesas-ceu: fix pm_runtime_get_sync() usage count
2021-04-24 6:44 ` [PATCH 06/78] media: renesas-ceu: " Mauro Carvalho Chehab
@ 2021-04-24 8:11 ` Jacopo Mondi
0 siblings, 0 replies; 11+ messages in thread
From: Jacopo Mondi @ 2021-04-24 8:11 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, linux-kernel,
linux-media, linux-renesas-soc
Hi Mauro,
On Sat, Apr 24, 2021 at 08:44:16AM +0200, Mauro Carvalho Chehab wrote:
> The pm_runtime_get_sync() internally increments the
> dev->power.usage_count without decrementing it, even on errors.
> replace it by the new pm_runtime_resume_and_get(), introduced by:
Nit: 'Replace' as it follows a full stop ?
> commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> in order to properly decrement the usage counter and avoid memory
> leaks.
>
> While here, check if the PM runtime was caught at open time.
Nit: Maybe "PM runtime error ..." or something similar as I'm missing the
subject of the phrase.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> drivers/media/platform/renesas-ceu.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
> index cd137101d41e..965a7259e707 100644
> --- a/drivers/media/platform/renesas-ceu.c
> +++ b/drivers/media/platform/renesas-ceu.c
> @@ -1099,7 +1099,10 @@ static int ceu_open(struct file *file)
>
> mutex_lock(&ceudev->mlock);
> /* Causes soft-reset and sensor power on on first open */
> - pm_runtime_get_sync(ceudev->dev);
> + ret = pm_runtime_resume_and_get(ceudev->dev);
> + if (ret < 0)
> + return ret;
The mutex should be released before returning
> +
> mutex_unlock(&ceudev->mlock);
>
> return 0;
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 69/78] media: rcar-vin: use pm_runtime_resume_and_get()
2021-04-24 6:45 ` [PATCH 69/78] media: rcar-vin: " Mauro Carvalho Chehab
@ 2021-04-24 8:33 ` Niklas Söderlund
2021-04-24 9:12 ` Geert Uytterhoeven
1 sibling, 0 replies; 11+ messages in thread
From: Niklas Söderlund @ 2021-04-24 8:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, linux-kernel,
linux-media, linux-renesas-soc
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 <mchehab+huawei@kernel.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 69/78] media: rcar-vin: use pm_runtime_resume_and_get()
2021-04-24 6:45 ` [PATCH 69/78] media: rcar-vin: " Mauro Carvalho Chehab
2021-04-24 8:33 ` Niklas Söderlund
@ 2021-04-24 9:12 ` Geert Uytterhoeven
2021-04-26 13:33 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2021-04-24 9:12 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linuxarm, mauro.chehab, Niklas Söderlund,
Mauro Carvalho Chehab, Linux Kernel Mailing List,
Linux Media Mailing List, Linux-Renesas
Hi Mauro,
On Sat, Apr 24, 2021 at 8:46 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> 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 <mchehab+huawei@kernel.org>
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 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 69/78] media: rcar-vin: use pm_runtime_resume_and_get()
2021-04-24 9:12 ` Geert Uytterhoeven
@ 2021-04-26 13:33 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-26 13:33 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linuxarm, mauro.chehab, Niklas Söderlund,
Mauro Carvalho Chehab, Linux Kernel Mailing List,
Linux Media Mailing List, Linux-Renesas
Em Sat, 24 Apr 2021 11:12:31 +0200
Geert Uytterhoeven <geert@linux-m68k.org> escreveu:
> Hi Mauro,
>
> On Sat, Apr 24, 2021 at 8:46 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> 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 <mchehab+huawei@kernel.org>
>
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync()
2021-04-24 6:44 [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Mauro Carvalho Chehab
` (4 preceding siblings ...)
2021-04-24 6:45 ` [PATCH 78/78] media: vsp1: " Mauro Carvalho Chehab
@ 2021-04-28 10:13 ` Dan Carpenter
5 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-04-28 10:13 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Shawn Tu, Ricardo Ribalda, Dafna Hirschfeld, Heiko Stuebner,
linuxarm, Todor Tomov, Bjorn Andersson, Andrzej Hajda, Lad,
Prabhakar, Thierry Reding, Pengutronix Kernel Team,
Dmitry Osipenko, linux-stm32, Andrzej Pietrasiewicz, Leon Luo,
Paul Kocialkowski, Mauro Carvalho Chehab, Dave Stevenson,
Matt Ranostay, Krzysztof Kozlowski, Jonathan Hunter,
linux-rockchip, Chen-Yu Tsai, Andy Gross, Matthias Brugger,
Dongchun Zhu, Sakari Ailus, Bingbu Cao, Marek Szyprowski,
Shunqian Zheng, Tianshu Qiu, NXP Linux Team, Philipp Zabel,
devel, Jacopo Mondi, Sylwester Nawrocki, linux-tegra,
Alexandre Torgue, Wenyou Yang, Manivannan Sadhasivam,
linux-arm-msm, Sascha Hauer, Steve Longerbeam, linux-media,
Maxime Ripard, Stanimir Varbanov, Benoit Parrot, Helen Koike,
linux-samsung-soc, linux-mediatek, Jacek Anaszewski,
mauro.chehab, Sylwester Nawrocki, Paul J. Murphy,
Ezequiel Garcia, Daniele Alessandrelli, Chiranjeevi Rapolu,
linux-arm-kernel, Jacob Chen, Jernej Skrabec, Hyungwoo Yang,
linux-kernel, Robert Foss, Dan Scally, Sowjanya Komatineni,
Maxime Coquelin, linux-renesas-soc, Yong Zhi, Shawn Guo
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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-04-28 10:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-24 6:44 [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Mauro Carvalho Chehab
2021-04-24 6:44 ` [PATCH 04/78] media: rcar_fdp1: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-04-24 6:44 ` [PATCH 06/78] media: renesas-ceu: " Mauro Carvalho Chehab
2021-04-24 8:11 ` Jacopo Mondi
2021-04-24 6:45 ` [PATCH 68/78] media: rcar-fcp: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
2021-04-24 6:45 ` [PATCH 69/78] media: rcar-vin: " Mauro Carvalho Chehab
2021-04-24 8:33 ` Niklas Söderlund
2021-04-24 9:12 ` Geert Uytterhoeven
2021-04-26 13:33 ` Mauro Carvalho Chehab
2021-04-24 6:45 ` [PATCH 78/78] media: vsp1: " Mauro Carvalho Chehab
2021-04-28 10:13 ` [PATCH 00/78] media: use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).