| From 51e6ed60492f730124786cbb3c0f8d88bd638c3c Mon Sep 17 00:00:00 2001 |
| From: Dongchun Zhu <dongchun.zhu@mediatek.com> |
| Date: Wed, 11 Mar 2020 11:47:28 +0100 |
| Subject: [PATCH] media: i2c: ov5695: Fix power on and off sequences |
| |
| commit f1a64f56663e9d03e509439016dcbddd0166b2da upstream. |
| |
| From the measured hardware signal, OV5695 reset pin goes high for a |
| short period of time during boot-up. From the sensor specification, the |
| reset pin is active low and the DT binding defines the pin as active |
| low, which means that the values set by the driver are inverted and thus |
| the value requested in probe ends up high. |
| |
| Fix it by changing probe to request the reset GPIO initialized to high, |
| which makes the initial state of the physical signal low. |
| |
| In addition, DOVDD rising must occur before DVDD rising from spec., but |
| regulator_bulk_enable() API enables all the regulators asynchronously. |
| Use an explicit loops of regulator_enable() instead. |
| |
| For power off sequence, it is required that DVDD falls first. Given the |
| bulk API does not give any guarantee about the order of regulators, |
| change the driver to use regulator_disable() instead. |
| |
| The sensor also requires a delay between reset high and first I2C |
| transaction, which was assumed to be 8192 XVCLK cycles, but 1ms is |
| recommended by the vendor. Fix this as well. |
| |
| Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com> |
| Signed-off-by: Tomasz Figa <tfiga@chromium.org> |
| Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> |
| Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c |
| index 5d107c53364d..be242bb8fefc 100644 |
| --- a/drivers/media/i2c/ov5695.c |
| +++ b/drivers/media/i2c/ov5695.c |
| @@ -974,16 +974,9 @@ static int ov5695_s_stream(struct v4l2_subdev *sd, int on) |
| return ret; |
| } |
| |
| -/* Calculate the delay in us by clock rate and clock cycles */ |
| -static inline u32 ov5695_cal_delay(u32 cycles) |
| -{ |
| - return DIV_ROUND_UP(cycles, OV5695_XVCLK_FREQ / 1000 / 1000); |
| -} |
| - |
| static int __ov5695_power_on(struct ov5695 *ov5695) |
| { |
| - int ret; |
| - u32 delay_us; |
| + int i, ret; |
| struct device *dev = &ov5695->client->dev; |
| |
| ret = clk_prepare_enable(ov5695->xvclk); |
| @@ -994,21 +987,28 @@ static int __ov5695_power_on(struct ov5695 *ov5695) |
| |
| gpiod_set_value_cansleep(ov5695->reset_gpio, 1); |
| |
| - ret = regulator_bulk_enable(OV5695_NUM_SUPPLIES, ov5695->supplies); |
| - if (ret < 0) { |
| - dev_err(dev, "Failed to enable regulators\n"); |
| - goto disable_clk; |
| + /* |
| + * The hardware requires the regulators to be powered on in order, |
| + * so enable them one by one. |
| + */ |
| + for (i = 0; i < OV5695_NUM_SUPPLIES; i++) { |
| + ret = regulator_enable(ov5695->supplies[i].consumer); |
| + if (ret) { |
| + dev_err(dev, "Failed to enable %s: %d\n", |
| + ov5695->supplies[i].supply, ret); |
| + goto disable_reg_clk; |
| + } |
| } |
| |
| gpiod_set_value_cansleep(ov5695->reset_gpio, 0); |
| |
| - /* 8192 cycles prior to first SCCB transaction */ |
| - delay_us = ov5695_cal_delay(8192); |
| - usleep_range(delay_us, delay_us * 2); |
| + usleep_range(1000, 1200); |
| |
| return 0; |
| |
| -disable_clk: |
| +disable_reg_clk: |
| + for (--i; i >= 0; i--) |
| + regulator_disable(ov5695->supplies[i].consumer); |
| clk_disable_unprepare(ov5695->xvclk); |
| |
| return ret; |
| @@ -1016,9 +1016,22 @@ static int __ov5695_power_on(struct ov5695 *ov5695) |
| |
| static void __ov5695_power_off(struct ov5695 *ov5695) |
| { |
| + struct device *dev = &ov5695->client->dev; |
| + int i, ret; |
| + |
| clk_disable_unprepare(ov5695->xvclk); |
| gpiod_set_value_cansleep(ov5695->reset_gpio, 1); |
| - regulator_bulk_disable(OV5695_NUM_SUPPLIES, ov5695->supplies); |
| + |
| + /* |
| + * The hardware requires the regulators to be powered off in order, |
| + * so disable them one by one. |
| + */ |
| + for (i = OV5695_NUM_SUPPLIES - 1; i >= 0; i--) { |
| + ret = regulator_disable(ov5695->supplies[i].consumer); |
| + if (ret) |
| + dev_err(dev, "Failed to disable %s: %d\n", |
| + ov5695->supplies[i].supply, ret); |
| + } |
| } |
| |
| static int __maybe_unused ov5695_runtime_resume(struct device *dev) |
| @@ -1288,7 +1301,7 @@ static int ov5695_probe(struct i2c_client *client, |
| if (clk_get_rate(ov5695->xvclk) != OV5695_XVCLK_FREQ) |
| dev_warn(dev, "xvclk mismatched, modes are based on 24MHz\n"); |
| |
| - ov5695->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); |
| + ov5695->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); |
| if (IS_ERR(ov5695->reset_gpio)) { |
| dev_err(dev, "Failed to get reset-gpios\n"); |
| return -EINVAL; |
| -- |
| 2.7.4 |
| |