* [PATCH v2 0/2] media: i2c: imx219: Trivial Fixes
@ 2021-03-11 9:52 Lad Prabhakar
2021-03-11 9:52 ` [PATCH v2 1/2] media: i2c: imx219: Move out locking/unlocking of vflip and hflip controls from imx219_set_stream Lad Prabhakar
2021-03-11 9:52 ` [PATCH v2 2/2] media: i2c: imx219: Balance runtime PM use-count Lad Prabhakar
0 siblings, 2 replies; 5+ messages in thread
From: Lad Prabhakar @ 2021-03-11 9:52 UTC (permalink / raw)
To: Dave Stevenson, Mauro Carvalho Chehab, Laurent Pinchart
Cc: Pavel Machek, Andrey Konovalov, Sakari Ailus, Jacopo Mondi,
Hans Verkuil, linux-media, linux-kernel, linux-renesas-soc,
Prabhakar, Lad Prabhakar, Biju Das
Hi All,
This patch series fixes trivial issues found in imx219 driver.
Cheers,
Prabhakar
Changes for v2:
* Dropped serialization patch
* Moved locking/unlocking of controls to imx219_start/imx219_stop as
suggested by Laurent.
* Moved incrementing/decrementing to imx219_start/imx219_stop as
suggested by Laurent.
Lad Prabhakar (2):
media: i2c: imx219: Move out locking/unlocking of vflip and hflip
controls from imx219_set_stream
media: i2c: imx219: Balance runtime PM use-count
drivers/media/i2c/imx219.c | 49 ++++++++++++++++++++++----------------
1 file changed, 29 insertions(+), 20 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 1/2] media: i2c: imx219: Move out locking/unlocking of vflip and hflip controls from imx219_set_stream 2021-03-11 9:52 [PATCH v2 0/2] media: i2c: imx219: Trivial Fixes Lad Prabhakar @ 2021-03-11 9:52 ` Lad Prabhakar 2021-03-15 0:02 ` Laurent Pinchart 2021-03-11 9:52 ` [PATCH v2 2/2] media: i2c: imx219: Balance runtime PM use-count Lad Prabhakar 1 sibling, 1 reply; 5+ messages in thread From: Lad Prabhakar @ 2021-03-11 9:52 UTC (permalink / raw) To: Dave Stevenson, Mauro Carvalho Chehab, Laurent Pinchart Cc: Pavel Machek, Andrey Konovalov, Sakari Ailus, Jacopo Mondi, Hans Verkuil, linux-media, linux-kernel, linux-renesas-soc, Prabhakar, Lad Prabhakar, Biju Das Move out locking/unlocking of vflip and hflip controls from imx219_set_stream() to the imx219_start_streaming()/ imx219_stop_streaming() respectively. This fixes an issue in resume callback error path where streaming is stopped and the controls are left in locked state. Fixes: 1283b3b8f82b9 ("media: i2c: Add driver for Sony IMX219 sensor") Reported-by: Pavel Machek <pavel@denx.de> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- drivers/media/i2c/imx219.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index 6e3382b85a90..82756cbfbaac 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -1056,8 +1056,16 @@ static int imx219_start_streaming(struct imx219 *imx219) return ret; /* set stream on register */ - return imx219_write_reg(imx219, IMX219_REG_MODE_SELECT, - IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING); + ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT, + IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING); + if (ret) + return ret; + + /* vflip and hflip cannot change during streaming */ + __v4l2_ctrl_grab(imx219->vflip, true); + __v4l2_ctrl_grab(imx219->hflip, true); + + return 0; } static void imx219_stop_streaming(struct imx219 *imx219) @@ -1070,6 +1078,9 @@ static void imx219_stop_streaming(struct imx219 *imx219) IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY); if (ret) dev_err(&client->dev, "%s failed to set stream\n", __func__); + + __v4l2_ctrl_grab(imx219->vflip, false); + __v4l2_ctrl_grab(imx219->hflip, false); } static int imx219_set_stream(struct v4l2_subdev *sd, int enable) @@ -1105,10 +1116,6 @@ static int imx219_set_stream(struct v4l2_subdev *sd, int enable) imx219->streaming = enable; - /* vflip and hflip cannot change during streaming */ - __v4l2_ctrl_grab(imx219->vflip, enable); - __v4l2_ctrl_grab(imx219->hflip, enable); - mutex_unlock(&imx219->mutex); return ret; -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] media: i2c: imx219: Move out locking/unlocking of vflip and hflip controls from imx219_set_stream 2021-03-11 9:52 ` [PATCH v2 1/2] media: i2c: imx219: Move out locking/unlocking of vflip and hflip controls from imx219_set_stream Lad Prabhakar @ 2021-03-15 0:02 ` Laurent Pinchart 0 siblings, 0 replies; 5+ messages in thread From: Laurent Pinchart @ 2021-03-15 0:02 UTC (permalink / raw) To: Lad Prabhakar Cc: Dave Stevenson, Mauro Carvalho Chehab, Pavel Machek, Andrey Konovalov, Sakari Ailus, Jacopo Mondi, Hans Verkuil, linux-media, linux-kernel, linux-renesas-soc, Prabhakar, Biju Das Hi Prabhakar, Thank you for the patch. On Thu, Mar 11, 2021 at 09:52:04AM +0000, Lad Prabhakar wrote: > Move out locking/unlocking of vflip and hflip controls from > imx219_set_stream() to the imx219_start_streaming()/ > imx219_stop_streaming() respectively. > > This fixes an issue in resume callback error path where streaming is > stopped and the controls are left in locked state. > > Fixes: 1283b3b8f82b9 ("media: i2c: Add driver for Sony IMX219 sensor") > Reported-by: Pavel Machek <pavel@denx.de> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/imx219.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index 6e3382b85a90..82756cbfbaac 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -1056,8 +1056,16 @@ static int imx219_start_streaming(struct imx219 *imx219) > return ret; > > /* set stream on register */ > - return imx219_write_reg(imx219, IMX219_REG_MODE_SELECT, > - IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING); > + ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT, > + IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING); > + if (ret) > + return ret; > + > + /* vflip and hflip cannot change during streaming */ > + __v4l2_ctrl_grab(imx219->vflip, true); > + __v4l2_ctrl_grab(imx219->hflip, true); > + > + return 0; > } > > static void imx219_stop_streaming(struct imx219 *imx219) > @@ -1070,6 +1078,9 @@ static void imx219_stop_streaming(struct imx219 *imx219) > IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY); > if (ret) > dev_err(&client->dev, "%s failed to set stream\n", __func__); > + > + __v4l2_ctrl_grab(imx219->vflip, false); > + __v4l2_ctrl_grab(imx219->hflip, false); > } > > static int imx219_set_stream(struct v4l2_subdev *sd, int enable) > @@ -1105,10 +1116,6 @@ static int imx219_set_stream(struct v4l2_subdev *sd, int enable) > > imx219->streaming = enable; > > - /* vflip and hflip cannot change during streaming */ > - __v4l2_ctrl_grab(imx219->vflip, enable); > - __v4l2_ctrl_grab(imx219->hflip, enable); > - > mutex_unlock(&imx219->mutex); > > return ret; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] media: i2c: imx219: Balance runtime PM use-count 2021-03-11 9:52 [PATCH v2 0/2] media: i2c: imx219: Trivial Fixes Lad Prabhakar 2021-03-11 9:52 ` [PATCH v2 1/2] media: i2c: imx219: Move out locking/unlocking of vflip and hflip controls from imx219_set_stream Lad Prabhakar @ 2021-03-11 9:52 ` Lad Prabhakar 2021-03-15 0:03 ` Laurent Pinchart 1 sibling, 1 reply; 5+ messages in thread From: Lad Prabhakar @ 2021-03-11 9:52 UTC (permalink / raw) To: Dave Stevenson, Mauro Carvalho Chehab, Laurent Pinchart Cc: Pavel Machek, Andrey Konovalov, Sakari Ailus, Jacopo Mondi, Hans Verkuil, linux-media, linux-kernel, linux-renesas-soc, Prabhakar, Lad Prabhakar, Biju Das Move incrementing/decrementing runtime PM count to imx219_start_streaming()/imx219_stop_streaming() functions respectively. This fixes an issue of unbalanced runtime PM count in resume callback error path where streaming is stopped and runtime PM count is left unbalanced. Fixes: 1283b3b8f82b9 ("media: i2c: Add driver for Sony IMX219 sensor") Reported-by: Pavel Machek <pavel@denx.de> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- drivers/media/i2c/imx219.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index 82756cbfbaac..49ba39418360 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -1035,37 +1035,47 @@ static int imx219_start_streaming(struct imx219 *imx219) const struct imx219_reg_list *reg_list; int ret; + ret = pm_runtime_get_sync(&client->dev); + if (ret < 0) { + pm_runtime_put_noidle(&client->dev); + return ret; + } + /* Apply default values of current mode */ reg_list = &imx219->mode->reg_list; ret = imx219_write_regs(imx219, reg_list->regs, reg_list->num_of_regs); if (ret) { dev_err(&client->dev, "%s failed to set mode\n", __func__); - return ret; + goto err_rpm_put; } ret = imx219_set_framefmt(imx219); if (ret) { dev_err(&client->dev, "%s failed to set frame format: %d\n", __func__, ret); - return ret; + goto err_rpm_put; } /* Apply customized values from user */ ret = __v4l2_ctrl_handler_setup(imx219->sd.ctrl_handler); if (ret) - return ret; + goto err_rpm_put; /* set stream on register */ ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT, IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING); if (ret) - return ret; + goto err_rpm_put; /* vflip and hflip cannot change during streaming */ __v4l2_ctrl_grab(imx219->vflip, true); __v4l2_ctrl_grab(imx219->hflip, true); return 0; + +err_rpm_put: + pm_runtime_put(&client->dev); + return ret; } static void imx219_stop_streaming(struct imx219 *imx219) @@ -1081,12 +1091,13 @@ static void imx219_stop_streaming(struct imx219 *imx219) __v4l2_ctrl_grab(imx219->vflip, false); __v4l2_ctrl_grab(imx219->hflip, false); + + pm_runtime_put(&client->dev); } static int imx219_set_stream(struct v4l2_subdev *sd, int enable) { struct imx219 *imx219 = to_imx219(sd); - struct i2c_client *client = v4l2_get_subdevdata(sd); int ret = 0; mutex_lock(&imx219->mutex); @@ -1096,22 +1107,15 @@ static int imx219_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); - goto err_unlock; - } - /* * Apply default & customized values * and then start streaming. */ ret = imx219_start_streaming(imx219); if (ret) - goto err_rpm_put; + goto err_unlock; } else { imx219_stop_streaming(imx219); - pm_runtime_put(&client->dev); } imx219->streaming = enable; @@ -1120,8 +1124,6 @@ static int imx219_set_stream(struct v4l2_subdev *sd, int enable) return ret; -err_rpm_put: - pm_runtime_put(&client->dev); err_unlock: mutex_unlock(&imx219->mutex); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] media: i2c: imx219: Balance runtime PM use-count 2021-03-11 9:52 ` [PATCH v2 2/2] media: i2c: imx219: Balance runtime PM use-count Lad Prabhakar @ 2021-03-15 0:03 ` Laurent Pinchart 0 siblings, 0 replies; 5+ messages in thread From: Laurent Pinchart @ 2021-03-15 0:03 UTC (permalink / raw) To: Lad Prabhakar Cc: Dave Stevenson, Mauro Carvalho Chehab, Pavel Machek, Andrey Konovalov, Sakari Ailus, Jacopo Mondi, Hans Verkuil, linux-media, linux-kernel, linux-renesas-soc, Prabhakar, Biju Das Hi Prabhakar, Thank you for the patch. On Thu, Mar 11, 2021 at 09:52:05AM +0000, Lad Prabhakar wrote: > Move incrementing/decrementing runtime PM count to > imx219_start_streaming()/imx219_stop_streaming() functions respectively. > > This fixes an issue of unbalanced runtime PM count in resume callback > error path where streaming is stopped and runtime PM count is left > unbalanced. > > Fixes: 1283b3b8f82b9 ("media: i2c: Add driver for Sony IMX219 sensor") > Reported-by: Pavel Machek <pavel@denx.de> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/imx219.c | 32 +++++++++++++++++--------------- > 1 file changed, 17 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index 82756cbfbaac..49ba39418360 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -1035,37 +1035,47 @@ static int imx219_start_streaming(struct imx219 *imx219) > const struct imx219_reg_list *reg_list; > int ret; > > + ret = pm_runtime_get_sync(&client->dev); > + if (ret < 0) { > + pm_runtime_put_noidle(&client->dev); > + return ret; > + } > + > /* Apply default values of current mode */ > reg_list = &imx219->mode->reg_list; > ret = imx219_write_regs(imx219, reg_list->regs, reg_list->num_of_regs); > if (ret) { > dev_err(&client->dev, "%s failed to set mode\n", __func__); > - return ret; > + goto err_rpm_put; > } > > ret = imx219_set_framefmt(imx219); > if (ret) { > dev_err(&client->dev, "%s failed to set frame format: %d\n", > __func__, ret); > - return ret; > + goto err_rpm_put; > } > > /* Apply customized values from user */ > ret = __v4l2_ctrl_handler_setup(imx219->sd.ctrl_handler); > if (ret) > - return ret; > + goto err_rpm_put; > > /* set stream on register */ > ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT, > IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING); > if (ret) > - return ret; > + goto err_rpm_put; > > /* vflip and hflip cannot change during streaming */ > __v4l2_ctrl_grab(imx219->vflip, true); > __v4l2_ctrl_grab(imx219->hflip, true); > > return 0; > + > +err_rpm_put: > + pm_runtime_put(&client->dev); > + return ret; > } > > static void imx219_stop_streaming(struct imx219 *imx219) > @@ -1081,12 +1091,13 @@ static void imx219_stop_streaming(struct imx219 *imx219) > > __v4l2_ctrl_grab(imx219->vflip, false); > __v4l2_ctrl_grab(imx219->hflip, false); > + > + pm_runtime_put(&client->dev); > } > > static int imx219_set_stream(struct v4l2_subdev *sd, int enable) > { > struct imx219 *imx219 = to_imx219(sd); > - struct i2c_client *client = v4l2_get_subdevdata(sd); > int ret = 0; > > mutex_lock(&imx219->mutex); > @@ -1096,22 +1107,15 @@ static int imx219_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); > - goto err_unlock; > - } > - > /* > * Apply default & customized values > * and then start streaming. > */ > ret = imx219_start_streaming(imx219); > if (ret) > - goto err_rpm_put; > + goto err_unlock; > } else { > imx219_stop_streaming(imx219); > - pm_runtime_put(&client->dev); > } > > imx219->streaming = enable; > @@ -1120,8 +1124,6 @@ static int imx219_set_stream(struct v4l2_subdev *sd, int enable) > > return ret; > > -err_rpm_put: > - pm_runtime_put(&client->dev); > err_unlock: > mutex_unlock(&imx219->mutex); > -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-15 0:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-11 9:52 [PATCH v2 0/2] media: i2c: imx219: Trivial Fixes Lad Prabhakar 2021-03-11 9:52 ` [PATCH v2 1/2] media: i2c: imx219: Move out locking/unlocking of vflip and hflip controls from imx219_set_stream Lad Prabhakar 2021-03-15 0:02 ` Laurent Pinchart 2021-03-11 9:52 ` [PATCH v2 2/2] media: i2c: imx219: Balance runtime PM use-count Lad Prabhakar 2021-03-15 0:03 ` Laurent Pinchart
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).