diff mbox series

[v2,5/5] media: i2c: ov5645: Call ov5645_entity_init_cfg() before registering the subdev

Message ID 20221014183459.181567-6-prabhakar.mahadev-lad.rj@bp.renesas.com
State New
Headers
Series media: i2c: ov5645 driver enhancements |

Commit Message

Lad, Prabhakar Oct. 14, 2022, 6:34 p.m. UTC 
  From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Make sure we call ov5645_entity_init_cfg() before registering the subdev
to make sure default formats are set up.

Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2
* New patch
---
 drivers/media/i2c/ov5645.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart Oct. 15, 2022, 6:26 a.m. UTC | #1 
Hi Prabhakar,

Thank you for the patch.

On Fri, Oct 14, 2022 at 07:34:59PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Make sure we call ov5645_entity_init_cfg() before registering the subdev
> to make sure default formats are set up.
> 
> Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

If you have a few spare cycles, it would be even better to convert the
driver to the subdev active state API :-) You could then drop this call
entirely.

> ---
> v1->v2
> * New patch
> ---
>  drivers/media/i2c/ov5645.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> index b3825294aaf1..14bcc6e42dd2 100644
> --- a/drivers/media/i2c/ov5645.c
> +++ b/drivers/media/i2c/ov5645.c
> @@ -1212,6 +1212,8 @@ static int ov5645_probe(struct i2c_client *client)
>  		goto err_pm_runtime;
>  	}
>  
> +	ov5645_entity_init_cfg(&ov5645->sd, NULL);
> +
>  	ret = v4l2_async_register_subdev(&ov5645->sd);
>  	if (ret < 0) {
>  		dev_err(dev, "could not register v4l2 device\n");
> @@ -1224,8 +1226,6 @@ static int ov5645_probe(struct i2c_client *client)
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_put_autosuspend(dev);
>  
> -	ov5645_entity_init_cfg(&ov5645->sd, NULL);
> -
>  	return 0;
>  
>  err_pm_runtime:
Lad, Prabhakar Oct. 26, 2022, 11:59 a.m. UTC | #2 
Hi Laurent,

Thank you for the review.

On Sat, Oct 15, 2022 at 7:26 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Fri, Oct 14, 2022 at 07:34:59PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Make sure we call ov5645_entity_init_cfg() before registering the subdev
> > to make sure default formats are set up.
> >
> > Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> If you have a few spare cycles, it would be even better to convert the
> driver to the subdev active state API :-) You could then drop this call
> entirely.
>
For v3 I did think of it, but it looks like I'll need to spend more
time on the subdev state for this driver (as this driver does cropping
which makes use of TRY/ACTIVE). So for v3 I'll keep this patch as and
will work on the subdev state switch in parallel and post when
complete. (Its just I dont want to miss the v6.2 window for RZ/G2L CRU
driver ;-))

Cheers,
Prabhakar
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index b3825294aaf1..14bcc6e42dd2 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c
@@ -1212,6 +1212,8 @@  static int ov5645_probe(struct i2c_client *client)
 		goto err_pm_runtime;
 	}
 
+	ov5645_entity_init_cfg(&ov5645->sd, NULL);
+
 	ret = v4l2_async_register_subdev(&ov5645->sd);
 	if (ret < 0) {
 		dev_err(dev, "could not register v4l2 device\n");
@@ -1224,8 +1226,6 @@  static int ov5645_probe(struct i2c_client *client)
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_put_autosuspend(dev);
 
-	ov5645_entity_init_cfg(&ov5645->sd, NULL);
-
 	return 0;
 
 err_pm_runtime: