diff mbox series

[5/5] media: imx: imx-mipi-csis: Protect mipi_csis_dump_regs()

Message ID 20220314103941.46021-6-jacopo@jmondi.org (mailing list archive)
State New, archived
Headers show
Series media: imx: imx-mipi-csis: Additional cleanups | expand

Commit Message

Jacopo Mondi March 14, 2022, 10:39 a.m. UTC
The mipi_csis_dump_regs() function accesses the interface registers
in order to printout their values for debug purposes.

As the function access the registers, it requires the interface to be
powered up. Currently this is only enforced in one of the function's
callers (mipi_csis_log_status)() but not when the function is called by
the debugfs attribute handler.

Make sure to access registers only if the interface is powered up and
remove the same check from the caller.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/platform/imx/imx-mipi-csis.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart March 15, 2022, 11:34 a.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Mon, Mar 14, 2022 at 11:39:41AM +0100, Jacopo Mondi wrote:
> The mipi_csis_dump_regs() function accesses the interface registers
> in order to printout their values for debug purposes.
> 
> As the function access the registers, it requires the interface to be
> powered up. Currently this is only enforced in one of the function's
> callers (mipi_csis_log_status)() but not when the function is called by

s/)()/())/

> the debugfs attribute handler.
> 
> Make sure to access registers only if the interface is powered up and
> remove the same check from the caller.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

But I think we can squash this with 2/5.

> ---
>  drivers/media/platform/imx/imx-mipi-csis.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
> index 4bb469fcb6b3..3437455b9c82 100644
> --- a/drivers/media/platform/imx/imx-mipi-csis.c
> +++ b/drivers/media/platform/imx/imx-mipi-csis.c
> @@ -857,6 +857,9 @@ static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
>  	unsigned int i;
>  	u32 cfg;
>  
> +	if (!pm_runtime_get_if_in_use(csis->dev))
> +		return 0;
> +
>  	dev_info(csis->dev, "--- REGISTERS ---\n");
>  
>  	for (i = 0; i < ARRAY_SIZE(registers); i++) {
> @@ -864,6 +867,8 @@ static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
>  		dev_info(csis->dev, "%14s: 0x%08x\n", registers[i].name, cfg);
>  	}
>  
> +	pm_runtime_put(csis->dev);
> +
>  	return 0;
>  }
>  
> @@ -1160,10 +1165,8 @@ static int mipi_csis_log_status(struct v4l2_subdev *sd)
>  	struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
>  
>  	mipi_csis_log_counters(csis, true);
> -	if (csis->debug.enable && pm_runtime_get_if_in_use(csis->dev)) {
> +	if (csis->debug.enable)
>  		mipi_csis_dump_regs(csis);
> -		pm_runtime_put(csis->dev);
> -	}
>  
>  	return 0;
>  }
diff mbox series

Patch

diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
index 4bb469fcb6b3..3437455b9c82 100644
--- a/drivers/media/platform/imx/imx-mipi-csis.c
+++ b/drivers/media/platform/imx/imx-mipi-csis.c
@@ -857,6 +857,9 @@  static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
 	unsigned int i;
 	u32 cfg;
 
+	if (!pm_runtime_get_if_in_use(csis->dev))
+		return 0;
+
 	dev_info(csis->dev, "--- REGISTERS ---\n");
 
 	for (i = 0; i < ARRAY_SIZE(registers); i++) {
@@ -864,6 +867,8 @@  static int mipi_csis_dump_regs(struct mipi_csis_device *csis)
 		dev_info(csis->dev, "%14s: 0x%08x\n", registers[i].name, cfg);
 	}
 
+	pm_runtime_put(csis->dev);
+
 	return 0;
 }
 
@@ -1160,10 +1165,8 @@  static int mipi_csis_log_status(struct v4l2_subdev *sd)
 	struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
 
 	mipi_csis_log_counters(csis, true);
-	if (csis->debug.enable && pm_runtime_get_if_in_use(csis->dev)) {
+	if (csis->debug.enable)
 		mipi_csis_dump_regs(csis);
-		pm_runtime_put(csis->dev);
-	}
 
 	return 0;
 }