Re: [BUG] OV02C10 on Dell 16 Premium DA16250 (ARL): INT3472 handshake-derived "dvdd" regulator registered but never linked to sensor, sensor probe fails with -EREMOTEIO

["Angioli Samuele (gmail)" <angioli.samuele@gmail.com>]

Marco,

Option A is in. Applied cleanly to v7.0 (the HANDSHAKE arm already 
shares register_regulator via the POWER_ENABLE fallthrough; 
second_sensor and avdd_second_sensor present), built as a single module, 
loaded.
The keying works: post-patch ov02c10 (which bulk-gets dovdd/avdd/dvdd) 
only logs "not found" for dovdd and avdd, no longer for dvdd, so
second_sensor="i2c-OVTI02C1:00" did attach the OV02C10 to DSC0's dvdd.
(regulator_summary still shows INT3472:0c-dvdd use=0 -- devm releasing 
the handle on the -121 probe failure, not the get failing.) So the
second_sensor mechanism is validated for dvdd.

But it does not bring the sensor up, and the reason closes the case in a
way the quirk cannot help. Two facts:

1. DSC1 is firmware-disabled:
      /sys/bus/acpi/devices/INT3472:01/status = 0   (\_SB.PC00.DSC1)
      /sys/bus/acpi/devices/INT3472:0c/status = 15  (\_SB.PC00.DSC0)
      /sys/bus/platform/devices/ : only INT3472:0c
    INT3472:01 has _STA=0, gets no platform device, never probes (with
    dyndbg on, only INT3472:0c appears). The OV02C10 _DEPs DSC1
    (DLK1 = {DSC1, HS09.VIC1}), but that instance is off.
    DSC0 is the only live INT3472.

2. DSC0 carries only dvdd + the IR-flood strobe -- nothing else:
      gpioinfo: a single int3472-held line, consumer="dvdd"
      int3472 enumeration (dyndbg): dvdd (HS09.VGPO) + GPIO type 0x02 
(the strobe), no reset/powerdown/clk/avdd/dovdd
    So DSC0 has no reset, clock, avdd or dovdd to offer anyone.

Put together: the OV02C10 needs dvdd/avdd/dovdd plus reset and clock. 
The only one any live instance provides is dvdd, on DSC0, which the 
quirk re-keys correctly. avdd/dovdd fall to dummies, and reset/clock 
would have been DSC1's -- but DSC1 is _STA=0. There is nothing on a live 
instance to re-key those to, so the second_sensor approach cannot 
synthesise them.

So the dvdd quirk is right and worth keeping for boards where DSC1 is
enabled, but it is not sufficient for the DA16250 as shipped: the 
sensor's remaining rails/reset/clock have no provider while DSC1 is 
disabled. That makes this a firmware defect that needs a firmware fix 
(enable DSC1, or expose the rails on a live instance), unless we go to a 
board-specific quirk that hardcodes fixed always-on regulators for 
avdd/dovdd and sources reset/clock directly -- which needs the DA16250 
schematic and only works if those rails are actually controllable.

Earlier revision: my previous "DSC1 owns avdd/dovdd/reset" was wrong --
DSC1 owns nothing, it is _STA=0.

Happy to test a board quirk if you want to prototype one, but I suspect
this one is on Dell's firmware.

Thanks,
Samuele

Il 14/06/26 22:23, Marco Nenciarini ha scritto:
> Hans, Sakari,
> 
> Samuele's data is in and it confirms both halves of the wrong-instance
> keying.
> 
> DSC0 = INT3472:0c, DSC1 = INT3472:01. The decisive line from DSC0's
> probe, with int3472 dyndbg on:
> 
>    int3472-discrete INT3472:0c: Sensor name HIMX1092:00
> 
> So DSC0's reverse-_DEP walk resolves to the first consumer that _DEPs
> it, the Himax IR camera (HIMX1092:00, Windows Hello), and that is the
> dev_name DSC0's dvdd supply_map is keyed on. The OV02C10 (OVTI02C1:00)
> never appears in it. regulator_summary corroborates from the other
> side: the only dvdd on
> the whole platform is INT3472:0c-dvdd (DSC0), orphaned at use=0, and
> DSC1 (which the RGB sensor actually _DEPs) exposes no dvdd at all, only
> avdd/dovdd/reset.
> 
> So the topology is settled: there is exactly one dvdd handshake on the
> platform, gated by DSC0, keyed to the IR sensor DSC0 serves, while the
> RGB sensor that appears to want it _DEPs DSC1 instead, and nothing
> connects the two. The remaining inference is the rail-to-failure link
> itself: dvdd falls to a dummy under full constraints, and the chip-ID
> read at 0x300a returns -EREMOTEIO with no retry. That chain is
> consistent with the data but I have not proven dvdd is the cause as
> opposed to a coincident orphan (see the test ask to Samuele below),
> so I would not call the failure mechanism closed yet, only the keying.
> 
> On whose defect this is: either the firmware under-specifies the
> dependency (the OV02C10's dvdd is physically gated by DSC0 but its _DEP
> points only at DSC1), or the kernel's reverse-_DEP consumer model
> cannot express a rail that lives on a sibling INT3472 instance. Either
> way this is shipping DA16250 firmware that will not change, so the
> camera needs an in-tree path regardless of where we assign blame.
> 
> That makes this the same class of problem int3472 already handles with
> the second_sensor quirk. avdd_second_sensor (the Lenovo Miix 510 entry
> in discrete_quirks.c) already plants a second supply_map entry, keyed
> to a hardcoded device name in addition to the reverse-_DEP sensor_name,
> and skl_int3472_register_regulator() takes second_sensor for exactly
> that. The DA16250 is the same shape, just on dvdd/HANDSHAKE rather than
> avdd/POWER_ENABLE, and the HANDSHAKE branch currently always passes
> second_sensor = NULL. So the contained fix is: extend second_sensor to
> cover the dvdd/HANDSHAKE arm too. The struct has a single second_sensor
> field today (avdd_second_sensor), so generalising that one field reads
> cleaner to me than adding a per-con_id dvdd_second_sensor, but either
> works; then add a DA16250 DMI entry pointing dvdd's second consumer at
> i2c-OVTI02C1:00. This adds OVTI02C1 as a second consumer of DSC0's
> dvdd, it does not move the rail off HIMX1092:00, so the IR camera's own
> supply is untouched.
> 
> The alternative is to make that second-consumer resolution automatic
> rather than DMI-gated, i.e. teach int3472 to discover that a sensor may
> draw a rail from an instance it does not _DEP on. More correct in
> principle, but ACPI gives no signal to key it on here (that is the
> firmware gap), so it would need a heuristic and I would not want it
> silently re-homing rails on boards where the current keying is right.
> 
> My instinct is the DMI quirk, extending the mechanism you already use
> for avdd. DMI is the right key rather than the int3472_gpio_map[] HID
> table, because the OV02C10 part is not the problem, the DA16250 _DEP
> topology is. I would leave the fully-automatic resolution open in case
> more ARL boards turn up the same split. Tell me which way you want it
> and I will prototype the quirk against the DA16250.
> 
> Samuele, here is a concrete test that closes the last gap. It is the
> proposed fix in miniature, so a positive result validates both at once.
> Two ways to run it, pick whichever suits your setup. In both, the
> signal is the same: does the 0x300a chip-ID read in dmesg succeed, or
> still return -EREMOTEIO?
> 
> Option A, the patch (preferred, it exercises the real consumer path).
> In skl_int3472_handle_gpio_resources(), in
> drivers/platform/x86/intel/int3472/discrete.c, add the two marked lines
> to the regulator arm of the switch:
> 
> 	case INT3472_GPIO_TYPE_POWER_ENABLE:
> 		second_sensor = int3472->quirks.avdd_second_sensor;
> 		fallthrough;
> 	case INT3472_GPIO_TYPE_DOVDD:
> 	case INT3472_GPIO_TYPE_HANDSHAKE:
> +		if (type == INT3472_GPIO_TYPE_HANDSHAKE)
> +			second_sensor = "i2c-OVTI02C1:00";	/* test */
> 		ret = skl_int3472_register_regulator(int3472, gpio, enable_time_us,
> 						     con_id, second_sensor);
> 
> This adds OVTI02C1:00 as a second consumer of DSC0's dvdd alongside the
> existing HIMX1092:00 mapping; it does not move the rail off the IR
> camera. The hardcoded string makes this test-only (it would mis-key on
> any other handshake board); the shipped form is the DMI-gated quirk
> above. The hunk applies to a recent mainline discrete.c, where the
> HANDSHAKE case shares the register_regulator call via the POWER_ENABLE
> fallthrough, so build from a source tree matching your running kernel.
> First confirm int3472 is a module, not built in:
> 
> 	modinfo intel_skl_int3472_discrete
> 	# or check CONFIG_INTEL_SKL_INT3472 in your kernel config
> 
> If it is =y you need a full kernel build instead. If =m, build and
> install just this module, then reboot for a clean re-probe:
> 
> 	make -C /lib/modules/$(uname -r)/build \
> 	     M=$PWD/drivers/platform/x86/intel/int3472 modules
> 	sudo make -C /lib/modules/$(uname -r)/build \
> 	     M=$PWD/drivers/platform/x86/intel/int3472 modules_install
> 	sudo depmod -a
> 	sudo reboot
> 
> (Build against the configured source for your running kernel, or the
> new .ko may refuse to load on a modversions/CRC mismatch.) After
> reboot, in /sys/kernel/debug/regulator/regulator_summary the
> INT3472:0c-dvdd line should now list the i2c-OVTI02C1:00 device as a
> consumer (use count > 0), and dmesg shows whether 0x300a now succeeds.
> 
> Option B, no rebuild, force the rail on by hand. On this board DSC0's
> _DSM exposes only func 2 (the dvdd handshake) and func 3 (the IR-flood
> strobe), so unbinding it drops exactly those two and nothing the
> OV02C10 needs. dvdd is a GPIO-gated regulator, so as root:
> 
> 	# 1. BEFORE unbinding, capture the dvdd enable line: int3472
> 	#    requests it with consumer label "dvdd", so in gpioinfo find
> 	#    the line whose consumer is "dvdd" and note its gpiochip (the
> 	#    block header) and offset. The label disappears once you
> 	#    unbind, so record chip+offset now. Your int3472 dyndbg log
> 	#    cross-checks it:
> 	#      "INT3472:0c: dvdd <acpi-gpio-path> pin <N> active-<high|low>"
> 	#    (that line gives the ACPI controller path and pin, not the
> 	#    gpiochip number, so map it through gpioinfo). Note the
> 	#    active- sense too, you need it in step 3:
> 	gpioinfo
> 
> 	# 2. release the line by unbinding the PMIC (drops only DSC0's
> 	#    dvdd regulator and IR strobe, not the sensor, which _DEPs
> 	#    DSC1):
> 	echo INT3472:0c > /sys/bus/platform/drivers/int3472-discrete/unbind
> 
> 	# 3. drive the line to its ON level and HOLD it (own shell, leave
> 	#    running). ON is =1 if step 1 showed active-high, =0 if
> 	#    active-low. Using the gpiochip and offset from step 1:
> 	#    libgpiod v1:  gpioset --mode=signal gpiochipN <offset>=<on>
> 	#    libgpiod v2:  gpioset -c gpiochipN <offset>=<on>  (holds until Ctrl-C)
> 	gpioset --mode=signal gpiochipN <offset>=<on>
> 
> 	# 4. in another shell, re-probe the sensor and read the log:
> 	echo i2c-OVTI02C1:00 > /sys/bus/i2c/drivers/ov02c10/bind
> 	dmesg | tail
> 
> 	# 5. when done, Ctrl-C the gpioset and reboot to restore normal
> 	#    driver state.
> 
> (If step 4 says the device is already bound, unbind it first via the
> same path, then bind.) regulator_get("dvdd") lands on a dummy here,
> since there is no provider once DSC0 is unbound, but that is a no-op
> enable and the rail is physically on because you are holding the GPIO
> at its ON level, so the chip-ID read is still the signal.
> 
> Either way: if 0x300a succeeds, that pins powering dvdd as what
> unblocks the chip-ID read, and the fix is exactly Option A folded into
> a DA16250 DMI quirk. If it still fails, dvdd is not the (only) problem
> and the -EREMOTEIO is coming from reset, clock, or the NX33 bridge, and
> we look there instead. Option A is the better single test, since it
> keeps DSC0 bound and exercises the real enable path with the right
> polarity and timing; Option B trades that for no rebuild, so if the two
> disagree, trust A.
> 
> Thanks,
> Marco