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

From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Prabhakar <prabhakar.csengg@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Hans Verkuil <hverkuil@xs4all.nl>, Shawn Tu <shawnx.tu@intel.com>,
	Jacopo Mondi <jacopo@jmondi.org>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org,
	Biju Das <biju.das.jz@bp.renesas.com>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: Re: [PATCH v3 6/9] media: i2c: ov5645: Use runtime PM
Date: Thu, 27 Oct 2022 09:22:26 +0000	[thread overview]
Message-ID: <Y1pN0oksKeauTpBA@paasikivi.fi.intel.com> (raw)
In-Reply-To: <20221026130658.45601-7-prabhakar.mahadev-lad.rj@bp.renesas.com>

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@bp.renesas.com>
> 
> Switch to using runtime PM for power management.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> 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.

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? :-)

Certainly out of scope of this set though.

-- 
Kind regards,

Sakari Ailus