[PATCH] HID: playstation: Support DualSense player LED brightness control

[PATCH] HID: playstation: Support DualSense player LED brightness control

[Kateřina Medvědová]

The DualSense's 5 player indicator LEDs are currently only exposed as
plain on/off LED classdevs (max_brightness = 1). The controller
firmware also supports a global brightness level (high/medium/low)
for whichever player LEDs are currently lit. The driver already
defines the brightness byte but never uses it
(dualsense_output_report_common::led_brightness).

This patch wires up the controls for the brightness byte, allowing
the player indicator brightness to be adjusted.

Increase max_brightness of each player's LED classdev from 1 to 3.

brightness = 0 still turns an LED off, writing brightness = 1..3
turns the LED on and applies the brightness to all lit player LEDs.
This is a hardware limitation and the driver reflects that by
keeping the sysfs brightness of player LEDs in sync. The last
brightness write wins.

The firmware encoding is inverted (lower is brighter), so map LED
core brightness values to the device-specific encoding before
sending reports.

Also set DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE
whenever player LED state is updated so firmware applies the
brightness byte.

Existing userspace that writes brightness=1 as a simple "on" value
continues to work, but LEDs will now be lit at the lowest brightness
instead of the highest brightness.

Tested on a DualSense controller (product ID 0x0ce6) using Bluetooth
and USB.

Assisted-by: OpenCode:claude-sonnet-5
Signed-off-by: Kateřina Medvědová <k8ie@mcld.eu>
---
First-time contributing a patch.
---
 drivers/hid/hid-playstation.c | 48 +++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index f9dc9378cf77..78e43a176248 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -164,11 +164,20 @@ struct ps_led_info {
 #define DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE		BIT(7)
 #define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE	BIT(1)
 #define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2		BIT(2)
+#define DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE	BIT(0)
 #define DS_OUTPUT_AUDIO_FLAGS_OUTPUT_PATH_SEL			GENMASK(5, 4)
 #define DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN			GENMASK(2, 0)
 #define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE			BIT(4)
 #define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT			BIT(1)
 
+/*
+ * Player LED brightness levels. Lower values are brighter; this is inverted
+ * from the LED subsystem's convention where higher values mean brighter.
+ */
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_HIGH			0
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_MEDIUM			1
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW			2
+
 /* DualSense hardware limits */
 #define DS_ACC_RES_PER_G	8192
 #define DS_ACC_RANGE		(4 * DS_ACC_RES_PER_G)
@@ -225,6 +234,7 @@ struct dualsense {
 	/* Player leds */
 	bool update_player_leds;
 	u8 player_leds_state;
+	u8 player_leds_brightness;
 	struct led_classdev player_leds[5];
 
 	struct work_struct output_worker;
@@ -1219,12 +1229,22 @@ static int dualsense_lightbar_set_brightness(struct led_classdev *cdev,
 	return 0;
 }
 
+/*
+ * The DualSense's player LEDs only support a single, shared brightness level
+ * for all lit LEDs -- there is no per-LED brightness control. We still expose
+ * per-LED on/off state plus 3 brightness levels through each LED classdev's
+ * max_brightness of 3; the last value written by any player LED classdev sets
+ * the shared level for all of them.
+ */
 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));
+	if (!(ds->player_leds_state & BIT(led - ds->player_leds)))
+		return LED_OFF;
+
+	return DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - ds->player_leds_brightness;
 }
 
 static int dualsense_player_led_set_brightness(struct led_classdev *led, enum led_brightness value)
@@ -1235,10 +1255,18 @@ static int dualsense_player_led_set_brightness(struct led_classdev *led, enum le
 
 	scoped_guard(spinlock_irqsave, &ds->base.lock) {
 		led_index = led - ds->player_leds;
-		if (value == LED_OFF)
+		if (value == LED_OFF) {
 			ds->player_leds_state &= ~BIT(led_index);
-		else
+		} else {
 			ds->player_leds_state |= BIT(led_index);
+			/* Convert Linux brightness (1=dimmest) to firmware
+			 * scale (0=brightest), clamping to the valid range.
+			 */
+			value = clamp_t(enum led_brightness, value, 1,
+					 DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1);
+			ds->player_leds_brightness =
+				DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - value;
+		}
 
 		ds->update_player_leds = true;
 	}
@@ -1355,6 +1383,10 @@ static void dualsense_output_worker(struct work_struct *work)
 				DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE;
 			common->player_leds = ds->player_leds_state;
 
+			common->valid_flag2 |=
+				DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE;
+			common->led_brightness = ds->player_leds_brightness;
+
 			ds->update_player_leds = false;
 		}
 
@@ -1742,15 +1774,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	int i, ret;
 
 	static const struct ps_led_info player_leds_info[] = {
-		{ LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER1, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER2, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER2, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER3, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER3, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER4, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER4, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER5, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER5, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness }
 	};
 
-- 
2.55.0

Re: [PATCH] HID: playstation: Support DualSense player LED brightness control

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Multi-variable data race on `ds->player_leds_state` and `ds->player_leds_brightness` in `dualsense_player_led_get_brightness()`.

Pre-existing issues:
- [High] Missing workqueue cancellation in the `dualsense_create()` and `dualshock4_create()` error paths causes a use-after-free if probe fails.
--

--- Patch [1]: [PATCH] HID: playstation: Support DualSense player LED brightness control ---
commit 9dd8a497081a6ac6ae8e211e738b3ef23a0669be
Author: Kateřina Medvědová <k8ie@mcld.eu>

    HID: playstation: Support DualSense player LED brightness control

This patch wires up the controls for the brightness byte, allowing
the player indicator brightness to be adjusted. It increases the
max_brightness of each player's LED classdev from 1 to 3, and correctly
updates the firmware encoding.

> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index f9dc9378cf777..78e43a176248a 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
[ ... ]
> @@ -1219,12 +1229,22 @@ static int dualsense_lightbar_set_brightness(struct led_classdev *cdev,
>  	return 0;
>  }
>  
> +/*
> + * The DualSense's player LEDs only support a single, shared brightness level
> + * for all lit LEDs -- there is no per-LED brightness control. We still expose
> + * per-LED on/off state plus 3 brightness levels through each LED classdev's
> + * max_brightness of 3; the last value written by any player LED classdev sets
> + * the shared level for all of them.
> + */
>  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));
> +	if (!(ds->player_leds_state & BIT(led - ds->player_leds)))
> +		return LED_OFF;
> +
> +	return DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - ds->player_leds_brightness;

[Severity: Medium]
Could this lockless read result in a multi-variable data race?

In dualsense_player_led_set_brightness(), ds->player_leds_state and
ds->player_leds_brightness are updated together under ds->base.lock.
If dualsense_player_led_get_brightness() reads them without the lock,
a concurrent read could observe the updated state but the old
brightness (or vice-versa due to compiler or CPU reordering), returning
an inconsistent value to userspace.

