Re: [PATCH] platform/x86: int3472: handle GPIO type 0x08 as a power regulator

Jakob Berg Jespersen <[email protected]>
Newsgroups org.kernel.vger.platform-driver-x86,org.kernel.vger.linux-kernel
Message-ID <wWCJgdk1-0MrfeN6bRValpATGnQ4BcEzShHI5qI7Nywi6LWcYiq9BDKaaaH5Y0SmHJl2IlGNepOmbjVkwr0APh4dA1fzX7M9VCWZM6XR_-w=@berg.pm>
On Monday, July 20th, 2026 at 12:14, Hans de Goede <[email protected]> wrote:

> Hi,
> 
> On 20-Jul-26 00:00, Sakari Ailus wrote:
> > Hi Jakob,
> >
> > Thanks for the patch.
> >
> > On Sun, Jul 19, 2026 at 08:15:12PM +0200, Jakob Berg Jespersen wrote:
> >> On the Microsoft Surface Pro 7+, the rear camera's INT3472 device
> >> (INT3472:01, sensor INT347A / ov8865) enumerates a GPIO of type 0x08
> >> in its _DSM which the driver does not recognise, so it is left
> >> unmapped:
> >>
> >>   int3472-discrete INT3472:01: GPIO type 0x08 unknown; the sensor may
> >>   not work
> >>
> >> The DSDT shows this pin (0x0100AF08, pin 175, active-high) sitting
> >> alongside a normal type-0x0b power-enable GPIO: it gates a second
> >> sensor power rail. Because the type is unknown the rail is never
> >> enabled, the ov8865's "dvdd" supply resolves to a dummy regulator, and
> >> the sensor never probes.
> >>
> >> Map type 0x08 for INT347A to a POWER_ENABLE regulator with con_id
> >> "dvdd", the supply the in-tree ov8865 driver already requests, so no
> >> sensor-driver change is needed. ("dvdd" rather than "avdd" because the
> >> type-0x0b power-enable pin on this INT3472 is already registered as
> >> "avdd".)
> >>
> >> Type 0x08 is not in the driver's documented function list and is not
> >> otherwise handled by mainline; based on the sensor's supply set and the
> >> con_id that brings it up, it appears to be the DVDD (digital core
> >> voltage) rail, the counterpart to the existing DOVDD (0x10) type.
> >>
> >> Related out-of-tree work approached the same rail by adding a new "pwr1"
> >> supply instead of the sensor's existing "dvdd" (linux-surface PR #1867
> >> for the Surface Pro 9, PR #2201 for the Pro 7+ ov8865); neither was sent
> >> upstream. Mapping to "dvdd" keeps the change contained to int3472.
> >>
> >> With this change the SP7+ rear camera probes and streams reliably,
> >> validated by over a month of use.
> >>
> >> Link: https://github.com/linux-surface/linux-surface/pull/1867
> >> Link: https://github.com/linux-surface/linux-surface/pull/2201
> >> Cc: Tooraj Taraz <[email protected]>
> >> Cc: Joseph V. Lavigne <[email protected]>
> >> Signed-off-by: Jakob Berg Jespersen <[email protected]>
> >> ---
> >> 0x08 is left as a bare literal rather than a named constant here; if
> >> its canonical meaning is known I'm happy to add e.g.
> >> INT3472_GPIO_TYPE_DVDD and/or handle it generically like DOVDD in v2.
> >>
> >> This is my first kernel contribution, so please bear with me on any
> >> process mistakes.
> >> ---
> >>  drivers/platform/x86/intel/int3472/discrete.c | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> >> index 115bb37577a1..98c7b9bc393f 100644
> >> --- a/drivers/platform/x86/intel/int3472/discrete.c
> >> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> >> @@ -164,6 +164,19 @@ static const struct int3472_gpio_map int3472_gpio_map[] = {
> >>  		.con_id = "dvdd",
> >>  		.enable_time_us = 45 * USEC_PER_MSEC,
> >>  	},
> >> +	{	/*
> >> +		 * Surface Pro 7+ ov8865 (rear camera): type 0x08 is an
> >> +		 * undocumented sensor power rail; left undriven the sensor
> >> +		 * stays in a failed power state. "dvdd" rather than "avdd"
> >> +		 * because this INT3472 also has a type 0x0b (power enable)
> >> +		 * pin already registered as "avdd".
> >> +		 */
> >
> > GPIO type 8 is in fact "POWER1" (type 7 being "POWER0"). This hasn't been
> > seen anywhere yet so support for it hasn't been added either.
> >
> > I understand the POWER_ENABLE GPIO is for executing the entire power
> > sequence and isn't expected to be found with the other regulators, hence an
> > apparent conflict here wouldn't be an issue.
> >
> > I think for now we could map type POWER1 (8) to "dvdd", however not using a
> > quirk but doing it for all devices, like mapping POWER_ENABLE to "avdd".
> > That'd leave mapping POWER0 to "avdd" for later. The GPIO definition for
> > POWER0 could be added in any case.
> 
> I was wondering what the internal Intel headers said for type 8, so thank
> you for clarifying that that is called POWER1. Adding a generic mapping
> of POWER1 to "dvdd" instead of using a quirk sounds good to me.
> 
> Regards,
> 
> Hans
> 
> 


Thanks both, and thanks for the POWER0/POWER1 naming.

That approach makes sense to me. For v2 I'll:

 * add INT3472_GPIO_TYPE_POWER0 (7) and INT3472_GPIO_TYPE_POWER1 (8)
   definitions,
 * map POWER1 to "dvdd" generically in int3472_get_con_id_and_polarity(),
   dropping the INT347A quirk,
 * leave POWER0 defined but unmapped for now.

I'll retest on the Surface Pro 7+ and send v2, with a Suggested-by:
trailer for you, Sakari, for the generic approach.

Thanks for the quick review.
Jakob
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.