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

Marco Nenciarini posted 4 patches 6 days, 8 hours ago
[PATCH v4 3/4] platform/x86: int3472: Introduce LED type enum and multi-LED support
Posted by Marco Nenciarini 6 days, 8 hours ago
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
Posted by Andy Shevchenko 6 days, 6 hours ago
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