>  }
>  
>  static int dualsense_player_led_set_brightness(struct led_classdev *led, enum led_brightness value)
[ ... ]
> @@ -1742,15 +1774,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
>  	int i, ret;
>  
>  	static const struct ps_led_info player_leds_info[] = {
> -		{ LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness,
> +		{ LED_FUNCTION_PLAYER1, "white", 3, dualsense_player_led_get_brightness,
>  				dualsense_player_led_set_brightness },
[ ... ]
> -		{ LED_FUNCTION_PLAYER5, "white", 1, dualsense_player_led_get_brightness,
> +		{ LED_FUNCTION_PLAYER5, "white", 3, dualsense_player_led_get_brightness,
>  				dualsense_player_led_set_brightness }
>  	};
>  

[Severity: High]
This is a pre-existing issue, but missing workqueue cancellation in the
dualsense_create() and dualshock4_create() error paths could cause a
use-after-free if probe fails.

During probe, dualsense_set_lightbar() is called to set the initial lightbar
color, which implicitly schedules ds->output_worker:

drivers/hid/hid-playstation.c:dualsense_create() {
    ...
	dualsense_set_lightbar(ds, 0, 0, 128); /* blue */

	for (i = 0; i < ARRAY_SIZE(player_leds_info); i++) {
		const struct ps_led_info *led_info = &player_leds_info[i];

		ret = ps_led_register(ps_dev, &ds->player_leds[i], led_info);
		if (ret < 0)
			goto err;
	}
    ...
}

If a subsequent initialization step fails, the error path returns and allows
devres to free the allocated ds struct:

err:
	ps_devices_list_remove(ps_dev);
	return ERR_PTR(ret);
}

Because the worker is not cancelled, it remains in the system workqueue and
will dereference the freed ds pointer when it executes. Does the error path
need an explicit cancel_work_sync(&ds->output_worker) to prevent this? The
exact same vulnerability exists in dualshock4_create() due to
dualshock4_set_bt_poll_interval().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714180424.70662-1-k8ie@mcld.eu?part=1

[PATCH v2] HID: playstation: Support DualSense player LED brightness control

[k8ie]

From: Kateřina Medvědová <k8ie@mcld.eu>

The DualSense's 5 player indicator LEDs are currently only exposed as
plain on/off LED classdevs (max_brightness = 1). The controller
firmware also supports a global brightness level (high/medium/low)
for whichever player LEDs are currently lit. The driver already
defines the brightness byte but never uses it
(dualsense_output_report_common::led_brightness).

This patch wires up the controls for the brightness byte, allowing
the player indicator brightness to be adjusted.

Increase max_brightness of each player's LED classdev from 1 to 3.

brightness = 0 still turns an LED off, writing brightness = 1..3
turns the LED on and applies the brightness to all lit player LEDs.
This is a hardware limitation and the driver reflects that by
keeping the sysfs brightness of player LEDs in sync. The last
brightness write wins.

The firmware encoding is inverted (lower is brighter), so map LED
core brightness values to the device-specific encoding before
sending reports.

Also set DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE
whenever player LED state is updated so firmware applies the
brightness byte.

Existing userspace that writes brightness=1 as a simple "on" value
continues to work, but LEDs will now be lit at the lowest brightness
instead of the highest brightness.

Tested on a DualSense controller (product ID 0x0ce6) using Bluetooth
and USB.

Assisted-by: OpenCode:claude-sonnet-5
Signed-off-by: Kateřina Medvědová <k8ie@mcld.eu>
---
First-time contributing a patch.

v1 -> v2: Fixed a data race flagged by the list's automated review bot:
dualsense_player_led_get_brightness() read player_leds_state and
player_leds_brightness without holding ds->base.lock, while the setter
updates both fields together under that lock. Now takes the same lock
in the getter.

The bot also flagged a pre-existing missing cancel_work_sync() in the
dualsense_create()/dualshock4_create() error paths. That is unrelated
to this change (present before this patch) and is intentionally left
unfixed.
---
 drivers/hid/hid-playstation.c | 50 +++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index f9dc9378cf77..58786117ec01 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -164,11 +164,20 @@ struct ps_led_info {
 #define DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE		BIT(7)
 #define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE	BIT(1)
 #define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2		BIT(2)
+#define DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE	BIT(0)
 #define DS_OUTPUT_AUDIO_FLAGS_OUTPUT_PATH_SEL			GENMASK(5, 4)
 #define DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN			GENMASK(2, 0)
 #define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE			BIT(4)
 #define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT			BIT(1)
 
+/*
+ * Player LED brightness levels. Lower values are brighter; this is inverted
+ * from the LED subsystem's convention where higher values mean brighter.
+ */
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_HIGH			0
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_MEDIUM			1
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW			2
+
 /* DualSense hardware limits */
 #define DS_ACC_RES_PER_G	8192
 #define DS_ACC_RANGE		(4 * DS_ACC_RES_PER_G)
@@ -225,6 +234,7 @@ struct dualsense {
 	/* Player leds */
 	bool update_player_leds;
 	u8 player_leds_state;
+	u8 player_leds_brightness;
 	struct led_classdev player_leds[5];
 
 	struct work_struct output_worker;
@@ -1219,12 +1229,24 @@ static int dualsense_lightbar_set_brightness(struct led_classdev *cdev,
 	return 0;
 }
 
+/*
+ * The DualSense's player LEDs only support a single, shared brightness level
+ * for all lit LEDs -- there is no per-LED brightness control. We still expose
+ * per-LED on/off state plus 3 brightness levels through each LED classdev's
+ * max_brightness of 3; the last value written by any player LED classdev sets
+ * the shared level for all of them.
+ */
 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));
+	guard(spinlock_irqsave)(&ds->base.lock);
+
+	if (!(ds->player_leds_state & BIT(led - ds->player_leds)))
+		return LED_OFF;
+
+	return DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - ds->player_leds_brightness;
 }
 
 static int dualsense_player_led_set_brightness(struct led_classdev *led, enum led_brightness value)
