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

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

[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>
---
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@raspberrypi.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Florian Fainelli <florian.fainelli@broadcom.com>
To: Ray Jui <rjui@broadcom.com>
To: Scott Branden <sbranden@broadcom.com>
To: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Hans Verkuil <hverkuil@kernel.org>
To: Naushir Patuck <naush@raspberrypi.com>
Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: linux-media@vger.kernel.org
Cc: linux-rpi-kernel@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@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)) {
+		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@kernel.org>

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

[Dave Stevenson]

Hi Eugen

On Fri, 22 May 2026 at 16:29, Eugen Hristev <ehristev@kernel.org> wrote:
>
> 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>

Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

Thanks.

> ---
> 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@raspberrypi.com>
> To: Mauro Carvalho Chehab <mchehab@kernel.org>
> To: Florian Fainelli <florian.fainelli@broadcom.com>
> To: Ray Jui <rjui@broadcom.com>
> To: Scott Branden <sbranden@broadcom.com>
> To: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> To: Sakari Ailus <sakari.ailus@linux.intel.com>
> To: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> To: Hans Verkuil <hverkuil@kernel.org>
> To: Naushir Patuck <naush@raspberrypi.com>
> Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Cc: linux-media@vger.kernel.org
> Cc: linux-rpi-kernel@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@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)) {
> +               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@kernel.org>
>

Re: [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@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@raspberrypi.com>
> To: Mauro Carvalho Chehab <mchehab@kernel.org>
> To: Florian Fainelli <florian.fainelli@broadcom.com>
> To: Ray Jui <rjui@broadcom.com>
> To: Scott Branden <sbranden@broadcom.com>
> To: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> To: Sakari Ailus <sakari.ailus@linux.intel.com>
> To: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> To: Hans Verkuil <hverkuil@kernel.org>
> To: Naushir Patuck <naush@raspberrypi.com>
> Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Cc: linux-media@vger.kernel.org
> Cc: linux-rpi-kernel@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@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@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@kernel.org>
> 

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

[Eugen Hristev]

On 5/29/26 08:12, Jean-Michel Hautbois wrote:
> 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@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@raspberrypi.com>
>> To: Mauro Carvalho Chehab <mchehab@kernel.org>
>> To: Florian Fainelli <florian.fainelli@broadcom.com>
>> To: Ray Jui <rjui@broadcom.com>
>> To: Scott Branden <sbranden@broadcom.com>
>> To: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
>> To: Sakari Ailus <sakari.ailus@linux.intel.com>
>> To: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
>> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> To: Hans Verkuil <hverkuil@kernel.org>
>> To: Naushir Patuck <naush@raspberrypi.com>
>> Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
>> Cc: linux-media@vger.kernel.org
>> Cc: linux-rpi-kernel@lists.infradead.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@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...

I saw that. Some do. This driver enables runtime pm in probe though. So
I guess it cannot happen, unless runtime pm would not selected in kernel
config, but the driver depends on PM.

Ultimately I guess it's up to Sakari or Hans to decide whether it's
worth checking for error code, but I picked the simpler path (and
considering <depends on PM> in Kconfig) .
> 
> With or without this small change:
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@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@kernel.org>
>>
> 

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

[Sakari Ailus]

Hi Eugen, others,

On Fri, May 29, 2026 at 06:06:42PM +0300, Eugen Hristev wrote:
> On 5/29/26 08:12, Jean-Michel Hautbois wrote:
> > 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@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@raspberrypi.com>
> >> To: Mauro Carvalho Chehab <mchehab@kernel.org>
> >> To: Florian Fainelli <florian.fainelli@broadcom.com>
> >> To: Ray Jui <rjui@broadcom.com>
> >> To: Scott Branden <sbranden@broadcom.com>
> >> To: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> >> To: Sakari Ailus <sakari.ailus@linux.intel.com>
> >> To: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> >> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> To: Hans Verkuil <hverkuil@kernel.org>
> >> To: Naushir Patuck <naush@raspberrypi.com>
> >> Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
> >> Cc: linux-media@vger.kernel.org
> >> Cc: linux-rpi-kernel@lists.infradead.org
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-kernel@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...
> 
> I saw that. Some do. This driver enables runtime pm in probe though. So
> I guess it cannot happen, unless runtime pm would not selected in kernel
> config, but the driver depends on PM.

Runtime PM can be disabled for a device via sysfs.

> 
> Ultimately I guess it's up to Sakari or Hans to decide whether it's
> worth checking for error code, but I picked the simpler path (and
> considering <depends on PM> in Kconfig) .

pm_runtime_put() musn't be called if there was an error as it decrements
usage_count unconditionally.

The vast majority of sensor drivers test for non-zero only and no-one has
complained. They should be fixed though...

> > 
> > With or without this small change:
> > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@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,

-- 
Regards,

Sakari Ailus

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

[Jai Luthra]

Hi Sakari, Eugen,

Quoting Sakari Ailus (2026-06-01 02:32:09)
> Hi Eugen, others,
> 
> On Fri, May 29, 2026 at 06:06:42PM +0300, Eugen Hristev wrote:
> > On 5/29/26 08:12, Jean-Michel Hautbois wrote:
> > > 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@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@raspberrypi.com>
> > >> To: Mauro Carvalho Chehab <mchehab@kernel.org>
> > >> To: Florian Fainelli <florian.fainelli@broadcom.com>
> > >> To: Ray Jui <rjui@broadcom.com>
> > >> To: Scott Branden <sbranden@broadcom.com>
> > >> To: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> > >> To: Sakari Ailus <sakari.ailus@linux.intel.com>
> > >> To: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > >> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >> To: Hans Verkuil <hverkuil@kernel.org>
> > >> To: Naushir Patuck <naush@raspberrypi.com>
> > >> Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > >> Cc: linux-media@vger.kernel.org
> > >> Cc: linux-rpi-kernel@lists.infradead.org
> > >> Cc: linux-arm-kernel@lists.infradead.org
> > >> Cc: linux-kernel@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...
> > 
> > I saw that. Some do. This driver enables runtime pm in probe though. So
> > I guess it cannot happen, unless runtime pm would not selected in kernel
> > config, but the driver depends on PM.
> 
> Runtime PM can be disabled for a device via sysfs.
> 

I tried this on my board, and you can only do:

echo on > /sys/class/<...>/control

Which leads to RPM count to always stay 1 regardless of get/put.

So if the driver enables RPM and never disables it unless it's removed,
there's no way from userspace to make this function return -EINVAL.

> > 
> > Ultimately I guess it's up to Sakari or Hans to decide whether it's
> > worth checking for error code, but I picked the simpler path (and
> > considering <depends on PM> in Kconfig) .
> 
> pm_runtime_put() musn't be called if there was an error as it decrements
> usage_count unconditionally.
> 
> The vast majority of sensor drivers test for non-zero only and no-one has
> complained. They should be fixed though...
> 

This driver has an explicit "depends on PM", but even for other sensor
drivers, the only case pm_runtime_get_if_active can return -EINVAL is when
CONFIG_PM=n, where pm_runtime_put is also stubbed out to return -ENOSYS..

So there is nothing to fix in sesnor drivers which do:

    if (!pm_runtime_get_if_active(&client->dev))
            return 0;

    /* write registers */

    /* unconditional put */
    pm_runtime_put(&client->dev)

Thanks,
    Jai

> > > 
> > > With or without this small change:
> > > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@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,
> 
> -- 
> Regards,
> 
> Sakari Ailus
>

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

[Jai Luthra]

Quoting Eugen Hristev (2026-05-22 20:58:50)
> 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>

Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>

> ---
> 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@raspberrypi.com>
> To: Mauro Carvalho Chehab <mchehab@kernel.org>
> To: Florian Fainelli <florian.fainelli@broadcom.com>
> To: Ray Jui <rjui@broadcom.com>
> To: Scott Branden <sbranden@broadcom.com>
> To: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> To: Sakari Ailus <sakari.ailus@linux.intel.com>
> To: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> To: Hans Verkuil <hverkuil@kernel.org>
> To: Naushir Patuck <naush@raspberrypi.com>
> Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Cc: linux-media@vger.kernel.org
> Cc: linux-rpi-kernel@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org

[snip]

Thanks,
    Jai