Hi Sakari,
thank you for review.
On 8/28/24 11:09, Sakari Ailus wrote:
Hi Vladimir,
Thanks for the update.
On Fri, Aug 23, 2024 at 01:27:31PM +0300, Vladimir Zapolskiy wrote:
Omnivision OG01A1B camera sensor is supplied by three power rails,
if supplies are present as device properties, include them into
the sensor power up sequence.
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@xxxxxxxxxx>
---
drivers/media/i2c/og01a1b.c | 86 ++++++++++++++++++++++++++++++++++++-
1 file changed, 85 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/og01a1b.c b/drivers/media/i2c/og01a1b.c
index 90a68201f43f..0150fdd2f424 100644
--- a/drivers/media/i2c/og01a1b.c
+++ b/drivers/media/i2c/og01a1b.c
@@ -9,6 +9,7 @@
#include <linux/i2c.h>
#include <linux/module.h>
#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-fwnode.h>
@@ -422,6 +423,9 @@ static const struct og01a1b_mode supported_modes[] = {
struct og01a1b {
struct clk *xvclk;
struct gpio_desc *reset_gpio;
+ struct regulator *avdd;
+ struct regulator *dovdd;
+ struct regulator *dvdd;
struct v4l2_subdev sd;
struct media_pad pad;
@@ -982,11 +986,46 @@ static int og01a1b_power_on(struct device *dev)
{
struct v4l2_subdev *sd = dev_get_drvdata(dev);
struct og01a1b *og01a1b = to_og01a1b(sd);
+ int ret;
+
+ if (og01a1b->avdd) {
+ ret = regulator_enable(og01a1b->avdd);
+ if (ret)
+ return ret;
+ }
+
+ if (og01a1b->dovdd) {
+ ret = regulator_enable(og01a1b->dovdd);
+ if (ret)
+ goto avdd_disable;
+ }
+
+ if (og01a1b->dvdd) {
+ ret = regulator_enable(og01a1b->dvdd);
+ if (ret)
+ goto dovdd_disable;
+ }
gpiod_set_value_cansleep(og01a1b->reset_gpio, 0);
usleep_range(USEC_PER_MSEC, 2 * USEC_PER_MSEC);
- return clk_prepare_enable(og01a1b->xvclk);
+ ret = clk_prepare_enable(og01a1b->xvclk);
+ if (ret)
+ goto dvdd_disable;
Virtually all sensors require some delay between lifting the reset (which
typically comes after enabling the regulators and the clock) and the first
I²C access. This one appears to require 8192 XCLK cycles.
According to the spec in case of a non-free running clock the delay mentioned
by you is needed after enabling the clock, and in turn the clock is enabled
after a release on a reset/shutdown GPIO line.
The only supported by the driver clock frequency OG01A1B_MCLK is 19.2 MHz,
so the expected delay after start of the clock is less than 1/10 of a delay
after releasing the shutdown line and before entering the software standby
state.
I believe it would make sense to rearrange the power up sequence in this way:
- enable clock,
- if there is a reset GPIO, then release it and wait for 5ms (1ms is too short),
- otherwise if there is no reset GPIO, then wait for 8192 XCLK cycles (0.5ms).
--
Best wishes,
Vladimir
