| Message ID | 20210511124437.9930-1-p.yadav@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: i2c: ov5648: Plug runtime pm counter leak | expand |
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
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); + return ret; + } state->streaming = !!enable;
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(-) -- 2.30.0