| Message ID | 20250324130109.375068-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: ov2740: Move pm-runtime cleanup on probe-errors to proper place | expand |
Hans, Thank you for the patch. Reviewed-by: Bingbu Cao <bingbu.cao@intel.com> On 3/24/25 9:01 PM, Hans de Goede wrote: > When v4l2_subdev_init_finalize() fails no changes have been made to > the runtime-pm device state yet, so the probe_error_media_entity_cleanup > rollback path should not touch the runtime-pm device state. > > Instead this should be done from the probe_error_v4l2_subdev_cleanup > rollback path. Note the pm_runtime_xxx() calls are put above > the v4l2_subdev_cleanup() call to have the reverse call order of probe(). > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/media/i2c/ov2740.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c > index 80d151e8ae29..6cf461e3373c 100644 > --- a/drivers/media/i2c/ov2740.c > +++ b/drivers/media/i2c/ov2740.c > @@ -1456,12 +1456,12 @@ static int ov2740_probe(struct i2c_client *client) > return 0; > > probe_error_v4l2_subdev_cleanup: > + pm_runtime_disable(&client->dev); > + pm_runtime_set_suspended(&client->dev); > v4l2_subdev_cleanup(&ov2740->sd); > > probe_error_media_entity_cleanup: > media_entity_cleanup(&ov2740->sd.entity); > - pm_runtime_disable(&client->dev); > - pm_runtime_set_suspended(&client->dev); > > probe_error_v4l2_ctrl_handler_free: > v4l2_ctrl_handler_free(ov2740->sd.ctrl_handler); >
Hi Hans, On Mon, Mar 24, 2025 at 02:01:09PM +0100, Hans de Goede wrote: > When v4l2_subdev_init_finalize() fails no changes have been made to > the runtime-pm device state yet, so the probe_error_media_entity_cleanup > rollback path should not touch the runtime-pm device state. > > Instead this should be done from the probe_error_v4l2_subdev_cleanup > rollback path. Note the pm_runtime_xxx() calls are put above > the v4l2_subdev_cleanup() call to have the reverse call order of probe(). > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Thanks for the fix! This also deserves: Fixes: 289c25923ecd ("media: ov2740: Use sub-device active state") Cc: stable@vger.kernel.org > --- > drivers/media/i2c/ov2740.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c > index 80d151e8ae29..6cf461e3373c 100644 > --- a/drivers/media/i2c/ov2740.c > +++ b/drivers/media/i2c/ov2740.c > @@ -1456,12 +1456,12 @@ static int ov2740_probe(struct i2c_client *client) > return 0; > > probe_error_v4l2_subdev_cleanup: > + pm_runtime_disable(&client->dev); > + pm_runtime_set_suspended(&client->dev); > v4l2_subdev_cleanup(&ov2740->sd); > > probe_error_media_entity_cleanup: > media_entity_cleanup(&ov2740->sd.entity); > - pm_runtime_disable(&client->dev); > - pm_runtime_set_suspended(&client->dev); > > probe_error_v4l2_ctrl_handler_free: > v4l2_ctrl_handler_free(ov2740->sd.ctrl_handler);
diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c index 80d151e8ae29..6cf461e3373c 100644 --- a/drivers/media/i2c/ov2740.c +++ b/drivers/media/i2c/ov2740.c @@ -1456,12 +1456,12 @@ static int ov2740_probe(struct i2c_client *client) return 0; probe_error_v4l2_subdev_cleanup: + pm_runtime_disable(&client->dev); + pm_runtime_set_suspended(&client->dev); v4l2_subdev_cleanup(&ov2740->sd); probe_error_media_entity_cleanup: media_entity_cleanup(&ov2740->sd.entity); - pm_runtime_disable(&client->dev); - pm_runtime_set_suspended(&client->dev); probe_error_v4l2_ctrl_handler_free: v4l2_ctrl_handler_free(ov2740->sd.ctrl_handler);
When v4l2_subdev_init_finalize() fails no changes have been made to the runtime-pm device state yet, so the probe_error_media_entity_cleanup rollback path should not touch the runtime-pm device state. Instead this should be done from the probe_error_v4l2_subdev_cleanup rollback path. Note the pm_runtime_xxx() calls are put above the v4l2_subdev_cleanup() call to have the reverse call order of probe(). Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/i2c/ov2740.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)