[PATCH v4 0/2] media: i2c: ov02c10: Fix race condition and power sequence

[PATCH v4 0/2] media: i2c: ov02c10: Fix race condition and power sequence

[Saikiran]

This series addresses driver correctness and stability issues in the OV02C10
sensor driver. It fixes a use-after-free race condition during removal and
aligns the power-on sequence with the datasheet requirements.

Note on v3/Brownouts:
The "Autosuspend" workaround proposed in v3 to handle regulator brownouts on
Qualcomm X1E80100 platforms has been dropped from this series.

I am pursuing the root-cause analysis for the regulator discharge delays
separately. The platform-specific constraints (2.3s passive discharge) will
be handled via the regulator subsystem (linux-arm-msm) and device tree,
keeping the media driver clean of platform-specific workarounds. I will
continue investigating the underlying physical discharge characteristics
and PMIC status registers as requested by maintainers.

This v4 series now strictly focuses on generic driver correctness:

Patch 1: Fixes a critical race condition in the remove() function where
resources were freed while the device was potentially still active, leading
to kernel oops.

Patch 2: Corrects the power-on sequence to strictly follow the datasheet
timing (T1) by asserting the reset pin for 5ms before enabling power rails.
This ensures the sensor enters a known clean state during cold boot.

Changes in v4:
- Dropped Patch 3 (Runtime PM Autosuspend) to separate platform-specific
  regulator fixes from generic driver cleanup.
- Modified Patch 2:
  - Reduced reset assertion delay from 10ms to 5ms (usleep_range 5000-6000)
    to match datasheet specs and maintainer feedback.
  - Removed the software reset (0x0103) and extra regulator delays to keep
    the sequence minimal and compliant.
- Patch 1 carried forward with Reviewed-by tag.

Changes in v3:
- Superseded previous "pipeline lock" and "brownout" series.
- Added strict power-on sequencing.
- Added fix for use-after-free in remove().

Saikiran (2):
  media: i2c: ov02c10: Fix use-after-free in remove function
  media: i2c: ov02c10: Correct power-on sequence and timing

 drivers/media/i2c/ov02c10.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

[PATCH v4 1/2] media: i2c: ov02c10: Fix use-after-free in remove function

[Saikiran]

The ov02c10_remove() function has a race condition where v4l2_ctrl_handler
and media_entity resources are freed before the device is powered off.
If userspace (e.g., PipeWire/WirePlumber) accesses the device during
removal, this causes a use-after-free leading to kernel oops with
"Execute from non-executable memory" errors.

The issue occurs because:
1. v4l2_ctrl_handler_free() is called first
2. Userspace may still have the device open
3. Control access triggers use-after-free
4. Device is powered off afterwards (too late)

Fix by reordering cleanup to disable runtime PM and power off the device
BEFORE freeing v4l2_ctrl_handler and media_entity resources. This ensures
the device is in a safe state before any resources are freed.

Call sequence after fix:
1. v4l2_async_unregister_subdev() - unregister from V4L2
2. pm_runtime_disable() - disable runtime PM
3. ov02c10_power_off() - power off device if needed
4. v4l2_subdev_cleanup() - clean up subdev
5. media_entity_cleanup() - clean up media entity
6. v4l2_ctrl_handler_free() - free control handler (safe now)

Tested-on: Lenovo Yoga Slim 7x (Snapdragon X Elite)
Fixes: 44f8901 ("media: i2c: add OmniVision OV02C10 sensor driver")
Cc: stable@vger.kernel.org
Reviewed-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Saikiran <bjsaikiran@gmail.com>
---
 drivers/media/i2c/ov02c10.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index cf93d36032e1..fa7cc48b769a 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -864,14 +864,14 @@ static void ov02c10_remove(struct i2c_client *client)
 	struct ov02c10 *ov02c10 = to_ov02c10(sd);
 
 	v4l2_async_unregister_subdev(sd);
