Re: [PATCH v3] platform/x86: int3472: support the POWER1 GPIO type

[Hans de Goede]

Hi,

On 30-Aug-26 3:41 PM, Sergey Lebedev wrote:
> Hans pointed me at this from a report I sent this morning about the same
> GPIO type on a Surface Pro 11 - thank you, and sorry for the duplicate
> question. I have now tested this patch on that machine, which is a third
> model and, more usefully, a different sensor. Result below, with the part
> that is still missing for this sensor family.
> 
> Tested-by: Sergey Lebedev <lsa.uz@xxxxx>   # Surface Pro 11, INT3472 side
> 
> What the patch fixes here
> -------------------------
> 
> Built on 7.0.0-30 (Ubuntu 26.04). The warning is gone and the rail is
> mapped:
> 
>   before: int3472-discrete INT3472:00: GPIO type 0x08 unknown;
>                                        the sensor may not work
>   after : no int3472 messages at all
> 
>   /sys/class/regulator:
>     regulator.1  INT3472:00-avdd
>     regulator.2  INT3472:00-dvdd     <- new, from this patch
>     regulator.3  INT3472:01-avdd
>     regulator.4  INT3472:01-dovdd
>     regulator.5  INT3472:02-avdd
> 
> Nothing else regressed: audio, Secure Boot and module signing unaffected,
> no failed units.
> 
> What it does not fix, and why that is not this patch's fault
> ------------------------------------------------------------
> 
> The camera is exactly as dead as before:
> 
>   ov13858 i2c-OVTID858:00: failed to find sensor: -5
>   every regulator: num_users=0, state=disabled
>   /dev/media0: 0 entities
> 
> The rear sensor here is an OV13858, and the in-tree ov13858 driver requests
> no regulators and touches no GPIOs at all - zero `regulator` and zero
> `gpiod` references in drivers/media/i2c/ov13858.c. So INT3472:00-dvdd is
> registered and then never claimed by anyone, and the sensor is still held
> in reset because nothing releases it.
> 
> That is exactly the difference between your machine and this one. ov8865
> asks for "dvdd", "dovdd" and "avdd" by name, so mapping POWER1 to "dvdd"
> completes the picture for the Surface Pro 7+. ov13858 asks for nothing.
> 
> The same conclusion was reached independently on the Surface Pro 10, which
> carries the same OV13858:
> 
>   https://github.com/linux-surface/linux-surface/issues/2153
> 
> There they had to add reset-GPIO handling to ov13858_probe() and force the
> regulators on, and describe the latter as too broad for upstream.
> 
> So: this patch is correct and necessary, and for the OV13858 machines it is
> not sufficient. The remaining work is in the sensor driver rather than in
> int3472, which seems worth stating explicitly so nobody expects the Pro 10
> or Pro 11 rear camera to start working when this lands.
> 
> If it would help, I am happy to test a patch teaching ov13858 to request
> its supplies and release reset - it is the same shape as what ov8865
> already does. The machine is here and I can build and boot kernels on it.

Right. Someone needs to write 2 patches for the ov13858 driver to add:

1. Regulator support, this should list the 3 standard:

static const char * const ov02c10_supply_names[] = {
        "dovdd",        /* Digital I/O power */
        "avdd",         /* Analog power */
        "dvdd",         /* Digital core power */
};

OV sensor supply names. This should use the bulk regulator API,
request these at probe and turn them on / off at stream start / stop time.

The ov2c10 driver in drivers/media/i2c/ov2c10 with its
ov02c10_get_pm_resources() ov02c10_power_off() and
ov02c10_power_on() functions is a good example of how
to handle this.

2. Add reset GPIO support, again the ov02c10 driver and
the 3 mentioned helper functions there are a good example
to copy for this.

Regards,

Hans