[PATCH] platform/x86: int3472: Add GPIO type 0x02 (strobe) mapping

[PATCH] platform/x86: int3472: Add GPIO type 0x02 (strobe) mapping

[Marco Nenciarini]

Some ACPI tables on Intel Meteor Lake and Arrow Lake platforms define
GPIO resources with type 0x02 for INT3472 camera sensor power control.
This type is not currently handled, causing a "GPIO type 0x02 unknown;
the sensor may not work" warning and preventing proper sensor power
sequencing.

Map GPIO type 0x02 (strobe) to the privacy LED handler, matching the
approach used for similar illumination-related GPIOs. This is consistent
with the original proposal by Hao Yao [1].

Tested on a Dell Pro Max 16 Premium (Arrow Lake-H) with OmniVision
OV08F4 sensor connected via Intel IPU6 and Synaptics SVP7500 USBIO
bridge. With this patch applied, the int3472-discrete driver no longer
warns about unknown GPIO type 0x02, and the sensor probes successfully.

[1] https://lore.kernel.org/all/20231007021309.9332-1-hao.yao@intel.com/

Link: https://bugs.debian.org/1130114
Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2138991
Cc: Hao Yao <hao.yao@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
 drivers/platform/x86/intel/int3472/discrete.c | 3 +++
 include/linux/platform_data/x86/int3472.h     | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 1505fc3..39ca60b 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -211,6 +211,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 		*con_id = "clk-enable";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
+	case INT3472_GPIO_TYPE_STROBE:
 	case INT3472_GPIO_TYPE_PRIVACY_LED:
 		*con_id = "privacy-led";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
@@ -330,6 +331,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 
 		break;
 	case INT3472_GPIO_TYPE_CLK_ENABLE:
+	case INT3472_GPIO_TYPE_STROBE:
 	case INT3472_GPIO_TYPE_PRIVACY_LED:
 	case INT3472_GPIO_TYPE_POWER_ENABLE:
 	case INT3472_GPIO_TYPE_HANDSHAKE:
@@ -347,6 +349,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 				err_msg = "Failed to register clock\n";
 
 			break;
+		case INT3472_GPIO_TYPE_STROBE:
 		case INT3472_GPIO_TYPE_PRIVACY_LED:
 			ret = skl_int3472_register_pled(int3472, gpio);
 			if (ret)
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index b1b8375..85adeb0 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
-- 
2.47.3

[PATCH v2] platform/x86: int3472: Handle GPIO type 0x02 (strobe) as IR flood LED

[Marco Nenciarini]

GPIO type 0x02 (strobe) appears in ACPI tables on recent Intel
Meteor Lake and Arrow Lake platforms (e.g. Dell Pro Max 14/16) for
controlling an IR flood LED used by infrared camera sensors. The
INT3472 discrete driver does not handle this type, logging "GPIO type
0x02 unknown; the sensor may not work" and potentially preventing the
associated sensor slot from initializing cleanly.

Add support for GPIO type 0x02 by registering it as an LED through
the existing LED framework, with the name "ir_flood_led" to
distinguish it from the privacy LED (type 0x0d).

Rename skl_int3472_register_pled() and skl_int3472_unregister_pled()
to skl_int3472_register_led() and skl_int3472_unregister_led(). The
register function takes a name parameter and an optional LED lookup
con_id. For the privacy LED, "privacy" is passed so sensor drivers
can discover it via devm_led_get(). For the IR flood LED, NULL is
passed to register only the LED classdev without a driver lookup,
since no sensor driver currently consumes it.

Link: https://lore.kernel.org/linux-media/ab0URIrzZPsYjWrM@spark.kcore.it/
Suggested-by: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
 drivers/platform/x86/intel/int3472/discrete.c | 20 ++++++++++++++++--
 drivers/platform/x86/intel/int3472/led.c      | 21 ++++++++++++-------
 include/linux/platform_data/x86/int3472.h     |  7 +++++--
 3 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 1505fc3..4d5f40a 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -211,6 +211,10 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 		*con_id = "clk-enable";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
+	case INT3472_GPIO_TYPE_STROBE:
+		*con_id = "ir-flood-led";
+		*gpio_flags = GPIO_ACTIVE_HIGH;
+		break;
 	case INT3472_GPIO_TYPE_PRIVACY_LED:
 		*con_id = "privacy-led";
 		*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 (IR flood LED)
  * 0x0b Power enable
  * 0x0c Clock enable
  * 0x0d Privacy LED
@@ -330,6 +335,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 
 		break;
 	case INT3472_GPIO_TYPE_CLK_ENABLE:
+	case INT3472_GPIO_TYPE_STROBE:
 	case INT3472_GPIO_TYPE_PRIVACY_LED:
 	case INT3472_GPIO_TYPE_POWER_ENABLE:
 	case INT3472_GPIO_TYPE_HANDSHAKE:
@@ -346,9 +352,19 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 			if (ret)
 				err_msg = "Failed to register clock\n";
 
+			break;
+		case INT3472_GPIO_TYPE_STROBE:
+			ret = skl_int3472_register_led(int3472, gpio,
+						       "ir_flood_led",
+						       NULL);
+			if (ret)
+				err_msg = "Failed to register LED\n";
+
 			break;
 		case INT3472_GPIO_TYPE_PRIVACY_LED:
-			ret = skl_int3472_register_pled(int3472, gpio);
+			ret = skl_int3472_register_led(int3472, gpio,
+						       "privacy_led",
+						       "privacy");
 			if (ret)
 				err_msg = "Failed to register LED\n";
 
@@ -422,7 +438,7 @@ void int3472_discrete_cleanup(struct int3472_discrete_device *int3472)
 	gpiod_remove_lookup_table(&int3472->gpios);
 
 	skl_int3472_unregister_clock(int3472);
-	skl_int3472_unregister_pled(int3472);
+	skl_int3472_unregister_led(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 b1d84b9..e3ab40e 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -16,7 +16,9 @@ static int int3472_pled_set(struct led_classdev *led_cdev,
 	return 0;
 }
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
+int skl_int3472_register_led(struct int3472_discrete_device *int3472,
+			    struct gpio_desc *gpio, const char *led_name,
+			    const char *lookup_con_id)
 {
 	char *p;
 	int ret;
@@ -28,7 +30,7 @@ int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gp
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
 	snprintf(int3472->pled.name, sizeof(int3472->pled.name),
-		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
+		 "%s::%s", acpi_dev_name(int3472->sensor), led_name);
 	p = strchr(int3472->pled.name, ':');
 	if (p)
 		*p = '_';
@@ -41,20 +43,23 @@ int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gp
 	if (ret)
 		return ret;
 
-	int3472->pled.lookup.provider = int3472->pled.name;
-	int3472->pled.lookup.dev_id = int3472->sensor_name;
-	int3472->pled.lookup.con_id = "privacy";
-	led_add_lookup(&int3472->pled.lookup);
+	if (lookup_con_id) {
+		int3472->pled.lookup.provider = int3472->pled.name;
+		int3472->pled.lookup.dev_id = int3472->sensor_name;
+		int3472->pled.lookup.con_id = lookup_con_id;
+		led_add_lookup(&int3472->pled.lookup);
+	}
 
 	return 0;
 }
 
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472)
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472)
 {
 	if (IS_ERR_OR_NULL(int3472->pled.classdev.dev))
 		return;
 
-	led_remove_lookup(&int3472->pled.lookup);
+	if (int3472->pled.lookup.con_id)
+		led_remove_lookup(&int3472->pled.lookup);
 	led_classdev_unregister(&int3472->pled.classdev);
 	gpiod_put(int3472->pled.gpio);
 }
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index b1b8375..a7cf290 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
@@ -160,7 +161,9 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
 				   const char *second_sensor);
 void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472);
+int skl_int3472_register_led(struct int3472_discrete_device *int3472,
+			    struct gpio_desc *gpio, const char *led_name,
+			    const char *lookup_con_id);
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
 
 #endif
-- 
2.47.3

Re: [PATCH v2] platform/x86: int3472: Handle GPIO type 0x02 (strobe) as IR flood LED

[Andy Shevchenko]

On Mon, Mar 23, 2026 at 10:35:35PM +0100, Marco Nenciarini wrote:
> GPIO type 0x02 (strobe) appears in ACPI tables on recent Intel
> Meteor Lake and Arrow Lake platforms (e.g. Dell Pro Max 14/16) for
> controlling an IR flood LED used by infrared camera sensors. The
> INT3472 discrete driver does not handle this type, logging "GPIO type
> 0x02 unknown; the sensor may not work" and potentially preventing the
> associated sensor slot from initializing cleanly.
> 
> Add support for GPIO type 0x02 by registering it as an LED through
> the existing LED framework, with the name "ir_flood_led" to
> distinguish it from the privacy LED (type 0x0d).
> 
> Rename skl_int3472_register_pled() and skl_int3472_unregister_pled()
> to skl_int3472_register_led() and skl_int3472_unregister_led(). The
> register function takes a name parameter and an optional LED lookup
> con_id. For the privacy LED, "privacy" is passed so sensor drivers
> can discover it via devm_led_get(). For the IR flood LED, NULL is
> passed to register only the LED classdev without a driver lookup,
> since no sensor driver currently consumes it.

This can be improved.

...

> +		case INT3472_GPIO_TYPE_STROBE:
> +			ret = skl_int3472_register_led(int3472, gpio,
> +						       "ir_flood_led",
> +						       NULL);
> +			if (ret)
> +				err_msg = "Failed to register LED\n";
> +
>  			break;
>  		case INT3472_GPIO_TYPE_PRIVACY_LED:
> -			ret = skl_int3472_register_pled(int3472, gpio);
> +			ret = skl_int3472_register_led(int3472, gpio,
> +						       "privacy_led",
> +						       "privacy");

Maybe pass the connection ID for a starter (separate prerequisite) and here add
a new LED and new parameter for the lookup instantiation?

>  			if (ret)
>  				err_msg = "Failed to register LED\n";

...

So, this one

> +int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> +			    struct gpio_desc *gpio, const char *led_name,
> +			    const char *lookup_con_id)

Becomes first

int skl_int3472_register_led(struct int3472_discrete_device *int3472,
			    struct gpio_desc *gpio, const char *con_id)

followed by

int skl_int3472_register_led(struct int3472_discrete_device *int3472,
			    struct gpio_desc *gpio, const char *con_id,
			    bool add_lookup)


>  	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
>  	snprintf(int3472->pled.name, sizeof(int3472->pled.name),
> -		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
> +		 "%s::%s", acpi_dev_name(int3472->sensor), led_name);

This will be

		 "%s::%s_led", acpi_dev_name(int3472->sensor), con_id);

...

> -	int3472->pled.lookup.provider = int3472->pled.name;
> -	int3472->pled.lookup.dev_id = int3472->sensor_name;
> -	int3472->pled.lookup.con_id = "privacy";
> -	led_add_lookup(&int3472->pled.lookup);
> +	if (lookup_con_id) {
> +		int3472->pled.lookup.provider = int3472->pled.name;
> +		int3472->pled.lookup.dev_id = int3472->sensor_name;
> +		int3472->pled.lookup.con_id = lookup_con_id;
> +		led_add_lookup(&int3472->pled.lookup);
> +	}
>  
>  	return 0;

And this

	if (!add_lookup)
		return 0;

	int3472->pled.lookup.provider = int3472->pled.name;
	int3472->pled.lookup.dev_id = int3472->sensor_name;
	int3472->pled.lookup.con_id = con_id;
	led_add_lookup(&int3472->pled.lookup);

	return 0;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] platform/x86: int3472: Handle GPIO type 0x02 (strobe) as IR flood LED

[Andy Shevchenko]

On Tue, Mar 24, 2026 at 02:56:09PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 23, 2026 at 10:35:35PM +0100, Marco Nenciarini wrote:

...

> So, this one
> 
> > +int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> > +			    struct gpio_desc *gpio, const char *led_name,
> > +			    const char *lookup_con_id)
> 
> Becomes first
> 
> int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> 			    struct gpio_desc *gpio, const char *con_id)
> 
> followed by
> 
> int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> 			    struct gpio_desc *gpio, const char *con_id,
> 			    bool add_lookup)

Alternatively we may pass a enum of led type...

...

> > -	int3472->pled.lookup.provider = int3472->pled.name;
> > -	int3472->pled.lookup.dev_id = int3472->sensor_name;
> > -	int3472->pled.lookup.con_id = "privacy";
> > -	led_add_lookup(&int3472->pled.lookup);
> > +	if (lookup_con_id) {
> > +		int3472->pled.lookup.provider = int3472->pled.name;
> > +		int3472->pled.lookup.dev_id = int3472->sensor_name;
> > +		int3472->pled.lookup.con_id = lookup_con_id;
> > +		led_add_lookup(&int3472->pled.lookup);
> > +	}
> >  
> >  	return 0;
> 
> And this
> 
> 	if (!add_lookup)
> 		return 0;
> 
> 	int3472->pled.lookup.provider = int3472->pled.name;
> 	int3472->pled.lookup.dev_id = int3472->sensor_name;
> 	int3472->pled.lookup.con_id = con_id;
> 	led_add_lookup(&int3472->pled.lookup);
> 
> 	return 0;

and use switch-case here

	switch (led_type) {
	case _PRIVACY:
		...add lookup...
		break;
	case _IR:
		// do nothing;
		break;
	default:
		break;
	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] platform/x86: int3472: Handle GPIO type 0x02 (strobe) as IR flood LED

[Andy Shevchenko]

On Tue, Mar 24, 2026 at 02:56:05PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 23, 2026 at 10:35:35PM +0100, Marco Nenciarini wrote:

Note, you use some outdated email addresses. Also I can point to my script [1]
that uses tools to (almost, as it's based on heuristics) properly form the To
and Cc list.

[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 0/2] platform/x86: int3472: Add support for strobe LED (GPIO type 0x02)

[Marco Nenciarini]

Some ACPI INT3472 devices include a GPIO with DSM type 0x02, used for
IR flood (strobe) illumination, which is currently unhandled.

This series first renames the privacy LED code to generic LED naming,
then adds strobe support with an enum-driven switch that controls both
the LED name and whether a lookup is registered.

Changes in v3:
- Split into 2 patches as requested by Andy Shevchenko
- Patch 1/2: pure rename of pled to led (no functional change)
- Patch 2/2: add strobe type with enum, convert single LED member to
  array to support devices with both privacy and strobe GPIOs

Marco Nenciarini (2):
  platform/x86: int3472: Rename pled to led in LED registration code
  platform/x86: int3472: Add support for GPIO type 0x02 (strobe LED)

 drivers/platform/x86/intel/int3472/discrete.c | 18 ++++-
 drivers/platform/x86/intel/int3472/led.c      | 79 +++++++++++++------
 include/linux/platform_data/x86/int3472.h     | 19 ++++-
 3 files changed, 84 insertions(+), 32 deletions(-)

-- 
2.47.3

[PATCH v3 1/2] platform/x86: int3472: Rename pled to led in LED registration code

[Marco Nenciarini]

Rename the privacy LED functions, struct type and member from
pled/int3472_pled/register_pled to led/int3472_led/register_led to
prepare for supporting additional LED types (e.g. strobe/IR flood).

No functional change.

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Changes in v3:
- New patch, split out from v2 single patch per Andy Shevchenko's request

 drivers/platform/x86/intel/int3472/discrete.c |  4 +-
 drivers/platform/x86/intel/int3472/led.c      | 45 ++++++++++---------
 include/linux/platform_data/x86/int3472.h     |  8 ++--
 3 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 1505fc3..cb24763 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -348,7 +348,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 
 			break;
 		case INT3472_GPIO_TYPE_PRIVACY_LED:
-			ret = skl_int3472_register_pled(int3472, gpio);
+			ret = skl_int3472_register_led(int3472, gpio);
 			if (ret)
 				err_msg = "Failed to register LED\n";
 
@@ -422,7 +422,7 @@ void int3472_discrete_cleanup(struct int3472_discrete_device *int3472)
 	gpiod_remove_lookup_table(&int3472->gpios);
 
 	skl_int3472_unregister_clock(int3472);
-	skl_int3472_unregister_pled(int3472);
+	skl_int3472_unregister_led(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 b1d84b9..8b26929 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -6,55 +6,56 @@
 #include <linux/leds.h>
 #include <linux/platform_data/x86/int3472.h>
 
-static int int3472_pled_set(struct led_classdev *led_cdev,
-				     enum led_brightness brightness)
+static int int3472_led_set(struct led_classdev *led_cdev,
+			   enum led_brightness brightness)
 {
 	struct int3472_discrete_device *int3472 =
-		container_of(led_cdev, struct int3472_discrete_device, pled.classdev);
+		container_of(led_cdev, struct int3472_discrete_device, led.classdev);
 
-	gpiod_set_value_cansleep(int3472->pled.gpio, brightness);
+	gpiod_set_value_cansleep(int3472->led.gpio, brightness);
 	return 0;
 }
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
+int skl_int3472_register_led(struct int3472_discrete_device *int3472,
+			     struct gpio_desc *gpio)
 {
 	char *p;
 	int ret;
 
-	if (int3472->pled.classdev.dev)
+	if (int3472->led.classdev.dev)
 		return -EBUSY;
 
-	int3472->pled.gpio = gpio;
+	int3472->led.gpio = gpio;
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
-	snprintf(int3472->pled.name, sizeof(int3472->pled.name),
+	snprintf(int3472->led.name, sizeof(int3472->led.name),
 		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
-	p = strchr(int3472->pled.name, ':');
+	p = strchr(int3472->led.name, ':');
 	if (p)
 		*p = '_';
 
-	int3472->pled.classdev.name = int3472->pled.name;
-	int3472->pled.classdev.max_brightness = 1;
-	int3472->pled.classdev.brightness_set_blocking = int3472_pled_set;
+	int3472->led.classdev.name = int3472->led.name;
+	int3472->led.classdev.max_brightness = 1;
+	int3472->led.classdev.brightness_set_blocking = int3472_led_set;
 
-	ret = led_classdev_register(int3472->dev, &int3472->pled.classdev);
+	ret = led_classdev_register(int3472->dev, &int3472->led.classdev);
 	if (ret)
 		return ret;
 
-	int3472->pled.lookup.provider = int3472->pled.name;
-	int3472->pled.lookup.dev_id = int3472->sensor_name;
-	int3472->pled.lookup.con_id = "privacy";
-	led_add_lookup(&int3472->pled.lookup);
+	int3472->led.lookup.provider = int3472->led.name;
+	int3472->led.lookup.dev_id = int3472->sensor_name;
+	int3472->led.lookup.con_id = "privacy";
+	led_add_lookup(&int3472->led.lookup);
 
 	return 0;
 }
 
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472)
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472)
 {
-	if (IS_ERR_OR_NULL(int3472->pled.classdev.dev))
+	if (IS_ERR_OR_NULL(int3472->led.classdev.dev))
 		return;
 
-	led_remove_lookup(&int3472->pled.lookup);
-	led_classdev_unregister(&int3472->pled.classdev);
-	gpiod_put(int3472->pled.gpio);
+	led_remove_lookup(&int3472->led.lookup);
+	led_classdev_unregister(&int3472->led.classdev);
+	gpiod_put(int3472->led.gpio);
 }
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index b1b8375..7af6731 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -121,12 +121,12 @@ struct int3472_discrete_device {
 		u8 imgclk_index;
 	} clock;
 
-	struct int3472_pled {
+	struct int3472_led {
 		struct led_classdev classdev;
 		struct led_lookup_data lookup;
 		char name[INT3472_LED_MAX_NAME_LEN];
 		struct gpio_desc *gpio;
-	} pled;
+	} led;
 
 	struct int3472_discrete_quirks quirks;
 
@@ -160,7 +160,7 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
 				   const char *second_sensor);
 void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472);
+int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
 
 #endif
-- 
2.47.3

[PATCH v3 2/2] platform/x86: int3472: Add support for GPIO type 0x02 (strobe LED)

[Marco Nenciarini]

Some ACPI INT3472 devices include a GPIO with DSM type 0x02, used for
IR flood (strobe) illumination. This GPIO type was previously
unhandled, resulting in the following warning during probe:

  int3472-discrete INT3472:00: GPIO type 0x02 is not currently
  supported

Add INT3472_GPIO_TYPE_STROBE (0x02) handling that registers the GPIO
as an LED class device via skl_int3472_register_led(). An enum
int3472_led_type parameter controls both the LED name suffix and
whether a lookup is registered for the sensor driver. Unlike the
privacy LED, the strobe LED is not consumed by the sensor driver, so
no LED lookup is registered.

To support devices that have both a privacy and a strobe LED, the
single struct int3472_led member is replaced with an array and the
container_of in the brightness callback now references the int3472_led
struct directly.

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Changes in v3:
- Use enum int3472_led_type to control name and lookup behavior
- Convert single LED member to array for multi-LED support
- Add default: return -EINVAL for safety

 drivers/platform/x86/intel/int3472/discrete.c | 18 ++++-
 drivers/platform/x86/intel/int3472/led.c      | 74 +++++++++++++------
 include/linux/platform_data/x86/int3472.h     | 17 ++++-
 3 files changed, 80 insertions(+), 29 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index cb24763..0cffd13 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -207,6 +207,10 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 		*con_id = "powerdown";
 		*gpio_flags = GPIO_ACTIVE_LOW;
 		break;
+	case INT3472_GPIO_TYPE_STROBE:
+		*con_id = "strobe";
+		*gpio_flags = GPIO_ACTIVE_HIGH;
+		break;
 	case INT3472_GPIO_TYPE_CLK_ENABLE:
 		*con_id = "clk-enable";
 		*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 (IR flood LED)
  * 0x0b Power enable
  * 0x0c Clock enable
  * 0x0d Privacy LED
@@ -329,6 +334,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 			err_msg = "Failed to map GPIO pin to sensor\n";
 
 		break;
+	case INT3472_GPIO_TYPE_STROBE:
 	case INT3472_GPIO_TYPE_CLK_ENABLE:
 	case INT3472_GPIO_TYPE_PRIVACY_LED:
 	case INT3472_GPIO_TYPE_POWER_ENABLE:
@@ -348,7 +354,15 @@ 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);
+			ret = skl_int3472_register_led(int3472, gpio,
+						       INT3472_LED_TYPE_PRIVACY);
+			if (ret)
+				err_msg = "Failed to register LED\n";
+
+			break;
+		case INT3472_GPIO_TYPE_STROBE:
+			ret = skl_int3472_register_led(int3472, gpio,
+						       INT3472_LED_TYPE_STROBE);
 			if (ret)
 				err_msg = "Failed to register LED\n";
 
@@ -422,7 +436,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 8b26929..de475db 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -9,53 +9,79 @@
 static int int3472_led_set(struct led_classdev *led_cdev,
 			   enum led_brightness brightness)
 {
-	struct int3472_discrete_device *int3472 =
-		container_of(led_cdev, struct int3472_discrete_device, led.classdev);
+	struct int3472_led *led =
+		container_of(led_cdev, struct int3472_led, classdev);
 
-	gpiod_set_value_cansleep(int3472->led.gpio, brightness);
+	gpiod_set_value_cansleep(led->gpio, brightness);
 	return 0;
 }
 
 int skl_int3472_register_led(struct int3472_discrete_device *int3472,
-			     struct gpio_desc *gpio)
+			     struct gpio_desc *gpio,
+			     enum int3472_led_type type)
 {
+	struct int3472_led *led;
+	const char *name_suffix;
+	bool add_lookup;
 	char *p;
 	int ret;
 
-	if (int3472->led.classdev.dev)
-		return -EBUSY;
+	if (int3472->n_leds >= INT3472_MAX_LEDS)
+		return -ENOSPC;
 
-	int3472->led.gpio = gpio;
+	switch (type) {
+	case INT3472_LED_TYPE_PRIVACY:
+		name_suffix = "privacy";
+		add_lookup = true;
+		break;
+	case INT3472_LED_TYPE_STROBE:
+		name_suffix = "strobe";
+		add_lookup = false;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	led = &int3472->leds[int3472->n_leds];
+	led->gpio = gpio;
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
-	snprintf(int3472->led.name, sizeof(int3472->led.name),
-		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
-	p = strchr(int3472->led.name, ':');
+	snprintf(led->name, sizeof(led->name),
+		 "%s::%s_led", acpi_dev_name(int3472->sensor), name_suffix);
+	p = strchr(led->name, ':');
 	if (p)
 		*p = '_';
 
-	int3472->led.classdev.name = int3472->led.name;
-	int3472->led.classdev.max_brightness = 1;
-	int3472->led.classdev.brightness_set_blocking = int3472_led_set;
+	led->classdev.name = led->name;
+	led->classdev.max_brightness = 1;
+	led->classdev.brightness_set_blocking = int3472_led_set;
 
-	ret = led_classdev_register(int3472->dev, &int3472->led.classdev);
+	ret = led_classdev_register(int3472->dev, &led->classdev);
 	if (ret)
 		return ret;
 
-	int3472->led.lookup.provider = int3472->led.name;
-	int3472->led.lookup.dev_id = int3472->sensor_name;
-	int3472->led.lookup.con_id = "privacy";
-	led_add_lookup(&int3472->led.lookup);
+	if (add_lookup) {
+		led->lookup.provider = led->name;
+		led->lookup.dev_id = int3472->sensor_name;
+		led->lookup.con_id = name_suffix;
+		led_add_lookup(&led->lookup);
+		led->has_lookup = true;
+	}
 
+	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)
 {
-	if (IS_ERR_OR_NULL(int3472->led.classdev.dev))
-		return;
+	unsigned int i;
+
+	for (i = 0; i < int3472->n_leds; i++) {
+		struct int3472_led *led = &int3472->leds[i];
 
-	led_remove_lookup(&int3472->led.lookup);
-	led_classdev_unregister(&int3472->led.classdev);
-	gpiod_put(int3472->led.gpio);
+		if (led->has_lookup)
+			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 7af6731..9893711 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
@@ -49,6 +50,7 @@
 #define GPIO_REGULATOR_OFF_ON_DELAY				(2 * USEC_PER_MSEC)
 
 #define INT3472_LED_MAX_NAME_LEN				32
+#define INT3472_MAX_LEDS					2
 
 #define CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET			86
 
@@ -68,6 +70,11 @@
 #define to_int3472_device(clk)					\
 	container_of(clk, struct int3472_discrete_device, clock)
 
+enum int3472_led_type {
+	INT3472_LED_TYPE_PRIVACY,
+	INT3472_LED_TYPE_STROBE,
+};
+
 struct acpi_device;
 struct dmi_system_id;
 struct i2c_client;
@@ -126,7 +133,9 @@ struct int3472_discrete_device {
 		struct led_lookup_data lookup;
 		char name[INT3472_LED_MAX_NAME_LEN];
 		struct gpio_desc *gpio;
-	} led;
+		bool has_lookup;
+	} leds[INT3472_MAX_LEDS];
+	unsigned int n_leds;
 
 	struct int3472_discrete_quirks quirks;
 
@@ -160,7 +169,9 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
 				   const char *second_sensor);
 void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
 
-int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
-void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
+int skl_int3472_register_led(struct int3472_discrete_device *int3472,
+			     struct gpio_desc *gpio,
+			     enum int3472_led_type type);
+void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472);
 
 #endif
-- 
2.47.3

Re: [PATCH v3 2/2] platform/x86: int3472: Add support for GPIO type 0x02 (strobe LED)

[Ilpo Järvinen]

On Wed, 25 Mar 2026, Marco Nenciarini wrote:

> Some ACPI INT3472 devices include a GPIO with DSM type 0x02, used for
> IR flood (strobe) illumination. This GPIO type was previously
> unhandled, resulting in the following warning during probe:
> 
>   int3472-discrete INT3472:00: GPIO type 0x02 is not currently
>   supported
> 
> Add INT3472_GPIO_TYPE_STROBE (0x02) handling that registers the GPIO
> as an LED class device via skl_int3472_register_led(). An enum
> int3472_led_type parameter controls both the LED name suffix and
> whether a lookup is registered for the sensor driver. Unlike the
> privacy LED, the strobe LED is not consumed by the sensor driver, so
> no LED lookup is registered.
> 
> To support devices that have both a privacy and a strobe LED, the
> single struct int3472_led member is replaced with an array and the
> container_of in the brightness callback now references the int3472_led
> struct directly.
> 
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
> ---
> Changes in v3:
> - Use enum int3472_led_type to control name and lookup behavior
> - Convert single LED member to array for multi-LED support
> - Add default: return -EINVAL for safety
> 
>  drivers/platform/x86/intel/int3472/discrete.c | 18 ++++-
>  drivers/platform/x86/intel/int3472/led.c      | 74 +++++++++++++------
>  include/linux/platform_data/x86/int3472.h     | 17 ++++-
>  3 files changed, 80 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index cb24763..0cffd13 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -207,6 +207,10 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>  		*con_id = "powerdown";
>  		*gpio_flags = GPIO_ACTIVE_LOW;
>  		break;
> +	case INT3472_GPIO_TYPE_STROBE:
> +		*con_id = "strobe";
> +		*gpio_flags = GPIO_ACTIVE_HIGH;
> +		break;
>  	case INT3472_GPIO_TYPE_CLK_ENABLE:
>  		*con_id = "clk-enable";
>  		*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 (IR flood LED)
>   * 0x0b Power enable
>   * 0x0c Clock enable
>   * 0x0d Privacy LED
> @@ -329,6 +334,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>  			err_msg = "Failed to map GPIO pin to sensor\n";
>  
>  		break;
> +	case INT3472_GPIO_TYPE_STROBE:
>  	case INT3472_GPIO_TYPE_CLK_ENABLE:
>  	case INT3472_GPIO_TYPE_PRIVACY_LED:
>  	case INT3472_GPIO_TYPE_POWER_ENABLE:
> @@ -348,7 +354,15 @@ 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);
> +			ret = skl_int3472_register_led(int3472, gpio,
> +						       INT3472_LED_TYPE_PRIVACY);
> +			if (ret)
> +				err_msg = "Failed to register LED\n";
> +
> +			break;
> +		case INT3472_GPIO_TYPE_STROBE:
> +			ret = skl_int3472_register_led(int3472, gpio,
> +						       INT3472_LED_TYPE_STROBE);
>  			if (ret)
>  				err_msg = "Failed to register LED\n";
>  
> @@ -422,7 +436,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 8b26929..de475db 100644
> --- a/drivers/platform/x86/intel/int3472/led.c
> +++ b/drivers/platform/x86/intel/int3472/led.c
> @@ -9,53 +9,79 @@
>  static int int3472_led_set(struct led_classdev *led_cdev,
>  			   enum led_brightness brightness)
>  {
> -	struct int3472_discrete_device *int3472 =
> -		container_of(led_cdev, struct int3472_discrete_device, led.classdev);
> +	struct int3472_led *led =
> +		container_of(led_cdev, struct int3472_led, classdev);
>  
> -	gpiod_set_value_cansleep(int3472->led.gpio, brightness);
> +	gpiod_set_value_cansleep(led->gpio, brightness);
>  	return 0;
>  }
>  
>  int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> -			     struct gpio_desc *gpio)
> +			     struct gpio_desc *gpio,
> +			     enum int3472_led_type type)
>  {
> +	struct int3472_led *led;
> +	const char *name_suffix;
> +	bool add_lookup;
>  	char *p;
>  	int ret;
>  
> -	if (int3472->led.classdev.dev)
> -		return -EBUSY;
> +	if (int3472->n_leds >= INT3472_MAX_LEDS)
> +		return -ENOSPC;
>  
> -	int3472->led.gpio = gpio;
> +	switch (type) {
> +	case INT3472_LED_TYPE_PRIVACY:
> +		name_suffix = "privacy";
> +		add_lookup = true;
> +		break;
> +	case INT3472_LED_TYPE_STROBE:
> +		name_suffix = "strobe";
> +		add_lookup = false;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	led = &int3472->leds[int3472->n_leds];
> +	led->gpio = gpio;
>  
>  	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
> -	snprintf(int3472->led.name, sizeof(int3472->led.name),
> -		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
> -	p = strchr(int3472->led.name, ':');
> +	snprintf(led->name, sizeof(led->name),
> +		 "%s::%s_led", acpi_dev_name(int3472->sensor), name_suffix);
> +	p = strchr(led->name, ':');
>  	if (p)
>  		*p = '_';
>  
> -	int3472->led.classdev.name = int3472->led.name;
> -	int3472->led.classdev.max_brightness = 1;
> -	int3472->led.classdev.brightness_set_blocking = int3472_led_set;
> +	led->classdev.name = led->name;
> +	led->classdev.max_brightness = 1;
> +	led->classdev.brightness_set_blocking = int3472_led_set;
>  
> -	ret = led_classdev_register(int3472->dev, &int3472->led.classdev);
> +	ret = led_classdev_register(int3472->dev, &led->classdev);
>  	if (ret)
>  		return ret;

Could all these int3472->led. => led-> conversion be made in a separate 
patch as well? I think it would reduce churn making the actual changes in 
this patch stand out better.

>  
> -	int3472->led.lookup.provider = int3472->led.name;
> -	int3472->led.lookup.dev_id = int3472->sensor_name;
> -	int3472->led.lookup.con_id = "privacy";
> -	led_add_lookup(&int3472->led.lookup);
> +	if (add_lookup) {
> +		led->lookup.provider = led->name;
> +		led->lookup.dev_id = int3472->sensor_name;
> +		led->lookup.con_id = name_suffix;
> +		led_add_lookup(&led->lookup);
> +		led->has_lookup = true;
> +	}>  
> +	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)
>  {
> -	if (IS_ERR_OR_NULL(int3472->led.classdev.dev))
> -		return;
> +	unsigned int i;
> +
> +	for (i = 0; i < int3472->n_leds; i++) {
> +		struct int3472_led *led = &int3472->leds[i];
>  
> -	led_remove_lookup(&int3472->led.lookup);
> -	led_classdev_unregister(&int3472->led.classdev);
> -	gpiod_put(int3472->led.gpio);
> +		if (led->has_lookup)
> +			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 7af6731..9893711 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
> @@ -49,6 +50,7 @@
>  #define GPIO_REGULATOR_OFF_ON_DELAY				(2 * USEC_PER_MSEC)
>  
>  #define INT3472_LED_MAX_NAME_LEN				32
> +#define INT3472_MAX_LEDS					2
>  
>  #define CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET			86
>  
> @@ -68,6 +70,11 @@
>  #define to_int3472_device(clk)					\
>  	container_of(clk, struct int3472_discrete_device, clock)
>  
> +enum int3472_led_type {
> +	INT3472_LED_TYPE_PRIVACY,
> +	INT3472_LED_TYPE_STROBE,
> +};
> +
>  struct acpi_device;
>  struct dmi_system_id;
>  struct i2c_client;
> @@ -126,7 +133,9 @@ struct int3472_discrete_device {
>  		struct led_lookup_data lookup;
>  		char name[INT3472_LED_MAX_NAME_LEN];
>  		struct gpio_desc *gpio;
> -	} led;
> +		bool has_lookup;
> +	} leds[INT3472_MAX_LEDS];
> +	unsigned int n_leds;
>  
>  	struct int3472_discrete_quirks quirks;
>  
> @@ -160,7 +169,9 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
>  				   const char *second_sensor);
>  void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
>  
> -int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
> -void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
> +int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> +			     struct gpio_desc *gpio,
> +			     enum int3472_led_type type);
> +void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472);
>  
>  #endif
> 

-- 
 i.

Re: [PATCH v3 2/2] platform/x86: int3472: Add support for GPIO type 0x02 (strobe LED)

[Andy Shevchenko]

On Thu, Mar 26, 2026 at 12:46:08PM +0200, Ilpo Järvinen wrote:
> On Wed, 25 Mar 2026, Marco Nenciarini wrote:
> 
> > Some ACPI INT3472 devices include a GPIO with DSM type 0x02, used for
> > IR flood (strobe) illumination. This GPIO type was previously
> > unhandled, resulting in the following warning during probe:
> > 
> >   int3472-discrete INT3472:00: GPIO type 0x02 is not currently
> >   supported
> > 
> > Add INT3472_GPIO_TYPE_STROBE (0x02) handling that registers the GPIO
> > as an LED class device via skl_int3472_register_led(). An enum
> > int3472_led_type parameter controls both the LED name suffix and
> > whether a lookup is registered for the sensor driver. Unlike the
> > privacy LED, the strobe LED is not consumed by the sensor driver, so
> > no LED lookup is registered.
> > 
> > To support devices that have both a privacy and a strobe LED, the
> > single struct int3472_led member is replaced with an array and the
> > container_of in the brightness callback now references the int3472_led
> > struct directly.

> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Move Cc...

> > Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
> > ---

...to be here as it reduces noise in the commit message.

> > Changes in v3:
> > - Use enum int3472_led_type to control name and lookup behavior
> > - Convert single LED member to array for multi-LED support
> > - Add default: return -EINVAL for safety

...

> > -	ret = led_classdev_register(int3472->dev, &int3472->led.classdev);
> > +	ret = led_classdev_register(int3472->dev, &led->classdev);
> >  	if (ret)
> >  		return ret;
> 
> Could all these int3472->led. => led-> conversion be made in a separate 
> patch as well? I think it would reduce churn making the actual changes in 
> this patch stand out better.

+1.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] platform/x86: int3472: Add support for GPIO type 0x02 (strobe LED)

[Andy Shevchenko]

On Wed, Mar 25, 2026 at 11:38:23PM +0100, Marco Nenciarini wrote:
> Some ACPI INT3472 devices include a GPIO with DSM type 0x02, used for
> IR flood (strobe) illumination. This GPIO type was previously
> unhandled, resulting in the following warning during probe:
> 
>   int3472-discrete INT3472:00: GPIO type 0x02 is not currently
>   supported
> 
> Add INT3472_GPIO_TYPE_STROBE (0x02) handling that registers the GPIO
> as an LED class device via skl_int3472_register_led(). An enum
> int3472_led_type parameter controls both the LED name suffix and
> whether a lookup is registered for the sensor driver. Unlike the
> privacy LED, the strobe LED is not consumed by the sensor driver, so
> no LED lookup is registered.
> 
> To support devices that have both a privacy and a strobe LED, the
> single struct int3472_led member is replaced with an array and the
> container_of in the brightness callback now references the int3472_led
> struct directly.

...

>  		case INT3472_GPIO_TYPE_PRIVACY_LED:
> -			ret = skl_int3472_register_led(int3472, gpio);
> +			ret = skl_int3472_register_led(int3472, gpio,
> +						       INT3472_LED_TYPE_PRIVACY);

Split this conversion to yet another separate change. At the end of the day
you should have something like this:

Patch 1: rename pled --> led
Patch 2: Use local led variable instead of dereferencing (as Ilpo asked for)
Patch 3: introduce led type and update API to use it
Patch 4: add IR flood support

> +			if (ret)
> +				err_msg = "Failed to register LED\n";

Move this now to the callee which may add a name/con_id of the led to the error
message.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] platform/x86: int3472: Add support for GPIO type 0x02 (strobe LED)

[Andy Shevchenko]

On Thu, Mar 26, 2026 at 12:55:43PM +0200, Andy Shevchenko wrote:
> On Wed, Mar 25, 2026 at 11:38:23PM +0100, Marco Nenciarini wrote:

...

> >  		case INT3472_GPIO_TYPE_PRIVACY_LED:
> > -			ret = skl_int3472_register_led(int3472, gpio);
> > +			ret = skl_int3472_register_led(int3472, gpio,
> > +						       INT3472_LED_TYPE_PRIVACY);
> 
> Split this conversion to yet another separate change. At the end of the day
> you should have something like this:
> 
> Patch 1: rename pled --> led
> Patch 2: Use local led variable instead of dereferencing (as Ilpo asked for)
> Patch 3: introduce led type and update API to use it
> Patch 4: add IR flood support
> 
> > +			if (ret)
> > +				err_msg = "Failed to register LED\n";
> 
> Move this now to the callee which may add a name/con_id of the led to the error
> message.

With the above proposed split to the patches, this comment should be addressed
already in patch 3.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] platform/x86: int3472: Add support for GPIO type 0x02 (strobe LED)

[Andy Shevchenko]

On Thu, Mar 26, 2026 at 12:57:49PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 26, 2026 at 12:55:43PM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 25, 2026 at 11:38:23PM +0100, Marco Nenciarini wrote:

...

> > >  		case INT3472_GPIO_TYPE_PRIVACY_LED:
> > > -			ret = skl_int3472_register_led(int3472, gpio);
> > > +			ret = skl_int3472_register_led(int3472, gpio,
> > > +						       INT3472_LED_TYPE_PRIVACY);
> > 
> > Split this conversion to yet another separate change. At the end of the day
> > you should have something like this:
> > 
> > Patch 1: rename pled --> led
> > Patch 2: Use local led variable instead of dereferencing (as Ilpo asked for)
> > Patch 3: introduce led type and update API to use it
> > Patch 4: add IR flood support
> > 
> > > +			if (ret)
> > > +				err_msg = "Failed to register LED\n";
> > 
> > Move this now to the callee which may add a name/con_id of the led to the error
> > message.
> 
> With the above proposed split to the patches, this comment should be addressed
> already in patch 3.

Hmm... Looking into the current implementation, it's harder achieve than say.
So, then just

				err_msg = "Failed to register privacy LED\n";

...

Also consider using a separate data structure to list the map between type and
name (con_id)

static const char * const led_con_ids[] = {
	[INT3472_LED_TYPE_PRIVACY] = "privacy",
};

it will allow to get rid of switch-case.

-- 
With Best Regards,
Andy Shevchenko

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

[Marco Nenciarini]

Some ACPI INT3472 devices include a GPIO with DSM type 0x02, used for
IR flood (strobe) illumination, which is currently unhandled.

This series incrementally refactors the LED code and then adds strobe
support:

  1/4: Rename pled to led (pure rename, no functional change)
  2/4: Introduce local variable for LED struct access
  3/4: Add LED type enum, lookup tables, and multi-LED support
  4/4: Add strobe LED type (GPIO 0x02)

Changes in v4:
- Split into 4 patches as requested by Ilpo Jarvinen and Andy
  Shevchenko (v3 was 2 patches)
- Patch 2/4: new patch extracting local variable refactor
- Patch 3/4: new patch for enum/multi-LED infrastructure
- Replaced switch-case with lookup tables for LED names and con_ids
  as suggested by Andy Shevchenko

Changes in v3:
- Split into 2 patches as requested by Andy Shevchenko
- Patch 1/2: pure rename of pled to led (no functional change)
- Patch 2/2: add strobe type with enum, convert single LED member to
  array to support devices with both privacy and strobe GPIOs

Marco Nenciarini (4):
  platform/x86: int3472: Rename pled to led in LED registration code
  platform/x86: int3472: Use local variable for LED struct access
  platform/x86: int3472: Introduce LED type enum and multi-LED support
  platform/x86: int3472: Add support for GPIO type 0x02 (strobe LED)

 drivers/platform/x86/intel/int3472/discrete.c | 20 ++++-
 drivers/platform/x86/intel/int3472/led.c      | 76 ++++++++++++-------
 include/linux/platform_data/x86/int3472.h     | 19 ++++-
 3 files changed, 82 insertions(+), 33 deletions(-)

-- 
2.47.3

[PATCH v4 1/4] platform/x86: int3472: Rename pled to led in LED registration code

[Marco Nenciarini]

Rename the privacy LED functions, struct type and member from
pled/int3472_pled/register_pled to led/int3472_led/register_led to
prepare for supporting additional LED types (e.g. strobe/IR flood).

No functional change.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/discrete.c |  4 +-
 drivers/platform/x86/intel/int3472/led.c      | 42 +++++++++----------
 include/linux/platform_data/x86/int3472.h     |  8 ++--
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 1505fc3..cb24763 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -348,7 +348,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 
 			break;
 		case INT3472_GPIO_TYPE_PRIVACY_LED:
-			ret = skl_int3472_register_pled(int3472, gpio);
+			ret = skl_int3472_register_led(int3472, gpio);
 			if (ret)
 				err_msg = "Failed to register LED\n";
 
@@ -422,7 +422,7 @@ void int3472_discrete_cleanup(struct int3472_discrete_device *int3472)
 	gpiod_remove_lookup_table(&int3472->gpios);
 
 	skl_int3472_unregister_clock(int3472);
-	skl_int3472_unregister_pled(int3472);
+	skl_int3472_unregister_led(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 b1d84b9..ccd51f2 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -6,55 +6,55 @@
 #include <linux/leds.h>
 #include <linux/platform_data/x86/int3472.h>
 
-static int int3472_pled_set(struct led_classdev *led_cdev,
+static int int3472_led_set(struct led_classdev *led_cdev,
 				     enum led_brightness brightness)
 {
 	struct int3472_discrete_device *int3472 =
-		container_of(led_cdev, struct int3472_discrete_device, pled.classdev);
+		container_of(led_cdev, struct int3472_discrete_device, led.classdev);
 
-	gpiod_set_value_cansleep(int3472->pled.gpio, brightness);
+	gpiod_set_value_cansleep(int3472->led.gpio, brightness);
 	return 0;
 }
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
+int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
 {
 	char *p;
 	int ret;
 
-	if (int3472->pled.classdev.dev)
+	if (int3472->led.classdev.dev)
 		return -EBUSY;
 
-	int3472->pled.gpio = gpio;
+	int3472->led.gpio = gpio;
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
-	snprintf(int3472->pled.name, sizeof(int3472->pled.name),
+	snprintf(int3472->led.name, sizeof(int3472->led.name),
 		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
-	p = strchr(int3472->pled.name, ':');
+	p = strchr(int3472->led.name, ':');
 	if (p)
 		*p = '_';
 
-	int3472->pled.classdev.name = int3472->pled.name;
-	int3472->pled.classdev.max_brightness = 1;
-	int3472->pled.classdev.brightness_set_blocking = int3472_pled_set;
+	int3472->led.classdev.name = int3472->led.name;
+	int3472->led.classdev.max_brightness = 1;
+	int3472->led.classdev.brightness_set_blocking = int3472_led_set;
 
-	ret = led_classdev_register(int3472->dev, &int3472->pled.classdev);
+	ret = led_classdev_register(int3472->dev, &int3472->led.classdev);
 	if (ret)
 		return ret;
 
-	int3472->pled.lookup.provider = int3472->pled.name;
-	int3472->pled.lookup.dev_id = int3472->sensor_name;
-	int3472->pled.lookup.con_id = "privacy";
-	led_add_lookup(&int3472->pled.lookup);
+	int3472->led.lookup.provider = int3472->led.name;
+	int3472->led.lookup.dev_id = int3472->sensor_name;
+	int3472->led.lookup.con_id = "privacy";
+	led_add_lookup(&int3472->led.lookup);
 
 	return 0;
 }
 
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472)
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472)
 {
-	if (IS_ERR_OR_NULL(int3472->pled.classdev.dev))
+	if (IS_ERR_OR_NULL(int3472->led.classdev.dev))
 		return;
 
-	led_remove_lookup(&int3472->pled.lookup);
-	led_classdev_unregister(&int3472->pled.classdev);
-	gpiod_put(int3472->pled.gpio);
+	led_remove_lookup(&int3472->led.lookup);
+	led_classdev_unregister(&int3472->led.classdev);
+	gpiod_put(int3472->led.gpio);
 }
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index b1b8375..7af6731 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -121,12 +121,12 @@ struct int3472_discrete_device {
 		u8 imgclk_index;
 	} clock;
 
-	struct int3472_pled {
+	struct int3472_led {
 		struct led_classdev classdev;
 		struct led_lookup_data lookup;
 		char name[INT3472_LED_MAX_NAME_LEN];
 		struct gpio_desc *gpio;
-	} pled;
+	} led;
 
 	struct int3472_discrete_quirks quirks;
 
@@ -160,7 +160,7 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
 				   const char *second_sensor);
 void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472);
+int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
 
 #endif
-- 
2.47.3

Re: [PATCH v4 1/4] platform/x86: int3472: Rename pled to led in LED registration code

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 10:07:50AM +0100, Marco Nenciarini wrote:
> Rename the privacy LED functions, struct type and member from
> pled/int3472_pled/register_pled to led/int3472_led/register_led to
> prepare for supporting additional LED types (e.g. strobe/IR flood).
> 
> No functional change.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/4] platform/x86: int3472: Rename pled to led in LED registration code

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 12:08:20PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 27, 2026 at 10:07:50AM +0100, Marco Nenciarini wrote:
> > Rename the privacy LED functions, struct type and member from
> > pled/int3472_pled/register_pled to led/int3472_led/register_led to
> > prepare for supporting additional LED types (e.g. strobe/IR flood).
> > 
> > No functional change.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Given the comment in the patch 2, please do not apply this (yet).

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 2/4] platform/x86: int3472: Use local variable for LED struct access

[Marco Nenciarini]

Introduce a local struct int3472_led pointer in the LED registration
and unregistration functions to avoid repeatedly dereferencing
int3472->led. In the brightness callback, use container_of to get the
int3472_led struct directly instead of going through
int3472_discrete_device.

No functional change.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/led.c | 43 +++++++++++++-----------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index ccd51f2..efbb02c 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -7,54 +7,57 @@
 #include <linux/platform_data/x86/int3472.h>
 
 static int int3472_led_set(struct led_classdev *led_cdev,
-				     enum led_brightness brightness)
+			   enum led_brightness brightness)
 {
-	struct int3472_discrete_device *int3472 =
-		container_of(led_cdev, struct int3472_discrete_device, led.classdev);
+	struct int3472_led *led =
+		container_of(led_cdev, struct int3472_led, classdev);
 
-	gpiod_set_value_cansleep(int3472->led.gpio, brightness);
+	gpiod_set_value_cansleep(led->gpio, brightness);
 	return 0;
 }
 
 int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
 {
+	struct int3472_led *led = &int3472->led;
 	char *p;
 	int ret;
 
-	if (int3472->led.classdev.dev)
+	if (led->classdev.dev)
 		return -EBUSY;
 
-	int3472->led.gpio = gpio;
+	led->gpio = gpio;
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
-	snprintf(int3472->led.name, sizeof(int3472->led.name),
+	snprintf(led->name, sizeof(led->name),
 		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
-	p = strchr(int3472->led.name, ':');
+	p = strchr(led->name, ':');
 	if (p)
 		*p = '_';
 
-	int3472->led.classdev.name = int3472->led.name;
-	int3472->led.classdev.max_brightness = 1;
-	int3472->led.classdev.brightness_set_blocking = int3472_led_set;
+	led->classdev.name = led->name;
+	led->classdev.max_brightness = 1;
+	led->classdev.brightness_set_blocking = int3472_led_set;
 
-	ret = led_classdev_register(int3472->dev, &int3472->led.classdev);
+	ret = led_classdev_register(int3472->dev, &led->classdev);
 	if (ret)
 		return ret;
 
-	int3472->led.lookup.provider = int3472->led.name;
-	int3472->led.lookup.dev_id = int3472->sensor_name;
-	int3472->led.lookup.con_id = "privacy";
-	led_add_lookup(&int3472->led.lookup);
+	led->lookup.provider = led->name;
+	led->lookup.dev_id = int3472->sensor_name;
+	led->lookup.con_id = "privacy";
+	led_add_lookup(&led->lookup);
 
 	return 0;
 }
 
 void skl_int3472_unregister_led(struct int3472_discrete_device *int3472)
 {
-	if (IS_ERR_OR_NULL(int3472->led.classdev.dev))
+	struct int3472_led *led = &int3472->led;
+
+	if (IS_ERR_OR_NULL(led->classdev.dev))
 		return;
 
-	led_remove_lookup(&int3472->led.lookup);
-	led_classdev_unregister(&int3472->led.classdev);
-	gpiod_put(int3472->led.gpio);
+	led_remove_lookup(&led->lookup);
+	led_classdev_unregister(&led->classdev);
+	gpiod_put(led->gpio);
 }
-- 
2.47.3

Re: [PATCH v4 2/4] platform/x86: int3472: Use local variable for LED struct access

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 10:07:51AM +0100, Marco Nenciarini wrote:
> Introduce a local struct int3472_led pointer in the LED registration
> and unregistration functions to avoid repeatedly dereferencing
> int3472->led. In the brightness callback, use container_of to get the

container_of()

> int3472_led struct directly instead of going through
> int3472_discrete_device.
> 
> No functional change.

...

>  static int int3472_led_set(struct led_classdev *led_cdev,
> -				     enum led_brightness brightness)
> +			   enum led_brightness brightness)
>  {
> -	struct int3472_discrete_device *int3472 =
> -		container_of(led_cdev, struct int3472_discrete_device, led.classdev);
> +	struct int3472_led *led =
> +		container_of(led_cdev, struct int3472_led, classdev);

Now it can be a single line

	struct int3472_led *led = container_of(led_cdev, struct int3472_led, classdev);

(the original code span to 85 characters, this line is 87, which is fine
 I believe).


> -	gpiod_set_value_cansleep(int3472->led.gpio, brightness);
> +	gpiod_set_value_cansleep(led->gpio, brightness);
>  	return 0;
>  }

...

>  int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
>  {
> +	struct int3472_led *led = &int3472->led;

Okay, make this patch going first, before renaming, so you will have the local
variable already be converted:

	struct int3472_led *led = &int3472->pled;

This will reduce a lot of churn in both patches.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 3/4] platform/x86: int3472: Introduce LED type enum and multi-LED support

[Marco Nenciarini]

Add an enum int3472_led_type to parameterize LED registration, and
lookup tables to derive the LED name suffix and con_id from the type.
This replaces the hardcoded "privacy" name and prepares for additional
LED types.

Convert the single struct int3472_led member to an array with a
counter to support devices with multiple LEDs. The LED lookup is now
conditionally registered based on the con_id lookup table entry, and
a has_lookup flag tracks whether to remove it during cleanup.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/discrete.c |  7 +--
 drivers/platform/x86/intel/int3472/led.c      | 51 +++++++++++++------
 include/linux/platform_data/x86/int3472.h     | 15 ++++--
 3 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index cb24763..2c554a0 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -348,9 +348,10 @@ 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);
+			ret = skl_int3472_register_led(int3472, gpio,
+						       INT3472_LED_TYPE_PRIVACY);
 			if (ret)
-				err_msg = "Failed to register LED\n";
+				err_msg = "Failed to register privacy LED\n";
 
 			break;
 		case INT3472_GPIO_TYPE_POWER_ENABLE:
@@ -422,7 +423,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 efbb02c..33b30f3 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -6,6 +6,14 @@
 #include <linux/leds.h>
 #include <linux/platform_data/x86/int3472.h>
 
+static const char * const int3472_led_names[] = {
+	[INT3472_LED_TYPE_PRIVACY] = "privacy",
+};
+
+static const char * const int3472_led_con_ids[] = {
+	[INT3472_LED_TYPE_PRIVACY] = "privacy",
+};
+
 static int int3472_led_set(struct led_classdev *led_cdev,
 			   enum led_brightness brightness)
 {
@@ -16,20 +24,25 @@ static int int3472_led_set(struct led_classdev *led_cdev,
 	return 0;
 }
 
-int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
+int skl_int3472_register_led(struct int3472_discrete_device *int3472,
+			     struct gpio_desc *gpio,
+			     enum int3472_led_type type)
 {
-	struct int3472_led *led = &int3472->led;
+	const char *name_suffix = int3472_led_names[type];
+	const char *con_id = int3472_led_con_ids[type];
+	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;
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
 	snprintf(led->name, sizeof(led->name),
-		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
+		 "%s::%s_led", acpi_dev_name(int3472->sensor), name_suffix);
 	p = strchr(led->name, ':');
 	if (p)
 		*p = '_';
@@ -42,22 +55,28 @@ int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpi
 	if (ret)
 		return ret;
 
-	led->lookup.provider = led->name;
-	led->lookup.dev_id = int3472->sensor_name;
-	led->lookup.con_id = "privacy";
-	led_add_lookup(&led->lookup);
+	if (con_id) {
+		led->lookup.provider = led->name;
+		led->lookup.dev_id = int3472->sensor_name;
+		led->lookup.con_id = con_id;
+		led_add_lookup(&led->lookup);
+		led->has_lookup = true;
+	}
 
+	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;
+	unsigned int i;
 
-	if (IS_ERR_OR_NULL(led->classdev.dev))
-		return;
+	for (i = 0; i < int3472->n_leds; i++) {
+		struct int3472_led *led = &int3472->leds[i];
 
-	led_remove_lookup(&led->lookup);
-	led_classdev_unregister(&led->classdev);
-	gpiod_put(led->gpio);
+		if (led->has_lookup)
+			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 7af6731..b6b5b36 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -49,6 +49,7 @@
 #define GPIO_REGULATOR_OFF_ON_DELAY				(2 * USEC_PER_MSEC)
 
 #define INT3472_LED_MAX_NAME_LEN				32
+#define INT3472_MAX_LEDS					2
 
 #define CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET			86
 
@@ -68,6 +69,10 @@
 #define to_int3472_device(clk)					\
 	container_of(clk, struct int3472_discrete_device, clock)
 
+enum int3472_led_type {
+	INT3472_LED_TYPE_PRIVACY,
+};
+
 struct acpi_device;
 struct dmi_system_id;
 struct i2c_client;
@@ -126,7 +131,9 @@ struct int3472_discrete_device {
 		struct led_lookup_data lookup;
 		char name[INT3472_LED_MAX_NAME_LEN];
 		struct gpio_desc *gpio;
-	} led;
+		bool has_lookup;
+	} leds[INT3472_MAX_LEDS];
+	unsigned int n_leds;
 
 	struct int3472_discrete_quirks quirks;
 
@@ -160,7 +167,9 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
 				   const char *second_sensor);
 void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
 
-int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
-void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
+int skl_int3472_register_led(struct int3472_discrete_device *int3472,
+			     struct gpio_desc *gpio,
+			     enum int3472_led_type type);
+void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472);
 
 #endif
-- 
2.47.3

Re: [PATCH v4 3/4] platform/x86: int3472: Introduce LED type enum and multi-LED support

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 10:07:52AM +0100, Marco Nenciarini wrote:
> Add an enum int3472_led_type to parameterize LED registration, and
> lookup tables to derive the LED name suffix and con_id from the type.
> This replaces the hardcoded "privacy" name and prepares for additional
> LED types.
> 
> Convert the single struct int3472_led member to an array with a
> counter to support devices with multiple LEDs. The LED lookup is now
> conditionally registered based on the con_id lookup table entry, and
> a has_lookup flag tracks whether to remove it during cleanup.

...

> +static const char * const int3472_led_names[] = {
> +	[INT3472_LED_TYPE_PRIVACY] = "privacy",
> +};
> +
> +static const char * const int3472_led_con_ids[] = {
> +	[INT3472_LED_TYPE_PRIVACY] = "privacy",
> +};

But why? Do we expect a deviation in these?

...

> +	if (con_id) {

Too early, we have only up to one led for now and we know what is that.

> +		led->lookup.provider = led->name;
> +		led->lookup.dev_id = int3472->sensor_name;
> +		led->lookup.con_id = con_id;
> +		led_add_lookup(&led->lookup);
> +		led->has_lookup = true;
> +	}

...

> -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;
> +	unsigned int i;
>  
> -	if (IS_ERR_OR_NULL(led->classdev.dev))
> -		return;
> +	for (i = 0; i < int3472->n_leds; i++) {


	for (unsigned int i = 0; i < int3472->n_leds; i++) {

> +		struct int3472_led *led = &int3472->leds[i];
>  
> -	led_remove_lookup(&led->lookup);
> -	led_classdev_unregister(&led->classdev);
> -	gpiod_put(led->gpio);

> +		if (led->has_lookup)

Too early, we know we don't have this right now.

> +			led_remove_lookup(&led->lookup);
> +		led_classdev_unregister(&led->classdev);
> +		gpiod_put(led->gpio);
> +	}
>  }

...

>  #define INT3472_LED_MAX_NAME_LEN				32
> +#define INT3472_MAX_LEDS					2

Too early. It's 1 for now.

-- 
With Best Regards,
Andy Shevchenko

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

[Marco Nenciarini]

Some ACPI INT3472 devices include a GPIO with DSM type 0x02, used for
IR flood (strobe) illumination. This GPIO type was previously
unhandled, resulting in the following warning during probe:

  int3472-discrete INT3472:00: GPIO type 0x02 unknown; the sensor
  may not work

Add INT3472_GPIO_TYPE_STROBE (0x02) handling that registers the GPIO
as an LED class device via skl_int3472_register_led(). Unlike the
privacy LED, the strobe LED is not consumed by the sensor driver, so
no LED lookup is registered.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/discrete.c | 13 +++++++++++++
 drivers/platform/x86/intel/int3472/led.c      |  2 ++
 include/linux/platform_data/x86/int3472.h     |  2 ++
 3 files changed, 17 insertions(+)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 2c554a0..03f0b49 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -207,6 +207,10 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 		*con_id = "powerdown";
 		*gpio_flags = GPIO_ACTIVE_LOW;
 		break;
+	case INT3472_GPIO_TYPE_STROBE:
+		*con_id = "strobe";
+		*gpio_flags = GPIO_ACTIVE_HIGH;
+		break;
 	case INT3472_GPIO_TYPE_CLK_ENABLE:
 		*con_id = "clk-enable";
 		*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 (IR flood LED)
  * 0x0b Power enable
  * 0x0c Clock enable
  * 0x0d Privacy LED
@@ -329,6 +334,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 			err_msg = "Failed to map GPIO pin to sensor\n";
 
 		break;
+	case INT3472_GPIO_TYPE_STROBE:
 	case INT3472_GPIO_TYPE_CLK_ENABLE:
 	case INT3472_GPIO_TYPE_PRIVACY_LED:
 	case INT3472_GPIO_TYPE_POWER_ENABLE:
@@ -353,6 +359,13 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 			if (ret)
 				err_msg = "Failed to register privacy LED\n";
 
+			break;
+		case INT3472_GPIO_TYPE_STROBE:
+			ret = skl_int3472_register_led(int3472, gpio,
+						       INT3472_LED_TYPE_STROBE);
+			if (ret)
+				err_msg = "Failed to register strobe LED\n";
+
 			break;
 		case INT3472_GPIO_TYPE_POWER_ENABLE:
 			second_sensor = int3472->quirks.avdd_second_sensor;
diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index 33b30f3..e6faba4 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -8,10 +8,12 @@
 
 static const char * const int3472_led_names[] = {
 	[INT3472_LED_TYPE_PRIVACY] = "privacy",
+	[INT3472_LED_TYPE_STROBE]  = "strobe",
 };
 
 static const char * const int3472_led_con_ids[] = {
 	[INT3472_LED_TYPE_PRIVACY] = "privacy",
+	[INT3472_LED_TYPE_STROBE]  = NULL,
 };
 
 static int int3472_led_set(struct led_classdev *led_cdev,
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index b6b5b36..9893711 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
@@ -71,6 +72,7 @@
 
 enum int3472_led_type {
 	INT3472_LED_TYPE_PRIVACY,
+	INT3472_LED_TYPE_STROBE,
 };
 
 struct acpi_device;
-- 
2.47.3

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

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 10:07:53AM +0100, Marco Nenciarini wrote:
> Some ACPI INT3472 devices include a GPIO with DSM type 0x02, used for
> IR flood (strobe) illumination. This GPIO type was previously
> unhandled, resulting in the following warning during probe:
> 
>   int3472-discrete INT3472:00: GPIO type 0x02 unknown; the sensor
>   may not work
> 
> Add INT3472_GPIO_TYPE_STROBE (0x02) handling that registers the GPIO
> as an LED class device via skl_int3472_register_led(). Unlike the
> privacy LED, the strobe LED is not consumed by the sensor driver, so
> no LED lookup is registered.

...

>  	[INT3472_LED_TYPE_PRIVACY] = "privacy",
> +	[INT3472_LED_TYPE_STROBE]  = "strobe",

This was renamed between the versions. Did I miss explanation "why"?

>  };
>  
>  static const char * const int3472_led_con_ids[] = {
>  	[INT3472_LED_TYPE_PRIVACY] = "privacy",
> +	[INT3472_LED_TYPE_STROBE]  = NULL,
>  };

Not sure if this is better than passing a boolean parameter to the function.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 10:07:49AM +0100, Marco Nenciarini wrote:
> Some ACPI INT3472 devices include a GPIO with DSM type 0x02, used for
> IR flood (strobe) illumination, which is currently unhandled.
> 
> This series incrementally refactors the LED code and then adds strobe
> support:
> 
>   1/4: Rename pled to led (pure rename, no functional change)
>   2/4: Introduce local variable for LED struct access
>   3/4: Add LED type enum, lookup tables, and multi-LED support
>   4/4: Add strobe LED type (GPIO 0x02)

Thanks for the update, more or less okay, a bit of technical routine with
reshuffling and rebasing and it should start looking really nice.

> Changes in v4:
> - Split into 4 patches as requested by Ilpo Jarvinen and Andy
>   Shevchenko (v3 was 2 patches)
> - Patch 2/4: new patch extracting local variable refactor
> - Patch 3/4: new patch for enum/multi-LED infrastructure
> - Replaced switch-case with lookup tables for LED names and con_ids
>   as suggested by Andy Shevchenko

The led was renamed from ir_flood to strobe. No description why.

-- 
With Best Regards,
Andy Shevchenko

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

[Marco Nenciarini]

Add support for INT3472 GPIO type 0x02 which controls an IR strobe LED
used for face authentication on some laptops (e.g. Dell Pro Max 16
Premium). Without this, the kernel logs "GPIO type 0x02 unknown; the
sensor may not work" and IR sensors paired with a strobe LED cannot
function.

The series first refactors the existing LED code (patches 1-3), then
adds the new strobe LED type with multi-LED support (patch 4).

Changes in v5:
- Swapped patches 1 and 2: local variable refactor first (on pled
  names), then rename, per Andy's suggestion to reduce churn
- Dropped enum and lookup tables in favor of simple function parameters
  (string name + bool has_lookup), per Andy's review
- Multi-LED infrastructure (array, counter, has_lookup) only introduced
  in patch 4 where it is actually needed
- container_of targets struct int3472_led directly (single line)
- Used C99 for-loop variable declaration in unregister loop
- Renamed LED from ir_flood (v1-v2) to strobe (v3+) to match the GPIO
  type name in ACPI _DSM tables and align with common camera
  terminology for IR illuminators. This rename happened in v3 but was
  not documented in the changelog until now.

Marco Nenciarini (4):
  platform/x86: int3472: Use local variable for LED struct access
  platform/x86: int3472: Rename pled to led in LED registration code
  platform/x86: int3472: Parameterize LED name in registration
  platform/x86: int3472: Add support for GPIO type 0x02 (strobe)

 drivers/platform/x86/intel/int3472/discrete.c | 16 ++++-
 drivers/platform/x86/intel/int3472/led.c      | 61 +++++++++++--------
 include/linux/platform_data/x86/int3472.h     | 14 +++--
 3 files changed, 59 insertions(+), 32 deletions(-)

-- 
2.47.3

[PATCH v5 1/4] platform/x86: int3472: Use local variable for LED struct access

[Marco Nenciarini]

Introduce a local struct int3472_pled pointer in the LED registration
and unregistration functions to avoid repeatedly dereferencing
int3472->pled. In the brightness callback, use container_of() to get
the int3472_pled struct directly instead of going through
int3472_discrete_device.

No functional change.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/led.c | 42 +++++++++++++-----------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index b1d84b9..4c52f99 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -7,54 +7,56 @@
 #include <linux/platform_data/x86/int3472.h>
 
 static int int3472_pled_set(struct led_classdev *led_cdev,
-				     enum led_brightness brightness)
+			    enum led_brightness brightness)
 {
-	struct int3472_discrete_device *int3472 =
-		container_of(led_cdev, struct int3472_discrete_device, pled.classdev);
+	struct int3472_pled *pled = container_of(led_cdev, struct int3472_pled, classdev);
 
-	gpiod_set_value_cansleep(int3472->pled.gpio, brightness);
+	gpiod_set_value_cansleep(pled->gpio, brightness);
 	return 0;
 }
 
 int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
 {
+	struct int3472_pled *pled = &int3472->pled;
 	char *p;
 	int ret;
 
-	if (int3472->pled.classdev.dev)
+	if (pled->classdev.dev)
 		return -EBUSY;
 
-	int3472->pled.gpio = gpio;
+	pled->gpio = gpio;
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
-	snprintf(int3472->pled.name, sizeof(int3472->pled.name),
+	snprintf(pled->name, sizeof(pled->name),
 		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
-	p = strchr(int3472->pled.name, ':');
+	p = strchr(pled->name, ':');
 	if (p)
 		*p = '_';
 
-	int3472->pled.classdev.name = int3472->pled.name;
-	int3472->pled.classdev.max_brightness = 1;
-	int3472->pled.classdev.brightness_set_blocking = int3472_pled_set;
+	pled->classdev.name = pled->name;
+	pled->classdev.max_brightness = 1;
+	pled->classdev.brightness_set_blocking = int3472_pled_set;
 
-	ret = led_classdev_register(int3472->dev, &int3472->pled.classdev);
+	ret = led_classdev_register(int3472->dev, &pled->classdev);
 	if (ret)
 		return ret;
 
-	int3472->pled.lookup.provider = int3472->pled.name;
-	int3472->pled.lookup.dev_id = int3472->sensor_name;
-	int3472->pled.lookup.con_id = "privacy";
-	led_add_lookup(&int3472->pled.lookup);
+	pled->lookup.provider = pled->name;
+	pled->lookup.dev_id = int3472->sensor_name;
+	pled->lookup.con_id = "privacy";
+	led_add_lookup(&pled->lookup);
 
 	return 0;
 }
 
 void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472)
 {
-	if (IS_ERR_OR_NULL(int3472->pled.classdev.dev))
+	struct int3472_pled *pled = &int3472->pled;
+
+	if (IS_ERR_OR_NULL(pled->classdev.dev))
 		return;
 
-	led_remove_lookup(&int3472->pled.lookup);
-	led_classdev_unregister(&int3472->pled.classdev);
-	gpiod_put(int3472->pled.gpio);
+	led_remove_lookup(&pled->lookup);
+	led_classdev_unregister(&pled->classdev);
+	gpiod_put(pled->gpio);
 }
-- 
2.47.3

Re: [PATCH v5 1/4] platform/x86: int3472: Use local variable for LED struct access

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 07:10:28PM +0100, Marco Nenciarini wrote:
> Introduce a local struct int3472_pled pointer in the LED registration
> and unregistration functions to avoid repeatedly dereferencing
> int3472->pled. In the brightness callback, use container_of() to get
> the int3472_pled struct directly instead of going through
> int3472_discrete_device.
> 
> No functional change.

...

>  static int int3472_pled_set(struct led_classdev *led_cdev,
> -				     enum led_brightness brightness)
> +			    enum led_brightness brightness)
>  {
> -	struct int3472_discrete_device *int3472 =
> -		container_of(led_cdev, struct int3472_discrete_device, pled.classdev);
> +	struct int3472_pled *pled = container_of(led_cdev, struct int3472_pled, classdev);

The idea was to call it led from the beginning, currently I don't see how this
reduces a churn I mentioned.

	struct int3472_pled *led = container_of(led_cdev, struct int3472_pled, classdev);

> -	gpiod_set_value_cansleep(int3472->pled.gpio, brightness);
> +	gpiod_set_value_cansleep(pled->gpio, brightness);

	gpiod_set_value_cansleep(led->gpio, brightness);

>  	return 0;

In the next patch it will be only a single line changed (data type on both sides).

>  }

...

>  int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)

Same here.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 2/4] platform/x86: int3472: Rename pled to led in LED registration code

[Marco Nenciarini]

Rename the privacy LED functions, struct type and member from
pled/int3472_pled/register_pled to led/int3472_led/register_led to
prepare for supporting additional LED types (e.g. strobe/IR flood).

No functional change.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/discrete.c |  4 +-
 drivers/platform/x86/intel/int3472/led.c      | 48 +++++++++----------
 include/linux/platform_data/x86/int3472.h     |  8 ++--
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 1505fc3..cb24763 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -348,7 +348,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 
 			break;
 		case INT3472_GPIO_TYPE_PRIVACY_LED:
-			ret = skl_int3472_register_pled(int3472, gpio);
+			ret = skl_int3472_register_led(int3472, gpio);
 			if (ret)
 				err_msg = "Failed to register LED\n";
 
@@ -422,7 +422,7 @@ void int3472_discrete_cleanup(struct int3472_discrete_device *int3472)
 	gpiod_remove_lookup_table(&int3472->gpios);
 
 	skl_int3472_unregister_clock(int3472);
-	skl_int3472_unregister_pled(int3472);
+	skl_int3472_unregister_led(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 4c52f99..8e92e8e 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -6,57 +6,57 @@
 #include <linux/leds.h>
 #include <linux/platform_data/x86/int3472.h>
 
-static int int3472_pled_set(struct led_classdev *led_cdev,
-			    enum led_brightness brightness)
+static int int3472_led_set(struct led_classdev *led_cdev,
+			   enum led_brightness brightness)
 {
-	struct int3472_pled *pled = container_of(led_cdev, struct int3472_pled, classdev);
+	struct int3472_led *led = container_of(led_cdev, struct int3472_led, classdev);
 
-	gpiod_set_value_cansleep(pled->gpio, brightness);
+	gpiod_set_value_cansleep(led->gpio, brightness);
 	return 0;
 }
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
+int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
 {
-	struct int3472_pled *pled = &int3472->pled;
+	struct int3472_led *led = &int3472->led;
 	char *p;
 	int ret;
 
-	if (pled->classdev.dev)
+	if (led->classdev.dev)
 		return -EBUSY;
 
-	pled->gpio = gpio;
+	led->gpio = gpio;
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
-	snprintf(pled->name, sizeof(pled->name),
+	snprintf(led->name, sizeof(led->name),
 		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
-	p = strchr(pled->name, ':');
+	p = strchr(led->name, ':');
 	if (p)
 		*p = '_';
 
-	pled->classdev.name = pled->name;
-	pled->classdev.max_brightness = 1;
-	pled->classdev.brightness_set_blocking = int3472_pled_set;
+	led->classdev.name = led->name;
+	led->classdev.max_brightness = 1;
+	led->classdev.brightness_set_blocking = int3472_led_set;
 
-	ret = led_classdev_register(int3472->dev, &pled->classdev);
+	ret = led_classdev_register(int3472->dev, &led->classdev);
 	if (ret)
 		return ret;
 
-	pled->lookup.provider = pled->name;
-	pled->lookup.dev_id = int3472->sensor_name;
-	pled->lookup.con_id = "privacy";
-	led_add_lookup(&pled->lookup);
+	led->lookup.provider = led->name;
+	led->lookup.dev_id = int3472->sensor_name;
+	led->lookup.con_id = "privacy";
+	led_add_lookup(&led->lookup);
 
 	return 0;
 }
 
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472)
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472)
 {
-	struct int3472_pled *pled = &int3472->pled;
+	struct int3472_led *led = &int3472->led;
 
-	if (IS_ERR_OR_NULL(pled->classdev.dev))
+	if (IS_ERR_OR_NULL(led->classdev.dev))
 		return;
 
-	led_remove_lookup(&pled->lookup);
-	led_classdev_unregister(&pled->classdev);
-	gpiod_put(pled->gpio);
+	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 b1b8375..7af6731 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -121,12 +121,12 @@ struct int3472_discrete_device {
 		u8 imgclk_index;
 	} clock;
 
-	struct int3472_pled {
+	struct int3472_led {
 		struct led_classdev classdev;
 		struct led_lookup_data lookup;
 		char name[INT3472_LED_MAX_NAME_LEN];
 		struct gpio_desc *gpio;
-	} pled;
+	} led;
 
 	struct int3472_discrete_quirks quirks;
 
@@ -160,7 +160,7 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
 				   const char *second_sensor);
 void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472);
+int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
 
 #endif
-- 
2.47.3

[PATCH v5 3/4] platform/x86: int3472: Parameterize LED name in registration

[Marco Nenciarini]

Add a name parameter to skl_int3472_register_led() to allow callers
to specify the LED name suffix instead of hardcoding "privacy". This
prepares for registering additional LED types with different names.

No functional change.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/discrete.c | 2 +-
 drivers/platform/x86/intel/int3472/led.c      | 5 +++--
 include/linux/platform_data/x86/int3472.h     | 3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index cb24763..57c3b2c 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -348,7 +348,7 @@ 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);
+			ret = skl_int3472_register_led(int3472, gpio, "privacy");
 			if (ret)
 				err_msg = "Failed to register LED\n";
 
diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index 8e92e8e..8222550 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -15,7 +15,8 @@ static int int3472_led_set(struct led_classdev *led_cdev,
 	return 0;
 }
 
-int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
+int skl_int3472_register_led(struct int3472_discrete_device *int3472,
+			     struct gpio_desc *gpio, const char *name)
 {
 	struct int3472_led *led = &int3472->led;
 	char *p;
@@ -28,7 +29,7 @@ int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpi
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
 	snprintf(led->name, sizeof(led->name),
-		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
+		 "%s::%s_led", acpi_dev_name(int3472->sensor), name);
 	p = strchr(led->name, ':');
 	if (p)
 		*p = '_';
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index 7af6731..3a077f4 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -160,7 +160,8 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
 				   const char *second_sensor);
 void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
 
-int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
+int skl_int3472_register_led(struct int3472_discrete_device *int3472,
+			     struct gpio_desc *gpio, const char *name);
 void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
 
 #endif
