Hi Laurent,
On Fri, Mar 11, 2022 at 03:55:35PM +0200, Laurent Pinchart wrote:
> The runtime PM resume handler is guaranteed to be called on a suspended
> device, and the suspend handler on a resumed device. The implementation
> can thus be simplified.
>
> While at it, rename the mipi_csis_device state field to powered, as the
> now state contains a single flag only.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/platform/imx/imx-mipi-csis.c | 38 ++++++++++------------
> 1 file changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
> index d656b8bfcc33..f6ff8d50843c 100644
> --- a/drivers/media/platform/imx/imx-mipi-csis.c
> +++ b/drivers/media/platform/imx/imx-mipi-csis.c
> @@ -248,10 +248,6 @@
> #define MIPI_CSI2_DATA_TYPE_RAW14 0x2d
> #define MIPI_CSI2_DATA_TYPE_USER(x) (0x30 + (x))
>
> -enum {
> - ST_POWERED = 1,
> -};
> -
> struct mipi_csis_event {
> bool debug;
> u32 mask;
> @@ -331,10 +327,10 @@ struct mipi_csis_device {
> u32 hs_settle;
> u32 clk_settle;
>
> - struct mutex lock; /* Protect csis_fmt, format_mbus and state */
> + struct mutex lock; /* Protect csis_fmt, format_mbus and powered */
> const struct csis_pix_format *csis_fmt;
> struct v4l2_mbus_framefmt format_mbus[CSIS_PADS_NUM];
> - u32 state;
> + bool powered;
>
> spinlock_t slock; /* Protect events */
> struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
> @@ -1193,7 +1189,7 @@ static int mipi_csis_log_status(struct v4l2_subdev *sd)
>
> mutex_lock(&csis->lock);
> mipi_csis_log_counters(csis, true);
> - if (csis->debug.enable && (csis->state & ST_POWERED))
> + if (csis->debug.enable && csis->powered)
Could you use instead pm_runtime_get_if_active() instead? You could then
drop the field.
> mipi_csis_dump_regs(csis);
> mutex_unlock(&csis->lock);
>
> @@ -1354,13 +1350,14 @@ static int __maybe_unused mipi_csis_runtime_suspend(struct device *dev)
> int ret = 0;
>
> mutex_lock(&csis->lock);
> - if (csis->state & ST_POWERED) {
> - ret = mipi_csis_phy_disable(csis);
> - if (ret)
> - goto unlock;
> - mipi_csis_clk_disable(csis);
> - csis->state &= ~ST_POWERED;
> - }
> +
> + ret = mipi_csis_phy_disable(csis);
> + if (ret)
> + goto unlock;
> +
> + mipi_csis_clk_disable(csis);
> +
> + csis->powered = false;
>
> unlock:
> mutex_unlock(&csis->lock);
> @@ -1376,14 +1373,13 @@ static int __maybe_unused mipi_csis_runtime_resume(struct device *dev)
>
> mutex_lock(&csis->lock);
>
> - if (!(csis->state & ST_POWERED)) {
> - ret = mipi_csis_phy_enable(csis);
> - if (ret)
> - goto unlock;
> + ret = mipi_csis_phy_enable(csis);
> + if (ret)
> + goto unlock;
>
> - csis->state |= ST_POWERED;
> - mipi_csis_clk_enable(csis);
> - }
> + mipi_csis_clk_enable(csis);
> +
> + csis->powered = true;
>
> unlock:
> mutex_unlock(&csis->lock);
--
Regards,
Sakari Ailus
