Re: [PATCH v3 3/3] media: i2c: ov02c10: Use runtime PM autosuspend to avoid brownouts

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 27/01/2026 10:56, Saikiran B wrote:
> Hi Bryan,
> 
> I understand your suspicion regarding the LDO behavior, but I lack the
> hardware documentation (PMIC register maps) and tooling to interrogate
> the SPMI registers to the depth you are requesting.
> 

So, SPMI is not exported in /sys/kernel/debug/regmap - however

drivers/regulator/qcom-rpmh-regulator.c

Lets add this to probe

unsigned int val, i;
     u16 bases[] = {0x4000, 0x4300, 0x4600}; // LDO1, LDO4, LDO7
     const char *names[] = {"LDO1(1.2V)", "LDO4(1.8V)", "LDO7(2.8V)"};
     struct regmap *p_regmap = dev_get_regmap(dev->parent, NULL);

     if (p_regmap) {
         pr_info("--- OV02C10 PMIC RAIL DUMP START ---\n");
         for (i = 0; i < 3; i++) {
             // Check Config (Active Discharge)
             regmap_read(p_regmap, bases[i] + 0x41, &val);
             pr_info("!!! %s SEC_CTRL (0x%04x) = 0x%02x (Bit7: Active 
Discharge)\n",
                     names[i], bases[i] + 0x41, val);

             // Check Status (Is it actually on?)
             regmap_read(p_regmap, bases[i] + 0x08, &val);
             pr_info("!!! %s STATUS   (0x%04x) = 0x%02x (Bit7: VREG_OK, 
Bit0: VREG_ON)\n",
                     names[i], bases[i] + 0x08, val);

             // Check Pull-down config (Secondary check)
             regmap_read(p_regmap, bases[i] + 0x42, &val);
             pr_info("!!! %s PD_CTRL   (0x%04x) = 0x%02x\n",
                     names[i], bases[i] + 0x42, val);
         }
         pr_info("--- OV02C10 PMIC RAIL DUMP END ---\n");
     }

>  From my end, the empirical reality on my machine is that the sensor 
> fails if power-cycled
> faster than 2.3s, and enforcing that delay (via software or regulator core)
> fixes the issue reliably.
> 
> Since we cannot agree on the root cause and I cannot perform the hardware
> debugging required, I will stop submitting patches for this issue.
> I'll maintain the regulator workaround in my local tree.
> 
> I kindly thank you and Hans for your time reviewing the previous versions.
> 
> Thanks & Regards,
> Saikiran
> 
> On Tue, 27 Jan 2026, 16:21 Bryan O'Donoghue, <bryan.odonoghue@linaro.org 
> <mailto:bryan.odonoghue@linaro.org>> wrote:
> 
>     On 27/01/2026 10:40, Saikiran B wrote:
>      > Hi Bryan,
>      >
>      > Regarding the 1.1s race condition:
>      >
>      > I have implemented support for the generic regulator-off-on-delay-us
>      > property
>      > in the qcom-rpmh-regulator driver and set the constraint to 2.3s
>     in the
>      > device tree for the Yoga Slim 7x.
> 
>     Yes but please listen to me. That is an extraordinary delay being
>     introduced.
> 
>     It is indicative of a serious problem we have not root caused. These
>     LDOs are used in mobile phones which are aggressively designed to save
>     power all the time.
> 
>     In fact the whole idea of voting for clocks and bandwidth is it
>     mitigate
>     the default assumption in these class of devices - switch off the power
>     first.
> 
>     What is that 2.3 seconds, why is it needed. "Brownout" but why ? I
>     don't
>     think we have really established.
> 
>      > I tested the 1.1s scenario you mentioned, and it is working fine.
>     The
>      > regulator
>      > core now correctly blocks the enable call until the physical
>     discharge delay
>      > has passed, preventing the brownout without needing logic in the
>     camera
>      > driver.
> 
>     The physical discharge delay we have _not_ established IMO. Have you
>     checked the CCI pins ?
> 
>     I think we should stop pushing patches until a root-cause has been
>     identified.
> 
>     For example - we can interrogate the LDO settings via SPMI registers to
>     see if the LDO is really switched off.
> 
>     Similarly we can interrogate the LDOs to see if they are set for active
>     discharge.
> 
>     A fix might be to make a platform driver to set those bits for the
>     relevant LDOs absent a firmware fix for the same.
> 
>     I'm not comfortable pushing changes predicated on papering over an
>     issue
>     that hasn't been root-caused.
> 
>      >
>      > I am going to drop the Autosuspend patch entirely and verify the
>     clean
>      > driver one last time.
>      >
>      > Plan for v4:
>      > 1. Submit the Regulator/DT fixes separately to linux-arm-msm.
>      > 2. Submit v4 of this series containing only the cleanup and power-
>      > sequence fixes.
>      >
>      > Thanks for pushing for the correct fix, the regulator approach is
>     indeed
>      > much cleaner.
>      >
>      > Thanks & Regards,
>      > Saikiran
>      >
>      >
>      > On Tue, 27 Jan 2026, 15:16 Bryan O'Donoghue,
>     <bryan.odonoghue@linaro.org <mailto:bryan.odonoghue@linaro.org>
>      > <mailto:bryan.odonoghue@linaro.org
>     <mailto:bryan.odonoghue@linaro.org>>> wrote:
>      >
>      >     On 26/01/2026 17:34, Saikiran wrote:
>      >      > On Qualcomm X1E80100 platforms, the OV02C10 sensor experiences
>      >     brownouts
>      >      > if power-cycled too quickly (< 2.3s) due to slow passive
>     discharge of
>      >      > regulator rails.
>      >      >
>      >      > Implement Runtime PM Autosuspend with a delay of 1000ms. This
>      >     keeps the
>      >      > regulators enabled for a short duration after the device
>     is closed,
>      >      > preventing costly power-off/power-on cycles during rapid user
>      >      > interactions (e.g., browser permission checks).
>      >
>      >     But if you try to power the sensor 1.1 seconds later what
>     happens ?
>      >
>      >     With this commit log this submission is a NAK, for example
>     why do I
>      >     want
>      >     this change on an x86 machine ?
>      >
>      >     We need to root-cause the failure not paper over it.
>      >
>      >     ---
>      >     bod
>      >
>