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

[Hans de Goede]

Hi,

On 31-Mar-26 12:55, Andy Shevchenko wrote:
> 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.

Why would we want to not register the lookup for the ir_flood
case too ?

Elsewhere in the thread we're discussing actually controlling
this LED from the v4l2-core which will require the lookup.

And if that does not materialize the lookup still does no harm.

Regards,

Hans