| Message ID | 20240910170610.226189-4-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: ov5645: Add support for streams | expand |
Hi Prabhakar, Thank you for the patch. On Tue, Sep 10, 2024 at 06:06:02PM +0100, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > To simplify the probe error path, enable runtime PM after the > v4l2_async_register_subdev() call. > > This change ensures that runtime PM is only enabled once the subdevice > registration is successful, avoiding unnecessary cleanup in the error > path. The subdev could start being used as soon as it gets registered, so runtime PM initialization should happen before v4l2_async_register_subdev(). > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > drivers/media/i2c/ov5645.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c > index ab3a419df2df..78b86438c798 100644 > --- a/drivers/media/i2c/ov5645.c > +++ b/drivers/media/i2c/ov5645.c > @@ -1239,18 +1239,17 @@ static int ov5645_probe(struct i2c_client *client) > goto power_down; > } > > - pm_runtime_set_active(dev); > - pm_runtime_get_noresume(dev); > - pm_runtime_enable(dev); > - > ov5645_init_state(&ov5645->sd, NULL); > > ret = v4l2_async_register_subdev(&ov5645->sd); > if (ret < 0) { > dev_err(dev, "could not register v4l2 device\n"); > - goto err_pm_runtime; > + goto power_down; > } > > + pm_runtime_set_active(dev); > + pm_runtime_get_noresume(dev); > + pm_runtime_enable(dev); > pm_runtime_set_autosuspend_delay(dev, 1000); > pm_runtime_use_autosuspend(dev); > pm_runtime_mark_last_busy(dev); > @@ -1258,9 +1257,6 @@ static int ov5645_probe(struct i2c_client *client) > > return 0; > > -err_pm_runtime: > - pm_runtime_disable(dev); > - pm_runtime_put_noidle(dev); > power_down: > ov5645_set_power_off(dev); > free_entity:
Hi Laurent, Thank you for the review. On Tue, Sep 24, 2024 at 11:38 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Prabhakar, > > Thank you for the patch. > > On Tue, Sep 10, 2024 at 06:06:02PM +0100, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > To simplify the probe error path, enable runtime PM after the > > v4l2_async_register_subdev() call. > > > > This change ensures that runtime PM is only enabled once the subdevice > > registration is successful, avoiding unnecessary cleanup in the error > > path. > > The subdev could start being used as soon as it gets registered, so > runtime PM initialization should happen before > v4l2_async_register_subdev(). > Agreed, I will drop this patch. Cheers, Prabhakar
diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c index ab3a419df2df..78b86438c798 100644 --- a/drivers/media/i2c/ov5645.c +++ b/drivers/media/i2c/ov5645.c @@ -1239,18 +1239,17 @@ static int ov5645_probe(struct i2c_client *client) goto power_down; } - pm_runtime_set_active(dev); - pm_runtime_get_noresume(dev); - pm_runtime_enable(dev); - ov5645_init_state(&ov5645->sd, NULL); ret = v4l2_async_register_subdev(&ov5645->sd); if (ret < 0) { dev_err(dev, "could not register v4l2 device\n"); - goto err_pm_runtime; + goto power_down; } + pm_runtime_set_active(dev); + pm_runtime_get_noresume(dev); + pm_runtime_enable(dev); pm_runtime_set_autosuspend_delay(dev, 1000); pm_runtime_use_autosuspend(dev); pm_runtime_mark_last_busy(dev); @@ -1258,9 +1257,6 @@ static int ov5645_probe(struct i2c_client *client) return 0; -err_pm_runtime: - pm_runtime_disable(dev); - pm_runtime_put_noidle(dev); power_down: ov5645_set_power_off(dev); free_entity: