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.
Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
The ACPI _DSM tables refer to this GPIO type as "strobe", hence the
INT3472_GPIO_TYPE_STROBE define. The userspace-visible LED name uses
"ir_flood" instead, as the hardware is an IR flood illuminator, not a
flash strobe.
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
drivers/platform/x86/intel/int3472/discrete.c | 16 +++++++-
drivers/platform/x86/intel/int3472/led.c | 39 +++++++++++--------
include/linux/platform_data/x86/int3472.h | 10 +++--
3 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 57c3b2c..78111e5 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -215,6 +215,10 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
*con_id = "privacy-led";
*gpio_flags = GPIO_ACTIVE_HIGH;
break;
+ case INT3472_GPIO_TYPE_STROBE:
+ *con_id = "ir_flood";
+ *gpio_flags = GPIO_ACTIVE_HIGH;
+ break;
case INT3472_GPIO_TYPE_HOTPLUG_DETECT:
*con_id = "hpd";
*gpio_flags = GPIO_ACTIVE_HIGH;
@@ -248,6 +252,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
*
* 0x00 Reset
* 0x01 Power down
+ * 0x02 Strobe
* 0x0b Power enable
* 0x0c Clock enable
* 0x0d Privacy LED
@@ -331,6 +336,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
break;
case INT3472_GPIO_TYPE_CLK_ENABLE:
case INT3472_GPIO_TYPE_PRIVACY_LED:
+ case INT3472_GPIO_TYPE_STROBE:
case INT3472_GPIO_TYPE_POWER_ENABLE:
case INT3472_GPIO_TYPE_HANDSHAKE:
gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
@@ -348,7 +354,13 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
break;
case INT3472_GPIO_TYPE_PRIVACY_LED:
- ret = skl_int3472_register_led(int3472, gpio, "privacy");
+ ret = skl_int3472_register_led(int3472, gpio, "privacy", true);
+ if (ret)
+ err_msg = "Failed to register LED\n";
+
+ break;
+ case INT3472_GPIO_TYPE_STROBE:
+ ret = skl_int3472_register_led(int3472, gpio, "ir_flood", false);
if (ret)
err_msg = "Failed to register LED\n";
@@ -422,7 +434,7 @@ void int3472_discrete_cleanup(struct int3472_discrete_device *int3472)
gpiod_remove_lookup_table(&int3472->gpios);
skl_int3472_unregister_clock(int3472);
- skl_int3472_unregister_led(int3472);
+ skl_int3472_unregister_leds(int3472);
skl_int3472_unregister_regulator(int3472);
}
EXPORT_SYMBOL_NS_GPL(int3472_discrete_cleanup, "INTEL_INT3472_DISCRETE");
diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index 39466d4..e7cd144 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -4,6 +4,7 @@
#include <linux/acpi.h>
#include <linux/gpio/consumer.h>
#include <linux/leds.h>
+#include <linux/list.h>
#include <linux/platform_data/x86/int3472.h>
static int int3472_led_set(struct led_classdev *led_cdev,
@@ -16,16 +17,19 @@ static int int3472_led_set(struct led_classdev *led_cdev,
}
int skl_int3472_register_led(struct int3472_discrete_device *int3472,
- struct gpio_desc *gpio, const char *con_id)
+ struct gpio_desc *gpio, const char *con_id,
+ bool add_lookup)
{
- struct int3472_led *led = &int3472->led;
+ struct int3472_led *led;
char *p;
int ret;
- if (led->classdev.dev)
- return -EBUSY;
+ if (int3472->n_leds >= INT3472_MAX_LEDS)
+ return -ENOSPC;
+ led = &int3472->leds[int3472->n_leds];
led->gpio = gpio;
+ INIT_LIST_HEAD(&led->lookup.list);
/* Generate the name, replacing the ':' in the ACPI devname with '_' */
snprintf(led->name, sizeof(led->name),
@@ -42,22 +46,25 @@ int skl_int3472_register_led(struct int3472_discrete_device *int3472,
if (ret)
return ret;
- led->lookup.provider = led->name;
- led->lookup.dev_id = int3472->sensor_name;
- led->lookup.con_id = con_id;
- led_add_lookup(&led->lookup);
+ if (add_lookup) {
+ led->lookup.provider = led->name;
+ led->lookup.dev_id = int3472->sensor_name;
+ led->lookup.con_id = con_id;
+ led_add_lookup(&led->lookup);
+ }
+ int3472->n_leds++;
return 0;
}
-void skl_int3472_unregister_led(struct int3472_discrete_device *int3472)
+void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472)
{
- struct int3472_led *led = &int3472->led;
+ for (unsigned int i = 0; i < int3472->n_leds; i++) {
+ struct int3472_led *led = &int3472->leds[i];
- if (IS_ERR_OR_NULL(led->classdev.dev))
- return;
-
- led_remove_lookup(&led->lookup);
- led_classdev_unregister(&led->classdev);
- gpiod_put(led->gpio);
+ if (!list_empty(&led->lookup.list))
+ led_remove_lookup(&led->lookup);
+ led_classdev_unregister(&led->classdev);
+ gpiod_put(led->gpio);
+ }
}
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index 5213911..7366a25 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -23,6 +23,7 @@
/* PMIC GPIO Types */
#define INT3472_GPIO_TYPE_RESET 0x00
#define INT3472_GPIO_TYPE_POWERDOWN 0x01
+#define INT3472_GPIO_TYPE_STROBE 0x02
#define INT3472_GPIO_TYPE_POWER_ENABLE 0x0b
#define INT3472_GPIO_TYPE_CLK_ENABLE 0x0c
#define INT3472_GPIO_TYPE_PRIVACY_LED 0x0d
@@ -31,6 +32,7 @@
#define INT3472_PDEV_MAX_NAME_LEN 23
#define INT3472_MAX_SENSOR_GPIOS 3
+#define INT3472_MAX_LEDS 2
#define INT3472_MAX_REGULATORS 3
/* E.g. "avdd\0" */
@@ -126,11 +128,12 @@ struct int3472_discrete_device {
struct led_lookup_data lookup;
char name[INT3472_LED_MAX_NAME_LEN];
struct gpio_desc *gpio;
- } led;
+ } leds[INT3472_MAX_LEDS];
struct int3472_discrete_quirks quirks;
unsigned int ngpios; /* how many GPIOs have we seen */
+ unsigned int n_leds; /* how many LEDs have we registered */
unsigned int n_sensor_gpios; /* how many have we mapped to sensor */
unsigned int n_regulator_gpios; /* how many have we mapped to a regulator */
struct gpiod_lookup_table gpios;
@@ -161,7 +164,8 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
int skl_int3472_register_led(struct int3472_discrete_device *int3472,
- struct gpio_desc *gpio, const char *con_id);
-void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
+ struct gpio_desc *gpio, const char *con_id,
+ bool add_lookup);
+void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472);
#endif
--
2.47.3On Tue, Mar 31, 2026 at 09:52:04AM +0200, 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. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> -- With Best Regards, Andy Shevchenko
Hi Marco,
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.
>
> Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
> ---
>
> The ACPI _DSM tables refer to this GPIO type as "strobe", hence the
> INT3472_GPIO_TYPE_STROBE define. The userspace-visible LED name uses
> "ir_flood" instead, as the hardware is an IR flood illuminator, not a
> flash strobe.
>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> drivers/platform/x86/intel/int3472/discrete.c | 16 +++++++-
> drivers/platform/x86/intel/int3472/led.c | 39 +++++++++++--------
> include/linux/platform_data/x86/int3472.h | 10 +++--
> 3 files changed, 44 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 57c3b2c..78111e5 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -215,6 +215,10 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
> *con_id = "privacy-led";
How about changing this to "privacy" (this is only
used internally really, so we're free to change it).
Note this should already be done in patch 3/4
to reduce churn below.
> *gpio_flags = GPIO_ACTIVE_HIGH;
> break;
> + case INT3472_GPIO_TYPE_STROBE:
> + *con_id = "ir_flood";
and here keep using "ir_flood" / keep as is.
> + *gpio_flags = GPIO_ACTIVE_HIGH;
> + break;
> case INT3472_GPIO_TYPE_HOTPLUG_DETECT:
> *con_id = "hpd";
> *gpio_flags = GPIO_ACTIVE_HIGH;
> @@ -248,6 +252,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
> *
> * 0x00 Reset
> * 0x01 Power down
> + * 0x02 Strobe
> * 0x0b Power enable
> * 0x0c Clock enable
> * 0x0d Privacy LED
> @@ -331,6 +336,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
> break;
> case INT3472_GPIO_TYPE_CLK_ENABLE:
> case INT3472_GPIO_TYPE_PRIVACY_LED:
> + case INT3472_GPIO_TYPE_STROBE:
> case INT3472_GPIO_TYPE_POWER_ENABLE:
> case INT3472_GPIO_TYPE_HANDSHAKE:
> gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
> @@ -348,7 +354,13 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>
> break;
> case INT3472_GPIO_TYPE_PRIVACY_LED:
> - ret = skl_int3472_register_led(int3472, gpio, "privacy");
> + ret = skl_int3472_register_led(int3472, gpio, "privacy", true);
> + if (ret)
> + err_msg = "Failed to register LED\n";
> +
> + break;
> + case INT3472_GPIO_TYPE_STROBE:
> + 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.
As an added bonus you've already named the new skl_int3472_register_led()
parameter con_id, so now that will nicely line-up too :)
Regards,
Hans
> if (ret)
> err_msg = "Failed to register LED\n";
>
> @@ -422,7 +434,7 @@ void int3472_discrete_cleanup(struct int3472_discrete_device *int3472)
> gpiod_remove_lookup_table(&int3472->gpios);
>
> skl_int3472_unregister_clock(int3472);
> - skl_int3472_unregister_led(int3472);
> + skl_int3472_unregister_leds(int3472);
> skl_int3472_unregister_regulator(int3472);
> }
> EXPORT_SYMBOL_NS_GPL(int3472_discrete_cleanup, "INTEL_INT3472_DISCRETE");
> diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
> index 39466d4..e7cd144 100644
> --- a/drivers/platform/x86/intel/int3472/led.c
> +++ b/drivers/platform/x86/intel/int3472/led.c
> @@ -4,6 +4,7 @@
> #include <linux/acpi.h>
> #include <linux/gpio/consumer.h>
> #include <linux/leds.h>
> +#include <linux/list.h>
> #include <linux/platform_data/x86/int3472.h>
>
> static int int3472_led_set(struct led_classdev *led_cdev,
> @@ -16,16 +17,19 @@ static int int3472_led_set(struct led_classdev *led_cdev,
> }
>
> int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> - struct gpio_desc *gpio, const char *con_id)
> + struct gpio_desc *gpio, const char *con_id,
> + bool add_lookup)
> {
> - struct int3472_led *led = &int3472->led;
> + struct int3472_led *led;
> char *p;
> int ret;
>
> - if (led->classdev.dev)
> - return -EBUSY;
> + if (int3472->n_leds >= INT3472_MAX_LEDS)
> + return -ENOSPC;
>
> + led = &int3472->leds[int3472->n_leds];
> led->gpio = gpio;
> + INIT_LIST_HEAD(&led->lookup.list);
>
> /* Generate the name, replacing the ':' in the ACPI devname with '_' */
> snprintf(led->name, sizeof(led->name),
> @@ -42,22 +46,25 @@ int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> if (ret)
> return ret;
>
> - led->lookup.provider = led->name;
> - led->lookup.dev_id = int3472->sensor_name;
> - led->lookup.con_id = con_id;
> - led_add_lookup(&led->lookup);
> + if (add_lookup) {
> + led->lookup.provider = led->name;
> + led->lookup.dev_id = int3472->sensor_name;
> + led->lookup.con_id = con_id;
> + led_add_lookup(&led->lookup);
> + }
>
> + int3472->n_leds++;
> return 0;
> }
>
> -void skl_int3472_unregister_led(struct int3472_discrete_device *int3472)
> +void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472)
> {
> - struct int3472_led *led = &int3472->led;
> + for (unsigned int i = 0; i < int3472->n_leds; i++) {
> + struct int3472_led *led = &int3472->leds[i];
>
> - if (IS_ERR_OR_NULL(led->classdev.dev))
> - return;
> -
> - led_remove_lookup(&led->lookup);
> - led_classdev_unregister(&led->classdev);
> - gpiod_put(led->gpio);
> + if (!list_empty(&led->lookup.list))
> + led_remove_lookup(&led->lookup);
> + led_classdev_unregister(&led->classdev);
> + gpiod_put(led->gpio);
> + }
> }
> diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
> index 5213911..7366a25 100644
> --- a/include/linux/platform_data/x86/int3472.h
> +++ b/include/linux/platform_data/x86/int3472.h
> @@ -23,6 +23,7 @@
> /* PMIC GPIO Types */
> #define INT3472_GPIO_TYPE_RESET 0x00
> #define INT3472_GPIO_TYPE_POWERDOWN 0x01
> +#define INT3472_GPIO_TYPE_STROBE 0x02
> #define INT3472_GPIO_TYPE_POWER_ENABLE 0x0b
> #define INT3472_GPIO_TYPE_CLK_ENABLE 0x0c
> #define INT3472_GPIO_TYPE_PRIVACY_LED 0x0d
> @@ -31,6 +32,7 @@
>
> #define INT3472_PDEV_MAX_NAME_LEN 23
> #define INT3472_MAX_SENSOR_GPIOS 3
> +#define INT3472_MAX_LEDS 2
> #define INT3472_MAX_REGULATORS 3
>
> /* E.g. "avdd\0" */
> @@ -126,11 +128,12 @@ struct int3472_discrete_device {
> struct led_lookup_data lookup;
> char name[INT3472_LED_MAX_NAME_LEN];
> struct gpio_desc *gpio;
> - } led;
> + } leds[INT3472_MAX_LEDS];
>
> struct int3472_discrete_quirks quirks;
>
> unsigned int ngpios; /* how many GPIOs have we seen */
> + unsigned int n_leds; /* how many LEDs have we registered */
> unsigned int n_sensor_gpios; /* how many have we mapped to sensor */
> unsigned int n_regulator_gpios; /* how many have we mapped to a regulator */
> struct gpiod_lookup_table gpios;
> @@ -161,7 +164,8 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
> void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
>
> int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> - struct gpio_desc *gpio, const char *con_id);
> -void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
> + struct gpio_desc *gpio, const char *con_id,
> + bool add_lookup);
> +void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472);
>
> #endif
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
© 2016 - 2026 Red Hat, Inc.