* [PATCH v2] platform/x86: int3472: map the ov7251 power enable GPIO to "vdda"
@ 2026-08-31 10:23 D. Manresa
2026-08-31 15:15 ` Jakob Berg Jespersen
0 siblings, 1 reply; 3+ messages in thread
From: D. Manresa @ 2026-08-31 10:23 UTC (permalink / raw)
To: Daniel Scally, Sakari Ailus, Hans de Goede, Ilpo Järvinen
Cc: Hans de Goede, Jakob Berg Jespersen, platform-driver-x86,
linux-media, linux-kernel, D . Manresa
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@oss.qualcomm.com>
Signed-off-by: D. Manresa <dmanresa@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] platform/x86: int3472: map the ov7251 power enable GPIO to "vdda"
2026-08-31 10:23 [PATCH v2] platform/x86: int3472: map the ov7251 power enable GPIO to "vdda" D. Manresa
@ 2026-08-31 15:15 ` Jakob Berg Jespersen
2026-09-01 6:33 ` D. Manresa
0 siblings, 1 reply; 3+ messages in thread
From: Jakob Berg Jespersen @ 2026-08-31 15:15 UTC (permalink / raw)
To: D. Manresa
Cc: Daniel Scally, Sakari Ailus, Hans de Goede, Ilpo Järvinen,
Hans de Goede, platform-driver-x86, linux-media, linux-kernel
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@berg.pm> # 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@gmail.com> 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@oss.qualcomm.com>
> Signed-off-by: D. Manresa <dmanresa@gmail.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] platform/x86: int3472: map the ov7251 power enable GPIO to "vdda"
2026-08-31 15:15 ` Jakob Berg Jespersen
@ 2026-09-01 6:33 ` D. Manresa
0 siblings, 0 replies; 3+ messages in thread
From: D. Manresa @ 2026-09-01 6:33 UTC (permalink / raw)
To: Jakob Berg Jespersen
Cc: Daniel Scally, Sakari Ailus, Hans de Goede, Hans de Goede,
Ilpo Järvinen, platform-driver-x86, linux-media,
linux-kernel, D . Manresa
On Sun, 31 Aug 2026, Jakob Berg Jespersen wrote:
> Tested-by: Jakob Berg Jespersen <dev@berg.pm> # Surface Pro 7+, probe/power-on
Thanks for the quick test!
> 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.
> [...]
> Did you have something equivalent downstream? Happy to send it to you
> or to the list if it would help.
Yes - and so does everyone else, independently, which is the strongest
possible argument for sending it to the list. The linux-surface patchset has
carried exactly that two-line change since 2023: "media: i2c: Clarify that
gain is Analogue gain in OV7251", by Dan Scally (on Cc), in
patches/6.19/0013-cameras.patch. It was never submitted to mainline. Last
week a Fedora user rediscovered the same fix from scratch on another Surface
Pro 7+ (linux-surface PR #2252), and now you make the third. Since libcamera
hard-requires V4L2_CID_ANALOGUE_GAIN, mainline ov7251 is unusable with
libcamera on every machine until this lands.
Dan, it is your patch - would you like to send it upstream yourself? If you
would rather not, either of us can, with your authorship preserved.
> 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.
That is almost certainly not the sensor: it matches, symptom for symptom, the
IPU6 CSI-2 receiver desynchronisation I reported on linux-media yesterday -
"ipu6-isys: MCD PHY is initialised before the sensor is powered; intermittent
D-PHY desync at stream start (TGL)". The signature: a session starts with the
receiver desynced, every frame arrives at the right size and rate but the
content is garbage railed at full scale (for a 10-bit sensor most rows pin
near 1023 - saturated white), the sensor's test pattern does not survive
either (nothing valid does), and dmesg shows "csi2-N error: DPHY fatal error /
SOT sync error" from the first frame. Three checks on your machine:
- dmesg right after the white session starts: if the DPHY/SOT errors are
there, it is the receiver, not the sensor;
- close the video device and start a new session: on my unit the desync is
per-STREAMON and clears on the next attempt (~3% failure rate at stock
30 fps timing, but it drifts with uptime - I have measured up to ~50%);
- if the front camera is streaming at the same time, stop it first: the
OV5693 (port 4) and OV7251 (port 5) share MCD PHY1, and starting the IR
while the front holds the PHY made it fail 22 times out of 22 here.
My IR capture path retries the session start automatically on that signature,
which makes it invisible in practice. Details and the register-level evidence
are in that thread.
D. Manresa <dmanresa@gmail.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-01 6:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 10:23 [PATCH v2] platform/x86: int3472: map the ov7251 power enable GPIO to "vdda" D. Manresa
2026-08-31 15:15 ` Jakob Berg Jespersen
2026-09-01 6:33 ` D. Manresa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox