Hi!
> The DualSense player LEDs were so far not adjustable from user-space.
> This patch exposes each LED individually through the LED class. Each
> LED uses the new 'player' function resulting in a name like:
> 'inputX:white:player-1' for the first LED.
>
>
> +struct ps_led_info {
...
> + enum led_brightness (*brightness_get)(struct led_classdev *cdev);
> + void (*brightness_set)(struct led_classdev *cdev, enum led_brightness);
> +};
Do you need these fields? They are constant in the driver...
> +static enum led_brightness dualsense_player_led_get_brightness(struct led_classdev *led)
> +{
> + struct hid_device *hdev = to_hid_device(led->dev->parent);
> + struct dualsense *ds = hid_get_drvdata(hdev);
> +
> + return !!(ds->player_leds_state & BIT(led - ds->player_leds));
> +}
You should not need to implement get if all it does is returning cached state.
> +static void dualsense_player_led_set_brightness(struct led_classdev *led, enum led_brightness value)
> +{
> + struct hid_device *hdev = to_hid_device(led->dev->parent);
> + struct dualsense *ds = hid_get_drvdata(hdev);
> + unsigned long flags;
> + unsigned int led_index;
> +
> + spin_lock_irqsave(&ds->base.lock, flags);
> +
> + led_index = led - ds->player_leds;
> + if (value == LED_OFF)
> + ds->player_leds_state &= ~BIT(led_index);
> + else
> + ds->player_leds_state |= BIT(led_index);
> +
> + ds->update_player_leds = true;
> + spin_unlock_irqrestore(&ds->base.lock, flags);
> +
> + schedule_work(&ds->output_worker);
> +}
LED core can handle scheduling the work for you. You should use it, in similar
way you handle the RGB led.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
