[PATCH] media: i2c: Fix regulator disable balance in ov8865

[PATCH] media: i2c: Fix regulator disable balance in ov8865

[Daniel Scally]

ov8865_sensor_power() disables all three of the sensor's regulators
on the error path, however not all of the regulators may have been
enabled at the time of the error, which will result in unbalanced
disable calls.

Fix the issue by adding specific error paths for each regulator.

Fixes: 11c0d8fdccc5 ("media: i2c: Add support for the OV8865 image sensor")

Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
 drivers/media/i2c/ov8865.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
index ebdb20d3fe9d..cb740d7e4f5a 100644
--- a/drivers/media/i2c/ov8865.c
+++ b/drivers/media/i2c/ov8865.c
@@ -2404,30 +2404,28 @@ static int ov8865_sensor_power(struct ov8865_sensor *sensor, bool on)
 		gpiod_set_value_cansleep(sensor->powerdown, 1);
 
 		ret = regulator_enable(sensor->dovdd);
-		if (ret) {
-			dev_err(sensor->dev,
-				"failed to enable DOVDD regulator\n");
-			goto disable;
-		}
+		if (ret)
+			return dev_err_probe(sensor->dev, ret,
+					     "failed to enable DOVDD\n");
 
 		ret = regulator_enable(sensor->avdd);
 		if (ret) {
 			dev_err(sensor->dev,
 				"failed to enable AVDD regulator\n");
-			goto disable;
+			goto err_disable_dovdd;
 		}
 
 		ret = regulator_enable(sensor->dvdd);
 		if (ret) {
 			dev_err(sensor->dev,
 				"failed to enable DVDD regulator\n");
-			goto disable;
+			goto err_disable_avdd;
 		}
 
 		ret = clk_prepare_enable(sensor->extclk);
 		if (ret) {
 			dev_err(sensor->dev, "failed to enable EXTCLK clock\n");
-			goto disable;
+			goto err_disable_dvdd;
 		}
 
 		gpiod_set_value_cansleep(sensor->reset, 0);
@@ -2436,7 +2434,6 @@ static int ov8865_sensor_power(struct ov8865_sensor *sensor, bool on)
 		/* Time to enter streaming mode according to power timings. */
 		usleep_range(10000, 12000);
 	} else {
-disable:
 		gpiod_set_value_cansleep(sensor->powerdown, 1);
 		gpiod_set_value_cansleep(sensor->reset, 1);
 
@@ -2447,6 +2444,15 @@ static int ov8865_sensor_power(struct ov8865_sensor *sensor, bool on)
 		regulator_disable(sensor->dovdd);
 	}
 
+	return ret;
+
+err_disable_dvdd:
+	regulator_disable(sensor->dvdd);
+err_disable_avdd:
+	regulator_disable(sensor->avdd);
+err_disable_dovdd:
+	regulator_disable(sensor->dovdd);
+
 	return ret;
 }
 
-- 
2.25.1

Re: [PATCH] media: i2c: Fix regulator disable balance in ov8865

[Sakari Ailus]

Hi Daniel,

On Sun, Jan 30, 2022 at 09:36:21PM +0000, Daniel Scally wrote:
> ov8865_sensor_power() disables all three of the sensor's regulators
> on the error path, however not all of the regulators may have been
> enabled at the time of the error, which will result in unbalanced
> disable calls.
> 
> Fix the issue by adding specific error paths for each regulator.
> 
> Fixes: 11c0d8fdccc5 ("media: i2c: Add support for the OV8865 image sensor")
> 
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Thanks for the patch.

This has been already fixed by commit
cbe0b3af73bf72fad197756f026084404e2bcdc7 (in media tree).

-- 
Kind regards,

Sakari Ailus

Re: [PATCH] media: i2c: Fix regulator disable balance in ov8865

[Daniel Scally]

Hi Sakari

On 02/02/2022 12:21, Sakari Ailus wrote:
> Hi Daniel,
>
> On Sun, Jan 30, 2022 at 09:36:21PM +0000, Daniel Scally wrote:
>> ov8865_sensor_power() disables all three of the sensor's regulators
>> on the error path, however not all of the regulators may have been
>> enabled at the time of the error, which will result in unbalanced
>> disable calls.
>>
>> Fix the issue by adding specific error paths for each regulator.
>>
>> Fixes: 11c0d8fdccc5 ("media: i2c: Add support for the OV8865 image sensor")
>>
>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> Thanks for the patch.
>
> This has been already fixed by commit
> cbe0b3af73bf72fad197756f026084404e2bcdc7 (in media tree).
>
Ah cool ok - my bad :)