Re: [PATCH v5 0/4] platform/x86: int3472: Add support for GPIO type 0x02 (strobe)

[Sakari Ailus]

Hi Hans,

On Tue, Mar 31, 2026 at 12:15:55PM +0200, johannes.goede@xxxxxxxxxxxxxxxx wrote:
> Hi Sakari,
> 
> On 30-Mar-26 22:21, Sakari Ailus wrote:
> > Hi Hans, Marco,
> > 
> > On Mon, Mar 30, 2026 at 05:12:21PM +0200, johannes.goede@xxxxxxxxxxxxxxxx wrote:
> >> Hi,
> >>
> >> On 30-Mar-26 16:55, Marco Nenciarini wrote:
> >>> Hi Hans,
> >>>
> >>> Thank you for the detailed feedback.
> >>>
> >>>> My main remark on the current patch set is that IMHO
> >>>> the LED name really should be: OVTIxxxx:00::ir_flood_led
> >>>> since that is what it actually does, strobe is typically
> >>>> related to flash LEDs which despite the naming in Intel's
> >>>> side this is not.
> >>>
> >>> The series actually started with "ir_flood" in v1-v2. It was renamed to
> >>> "strobe" in v3 to match the GPIO type name used in Intel's ACPI _DSM
> >>> tables. But I agree with you that the userspace-visible LED name should
> >>> describe what the hardware actually does, not mirror an internal ACPI
> >>> label. I am happy to go back to "ir_flood" for the LED name.
> >>>
> >>> We could keep INT3472_GPIO_TYPE_STROBE for the define (matching the ACPI
> >>> type value) but pass "ir_flood" as the con_id to the LED registration,
> >>> so userspace sees OVTIxxxx:00::ir_flood_led. Would that work for
> >>> everyone?
> >>
> >> That sounds good to me.
> > 
> > Seems reasonable.
> > 
> >>
> >>>> We really need to get some input from the V4L2 maintainers
> >>>> here on if this is a good idea, before merging this series.
> >>>
> >>> Agreed.
> >>>
> >>>> I can even imagine
> >>>> a default simple mode where the v4l2-core just turns
> >>>> on the LED when streaming starts and off again when
> >>>> streaming stops (very much like the privacy LED) and
> >>>> then in the future maybe we can extend this, e.g.
> >>>> add a control on the sensor device to make this
> >>>> configurable ?
> >>>
> >>> That makes a lot of sense. The current series intentionally keeps things
> >>> minimal (just exposing the LED under /sys/class/leds with no V4L2
> >>> integration), but this future direction sounds right.
> >>>
> >>> I will hold off on sending v6 until we have agreement on the naming and
> >>> Sakari has had a chance to weigh in.
> >>
> >> Ack, lets wait for input from Sakari here.
> > 
> > I wonder if there would be any deterministic ways to find the LED device
> > based on the sensor. It'd probably require more information via MC / V4L2
> > controls to allow that.
> 
> Yes, the int3472 code adds a LED lookup table entry with the consumer-
> device being the sensor.
> 
> So just like v4l2-subdev.c currently does:
> 
> 	sd->privacy_led = led_get(sd->dev, "privacy");
> 
> it will also be able to do:
> 
> 	sd->privacy_led = led_get(sd->dev, "ir_flood");
> 
> > Alternatively we could use a boolean control for this, but I think I'd
> > avoid adding that now and rely on LED API instead.
> > 
> > Are there use cases for this LED, apart from Windows Hello? :-)
> 
> As mentioned for now we could just treat it exactly the same
> as the privacy LED and simply turn it on while streaming
> inside the v4l2-core / v4l2-subdev code.
> 
> This would nicely cover the face ID use-case for which
> Linux implementations exist to.
> 
> And then later of some other use-case comes up we could
> make this a menu-control with on/off/auto values, defaulting
> to auto which would preserve the turn it on while streaming
> behavior.
> 
> We do need to come up with something here, since use-cases
> like Howdy (Linux face-id) will need it. My vote goes to
> give it the same treatment as the privacy LED in the v4l2-core
> for now making things "just work".
> 
> Alternatively we could document that userspace needs to
> find the LED and turn it on itself through the LED classdev
> but that will be painful for userspace to do for various
> reasons including classdev access requiring root rights.

That was actually my concern as well.

Still, if we bind this to the privacy LED now and the LED will effectively
be powered whenever the camera is streaming, we can't drop that interaction
in the future as doing so would break existing users. I'm not sure if there
would ever be a need though.

Either way, I'm starting to feel the V4L2 control might actually be the
best way to go from here.

-- 
Kind regards,

Sakari Ailus