-	v4l2_subdev_cleanup(sd);
-	media_entity_cleanup(&sd->entity);
-	v4l2_ctrl_handler_free(sd->ctrl_handler);
 	pm_runtime_disable(ov02c10->dev);
 	if (!pm_runtime_status_suspended(ov02c10->dev)) {
 		ov02c10_power_off(ov02c10->dev);
 		pm_runtime_set_suspended(ov02c10->dev);
 	}
+	v4l2_subdev_cleanup(sd);
+	media_entity_cleanup(&sd->entity);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
 }
 
 static int ov02c10_probe(struct i2c_client *client)
-- 
2.51.0

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

[Saikiran]

The previous power-on sequence did not strictly follow the hardware timing
requirements (T1), potentially leading to initialization failures on some
platforms.

Update the sequence to match the datasheet and maintainer recommendations:
1. Assert XSHUTDOWN (reset) for 5ms (T1 >= 5ms) before enabling power
   resources.
2. Enable clock and regulators in the standard order.
3. De-assert XSHUTDOWN.
4. Wait 5ms (T2 >= 5ms) for sensor boot before I2C access (using a wider
   range for timer coalescing).

This ensures the sensor enters a clean state during cold boot.

Tested-on: Lenovo Yoga Slim 7x (Snapdragon X Elite)
Fixes: 44f8901 ("media: i2c: add OmniVision OV02C10 sensor driver")
Cc: stable@vger.kernel.org
Signed-off-by: Saikiran <bjsaikiran@gmail.com>
---
 drivers/media/i2c/ov02c10.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index fa7cc48b769a..3bfbd0deb126 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -676,6 +676,12 @@ static int ov02c10_power_on(struct device *dev)
 	struct ov02c10 *ov02c10 = to_ov02c10(sd);
 	int ret;
 
+	/* Assert reset for 5ms to ensure sensor is in reset state */
+	if (ov02c10->reset) {
+		gpiod_set_value_cansleep(ov02c10->reset, 1);
+		usleep_range(5000, 6000);
+	}
+
 	ret = clk_prepare_enable(ov02c10->img_clk);
 	if (ret < 0) {
 		dev_err(dev, "failed to enable imaging clock: %d", ret);
@@ -691,10 +697,8 @@ static int ov02c10_power_on(struct device *dev)
 	}
 
 	if (ov02c10->reset) {
-		/* Assert reset for at least 2ms on back to back off-on */
-		usleep_range(2000, 2200);
 		gpiod_set_value_cansleep(ov02c10->reset, 0);
-		usleep_range(5000, 5100);
+		usleep_range(5000, 5500);
 	}
 
 	return 0;
-- 
2.51.0

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

[Sakari Ailus]

HI Saikiran,

On Tue, Jan 27, 2026 at 10:20:24PM +0530, Saikiran wrote:
> The previous power-on sequence did not strictly follow the hardware timing
> requirements (T1), potentially leading to initialization failures on some
> platforms.
> 
> Update the sequence to match the datasheet and maintainer recommendations:
> 1. Assert XSHUTDOWN (reset) for 5ms (T1 >= 5ms) before enabling power
>    resources.
> 2. Enable clock and regulators in the standard order.
> 3. De-assert XSHUTDOWN.
> 4. Wait 5ms (T2 >= 5ms) for sensor boot before I2C access (using a wider
>    range for timer coalescing).
> 
> This ensures the sensor enters a clean state during cold boot.
> 
> Tested-on: Lenovo Yoga Slim 7x (Snapdragon X Elite)
> Fixes: 44f8901 ("media: i2c: add OmniVision OV02C10 sensor driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Saikiran <bjsaikiran@gmail.com>
> ---
>  drivers/media/i2c/ov02c10.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
> index fa7cc48b769a..3bfbd0deb126 100644
> --- a/drivers/media/i2c/ov02c10.c
> +++ b/drivers/media/i2c/ov02c10.c
> @@ -676,6 +676,12 @@ static int ov02c10_power_on(struct device *dev)
>  	struct ov02c10 *ov02c10 = to_ov02c10(sd);
>  	int ret;
>  
> +	/* 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?

> +		usleep_range(5000, 6000);
> +	}
> +
>  	ret = clk_prepare_enable(ov02c10->img_clk);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to enable imaging clock: %d", ret);
> @@ -691,10 +697,8 @@ static int ov02c10_power_on(struct device *dev)
>  	}
>  
>  	if (ov02c10->reset) {
> -		/* Assert reset for at least 2ms on back to back off-on */
> -		usleep_range(2000, 2200);
>  		gpiod_set_value_cansleep(ov02c10->reset, 0);
> -		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 I²C access.

>  	}
>  
>  	return 0;

-- 
Regards,

Sakari Ailus

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

[Saikiran B]

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.

> > +             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

On Tue, Jan 27, 2026 at 10:37 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> HI Saikiran,
>
> On Tue, Jan 27, 2026 at 10:20:24PM +0530, Saikiran wrote:
> > The previous power-on sequence did not strictly follow the hardware timing
> > requirements (T1), potentially leading to initialization failures on some
> > platforms.
> >
> > Update the sequence to match the datasheet and maintainer recommendations:
> > 1. Assert XSHUTDOWN (reset) for 5ms (T1 >= 5ms) before enabling power
> >    resources.
> > 2. Enable clock and regulators in the standard order.
> > 3. De-assert XSHUTDOWN.
> > 4. Wait 5ms (T2 >= 5ms) for sensor boot before I2C access (using a wider
> >    range for timer coalescing).
> >
> > This ensures the sensor enters a clean state during cold boot.
> >
> > Tested-on: Lenovo Yoga Slim 7x (Snapdragon X Elite)
> > Fixes: 44f8901 ("media: i2c: add OmniVision OV02C10 sensor driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Saikiran <bjsaikiran@gmail.com>
> > ---
> >  drivers/media/i2c/ov02c10.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
> > index fa7cc48b769a..3bfbd0deb126 100644
> > --- a/drivers/media/i2c/ov02c10.c
> > +++ b/drivers/media/i2c/ov02c10.c
> > @@ -676,6 +676,12 @@ static int ov02c10_power_on(struct device *dev)
> >       struct ov02c10 *ov02c10 = to_ov02c10(sd);
> >       int ret;
> >
> > +     /* 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?
>
> > +             usleep_range(5000, 6000);
> > +     }
> > +
> >       ret = clk_prepare_enable(ov02c10->img_clk);
> >       if (ret < 0) {
> >               dev_err(dev, "failed to enable imaging clock: %d", ret);
> > @@ -691,10 +697,8 @@ static int ov02c10_power_on(struct device *dev)
> >       }
> >
> >       if (ov02c10->reset) {
> > -             /* Assert reset for at least 2ms on back to back off-on */
> > -             usleep_range(2000, 2200);
> >               gpiod_set_value_cansleep(ov02c10->reset, 0);
> > -             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 I涎 access.
>
> >       }
> >
> >       return 0;
>
> --
> Regards,
>
> Sakari Ailus

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

