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@kernel.org>
---
 drivers/media/platform/broadcom/bcm2835-unicam.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
index 8d28ba0b59a3..818694f007e2 100644
--- a/drivers/media/platform/broadcom/bcm2835-unicam.c
+++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
@@ -2052,6 +2052,10 @@ 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_in_use(unicam->dev))
+		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),

---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260521-bcmpipm-6c578e73239c

Best regards,
--  
Eugen Hristev <ehristev@kernel.org>

Hi Eugen,

Thanks for the fix, the issue is real, but I think the patch leaks a 
runtime PM reference.

Le 21/05/2026 à 20:09, 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@kernel.org>
> ---
>   drivers/media/platform/broadcom/bcm2835-unicam.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> index 8d28ba0b59a3..818694f007e2 100644
> --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> @@ -2052,6 +2052,10 @@ 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_in_use(unicam->dev))
> +		return 0;
> +

pm_runtime_get_if_in_use() returns 1 and increments the usage counter is 
active and in use.

I think we need to add:
pm_runtime_put(unicam->dev);

Just before the return 0;

BTW, we may miss a dev_info explaining why the live data is skipped when 
the device is suspended ?

Thanks,
JM

>   	reg = unicam_reg_read(unicam, UNICAM_IPIPE);
>   	dev_info(unicam->dev, "Unpacking/packing:   %u / %u\n",
>   		 unicam_get_field(reg, UNICAM_PUM_MASK),
> 
> ---
> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
> change-id: 20260521-bcmpipm-6c578e73239c
> 
> Best regards,
> --
> Eugen Hristev <ehristev@kernel.org>
> 

Hi Eugen, Jean-Michel,

On Fri, May 22, 2026 at 10:31:51AM +0200, Jean-Michel Hautbois wrote:
> Hi Eugen,
> 
> Thanks for the fix, the issue is real, but I think the patch leaks a runtime
> PM reference.
> 
> Le 21/05/2026 à 20:09, 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.

Please rewrap this, most editors can do that automatically.

> > 
> > Fixes: 392cd78d495f ("media: bcm2835-unicam: Add support for CCP2/CSI2 camera interface")
> > Signed-off-by: Eugen Hristev <ehristev@kernel.org>
> > ---
> >   drivers/media/platform/broadcom/bcm2835-unicam.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> > index 8d28ba0b59a3..818694f007e2 100644
> > --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> > +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> > @@ -2052,6 +2052,10 @@ 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_in_use(unicam->dev))
> > +		return 0;
> > +
> 
> pm_runtime_get_if_in_use() returns 1 and increments the usage counter is
> active and in use.

Also, should this be pm_runtime_get_if_active() instead?

> 
> I think we need to add:
> pm_runtime_put(unicam->dev);
> 
> Just before the return 0;
> 
> BTW, we may miss a dev_info explaining why the live data is skipped when the
> device is suspended ?

-- 
Regards,

Sakari Ailus