-- 
2.47.3

Re: [PATCH v5 3/4] platform/x86: int3472: Parameterize LED name in registration

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 07:10:30PM +0100, Marco Nenciarini wrote:
> Add a name parameter to skl_int3472_register_led() to allow callers
> to specify the LED name suffix instead of hardcoding "privacy". This
> prepares for registering additional LED types with different names.
> 
> No functional change.

...

> -int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
> +int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> +			     struct gpio_desc *gpio, const char *name)

I believe it's better to call it con_id...

>  	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
>  	snprintf(led->name, sizeof(led->name),
> -		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
> +		 "%s::%s_led", acpi_dev_name(int3472->sensor), name);
>  	p = strchr(led->name, ':');
>  	if (p)
>  		*p = '_';

...and replace the actual con_id assignment as well.

-- 
With Best Regards,
Andy Shevchenko

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

[Marco Nenciarini]

Add support for GPIO type 0x02, which controls an IR strobe 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 strobe LED cannot function.

The strobe 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.

This GPIO type was previously referred to as "ir_flood" in early
versions of this patch series. It has been renamed to "strobe" to match
the GPIO type name used in ACPI _DSM tables and align with common
camera terminology for IR illuminators.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/discrete.c | 16 +++++++-
 drivers/platform/x86/intel/int3472/led.c      | 38 +++++++++++--------
 include/linux/platform_data/x86/int3472.h     | 11 ++++--
 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..4c03845 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 = "strobe";
+		*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, "strobe", 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 8222550..77f61a4 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -16,15 +16,17 @@ 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 *name)
+			     struct gpio_desc *gpio, const char *name,
+			     bool has_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;
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
@@ -42,22 +44,26 @@ 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 = "privacy";
-	led_add_lookup(&led->lookup);
+	if (has_lookup) {
+		led->lookup.provider = led->name;
+		led->lookup.dev_id = int3472->sensor_name;
+		led->lookup.con_id = name;
+		led_add_lookup(&led->lookup);
+		led->has_lookup = true;
+	}
 
+	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 (led->has_lookup)
+			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 3a077f4..540e3f2 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,12 +128,14 @@ struct int3472_discrete_device {
 		struct led_lookup_data lookup;
 		char name[INT3472_LED_MAX_NAME_LEN];
 		struct gpio_desc *gpio;
-	} led;
+		bool has_lookup;
+	} leds[INT3472_MAX_LEDS];
 
 	struct int3472_discrete_quirks quirks;
 
 	unsigned int ngpios; /* how many GPIOs have we seen */
 	unsigned int n_sensor_gpios; /* how many have we mapped to sensor */
+	unsigned int n_leds; /* how many LEDs have we registered */
 	unsigned int n_regulator_gpios; /* how many have we mapped to a regulator */
 	struct gpiod_lookup_table gpios;
 };
@@ -161,7 +165,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 *name);
-void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
+			     struct gpio_desc *gpio, const char *name,
+			     bool has_lookup);
+void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472);
 
 #endif
-- 
2.47.3

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

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 07:10:31PM +0100, Marco Nenciarini wrote:
> Add support for GPIO type 0x02, which controls an IR strobe 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 strobe LED cannot function.
> 
> The strobe 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.

> This GPIO type was previously referred to as "ir_flood" in early
> versions of this patch series. It has been renamed to "strobe" to match
> the GPIO type name used in ACPI _DSM tables and align with common
> camera terminology for IR illuminators.

This paragraph should not be in the commit message, but...

> Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
> ---

...rather somewhere here (after '---' line).

> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> struct int3472_discrete_device {

>  		struct led_lookup_data lookup;
>  		char name[INT3472_LED_MAX_NAME_LEN];
>  		struct gpio_desc *gpio;
> -	} led;
> +		bool has_lookup;

Do we need it here?

> +	} leds[INT3472_MAX_LEDS];

Can't we simply check this by list_empty() in the removal stage?

*Yes, for that we always need to initialise the list pointers at adding stage.

With that being done, I would rename the parameter to add_lookup.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 07:10:27PM +0100, Marco Nenciarini wrote:
> Add support for INT3472 GPIO type 0x02 which controls an IR strobe LED
> used for face authentication on some laptops (e.g. Dell Pro Max 16
> Premium). Without this, the kernel logs "GPIO type 0x02 unknown; the
> sensor may not work" and IR sensors paired with a strobe LED cannot
> function.
> 
> The series first refactors the existing LED code (patches 1-3), then
> adds the new strobe LED type with multi-LED support (patch 4).

Thanks for the update! I have given mostly cosmetic change requests, but
the main one is to rename local variable in the first patch pled --> led.

-- 
With Best Regards,
Andy Shevchenko

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

[johannes.goede]

Hi Marco,

On 27-Mar-26 19:10, Marco Nenciarini wrote:
> Add support for INT3472 GPIO type 0x02 which controls an IR strobe LED
> used for face authentication on some laptops (e.g. Dell Pro Max 16
> Premium). Without this, the kernel logs "GPIO type 0x02 unknown; the
> sensor may not work" and IR sensors paired with a strobe LED cannot
> function.
> 
> The series first refactors the existing LED code (patches 1-3), then
> adds the new strobe LED type with multi-LED support (patch 4).
> 
> Changes in v5:
> - Swapped patches 1 and 2: local variable refactor first (on pled
>   names), then rename, per Andy's suggestion to reduce churn
> - Dropped enum and lookup tables in favor of simple function parameters
>   (string name + bool has_lookup), per Andy's review
> - Multi-LED infrastructure (array, counter, has_lookup) only introduced
>   in patch 4 where it is actually needed
> - container_of targets struct int3472_led directly (single line)
> - Used C99 for-loop variable declaration in unregister loop
> - Renamed LED from ir_flood (v1-v2) to strobe (v3+) to match the GPIO
>   type name in ACPI _DSM tables and align with common camera
>   terminology for IR illuminators. This rename happened in v3 but was
>   not documented in the changelog until now.

Thank you for your work on this.

Besides Andy's comment I've no comments on the code itself.

But this does expose a new userspace API in the form of
a e.g. OVTIxxxx:00::strobe_led LED class device.

We really need to get some input from the V4L2 maintainers
here on if this is a good idea, before merging this series.

Sakari the discrete (GPIO based) INT3472 power-sequencing
sensor companion device for the *IR/monochrome* sensor on various
ipu6/ipu7 lists a GPIO with type '2' which according to
previous patches from Intel translates to "strobe". Strobe
suggests it triggers a flash, but in reality it seems to
be an on/off GPIO for an IR flood LED to IR illuminate the
scene when using the IR sensor.

Since this is just a LED on/off signal exporting it as
a plain GPIO based LED class device seems to make sense.

For now this patch-series really does nothing other then
making the LED show up under /sys/class/leds . I think
eventually we will want the v4l2-core to do something
with this, like with the privacy LED. 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 ?

Regardless of the exact way we want to deal with this
on the V4L2 side an important question for now is if
it is ok to export this as a OVTIxxxx:00::strobe_led.

I believe it should be, but we really need your input here.

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.

Regards,

Hans






> 
> Marco Nenciarini (4):
>   platform/x86: int3472: Use local variable for LED struct access
>   platform/x86: int3472: Rename pled to led in LED registration code
>   platform/x86: int3472: Parameterize LED name in registration
>   platform/x86: int3472: Add support for GPIO type 0x02 (strobe)
> 
>  drivers/platform/x86/intel/int3472/discrete.c | 16 ++++-
>  drivers/platform/x86/intel/int3472/led.c      | 61 +++++++++++--------
>  include/linux/platform_data/x86/int3472.h     | 14 +++--
>  3 files changed, 59 insertions(+), 32 deletions(-)
> 

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

[Marco Nenciarini]

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?

> 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.

Regards,
Marco

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

[johannes.goede]

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.

>> 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.

Regards,

Hans

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

[Sakari Ailus]

Hi Hans, Marco,

On Mon, Mar 30, 2026 at 05:12:21PM +0200, johannes.goede@oss.qualcomm.com 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.

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? :-)

-- 
Regards,

Sakari Ailus

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

[Marco Nenciarini]

Hi Sakari,

Thank you for looking at this.

On Mon, Mar 30, 2026 at 11:21:32PM +0300, Sakari Ailus wrote:
> 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.

The LED name includes the sensor's ACPI device name (e.g.
OVTI9734:00::ir_flood_led), so userspace can match by naming
convention. For proper integration, the privacy LED already has a
led_get() lookup keyed on the sensor's ACPI name, and future work
could add a similar lookup for the IR flood LED once there is a
consumer driver.

> Alternatively we could use a boolean control for this, but I think I'd
> avoid adding that now and rely on LED API instead.

Agreed, LED API keeps things simple for now and leaves room for V4L2
integration later.

> Are there use cases for this LED, apart from Windows Hello? :-)

Face authentication is the primary one. On Linux, projects like Howdy
can use it. More broadly, any application that needs the IR sensor
benefits since without this patch the kernel refuses to register the
GPIO and the sensor cannot function at all.

Since everyone agrees on "ir_flood_led" for the name and the LED API
approach, I will prepare v6 with that rename plus Andy's cosmetic
fixes.

Regards,
Marco

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

[johannes.goede]

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@oss.qualcomm.com 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.

Regards,

Hans

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@oss.qualcomm.com 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@oss.qualcomm.com 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

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

[johannes.goede]

Hi,

On 31-Mar-26 23:28, Sakari Ailus wrote:
> Hi Hans,
> 
> On Tue, Mar 31, 2026 at 12:15:55PM +0200, johannes.goede@oss.qualcomm.com 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@oss.qualcomm.com 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

To be clear my suggestion is to treat it the same as we
currently treat privacy-LEDs (auto-on while streaming) but
have a separate code-path, or at least a separate LED name
("ir_flood" vs "privacy") to allow different behavior in
the future.

> and the LED will effectively
> be powered whenever the camera is streaming,

Ack.

> we can't drop that interaction
> in the future as doing so would break existing users.

Ack.

> I'm not sure if there would ever be a need though.

Ack.

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

I think we can postpone adding a ctrl to if a use-case needing
different behavior ever shows up (which is unlikely?) .

This ctrl can then be a menu ctrl with auto/on/off values
defaulting to auto and auto preserving the on-while-streaking
behavior, so not breaking existing users.

TL;DR: my proposal is:

1. Name the new LED classdev ir_flood[_led] including
   adding an "ir_flood" LED lookup with the sensor as
   consuming device of the LED.

2. Make v4l2-core turn the ir_flood on automatically while
   streaming, just like it does for a "privacy" LED currently
   (mostly re-using the existing privacy LED code).

3. If in the future userspace needs more fine-grained control
   at a ir_flood V4L2 menu control on the sensor with
   auto/on/off values, defaulting to auto which preserves
   the behavior from 2.

Regards,

Hans

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

[Marco Nenciarini]

On 01-Apr-26 15:38, johannes.goede@oss.qualcomm.com wrote:
> TL;DR: my proposal is:
> 
> 1. Name the new LED classdev ir_flood[_led] including
>    adding an "ir_flood" LED lookup with the sensor as
>    consuming device of the LED.
> 
> 2. Make v4l2-core turn the ir_flood on automatically while
>    streaming, just like it does for a "privacy" LED currently
>    (mostly re-using the existing privacy LED code).
> 
> 3. If in the future userspace needs more fine-grained control
>    at a ir_flood V4L2 menu control on the sensor with
>    auto/on/off values, defaulting to auto which preserves
>    the behavior from 2.

This plan sounds good to me.

I have a v8 ready that addresses your feedback on patch 4/4: the LED
lookup is now always registered (for ir_flood too), removing the type
parameter and the conditional switch-case. Shall I go ahead and send it?

Marco

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

[johannes.goede]

Hi,

On 1-Apr-26 19:13, Marco Nenciarini wrote:
> On 01-Apr-26 15:38, johannes.goede@oss.qualcomm.com wrote:
>> TL;DR: my proposal is:
>>
>> 1. Name the new LED classdev ir_flood[_led] including
>>    adding an "ir_flood" LED lookup with the sensor as
>>    consuming device of the LED.
>>
>> 2. Make v4l2-core turn the ir_flood on automatically while
>>    streaming, just like it does for a "privacy" LED currently
>>    (mostly re-using the existing privacy LED code).
>>
>> 3. If in the future userspace needs more fine-grained control
>>    at a ir_flood V4L2 menu control on the sensor with
>>    auto/on/off values, defaulting to auto which preserves
>>    the behavior from 2.
> 
> This plan sounds good to me.
> 
> I have a v8 ready that addresses your feedback on patch 4/4: the LED
> lookup is now always registered (for ir_flood too), removing the type
> parameter and the conditional switch-case. Shall I go ahead and send it?

Since Andy indicated he is ok with that approach, yes please
submit v8 and then this should be ready for merging.

Regards,

Hans

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

[Marco Nenciarini]

Add support for INT3472 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, the kernel logs "GPIO type 0x02 unknown; the
sensor may not work" and IR sensors paired with a flood LED cannot
function.

The series first refactors the existing LED code (patches 1-3), then
adds the new IR flood LED type with multi-LED support (patch 4).

