[1/2] media: ov5645: Remove unneeded regulator_set_voltage()

Message ID 20190626235614.26587-1-festevam@gmail.com (mailing list archive)
State Superseded, archived
Delegated to: Sakari Ailus
Headers
Series [1/2] media: ov5645: Remove unneeded regulator_set_voltage() |

Commit Message

Fabio Estevam June 26, 2019, 11:56 p.m. UTC
  There is no need to call regulator_set_voltage() for each regulator
that powers the camera.

The voltage value for each regulator should be retrieved from the
device tree, so remove the unneeded regulator_set_voltage().

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/media/i2c/ov5645.c | 28 ----------------------------
 1 file changed, 28 deletions(-)
  

Comments

Sakari Ailus June 27, 2019, 4:28 p.m. UTC | #1
Hi Fabio,

On Wed, Jun 26, 2019 at 08:56:13PM -0300, Fabio Estevam wrote:
> There is no need to call regulator_set_voltage() for each regulator
> that powers the camera.
> 
> The voltage value for each regulator should be retrieved from the
> device tree, so remove the unneeded regulator_set_voltage().
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Thanks for the patchset.

I wonder if there are chances of this breaking something as the driver did
not depend on the voltage being set correctly in DT. But we don't seem to
have any users in DT source shipped with the kernel. So I guess I'll merge
these at least if no-one complains (see the comment on the 2nd patch, too).

No other sensor (I²C) driver appears to be touching the regulator voltage
either.
  

Patch

diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index 124c8df04633..4e302dc15177 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c
@@ -34,10 +34,6 @@ 
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
-#define OV5645_VOLTAGE_ANALOG               2800000
-#define OV5645_VOLTAGE_DIGITAL_CORE         1500000
-#define OV5645_VOLTAGE_DIGITAL_IO           1800000
-
 #define OV5645_SYSTEM_CTRL0		0x3008
 #define		OV5645_SYSTEM_CTRL0_START	0x02
 #define		OV5645_SYSTEM_CTRL0_STOP	0x42
@@ -1156,42 +1152,18 @@  static int ov5645_probe(struct i2c_client *client,
 		return PTR_ERR(ov5645->io_regulator);
 	}
 
-	ret = regulator_set_voltage(ov5645->io_regulator,
-				    OV5645_VOLTAGE_DIGITAL_IO,
-				    OV5645_VOLTAGE_DIGITAL_IO);
-	if (ret < 0) {
-		dev_err(dev, "cannot set io voltage\n");
-		return ret;
-	}
-
 	ov5645->core_regulator = devm_regulator_get(dev, "vddd");
 	if (IS_ERR(ov5645->core_regulator)) {
 		dev_err(dev, "cannot get core regulator\n");
 		return PTR_ERR(ov5645->core_regulator);
 	}
 
-	ret = regulator_set_voltage(ov5645->core_regulator,
-				    OV5645_VOLTAGE_DIGITAL_CORE,
-				    OV5645_VOLTAGE_DIGITAL_CORE);
-	if (ret < 0) {
-		dev_err(dev, "cannot set core voltage\n");
-		return ret;
-	}
-
 	ov5645->analog_regulator = devm_regulator_get(dev, "vdda");
 	if (IS_ERR(ov5645->analog_regulator)) {
 		dev_err(dev, "cannot get analog regulator\n");
 		return PTR_ERR(ov5645->analog_regulator);
 	}
 
-	ret = regulator_set_voltage(ov5645->analog_regulator,
-				    OV5645_VOLTAGE_ANALOG,
-				    OV5645_VOLTAGE_ANALOG);
-	if (ret < 0) {
-		dev_err(dev, "cannot set analog voltage\n");
-		return ret;
-	}
-
 	ov5645->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
 	if (IS_ERR(ov5645->enable_gpio)) {
 		dev_err(dev, "cannot get enable gpio\n");