| Message ID | 10ec24ee3e3a33a4ca5c431fc19d3441d59136c3.1619621413.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Address some issues with PM runtime at media subsystem | expand |
On Wed, 28 Apr 2021 16:51:27 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > The PM runtime get logic is currently broken, as it checks if > ret is zero instead of checking if it is an error code, > as reported by Dan Carpenter. > > While here, use the pm_runtime_resume_and_get() as added by: > 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. Perhaps also call out that a fail of pm_runtime_get_sync in current code would potentially result in a spurious runtime_suspend() call whereas using pm_runtime_resume_and_get() will call pm_runtime_put_noidle() > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Reviewed-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > drivers/media/i2c/imx334.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > index 047aa7658d21..23f28606e570 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); > - if (ret) > - goto error_power_off; > + ret = pm_runtime_resume_and_get(imx334->dev); > + if (ret < 0) > + 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;
diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c index 047aa7658d21..23f28606e570 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); - if (ret) - goto error_power_off; + ret = pm_runtime_resume_and_get(imx334->dev); + if (ret < 0) + 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;
The PM runtime get logic is currently broken, as it checks if ret is zero instead of checking if it is an error code, as reported by Dan Carpenter. While here, use the pm_runtime_resume_and_get() as added by: 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. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> --- drivers/media/i2c/imx334.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)