Messages in this thread | | | Date | Fri, 21 May 2021 17:00:38 +0300 | From | Sakari Ailus <> | Subject | Re: [PATCH] media: i2c: ov5648: Plug runtime pm counter leak |
| |
Hi Pratyush,
Thanks for the patch.
On Tue, May 11, 2021 at 06:14:37PM +0530, Pratyush Yadav wrote: > When the stream is being enabled, the runtime pm usage counter is > incremented. Then if ov5648_sw_standby() fails, the function returns > error without decrementing the counter, leaking it. > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > --- > > Hi, > > I spotted this when converting OV5640 driver to use runtime PM using > this driver as reference. I only have a very surface level understanding > of runtime PM system as of now so please review with that in mind. > > This patch is only compile-tested since I don't have the hardware with > me. > > drivers/media/i2c/ov5648.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/ov5648.c b/drivers/media/i2c/ov5648.c > index 3ecb4a3e8773..6aa2c950f505 100644 > --- a/drivers/media/i2c/ov5648.c > +++ b/drivers/media/i2c/ov5648.c > @@ -2143,8 +2143,12 @@ static int ov5648_s_stream(struct v4l2_subdev *subdev, int enable) > ret = ov5648_sw_standby(sensor, !enable); > mutex_unlock(&sensor->mutex); > > - if (ret) > + if (ret) { > + if (enable) > + pm_runtime_put(sensor->dev);
The function powers the sensor off at the end if streaming was to be disabled. You could do:
if (ret || !enable) pm_runtime_put(sensor->dev);
> + > return ret; > + } > > state->streaming = !!enable;
state->streaming is assigned when not holding the lock. Could you address that at the same time?
-- Kind regards,
Sakari Ailus
| |