Changes in v6:
- Renamed LED from "strobe" back to "ir_flood" to describe what the
  hardware actually does, per Hans de Goede's review. The internal
  define INT3472_GPIO_TYPE_STROBE is kept to match the ACPI _DSM name.
- Used local `led` variable from patch 1 (was `pled`, now `led`
  throughout), per Andy's suggestion to reduce churn
- Renamed function parameter from `name` to `con_id` in patch 3
- Replaced `has_lookup` bool with INIT_LIST_HEAD/list_empty check and
  renamed parameter to `add_lookup` in patch 4, per Andy's review
- Added #include <linux/list.h> in patch 4

Marco Nenciarini (4):
  platform/x86: int3472: Use local variable for LED struct access
  platform/x86: int3472: Rename pled to led in LED registration code
  platform/x86: int3472: Parameterize LED con_id in registration
  platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED)

 drivers/platform/x86/intel/int3472/discrete.c | 16 ++++-
 drivers/platform/x86/intel/int3472/led.c      | 62 +++++++++++--------
 include/linux/platform_data/x86/int3472.h     | 13 ++--
 3 files changed, 59 insertions(+), 32 deletions(-)

-- 
2.47.3

[PATCH v6 1/4] platform/x86: int3472: Use local variable for LED struct access

[Marco Nenciarini]

Introduce a local struct int3472_pled pointer in the LED registration,
unregistration, and brightness callback functions to avoid repeatedly
dereferencing int3472->pled. In the brightness callback, use
container_of() to get the int3472_pled struct directly instead of
going through int3472_discrete_device.

No functional change.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/led.c | 42 +++++++++++++-----------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index b1d84b9..86df81a 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -7,54 +7,56 @@
 #include <linux/platform_data/x86/int3472.h>
 
 static int int3472_pled_set(struct led_classdev *led_cdev,
-				     enum led_brightness brightness)
+			    enum led_brightness brightness)
 {
-	struct int3472_discrete_device *int3472 =
-		container_of(led_cdev, struct int3472_discrete_device, pled.classdev);
+	struct int3472_pled *led = container_of(led_cdev, struct int3472_pled, classdev);
 
-	gpiod_set_value_cansleep(int3472->pled.gpio, brightness);
+	gpiod_set_value_cansleep(led->gpio, brightness);
 	return 0;
 }
 
 int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
 {
+	struct int3472_pled *led = &int3472->pled;
 	char *p;
 	int ret;
 
-	if (int3472->pled.classdev.dev)
+	if (led->classdev.dev)
 		return -EBUSY;
 
-	int3472->pled.gpio = gpio;
+	led->gpio = gpio;
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
-	snprintf(int3472->pled.name, sizeof(int3472->pled.name),
+	snprintf(led->name, sizeof(led->name),
 		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
-	p = strchr(int3472->pled.name, ':');
+	p = strchr(led->name, ':');
 	if (p)
 		*p = '_';
 
-	int3472->pled.classdev.name = int3472->pled.name;
-	int3472->pled.classdev.max_brightness = 1;
-	int3472->pled.classdev.brightness_set_blocking = int3472_pled_set;
+	led->classdev.name = led->name;
+	led->classdev.max_brightness = 1;
+	led->classdev.brightness_set_blocking = int3472_pled_set;
 
-	ret = led_classdev_register(int3472->dev, &int3472->pled.classdev);
+	ret = led_classdev_register(int3472->dev, &led->classdev);
 	if (ret)
 		return ret;
 
-	int3472->pled.lookup.provider = int3472->pled.name;
-	int3472->pled.lookup.dev_id = int3472->sensor_name;
-	int3472->pled.lookup.con_id = "privacy";
-	led_add_lookup(&int3472->pled.lookup);
+	led->lookup.provider = led->name;
+	led->lookup.dev_id = int3472->sensor_name;
+	led->lookup.con_id = "privacy";
+	led_add_lookup(&led->lookup);
 
 	return 0;
 }
 
 void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472)
 {
-	if (IS_ERR_OR_NULL(int3472->pled.classdev.dev))
+	struct int3472_pled *led = &int3472->pled;
+
+	if (IS_ERR_OR_NULL(led->classdev.dev))
 		return;
 
-	led_remove_lookup(&int3472->pled.lookup);
-	led_classdev_unregister(&int3472->pled.classdev);
-	gpiod_put(int3472->pled.gpio);
+	led_remove_lookup(&led->lookup);
+	led_classdev_unregister(&led->classdev);
+	gpiod_put(led->gpio);
 }
-- 
2.47.3

Re: [PATCH v6 1/4] platform/x86: int3472: Use local variable for LED struct access

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 09:52:01AM +0200, Marco Nenciarini wrote:
> Introduce a local struct int3472_pled pointer in the LED registration,
> unregistration, and brightness callback functions to avoid repeatedly
> dereferencing int3472->pled. In the brightness callback, use
> container_of() to get the int3472_pled struct directly instead of
> going through int3472_discrete_device.
> 
> No functional change.

...

>  static int int3472_pled_set(struct led_classdev *led_cdev,
> -				     enum led_brightness brightness)
> +			    enum led_brightness brightness)

No need to touch it here as you also need (currently missed) update in the next
patch again.

With this being addressed,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

[PATCH v6 2/4] platform/x86: int3472: Rename pled to led in LED registration code

[Marco Nenciarini]

Rename the privacy LED type, struct member, and functions from "pled"
to "led" in preparation for supporting additional LED types beyond
just the privacy LED.

No functional change.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/discrete.c |  4 ++--
 drivers/platform/x86/intel/int3472/led.c      | 14 +++++++-------
 include/linux/platform_data/x86/int3472.h     |  8 ++++----
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 1505fc3..cb24763 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -348,7 +348,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 
 			break;
 		case INT3472_GPIO_TYPE_PRIVACY_LED:
-			ret = skl_int3472_register_pled(int3472, gpio);
+			ret = skl_int3472_register_led(int3472, gpio);
 			if (ret)
 				err_msg = "Failed to register LED\n";
 
@@ -422,7 +422,7 @@ void int3472_discrete_cleanup(struct int3472_discrete_device *int3472)
 	gpiod_remove_lookup_table(&int3472->gpios);
 
 	skl_int3472_unregister_clock(int3472);
-	skl_int3472_unregister_pled(int3472);
+	skl_int3472_unregister_led(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 86df81a..8bd4903 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -6,18 +6,18 @@
 #include <linux/leds.h>
 #include <linux/platform_data/x86/int3472.h>
 
-static int int3472_pled_set(struct led_classdev *led_cdev,
+static int int3472_led_set(struct led_classdev *led_cdev,
 			    enum led_brightness brightness)
 {
-	struct int3472_pled *led = container_of(led_cdev, struct int3472_pled, classdev);
+	struct int3472_led *led = container_of(led_cdev, struct int3472_led, classdev);
 
 	gpiod_set_value_cansleep(led->gpio, brightness);
 	return 0;
 }
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
+int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
 {
-	struct int3472_pled *led = &int3472->pled;
+	struct int3472_led *led = &int3472->led;
 	char *p;
 	int ret;
 
@@ -35,7 +35,7 @@ int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gp
 
 	led->classdev.name = led->name;
 	led->classdev.max_brightness = 1;
-	led->classdev.brightness_set_blocking = int3472_pled_set;
+	led->classdev.brightness_set_blocking = int3472_led_set;
 
 	ret = led_classdev_register(int3472->dev, &led->classdev);
 	if (ret)
@@ -49,9 +49,9 @@ int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gp
 	return 0;
 }
 
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472)
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472)
 {
-	struct int3472_pled *led = &int3472->pled;
+	struct int3472_led *led = &int3472->led;
 
 	if (IS_ERR_OR_NULL(led->classdev.dev))
 		return;
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index b1b8375..7af6731 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -121,12 +121,12 @@ struct int3472_discrete_device {
 		u8 imgclk_index;
 	} clock;
 
-	struct int3472_pled {
+	struct int3472_led {
 		struct led_classdev classdev;
 		struct led_lookup_data lookup;
 		char name[INT3472_LED_MAX_NAME_LEN];
 		struct gpio_desc *gpio;
-	} pled;
+	} led;
 
 	struct int3472_discrete_quirks quirks;
 
@@ -160,7 +160,7 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
 				   const char *second_sensor);
 void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
 
-int skl_int3472_register_pled(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
-void skl_int3472_unregister_pled(struct int3472_discrete_device *int3472);
+int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
+void skl_int3472_unregister_led(struct int3472_discrete_device *int3472);
 
 #endif
-- 
2.47.3

Re: [PATCH v6 2/4] platform/x86: int3472: Rename pled to led in LED registration code

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 09:52:02AM +0200, Marco Nenciarini wrote:
> Rename the privacy LED type, struct member, and functions from "pled"
> to "led" in preparation for supporting additional LED types beyond
> just the privacy LED.
> 
> No functional change.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> -static int int3472_pled_set(struct led_classdev *led_cdev,
> +static int int3472_led_set(struct led_classdev *led_cdev,
>  			    enum led_brightness brightness)

Second line needs to be fixed as well (indentation).

-- 
With Best Regards,
Andy Shevchenko

[PATCH v6 3/4] platform/x86: int3472: Parameterize LED con_id in registration

[Marco Nenciarini]

Add a con_id parameter to skl_int3472_register_led() to allow callers
to specify both the LED name suffix and lookup con_id instead of
hardcoding "privacy". This prepares for registering additional LED
types with different names.

No functional change.

Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 drivers/platform/x86/intel/int3472/discrete.c | 2 +-
 drivers/platform/x86/intel/int3472/led.c      | 7 ++++---
 include/linux/platform_data/x86/int3472.h     | 3 ++-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index cb24763..57c3b2c 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -348,7 +348,7 @@ 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);
+			ret = skl_int3472_register_led(int3472, gpio, "privacy");
 			if (ret)
 				err_msg = "Failed to register LED\n";
 
diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index 8bd4903..39466d4 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -15,7 +15,8 @@ static int int3472_led_set(struct led_classdev *led_cdev,
 	return 0;
 }
 
-int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio)
+int skl_int3472_register_led(struct int3472_discrete_device *int3472,
+			     struct gpio_desc *gpio, const char *con_id)
 {
 	struct int3472_led *led = &int3472->led;
 	char *p;
@@ -28,7 +29,7 @@ int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpi
 
 	/* Generate the name, replacing the ':' in the ACPI devname with '_' */
 	snprintf(led->name, sizeof(led->name),
-		 "%s::privacy_led", acpi_dev_name(int3472->sensor));
+		 "%s::%s_led", acpi_dev_name(int3472->sensor), con_id);
 	p = strchr(led->name, ':');
 	if (p)
 		*p = '_';
@@ -43,7 +44,7 @@ int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpi
 
 	led->lookup.provider = led->name;
 	led->lookup.dev_id = int3472->sensor_name;
-	led->lookup.con_id = "privacy";
+	led->lookup.con_id = con_id;
 	led_add_lookup(&led->lookup);
 
 	return 0;
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index 7af6731..5213911 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -160,7 +160,8 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
 				   const char *second_sensor);
 void skl_int3472_unregister_regulator(struct int3472_discrete_device *int3472);
 
-int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
+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);
 
 #endif
-- 
2.47.3

Re: [PATCH v6 3/4] platform/x86: int3472: Parameterize LED con_id in registration

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 09:52:03AM +0200, Marco Nenciarini wrote:
> Add a con_id parameter to skl_int3472_register_led() to allow callers
> to specify both the LED name suffix and lookup con_id instead of
> hardcoding "privacy". This prepares for registering additional LED
> types with different names.
> 
> No functional change.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> -int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio);
> +int skl_int3472_register_led(struct int3472_discrete_device *int3472,
> +			     struct gpio_desc *gpio, const char *con_id);

No need to wrap the line, actually with comma it becomes shorter by 1 character.

int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio,
			     const char *con_id);

...

Speaking of this, maybe in the first patch you can simply move the enum to the
same line instead of dragging it here and there.

-- 
With Best Regards,
Andy Shevchenko

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

[Marco Nenciarini]

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.3

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

[Hans de Goede]

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

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

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

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

[Andy Shevchenko]

On Wed, Apr 01, 2026 at 03:36:01PM +0200, Hans de Goede wrote:
> 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:

...

> 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.

TBH, I'm fine with either case. If you think it's better, than it will
even be simpler in the code!

-- 
With Best Regards,
Andy Shevchenko

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 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

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

[Andy Shevchenko]

On Tue, Mar 31, 2026 at 09:52:00AM +0200, Marco Nenciarini wrote:
> Add support for INT3472 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, the kernel logs "GPIO type 0x02 unknown; the
> sensor may not work" and IR sensors paired with a flood LED cannot
> function.
> 
> The series first refactors the existing LED code (patches 1-3), then
> adds the new IR flood LED type with multi-LED support (patch 4).

Almost. A few cosmetic changes and I'm glad to see it's done.

Also please note, when sending a new version make sure it starts a new email
thread (no In-Reply-To should be filled by the cover letter).

-- 
With Best Regards,
Andy Shevchenko