Re: [PATCH v2] platform/x86: int3472: map the ov7251 power enable GPIO to "vdda"

[Jakob Berg Jespersen]

Hi,

Tested on a second Surface Pro 7+ (Tiger Lake, IPU6), on v7.3-rc1 with
your patch as the only change, taint 0 and no out-of-tree modules.

Before:

  ov7251 i2c-INT347E:00: supply vdda not found, using dummy regulator
  ov7251 i2c-INT347E:00: ov7251_write_reg: write reg error -121: reg=103, val=1
  ov7251 i2c-INT347E:00: probe with driver ov7251 failed with error -121

After:

  ov7251 i2c-INT347E:00: supply vdddo not found, using dummy regulator
  ov7251 i2c-INT347E:00: supply vddd not found, using dummy regulator
  ov7251 i2c-INT347E:00: OV7251 revision 7 (1F) detected at address 0x60

and the device binds. vddd and vdddo stay on dummy regulators as you
describe.

Tested-by: Jakob Berg Jespersen <dev@xxxxxxx>   # Surface Pro 7+, probe/power-on

I have only confirmed that the sensor powers up and probes. 
I could not get it streaming here, libcamera 0.7.1 refuses 
the sensor on this machine:

  'ov7251 3-0060': Mandatory V4L2 control 0x009e0903 not available
  Failed to create sensor for 'ov7251 3-0060': -22

0x009e0903 is V4L2_CID_ANALOGUE_GAIN, and the in-tree ov7251 registers
V4L2_CID_GAIN instead, so only the front and rear cameras get
enumerated. 

I have a small unsubmitted patch that I had set aside to focus on 
the front and back cameras.  With this patch, on top of yours, 
libcamera lists the IR camera as well:

  Available cameras:
  1: Internal front camera (\_SB_.PC00.I2C2.CAMF)
  2: 'ov7251' (\_SB_.PC00.I2C3.CAM3)
  3: Internal back camera (\_SB_.PC00.I2C3.CAMR)

Did you have something equivalent downstream? Happy to send it to you
or to the list if it would help. 

I should say I have not got a usable image out of it here yet: Frames
come through at the right size and rate, but the content is saturated
white, and the sensor's own test pattern does not come through either.


Regards,
Jakob


On Monday, August 31st, 2026 at 12:23, D. Manresa <dmanresa@xxxxxxxxx> wrote:

> On IPU6 designs carrying an OV7251 IR sensor (ACPI HID INT347E) the INT3472
> describes the sensor's single gated rail as a power-enable GPIO, which the
> driver registers as a regulator with con_id "avdd". The ov7251 driver and its
> DT bindings name the sensor supplies vdda, vddd and vdddo, so the lookup finds
> nothing, the rail is never enabled, and probe fails on the first I2C access:
> 
>   int3472-discrete INT3472:02: avdd  \_SB.GPI0 pin 131 active-high
>   ov7251 i2c-INT347E:00: supply vdda not found, using dummy regulator
>   ov7251 i2c-INT347E:00: probe with driver ov7251 failed with error -121
> 
> Map the power-enable GPIO to con_id "vdda" for INT347E, the same way the
> existing entries map it to "vdd" for the mt9m114 and to "vana" for the imx471.
> The sensor's other two supplies keep falling back to the dummy regulator,
> which is correct here: the firmware gates a single rail.
> 
> The table already has an INT347E entry mapping the reset GPIO to "enable",
> added when the ov7251 was brought up on these machines; this is the same class
> of firmware-vs-binding name mismatch for the remaining pin.
> 
> Tested on a Microsoft Surface Pro 7+ (Tiger Lake, IPU6): with this change the
> IR sensor probes, the illuminator strobes, and the camera streams 640x480 at
> 30 fps through libcamera. The same failure was reproduced independently on a
> second Pro 7+ running stock firmware and a distro kernel, with no out-of-tree
> code, in [1].
> 
> Developed with the assistance of an AI tool (Claude) and verified on hardware.
> 
> Link: https://github.com/linux-surface/linux-surface/issues/1702 [1]
> Reviewed-by: Hans de Goede <johannes.goede@xxxxxxxxxxxxxxxx>
> Signed-off-by: D. Manresa <dmanresa@xxxxxxxxx>
> ---
> v2: no code changes. v1 never reached the lists (a mail tooling error on
> my side dropped the Cc headers); resent with platform-driver-x86 on Cc as
> Hans requested, with his Reviewed-by collected.
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 6c729fc..157345e 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -143,6 +143,11 @@ static const char * const power_enable_hids_enable[] = {
>  	NULL
>  };
> 
> +static const char * const power_enable_hids_vdda[] = {
> +	"INT347E", /* ov7251 */
> +	NULL
> +};
> +
>  /**
>   * struct int3472_gpio_map - Map GPIOs to whatever is expected by the
>   * sensor driver (as in DT bindings)
> @@ -185,6 +190,13 @@ static const struct int3472_gpio_map int3472_gpio_map[] = {
>  		.con_id = "dvdd",
>  		.enable_time_us = 45 * USEC_PER_MSEC,
>  	},
> +	{	/* Sensors which expect "vdda" as con_id for power enable */
> +		.hids = power_enable_hids_vdda,
> +		.type_from = INT3472_GPIO_TYPE_POWER_ENABLE,
> +		.type_to = INT3472_GPIO_TYPE_POWER_ENABLE,
> +		.con_id = "vdda",
> +		.enable_time_us = GPIO_REGULATOR_ENABLE_TIME,
> +	},
>  	{	/* Sensors which expect "vana" as con_id for power enable */
>  		.hids = power_enable_hids_vana,
>  		.type_from = INT3472_GPIO_TYPE_POWER_ENABLE,
> --
> 2.43.0
>