* [PATCH v7 1/4] platform/x86: int3472: Use local variable for LED struct access
2026-03-31 13:44 [PATCH v7 0/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED) Marco Nenciarini
@ 2026-03-31 13:44 ` Marco Nenciarini
2026-04-01 13:40 ` Hans de Goede
2026-03-31 13:44 ` [PATCH v7 2/4] platform/x86: int3472: Rename pled to led in LED registration code Marco Nenciarini
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Marco Nenciarini @ 2026-03-31 13:44 UTC (permalink / raw)
To: Daniel Scally, Sakari Ailus, Ilpo Järvinen
Cc: Andy Shevchenko, Hans de Goede, platform-driver-x86, linux-kernel,
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.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
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, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index b1d84b9..35abad9 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_pled_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);
+ 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v7 1/4] platform/x86: int3472: Use local variable for LED struct access
2026-03-31 13:44 ` [PATCH v7 1/4] platform/x86: int3472: Use local variable for LED struct access Marco Nenciarini
@ 2026-04-01 13:40 ` Hans de Goede
0 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2026-04-01 13:40 UTC (permalink / raw)
To: Marco Nenciarini, Daniel Scally, Sakari Ailus, Ilpo Järvinen
Cc: Andy Shevchenko, platform-driver-x86, linux-kernel
Hi,
On 31-Mar-26 15:44, 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.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Regards,
Hans
> ---
>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> drivers/platform/x86/intel/int3472/led.c | 43 ++++++++++++------------
> 1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
> index b1d84b9..35abad9 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_pled_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);
> + 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);
> }
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v7 2/4] platform/x86: int3472: Rename pled to led in LED registration code
2026-03-31 13:44 [PATCH v7 0/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED) Marco Nenciarini
2026-03-31 13:44 ` [PATCH v7 1/4] platform/x86: int3472: Use local variable for LED struct access Marco Nenciarini
@ 2026-03-31 13:44 ` Marco Nenciarini
2026-04-01 13:42 ` Hans de Goede
2026-03-31 13:44 ` [PATCH v7 3/4] platform/x86: int3472: Parameterize LED con_id in registration Marco Nenciarini
2026-03-31 13:44 ` [PATCH v7 4/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED) Marco Nenciarini
3 siblings, 1 reply; 11+ messages in thread
From: Marco Nenciarini @ 2026-03-31 13:44 UTC (permalink / raw)
To: Daniel Scally, Sakari Ailus, Ilpo Järvinen
Cc: Andy Shevchenko, Hans de Goede, platform-driver-x86, linux-kernel,
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.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
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 35abad9..fe412cb 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -6,17 +6,17 @@
#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 *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;
@@ -34,7 +34,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)
@@ -48,9 +48,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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v7 2/4] platform/x86: int3472: Rename pled to led in LED registration code
2026-03-31 13:44 ` [PATCH v7 2/4] platform/x86: int3472: Rename pled to led in LED registration code Marco Nenciarini
@ 2026-04-01 13:42 ` Hans de Goede
0 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2026-04-01 13:42 UTC (permalink / raw)
To: Marco Nenciarini, Daniel Scally, Sakari Ailus, Ilpo Järvinen
Cc: Andy Shevchenko, platform-driver-x86, linux-kernel
Hi,
On 31-Mar-26 15:44, 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>
> Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Regards,
Hans
> ---
>
> 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 35abad9..fe412cb 100644
> --- a/drivers/platform/x86/intel/int3472/led.c
> +++ b/drivers/platform/x86/intel/int3472/led.c
> @@ -6,17 +6,17 @@
> #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 *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;
>
> @@ -34,7 +34,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)
> @@ -48,9 +48,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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v7 3/4] platform/x86: int3472: Parameterize LED con_id in registration
2026-03-31 13:44 [PATCH v7 0/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED) Marco Nenciarini
2026-03-31 13:44 ` [PATCH v7 1/4] platform/x86: int3472: Use local variable for LED struct access Marco Nenciarini
2026-03-31 13:44 ` [PATCH v7 2/4] platform/x86: int3472: Rename pled to led in LED registration code Marco Nenciarini
@ 2026-03-31 13:44 ` Marco Nenciarini
2026-03-31 18:50 ` Andy Shevchenko
2026-04-01 13:43 ` Hans de Goede
2026-03-31 13:44 ` [PATCH v7 4/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED) Marco Nenciarini
3 siblings, 2 replies; 11+ messages in thread
From: Marco Nenciarini @ 2026-03-31 13:44 UTC (permalink / raw)
To: Daniel Scally, Sakari Ailus, Ilpo Järvinen
Cc: Andy Shevchenko, Hans de Goede, platform-driver-x86, linux-kernel,
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.
While at it, rename the privacy LED's GPIO con_id from "privacy-led"
to "privacy" in int3472_get_con_id_and_polarity() and pass it
directly to skl_int3472_register_led(), reducing churn when adding
new LED types.
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 | 7 ++++---
include/linux/platform_data/x86/int3472.h | 3 ++-
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index cb24763..a45a930 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -212,7 +212,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
*gpio_flags = GPIO_ACTIVE_HIGH;
break;
case INT3472_GPIO_TYPE_PRIVACY_LED:
- *con_id = "privacy-led";
+ *con_id = "privacy";
*gpio_flags = GPIO_ACTIVE_HIGH;
break;
case INT3472_GPIO_TYPE_HOTPLUG_DETECT:
@@ -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, con_id);
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 fe412cb..22d0d6c 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -14,7 +14,8 @@ static int int3472_led_set(struct led_classdev *led_cdev, enum led_brightness br
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;
@@ -27,7 +28,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 = '_';
@@ -42,7 +43,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..3ba0d56 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v7 3/4] platform/x86: int3472: Parameterize LED con_id in registration
2026-03-31 13:44 ` [PATCH v7 3/4] platform/x86: int3472: Parameterize LED con_id in registration Marco Nenciarini
@ 2026-03-31 18:50 ` Andy Shevchenko
2026-04-01 13:43 ` Hans de Goede
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2026-03-31 18:50 UTC (permalink / raw)
To: Marco Nenciarini
Cc: Daniel Scally, Sakari Ailus, Ilpo Järvinen, Hans de Goede,
platform-driver-x86, linux-kernel
On Tue, Mar 31, 2026 at 03:44:42PM +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.
>
> While at it, rename the privacy LED's GPIO con_id from "privacy-led"
> to "privacy" in int3472_get_con_id_and_polarity() and pass it
> directly to skl_int3472_register_led(), reducing churn when adding
> new LED types.
>
> No functional change.
LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 3/4] platform/x86: int3472: Parameterize LED con_id in registration
2026-03-31 13:44 ` [PATCH v7 3/4] platform/x86: int3472: Parameterize LED con_id in registration Marco Nenciarini
2026-03-31 18:50 ` Andy Shevchenko
@ 2026-04-01 13:43 ` Hans de Goede
1 sibling, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2026-04-01 13:43 UTC (permalink / raw)
To: Marco Nenciarini, Daniel Scally, Sakari Ailus, Ilpo Järvinen
Cc: Andy Shevchenko, platform-driver-x86, linux-kernel
Hi,
On 31-Mar-26 15:44, 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.
>
> While at it, rename the privacy LED's GPIO con_id from "privacy-led"
> to "privacy" in int3472_get_con_id_and_polarity() and pass it
> directly to skl_int3472_register_led(), reducing churn when adding
> new LED types.
>
> No functional change.
>
> Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Regards,
Hans
> ---
>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> drivers/platform/x86/intel/int3472/discrete.c | 4 ++--
> drivers/platform/x86/intel/int3472/led.c | 7 ++++---
> include/linux/platform_data/x86/int3472.h | 3 ++-
> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index cb24763..a45a930 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -212,7 +212,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
> *gpio_flags = GPIO_ACTIVE_HIGH;
> break;
> case INT3472_GPIO_TYPE_PRIVACY_LED:
> - *con_id = "privacy-led";
> + *con_id = "privacy";
> *gpio_flags = GPIO_ACTIVE_HIGH;
> break;
> case INT3472_GPIO_TYPE_HOTPLUG_DETECT:
> @@ -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, con_id);
> 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 fe412cb..22d0d6c 100644
> --- a/drivers/platform/x86/intel/int3472/led.c
> +++ b/drivers/platform/x86/intel/int3472/led.c
> @@ -14,7 +14,8 @@ static int int3472_led_set(struct led_classdev *led_cdev, enum led_brightness br
> 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;
> @@ -27,7 +28,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 = '_';
> @@ -42,7 +43,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..3ba0d56 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v7 4/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED)
2026-03-31 13:44 [PATCH v7 0/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED) Marco Nenciarini
` (2 preceding siblings ...)
2026-03-31 13:44 ` [PATCH v7 3/4] platform/x86: int3472: Parameterize LED con_id in registration Marco Nenciarini
@ 2026-03-31 13:44 ` Marco Nenciarini
2026-03-31 18:54 ` Andy Shevchenko
2026-04-01 13:45 ` Hans de Goede
3 siblings, 2 replies; 11+ messages in thread
From: Marco Nenciarini @ 2026-03-31 13:44 UTC (permalink / raw)
To: Daniel Scally, Sakari Ailus, Ilpo Järvinen
Cc: Andy Shevchenko, Hans de Goede, platform-driver-x86, linux-kernel,
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 | 11 ++++-
drivers/platform/x86/intel/int3472/led.c | 44 ++++++++++++-------
include/linux/platform_data/x86/int3472.h | 9 ++--
3 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index a45a930..438b2e3 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";
*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,8 @@ 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, con_id);
+ case INT3472_GPIO_TYPE_STROBE:
+ ret = skl_int3472_register_led(int3472, gpio, con_id, type);
if (ret)
err_msg = "Failed to register LED\n";
@@ -422,7 +429,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 22d0d6c..2888844 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, enum led_brightness brightness)
@@ -15,16 +16,18 @@ static int int3472_led_set(struct led_classdev *led_cdev, enum led_brightness br
}
int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio,
- const char *con_id)
+ const char *con_id, u8 type)
{
- 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),
@@ -41,22 +44,31 @@ 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 = con_id;
- led_add_lookup(&led->lookup);
+ /* Register lookup for LED types that have a consumer driver */
+ switch (type) {
+ case INT3472_GPIO_TYPE_PRIVACY_LED:
+ led->lookup.provider = led->name;
+ led->lookup.dev_id = int3472->sensor_name;
+ led->lookup.con_id = con_id;
+ led_add_lookup(&led->lookup);
+ break;
+ case INT3472_GPIO_TYPE_STROBE:
+ default:
+ break;
+ }
+ 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 3ba0d56..f14032c 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,7 @@ 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);
+ const char *con_id, u8 type);
+void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472);
#endif
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v7 4/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED)
2026-03-31 13:44 ` [PATCH v7 4/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED) Marco Nenciarini
@ 2026-03-31 18:54 ` Andy Shevchenko
2026-04-01 13:45 ` Hans de Goede
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2026-03-31 18:54 UTC (permalink / raw)
To: Marco Nenciarini
Cc: Daniel Scally, Sakari Ailus, Ilpo Järvinen, Hans de Goede,
platform-driver-x86, linux-kernel
On Tue, Mar 31, 2026 at 03:44:43PM +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.
I think this is what we want, thanks!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 4/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED)
2026-03-31 13:44 ` [PATCH v7 4/4] platform/x86: int3472: Add support for GPIO type 0x02 (IR flood LED) Marco Nenciarini
2026-03-31 18:54 ` Andy Shevchenko
@ 2026-04-01 13:45 ` Hans de Goede
1 sibling, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2026-04-01 13:45 UTC (permalink / raw)
To: Marco Nenciarini, Daniel Scally, Sakari Ailus, Ilpo Järvinen
Cc: Andy Shevchenko, platform-driver-x86, linux-kernel
Hi,
On 31-Mar-26 15:44, 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 | 11 ++++-
> drivers/platform/x86/intel/int3472/led.c | 44 ++++++++++++-------
> include/linux/platform_data/x86/int3472.h | 9 ++--
> 3 files changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index a45a930..438b2e3 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";
> *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,8 @@ 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, con_id);
> + case INT3472_GPIO_TYPE_STROBE:
> + ret = skl_int3472_register_led(int3472, gpio, con_id, type);
As mentioned in the v6 discussion I believe we *always* want the lookup,
so we don't need the addition of passing "type" here + and we also don't
need the part of the changes below which deal with having the registering
of the lookup be conditional.
But maybe wait with sending a v8 until the v6 discussion with Sakari
is done.
Regards,
Hans
> if (ret)
> err_msg = "Failed to register LED\n";
>
> @@ -422,7 +429,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 22d0d6c..2888844 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, enum led_brightness brightness)
> @@ -15,16 +16,18 @@ static int int3472_led_set(struct led_classdev *led_cdev, enum led_brightness br
> }
>
> int skl_int3472_register_led(struct int3472_discrete_device *int3472, struct gpio_desc *gpio,
> - const char *con_id)
> + const char *con_id, u8 type)
> {
> - 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),
> @@ -41,22 +44,31 @@ 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 = con_id;
> - led_add_lookup(&led->lookup);
> + /* Register lookup for LED types that have a consumer driver */
> + switch (type) {
> + case INT3472_GPIO_TYPE_PRIVACY_LED:
> + led->lookup.provider = led->name;
> + led->lookup.dev_id = int3472->sensor_name;
> + led->lookup.con_id = con_id;
> + led_add_lookup(&led->lookup);
> + break;
> + case INT3472_GPIO_TYPE_STROBE:
> + default:
> + break;
> + }
>
> + 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 3ba0d56..f14032c 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,7 @@ 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);
> + const char *con_id, u8 type);
> +void skl_int3472_unregister_leds(struct int3472_discrete_device *int3472);
>
> #endif
^ permalink raw reply [flat|nested] 11+ messages in thread