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

[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

[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

[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