Re: [PATCH v4 2/2] media: i2c: ov02c10: Correct power-on sequence and timing

[Bryan O'Donoghue <bod@kernel.org>]

On 27/01/2026 17:11, Saikiran B wrote:
> Hi Sakari,
> 
> Thanks for the review.
> 
> On Tue, 27 Jan 2026 at 19:07, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>>> +     /* Assert reset for 5ms to ensure sensor is in reset state */
>>> +     if (ov02c10->reset) {
>>> +             gpiod_set_value_cansleep(ov02c10->reset, 1);
>> Is this needed? Isn't XSHUTDOWN already asserted here?
> 
> You are correct that "power_off()" asserts the reset line. However,
> Hans de Goede (Cc'd) suggested explicitly asserting it here to strictly
> enforce the datasheet's T1 timing requirement (Reset low > 5ms) during
> the power-on sequence. This ensures the sensor is in a known clean state
> before power rails are enabled, even if the prior state was inconsistent.

The data-sheet doesn't specify 5 milliseconds - it specifies T3 as 
infinite that is the period between XSHUTDOWN assert and VDD off is 
called "hardware standby"

Does this reset fix the problem for you though ?

We might try this to stop the reset pin floating

reset-n-pins {
     pins = "gpio237";
     function = "gpio";
     drive-strength = <2>;
     bias-pull-down;
};

> 
>>> +             usleep_range(5000, 6000);
>>> +     }
> 
>>> -             usleep_range(5000, 5100);
>>> +             usleep_range(5000, 5500);
>> According to the datasheet you seem to need 8192 XVCLK cycles after
>> deasserting XSHUTDOWN before proceeding with I2C access.
> 
> The 5ms delay covers this requirement with a safe margin.
> With a standard XVCLK of 19.2 MHz (or even 9.6 MHz), 8192 cycles
> takes approximately 0.4ms to 0.8ms.
> 
> The 5ms delay (usleep_range 5000-5500) ensures we are well beyond the
> 8192 cycle requirement for any supported clock frequency.
> 
> Thanks & Regards,
> Saikiran

Adding reset to power-on @ 5 milliseconds if it actually fixes the 
problem is defensible but, be careful about claiming it is being done 
because of hardware requirements, since the data-sheet doesn't mention that.

It sounds to me as if the reset added here isn't ? actually fixing the 
problem for you ?

If so we might try

- Setting bias-pull-down on the reset line

- Making sure the CCI lines aren't supplying voltage
   I may have missed but, did you give that a try

- And again interrogating the PMIC LDO register states
   to show if the LDO is on/off when we think
   Since the RPMh is a firmware which takes cast votes
   if we have messed up sharing say VDD somehow, it is
   possible power is on when we think it is not.

- It is also possible active-discharge is not set on the LDOs

I guess one way to know for sure if XSHUTDOWN or regulators is our 
problem is to never turn the regulators off.

bool enabled = false;

power_on() {
	if (!enabled) {
		regulator_bulk_set();
		enabled = true;
	}
}

power_off() {

}

If we leave all other delays out - nothing in DT, no changes in ov02c10 
and simply never switch the regulator off, once it has been switched on, 
we'd have some pretty conclusive evidence if brown-out was a thing.

How about it ?

---
bod