[PATCH] media: i2c: ov2740: fix power cleanup on remove

Guangshuo Li posted 1 patch 1 week, 2 days ago
drivers/media/i2c/ov2740.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH] media: i2c: ov2740: fix power cleanup on remove
Posted by Guangshuo Li 1 week, 2 days ago
ov2740_remove() disables runtime PM without powering off the sensor if
it is still runtime active or updating the runtime PM state to
suspended.

pm_runtime_disable() prevents further runtime PM callbacks and waits for
pending operations, but it does not force the runtime suspend callback
to run. If the sensor is active when the driver is removed, the reset
and powerdown GPIOs can remain deasserted and the clock and regulators
can remain enabled.

After disabling runtime PM, call ov2740_suspend() if the device is not
already runtime suspended, and then mark the runtime PM state as
suspended. Checking the runtime status avoids disabling the hardware
resources a second time when runtime PM has already suspended the
sensor.

This issue was found by manual code inspection.

Fixes: 1a140854bc8c ("media: ov2740: Add support for reset GPIO")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/media/i2c/ov2740.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 39003c1632ad..a5346adf2281 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -1198,6 +1198,8 @@ static int ov2740_check_hwcfg(struct ov2740 *ov2740)
 	return ret;
 }
 
+static int ov2740_suspend(struct device *dev);
+
 static void ov2740_remove(struct i2c_client *client)
 {
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -1207,6 +1209,9 @@ static void ov2740_remove(struct i2c_client *client)
 	v4l2_subdev_cleanup(sd);
 	v4l2_ctrl_handler_free(sd->ctrl_handler);
 	pm_runtime_disable(&client->dev);
+	if (!pm_runtime_status_suspended(&client->dev))
+		ov2740_suspend(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
 }
 
 static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
-- 
2.43.0
Re: [PATCH] media: i2c: ov2740: fix power cleanup on remove
Posted by Sakari Ailus 1 week, 2 days ago
On Tue, Sep 15, 2026 at 04:59:02PM +0800, Guangshuo Li wrote:
> ov2740_remove() disables runtime PM without powering off the sensor if
> it is still runtime active or updating the runtime PM state to
> suspended.
> 
> pm_runtime_disable() prevents further runtime PM callbacks and waits for
> pending operations, but it does not force the runtime suspend callback
> to run. If the sensor is active when the driver is removed, the reset
> and powerdown GPIOs can remain deasserted and the clock and regulators
> can remain enabled.
> 
> After disabling runtime PM, call ov2740_suspend() if the device is not
> already runtime suspended, and then mark the runtime PM state as
> suspended. Checking the runtime status avoids disabling the hardware
> resources a second time when runtime PM has already suspended the
> sensor.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 1a140854bc8c ("media: ov2740: Add support for reset GPIO")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/media/i2c/ov2740.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 39003c1632ad..a5346adf2281 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -1198,6 +1198,8 @@ static int ov2740_check_hwcfg(struct ov2740 *ov2740)
>  	return ret;
>  }
>  
> +static int ov2740_suspend(struct device *dev);
> +
>  static void ov2740_remove(struct i2c_client *client)
>  {
>  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> @@ -1207,6 +1209,9 @@ static void ov2740_remove(struct i2c_client *client)
>  	v4l2_subdev_cleanup(sd);
>  	v4l2_ctrl_handler_free(sd->ctrl_handler);
>  	pm_runtime_disable(&client->dev);
> +	if (!pm_runtime_status_suspended(&client->dev))
> +		ov2740_suspend(&client->dev);
> +	pm_runtime_set_suspended(&client->dev);

This, too, need to be called conditionally.

>  }
>  
>  static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,

-- 
Sakari Ailus
Re: [PATCH] media: i2c: ov2740: fix power cleanup on remove
Posted by Sakari Ailus 1 week, 2 days ago
Hi Guangshuo,

On Tue, Sep 15, 2026 at 04:59:02PM +0800, Guangshuo Li wrote:
> ov2740_remove() disables runtime PM without powering off the sensor if
> it is still runtime active or updating the runtime PM state to
> suspended.
> 
> pm_runtime_disable() prevents further runtime PM callbacks and waits for
> pending operations, but it does not force the runtime suspend callback
> to run. If the sensor is active when the driver is removed, the reset
> and powerdown GPIOs can remain deasserted and the clock and regulators
> can remain enabled.
> 
> After disabling runtime PM, call ov2740_suspend() if the device is not
> already runtime suspended, and then mark the runtime PM state as
> suspended. Checking the runtime status avoids disabling the hardware
> resources a second time when runtime PM has already suspended the
> sensor.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 1a140854bc8c ("media: ov2740: Add support for reset GPIO")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/media/i2c/ov2740.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 39003c1632ad..a5346adf2281 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -1198,6 +1198,8 @@ static int ov2740_check_hwcfg(struct ov2740 *ov2740)
>  	return ret;
>  }
>  
> +static int ov2740_suspend(struct device *dev);
> +
>  static void ov2740_remove(struct i2c_client *client)

Could you move ov2740_remove() just after probe instead of adding a
prototype?

>  {
>  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> @@ -1207,6 +1209,9 @@ static void ov2740_remove(struct i2c_client *client)
>  	v4l2_subdev_cleanup(sd);
>  	v4l2_ctrl_handler_free(sd->ctrl_handler);
>  	pm_runtime_disable(&client->dev);
> +	if (!pm_runtime_status_suspended(&client->dev))
> +		ov2740_suspend(&client->dev);
> +	pm_runtime_set_suspended(&client->dev);
>  }
>  
>  static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,

-- 
Regards,

Sakari Ailus