Re: [PATCH v3 6/9] media: i2c: ov5645: Use runtime PM

[Lad, Prabhakar]

Hi Sakari,

On Thu, Oct 27, 2022 at 10:22 AM Sakari Ailus
<sakari.ailus@xxxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> Thanks for the update.
>
> On Wed, Oct 26, 2022 at 02:06:55PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Switch to using runtime PM for power management.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > v2->v3
> > * Jumped to err_pm_runtime label in case of sd register failure
> > * Now calling pm_runtime_mark_last_busy() before pm_runtime_put_autosuspend()
> >   call
> > * Now calling pm_runtime_put_sync() in case s_stream(1) fails
> > * In s_stream(0) no calling pm_runtime_mark_last_busy() and
> >   pm_runtime_put_autosuspend()
> > * Included RB tag from Laurent.
> >
> > v1->v2
> > * Moved pm_runtime_*_autosuspend() calls after registering the subdev.
> > ---
> >  drivers/media/i2c/Kconfig  |   2 +-
> >  drivers/media/i2c/ov5645.c | 141 +++++++++++++++++++------------------
> >  2 files changed, 73 insertions(+), 70 deletions(-)
> >
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 7806d4b81716..c0edd1017fe8 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -459,7 +459,7 @@ config VIDEO_OV5640
> >  config VIDEO_OV5645
> >       tristate "OmniVision OV5645 sensor support"
> >       depends on OF
> > -     depends on I2C && VIDEO_DEV
> > +     depends on I2C && PM && VIDEO_DEV
>
> I think you can drop the PM dependency --- the driver will work fine
> without CONFIG_PM.
>
Agreed, I'll send a new version dropping this and fixing the comments
on patch #5 and patch #8.

> Although one could question why do we have CONFIG_PM. Some systems won't
> boot without it and who would want to consume more power than necessary?
>
> Could this be removed altogether? Or perhaps we could add CONFIG_PM
> dependency to V4L2 and DVB? :-)
>
Or rather this option should be selected by the platform itself rather
than subsystem?

Cheers,
Prabhakar