diff mbox series

[v2,11/18] media: i2c: imx219: Don't store the current mode in the imx219 structure

Message ID 20230821223001.28480-12-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series [v2,01/18] media: i2c: imx219: Convert to CCI register access helpers | expand

Commit Message

Laurent Pinchart Aug. 21, 2023, 10:29 p.m. UTC
The mode field of the imx219 structure is only used in
imx219_init_controls(), after the probe function sets it to point to the
default mode. Use the default mode directly when initializing controls,
and drop the mode field from the imx219 structure.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx219.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

Comments

Jacopo Mondi Aug. 28, 2023, 1:44 p.m. UTC | #1
Hi Laurent

On Tue, Aug 22, 2023 at 01:29:54AM +0300, Laurent Pinchart wrote:
> The mode field of the imx219 structure is only used in
> imx219_init_controls(), after the probe function sets it to point to the
> default mode. Use the default mode directly when initializing controls,
> and drop the mode field from the imx219 structure.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/i2c/imx219.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 4e9a50117a0a..67a30dc39641 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -363,9 +363,6 @@ struct imx219 {
>  	struct v4l2_ctrl *vblank;
>  	struct v4l2_ctrl *hblank;
>
> -	/* Current mode */
> -	const struct imx219_mode *mode;
> -
>  	/* Streaming on/off */
>  	bool streaming;
>
> @@ -584,7 +581,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
>  	*crop = mode->crop;
>
>  	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> -		imx219->mode = mode;
>  		/* Update limits and set FPS to default */
>  		__v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN,
>  					 IMX219_VTS_MAX - mode->height, 1,
> @@ -967,8 +963,8 @@ static unsigned long imx219_get_pixel_rate(struct imx219 *imx219)
>  static int imx219_init_controls(struct imx219 *imx219)
>  {
>  	struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
> +	const struct imx219_mode *mode = &supported_modes[0];
>  	struct v4l2_ctrl_handler *ctrl_hdlr;
> -	unsigned int height = imx219->mode->height;
>  	struct v4l2_fwnode_device_properties props;
>  	int exposure_max, exposure_def, hblank;
>  	int i, ret;
> @@ -997,15 +993,15 @@ static int imx219_init_controls(struct imx219 *imx219)
>  	/* Initial vblank/hblank/exposure parameters based on current mode */
>  	imx219->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
>  					   V4L2_CID_VBLANK, IMX219_VBLANK_MIN,
> -					   IMX219_VTS_MAX - height, 1,
> -					   imx219->mode->vts_def - height);
> -	hblank = IMX219_PPL_DEFAULT - imx219->mode->width;
> +					   IMX219_VTS_MAX - mode->height, 1,
> +					   mode->vts_def - mode->height);
> +	hblank = IMX219_PPL_DEFAULT - mode->width;
>  	imx219->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
>  					   V4L2_CID_HBLANK, hblank, hblank,
>  					   1, hblank);
>  	if (imx219->hblank)
>  		imx219->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> -	exposure_max = imx219->mode->vts_def - 4;
> +	exposure_max = mode->vts_def - 4;
>  	exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ?
>  		exposure_max : IMX219_EXPOSURE_DEFAULT;
>  	imx219->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
> @@ -1192,10 +1188,8 @@ static int imx219_probe(struct i2c_client *client)
>  	if (ret)
>  		goto error_power_off;
>
> -	/* Set default mode to max resolution */
> -	imx219->mode = &supported_modes[0];
> -
> -	/* sensor doesn't enter LP-11 state upon power up until and unless
> +	/*
> +	 * Sensor doesn't enter LP-11 state upon power up until and unless

Unrelated but doesn't hurt

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>  	 * streaming is started, so upon power up switch the modes to:
>  	 * streaming -> standby
>  	 */
> --
> Regards,
>
> Laurent Pinchart
>
diff mbox series

Patch

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 4e9a50117a0a..67a30dc39641 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -363,9 +363,6 @@  struct imx219 {
 	struct v4l2_ctrl *vblank;
 	struct v4l2_ctrl *hblank;
 
-	/* Current mode */
-	const struct imx219_mode *mode;
-
 	/* Streaming on/off */
 	bool streaming;
 
@@ -584,7 +581,6 @@  static int imx219_set_pad_format(struct v4l2_subdev *sd,
 	*crop = mode->crop;
 
 	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
-		imx219->mode = mode;
 		/* Update limits and set FPS to default */
 		__v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN,
 					 IMX219_VTS_MAX - mode->height, 1,
@@ -967,8 +963,8 @@  static unsigned long imx219_get_pixel_rate(struct imx219 *imx219)
 static int imx219_init_controls(struct imx219 *imx219)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
+	const struct imx219_mode *mode = &supported_modes[0];
 	struct v4l2_ctrl_handler *ctrl_hdlr;
-	unsigned int height = imx219->mode->height;
 	struct v4l2_fwnode_device_properties props;
 	int exposure_max, exposure_def, hblank;
 	int i, ret;
@@ -997,15 +993,15 @@  static int imx219_init_controls(struct imx219 *imx219)
 	/* Initial vblank/hblank/exposure parameters based on current mode */
 	imx219->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
 					   V4L2_CID_VBLANK, IMX219_VBLANK_MIN,
-					   IMX219_VTS_MAX - height, 1,
-					   imx219->mode->vts_def - height);
-	hblank = IMX219_PPL_DEFAULT - imx219->mode->width;
+					   IMX219_VTS_MAX - mode->height, 1,
+					   mode->vts_def - mode->height);
+	hblank = IMX219_PPL_DEFAULT - mode->width;
 	imx219->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
 					   V4L2_CID_HBLANK, hblank, hblank,
 					   1, hblank);
 	if (imx219->hblank)
 		imx219->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
-	exposure_max = imx219->mode->vts_def - 4;
+	exposure_max = mode->vts_def - 4;
 	exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ?
 		exposure_max : IMX219_EXPOSURE_DEFAULT;
 	imx219->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
@@ -1192,10 +1188,8 @@  static int imx219_probe(struct i2c_client *client)
 	if (ret)
 		goto error_power_off;
 
-	/* Set default mode to max resolution */
-	imx219->mode = &supported_modes[0];
-
-	/* sensor doesn't enter LP-11 state upon power up until and unless
+	/*
+	 * Sensor doesn't enter LP-11 state upon power up until and unless
 	 * streaming is started, so upon power up switch the modes to:
 	 * streaming -> standby
 	 */