[PATCH v2] media: bcm2835-unicam: Fix log status runtime access

[Jean-Michel Hautbois]

Hi Eugen,

Le 22/05/2026 à 17:28, Eugen Hristev a écrit :
> When requesting log status, the block might be powered off, but registers
> are being read.
> Avoid reading the registers if the device is not resumed, thus also avoid
> powering up the device just for log status.
> 
> Fixes: 392cd78d495f ("media: bcm2835-unicam: Add support for CCP2/CSI2 camera interface")
> Signed-off-by: Eugen Hristev <ehristev at kernel.org>
> ---
> Changes in v2:
> - changed to use pm_runtime_get_if_active()
> - add corresponding put()
> - Link to v1: https://patch.msgid.link/20260521-bcmpipm-v1-1-3eba88d88045@kernel.org
> 
> To: Raspberry Pi Kernel Maintenance <kernel-list at raspberrypi.com>
> To: Mauro Carvalho Chehab <mchehab at kernel.org>
> To: Florian Fainelli <florian.fainelli at broadcom.com>
> To: Ray Jui <rjui at broadcom.com>
> To: Scott Branden <sbranden at broadcom.com>
> To: Broadcom internal kernel review list <bcm-kernel-feedback-list at broadcom.com>
> To: Sakari Ailus <sakari.ailus at linux.intel.com>
> To: Jean-Michel Hautbois <jeanmichel.hautbois at yoseli.org>
> To: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> To: Hans Verkuil <hverkuil at kernel.org>
> To: Naushir Patuck <naush at raspberrypi.com>
> Cc: Dave Stevenson <dave.stevenson at raspberrypi.com>
> Cc: linux-media at vger.kernel.org
> Cc: linux-rpi-kernel at lists.infradead.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> ---
>   drivers/media/platform/broadcom/bcm2835-unicam.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> index 8d28ba0b59a3..93815b8ab930 100644
> --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> @@ -2052,6 +2052,13 @@ static int unicam_log_status(struct file *file, void *fh)
>   		 node->fmt.fmt.pix.width, node->fmt.fmt.pix.height);
>   	dev_info(unicam->dev, "V4L2 format:         %08x\n",
>   		 node->fmt.fmt.pix.pixelformat);
> +
> +	if (!pm_runtime_get_if_active(unicam->dev)) {

Well, if I am picky I would say that pm_runtime_get_if_active() can 
return -EINVAL if runtime PM is disabled for the device. It should then 
be tested against '<= 0' ?

I suppose this should not happen really often, as very few drivers 
actually test this case...

With or without this small change:
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at yoseli.org>

Thanks,
JM

> +		dev_info(unicam->dev,
> +			 "Live data N/A due to device inactive\n");
> +		return 0;
> +	}
> +
>   	reg = unicam_reg_read(unicam, UNICAM_IPIPE);
>   	dev_info(unicam->dev, "Unpacking/packing:   %u / %u\n",
>   		 unicam_get_field(reg, UNICAM_PUM_MASK),
> @@ -2065,6 +2072,8 @@ static int unicam_log_status(struct file *file, void *fh)
>   	dev_info(unicam->dev, "Write pointer:       %08x\n",
>   		 unicam_reg_read(unicam, UNICAM_IBWP));
>   
> +	pm_runtime_put(unicam->dev);
> +
>   	return 0;
>   }
>   
> 
> ---
> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
> change-id: 20260521-bcmpipm-6c578e73239c
> 
> Best regards,
> --
> Eugen Hristev <ehristev at kernel.org>
>