* [PATCH] media: bcm2835-unicam: Fix log status runtime access
@ 2026-05-21 18:09 Eugen Hristev
2026-05-22 8:31 ` Jean-Michel Hautbois
0 siblings, 1 reply; 3+ messages in thread
From: Eugen Hristev @ 2026-05-21 18:09 UTC (permalink / raw)
To: Raspberry Pi Kernel Maintenance, Mauro Carvalho Chehab,
Florian Fainelli, Broadcom internal kernel review list, Ray Jui,
Scott Branden, Dave Stevenson, Hans Verkuil, Laurent Pinchart,
Sakari Ailus, Jean-Michel Hautbois
Cc: Naushir Patuck, linux-media, linux-rpi-kernel, linux-arm-kernel,
linux-kernel, Eugen Hristev
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>
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] media: bcm2835-unicam: Fix log status runtime access
2026-05-21 18:09 [PATCH] media: bcm2835-unicam: Fix log status runtime access Eugen Hristev
@ 2026-05-22 8:31 ` Jean-Michel Hautbois
2026-05-22 10:28 ` Sakari Ailus
0 siblings, 1 reply; 3+ messages in thread
From: Jean-Michel Hautbois @ 2026-05-22 8:31 UTC (permalink / raw)
To: Eugen Hristev, Raspberry Pi Kernel Maintenance,
Mauro Carvalho Chehab, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Dave Stevenson, Hans Verkuil, Laurent Pinchart, Sakari Ailus
Cc: Naushir Patuck, linux-media, linux-rpi-kernel, linux-arm-kernel,
linux-kernel
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>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] media: bcm2835-unicam: Fix log status runtime access
2026-05-22 8:31 ` Jean-Michel Hautbois
@ 2026-05-22 10:28 ` Sakari Ailus
0 siblings, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2026-05-22 10:28 UTC (permalink / raw)
To: Jean-Michel Hautbois
Cc: Eugen Hristev, Raspberry Pi Kernel Maintenance,
Mauro Carvalho Chehab, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Dave Stevenson, Hans Verkuil, Laurent Pinchart, Naushir Patuck,
linux-media, linux-rpi-kernel, linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-22 10:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21 18:09 [PATCH] media: bcm2835-unicam: Fix log status runtime access Eugen Hristev
2026-05-22 8:31 ` Jean-Michel Hautbois
2026-05-22 10:28 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox