Re: [PATCH v6 4/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED)

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 12:36:28PM +0200, Hans de Goede wrote:
> On 31-Mar-26 09:52, Marco Nenciarini wrote:
> > Add support for GPIO type 0x02, which controls an IR flood LED used
> > for face authentication on some laptops (e.g. Dell Pro Max 16 Premium).
> > 
> > Without this patch, the kernel logs "GPIO type 0x02 unknown; the sensor
> > may not work" and IR sensors paired with a flood LED cannot function.
> > 
> > The flood LED is registered through the LED subsystem like the existing
> > privacy LED. Unlike the privacy LED, it does not have a lookup entry
> > since there is no consumer driver expecting it via led_get().
> > 
> > To support multiple LEDs per INT3472 device, convert the single led
> > struct member to an array with a counter.

...

> > +			ret = skl_int3472_register_led(int3472, gpio, "privacy", true);
> > +			if (ret)
> > +				err_msg = "Failed to register LED\n";
> > +

> > +			ret = skl_int3472_register_led(int3472, gpio, "ir_flood", false);
> 
> and then here instead of having 2 almost identical cases just pass con_id
> note please already use con_id here in patch 3/4 to avoid churn.
> 
> After changing that in 3/4 you only need to add a single:
> 
> +		case INT3472_GPIO_TYPE_STROBE:
> 
> here to make it also handle the strobe/ir_flood case.

We would also need to recognize need for a lookup. Either keeping
a boolean that needs to be initialised into something like
`type != INT3472_GPIO_TYPE_STROBE` or turning back to switch-case
for that in the callee:

	switch (type) {
	case _PRIVACY:
		lookup = ...
		led_add_lookup(...);
		break;
	case _STROBE: // no lookup for now
	default:
		break;
	}

given the nice and neat caller side I would move to switch-case.

-- 
With Best Regards,
Andy Shevchenko