[Bryan O'Donoghue]

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

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

[Saikiran B]

> Does this reset fix the problem for you though?

The reset cleanup in this patch (v4) is for correctness (to match T1), but the
primary stability fix for my platform is indeed handling the regulator brownout.

> It is also possible active-discharge is not set on the LDOs ... I guess one way
> to know for sure ... is to never turn the regulators off.

You are spot on. My testing confirms exactly this: if the rail doesn't toggle
(or is given enough time to discharge like right now when I'm using
delay in my dt) -
tested and proven in my v2 patchset
where I kept the regulator on after initial toggle and just handled the camera
via software reset - the sensor works perfectly - which confirms that
the brownout
is actually happening.

The failure only occurs if we toggle the regulator off and on again too fast for
the bulk capacitors to discharge (passive discharge).

Regarding Active Discharge: The `qcom-rpmh-regulator` driver currently lacks
support for the `regulator-pull-down` property (it doesn't send the required
RPMh resource commands). I plan to investigate adding that support separately,
as it would be the ideal long-term fix.

For now, I have submitted a patch series to `linux-regulator` to add
`regulator-off-on-delay-us` support to the `qcom-rpmh-regulator` driver.
Mark Brown has already reviewed it and I have just sent v3.

Once that lands, the Yoga Slim 7x DTS will enforce the physical delay at the
regulator level, which resolves the brownout cleanly.

This media series (v4) is now purely to ensure the OV02C10 driver follows
the datasheet power-up sequence correctly, independent of the platform-specific
brownout.

Thanks & Regards,
Saikiran

On Wed, Jan 28, 2026 at 3:50 AM Bryan O'Donoghue <bod@kernel.org> wrote:
>
> 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

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

[Bryan O'Donoghue]

On 28/01/2026 03:41, Saikiran B wrote:
> Hi Bryan,
> 
>  > Does this reset fix the problem for you though?
> 
> The reset cleanup in this patch (v4) is for correctness (to match T1), 
> but the
> primary stability fix for my platform is indeed handling the regulator 
> brownout.

Hmm. Still not sure I agree with you that its browning out.

>  > It is also possible active-discharge is not set on the LDOs ... I 
> guess one way
>  > to know for sure ... is to never turn the regulators off.
> 
> You are spot on. My testing confirms exactly this: if the rail doesn't 
> toggle -
> (or is given enough time to discharge) tested and proven in v2 patchset 
> where I kept the regulator on after initial toggle and just handled the 
> camera via software reset, the sensor worked perfectly.

Just to be difficult - I'm specifically asking to test never switching 
the regulator off - not having a long delay.


>   The failure only occurs if we toggle the regulator off and on again 
> too fast for
> the bulk capacitors to discharge (passive discharge).

A statement I still don't believe is supported by the available evidence 
have you tested.

- The CCI pin change ?
   This is to see if the CCI pins are inadvertently supplying voltage

- The XSHUTDOWN pin floating change ?


> Regarding Active Discharge: The `qcom-rpmh-regulator` driver currently lacks
> support for the `regulator-pull-down` property (it doesn't send the required
> RPMh resource commands). I plan to investigate adding that support 
> separately,
> as it would be the ideal long-term fix.

I'm not sure RPMh supports that.

What I've done in previous email is show you how we should go about 
determining the setup and the state of the LDOs.

Linux -> RPMh -> SPMI -> LDOs

We can also do

Linux -> SPMI -> LDOs to look at the register state directly.

I have to say I am 100% against adding random delays in the order of 
_seconds_ without getting a good hard look at the LDOs, testing the CCI 
changes and/or testing to see if XSHUTDOWN is floating.

> For now, I have submitted a patch series to `linux-regulator` to add
> `regulator-off-on-delay-us` support to the `qcom-rpmh-regulator` driver.
> Mark Brown has already reviewed it and I have just sent v3.
> 
> Once that lands, the Yoga Slim 7x DTS will enforce the physical delay at the
> regulator level, which resolves the brownout cleanly.

I again.

- Do not believe we have root caused a regulator brown out
- Believe we should interrogate the LDO settings
   We can look at the bits and see if active-discharge is set.

Please do that !

> This media series (v4) is now purely to ensure the OV02C10 driver follows
> the datasheet power-up sequence correctly, independent of the platform- 
> specific
> brownout.

I have the data-sheet so while I think XSHUTDOWN at the start of 
power_on() would be justified IF it fixed the problem for you, we've 
established it does _not_ fix the problem for you.

Please, please, please - re-read the asks I have for you on debugging 
this and engage with them !

---
bod

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

[Bryan O'Donoghue]

On 28/01/2026 10:19, Saikiran B wrote:
>  > Just to be difficult - I'm specifically asking to test never switching
>  > the regulator off - not having a long delay.
> 
> To be absolutely clear:
> 
> ***I have tested exactly this.***
> 
> In my local v2 testing, I modified the driver to keep the regulators
> permanently ENABLED and only toggled the software standby/reset lines.
> 
> Result: The camera was 100% stable over hundreds of cycles.
> 
> This isolates the issue:
> 1. CCI Leaking? No. If CCI were leaking, the "Always On" test would 
> eventually
>     fail or show instability. It did not.

I have to say, I'm not an electrical engineer by profession but, I don't 
believe you can make this blanket statement.

What is the problem with testing the hypothesis ?

> 2. XSHUTDOWN Floating? No. The "Always On" test relies on XSHUTDOWN working
>     correctly to wake the sensor. It worked perfectly.

Yes I agree there, if always-on shows no failure then XSHUTDOWN isnt' 
floating.

In which case this patch can be dropped, its not helping.

> The instability ***only*** appears when we physically toggle the PMIC rail.
> 
>  > Do not believe we have root caused a regulator brown out
>  > Believe we should interrogate the LDO settings
> 
> I cannot easily dump raw SPMI registers on my personal machine, but
> we can derive the LDO state physically from the discharge curve (RC Time 
> Constant).

?

I gave you code to do just that. If you can iterate sensor and DTS 
changes - you can use that code to dump out the requested LDO states.

> We know the physics of the PM8550 PMIC:
> - Active Discharge Resistor (R_active): ~1 kΩ (Typical)
> - Bulk Capacitance (C_bulk): ~10 µF (Estimated for this rail)
> 
> Scenario A: If Active Discharge IS set:
>     Time Constant (T) = R * C = 1000 * 10e-6 = 0.01s (10ms)
>     Complete discharge (5T) would happen in ~50ms.
> 
> Scenario B: If Active Discharge is NOT set (Passive Leakage):
>     The rail discharges through the high-impedance sensor (~200kΩ+).
>     Time Constant (T) = 200,000 * 10e-6 = 2.0s.
> 
> My measurements show the rail takes ~2.0s to reach the Brownout Threshold
> (failure point) and ~2.3s to reach a clean 0V (success point).
> 
> This 2.3s duration is physically impossible if the Active Discharge bit 
> was set.
> It mathematically proves the LDO is in High-Z mode (Passive Discharge).
> 
> Here are the specific logs capturing the failure at exactly the 2.0s mark.
That's great. We should be able to interrogate the PMIC regs and see the 
state of the LDO configuration - code I've shared with you.

If they show active-state isn't set on one or more of our LDOs then we 
can write some platform quirk code to set them.

A 2.3 second delay on every start/stop stream is not an acceptable 
upstream fix.

And please stop top posting !

---
bod

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

[Saikiran B]

On Wed, Jan 28, 2026 at 4:11 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 28/01/2026 10:19, Saikiran B wrote:
> >  > Just to be difficult - I'm specifically asking to test never switching
> >  > the regulator off - not having a long delay.
> >
> > To be absolutely clear:
> >
> > ***I have tested exactly this.***
> >
> > In my local v2 testing, I modified the driver to keep the regulators
> > permanently ENABLED and only toggled the software standby/reset lines.
> >
> > Result: The camera was 100% stable over hundreds of cycles.
> >
> > This isolates the issue:
> > 1. CCI Leaking? No. If CCI were leaking, the "Always On" test would
> > eventually
> >     fail or show instability. It did not.
>
> I have to say, I'm not an electrical engineer by profession but, I don't
> believe you can make this blanket statement.
>
> What is the problem with testing the hypothesis ?
>
> > 2. XSHUTDOWN Floating? No. The "Always On" test relies on XSHUTDOWN working
> >     correctly to wake the sensor. It worked perfectly.
>
> Yes I agree there, if always-on shows no failure then XSHUTDOWN isnt'
> floating.
>
> In which case this patch can be dropped, its not helping.
>
> > The instability ***only*** appears when we physically toggle the PMIC rail.
> >
> >  > Do not believe we have root caused a regulator brown out
> >  > Believe we should interrogate the LDO settings
> >
> > I cannot easily dump raw SPMI registers on my personal machine, but
> > we can derive the LDO state physically from the discharge curve (RC Time
> > Constant).
>
> ?
>
> I gave you code to do just that. If you can iterate sensor and DTS
> changes - you can use that code to dump out the requested LDO states.
>
> > We know the physics of the PM8550 PMIC:
> > - Active Discharge Resistor (R_active): ~1 kΩ (Typical)
> > - Bulk Capacitance (C_bulk): ~10 µF (Estimated for this rail)
> >
> > Scenario A: If Active Discharge IS set:
> >     Time Constant (T) = R * C = 1000 * 10e-6 = 0.01s (10ms)
> >     Complete discharge (5T) would happen in ~50ms.
> >
> > Scenario B: If Active Discharge is NOT set (Passive Leakage):
> >     The rail discharges through the high-impedance sensor (~200kΩ+).
> >     Time Constant (T) = 200,000 * 10e-6 = 2.0s.
> >
> > My measurements show the rail takes ~2.0s to reach the Brownout Threshold
> > (failure point) and ~2.3s to reach a clean 0V (success point).
> >
> > This 2.3s duration is physically impossible if the Active Discharge bit
> > was set.
> > It mathematically proves the LDO is in High-Z mode (Passive Discharge).
> >
> > Here are the specific logs capturing the failure at exactly the 2.0s mark.
> That's great. We should be able to interrogate the PMIC regs and see the
> state of the LDO configuration - code I've shared with you.
>
> If they show active-state isn't set on one or more of our LDOs then we
> can write some platform quirk code to set them.
>
> A 2.3 second delay on every start/stop stream is not an acceptable
> upstream fix.
>
> And please stop top posting !
>
> ---
> bod

Regarding the register dump code you shared:

I have already attempted to use it. It fails to read the registers on this
device. This is a consumer laptop secured by the Gunyah hypervisor
and TrustZone, not an open development board. The SPMI transactions to raw
PMIC addresses are firewalled by the firmware; the Linux HLOS is physically
blocked from reading them.

You mentioned in your previous reply that you are not an electrical engineer.

I am an Electronics Engineer.

The assertion that a 2.3s discharge time proves the absence of Active
Discharge is not a "blanket statement" - it is a fundamental calculation based
on the RC time constant. A 1kΩ active discharge path cannot physically sustain
voltage for 2.3 seconds against a 10µF load; it would drain in milliseconds.
Rejecting this derived data because it relies on physics rather than a
register bit (which the hardware prevents us from reading) is scientifically
unsound.

I have a full-time job and have spent significant personal time debugging this
to the millisecond.

Since we have reached an impasse where the physical
evidence is rejected in favor of impossible diagnostics, I am stopping here.

I will keep my patches in my local tree as now my laptop is usable to me.

I am withdrawing this patchset from upstream consideration.

Thank you.

Regards,
Saikiran

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

[Bryan O'Donoghue]

On 28/01/2026 11:06, Saikiran B wrote:
> On Wed, Jan 28, 2026 at 4:11 PM Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> On 28/01/2026 10:19, Saikiran B wrote:
>>>   > Just to be difficult - I'm specifically asking to test never switching
>>>   > the regulator off - not having a long delay.
>>>
>>> To be absolutely clear:
>>>
>>> ***I have tested exactly this.***
>>>
>>> In my local v2 testing, I modified the driver to keep the regulators
>>> permanently ENABLED and only toggled the software standby/reset lines.
>>>
>>> Result: The camera was 100% stable over hundreds of cycles.
>>>
>>> This isolates the issue:
>>> 1. CCI Leaking? No. If CCI were leaking, the "Always On" test would
>>> eventually
>>>      fail or show instability. It did not.
>>
>> I have to say, I'm not an electrical engineer by profession but, I don't
>> believe you can make this blanket statement.
>>
>> What is the problem with testing the hypothesis ?
>>
>>> 2. XSHUTDOWN Floating? No. The "Always On" test relies on XSHUTDOWN working
>>>      correctly to wake the sensor. It worked perfectly.
>>
>> Yes I agree there, if always-on shows no failure then XSHUTDOWN isnt'
>> floating.
>>
>> In which case this patch can be dropped, its not helping.
>>
>>> The instability ***only*** appears when we physically toggle the PMIC rail.
>>>
>>>   > Do not believe we have root caused a regulator brown out
>>>   > Believe we should interrogate the LDO settings
>>>
>>> I cannot easily dump raw SPMI registers on my personal machine, but
>>> we can derive the LDO state physically from the discharge curve (RC Time
>>> Constant).
>>
>> ?
>>
>> I gave you code to do just that. If you can iterate sensor and DTS
>> changes - you can use that code to dump out the requested LDO states.
>>
>>> We know the physics of the PM8550 PMIC:
>>> - Active Discharge Resistor (R_active): ~1 kΩ (Typical)
>>> - Bulk Capacitance (C_bulk): ~10 µF (Estimated for this rail)
>>>
>>> Scenario A: If Active Discharge IS set:
>>>      Time Constant (T) = R * C = 1000 * 10e-6 = 0.01s (10ms)
>>>      Complete discharge (5T) would happen in ~50ms.
>>>
>>> Scenario B: If Active Discharge is NOT set (Passive Leakage):
>>>      The rail discharges through the high-impedance sensor (~200kΩ+).
>>>      Time Constant (T) = 200,000 * 10e-6 = 2.0s.
>>>
>>> My measurements show the rail takes ~2.0s to reach the Brownout Threshold
>>> (failure point) and ~2.3s to reach a clean 0V (success point).
>>>
>>> This 2.3s duration is physically impossible if the Active Discharge bit
>>> was set.
>>> It mathematically proves the LDO is in High-Z mode (Passive Discharge).
>>>
>>> Here are the specific logs capturing the failure at exactly the 2.0s mark.
>> That's great. We should be able to interrogate the PMIC regs and see the
>> state of the LDO configuration - code I've shared with you.
>>
>> If they show active-state isn't set on one or more of our LDOs then we
>> can write some platform quirk code to set them.
>>
>> A 2.3 second delay on every start/stop stream is not an acceptable
>> upstream fix.
>>
>> And please stop top posting !
>>
>> ---
>> bod
> 
> Regarding the register dump code you shared:
> 
> I have already attempted to use it. It fails to read the registers on this
> device. This is a consumer laptop secured by the Gunyah hypervisor
> and TrustZone, not an open development board. The SPMI transactions to raw
> PMIC addresses are firewalled by the firmware; the Linux HLOS is physically
> blocked from reading them.
> 
> You mentioned in your previous reply that you are not an electrical engineer.
> 
> I am an Electronics Engineer.
> 
> The assertion that a 2.3s discharge time proves the absence of Active
> Discharge is not a "blanket statement" - it is a fundamental calculation based
> on the RC time constant. A 1kΩ active discharge path cannot physically sustain
> voltage for 2.3 seconds against a 10µF load; it would drain in milliseconds.
> Rejecting this derived data because it relies on physics rather than a
> register bit (which the hardware prevents us from reading) is scientifically
> unsound.
> 
> I have a full-time job and have spent significant personal time debugging this
> to the millisecond.
> 
> Since we have reached an impasse where the physical
> evidence is rejected in favor of impossible diagnostics, I am stopping here.
> 
> I will keep my patches in my local tree as now my laptop is usable to me.
> 
> I am withdrawing this patchset from upstream consideration.
> 
> Thank you.
> 
> Regards,
> Saikiran

That's a shame.

---
bod