@@ -1235,10 +1257,18 @@ static int dualsense_player_led_set_brightness(struct led_classdev *led, enum le
 
 	scoped_guard(spinlock_irqsave, &ds->base.lock) {
 		led_index = led - ds->player_leds;
-		if (value == LED_OFF)
+		if (value == LED_OFF) {
 			ds->player_leds_state &= ~BIT(led_index);
-		else
+		} else {
 			ds->player_leds_state |= BIT(led_index);
+			/* Convert Linux brightness (1=dimmest) to firmware
+			 * scale (0=brightest), clamping to the valid range.
+			 */
+			value = clamp_t(enum led_brightness, value, 1,
+					 DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1);
+			ds->player_leds_brightness =
+				DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - value;
+		}
 
 		ds->update_player_leds = true;
 	}
@@ -1355,6 +1385,10 @@ static void dualsense_output_worker(struct work_struct *work)
 				DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE;
 			common->player_leds = ds->player_leds_state;
 
+			common->valid_flag2 |=
+				DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE;
+			common->led_brightness = ds->player_leds_brightness;
+
 			ds->update_player_leds = false;
 		}
 
@@ -1742,15 +1776,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	int i, ret;
 
 	static const struct ps_led_info player_leds_info[] = {
-		{ LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER1, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER2, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER2, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER3, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER3, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER4, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER4, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER5, "white", 1, dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER5, "white", 3, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness }
 	};
 
-- 
2.55.0

Re: [PATCH v2] HID: playstation: Support DualSense player LED brightness control

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing cancel_work_sync() in the probe error paths of dualsense_create() and dualshock4_create() causes a use-after-free vulnerability.
--

--- Patch [1]: [PATCH v2] HID: playstation: Support DualSense player LED brightness control ---
commit c4af8f1ee7d50867512c691be4fbde7636ea1510
Author: Kateřina Medvědová <k8ie@mcld.eu>

HID: playstation: Support DualSense player LED brightness control

This patch wires up the controls for the brightness byte on the DualSense
player indicator LEDs. It increases the max_brightness of each player's LED
classdev from 1 to 3, and updates the brightness values before sending
the output reports.

> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index f9dc9378cf777..58786117ec016 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
[ ... ]
> @@ -1742,15 +1776,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
>  	int i, ret;
>  
>  	static const struct ps_led_info player_leds_info[] = {
> -		{ LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness,
> +		{ LED_FUNCTION_PLAYER1, "white", 3, dualsense_player_led_get_brightness,
>  				dualsense_player_led_set_brightness },
> -		{ LED_FUNCTION_PLAYER2, "white", 1, dualsense_player_led_get_brightness,
> +		{ LED_FUNCTION_PLAYER2, "white", 3, dualsense_player_led_get_brightness,
>  				dualsense_player_led_set_brightness },

[Severity: Critical]
This is a pre-existing issue, but the error paths for dualsense_create() and
dualshock4_create() appear to be missing a cancel_work_sync() call.

If initialization fails after the work is scheduled (for example, if
ps_led_register() fails in dualsense_create()), the code jumps to the err
path:

drivers/hid/hid-playstation.c:dualsense_create() {
    ...
err:
	ps_devices_list_remove(ps_dev);
	return ERR_PTR(ret);
}

Since ds->output_worker is not canceled before the devm routines free the
ds/ds4 structure, could this result in a use-after-free when the global
workqueue eventually executes the worker function?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714184225.84646-1-k8ie@mcld.eu?part=1