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

From: Andy Shevchenko

Date: Fri Mar 27 2026 - 06:37:23 EST


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