[PATCH 00/11] HID: playstation: Add support for audio jack handling on DualSense

[PATCH 00/11] HID: playstation: Add support for audio jack handling on DualSense

[Cristian Ciocaltea]

The Sony DualSense wireless controller (PS5) provides an internal mono
speaker, in addition to the 3.5mm jack socket for headphone output and
headset microphone input.  However, the default audio output path is set
to headphones, regardless of whether they are actually inserted or not.

This patch series aims to improve the audio support by implementing the
following changes:

* Detect when the plugged state of the audio jack changes and toggle
  audio output between headphones and internal speaker, as required.
  The latter is achieved by essentially routing the right channel of the
  audio source to the mono speaker.

* Adjust the speaker volume since its default level is too low and,
  therefore, cannot generate any audible sound.

* Register a dedicated input device for the audio jack and use it to
  report all headphone and headset mic insert events.

It's worth noting the latter is necessary since the controller complies
with v1.0 of the USB Audio Class spec (UAC1) and, therefore, cannot
advertise any jack detection capability.  However, this feature can be
implemented in the generic USB audio driver via quirks, i.e. by
configuring an input handler to receive hotplug events from the HID
driver.

Unrelated to the above, also provide a few driver cleanup patches, e.g.
make use of bitfields macros, simplify locking, fix coding style.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Cristian Ciocaltea (11):
      HID: playstation: Make use of bitfield macros
      HID: playstation: Add spaces around arithmetic operators
      HID: playstation: Simplify locking with guard() and scoped_guard()
      HID: playstation: Replace uint{32,16,8}_t with u{32,16,8}
      HID: playstation: Correct spelling in comment sections
      HID: playstation: Fix all alignment and line length issues
      HID: playstation: Document spinlock_t usage
      HID: playstation: Prefer kzalloc(sizeof(*buf)...)
      HID: playstation: Rename DualSense input report status field
      HID: playstation: Support DualSense audio jack hotplug detection
      HID: playstation: Support DualSense audio jack event reporting

 drivers/hid/hid-playstation.c | 885 ++++++++++++++++++++++++------------------
 1 file changed, 500 insertions(+), 385 deletions(-)
---
base-commit: 7bac2c97af4078d7a627500c9bcdd5b033f97718
change-id: 20250522-dualsense-hid-jack-d3cb65b75da1

[PATCH 01/11] HID: playstation: Make use of bitfield macros

[Cristian Ciocaltea]

Improve code readability by replacing open coded bit operations with the
equivalent bitfield macros.

While at it, vertically align all DS_OUTPUT_* bit constants.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 68 ++++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 1468fb11e39dffc883181663a4ad44252e0a7ebb..538194ce8902fe1383b57ac59743f32838dcb0df 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -5,6 +5,7 @@
  *  Copyright (c) 2020-2022 Sony Interactive Entertainment
  */
 
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/crc32.h>
 #include <linux/device.h>
@@ -116,29 +117,41 @@ struct ps_led_info {
 #define DS_STATUS_CHARGING_SHIFT	4
 
 /* Feature version from DualSense Firmware Info report. */
-#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
-
+#define DS_FEATURE_VERSION_MINOR		GENMASK(7, 0)
+#define DS_FEATURE_VERSION_MAJOR		GENMASK(15, 8)
+#define DS_FEATURE_VERSION(major, minor)	(FIELD_PREP(DS_FEATURE_VERSION_MAJOR, major) | \
+						 FIELD_PREP(DS_FEATURE_VERSION_MINOR, minor))
 /*
  * Status of a DualSense touch point contact.
  * Contact IDs, with highest bit set are 'inactive'
  * and any associated data is then invalid.
  */
-#define DS_TOUCH_POINT_INACTIVE BIT(7)
+#define DS_TOUCH_POINT_INACTIVE			BIT(7)
+#define DS_TOUCH_POINT_X_LO			GENMASK(7, 0)
+#define DS_TOUCH_POINT_X_HI			GENMASK(11, 8)
+#define DS_TOUCH_POINT_X(hi, lo)		(FIELD_PREP(DS_TOUCH_POINT_X_HI, hi) | \
+						 FIELD_PREP(DS_TOUCH_POINT_X_LO, lo))
+#define DS_TOUCH_POINT_Y_LO			GENMASK(3, 0)
+#define DS_TOUCH_POINT_Y_HI			GENMASK(11, 4)
+#define DS_TOUCH_POINT_Y(hi, lo)		(FIELD_PREP(DS_TOUCH_POINT_Y_HI, hi) | \
+						 FIELD_PREP(DS_TOUCH_POINT_Y_LO, lo))
 
  /* Magic value required in tag field of Bluetooth output report. */
-#define DS_OUTPUT_TAG 0x10
+#define DS_OUTPUT_TAG				0x10
+#define DS_OUTPUT_SEQ_TAG			GENMASK(3, 0)
+#define DS_OUTPUT_SEQ_NO			GENMASK(7, 4)
 /* Flags for DualSense output report. */
-#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION BIT(0)
-#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT BIT(1)
-#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE BIT(0)
-#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE BIT(1)
-#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE BIT(2)
-#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
-#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
-#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
-#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
-#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
-#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
+#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION		BIT(0)
+#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT			BIT(1)
+#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE	BIT(0)
+#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE		BIT(1)
+#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE		BIT(2)
+#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS			BIT(3)
+#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE	BIT(4)
+#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE	BIT(1)
+#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2		BIT(2)
+#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE			BIT(4)
+#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT			BIT(1)
 
 /* DualSense hardware limits */
 #define DS_ACC_RES_PER_G	8192
@@ -315,7 +328,9 @@ struct dualsense_output_report {
  * Contact IDs, with highest bit set are 'inactive'
  * and any associated data is then invalid.
  */
-#define DS4_TOUCH_POINT_INACTIVE BIT(7)
+#define DS4_TOUCH_POINT_INACTIVE	BIT(7)
+#define DS4_TOUCH_POINT_X(hi, lo)	DS_TOUCH_POINT_X(hi, lo)
+#define DS4_TOUCH_POINT_Y(hi, lo)	DS_TOUCH_POINT_Y(hi, lo)
 
 /* Status field of DualShock4 input report. */
 #define DS4_STATUS0_BATTERY_CAPACITY	GENMASK(3, 0)
@@ -1194,7 +1209,8 @@ static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_
 		 * Highest 4-bit is a sequence number, which needs to be increased
 		 * every report. Lowest 4-bit is tag and can be zero for now.
 		 */
-		bt->seq_tag = (ds->output_seq << 4) | 0x0;
+		bt->seq_tag = FIELD_PREP(DS_OUTPUT_SEQ_NO, ds->output_seq) |
+			      FIELD_PREP(DS_OUTPUT_SEQ_TAG, 0x0);
 		if (++ds->output_seq == 16)
 			ds->output_seq = 0;
 
@@ -1439,11 +1455,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 		input_mt_report_slot_state(ds->touchpad, MT_TOOL_FINGER, active);
 
 		if (active) {
-			int x = (point->x_hi << 8) | point->x_lo;
-			int y = (point->y_hi << 4) | point->y_lo;
-
-			input_report_abs(ds->touchpad, ABS_MT_POSITION_X, x);
-			input_report_abs(ds->touchpad, ABS_MT_POSITION_Y, y);
+			input_report_abs(ds->touchpad, ABS_MT_POSITION_X,
+					 DS_TOUCH_POINT_X(point->x_hi, point->x_lo));
+			input_report_abs(ds->touchpad, ABS_MT_POSITION_Y,
+					 DS_TOUCH_POINT_Y(point->y_hi, point->y_lo));
 		}
 	}
 	input_mt_sync_frame(ds->touchpad);
@@ -2351,11 +2366,10 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 			input_mt_report_slot_state(ds4->touchpad, MT_TOOL_FINGER, active);
 
 			if (active) {
-				int x = (point->x_hi << 8) | point->x_lo;
-				int y = (point->y_hi << 4) | point->y_lo;
-
-				input_report_abs(ds4->touchpad, ABS_MT_POSITION_X, x);
-				input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y, y);
+				input_report_abs(ds4->touchpad, ABS_MT_POSITION_X,
+						 DS4_TOUCH_POINT_X(point->x_hi, point->x_lo));
+				input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y,
+						 DS4_TOUCH_POINT_Y(point->y_hi, point->y_lo));
 			}
 		}
 		input_mt_sync_frame(ds4->touchpad);

-- 
2.49.0

[PATCH 02/11] HID: playstation: Add spaces around arithmetic operators

[Cristian Ciocaltea]

Get rid of several checkpatch.pl complaints:

  CHECK: spaces preferred around that '*' (ctx:VxV)
  CHECK: spaces preferred around that '/' (ctx:VxV)

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 538194ce8902fe1383b57ac59743f32838dcb0df..a82174a21dcce6523a69a8ec374a72504614cedc 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -155,9 +155,9 @@ struct ps_led_info {
 
 /* DualSense hardware limits */
 #define DS_ACC_RES_PER_G	8192
-#define DS_ACC_RANGE		(4*DS_ACC_RES_PER_G)
+#define DS_ACC_RANGE		(4 * DS_ACC_RES_PER_G)
 #define DS_GYRO_RES_PER_DEG_S	1024
-#define DS_GYRO_RANGE		(2048*DS_GYRO_RES_PER_DEG_S)
+#define DS_GYRO_RANGE		(2048 * DS_GYRO_RES_PER_DEG_S)
 #define DS_TOUCHPAD_WIDTH	1920
 #define DS_TOUCHPAD_HEIGHT	1080
 
@@ -364,9 +364,9 @@ struct dualsense_output_report {
 
 /* DualShock4 hardware limits */
 #define DS4_ACC_RES_PER_G	8192
-#define DS4_ACC_RANGE		(4*DS_ACC_RES_PER_G)
+#define DS4_ACC_RANGE		(4 * DS_ACC_RES_PER_G)
 #define DS4_GYRO_RES_PER_DEG_S	1024
-#define DS4_GYRO_RANGE		(2048*DS_GYRO_RES_PER_DEG_S)
+#define DS4_GYRO_RANGE		(2048 * DS_GYRO_RES_PER_DEG_S)
 #define DS4_LIGHTBAR_MAX_BLINK	255 /* 255 centiseconds */
 #define DS4_TOUCHPAD_WIDTH	1920
 #define DS4_TOUCHPAD_HEIGHT	942
@@ -1016,19 +1016,19 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 	speed_2x = (gyro_speed_plus + gyro_speed_minus);
 	ds->gyro_calib_data[0].abs_code = ABS_RX;
 	ds->gyro_calib_data[0].bias = 0;
-	ds->gyro_calib_data[0].sens_numer = speed_2x*DS_GYRO_RES_PER_DEG_S;
+	ds->gyro_calib_data[0].sens_numer = speed_2x * DS_GYRO_RES_PER_DEG_S;
 	ds->gyro_calib_data[0].sens_denom = abs(gyro_pitch_plus - gyro_pitch_bias) +
 			abs(gyro_pitch_minus - gyro_pitch_bias);
 
 	ds->gyro_calib_data[1].abs_code = ABS_RY;
 	ds->gyro_calib_data[1].bias = 0;
-	ds->gyro_calib_data[1].sens_numer = speed_2x*DS_GYRO_RES_PER_DEG_S;
+	ds->gyro_calib_data[1].sens_numer = speed_2x * DS_GYRO_RES_PER_DEG_S;
 	ds->gyro_calib_data[1].sens_denom = abs(gyro_yaw_plus - gyro_yaw_bias) +
 			abs(gyro_yaw_minus - gyro_yaw_bias);
 
 	ds->gyro_calib_data[2].abs_code = ABS_RZ;
 	ds->gyro_calib_data[2].bias = 0;
-	ds->gyro_calib_data[2].sens_numer = speed_2x*DS_GYRO_RES_PER_DEG_S;
+	ds->gyro_calib_data[2].sens_numer = speed_2x * DS_GYRO_RES_PER_DEG_S;
 	ds->gyro_calib_data[2].sens_denom = abs(gyro_roll_plus - gyro_roll_bias) +
 			abs(gyro_roll_minus - gyro_roll_bias);
 
@@ -1054,19 +1054,19 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 	range_2g = acc_x_plus - acc_x_minus;
 	ds->accel_calib_data[0].abs_code = ABS_X;
 	ds->accel_calib_data[0].bias = acc_x_plus - range_2g / 2;
-	ds->accel_calib_data[0].sens_numer = 2*DS_ACC_RES_PER_G;
+	ds->accel_calib_data[0].sens_numer = 2 * DS_ACC_RES_PER_G;
 	ds->accel_calib_data[0].sens_denom = range_2g;
 
 	range_2g = acc_y_plus - acc_y_minus;
 	ds->accel_calib_data[1].abs_code = ABS_Y;
 	ds->accel_calib_data[1].bias = acc_y_plus - range_2g / 2;
-	ds->accel_calib_data[1].sens_numer = 2*DS_ACC_RES_PER_G;
+	ds->accel_calib_data[1].sens_numer = 2 * DS_ACC_RES_PER_G;
 	ds->accel_calib_data[1].sens_denom = range_2g;
 
 	range_2g = acc_z_plus - acc_z_minus;
 	ds->accel_calib_data[2].abs_code = ABS_Z;
 	ds->accel_calib_data[2].bias = acc_z_plus - range_2g / 2;
-	ds->accel_calib_data[2].sens_numer = 2*DS_ACC_RES_PER_G;
+	ds->accel_calib_data[2].sens_numer = 2 * DS_ACC_RES_PER_G;
 	ds->accel_calib_data[2].sens_denom = range_2g;
 
 	/*
@@ -1882,19 +1882,19 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	speed_2x = (gyro_speed_plus + gyro_speed_minus);
 	ds4->gyro_calib_data[0].abs_code = ABS_RX;
 	ds4->gyro_calib_data[0].bias = 0;
-	ds4->gyro_calib_data[0].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
+	ds4->gyro_calib_data[0].sens_numer = speed_2x * DS4_GYRO_RES_PER_DEG_S;
 	ds4->gyro_calib_data[0].sens_denom = abs(gyro_pitch_plus - gyro_pitch_bias) +
 			abs(gyro_pitch_minus - gyro_pitch_bias);
 
 	ds4->gyro_calib_data[1].abs_code = ABS_RY;
 	ds4->gyro_calib_data[1].bias = 0;
-	ds4->gyro_calib_data[1].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
+	ds4->gyro_calib_data[1].sens_numer = speed_2x * DS4_GYRO_RES_PER_DEG_S;
 	ds4->gyro_calib_data[1].sens_denom = abs(gyro_yaw_plus - gyro_yaw_bias) +
 			abs(gyro_yaw_minus - gyro_yaw_bias);
 
 	ds4->gyro_calib_data[2].abs_code = ABS_RZ;
 	ds4->gyro_calib_data[2].bias = 0;
-	ds4->gyro_calib_data[2].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
+	ds4->gyro_calib_data[2].sens_numer = speed_2x * DS4_GYRO_RES_PER_DEG_S;
 	ds4->gyro_calib_data[2].sens_denom = abs(gyro_roll_plus - gyro_roll_bias) +
 			abs(gyro_roll_minus - gyro_roll_bias);
 
@@ -1905,19 +1905,19 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	range_2g = acc_x_plus - acc_x_minus;
 	ds4->accel_calib_data[0].abs_code = ABS_X;
 	ds4->accel_calib_data[0].bias = acc_x_plus - range_2g / 2;
-	ds4->accel_calib_data[0].sens_numer = 2*DS4_ACC_RES_PER_G;
+	ds4->accel_calib_data[0].sens_numer = 2 * DS4_ACC_RES_PER_G;
 	ds4->accel_calib_data[0].sens_denom = range_2g;
 
 	range_2g = acc_y_plus - acc_y_minus;
 	ds4->accel_calib_data[1].abs_code = ABS_Y;
 	ds4->accel_calib_data[1].bias = acc_y_plus - range_2g / 2;
-	ds4->accel_calib_data[1].sens_numer = 2*DS4_ACC_RES_PER_G;
+	ds4->accel_calib_data[1].sens_numer = 2 * DS4_ACC_RES_PER_G;
 	ds4->accel_calib_data[1].sens_denom = range_2g;
 
 	range_2g = acc_z_plus - acc_z_minus;
 	ds4->accel_calib_data[2].abs_code = ABS_Z;
 	ds4->accel_calib_data[2].bias = acc_z_plus - range_2g / 2;
-	ds4->accel_calib_data[2].sens_numer = 2*DS4_ACC_RES_PER_G;
+	ds4->accel_calib_data[2].sens_numer = 2 * DS4_ACC_RES_PER_G;
 	ds4->accel_calib_data[2].sens_denom = range_2g;
 
 transfer_failed:
@@ -2059,8 +2059,10 @@ static int dualshock4_led_set_blink(struct led_classdev *led, unsigned long *del
 		ds4->lightbar_blink_off = 50;
 	} else {
 		/* Blink delays in centiseconds. */
-		ds4->lightbar_blink_on = min_t(unsigned long, *delay_on/10, DS4_LIGHTBAR_MAX_BLINK);
-		ds4->lightbar_blink_off = min_t(unsigned long, *delay_off/10, DS4_LIGHTBAR_MAX_BLINK);
+		ds4->lightbar_blink_on = min_t(unsigned long, *delay_on / 10,
+					       DS4_LIGHTBAR_MAX_BLINK);
+		ds4->lightbar_blink_off = min_t(unsigned long, *delay_off / 10,
+						DS4_LIGHTBAR_MAX_BLINK);
 	}
 
 	ds4->update_lightbar_blink = true;
@@ -2340,7 +2342,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	/* Convert timestamp (in 5.33us unit) to timestamp_us */
 	sensor_timestamp = le16_to_cpu(ds4_report->sensor_timestamp);
 	if (!ds4->sensor_timestamp_initialized) {
-		ds4->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp*16, 3);
+		ds4->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp * 16, 3);
 		ds4->sensor_timestamp_initialized = true;
 	} else {
 		uint16_t delta;
@@ -2349,7 +2351,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 			delta = (U16_MAX - ds4->prev_sensor_timestamp + sensor_timestamp + 1);
 		else
 			delta = sensor_timestamp - ds4->prev_sensor_timestamp;
-		ds4->sensor_timestamp_us += DIV_ROUND_CLOSEST(delta*16, 3);
+		ds4->sensor_timestamp_us += DIV_ROUND_CLOSEST(delta * 16, 3);
 	}
 	ds4->prev_sensor_timestamp = sensor_timestamp;
 	input_event(ds4->sensors, EV_MSC, MSC_TIMESTAMP, ds4->sensor_timestamp_us);

-- 
2.49.0

[PATCH 03/11] HID: playstation: Simplify locking with guard() and scoped_guard()

[Cristian Ciocaltea]

Use guard() and scoped_guard() infrastructure instead of explicitly
acquiring and releasing spinlocks and mutexes to simplify the code and
ensure that all locks are released properly.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 216 ++++++++++++++++++------------------------
 1 file changed, 93 insertions(+), 123 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index a82174a21dcce6523a69a8ec374a72504614cedc..16c576cc74284cc680cc7fc17e1bbe6bbcf84177 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -7,6 +7,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/crc32.h>
 #include <linux/device.h>
 #include <linux/hid.h>
@@ -567,26 +568,25 @@ static int ps_devices_list_add(struct ps_device *dev)
 {
 	struct ps_device *entry;
 
-	mutex_lock(&ps_devices_lock);
+	guard(mutex)(&ps_devices_lock);
+
 	list_for_each_entry(entry, &ps_devices_list, list) {
 		if (!memcmp(entry->mac_address, dev->mac_address, sizeof(dev->mac_address))) {
 			hid_err(dev->hdev, "Duplicate device found for MAC address %pMR.\n",
 					dev->mac_address);
-			mutex_unlock(&ps_devices_lock);
 			return -EEXIST;
 		}
 	}
 
 	list_add_tail(&dev->list, &ps_devices_list);
-	mutex_unlock(&ps_devices_lock);
 	return 0;
 }
 
 static int ps_devices_list_remove(struct ps_device *dev)
 {
-	mutex_lock(&ps_devices_lock);
+	guard(mutex)(&ps_devices_lock);
+
 	list_del(&dev->list);
-	mutex_unlock(&ps_devices_lock);
 	return 0;
 }
 
@@ -650,13 +650,12 @@ static int ps_battery_get_property(struct power_supply *psy,
 	struct ps_device *dev = power_supply_get_drvdata(psy);
 	uint8_t battery_capacity;
 	int battery_status;
-	unsigned long flags;
 	int ret = 0;
 
-	spin_lock_irqsave(&dev->lock, flags);
-	battery_capacity = dev->battery_capacity;
-	battery_status = dev->battery_status;
-	spin_unlock_irqrestore(&dev->lock, flags);
+	scoped_guard(spinlock_irqsave, &dev->lock) {
+		battery_capacity = dev->battery_capacity;
+		battery_status = dev->battery_status;
+	}
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:
@@ -1174,19 +1173,17 @@ static int dualsense_player_led_set_brightness(struct led_classdev *led, enum le
 {
 	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);
+	scoped_guard(spinlock_irqsave, &ds->base.lock) {
+		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);
+		ds->update_player_leds = true;
+	}
 
 	dualsense_schedule_work(ds);
 
@@ -1235,12 +1232,9 @@ static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_
 
 static inline void dualsense_schedule_work(struct dualsense *ds)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ds->base.lock, flags);
+	guard(spinlock_irqsave)(&ds->base.lock);
 	if (ds->output_worker_initialized)
 		schedule_work(&ds->output_worker);
-	spin_unlock_irqrestore(&ds->base.lock, flags);
 }
 
 /*
@@ -1338,7 +1332,6 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 	int battery_status;
 	uint32_t sensor_timestamp;
 	bool btn_mic_state;
-	unsigned long flags;
 	int i;
 
 	/*
@@ -1400,10 +1393,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 	 */
 	btn_mic_state = !!(ds_report->buttons[2] & DS_BUTTONS2_MIC_MUTE);
 	if (btn_mic_state && !ds->last_btn_mic_state) {
-		spin_lock_irqsave(&ps_dev->lock, flags);
-		ds->update_mic_mute = true;
-		ds->mic_muted = !ds->mic_muted; /* toggle */
-		spin_unlock_irqrestore(&ps_dev->lock, flags);
+		scoped_guard(spinlock_irqsave, &ps_dev->lock) {
+			ds->update_mic_mute = true;
+			ds->mic_muted = !ds->mic_muted; /* toggle */
+		}
 
 		/* Schedule updating of microphone state at hardware level. */
 		dualsense_schedule_work(ds);
@@ -1496,10 +1489,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 		battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
 	}
 
-	spin_lock_irqsave(&ps_dev->lock, flags);
-	ps_dev->battery_capacity = battery_capacity;
-	ps_dev->battery_status = battery_status;
-	spin_unlock_irqrestore(&ps_dev->lock, flags);
+	scoped_guard(spinlock_irqsave, &ps_dev->lock) {
+		ps_dev->battery_capacity = battery_capacity;
+		ps_dev->battery_status = battery_status;
+	}
 
 	return 0;
 }
@@ -1508,16 +1501,15 @@ static int dualsense_play_effect(struct input_dev *dev, void *data, struct ff_ef
 {
 	struct hid_device *hdev = input_get_drvdata(dev);
 	struct dualsense *ds = hid_get_drvdata(hdev);
-	unsigned long flags;
 
 	if (effect->type != FF_RUMBLE)
 		return 0;
 
-	spin_lock_irqsave(&ds->base.lock, flags);
-	ds->update_rumble = true;
-	ds->motor_left = effect->u.rumble.strong_magnitude / 256;
-	ds->motor_right = effect->u.rumble.weak_magnitude / 256;
-	spin_unlock_irqrestore(&ds->base.lock, flags);
+	scoped_guard(spinlock_irqsave, &ds->base.lock) {
+		ds->update_rumble = true;
+		ds->motor_left = effect->u.rumble.strong_magnitude / 256;
+		ds->motor_right = effect->u.rumble.weak_magnitude / 256;
+	}
 
 	dualsense_schedule_work(ds);
 	return 0;
@@ -1526,11 +1518,9 @@ static int dualsense_play_effect(struct input_dev *dev, void *data, struct ff_ef
 static void dualsense_remove(struct ps_device *ps_dev)
 {
 	struct dualsense *ds = container_of(ps_dev, struct dualsense, base);
-	unsigned long flags;
 
-	spin_lock_irqsave(&ds->base.lock, flags);
-	ds->output_worker_initialized = false;
-	spin_unlock_irqrestore(&ds->base.lock, flags);
+	scoped_guard(spinlock_irqsave, &ds->base.lock)
+		ds->output_worker_initialized = false;
 
 	cancel_work_sync(&ds->output_worker);
 }
@@ -1562,14 +1552,12 @@ static int dualsense_reset_leds(struct dualsense *ds)
 
 static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ds->base.lock, flags);
-	ds->update_lightbar = true;
-	ds->lightbar_red = red;
-	ds->lightbar_green = green;
-	ds->lightbar_blue = blue;
-	spin_unlock_irqrestore(&ds->base.lock, flags);
+	scoped_guard(spinlock_irqsave, &ds->base.lock) {
+		ds->update_lightbar = true;
+		ds->lightbar_red = red;
+		ds->lightbar_green = green;
+		ds->lightbar_blue = blue;
+	}
 
 	dualsense_schedule_work(ds);
 }
@@ -1756,7 +1744,6 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 static void dualshock4_dongle_calibration_work(struct work_struct *work)
 {
 	struct dualshock4 *ds4 = container_of(work, struct dualshock4, dongle_hotplug_worker);
-	unsigned long flags;
 	enum dualshock4_dongle_state dongle_state;
 	int ret;
 
@@ -1775,9 +1762,8 @@ static void dualshock4_dongle_calibration_work(struct work_struct *work)
 		dongle_state = DONGLE_CONNECTED;
 	}
 
-	spin_lock_irqsave(&ds4->base.lock, flags);
-	ds4->dongle_state = dongle_state;
-	spin_unlock_irqrestore(&ds4->base.lock, flags);
+	scoped_guard(spinlock_irqsave, &ds4->base.lock)
+		ds4->dongle_state = dongle_state;
 }
 
 static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
@@ -2049,26 +2035,23 @@ static int dualshock4_led_set_blink(struct led_classdev *led, unsigned long *del
 {
 	struct hid_device *hdev = to_hid_device(led->dev->parent);
 	struct dualshock4 *ds4 = hid_get_drvdata(hdev);
-	unsigned long flags;
 
-	spin_lock_irqsave(&ds4->base.lock, flags);
+	scoped_guard(spinlock_irqsave, &ds4->base.lock) {
+		if (!*delay_on && !*delay_off) {
+			/* Default to 1 Hz (50 centiseconds on, 50 centiseconds off). */
+			ds4->lightbar_blink_on = 50;
+			ds4->lightbar_blink_off = 50;
+		} else {
+			/* Blink delays in centiseconds. */
+			ds4->lightbar_blink_on = min_t(unsigned long, *delay_on / 10,
+						       DS4_LIGHTBAR_MAX_BLINK);
+			ds4->lightbar_blink_off = min_t(unsigned long, *delay_off / 10,
+							DS4_LIGHTBAR_MAX_BLINK);
+		}
 
-	if (!*delay_on && !*delay_off) {
-		/* Default to 1 Hz (50 centiseconds on, 50 centiseconds off). */
-		ds4->lightbar_blink_on = 50;
-		ds4->lightbar_blink_off = 50;
-	} else {
-		/* Blink delays in centiseconds. */
-		ds4->lightbar_blink_on = min_t(unsigned long, *delay_on / 10,
-					       DS4_LIGHTBAR_MAX_BLINK);
-		ds4->lightbar_blink_off = min_t(unsigned long, *delay_off / 10,
-						DS4_LIGHTBAR_MAX_BLINK);
+		ds4->update_lightbar_blink = true;
 	}
 
-	ds4->update_lightbar_blink = true;
-
-	spin_unlock_irqrestore(&ds4->base.lock, flags);
-
 	dualshock4_schedule_work(ds4);
 
 	/* Report scaled values back to LED subsystem */
@@ -2082,36 +2065,33 @@ static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brig
 {
 	struct hid_device *hdev = to_hid_device(led->dev->parent);
 	struct dualshock4 *ds4 = hid_get_drvdata(hdev);
-	unsigned long flags;
 	unsigned int led_index;
 
-	spin_lock_irqsave(&ds4->base.lock, flags);
-
-	led_index = led - ds4->lightbar_leds;
-	switch (led_index) {
-	case 0:
-		ds4->lightbar_red = value;
-		break;
-	case 1:
-		ds4->lightbar_green = value;
-		break;
-	case 2:
-		ds4->lightbar_blue = value;
-		break;
-	case 3:
-		ds4->lightbar_enabled = !!value;
-
-		/* brightness = 0 also cancels blinking in Linux. */
-		if (!ds4->lightbar_enabled) {
-			ds4->lightbar_blink_off = 0;
-			ds4->lightbar_blink_on = 0;
-			ds4->update_lightbar_blink = true;
+	scoped_guard(spinlock_irqsave, &ds4->base.lock) {
+		led_index = led - ds4->lightbar_leds;
+		switch (led_index) {
+		case 0:
+			ds4->lightbar_red = value;
+			break;
+		case 1:
+			ds4->lightbar_green = value;
+			break;
+		case 2:
+			ds4->lightbar_blue = value;
+			break;
+		case 3:
+			ds4->lightbar_enabled = !!value;
+
+			/* brightness = 0 also cancels blinking in Linux. */
+			if (!ds4->lightbar_enabled) {
+				ds4->lightbar_blink_off = 0;
+				ds4->lightbar_blink_on = 0;
+				ds4->update_lightbar_blink = true;
+			}
 		}
-	}
-
-	ds4->update_lightbar = true;
 
-	spin_unlock_irqrestore(&ds4->base.lock, flags);
+		ds4->update_lightbar = true;
+	}
 
 	dualshock4_schedule_work(ds4);
 
@@ -2243,7 +2223,6 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	uint8_t battery_capacity, num_touch_reports, value;
 	int battery_status, i, j;
 	uint16_t sensor_timestamp;
-	unsigned long flags;
 	bool is_minimal = false;
 
 	/*
@@ -2421,10 +2400,10 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 		battery_status = POWER_SUPPLY_STATUS_DISCHARGING;
 	}
 
-	spin_lock_irqsave(&ps_dev->lock, flags);
-	ps_dev->battery_capacity = battery_capacity;
-	ps_dev->battery_status = battery_status;
-	spin_unlock_irqrestore(&ps_dev->lock, flags);
+	scoped_guard(spinlock_irqsave, &ps_dev->lock) {
+		ps_dev->battery_capacity = battery_capacity;
+		ps_dev->battery_status = battery_status;
+	}
 
 	return 0;
 }
@@ -2442,7 +2421,6 @@ static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_r
 	 */
 	if (data[0] == DS4_INPUT_REPORT_USB && size == DS4_INPUT_REPORT_USB_SIZE) {
 		struct dualshock4_input_report_common *ds4_report = (struct dualshock4_input_report_common *)&data[1];
-		unsigned long flags;
 
 		connected = ds4_report->status[1] & DS4_STATUS1_DONGLE_STATE ? false : true;
 
@@ -2451,9 +2429,8 @@ static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_r
 
 			dualshock4_set_default_lightbar_colors(ds4);
 
-			spin_lock_irqsave(&ps_dev->lock, flags);
-			ds4->dongle_state = DONGLE_CALIBRATING;
-			spin_unlock_irqrestore(&ps_dev->lock, flags);
+			scoped_guard(spinlock_irqsave, &ps_dev->lock)
+				ds4->dongle_state = DONGLE_CALIBRATING;
 
 			schedule_work(&ds4->dongle_hotplug_worker);
 
@@ -2465,9 +2442,8 @@ static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_r
 			    ds4->dongle_state == DONGLE_DISABLED) && !connected) {
 			hid_info(ps_dev->hdev, "DualShock 4 USB dongle: controller disconnected\n");
 
-			spin_lock_irqsave(&ps_dev->lock, flags);
-			ds4->dongle_state = DONGLE_DISCONNECTED;
-			spin_unlock_irqrestore(&ps_dev->lock, flags);
+			scoped_guard(spinlock_irqsave, &ps_dev->lock)
+				ds4->dongle_state = DONGLE_DISCONNECTED;
 
 			/* Return 0, so hidraw can get the report. */
 			return 0;
@@ -2489,16 +2465,15 @@ static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_e
 {
 	struct hid_device *hdev = input_get_drvdata(dev);
 	struct dualshock4 *ds4 = hid_get_drvdata(hdev);
-	unsigned long flags;
 
 	if (effect->type != FF_RUMBLE)
 		return 0;
 
-	spin_lock_irqsave(&ds4->base.lock, flags);
-	ds4->update_rumble = true;
-	ds4->motor_left = effect->u.rumble.strong_magnitude / 256;
-	ds4->motor_right = effect->u.rumble.weak_magnitude / 256;
-	spin_unlock_irqrestore(&ds4->base.lock, flags);
+	scoped_guard(spinlock_irqsave, &ds4->base.lock) {
+		ds4->update_rumble = true;
+		ds4->motor_left = effect->u.rumble.strong_magnitude / 256;
+		ds4->motor_right = effect->u.rumble.weak_magnitude / 256;
+	}
 
 	dualshock4_schedule_work(ds4);
 	return 0;
@@ -2507,11 +2482,9 @@ static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_e
 static void dualshock4_remove(struct ps_device *ps_dev)
 {
 	struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
-	unsigned long flags;
 
-	spin_lock_irqsave(&ds4->base.lock, flags);
-	ds4->output_worker_initialized = false;
-	spin_unlock_irqrestore(&ds4->base.lock, flags);
+	scoped_guard(spinlock_irqsave, &ds4->base.lock)
+		ds4->output_worker_initialized = false;
 
 	cancel_work_sync(&ds4->output_worker);
 
@@ -2521,12 +2494,9 @@ static void dualshock4_remove(struct ps_device *ps_dev)
 
 static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ds4->base.lock, flags);
+	guard(spinlock_irqsave)(&ds4->base.lock);
 	if (ds4->output_worker_initialized)
 		schedule_work(&ds4->output_worker);
-	spin_unlock_irqrestore(&ds4->base.lock, flags);
 }
 
 static void dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, uint8_t interval)

-- 
2.49.0

[PATCH 04/11] HID: playstation: Replace uint{32,16,8}_t with u{32,16,8}

[Cristian Ciocaltea]

And get rid of the following checkpatch.pl complaints:

  CHECK: Prefer kernel type 'u32' over 'uint32_t'
  CHECK: Prefer kernel type 'u16' over 'uint16_t'
  CHECK: Prefer kernel type 'u8' over 'uint8_t'

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 272 +++++++++++++++++++++---------------------
 1 file changed, 136 insertions(+), 136 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 16c576cc74284cc680cc7fc17e1bbe6bbcf84177..d36370d001edd0d82e8616f8f2803d8b7ad13954 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -40,17 +40,17 @@ struct ps_device {
 	struct hid_device *hdev;
 	spinlock_t lock;
 
-	uint32_t player_id;
+	u32 player_id;
 
 	struct power_supply_desc battery_desc;
 	struct power_supply *battery;
-	uint8_t battery_capacity;
+	u8 battery_capacity;
 	int battery_status;
 
 	const char *input_dev_name; /* Name of primary input device. */
-	uint8_t mac_address[6]; /* Note: stored in little endian order. */
-	uint32_t hw_version;
-	uint32_t fw_version;
+	u8 mac_address[6]; /* Note: stored in little endian order. */
+	u32 hw_version;
+	u32 fw_version;
 
 	int (*parse_report)(struct ps_device *dev, struct hid_report *report, u8 *data, int size);
 	void (*remove)(struct ps_device *dev);
@@ -169,7 +169,7 @@ struct dualsense {
 	struct input_dev *touchpad;
 
 	/* Update version is used as a feature/capability version. */
-	uint16_t update_version;
+	u16 update_version;
 
 	/* Calibration data for accelerometer and gyroscope. */
 	struct ps_calibration_data accel_calib_data[3];
@@ -177,21 +177,21 @@ struct dualsense {
 
 	/* Timestamp for sensor data */
 	bool sensor_timestamp_initialized;
-	uint32_t prev_sensor_timestamp;
-	uint32_t sensor_timestamp_us;
+	u32 prev_sensor_timestamp;
+	u32 sensor_timestamp_us;
 
 	/* Compatible rumble state */
 	bool use_vibration_v2;
 	bool update_rumble;
-	uint8_t motor_left;
-	uint8_t motor_right;
+	u8 motor_left;
+	u8 motor_right;
 
 	/* RGB lightbar */
 	struct led_classdev_mc lightbar;
 	bool update_lightbar;
-	uint8_t lightbar_red;
-	uint8_t lightbar_green;
-	uint8_t lightbar_blue;
+	u8 lightbar_red;
+	u8 lightbar_green;
+	u8 lightbar_blue;
 
 	/* Microphone */
 	bool update_mic_mute;
@@ -200,90 +200,90 @@ struct dualsense {
 
 	/* Player leds */
 	bool update_player_leds;
-	uint8_t player_leds_state;
+	u8 player_leds_state;
 	struct led_classdev player_leds[5];
 
 	struct work_struct output_worker;
 	bool output_worker_initialized;
 	void *output_report_dmabuf;
-	uint8_t output_seq; /* Sequence number for output report. */
+	u8 output_seq; /* Sequence number for output report. */
 };
 
 struct dualsense_touch_point {
-	uint8_t contact;
-	uint8_t x_lo;
-	uint8_t x_hi:4, y_lo:4;
-	uint8_t y_hi;
+	u8 contact;
+	u8 x_lo;
+	u8 x_hi:4, y_lo:4;
+	u8 y_hi;
 } __packed;
 static_assert(sizeof(struct dualsense_touch_point) == 4);
 
 /* Main DualSense input report excluding any BT/USB specific headers. */
 struct dualsense_input_report {
-	uint8_t x, y;
-	uint8_t rx, ry;
-	uint8_t z, rz;
-	uint8_t seq_number;
-	uint8_t buttons[4];
-	uint8_t reserved[4];
+	u8 x, y;
+	u8 rx, ry;
+	u8 z, rz;
+	u8 seq_number;
+	u8 buttons[4];
+	u8 reserved[4];
 
 	/* Motion sensors */
 	__le16 gyro[3]; /* x, y, z */
 	__le16 accel[3]; /* x, y, z */
 	__le32 sensor_timestamp;
-	uint8_t reserved2;
+	u8 reserved2;
 
 	/* Touchpad */
 	struct dualsense_touch_point points[2];
 
-	uint8_t reserved3[12];
-	uint8_t status;
-	uint8_t reserved4[10];
+	u8 reserved3[12];
+	u8 status;
+	u8 reserved4[10];
 } __packed;
 /* Common input report size shared equals the size of the USB report minus 1 byte for ReportID. */
 static_assert(sizeof(struct dualsense_input_report) == DS_INPUT_REPORT_USB_SIZE - 1);
 
 /* Common data between DualSense BT/USB main output report. */
 struct dualsense_output_report_common {
-	uint8_t valid_flag0;
-	uint8_t valid_flag1;
+	u8 valid_flag0;
+	u8 valid_flag1;
 
 	/* For DualShock 4 compatibility mode. */
-	uint8_t motor_right;
-	uint8_t motor_left;
+	u8 motor_right;
+	u8 motor_left;
 
 	/* Audio controls */
-	uint8_t reserved[4];
-	uint8_t mute_button_led;
+	u8 reserved[4];
+	u8 mute_button_led;
 
-	uint8_t power_save_control;
-	uint8_t reserved2[28];
+	u8 power_save_control;
+	u8 reserved2[28];
 
 	/* LEDs and lightbar */
-	uint8_t valid_flag2;
-	uint8_t reserved3[2];
-	uint8_t lightbar_setup;
-	uint8_t led_brightness;
-	uint8_t player_leds;
-	uint8_t lightbar_red;
-	uint8_t lightbar_green;
-	uint8_t lightbar_blue;
+	u8 valid_flag2;
+	u8 reserved3[2];
+	u8 lightbar_setup;
+	u8 led_brightness;
+	u8 player_leds;
+	u8 lightbar_red;
+	u8 lightbar_green;
+	u8 lightbar_blue;
 } __packed;
 static_assert(sizeof(struct dualsense_output_report_common) == 47);
 
 struct dualsense_output_report_bt {
-	uint8_t report_id; /* 0x31 */
-	uint8_t seq_tag;
-	uint8_t tag;
+	u8 report_id; /* 0x31 */
+	u8 seq_tag;
+	u8 tag;
 	struct dualsense_output_report_common common;
-	uint8_t reserved[24];
+	u8 reserved[24];
 	__le32 crc32;
 } __packed;
 static_assert(sizeof(struct dualsense_output_report_bt) == DS_OUTPUT_REPORT_BT_SIZE);
 
 struct dualsense_output_report_usb {
-	uint8_t report_id; /* 0x02 */
+	u8 report_id; /* 0x02 */
 	struct dualsense_output_report_common common;
-	uint8_t reserved[15];
+	u8 reserved[15];
 } __packed;
 static_assert(sizeof(struct dualsense_output_report_usb) == DS_OUTPUT_REPORT_USB_SIZE);
 
@@ -293,8 +293,8 @@ static_assert(sizeof(struct dualsense_output_report_usb) == DS_OUTPUT_REPORT_USB
  * This structure hide the differences between the two to simplify sending output reports.
  */
 struct dualsense_output_report {
-	uint8_t *data; /* Start of data */
-	uint8_t len; /* Size of output report */
+	u8 *data; /* Start of data */
+	u8 len; /* Size of output report */
 
 	/* Points to Bluetooth data payload in case for a Bluetooth report else NULL. */
 	struct dualsense_output_report_bt *bt;
@@ -396,26 +396,26 @@ struct dualshock4 {
 
 	/* Timestamp for sensor data */
 	bool sensor_timestamp_initialized;
-	uint32_t prev_sensor_timestamp;
-	uint32_t sensor_timestamp_us;
+	u32 prev_sensor_timestamp;
+	u32 sensor_timestamp_us;
 
 	/* Bluetooth poll interval */
 	bool update_bt_poll_interval;
-	uint8_t bt_poll_interval;
+	u8 bt_poll_interval;
 
 	bool update_rumble;
-	uint8_t motor_left;
-	uint8_t motor_right;
+	u8 motor_left;
+	u8 motor_right;
 
 	/* Lightbar leds */
 	bool update_lightbar;
 	bool update_lightbar_blink;
 	bool lightbar_enabled; /* For use by global LED control. */
-	uint8_t lightbar_red;
-	uint8_t lightbar_green;
-	uint8_t lightbar_blue;
-	uint8_t lightbar_blink_on; /* In increments of 10ms. */
-	uint8_t lightbar_blink_off; /* In increments of 10ms. */
+	u8 lightbar_red;
+	u8 lightbar_green;
+	u8 lightbar_blue;
+	u8 lightbar_blink_on; /* In increments of 10ms. */
+	u8 lightbar_blink_off; /* In increments of 10ms. */
 	struct led_classdev lightbar_leds[4];
 
 	struct work_struct output_worker;
@@ -424,88 +424,88 @@ struct dualshock4 {
 };
 
 struct dualshock4_touch_point {
-	uint8_t contact;
-	uint8_t x_lo;
-	uint8_t x_hi:4, y_lo:4;
-	uint8_t y_hi;
+	u8 contact;
+	u8 x_lo;
+	u8 x_hi:4, y_lo:4;
+	u8 y_hi;
 } __packed;
 static_assert(sizeof(struct dualshock4_touch_point) == 4);
 
 struct dualshock4_touch_report {
-	uint8_t timestamp;
+	u8 timestamp;
 	struct dualshock4_touch_point points[2];
 } __packed;
 static_assert(sizeof(struct dualshock4_touch_report) == 9);
 
 /* Main DualShock4 input report excluding any BT/USB specific headers. */
 struct dualshock4_input_report_common {
-	uint8_t x, y;
-	uint8_t rx, ry;
-	uint8_t buttons[3];
-	uint8_t z, rz;
+	u8 x, y;
+	u8 rx, ry;
+	u8 buttons[3];
+	u8 z, rz;
 
 	/* Motion sensors */
 	__le16 sensor_timestamp;
-	uint8_t sensor_temperature;
+	u8 sensor_temperature;
 	__le16 gyro[3]; /* x, y, z */
 	__le16 accel[3]; /* x, y, z */
-	uint8_t reserved2[5];
+	u8 reserved2[5];
 
-	uint8_t status[2];
-	uint8_t reserved3;
+	u8 status[2];
+	u8 reserved3;
 } __packed;
 static_assert(sizeof(struct dualshock4_input_report_common) == 32);
 
 struct dualshock4_input_report_usb {
-	uint8_t report_id; /* 0x01 */
+	u8 report_id; /* 0x01 */
 	struct dualshock4_input_report_common common;
-	uint8_t num_touch_reports;
+	u8 num_touch_reports;
 	struct dualshock4_touch_report touch_reports[3];
-	uint8_t reserved[3];
+	u8 reserved[3];
 } __packed;
 static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE);
 
 struct dualshock4_input_report_bt {
-	uint8_t report_id; /* 0x11 */
-	uint8_t reserved[2];
+	u8 report_id; /* 0x11 */
+	u8 reserved[2];
 	struct dualshock4_input_report_common common;
-	uint8_t num_touch_reports;
+	u8 num_touch_reports;
 	struct dualshock4_touch_report touch_reports[4]; /* BT has 4 compared to 3 for USB */
-	uint8_t reserved2[2];
+	u8 reserved2[2];
 	__le32 crc32;
 } __packed;
 static_assert(sizeof(struct dualshock4_input_report_bt) == DS4_INPUT_REPORT_BT_SIZE);
 
 /* Common data between Bluetooth and USB DualShock4 output reports. */
 struct dualshock4_output_report_common {
-	uint8_t valid_flag0;
-	uint8_t valid_flag1;
+	u8 valid_flag0;
+	u8 valid_flag1;
 
-	uint8_t reserved;
+	u8 reserved;
 
-	uint8_t motor_right;
-	uint8_t motor_left;
+	u8 motor_right;
+	u8 motor_left;
 
-	uint8_t lightbar_red;
-	uint8_t lightbar_green;
-	uint8_t lightbar_blue;
-	uint8_t lightbar_blink_on;
-	uint8_t lightbar_blink_off;
+	u8 lightbar_red;
+	u8 lightbar_green;
+	u8 lightbar_blue;
+	u8 lightbar_blink_on;
+	u8 lightbar_blink_off;
 } __packed;
 
 struct dualshock4_output_report_usb {
-	uint8_t report_id; /* 0x5 */
+	u8 report_id; /* 0x5 */
 	struct dualshock4_output_report_common common;
-	uint8_t reserved[21];
+	u8 reserved[21];
 } __packed;
 static_assert(sizeof(struct dualshock4_output_report_usb) == DS4_OUTPUT_REPORT_USB_SIZE);
 
 struct dualshock4_output_report_bt {
-	uint8_t report_id; /* 0x11 */
-	uint8_t hw_control;
-	uint8_t audio_control;
+	u8 report_id; /* 0x11 */
+	u8 hw_control;
+	u8 audio_control;
 	struct dualshock4_output_report_common common;
-	uint8_t reserved[61];
+	u8 reserved[61];
 	__le32 crc32;
 } __packed;
 static_assert(sizeof(struct dualshock4_output_report_bt) == DS4_OUTPUT_REPORT_BT_SIZE);
@@ -516,8 +516,8 @@ static_assert(sizeof(struct dualshock4_output_report_bt) == DS4_OUTPUT_REPORT_BT
  * This structure hide the differences between the two to simplify sending output reports.
  */
 struct dualshock4_output_report {
-	uint8_t *data; /* Start of data */
-	uint8_t len; /* Size of output report */
+	u8 *data; /* Start of data */
+	u8 len; /* Size of output report */
 
 	/* Points to Bluetooth data payload in case for a Bluetooth report else NULL. */
 	struct dualshock4_output_report_bt *bt;
@@ -556,7 +556,7 @@ static const struct {int x; int y; } ps_gamepad_hat_mapping[] = {
 static int dualshock4_get_calibration_data(struct dualshock4 *ds4);
 static inline void dualsense_schedule_work(struct dualsense *ds);
 static inline void dualshock4_schedule_work(struct dualshock4 *ds4);
-static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue);
+static void dualsense_set_lightbar(struct dualsense *ds, u8 red, u8 green, u8 blue);
 static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4);
 
 /*
@@ -648,7 +648,7 @@ static int ps_battery_get_property(struct power_supply *psy,
 		union power_supply_propval *val)
 {
 	struct ps_device *dev = power_supply_get_drvdata(psy);
-	uint8_t battery_capacity;
+	u8 battery_capacity;
 	int battery_status;
 	int ret = 0;
 
@@ -711,9 +711,9 @@ static int ps_device_register_battery(struct ps_device *dev)
 }
 
 /* Compute crc32 of HID data and compare against expected CRC. */
-static bool ps_check_crc32(uint8_t seed, uint8_t *data, size_t len, uint32_t report_crc)
+static bool ps_check_crc32(u8 seed, u8 *data, size_t len, u32 report_crc)
 {
-	uint32_t crc;
+	u32 crc;
 
 	crc = crc32_le(0xFFFFFFFF, &seed, 1);
 	crc = ~crc32_le(crc, data, len);
@@ -759,7 +759,7 @@ static struct input_dev *ps_gamepad_create(struct hid_device *hdev,
 	return gamepad;
 }
 
-static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *buf, size_t size,
+static int ps_get_report(struct hid_device *hdev, u8 report_id, u8 *buf, size_t size,
 		bool check_crc)
 {
 	int ret;
@@ -783,8 +783,8 @@ static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *bu
 
 	if (hdev->bus == BUS_BLUETOOTH && check_crc) {
 		/* Last 4 bytes contains crc32. */
-		uint8_t crc_offset = size - 4;
-		uint32_t report_crc = get_unaligned_le32(&buf[crc_offset]);
+		u8 crc_offset = size - 4;
+		u32 report_crc = get_unaligned_le32(&buf[crc_offset]);
 
 		if (!ps_check_crc32(PS_FEATURE_CRC32_SEED, buf, crc_offset, report_crc)) {
 			hid_err(hdev, "CRC check failed for reportID=%d\n", report_id);
@@ -977,7 +977,7 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 	int range_2g;
 	int ret = 0;
 	int i;
-	uint8_t *buf;
+	u8 *buf;
 
 	buf = kzalloc(DS_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
 	if (!buf)
@@ -1091,7 +1091,7 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 
 static int dualsense_get_firmware_info(struct dualsense *ds)
 {
-	uint8_t *buf;
+	u8 *buf;
 	int ret;
 
 	buf = kzalloc(DS_FEATURE_REPORT_FIRMWARE_INFO_SIZE, GFP_KERNEL);
@@ -1124,7 +1124,7 @@ static int dualsense_get_firmware_info(struct dualsense *ds)
 
 static int dualsense_get_mac_address(struct dualsense *ds)
 {
-	uint8_t *buf;
+	u8 *buf;
 	int ret = 0;
 
 	buf = kzalloc(DS_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL);
@@ -1150,7 +1150,7 @@ static int dualsense_lightbar_set_brightness(struct led_classdev *cdev,
 {
 	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
 	struct dualsense *ds = container_of(mc_cdev, struct dualsense, lightbar);
-	uint8_t red, green, blue;
+	u8 red, green, blue;
 
 	led_mc_calc_color_components(mc_cdev, brightness);
 	red = mc_cdev->subled_info[0].brightness;
@@ -1248,8 +1248,8 @@ static void dualsense_send_output_report(struct dualsense *ds,
 
 	/* Bluetooth packets need to be signed with a CRC in the last 4 bytes. */
 	if (report->bt) {
-		uint32_t crc;
-		uint8_t seed = PS_OUTPUT_CRC32_SEED;
+		u32 crc;
+		u8 seed = PS_OUTPUT_CRC32_SEED;
 
 		crc = crc32_le(0xFFFFFFFF, &seed, 1);
 		crc = ~crc32_le(crc, report->data, report->len - 4);
@@ -1328,9 +1328,9 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 	struct hid_device *hdev = ps_dev->hdev;
 	struct dualsense *ds = container_of(ps_dev, struct dualsense, base);
 	struct dualsense_input_report *ds_report;
-	uint8_t battery_data, battery_capacity, charging_status, value;
+	u8 battery_data, battery_capacity, charging_status, value;
 	int battery_status;
-	uint32_t sensor_timestamp;
+	u32 sensor_timestamp;
 	bool btn_mic_state;
 	int i;
 
@@ -1345,7 +1345,7 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 	} else if (hdev->bus == BUS_BLUETOOTH && report->id == DS_INPUT_REPORT_BT &&
 			size == DS_INPUT_REPORT_BT_SIZE) {
 		/* Last 4 bytes of input report contain crc32 */
-		uint32_t report_crc = get_unaligned_le32(&data[size - 4]);
+		u32 report_crc = get_unaligned_le32(&data[size - 4]);
 
 		if (!ps_check_crc32(PS_INPUT_CRC32_SEED, data, size - 4, report_crc)) {
 			hid_err(hdev, "DualSense input CRC's check failed\n");
@@ -1428,7 +1428,7 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 		ds->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp, 3);
 		ds->sensor_timestamp_initialized = true;
 	} else {
-		uint32_t delta;
+		u32 delta;
 
 		if (ds->prev_sensor_timestamp > sensor_timestamp)
 			delta = (U32_MAX - ds->prev_sensor_timestamp + sensor_timestamp + 1);
@@ -1528,7 +1528,7 @@ static void dualsense_remove(struct ps_device *ps_dev)
 static int dualsense_reset_leds(struct dualsense *ds)
 {
 	struct dualsense_output_report report;
-	uint8_t *buf;
+	u8 *buf;
 
 	buf = kzalloc(sizeof(struct dualsense_output_report_bt), GFP_KERNEL);
 	if (!buf)
@@ -1550,7 +1550,7 @@ static int dualsense_reset_leds(struct dualsense *ds)
 	return 0;
 }
 
-static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue)
+static void dualsense_set_lightbar(struct dualsense *ds, u8 red, u8 green, u8 blue)
 {
 	scoped_guard(spinlock_irqsave, &ds->base.lock) {
 		ds->update_lightbar = true;
@@ -1578,7 +1578,7 @@ static void dualsense_set_player_leds(struct dualsense *ds)
 		BIT(4) | BIT(3) | BIT(2) | BIT(1) | BIT(0)
 	};
 
-	uint8_t player_id = ds->base.player_id % ARRAY_SIZE(player_ids);
+	u8 player_id = ds->base.player_id % ARRAY_SIZE(player_ids);
 
 	ds->update_player_leds = true;
 	ds->player_leds_state = player_ids[player_id];
@@ -1589,7 +1589,7 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 {
 	struct dualsense *ds;
 	struct ps_device *ps_dev;
-	uint8_t max_output_report_size;
+	u8 max_output_report_size;
 	int i, ret;
 
 	static const struct ps_led_info player_leds_info[] = {
@@ -1780,7 +1780,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	int range_2g;
 	int ret = 0;
 	int i;
-	uint8_t *buf;
+	u8 *buf;
 
 	if (ds4->base.hdev->bus == BUS_USB) {
 		int retries;
@@ -1944,7 +1944,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 
 static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
 {
-	uint8_t *buf;
+	u8 *buf;
 	int ret;
 
 	buf = kzalloc(DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, GFP_KERNEL);
@@ -1972,7 +1972,7 @@ static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
 static int dualshock4_get_mac_address(struct dualshock4 *ds4)
 {
 	struct hid_device *hdev = ds4->base.hdev;
-	uint8_t *buf;
+	u8 *buf;
 	int ret = 0;
 
 	if (hdev->bus == BUS_USB) {
@@ -2191,8 +2191,8 @@ static void dualshock4_output_worker(struct work_struct *work)
 
 	/* Bluetooth packets need additional flags as well as a CRC in the last 4 bytes. */
 	if (report.bt) {
-		uint32_t crc;
-		uint8_t seed = PS_OUTPUT_CRC32_SEED;
+		u32 crc;
+		u8 seed = PS_OUTPUT_CRC32_SEED;
 
 		/* Hardware control flags need to set to let the device know
 		 * there is HID data as well as CRC.
@@ -2220,9 +2220,9 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
 	struct dualshock4_input_report_common *ds4_report;
 	struct dualshock4_touch_report *touch_reports;
-	uint8_t battery_capacity, num_touch_reports, value;
+	u8 battery_capacity, num_touch_reports, value;
 	int battery_status, i, j;
-	uint16_t sensor_timestamp;
+	u16 sensor_timestamp;
 	bool is_minimal = false;
 
 	/*
@@ -2240,7 +2240,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	} else if (hdev->bus == BUS_BLUETOOTH && report->id == DS4_INPUT_REPORT_BT &&
 			size == DS4_INPUT_REPORT_BT_SIZE) {
 		struct dualshock4_input_report_bt *bt = (struct dualshock4_input_report_bt *)data;
-		uint32_t report_crc = get_unaligned_le32(&bt->crc32);
+		u32 report_crc = get_unaligned_le32(&bt->crc32);
 
 		/* Last 4 bytes of input report contains CRC. */
 		if (!ps_check_crc32(PS_INPUT_CRC32_SEED, data, size - 4, report_crc)) {
@@ -2324,7 +2324,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 		ds4->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp * 16, 3);
 		ds4->sensor_timestamp_initialized = true;
 	} else {
-		uint16_t delta;
+		u16 delta;
 
 		if (ds4->prev_sensor_timestamp > sensor_timestamp)
 			delta = (U16_MAX - ds4->prev_sensor_timestamp + sensor_timestamp + 1);
@@ -2369,7 +2369,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	 * - 15: charge error
 	 */
 	if (ds4_report->status[0] & DS4_STATUS0_CABLE_STATE) {
-		uint8_t battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
+		u8 battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
 
 		if (battery_data < 10) {
 			/* Take the mid-point for each battery capacity value,
@@ -2390,7 +2390,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 			battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
 		}
 	} else {
-		uint8_t battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
+		u8 battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
 
 		if (battery_data < 10)
 			battery_capacity = battery_data * 10 + 5;
@@ -2499,7 +2499,7 @@ static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
 		schedule_work(&ds4->output_worker);
 }
 
-static void dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, uint8_t interval)
+static void dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, u8 interval)
 {
 	ds4->bt_poll_interval = interval;
 	ds4->update_bt_poll_interval = true;
@@ -2519,7 +2519,7 @@ static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4)
 		{ 0x20, 0x00, 0x20 }  /* Pink */
 	};
 
-	uint8_t player_id = ds4->base.player_id % ARRAY_SIZE(player_colors);
+	u8 player_id = ds4->base.player_id % ARRAY_SIZE(player_colors);
 
 	ds4->lightbar_enabled = true;
 	ds4->lightbar_red = player_colors[player_id][0];
@@ -2534,7 +2534,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 {
 	struct dualshock4 *ds4;
 	struct ps_device *ps_dev;
-	uint8_t max_output_report_size;
+	u8 max_output_report_size;
 	int i, ret;
 
 	/* The DualShock4 has an RGB lightbar, which the original hid-sony driver

-- 
2.49.0

[PATCH 05/11] HID: playstation: Correct spelling in comment sections

[Cristian Ciocaltea]

Handle a couple of spelling complaints from checkpatch.pl:

  CHECK: 'connectd' may be misspelled - perhaps 'connected'?
  CHECK: 'Comptabile' may be misspelled - perhaps 'Compatible'?

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index d36370d001edd0d82e8616f8f2803d8b7ad13954..837d6917b1002b80474b4e0d6241c455708294c8 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -339,7 +339,7 @@ struct dualsense_output_report {
 /* Battery status within batery_status field. */
 #define DS4_BATTERY_STATUS_FULL		11
 /* Status1 bit2 contains dongle connection state:
- * 0 = connectd
+ * 0 = connected
  * 1 = disconnected
  */
 #define DS4_STATUS1_DONGLE_STATE	BIT(2)
@@ -2170,7 +2170,7 @@ static void dualshock4_output_worker(struct work_struct *work)
 
 	if (ds4->update_lightbar) {
 		common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_LED;
-		/* Comptabile behavior with hid-sony, which used a dummy global LED to
+		/* Compatible behavior with hid-sony, which used a dummy global LED to
 		 * allow enabling/disabling the lightbar. The global LED maps to
 		 * lightbar_enabled.
 		 */

-- 
2.49.0

[PATCH 06/11] HID: playstation: Fix all alignment and line length issues

[Cristian Ciocaltea]

Format code to address the following checkpatch.pl reports:

  CHECK: Alignment should match open parenthesis
  CHECK: line length of <N> exceeds 100 columns
  CHECK: Please don't use multiple blank lines

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 163 +++++++++++++++++++++++-------------------
 1 file changed, 89 insertions(+), 74 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 837d6917b1002b80474b4e0d6241c455708294c8..d8ef26ce41b24bc78a897708680136da52c01fa3 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -573,7 +573,7 @@ static int ps_devices_list_add(struct ps_device *dev)
 	list_for_each_entry(entry, &ps_devices_list, list) {
 		if (!memcmp(entry->mac_address, dev->mac_address, sizeof(dev->mac_address))) {
 			hid_err(dev->hdev, "Duplicate device found for MAC address %pMR.\n",
-					dev->mac_address);
+				dev->mac_address);
 			return -EEXIST;
 		}
 	}
@@ -608,7 +608,8 @@ static void ps_device_release_player_id(struct ps_device *dev)
 	dev->player_id = U32_MAX;
 }
 
-static struct input_dev *ps_allocate_input_dev(struct hid_device *hdev, const char *name_suffix)
+static struct input_dev *ps_allocate_input_dev(struct hid_device *hdev,
+					       const char *name_suffix)
 {
 	struct input_dev *input_dev;
 
@@ -623,8 +624,8 @@ static struct input_dev *ps_allocate_input_dev(struct hid_device *hdev, const ch
 	input_dev->uniq = hdev->uniq;
 
 	if (name_suffix) {
-		input_dev->name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s %s", hdev->name,
-				name_suffix);
+		input_dev->name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s %s",
+						 hdev->name, name_suffix);
 		if (!input_dev->name)
 			return ERR_PTR(-ENOMEM);
 	} else {
@@ -644,8 +645,8 @@ static enum power_supply_property ps_power_supply_props[] = {
 };
 
 static int ps_battery_get_property(struct power_supply *psy,
-		enum power_supply_property psp,
-		union power_supply_propval *val)
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
 {
 	struct ps_device *dev = power_supply_get_drvdata(psy);
 	u8 battery_capacity;
@@ -689,7 +690,7 @@ static int ps_device_register_battery(struct ps_device *dev)
 	dev->battery_desc.num_properties = ARRAY_SIZE(ps_power_supply_props);
 	dev->battery_desc.get_property = ps_battery_get_property;
 	dev->battery_desc.name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
-			"ps-controller-battery-%pMR", dev->mac_address);
+						"ps-controller-battery-%pMR", dev->mac_address);
 	if (!dev->battery_desc.name)
 		return -ENOMEM;
 
@@ -721,8 +722,9 @@ static bool ps_check_crc32(u8 seed, u8 *data, size_t len, u32 report_crc)
 	return crc == report_crc;
 }
 
-static struct input_dev *ps_gamepad_create(struct hid_device *hdev,
-		int (*play_effect)(struct input_dev *, void *, struct ff_effect *))
+static struct input_dev *
+ps_gamepad_create(struct hid_device *hdev,
+		  int (*play_effect)(struct input_dev *, void *, struct ff_effect *))
 {
 	struct input_dev *gamepad;
 	unsigned int i;
@@ -759,8 +761,8 @@ static struct input_dev *ps_gamepad_create(struct hid_device *hdev,
 	return gamepad;
 }
 
-static int ps_get_report(struct hid_device *hdev, u8 report_id, u8 *buf, size_t size,
-		bool check_crc)
+static int ps_get_report(struct hid_device *hdev, u8 report_id, u8 *buf,
+			 size_t size, bool check_crc)
 {
 	int ret;
 
@@ -796,17 +798,20 @@ static int ps_get_report(struct hid_device *hdev, u8 report_id, u8 *buf, size_t
 }
 
 static int ps_led_register(struct ps_device *ps_dev, struct led_classdev *led,
-		const struct ps_led_info *led_info)
+			   const struct ps_led_info *led_info)
 {
 	int ret;
 
 	if (led_info->name) {
-		led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL,
-				"%s:%s:%s", ps_dev->input_dev_name, led_info->color, led_info->name);
+		led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL, "%s:%s:%s",
+					   ps_dev->input_dev_name, led_info->color,
+					   led_info->name);
 	} else {
-		/* Backwards compatible mode for hid-sony, but not compliant with LED class spec. */
-		led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL,
-				"%s:%s", ps_dev->input_dev_name, led_info->color);
+		/* Backwards compatible mode for hid-sony, but not compliant
+		 * with LED class spec.
+		 */
+		led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL, "%s:%s",
+					   ps_dev->input_dev_name, led_info->color);
 	}
 
 	if (!led->name)
@@ -830,7 +835,7 @@ static int ps_led_register(struct ps_device *ps_dev, struct led_classdev *led,
 
 /* Register a DualSense/DualShock4 RGB lightbar represented by a multicolor LED. */
 static int ps_lightbar_register(struct ps_device *ps_dev, struct led_classdev_mc *lightbar_mc_dev,
-	int (*brightness_set)(struct led_classdev *, enum led_brightness))
+				int (*brightness_set)(struct led_classdev *, enum led_brightness))
 {
 	struct hid_device *hdev = ps_dev->hdev;
 	struct mc_subled *mc_led_info;
@@ -851,7 +856,7 @@ static int ps_lightbar_register(struct ps_device *ps_dev, struct led_classdev_mc
 
 	led_cdev = &lightbar_mc_dev->led_cdev;
 	led_cdev->name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s:rgb:indicator",
-			ps_dev->input_dev_name);
+					ps_dev->input_dev_name);
 	if (!led_cdev->name)
 		return -ENOMEM;
 	led_cdev->brightness = 255;
@@ -867,8 +872,8 @@ static int ps_lightbar_register(struct ps_device *ps_dev, struct led_classdev_mc
 	return 0;
 }
 
-static struct input_dev *ps_sensors_create(struct hid_device *hdev, int accel_range, int accel_res,
-		int gyro_range, int gyro_res)
+static struct input_dev *ps_sensors_create(struct hid_device *hdev, int accel_range,
+					   int accel_res, int gyro_range, int gyro_res)
 {
 	struct input_dev *sensors;
 	int ret;
@@ -904,8 +909,8 @@ static struct input_dev *ps_sensors_create(struct hid_device *hdev, int accel_ra
 	return sensors;
 }
 
-static struct input_dev *ps_touchpad_create(struct hid_device *hdev, int width, int height,
-		unsigned int num_contacts)
+static struct input_dev *ps_touchpad_create(struct hid_device *hdev, int width,
+					    int height, unsigned int num_contacts)
 {
 	struct input_dev *touchpad;
 	int ret;
@@ -933,8 +938,7 @@ static struct input_dev *ps_touchpad_create(struct hid_device *hdev, int width,
 }
 
 static ssize_t firmware_version_show(struct device *dev,
-				struct device_attribute
-				*attr, char *buf)
+				     struct device_attribute *attr, char *buf)
 {
 	struct hid_device *hdev = to_hid_device(dev);
 	struct ps_device *ps_dev = hid_get_drvdata(hdev);
@@ -945,8 +949,7 @@ static ssize_t firmware_version_show(struct device *dev,
 static DEVICE_ATTR_RO(firmware_version);
 
 static ssize_t hardware_version_show(struct device *dev,
-				struct device_attribute
-				*attr, char *buf)
+				     struct device_attribute *attr, char *buf)
 {
 	struct hid_device *hdev = to_hid_device(dev);
 	struct ps_device *ps_dev = hid_get_drvdata(hdev);
@@ -984,7 +987,7 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 		return -ENOMEM;
 
 	ret = ps_get_report(ds->base.hdev, DS_FEATURE_REPORT_CALIBRATION, buf,
-			DS_FEATURE_REPORT_CALIBRATION_SIZE, true);
+			    DS_FEATURE_REPORT_CALIBRATION_SIZE, true);
 	if (ret) {
 		hid_err(ds->base.hdev, "Failed to retrieve DualSense calibration info: %d\n", ret);
 		goto err_free;
@@ -1038,8 +1041,9 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 	 */
 	for (i = 0; i < ARRAY_SIZE(ds->gyro_calib_data); i++) {
 		if (ds->gyro_calib_data[i].sens_denom == 0) {
-			hid_warn(hdev, "Invalid gyro calibration data for axis (%d), disabling calibration.",
-					ds->gyro_calib_data[i].abs_code);
+			hid_warn(hdev,
+				 "Invalid gyro calibration data for axis (%d), disabling calibration.",
+				 ds->gyro_calib_data[i].abs_code);
 			ds->gyro_calib_data[i].bias = 0;
 			ds->gyro_calib_data[i].sens_numer = DS_GYRO_RANGE;
 			ds->gyro_calib_data[i].sens_denom = S16_MAX;
@@ -1075,8 +1079,9 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 	 */
 	for (i = 0; i < ARRAY_SIZE(ds->accel_calib_data); i++) {
 		if (ds->accel_calib_data[i].sens_denom == 0) {
-			hid_warn(hdev, "Invalid accelerometer calibration data for axis (%d), disabling calibration.",
-					ds->accel_calib_data[i].abs_code);
+			hid_warn(hdev,
+				 "Invalid accelerometer calibration data for axis (%d), disabling calibration.",
+				 ds->accel_calib_data[i].abs_code);
 			ds->accel_calib_data[i].bias = 0;
 			ds->accel_calib_data[i].sens_numer = DS_ACC_RANGE;
 			ds->accel_calib_data[i].sens_denom = S16_MAX;
@@ -1088,7 +1093,6 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 	return ret;
 }
 
-
 static int dualsense_get_firmware_info(struct dualsense *ds)
 {
 	u8 *buf;
@@ -1099,7 +1103,7 @@ static int dualsense_get_firmware_info(struct dualsense *ds)
 		return -ENOMEM;
 
 	ret = ps_get_report(ds->base.hdev, DS_FEATURE_REPORT_FIRMWARE_INFO, buf,
-			DS_FEATURE_REPORT_FIRMWARE_INFO_SIZE, true);
+			    DS_FEATURE_REPORT_FIRMWARE_INFO_SIZE, true);
 	if (ret) {
 		hid_err(ds->base.hdev, "Failed to retrieve DualSense firmware info: %d\n", ret);
 		goto err_free;
@@ -1132,7 +1136,7 @@ static int dualsense_get_mac_address(struct dualsense *ds)
 		return -ENOMEM;
 
 	ret = ps_get_report(ds->base.hdev, DS_FEATURE_REPORT_PAIRING_INFO, buf,
-			DS_FEATURE_REPORT_PAIRING_INFO_SIZE, true);
+			    DS_FEATURE_REPORT_PAIRING_INFO_SIZE, true);
 	if (ret) {
 		hid_err(ds->base.hdev, "Failed to retrieve DualSense pairing info: %d\n", ret);
 		goto err_free;
@@ -1146,7 +1150,7 @@ static int dualsense_get_mac_address(struct dualsense *ds)
 }
 
 static int dualsense_lightbar_set_brightness(struct led_classdev *cdev,
-	enum led_brightness brightness)
+					     enum led_brightness brightness)
 {
 	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
 	struct dualsense *ds = container_of(mc_cdev, struct dualsense, lightbar);
@@ -1190,8 +1194,8 @@ static int dualsense_player_led_set_brightness(struct led_classdev *led, enum le
 	return 0;
 }
 
-static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_output_report *rp,
-		void *buf)
+static void dualsense_init_output_report(struct dualsense *ds,
+					 struct dualsense_output_report *rp, void *buf)
 {
 	struct hid_device *hdev = ds->base.hdev;
 
@@ -1242,7 +1246,7 @@ static inline void dualsense_schedule_work(struct dualsense *ds)
  * for Bluetooth reports.
  */
 static void dualsense_send_output_report(struct dualsense *ds,
-		struct dualsense_output_report *report)
+					 struct dualsense_output_report *report)
 {
 	struct hid_device *hdev = ds->base.hdev;
 
@@ -1323,7 +1327,7 @@ static void dualsense_output_worker(struct work_struct *work)
 }
 
 static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *report,
-		u8 *data, int size)
+				  u8 *data, int size)
 {
 	struct hid_device *hdev = ps_dev->hdev;
 	struct dualsense *ds = container_of(ps_dev, struct dualsense, base);
@@ -1340,10 +1344,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 	 * the full report using reportID 49.
 	 */
 	if (hdev->bus == BUS_USB && report->id == DS_INPUT_REPORT_USB &&
-			size == DS_INPUT_REPORT_USB_SIZE) {
+	    size == DS_INPUT_REPORT_USB_SIZE) {
 		ds_report = (struct dualsense_input_report *)&data[1];
 	} else if (hdev->bus == BUS_BLUETOOTH && report->id == DS_INPUT_REPORT_BT &&
-			size == DS_INPUT_REPORT_BT_SIZE) {
+		   size == DS_INPUT_REPORT_BT_SIZE) {
 		/* Last 4 bytes of input report contain crc32 */
 		u32 report_crc = get_unaligned_le32(&data[size - 4]);
 
@@ -1678,7 +1682,7 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	ps_dev->input_dev_name = dev_name(&ds->gamepad->dev);
 
 	ds->sensors = ps_sensors_create(hdev, DS_ACC_RANGE, DS_ACC_RES_PER_G,
-			DS_GYRO_RANGE, DS_GYRO_RES_PER_DEG_S);
+					DS_GYRO_RANGE, DS_GYRO_RES_PER_DEG_S);
 	if (IS_ERR(ds->sensors)) {
 		ret = PTR_ERR(ds->sensors);
 		goto err;
@@ -1732,7 +1736,7 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	 * can change behavior.
 	 */
 	hid_info(hdev, "Registered DualSense controller hw_version=0x%08x fw_version=0x%08x\n",
-			ds->base.hw_version, ds->base.fw_version);
+		 ds->base.hw_version, ds->base.fw_version);
 
 	return &ds->base;
 
@@ -1755,7 +1759,8 @@ static void dualshock4_dongle_calibration_work(struct work_struct *work)
 		 * DS4 hotplug is detect from sony_raw_event as any issues
 		 * are likely resolved then (the dongle is quite stupid).
 		 */
-		hid_err(ds4->base.hdev, "DualShock 4 USB dongle: calibration failed, disabling device\n");
+		hid_err(ds4->base.hdev,
+			"DualShock 4 USB dongle: calibration failed, disabling device\n");
 		dongle_state = DONGLE_DISABLED;
 	} else {
 		hid_info(ds4->base.hdev, "DualShock 4 USB dongle: calibration completed\n");
@@ -1799,14 +1804,17 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 		 */
 		for (retries = 0; retries < 3; retries++) {
 			ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf,
-					DS4_FEATURE_REPORT_CALIBRATION_SIZE, true);
+					    DS4_FEATURE_REPORT_CALIBRATION_SIZE, true);
 			if (ret) {
 				if (retries < 2) {
-					hid_warn(hdev, "Retrying DualShock 4 get calibration report (0x02) request\n");
+					hid_warn(hdev,
+						 "Retrying DualShock 4 get calibration report (0x02) request\n");
 					continue;
 				}
 
-				hid_warn(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
+				hid_warn(hdev,
+					 "Failed to retrieve DualShock4 calibration info: %d\n",
+					 ret);
 				ret = -EILSEQ;
 				goto transfer_failed;
 			} else {
@@ -1821,7 +1829,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 		}
 
 		ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION_BT, buf,
-				DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, true);
+				    DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, true);
 
 		if (ret) {
 			hid_warn(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
@@ -1915,8 +1923,9 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	for (i = 0; i < ARRAY_SIZE(ds4->gyro_calib_data); i++) {
 		if (ds4->gyro_calib_data[i].sens_denom == 0) {
 			ds4->gyro_calib_data[i].abs_code = ABS_RX + i;
-			hid_warn(hdev, "Invalid gyro calibration data for axis (%d), disabling calibration.",
-					ds4->gyro_calib_data[i].abs_code);
+			hid_warn(hdev,
+				 "Invalid gyro calibration data for axis (%d), disabling calibration.",
+				 ds4->gyro_calib_data[i].abs_code);
 			ds4->gyro_calib_data[i].bias = 0;
 			ds4->gyro_calib_data[i].sens_numer = DS4_GYRO_RANGE;
 			ds4->gyro_calib_data[i].sens_denom = S16_MAX;
@@ -1931,8 +1940,9 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	for (i = 0; i < ARRAY_SIZE(ds4->accel_calib_data); i++) {
 		if (ds4->accel_calib_data[i].sens_denom == 0) {
 			ds4->accel_calib_data[i].abs_code = ABS_X + i;
-			hid_warn(hdev, "Invalid accelerometer calibration data for axis (%d), disabling calibration.",
-					ds4->accel_calib_data[i].abs_code);
+			hid_warn(hdev,
+				 "Invalid accelerometer calibration data for axis (%d), disabling calibration.",
+				 ds4->accel_calib_data[i].abs_code);
 			ds4->accel_calib_data[i].bias = 0;
 			ds4->accel_calib_data[i].sens_numer = DS4_ACC_RANGE;
 			ds4->accel_calib_data[i].sens_denom = S16_MAX;
@@ -1955,7 +1965,7 @@ static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
 	 * lacks CRC support, so must be disabled in ps_get_report.
 	 */
 	ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_FIRMWARE_INFO, buf,
-			DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, false);
+			    DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, false);
 	if (ret) {
 		hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 firmware info: %d\n", ret);
 		goto err_free;
@@ -1981,7 +1991,7 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4)
 			return -ENOMEM;
 
 		ret = ps_get_report(hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf,
-				DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, false);
+				    DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, false);
 		if (ret) {
 			hid_err(hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret);
 			goto err_free;
@@ -1994,9 +2004,9 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4)
 			return -EINVAL;
 
 		ret = sscanf(hdev->uniq, "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
-				&ds4->base.mac_address[5], &ds4->base.mac_address[4],
-				&ds4->base.mac_address[3], &ds4->base.mac_address[2],
-				&ds4->base.mac_address[1], &ds4->base.mac_address[0]);
+			     &ds4->base.mac_address[5], &ds4->base.mac_address[4],
+			     &ds4->base.mac_address[3], &ds4->base.mac_address[2],
+			     &ds4->base.mac_address[1], &ds4->base.mac_address[0]);
 
 		if (ret != sizeof(ds4->base.mac_address))
 			return -EINVAL;
@@ -2031,7 +2041,7 @@ static enum led_brightness dualshock4_led_get_brightness(struct led_classdev *le
 }
 
 static int dualshock4_led_set_blink(struct led_classdev *led, unsigned long *delay_on,
-		unsigned long *delay_off)
+				    unsigned long *delay_off)
 {
 	struct hid_device *hdev = to_hid_device(led->dev->parent);
 	struct dualshock4 *ds4 = hid_get_drvdata(hdev);
@@ -2099,7 +2109,7 @@ static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brig
 }
 
 static void dualshock4_init_output_report(struct dualshock4 *ds4,
-		struct dualshock4_output_report *rp, void *buf)
+					  struct dualshock4_output_report *rp, void *buf)
 {
 	struct hid_device *hdev = ds4->base.hdev;
 
@@ -2214,7 +2224,7 @@ static void dualshock4_output_worker(struct work_struct *work)
 }
 
 static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *report,
-		u8 *data, int size)
+				   u8 *data, int size)
 {
 	struct hid_device *hdev = ps_dev->hdev;
 	struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
@@ -2231,14 +2241,15 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	 * the full report using reportID 17.
 	 */
 	if (hdev->bus == BUS_USB && report->id == DS4_INPUT_REPORT_USB &&
-			size == DS4_INPUT_REPORT_USB_SIZE) {
-		struct dualshock4_input_report_usb *usb = (struct dualshock4_input_report_usb *)data;
+	    size == DS4_INPUT_REPORT_USB_SIZE) {
+		struct dualshock4_input_report_usb *usb =
+			(struct dualshock4_input_report_usb *)data;
 
 		ds4_report = &usb->common;
 		num_touch_reports = usb->num_touch_reports;
 		touch_reports = usb->touch_reports;
 	} else if (hdev->bus == BUS_BLUETOOTH && report->id == DS4_INPUT_REPORT_BT &&
-			size == DS4_INPUT_REPORT_BT_SIZE) {
+		   size == DS4_INPUT_REPORT_BT_SIZE) {
 		struct dualshock4_input_report_bt *bt = (struct dualshock4_input_report_bt *)data;
 		u32 report_crc = get_unaligned_le32(&bt->crc32);
 
@@ -2409,7 +2420,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 }
 
 static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_report *report,
-		u8 *data, int size)
+					  u8 *data, int size)
 {
 	struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
 	bool connected = false;
@@ -2420,7 +2431,8 @@ static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_r
 	 * parsing code.
 	 */
 	if (data[0] == DS4_INPUT_REPORT_USB && size == DS4_INPUT_REPORT_USB_SIZE) {
-		struct dualshock4_input_report_common *ds4_report = (struct dualshock4_input_report_common *)&data[1];
+		struct dualshock4_input_report_common *ds4_report =
+			(struct dualshock4_input_report_common *)&data[1];
 
 		connected = ds4_report->status[1] & DS4_STATUS1_DONGLE_STATE ? false : true;
 
@@ -2547,11 +2559,14 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 	 * existing applications (e.g. Android). Nothing matches against MAC address.
 	 */
 	static const struct ps_led_info lightbar_leds_info[] = {
-		{ NULL, "red", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
-		{ NULL, "green", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
-		{ NULL, "blue", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
-		{ NULL, "global", 1, dualshock4_led_get_brightness, dualshock4_led_set_brightness,
-				dualshock4_led_set_blink },
+		{ NULL, "red", 255, dualshock4_led_get_brightness,
+		  dualshock4_led_set_brightness },
+		{ NULL, "green", 255, dualshock4_led_get_brightness,
+		  dualshock4_led_set_brightness },
+		{ NULL, "blue", 255, dualshock4_led_get_brightness,
+		  dualshock4_led_set_brightness },
+		{ NULL, "global", 1, dualshock4_led_get_brightness,
+		  dualshock4_led_set_brightness, dualshock4_led_set_blink },
 	};
 
 	ds4 = devm_kzalloc(&hdev->dev, sizeof(*ds4), GFP_KERNEL);
@@ -2621,7 +2636,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 	ps_dev->input_dev_name = dev_name(&ds4->gamepad->dev);
 
 	ds4->sensors = ps_sensors_create(hdev, DS4_ACC_RANGE, DS4_ACC_RES_PER_G,
-			DS4_GYRO_RANGE, DS4_GYRO_RES_PER_DEG_S);
+					 DS4_GYRO_RANGE, DS4_GYRO_RES_PER_DEG_S);
 	if (IS_ERR(ds4->sensors)) {
 		ret = PTR_ERR(ds4->sensors);
 		goto err;
@@ -2660,7 +2675,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 	 * can change behavior.
 	 */
 	hid_info(hdev, "Registered DualShock4 controller hw_version=0x%08x fw_version=0x%08x\n",
-			ds4->base.hw_version, ds4->base.fw_version);
+		 ds4->base.hw_version, ds4->base.fw_version);
 	return &ds4->base;
 
 err:
@@ -2669,7 +2684,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 }
 
 static int ps_raw_event(struct hid_device *hdev, struct hid_report *report,
-		u8 *data, int size)
+			u8 *data, int size)
 {
 	struct ps_device *dev = hid_get_drvdata(hdev);
 

-- 
2.49.0

[PATCH 07/11] HID: playstation: Document spinlock_t usage

[Cristian Ciocaltea]

Document the usage of 'lock' member in struct ps_device and silent
checkpatch.pl complaint:

  CHECK: spinlock_t definition without comment

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index d8ef26ce41b24bc78a897708680136da52c01fa3..0671ca39dcf3d9c5cbddad7e46b2da13417a952e 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -38,7 +38,7 @@ enum PS_TYPE {
 struct ps_device {
 	struct list_head list;
 	struct hid_device *hdev;
-	spinlock_t lock;
+	spinlock_t lock; /* Sync between event handler and workqueue */
 
 	u32 player_id;
 

-- 
2.49.0

[PATCH 08/11] HID: playstation: Prefer kzalloc(sizeof(*buf)...)

[Cristian Ciocaltea]

Use the shorter variant as suggested by checkpatch.pl:

  CHECK: Prefer kzalloc(sizeof(*buf)...) over kzalloc(sizeof(struct dualsense_output_report_bt)...)

This also improves further maintainability.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 0671ca39dcf3d9c5cbddad7e46b2da13417a952e..937b14679c8b17c5d3d245eae1cc4e0e56dadb71 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1532,9 +1532,9 @@ static void dualsense_remove(struct ps_device *ps_dev)
 static int dualsense_reset_leds(struct dualsense *ds)
 {
 	struct dualsense_output_report report;
-	u8 *buf;
+	struct dualsense_output_report_bt *buf;
 
-	buf = kzalloc(sizeof(struct dualsense_output_report_bt), GFP_KERNEL);
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 

-- 
2.49.0

[PATCH 09/11] HID: playstation: Rename DualSense input report status field

[Cristian Ciocaltea]

The name of the 'status' member of struct dualsense_input_report is too
generic, since it is only used to provide battery data.

In preparation to support handling additional (non-battery) status
information and avoid ambiguity, rename the field to status_battery.
For consistency, also rename the related DS_STATUS_* bitfield macros.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 937b14679c8b17c5d3d245eae1cc4e0e56dadb71..b7777d3230b2fe277a4a2217ef6b1eff9cfad182 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -112,10 +112,10 @@ struct ps_led_info {
 #define DS_BUTTONS2_TOUCHPAD	BIT(1)
 #define DS_BUTTONS2_MIC_MUTE	BIT(2)
 
-/* Status field of DualSense input report. */
-#define DS_STATUS_BATTERY_CAPACITY	GENMASK(3, 0)
-#define DS_STATUS_CHARGING		GENMASK(7, 4)
-#define DS_STATUS_CHARGING_SHIFT	4
+/* Battery status field of DualSense input report. */
+#define DS_STATUS_BATTERY_CAPACITY		GENMASK(3, 0)
+#define DS_STATUS_BATTERY_CHARGING		GENMASK(7, 4)
+#define DS_STATUS_BATTERY_CHARGING_SHIFT	4
 
 /* Feature version from DualSense Firmware Info report. */
 #define DS_FEATURE_VERSION_MINOR		GENMASK(7, 0)
@@ -236,7 +236,7 @@ struct dualsense_input_report {
 	struct dualsense_touch_point points[2];
 
 	u8 reserved3[12];
-	u8 status;
+	u8 status_battery;
 	u8 reserved4[10];
 } __packed;
 /* Common input report size shared equals the size of the USB report minus 1 byte for ReportID. */
@@ -1462,8 +1462,9 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 	input_report_key(ds->touchpad, BTN_LEFT, ds_report->buttons[2] & DS_BUTTONS2_TOUCHPAD);
 	input_sync(ds->touchpad);
 
-	battery_data = ds_report->status & DS_STATUS_BATTERY_CAPACITY;
-	charging_status = (ds_report->status & DS_STATUS_CHARGING) >> DS_STATUS_CHARGING_SHIFT;
+	battery_data = ds_report->status_battery & DS_STATUS_BATTERY_CAPACITY;
+	charging_status = ds_report->status_battery & DS_STATUS_BATTERY_CHARGING;
+	charging_status >>= DS_STATUS_BATTERY_CHARGING_SHIFT;
 
 	switch (charging_status) {
 	case 0x0:

-- 
2.49.0

[PATCH 10/11] HID: playstation: Support DualSense audio jack hotplug detection

[Cristian Ciocaltea]

The default audio output path on DualSense controller hardware is set to
headphones, regardless of whether they are actually inserted or not.

Detect when the plugged state of the 3.5mm audio jack changes and toggle
audio output between headphones and internal speaker, as required.  The
latter is achieved by essentially routing the right channel of the audio
source to the mono speaker.

Additionally, adjust the speaker volume since its default level is too
low and, therefore, cannot generate any audible sound.

It's worth noting the audio functionality is currently not supported for
Bluetooth, hence it's limited to USB connectivity.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 86 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 83 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index b7777d3230b2fe277a4a2217ef6b1eff9cfad182..ed8e67c5bf5d3a9e9721da31a6bd84f0b6802b14 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -117,6 +117,12 @@ struct ps_led_info {
 #define DS_STATUS_BATTERY_CHARGING		GENMASK(7, 4)
 #define DS_STATUS_BATTERY_CHARGING_SHIFT	4
 
+/* Plugged status field of DualSense input report. */
+#define DS_STATUS_PLUGGED_HEADPHONES		BIT(0)
+#define DS_STATUS_PLUGGED_MIC			BIT(1)
+#define DS_STATUS_PLUGGED_JACK			(DS_STATUS_PLUGGED_HEADPHONES | \
+						 DS_STATUS_PLUGGED_MIC)
+
 /* Feature version from DualSense Firmware Info report. */
 #define DS_FEATURE_VERSION_MINOR		GENMASK(7, 0)
 #define DS_FEATURE_VERSION_MAJOR		GENMASK(15, 8)
@@ -144,13 +150,18 @@ struct ps_led_info {
 /* Flags for DualSense output report. */
 #define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION		BIT(0)
 #define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT			BIT(1)
+#define DS_OUTPUT_VALID_FLAG0_SPEAKER_VOLUME_ENABLE		BIT(5)
+#define DS_OUTPUT_VALID_FLAG0_AUDIO_CONTROL_ENABLE		BIT(7)
 #define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE	BIT(0)
 #define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE		BIT(1)
 #define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE		BIT(2)
 #define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS			BIT(3)
 #define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE	BIT(4)
+#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_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)
 
@@ -193,6 +204,11 @@ struct dualsense {
 	u8 lightbar_green;
 	u8 lightbar_blue;
 
+	/* Audio Jack plugged state */
+	u8 plugged_state;
+	u8 prev_plugged_state;
+	bool prev_plugged_state_valid;
+
 	/* Microphone */
 	bool update_mic_mute;
 	bool mic_muted;
@@ -237,7 +253,8 @@ struct dualsense_input_report {
 
 	u8 reserved3[12];
 	u8 status_battery;
-	u8 reserved4[10];
+	u8 status_plugged;
+	u8 reserved4[9];
 } __packed;
 /* Common input report size shared equals the size of the USB report minus 1 byte for ReportID. */
 static_assert(sizeof(struct dualsense_input_report) == DS_INPUT_REPORT_USB_SIZE - 1);
@@ -252,11 +269,15 @@ struct dualsense_output_report_common {
 	u8 motor_left;
 
 	/* Audio controls */
-	u8 reserved[4];
+	u8 headphone_audio_volume;		/* 0-0x7f */
+	u8 speaker_audio_volume;		/* 0-0xff */
+	u8 internal_microphone_volume;	/* 0-0x40 */
+	u8 audio_flags;
 	u8 mute_button_led;
 
 	u8 power_save_control;
-	u8 reserved2[28];
+	u8 reserved2[27];
+	u8 audio_flags2;
 
 	/* LEDs and lightbar */
 	u8 valid_flag2;
@@ -1304,6 +1325,46 @@ static void dualsense_output_worker(struct work_struct *work)
 		ds->update_player_leds = false;
 	}
 
+	if (ds->plugged_state != ds->prev_plugged_state) {
+		u8 val = ds->plugged_state & DS_STATUS_PLUGGED_HEADPHONES;
+
+		if (val != (ds->prev_plugged_state & DS_STATUS_PLUGGED_HEADPHONES)) {
+			common->valid_flag0 = DS_OUTPUT_VALID_FLAG0_AUDIO_CONTROL_ENABLE;
+			/*
+			 *  _--------> Output path setup in audio_flag0
+			 * /  _------> Headphone (HP) Left channel sink
+			 * | /  _----> Headphone (HP) Right channel sink
+			 * | | /  _--> Internal Speaker (SP) sink
+			 * | | | /
+			 * | | | |     L/R - Left/Right channel source
+			 * 0 L-R X       X - Unrouted (muted) channel source
+			 * 1 L-L X
+			 * 2 L-L R
+			 * 3 X-X R
+			 */
+			if (val) {
+				/* Mute SP and route L+R channels to HP */
+				common->audio_flags = 0;
+			} else {
+				/* Mute HP and route R channel to SP */
+				common->audio_flags =
+					FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS_OUTPUT_PATH_SEL, 0x3);
+				/*
+				 * Set SP hardware volume to 100%.
+				 * Note the accepted range seems to be [0x3d..0x64]
+				 */
+				common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_SPEAKER_VOLUME_ENABLE;
+				common->speaker_audio_volume = 0x64;
+				/* Set SP preamp gain to ~30% */
+				common->valid_flag1 = DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE;
+				common->audio_flags2 =
+					FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN, 0x2);
+			}
+		}
+
+		ds->prev_plugged_state = ds->plugged_state;
+	}
+
 	if (ds->update_mic_mute) {
 		common->valid_flag1 |= DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE;
 		common->mute_button_led = ds->mic_muted;
@@ -1407,6 +1468,25 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 	}
 	ds->last_btn_mic_state = btn_mic_state;
 
+	/* Parse HP/MIC plugged state data */
+	value = ds_report->status_plugged & DS_STATUS_PLUGGED_JACK;
+	if (!ds->prev_plugged_state_valid) {
+		/* Initial handling of the plugged state report */
+		scoped_guard(spinlock_irqsave, &ps_dev->lock) {
+			ds->plugged_state = (~value) & DS_STATUS_PLUGGED_JACK;
+			ds->prev_plugged_state_valid = true;
+		}
+	}
+	if (value != ds->plugged_state) {
+		scoped_guard(spinlock_irqsave, &ps_dev->lock) {
+			ds->prev_plugged_state = ds->plugged_state;
+			ds->plugged_state = value;
+		}
+
+		/* Schedule audio routing towards active endpoint. */
+		dualsense_schedule_work(ds);
+	}
+
 	/* Parse and calibrate gyroscope data. */
 	for (i = 0; i < ARRAY_SIZE(ds_report->gyro); i++) {
 		int raw_data = (short)le16_to_cpu(ds_report->gyro[i]);

-- 
2.49.0

[PATCH 11/11] HID: playstation: Support DualSense audio jack event reporting

[Cristian Ciocaltea]

The DualSense controller complies with v1.0 of the USB Audio Class spec
(UAC1), hence it cannot advertise any jack detection capability.

However, this feature can be implemented in the generic USB audio driver
via quirks, i.e. by configuring an input handler to receive hotplug
events from the HID driver.

Register a dedicated input device for the audio jack and use it to
report all headphone and headset mic insert events.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index ed8e67c5bf5d3a9e9721da31a6bd84f0b6802b14..1231165098691398cd471e71cb9787651f753023 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -178,6 +178,7 @@ struct dualsense {
 	struct input_dev *gamepad;
 	struct input_dev *sensors;
 	struct input_dev *touchpad;
+	struct input_dev *jack;
 
 	/* Update version is used as a feature/capability version. */
 	u16 update_version;
@@ -958,6 +959,25 @@ static struct input_dev *ps_touchpad_create(struct hid_device *hdev, int width,
 	return touchpad;
 }
 
+static struct input_dev *ps_headset_jack_create(struct hid_device *hdev)
+{
+	struct input_dev *jack;
+	int ret;
+
+	jack = ps_allocate_input_dev(hdev, "Headset Jack");
+	if (IS_ERR(jack))
+		return ERR_CAST(jack);
+
+	input_set_capability(jack, EV_SW, SW_HEADPHONE_INSERT);
+	input_set_capability(jack, EV_SW, SW_MICROPHONE_INSERT);
+
+	ret = input_register_device(jack);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return jack;
+}
+
 static ssize_t firmware_version_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
@@ -1360,8 +1380,15 @@ static void dualsense_output_worker(struct work_struct *work)
 				common->audio_flags2 =
 					FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN, 0x2);
 			}
+
+			input_report_switch(ds->jack, SW_HEADPHONE_INSERT, val);
 		}
 
+		val = ds->plugged_state & DS_STATUS_PLUGGED_MIC;
+		if (val != (ds->prev_plugged_state & DS_STATUS_PLUGGED_MIC))
+			input_report_switch(ds->jack, SW_MICROPHONE_INSERT, val);
+
+		input_sync(ds->jack);
 		ds->prev_plugged_state = ds->plugged_state;
 	}
 
@@ -1775,6 +1802,12 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 		goto err;
 	}
 
+	ds->jack = ps_headset_jack_create(hdev);
+	if (IS_ERR(ds->jack)) {
+		ret = PTR_ERR(ds->jack);
+		goto err;
+	}
+
 	ret = ps_device_register_battery(ps_dev);
 	if (ret)
 		goto err;

-- 
2.49.0

Re: [PATCH 00/11] HID: playstation: Add support for audio jack handling on DualSense

[Roderick Colenbrander]

Hi Cristian,

Thanks for sharing your patches around audio. I need to have a closer
look at some of those and how the console also behaves (we try to keep
things in-sync'ish when possible). I need to double check the
datasheets as well.

The series does contain some other patches around style and stuff.
Some of them for me are entering that slippery slope of what to
change. There are some different styles in use around the kernel (e.g.
uint32_t etcetera is fine). But then if you use super strict mode on
checkpatch half the kernel almost needs to be touched. I'm a bit
skeptical on those kind of patches.

Thanks,
Roderick

On Mon, May 26, 2025 at 5:52 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> The Sony DualSense wireless controller (PS5) provides an internal mono
> speaker, in addition to the 3.5mm jack socket for headphone output and
> headset microphone input.  However, the default audio output path is set
> to headphones, regardless of whether they are actually inserted or not.
>
> This patch series aims to improve the audio support by implementing the
> following changes:
>
> * Detect when the plugged state of the audio jack changes and toggle
>   audio output between headphones and internal speaker, as required.
>   The latter is achieved by essentially routing the right channel of the
>   audio source to the mono speaker.
>
> * Adjust the speaker volume since its default level is too low and,
>   therefore, cannot generate any audible sound.
>
> * Register a dedicated input device for the audio jack and use it to
>   report all headphone and headset mic insert events.
>
> It's worth noting the latter is necessary since the controller complies
> with v1.0 of the USB Audio Class spec (UAC1) and, therefore, cannot
> advertise any jack detection capability.  However, this feature can be
> implemented in the generic USB audio driver via quirks, i.e. by
> configuring an input handler to receive hotplug events from the HID
> driver.
>
> Unrelated to the above, also provide a few driver cleanup patches, e.g.
> make use of bitfields macros, simplify locking, fix coding style.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
> Cristian Ciocaltea (11):
>       HID: playstation: Make use of bitfield macros
>       HID: playstation: Add spaces around arithmetic operators
>       HID: playstation: Simplify locking with guard() and scoped_guard()
>       HID: playstation: Replace uint{32,16,8}_t with u{32,16,8}
>       HID: playstation: Correct spelling in comment sections
>       HID: playstation: Fix all alignment and line length issues
>       HID: playstation: Document spinlock_t usage
>       HID: playstation: Prefer kzalloc(sizeof(*buf)...)
>       HID: playstation: Rename DualSense input report status field
>       HID: playstation: Support DualSense audio jack hotplug detection
>       HID: playstation: Support DualSense audio jack event reporting
>
>  drivers/hid/hid-playstation.c | 885 ++++++++++++++++++++++++------------------
>  1 file changed, 500 insertions(+), 385 deletions(-)
> ---
> base-commit: 7bac2c97af4078d7a627500c9bcdd5b033f97718
> change-id: 20250522-dualsense-hid-jack-d3cb65b75da1
>
>

Re: [PATCH 01/11] HID: playstation: Make use of bitfield macros

[Roderick Colenbrander]

Hi Christian,

Thanks for the effort into this patch. Personally this is a patch I
would rather drop. There is the trade-off between magical numbers and
code readability. I think this patch makes things much less readable.
Having to introduce more macros and FIELD_PREP vs a single bitshift
here and there.

Thanks,
Roderick

On Mon, May 26, 2025 at 5:52 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> Improve code readability by replacing open coded bit operations with the
> equivalent bitfield macros.
>
> While at it, vertically align all DS_OUTPUT_* bit constants.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/hid/hid-playstation.c | 68 ++++++++++++++++++++++++++-----------------
>  1 file changed, 41 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 1468fb11e39dffc883181663a4ad44252e0a7ebb..538194ce8902fe1383b57ac59743f32838dcb0df 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -5,6 +5,7 @@
>   *  Copyright (c) 2020-2022 Sony Interactive Entertainment
>   */
>
> +#include <linux/bitfield.h>
>  #include <linux/bits.h>
>  #include <linux/crc32.h>
>  #include <linux/device.h>
> @@ -116,29 +117,41 @@ struct ps_led_info {
>  #define DS_STATUS_CHARGING_SHIFT       4
>
>  /* Feature version from DualSense Firmware Info report. */
> -#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
> -
> +#define DS_FEATURE_VERSION_MINOR               GENMASK(7, 0)
> +#define DS_FEATURE_VERSION_MAJOR               GENMASK(15, 8)
> +#define DS_FEATURE_VERSION(major, minor)       (FIELD_PREP(DS_FEATURE_VERSION_MAJOR, major) | \
> +                                                FIELD_PREP(DS_FEATURE_VERSION_MINOR, minor))
>  /*
>   * Status of a DualSense touch point contact.
>   * Contact IDs, with highest bit set are 'inactive'
>   * and any associated data is then invalid.
>   */
> -#define DS_TOUCH_POINT_INACTIVE BIT(7)
> +#define DS_TOUCH_POINT_INACTIVE                        BIT(7)
> +#define DS_TOUCH_POINT_X_LO                    GENMASK(7, 0)
> +#define DS_TOUCH_POINT_X_HI                    GENMASK(11, 8)
> +#define DS_TOUCH_POINT_X(hi, lo)               (FIELD_PREP(DS_TOUCH_POINT_X_HI, hi) | \
> +                                                FIELD_PREP(DS_TOUCH_POINT_X_LO, lo))
> +#define DS_TOUCH_POINT_Y_LO                    GENMASK(3, 0)
> +#define DS_TOUCH_POINT_Y_HI                    GENMASK(11, 4)
> +#define DS_TOUCH_POINT_Y(hi, lo)               (FIELD_PREP(DS_TOUCH_POINT_Y_HI, hi) | \
> +                                                FIELD_PREP(DS_TOUCH_POINT_Y_LO, lo))
>
>   /* Magic value required in tag field of Bluetooth output report. */
> -#define DS_OUTPUT_TAG 0x10
> +#define DS_OUTPUT_TAG                          0x10
> +#define DS_OUTPUT_SEQ_TAG                      GENMASK(3, 0)
> +#define DS_OUTPUT_SEQ_NO                       GENMASK(7, 4)
>  /* Flags for DualSense output report. */
> -#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION BIT(0)
> -#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT BIT(1)
> -#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE BIT(0)
> -#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE BIT(1)
> -#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE BIT(2)
> -#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
> -#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
> -#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
> -#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
> -#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
> -#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
> +#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION             BIT(0)
> +#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT                   BIT(1)
> +#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE      BIT(0)
> +#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE                BIT(1)
> +#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE          BIT(2)
> +#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS                     BIT(3)
> +#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE  BIT(4)
> +#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE    BIT(1)
> +#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2            BIT(2)
> +#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE                  BIT(4)
> +#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT                     BIT(1)
>
>  /* DualSense hardware limits */
>  #define DS_ACC_RES_PER_G       8192
> @@ -315,7 +328,9 @@ struct dualsense_output_report {
>   * Contact IDs, with highest bit set are 'inactive'
>   * and any associated data is then invalid.
>   */
> -#define DS4_TOUCH_POINT_INACTIVE BIT(7)
> +#define DS4_TOUCH_POINT_INACTIVE       BIT(7)
> +#define DS4_TOUCH_POINT_X(hi, lo)      DS_TOUCH_POINT_X(hi, lo)
> +#define DS4_TOUCH_POINT_Y(hi, lo)      DS_TOUCH_POINT_Y(hi, lo)
>
>  /* Status field of DualShock4 input report. */
>  #define DS4_STATUS0_BATTERY_CAPACITY   GENMASK(3, 0)
> @@ -1194,7 +1209,8 @@ static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_
>                  * Highest 4-bit is a sequence number, which needs to be increased
>                  * every report. Lowest 4-bit is tag and can be zero for now.
>                  */
> -               bt->seq_tag = (ds->output_seq << 4) | 0x0;
> +               bt->seq_tag = FIELD_PREP(DS_OUTPUT_SEQ_NO, ds->output_seq) |
> +                             FIELD_PREP(DS_OUTPUT_SEQ_TAG, 0x0);
>                 if (++ds->output_seq == 16)
>                         ds->output_seq = 0;
>
> @@ -1439,11 +1455,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>                 input_mt_report_slot_state(ds->touchpad, MT_TOOL_FINGER, active);
>
>                 if (active) {
> -                       int x = (point->x_hi << 8) | point->x_lo;
> -                       int y = (point->y_hi << 4) | point->y_lo;
> -
> -                       input_report_abs(ds->touchpad, ABS_MT_POSITION_X, x);
> -                       input_report_abs(ds->touchpad, ABS_MT_POSITION_Y, y);
> +                       input_report_abs(ds->touchpad, ABS_MT_POSITION_X,
> +                                        DS_TOUCH_POINT_X(point->x_hi, point->x_lo));
> +                       input_report_abs(ds->touchpad, ABS_MT_POSITION_Y,
> +                                        DS_TOUCH_POINT_Y(point->y_hi, point->y_lo));
>                 }
>         }
>         input_mt_sync_frame(ds->touchpad);
> @@ -2351,11 +2366,10 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>                         input_mt_report_slot_state(ds4->touchpad, MT_TOOL_FINGER, active);
>
>                         if (active) {
> -                               int x = (point->x_hi << 8) | point->x_lo;
> -                               int y = (point->y_hi << 4) | point->y_lo;
> -
> -                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_X, x);
> -                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y, y);
> +                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_X,
> +                                                DS4_TOUCH_POINT_X(point->x_hi, point->x_lo));
> +                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y,
> +                                                DS4_TOUCH_POINT_Y(point->y_hi, point->y_lo));
>                         }
>                 }
>                 input_mt_sync_frame(ds4->touchpad);
>
> --
> 2.49.0
>
>

Re: [PATCH 02/11] HID: playstation: Add spaces around arithmetic operators

[Roderick Colenbrander]

Hi Christian,

We could change some of this. I guess checkpatch changed some output
since original submission or is more strict in --strict mode. Though I
have an opinion on these kind of silly patches.

Thanks,
Roderick

On Mon, May 26, 2025 at 5:52 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> Get rid of several checkpatch.pl complaints:
>
>   CHECK: spaces preferred around that '*' (ctx:VxV)
>   CHECK: spaces preferred around that '/' (ctx:VxV)
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/hid/hid-playstation.c | 42 ++++++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 538194ce8902fe1383b57ac59743f32838dcb0df..a82174a21dcce6523a69a8ec374a72504614cedc 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -155,9 +155,9 @@ struct ps_led_info {
>
>  /* DualSense hardware limits */
>  #define DS_ACC_RES_PER_G       8192
> -#define DS_ACC_RANGE           (4*DS_ACC_RES_PER_G)
> +#define DS_ACC_RANGE           (4 * DS_ACC_RES_PER_G)
>  #define DS_GYRO_RES_PER_DEG_S  1024
> -#define DS_GYRO_RANGE          (2048*DS_GYRO_RES_PER_DEG_S)
> +#define DS_GYRO_RANGE          (2048 * DS_GYRO_RES_PER_DEG_S)
>  #define DS_TOUCHPAD_WIDTH      1920
>  #define DS_TOUCHPAD_HEIGHT     1080
>
> @@ -364,9 +364,9 @@ struct dualsense_output_report {
>
>  /* DualShock4 hardware limits */
>  #define DS4_ACC_RES_PER_G      8192
> -#define DS4_ACC_RANGE          (4*DS_ACC_RES_PER_G)
> +#define DS4_ACC_RANGE          (4 * DS_ACC_RES_PER_G)
>  #define DS4_GYRO_RES_PER_DEG_S 1024
> -#define DS4_GYRO_RANGE         (2048*DS_GYRO_RES_PER_DEG_S)
> +#define DS4_GYRO_RANGE         (2048 * DS_GYRO_RES_PER_DEG_S)
>  #define DS4_LIGHTBAR_MAX_BLINK 255 /* 255 centiseconds */
>  #define DS4_TOUCHPAD_WIDTH     1920
>  #define DS4_TOUCHPAD_HEIGHT    942
> @@ -1016,19 +1016,19 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
>         speed_2x = (gyro_speed_plus + gyro_speed_minus);
>         ds->gyro_calib_data[0].abs_code = ABS_RX;
>         ds->gyro_calib_data[0].bias = 0;
> -       ds->gyro_calib_data[0].sens_numer = speed_2x*DS_GYRO_RES_PER_DEG_S;
> +       ds->gyro_calib_data[0].sens_numer = speed_2x * DS_GYRO_RES_PER_DEG_S;
>         ds->gyro_calib_data[0].sens_denom = abs(gyro_pitch_plus - gyro_pitch_bias) +
>                         abs(gyro_pitch_minus - gyro_pitch_bias);
>
>         ds->gyro_calib_data[1].abs_code = ABS_RY;
>         ds->gyro_calib_data[1].bias = 0;
> -       ds->gyro_calib_data[1].sens_numer = speed_2x*DS_GYRO_RES_PER_DEG_S;
> +       ds->gyro_calib_data[1].sens_numer = speed_2x * DS_GYRO_RES_PER_DEG_S;
>         ds->gyro_calib_data[1].sens_denom = abs(gyro_yaw_plus - gyro_yaw_bias) +
>                         abs(gyro_yaw_minus - gyro_yaw_bias);
>
>         ds->gyro_calib_data[2].abs_code = ABS_RZ;
>         ds->gyro_calib_data[2].bias = 0;
> -       ds->gyro_calib_data[2].sens_numer = speed_2x*DS_GYRO_RES_PER_DEG_S;
> +       ds->gyro_calib_data[2].sens_numer = speed_2x * DS_GYRO_RES_PER_DEG_S;
>         ds->gyro_calib_data[2].sens_denom = abs(gyro_roll_plus - gyro_roll_bias) +
>                         abs(gyro_roll_minus - gyro_roll_bias);
>
> @@ -1054,19 +1054,19 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
>         range_2g = acc_x_plus - acc_x_minus;
>         ds->accel_calib_data[0].abs_code = ABS_X;
>         ds->accel_calib_data[0].bias = acc_x_plus - range_2g / 2;
> -       ds->accel_calib_data[0].sens_numer = 2*DS_ACC_RES_PER_G;
> +       ds->accel_calib_data[0].sens_numer = 2 * DS_ACC_RES_PER_G;
>         ds->accel_calib_data[0].sens_denom = range_2g;
>
>         range_2g = acc_y_plus - acc_y_minus;
>         ds->accel_calib_data[1].abs_code = ABS_Y;
>         ds->accel_calib_data[1].bias = acc_y_plus - range_2g / 2;
> -       ds->accel_calib_data[1].sens_numer = 2*DS_ACC_RES_PER_G;
> +       ds->accel_calib_data[1].sens_numer = 2 * DS_ACC_RES_PER_G;
>         ds->accel_calib_data[1].sens_denom = range_2g;
>
>         range_2g = acc_z_plus - acc_z_minus;
>         ds->accel_calib_data[2].abs_code = ABS_Z;
>         ds->accel_calib_data[2].bias = acc_z_plus - range_2g / 2;
> -       ds->accel_calib_data[2].sens_numer = 2*DS_ACC_RES_PER_G;
> +       ds->accel_calib_data[2].sens_numer = 2 * DS_ACC_RES_PER_G;
>         ds->accel_calib_data[2].sens_denom = range_2g;
>
>         /*
> @@ -1882,19 +1882,19 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
>         speed_2x = (gyro_speed_plus + gyro_speed_minus);
>         ds4->gyro_calib_data[0].abs_code = ABS_RX;
>         ds4->gyro_calib_data[0].bias = 0;
> -       ds4->gyro_calib_data[0].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
> +       ds4->gyro_calib_data[0].sens_numer = speed_2x * DS4_GYRO_RES_PER_DEG_S;
>         ds4->gyro_calib_data[0].sens_denom = abs(gyro_pitch_plus - gyro_pitch_bias) +
>                         abs(gyro_pitch_minus - gyro_pitch_bias);
>
>         ds4->gyro_calib_data[1].abs_code = ABS_RY;
>         ds4->gyro_calib_data[1].bias = 0;
> -       ds4->gyro_calib_data[1].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
> +       ds4->gyro_calib_data[1].sens_numer = speed_2x * DS4_GYRO_RES_PER_DEG_S;
>         ds4->gyro_calib_data[1].sens_denom = abs(gyro_yaw_plus - gyro_yaw_bias) +
>                         abs(gyro_yaw_minus - gyro_yaw_bias);
>
>         ds4->gyro_calib_data[2].abs_code = ABS_RZ;
>         ds4->gyro_calib_data[2].bias = 0;
> -       ds4->gyro_calib_data[2].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
> +       ds4->gyro_calib_data[2].sens_numer = speed_2x * DS4_GYRO_RES_PER_DEG_S;
>         ds4->gyro_calib_data[2].sens_denom = abs(gyro_roll_plus - gyro_roll_bias) +
>                         abs(gyro_roll_minus - gyro_roll_bias);
>
> @@ -1905,19 +1905,19 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
>         range_2g = acc_x_plus - acc_x_minus;
>         ds4->accel_calib_data[0].abs_code = ABS_X;
>         ds4->accel_calib_data[0].bias = acc_x_plus - range_2g / 2;
> -       ds4->accel_calib_data[0].sens_numer = 2*DS4_ACC_RES_PER_G;
> +       ds4->accel_calib_data[0].sens_numer = 2 * DS4_ACC_RES_PER_G;
>         ds4->accel_calib_data[0].sens_denom = range_2g;
>
>         range_2g = acc_y_plus - acc_y_minus;
>         ds4->accel_calib_data[1].abs_code = ABS_Y;
>         ds4->accel_calib_data[1].bias = acc_y_plus - range_2g / 2;
> -       ds4->accel_calib_data[1].sens_numer = 2*DS4_ACC_RES_PER_G;
> +       ds4->accel_calib_data[1].sens_numer = 2 * DS4_ACC_RES_PER_G;
>         ds4->accel_calib_data[1].sens_denom = range_2g;
>
>         range_2g = acc_z_plus - acc_z_minus;
>         ds4->accel_calib_data[2].abs_code = ABS_Z;
>         ds4->accel_calib_data[2].bias = acc_z_plus - range_2g / 2;
> -       ds4->accel_calib_data[2].sens_numer = 2*DS4_ACC_RES_PER_G;
> +       ds4->accel_calib_data[2].sens_numer = 2 * DS4_ACC_RES_PER_G;
>         ds4->accel_calib_data[2].sens_denom = range_2g;
>
>  transfer_failed:
> @@ -2059,8 +2059,10 @@ static int dualshock4_led_set_blink(struct led_classdev *led, unsigned long *del
>                 ds4->lightbar_blink_off = 50;
>         } else {
>                 /* Blink delays in centiseconds. */
> -               ds4->lightbar_blink_on = min_t(unsigned long, *delay_on/10, DS4_LIGHTBAR_MAX_BLINK);
> -               ds4->lightbar_blink_off = min_t(unsigned long, *delay_off/10, DS4_LIGHTBAR_MAX_BLINK);
> +               ds4->lightbar_blink_on = min_t(unsigned long, *delay_on / 10,
> +                                              DS4_LIGHTBAR_MAX_BLINK);
> +               ds4->lightbar_blink_off = min_t(unsigned long, *delay_off / 10,
> +                                               DS4_LIGHTBAR_MAX_BLINK);
>         }
>
>         ds4->update_lightbar_blink = true;
> @@ -2340,7 +2342,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>         /* Convert timestamp (in 5.33us unit) to timestamp_us */
>         sensor_timestamp = le16_to_cpu(ds4_report->sensor_timestamp);
>         if (!ds4->sensor_timestamp_initialized) {
> -               ds4->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp*16, 3);
> +               ds4->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp * 16, 3);
>                 ds4->sensor_timestamp_initialized = true;
>         } else {
>                 uint16_t delta;
> @@ -2349,7 +2351,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>                         delta = (U16_MAX - ds4->prev_sensor_timestamp + sensor_timestamp + 1);
>                 else
>                         delta = sensor_timestamp - ds4->prev_sensor_timestamp;
> -               ds4->sensor_timestamp_us += DIV_ROUND_CLOSEST(delta*16, 3);
> +               ds4->sensor_timestamp_us += DIV_ROUND_CLOSEST(delta * 16, 3);
>         }
>         ds4->prev_sensor_timestamp = sensor_timestamp;
>         input_event(ds4->sensors, EV_MSC, MSC_TIMESTAMP, ds4->sensor_timestamp_us);
>
> --
> 2.49.0
>
>

Re: [PATCH 03/11] HID: playstation: Simplify locking with guard() and scoped_guard()

[Roderick Colenbrander]

Hi Christian,

This patch look fine and does simplify some things. Though in terms of
size the patch is a bit long, so it took some careful looking at the
code.

Thanks,
Roderick

On Mon, May 26, 2025 at 5:53 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> Use guard() and scoped_guard() infrastructure instead of explicitly
> acquiring and releasing spinlocks and mutexes to simplify the code and
> ensure that all locks are released properly.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/hid/hid-playstation.c | 216 ++++++++++++++++++------------------------
>  1 file changed, 93 insertions(+), 123 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index a82174a21dcce6523a69a8ec374a72504614cedc..16c576cc74284cc680cc7fc17e1bbe6bbcf84177 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -7,6 +7,7 @@
>
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
> +#include <linux/cleanup.h>
>  #include <linux/crc32.h>
>  #include <linux/device.h>
>  #include <linux/hid.h>
> @@ -567,26 +568,25 @@ static int ps_devices_list_add(struct ps_device *dev)
>  {
>         struct ps_device *entry;
>
> -       mutex_lock(&ps_devices_lock);
> +       guard(mutex)(&ps_devices_lock);
> +
>         list_for_each_entry(entry, &ps_devices_list, list) {
>                 if (!memcmp(entry->mac_address, dev->mac_address, sizeof(dev->mac_address))) {
>                         hid_err(dev->hdev, "Duplicate device found for MAC address %pMR.\n",
>                                         dev->mac_address);
> -                       mutex_unlock(&ps_devices_lock);
>                         return -EEXIST;
>                 }
>         }
>
>         list_add_tail(&dev->list, &ps_devices_list);
> -       mutex_unlock(&ps_devices_lock);
>         return 0;
>  }
>
>  static int ps_devices_list_remove(struct ps_device *dev)
>  {
> -       mutex_lock(&ps_devices_lock);
> +       guard(mutex)(&ps_devices_lock);
> +
>         list_del(&dev->list);
> -       mutex_unlock(&ps_devices_lock);
>         return 0;
>  }
>
> @@ -650,13 +650,12 @@ static int ps_battery_get_property(struct power_supply *psy,
>         struct ps_device *dev = power_supply_get_drvdata(psy);
>         uint8_t battery_capacity;
>         int battery_status;
> -       unsigned long flags;
>         int ret = 0;
>
> -       spin_lock_irqsave(&dev->lock, flags);
> -       battery_capacity = dev->battery_capacity;
> -       battery_status = dev->battery_status;
> -       spin_unlock_irqrestore(&dev->lock, flags);
> +       scoped_guard(spinlock_irqsave, &dev->lock) {
> +               battery_capacity = dev->battery_capacity;
> +               battery_status = dev->battery_status;
> +       }
>
>         switch (psp) {
>         case POWER_SUPPLY_PROP_STATUS:
> @@ -1174,19 +1173,17 @@ static int dualsense_player_led_set_brightness(struct led_classdev *led, enum le
>  {
>         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);
> +       scoped_guard(spinlock_irqsave, &ds->base.lock) {
> +               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);
> +               ds->update_player_leds = true;
> +       }
>
>         dualsense_schedule_work(ds);
>
> @@ -1235,12 +1232,9 @@ static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_
>
>  static inline void dualsense_schedule_work(struct dualsense *ds)
>  {
> -       unsigned long flags;
> -
> -       spin_lock_irqsave(&ds->base.lock, flags);
> +       guard(spinlock_irqsave)(&ds->base.lock);
>         if (ds->output_worker_initialized)
>                 schedule_work(&ds->output_worker);
> -       spin_unlock_irqrestore(&ds->base.lock, flags);
>  }
>
>  /*
> @@ -1338,7 +1332,6 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>         int battery_status;
>         uint32_t sensor_timestamp;
>         bool btn_mic_state;
> -       unsigned long flags;
>         int i;
>
>         /*
> @@ -1400,10 +1393,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>          */
>         btn_mic_state = !!(ds_report->buttons[2] & DS_BUTTONS2_MIC_MUTE);
>         if (btn_mic_state && !ds->last_btn_mic_state) {
> -               spin_lock_irqsave(&ps_dev->lock, flags);
> -               ds->update_mic_mute = true;
> -               ds->mic_muted = !ds->mic_muted; /* toggle */
> -               spin_unlock_irqrestore(&ps_dev->lock, flags);
> +               scoped_guard(spinlock_irqsave, &ps_dev->lock) {
> +                       ds->update_mic_mute = true;
> +                       ds->mic_muted = !ds->mic_muted; /* toggle */
> +               }
>
>                 /* Schedule updating of microphone state at hardware level. */
>                 dualsense_schedule_work(ds);
> @@ -1496,10 +1489,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>                 battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
>         }
>
> -       spin_lock_irqsave(&ps_dev->lock, flags);
> -       ps_dev->battery_capacity = battery_capacity;
> -       ps_dev->battery_status = battery_status;
> -       spin_unlock_irqrestore(&ps_dev->lock, flags);
> +       scoped_guard(spinlock_irqsave, &ps_dev->lock) {
> +               ps_dev->battery_capacity = battery_capacity;
> +               ps_dev->battery_status = battery_status;
> +       }
>
>         return 0;
>  }
> @@ -1508,16 +1501,15 @@ static int dualsense_play_effect(struct input_dev *dev, void *data, struct ff_ef
>  {
>         struct hid_device *hdev = input_get_drvdata(dev);
>         struct dualsense *ds = hid_get_drvdata(hdev);
> -       unsigned long flags;
>
>         if (effect->type != FF_RUMBLE)
>                 return 0;
>
> -       spin_lock_irqsave(&ds->base.lock, flags);
> -       ds->update_rumble = true;
> -       ds->motor_left = effect->u.rumble.strong_magnitude / 256;
> -       ds->motor_right = effect->u.rumble.weak_magnitude / 256;
> -       spin_unlock_irqrestore(&ds->base.lock, flags);
> +       scoped_guard(spinlock_irqsave, &ds->base.lock) {
> +               ds->update_rumble = true;
> +               ds->motor_left = effect->u.rumble.strong_magnitude / 256;
> +               ds->motor_right = effect->u.rumble.weak_magnitude / 256;
> +       }
>
>         dualsense_schedule_work(ds);
>         return 0;
> @@ -1526,11 +1518,9 @@ static int dualsense_play_effect(struct input_dev *dev, void *data, struct ff_ef
>  static void dualsense_remove(struct ps_device *ps_dev)
>  {
>         struct dualsense *ds = container_of(ps_dev, struct dualsense, base);
> -       unsigned long flags;
>
> -       spin_lock_irqsave(&ds->base.lock, flags);
> -       ds->output_worker_initialized = false;
> -       spin_unlock_irqrestore(&ds->base.lock, flags);
> +       scoped_guard(spinlock_irqsave, &ds->base.lock)
> +               ds->output_worker_initialized = false;
>
>         cancel_work_sync(&ds->output_worker);
>  }
> @@ -1562,14 +1552,12 @@ static int dualsense_reset_leds(struct dualsense *ds)
>
>  static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue)
>  {
> -       unsigned long flags;
> -
> -       spin_lock_irqsave(&ds->base.lock, flags);
> -       ds->update_lightbar = true;
> -       ds->lightbar_red = red;
> -       ds->lightbar_green = green;
> -       ds->lightbar_blue = blue;
> -       spin_unlock_irqrestore(&ds->base.lock, flags);
> +       scoped_guard(spinlock_irqsave, &ds->base.lock) {
> +               ds->update_lightbar = true;
> +               ds->lightbar_red = red;
> +               ds->lightbar_green = green;
> +               ds->lightbar_blue = blue;
> +       }
>
>         dualsense_schedule_work(ds);
>  }
> @@ -1756,7 +1744,6 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
>  static void dualshock4_dongle_calibration_work(struct work_struct *work)
>  {
>         struct dualshock4 *ds4 = container_of(work, struct dualshock4, dongle_hotplug_worker);
> -       unsigned long flags;
>         enum dualshock4_dongle_state dongle_state;
>         int ret;
>
> @@ -1775,9 +1762,8 @@ static void dualshock4_dongle_calibration_work(struct work_struct *work)
>                 dongle_state = DONGLE_CONNECTED;
>         }
>
> -       spin_lock_irqsave(&ds4->base.lock, flags);
> -       ds4->dongle_state = dongle_state;
> -       spin_unlock_irqrestore(&ds4->base.lock, flags);
> +       scoped_guard(spinlock_irqsave, &ds4->base.lock)
> +               ds4->dongle_state = dongle_state;
>  }
>
>  static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
> @@ -2049,26 +2035,23 @@ static int dualshock4_led_set_blink(struct led_classdev *led, unsigned long *del
>  {
>         struct hid_device *hdev = to_hid_device(led->dev->parent);
>         struct dualshock4 *ds4 = hid_get_drvdata(hdev);
> -       unsigned long flags;
>
> -       spin_lock_irqsave(&ds4->base.lock, flags);
> +       scoped_guard(spinlock_irqsave, &ds4->base.lock) {
> +               if (!*delay_on && !*delay_off) {
> +                       /* Default to 1 Hz (50 centiseconds on, 50 centiseconds off). */
> +                       ds4->lightbar_blink_on = 50;
> +                       ds4->lightbar_blink_off = 50;
> +               } else {
> +                       /* Blink delays in centiseconds. */
> +                       ds4->lightbar_blink_on = min_t(unsigned long, *delay_on / 10,
> +                                                      DS4_LIGHTBAR_MAX_BLINK);
> +                       ds4->lightbar_blink_off = min_t(unsigned long, *delay_off / 10,
> +                                                       DS4_LIGHTBAR_MAX_BLINK);
> +               }
>
> -       if (!*delay_on && !*delay_off) {
> -               /* Default to 1 Hz (50 centiseconds on, 50 centiseconds off). */
> -               ds4->lightbar_blink_on = 50;
> -               ds4->lightbar_blink_off = 50;
> -       } else {
> -               /* Blink delays in centiseconds. */
> -               ds4->lightbar_blink_on = min_t(unsigned long, *delay_on / 10,
> -                                              DS4_LIGHTBAR_MAX_BLINK);
> -               ds4->lightbar_blink_off = min_t(unsigned long, *delay_off / 10,
> -                                               DS4_LIGHTBAR_MAX_BLINK);
> +               ds4->update_lightbar_blink = true;
>         }
>
> -       ds4->update_lightbar_blink = true;
> -
> -       spin_unlock_irqrestore(&ds4->base.lock, flags);
> -
>         dualshock4_schedule_work(ds4);
>
>         /* Report scaled values back to LED subsystem */
> @@ -2082,36 +2065,33 @@ static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brig
>  {
>         struct hid_device *hdev = to_hid_device(led->dev->parent);
>         struct dualshock4 *ds4 = hid_get_drvdata(hdev);
> -       unsigned long flags;
>         unsigned int led_index;
>
> -       spin_lock_irqsave(&ds4->base.lock, flags);
> -
> -       led_index = led - ds4->lightbar_leds;
> -       switch (led_index) {
> -       case 0:
> -               ds4->lightbar_red = value;
> -               break;
> -       case 1:
> -               ds4->lightbar_green = value;
> -               break;
> -       case 2:
> -               ds4->lightbar_blue = value;
> -               break;
> -       case 3:
> -               ds4->lightbar_enabled = !!value;
> -
> -               /* brightness = 0 also cancels blinking in Linux. */
> -               if (!ds4->lightbar_enabled) {
> -                       ds4->lightbar_blink_off = 0;
> -                       ds4->lightbar_blink_on = 0;
> -                       ds4->update_lightbar_blink = true;
> +       scoped_guard(spinlock_irqsave, &ds4->base.lock) {
> +               led_index = led - ds4->lightbar_leds;
> +               switch (led_index) {
> +               case 0:
> +                       ds4->lightbar_red = value;
> +                       break;
> +               case 1:
> +                       ds4->lightbar_green = value;
> +                       break;
> +               case 2:
> +                       ds4->lightbar_blue = value;
> +                       break;
> +               case 3:
> +                       ds4->lightbar_enabled = !!value;
> +
> +                       /* brightness = 0 also cancels blinking in Linux. */
> +                       if (!ds4->lightbar_enabled) {
> +                               ds4->lightbar_blink_off = 0;
> +                               ds4->lightbar_blink_on = 0;
> +                               ds4->update_lightbar_blink = true;
> +                       }
>                 }
> -       }
> -
> -       ds4->update_lightbar = true;
>
> -       spin_unlock_irqrestore(&ds4->base.lock, flags);
> +               ds4->update_lightbar = true;
> +       }
>
>         dualshock4_schedule_work(ds4);
>
> @@ -2243,7 +2223,6 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>         uint8_t battery_capacity, num_touch_reports, value;
>         int battery_status, i, j;
>         uint16_t sensor_timestamp;
> -       unsigned long flags;
>         bool is_minimal = false;
>
>         /*
> @@ -2421,10 +2400,10 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>                 battery_status = POWER_SUPPLY_STATUS_DISCHARGING;
>         }
>
> -       spin_lock_irqsave(&ps_dev->lock, flags);
> -       ps_dev->battery_capacity = battery_capacity;
> -       ps_dev->battery_status = battery_status;
> -       spin_unlock_irqrestore(&ps_dev->lock, flags);
> +       scoped_guard(spinlock_irqsave, &ps_dev->lock) {
> +               ps_dev->battery_capacity = battery_capacity;
> +               ps_dev->battery_status = battery_status;
> +       }
>
>         return 0;
>  }
> @@ -2442,7 +2421,6 @@ static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_r
>          */
>         if (data[0] == DS4_INPUT_REPORT_USB && size == DS4_INPUT_REPORT_USB_SIZE) {
>                 struct dualshock4_input_report_common *ds4_report = (struct dualshock4_input_report_common *)&data[1];
> -               unsigned long flags;
>
>                 connected = ds4_report->status[1] & DS4_STATUS1_DONGLE_STATE ? false : true;
>
> @@ -2451,9 +2429,8 @@ static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_r
>
>                         dualshock4_set_default_lightbar_colors(ds4);
>
> -                       spin_lock_irqsave(&ps_dev->lock, flags);
> -                       ds4->dongle_state = DONGLE_CALIBRATING;
> -                       spin_unlock_irqrestore(&ps_dev->lock, flags);
> +                       scoped_guard(spinlock_irqsave, &ps_dev->lock)
> +                               ds4->dongle_state = DONGLE_CALIBRATING;
>
>                         schedule_work(&ds4->dongle_hotplug_worker);
>
> @@ -2465,9 +2442,8 @@ static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_r
>                             ds4->dongle_state == DONGLE_DISABLED) && !connected) {
>                         hid_info(ps_dev->hdev, "DualShock 4 USB dongle: controller disconnected\n");
>
> -                       spin_lock_irqsave(&ps_dev->lock, flags);
> -                       ds4->dongle_state = DONGLE_DISCONNECTED;
> -                       spin_unlock_irqrestore(&ps_dev->lock, flags);
> +                       scoped_guard(spinlock_irqsave, &ps_dev->lock)
> +                               ds4->dongle_state = DONGLE_DISCONNECTED;
>
>                         /* Return 0, so hidraw can get the report. */
>                         return 0;
> @@ -2489,16 +2465,15 @@ static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_e
>  {
>         struct hid_device *hdev = input_get_drvdata(dev);
>         struct dualshock4 *ds4 = hid_get_drvdata(hdev);
> -       unsigned long flags;
>
>         if (effect->type != FF_RUMBLE)
>                 return 0;
>
> -       spin_lock_irqsave(&ds4->base.lock, flags);
> -       ds4->update_rumble = true;
> -       ds4->motor_left = effect->u.rumble.strong_magnitude / 256;
> -       ds4->motor_right = effect->u.rumble.weak_magnitude / 256;
> -       spin_unlock_irqrestore(&ds4->base.lock, flags);
> +       scoped_guard(spinlock_irqsave, &ds4->base.lock) {
> +               ds4->update_rumble = true;
> +               ds4->motor_left = effect->u.rumble.strong_magnitude / 256;
> +               ds4->motor_right = effect->u.rumble.weak_magnitude / 256;
> +       }
>
>         dualshock4_schedule_work(ds4);
>         return 0;
> @@ -2507,11 +2482,9 @@ static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_e
>  static void dualshock4_remove(struct ps_device *ps_dev)
>  {
>         struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
> -       unsigned long flags;
>
> -       spin_lock_irqsave(&ds4->base.lock, flags);
> -       ds4->output_worker_initialized = false;
> -       spin_unlock_irqrestore(&ds4->base.lock, flags);
> +       scoped_guard(spinlock_irqsave, &ds4->base.lock)
> +               ds4->output_worker_initialized = false;
>
>         cancel_work_sync(&ds4->output_worker);
>
> @@ -2521,12 +2494,9 @@ static void dualshock4_remove(struct ps_device *ps_dev)
>
>  static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
>  {
> -       unsigned long flags;
> -
> -       spin_lock_irqsave(&ds4->base.lock, flags);
> +       guard(spinlock_irqsave)(&ds4->base.lock);
>         if (ds4->output_worker_initialized)
>                 schedule_work(&ds4->output_worker);
> -       spin_unlock_irqrestore(&ds4->base.lock, flags);
>  }
>
>  static void dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, uint8_t interval)
>
> --
> 2.49.0
>
>

Re: [PATCH 04/11] HID: playstation: Replace uint{32,16,8}_t with u{32,16,8}

[Roderick Colenbrander]

Hi Christian,

I would rather skip this one. The use of uint32_t and family is fine
and many drivers do the same thing.

Thanks,
Roderick

On Mon, May 26, 2025 at 5:53 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> And get rid of the following checkpatch.pl complaints:
>
>   CHECK: Prefer kernel type 'u32' over 'uint32_t'
>   CHECK: Prefer kernel type 'u16' over 'uint16_t'
>   CHECK: Prefer kernel type 'u8' over 'uint8_t'
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/hid/hid-playstation.c | 272 +++++++++++++++++++++---------------------
>  1 file changed, 136 insertions(+), 136 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 16c576cc74284cc680cc7fc17e1bbe6bbcf84177..d36370d001edd0d82e8616f8f2803d8b7ad13954 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -40,17 +40,17 @@ struct ps_device {
>         struct hid_device *hdev;
>         spinlock_t lock;
>
> -       uint32_t player_id;
> +       u32 player_id;
>
>         struct power_supply_desc battery_desc;
>         struct power_supply *battery;
> -       uint8_t battery_capacity;
> +       u8 battery_capacity;
>         int battery_status;
>
>         const char *input_dev_name; /* Name of primary input device. */
> -       uint8_t mac_address[6]; /* Note: stored in little endian order. */
> -       uint32_t hw_version;
> -       uint32_t fw_version;
> +       u8 mac_address[6]; /* Note: stored in little endian order. */
> +       u32 hw_version;
> +       u32 fw_version;
>
>         int (*parse_report)(struct ps_device *dev, struct hid_report *report, u8 *data, int size);
>         void (*remove)(struct ps_device *dev);
> @@ -169,7 +169,7 @@ struct dualsense {
>         struct input_dev *touchpad;
>
>         /* Update version is used as a feature/capability version. */
> -       uint16_t update_version;
> +       u16 update_version;
>
>         /* Calibration data for accelerometer and gyroscope. */
>         struct ps_calibration_data accel_calib_data[3];
> @@ -177,21 +177,21 @@ struct dualsense {
>
>         /* Timestamp for sensor data */
>         bool sensor_timestamp_initialized;
> -       uint32_t prev_sensor_timestamp;
> -       uint32_t sensor_timestamp_us;
> +       u32 prev_sensor_timestamp;
> +       u32 sensor_timestamp_us;
>
>         /* Compatible rumble state */
>         bool use_vibration_v2;
>         bool update_rumble;
> -       uint8_t motor_left;
> -       uint8_t motor_right;
> +       u8 motor_left;
> +       u8 motor_right;
>
>         /* RGB lightbar */
>         struct led_classdev_mc lightbar;
>         bool update_lightbar;
> -       uint8_t lightbar_red;
> -       uint8_t lightbar_green;
> -       uint8_t lightbar_blue;
> +       u8 lightbar_red;
> +       u8 lightbar_green;
> +       u8 lightbar_blue;
>
>         /* Microphone */
>         bool update_mic_mute;
> @@ -200,90 +200,90 @@ struct dualsense {
>
>         /* Player leds */
>         bool update_player_leds;
> -       uint8_t player_leds_state;
> +       u8 player_leds_state;
>         struct led_classdev player_leds[5];
>
>         struct work_struct output_worker;
>         bool output_worker_initialized;
>         void *output_report_dmabuf;
> -       uint8_t output_seq; /* Sequence number for output report. */
> +       u8 output_seq; /* Sequence number for output report. */
>  };
>
>  struct dualsense_touch_point {
> -       uint8_t contact;
> -       uint8_t x_lo;
> -       uint8_t x_hi:4, y_lo:4;
> -       uint8_t y_hi;
> +       u8 contact;
> +       u8 x_lo;
> +       u8 x_hi:4, y_lo:4;
> +       u8 y_hi;
>  } __packed;
>  static_assert(sizeof(struct dualsense_touch_point) == 4);
>
>  /* Main DualSense input report excluding any BT/USB specific headers. */
>  struct dualsense_input_report {
> -       uint8_t x, y;
> -       uint8_t rx, ry;
> -       uint8_t z, rz;
> -       uint8_t seq_number;
> -       uint8_t buttons[4];
> -       uint8_t reserved[4];
> +       u8 x, y;
> +       u8 rx, ry;
> +       u8 z, rz;
> +       u8 seq_number;
> +       u8 buttons[4];
> +       u8 reserved[4];
>
>         /* Motion sensors */
>         __le16 gyro[3]; /* x, y, z */
>         __le16 accel[3]; /* x, y, z */
>         __le32 sensor_timestamp;
> -       uint8_t reserved2;
> +       u8 reserved2;
>
>         /* Touchpad */
>         struct dualsense_touch_point points[2];
>
> -       uint8_t reserved3[12];
> -       uint8_t status;
> -       uint8_t reserved4[10];
> +       u8 reserved3[12];
> +       u8 status;
> +       u8 reserved4[10];
>  } __packed;
>  /* Common input report size shared equals the size of the USB report minus 1 byte for ReportID. */
>  static_assert(sizeof(struct dualsense_input_report) == DS_INPUT_REPORT_USB_SIZE - 1);
>
>  /* Common data between DualSense BT/USB main output report. */
>  struct dualsense_output_report_common {
> -       uint8_t valid_flag0;
> -       uint8_t valid_flag1;
> +       u8 valid_flag0;
> +       u8 valid_flag1;
>
>         /* For DualShock 4 compatibility mode. */
> -       uint8_t motor_right;
> -       uint8_t motor_left;
> +       u8 motor_right;
> +       u8 motor_left;
>
>         /* Audio controls */
> -       uint8_t reserved[4];
> -       uint8_t mute_button_led;
> +       u8 reserved[4];
> +       u8 mute_button_led;
>
> -       uint8_t power_save_control;
> -       uint8_t reserved2[28];
> +       u8 power_save_control;
> +       u8 reserved2[28];
>
>         /* LEDs and lightbar */
> -       uint8_t valid_flag2;
> -       uint8_t reserved3[2];
> -       uint8_t lightbar_setup;
> -       uint8_t led_brightness;
> -       uint8_t player_leds;
> -       uint8_t lightbar_red;
> -       uint8_t lightbar_green;
> -       uint8_t lightbar_blue;
> +       u8 valid_flag2;
> +       u8 reserved3[2];
> +       u8 lightbar_setup;
> +       u8 led_brightness;
> +       u8 player_leds;
> +       u8 lightbar_red;
> +       u8 lightbar_green;
> +       u8 lightbar_blue;
>  } __packed;
>  static_assert(sizeof(struct dualsense_output_report_common) == 47);
>
>  struct dualsense_output_report_bt {
> -       uint8_t report_id; /* 0x31 */
> -       uint8_t seq_tag;
> -       uint8_t tag;
> +       u8 report_id; /* 0x31 */
> +       u8 seq_tag;
> +       u8 tag;
>         struct dualsense_output_report_common common;
> -       uint8_t reserved[24];
> +       u8 reserved[24];
>         __le32 crc32;
>  } __packed;
>  static_assert(sizeof(struct dualsense_output_report_bt) == DS_OUTPUT_REPORT_BT_SIZE);
>
>  struct dualsense_output_report_usb {
> -       uint8_t report_id; /* 0x02 */
> +       u8 report_id; /* 0x02 */
>         struct dualsense_output_report_common common;
> -       uint8_t reserved[15];
> +       u8 reserved[15];
>  } __packed;
>  static_assert(sizeof(struct dualsense_output_report_usb) == DS_OUTPUT_REPORT_USB_SIZE);
>
> @@ -293,8 +293,8 @@ static_assert(sizeof(struct dualsense_output_report_usb) == DS_OUTPUT_REPORT_USB
>   * This structure hide the differences between the two to simplify sending output reports.
>   */
>  struct dualsense_output_report {
> -       uint8_t *data; /* Start of data */
> -       uint8_t len; /* Size of output report */
> +       u8 *data; /* Start of data */
> +       u8 len; /* Size of output report */
>
>         /* Points to Bluetooth data payload in case for a Bluetooth report else NULL. */
>         struct dualsense_output_report_bt *bt;
> @@ -396,26 +396,26 @@ struct dualshock4 {
>
>         /* Timestamp for sensor data */
>         bool sensor_timestamp_initialized;
> -       uint32_t prev_sensor_timestamp;
> -       uint32_t sensor_timestamp_us;
> +       u32 prev_sensor_timestamp;
> +       u32 sensor_timestamp_us;
>
>         /* Bluetooth poll interval */
>         bool update_bt_poll_interval;
> -       uint8_t bt_poll_interval;
> +       u8 bt_poll_interval;
>
>         bool update_rumble;
> -       uint8_t motor_left;
> -       uint8_t motor_right;
> +       u8 motor_left;
> +       u8 motor_right;
>
>         /* Lightbar leds */
>         bool update_lightbar;
>         bool update_lightbar_blink;
>         bool lightbar_enabled; /* For use by global LED control. */
> -       uint8_t lightbar_red;
> -       uint8_t lightbar_green;
> -       uint8_t lightbar_blue;
> -       uint8_t lightbar_blink_on; /* In increments of 10ms. */
> -       uint8_t lightbar_blink_off; /* In increments of 10ms. */
> +       u8 lightbar_red;
> +       u8 lightbar_green;
> +       u8 lightbar_blue;
> +       u8 lightbar_blink_on; /* In increments of 10ms. */
> +       u8 lightbar_blink_off; /* In increments of 10ms. */
>         struct led_classdev lightbar_leds[4];
>
>         struct work_struct output_worker;
> @@ -424,88 +424,88 @@ struct dualshock4 {
>  };
>
>  struct dualshock4_touch_point {
> -       uint8_t contact;
> -       uint8_t x_lo;
> -       uint8_t x_hi:4, y_lo:4;
> -       uint8_t y_hi;
> +       u8 contact;
> +       u8 x_lo;
> +       u8 x_hi:4, y_lo:4;
> +       u8 y_hi;
>  } __packed;
>  static_assert(sizeof(struct dualshock4_touch_point) == 4);
>
>  struct dualshock4_touch_report {
> -       uint8_t timestamp;
> +       u8 timestamp;
>         struct dualshock4_touch_point points[2];
>  } __packed;
>  static_assert(sizeof(struct dualshock4_touch_report) == 9);
>
>  /* Main DualShock4 input report excluding any BT/USB specific headers. */
>  struct dualshock4_input_report_common {
> -       uint8_t x, y;
> -       uint8_t rx, ry;
> -       uint8_t buttons[3];
> -       uint8_t z, rz;
> +       u8 x, y;
> +       u8 rx, ry;
> +       u8 buttons[3];
> +       u8 z, rz;
>
>         /* Motion sensors */
>         __le16 sensor_timestamp;
> -       uint8_t sensor_temperature;
> +       u8 sensor_temperature;
>         __le16 gyro[3]; /* x, y, z */
>         __le16 accel[3]; /* x, y, z */
> -       uint8_t reserved2[5];
> +       u8 reserved2[5];
>
> -       uint8_t status[2];
> -       uint8_t reserved3;
> +       u8 status[2];
> +       u8 reserved3;
>  } __packed;
>  static_assert(sizeof(struct dualshock4_input_report_common) == 32);
>
>  struct dualshock4_input_report_usb {
> -       uint8_t report_id; /* 0x01 */
> +       u8 report_id; /* 0x01 */
>         struct dualshock4_input_report_common common;
> -       uint8_t num_touch_reports;
> +       u8 num_touch_reports;
>         struct dualshock4_touch_report touch_reports[3];
> -       uint8_t reserved[3];
> +       u8 reserved[3];
>  } __packed;
>  static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE);
>
>  struct dualshock4_input_report_bt {
> -       uint8_t report_id; /* 0x11 */
> -       uint8_t reserved[2];
> +       u8 report_id; /* 0x11 */
> +       u8 reserved[2];
>         struct dualshock4_input_report_common common;
> -       uint8_t num_touch_reports;
> +       u8 num_touch_reports;
>         struct dualshock4_touch_report touch_reports[4]; /* BT has 4 compared to 3 for USB */
> -       uint8_t reserved2[2];
> +       u8 reserved2[2];
>         __le32 crc32;
>  } __packed;
>  static_assert(sizeof(struct dualshock4_input_report_bt) == DS4_INPUT_REPORT_BT_SIZE);
>
>  /* Common data between Bluetooth and USB DualShock4 output reports. */
>  struct dualshock4_output_report_common {
> -       uint8_t valid_flag0;
> -       uint8_t valid_flag1;
> +       u8 valid_flag0;
> +       u8 valid_flag1;
>
> -       uint8_t reserved;
> +       u8 reserved;
>
> -       uint8_t motor_right;
> -       uint8_t motor_left;
> +       u8 motor_right;
> +       u8 motor_left;
>
> -       uint8_t lightbar_red;
> -       uint8_t lightbar_green;
> -       uint8_t lightbar_blue;
> -       uint8_t lightbar_blink_on;
> -       uint8_t lightbar_blink_off;
> +       u8 lightbar_red;
> +       u8 lightbar_green;
> +       u8 lightbar_blue;
> +       u8 lightbar_blink_on;
> +       u8 lightbar_blink_off;
>  } __packed;
>
>  struct dualshock4_output_report_usb {
> -       uint8_t report_id; /* 0x5 */
> +       u8 report_id; /* 0x5 */
>         struct dualshock4_output_report_common common;
> -       uint8_t reserved[21];
> +       u8 reserved[21];
>  } __packed;
>  static_assert(sizeof(struct dualshock4_output_report_usb) == DS4_OUTPUT_REPORT_USB_SIZE);
>
>  struct dualshock4_output_report_bt {
> -       uint8_t report_id; /* 0x11 */
> -       uint8_t hw_control;
> -       uint8_t audio_control;
> +       u8 report_id; /* 0x11 */
> +       u8 hw_control;
> +       u8 audio_control;
>         struct dualshock4_output_report_common common;
> -       uint8_t reserved[61];
> +       u8 reserved[61];
>         __le32 crc32;
>  } __packed;
>  static_assert(sizeof(struct dualshock4_output_report_bt) == DS4_OUTPUT_REPORT_BT_SIZE);
> @@ -516,8 +516,8 @@ static_assert(sizeof(struct dualshock4_output_report_bt) == DS4_OUTPUT_REPORT_BT
>   * This structure hide the differences between the two to simplify sending output reports.
>   */
>  struct dualshock4_output_report {
> -       uint8_t *data; /* Start of data */
> -       uint8_t len; /* Size of output report */
> +       u8 *data; /* Start of data */
> +       u8 len; /* Size of output report */
>
>         /* Points to Bluetooth data payload in case for a Bluetooth report else NULL. */
>         struct dualshock4_output_report_bt *bt;
> @@ -556,7 +556,7 @@ static const struct {int x; int y; } ps_gamepad_hat_mapping[] = {
>  static int dualshock4_get_calibration_data(struct dualshock4 *ds4);
>  static inline void dualsense_schedule_work(struct dualsense *ds);
>  static inline void dualshock4_schedule_work(struct dualshock4 *ds4);
> -static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue);
> +static void dualsense_set_lightbar(struct dualsense *ds, u8 red, u8 green, u8 blue);
>  static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4);
>
>  /*
> @@ -648,7 +648,7 @@ static int ps_battery_get_property(struct power_supply *psy,
>                 union power_supply_propval *val)
>  {
>         struct ps_device *dev = power_supply_get_drvdata(psy);
> -       uint8_t battery_capacity;
> +       u8 battery_capacity;
>         int battery_status;
>         int ret = 0;
>
> @@ -711,9 +711,9 @@ static int ps_device_register_battery(struct ps_device *dev)
>  }
>
>  /* Compute crc32 of HID data and compare against expected CRC. */
> -static bool ps_check_crc32(uint8_t seed, uint8_t *data, size_t len, uint32_t report_crc)
> +static bool ps_check_crc32(u8 seed, u8 *data, size_t len, u32 report_crc)
>  {
> -       uint32_t crc;
> +       u32 crc;
>
>         crc = crc32_le(0xFFFFFFFF, &seed, 1);
>         crc = ~crc32_le(crc, data, len);
> @@ -759,7 +759,7 @@ static struct input_dev *ps_gamepad_create(struct hid_device *hdev,
>         return gamepad;
>  }
>
> -static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *buf, size_t size,
> +static int ps_get_report(struct hid_device *hdev, u8 report_id, u8 *buf, size_t size,
>                 bool check_crc)
>  {
>         int ret;
> @@ -783,8 +783,8 @@ static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *bu
>
>         if (hdev->bus == BUS_BLUETOOTH && check_crc) {
>                 /* Last 4 bytes contains crc32. */
> -               uint8_t crc_offset = size - 4;
> -               uint32_t report_crc = get_unaligned_le32(&buf[crc_offset]);
> +               u8 crc_offset = size - 4;
> +               u32 report_crc = get_unaligned_le32(&buf[crc_offset]);
>
>                 if (!ps_check_crc32(PS_FEATURE_CRC32_SEED, buf, crc_offset, report_crc)) {
>                         hid_err(hdev, "CRC check failed for reportID=%d\n", report_id);
> @@ -977,7 +977,7 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
>         int range_2g;
>         int ret = 0;
>         int i;
> -       uint8_t *buf;
> +       u8 *buf;
>
>         buf = kzalloc(DS_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
>         if (!buf)
> @@ -1091,7 +1091,7 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
>
>  static int dualsense_get_firmware_info(struct dualsense *ds)
>  {
> -       uint8_t *buf;
> +       u8 *buf;
>         int ret;
>
>         buf = kzalloc(DS_FEATURE_REPORT_FIRMWARE_INFO_SIZE, GFP_KERNEL);
> @@ -1124,7 +1124,7 @@ static int dualsense_get_firmware_info(struct dualsense *ds)
>
>  static int dualsense_get_mac_address(struct dualsense *ds)
>  {
> -       uint8_t *buf;
> +       u8 *buf;
>         int ret = 0;
>
>         buf = kzalloc(DS_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL);
> @@ -1150,7 +1150,7 @@ static int dualsense_lightbar_set_brightness(struct led_classdev *cdev,
>  {
>         struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
>         struct dualsense *ds = container_of(mc_cdev, struct dualsense, lightbar);
> -       uint8_t red, green, blue;
> +       u8 red, green, blue;
>
>         led_mc_calc_color_components(mc_cdev, brightness);
>         red = mc_cdev->subled_info[0].brightness;
> @@ -1248,8 +1248,8 @@ static void dualsense_send_output_report(struct dualsense *ds,
>
>         /* Bluetooth packets need to be signed with a CRC in the last 4 bytes. */
>         if (report->bt) {
> -               uint32_t crc;
> -               uint8_t seed = PS_OUTPUT_CRC32_SEED;
> +               u32 crc;
> +               u8 seed = PS_OUTPUT_CRC32_SEED;
>
>                 crc = crc32_le(0xFFFFFFFF, &seed, 1);
>                 crc = ~crc32_le(crc, report->data, report->len - 4);
> @@ -1328,9 +1328,9 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>         struct hid_device *hdev = ps_dev->hdev;
>         struct dualsense *ds = container_of(ps_dev, struct dualsense, base);
>         struct dualsense_input_report *ds_report;
> -       uint8_t battery_data, battery_capacity, charging_status, value;
> +       u8 battery_data, battery_capacity, charging_status, value;
>         int battery_status;
> -       uint32_t sensor_timestamp;
> +       u32 sensor_timestamp;
>         bool btn_mic_state;
>         int i;
>
> @@ -1345,7 +1345,7 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>         } else if (hdev->bus == BUS_BLUETOOTH && report->id == DS_INPUT_REPORT_BT &&
>                         size == DS_INPUT_REPORT_BT_SIZE) {
>                 /* Last 4 bytes of input report contain crc32 */
> -               uint32_t report_crc = get_unaligned_le32(&data[size - 4]);
> +               u32 report_crc = get_unaligned_le32(&data[size - 4]);
>
>                 if (!ps_check_crc32(PS_INPUT_CRC32_SEED, data, size - 4, report_crc)) {
>                         hid_err(hdev, "DualSense input CRC's check failed\n");
> @@ -1428,7 +1428,7 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>                 ds->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp, 3);
>                 ds->sensor_timestamp_initialized = true;
>         } else {
> -               uint32_t delta;
> +               u32 delta;
>
>                 if (ds->prev_sensor_timestamp > sensor_timestamp)
>                         delta = (U32_MAX - ds->prev_sensor_timestamp + sensor_timestamp + 1);
> @@ -1528,7 +1528,7 @@ static void dualsense_remove(struct ps_device *ps_dev)
>  static int dualsense_reset_leds(struct dualsense *ds)
>  {
>         struct dualsense_output_report report;
> -       uint8_t *buf;
> +       u8 *buf;
>
>         buf = kzalloc(sizeof(struct dualsense_output_report_bt), GFP_KERNEL);
>         if (!buf)
> @@ -1550,7 +1550,7 @@ static int dualsense_reset_leds(struct dualsense *ds)
>         return 0;
>  }
>
> -static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue)
> +static void dualsense_set_lightbar(struct dualsense *ds, u8 red, u8 green, u8 blue)
>  {
>         scoped_guard(spinlock_irqsave, &ds->base.lock) {
>                 ds->update_lightbar = true;
> @@ -1578,7 +1578,7 @@ static void dualsense_set_player_leds(struct dualsense *ds)
>                 BIT(4) | BIT(3) | BIT(2) | BIT(1) | BIT(0)
>         };
>
> -       uint8_t player_id = ds->base.player_id % ARRAY_SIZE(player_ids);
> +       u8 player_id = ds->base.player_id % ARRAY_SIZE(player_ids);
>
>         ds->update_player_leds = true;
>         ds->player_leds_state = player_ids[player_id];
> @@ -1589,7 +1589,7 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
>  {
>         struct dualsense *ds;
>         struct ps_device *ps_dev;
> -       uint8_t max_output_report_size;
> +       u8 max_output_report_size;
>         int i, ret;
>
>         static const struct ps_led_info player_leds_info[] = {
> @@ -1780,7 +1780,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
>         int range_2g;
>         int ret = 0;
>         int i;
> -       uint8_t *buf;
> +       u8 *buf;
>
>         if (ds4->base.hdev->bus == BUS_USB) {
>                 int retries;
> @@ -1944,7 +1944,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
>
>  static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
>  {
> -       uint8_t *buf;
> +       u8 *buf;
>         int ret;
>
>         buf = kzalloc(DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, GFP_KERNEL);
> @@ -1972,7 +1972,7 @@ static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
>  static int dualshock4_get_mac_address(struct dualshock4 *ds4)
>  {
>         struct hid_device *hdev = ds4->base.hdev;
> -       uint8_t *buf;
> +       u8 *buf;
>         int ret = 0;
>
>         if (hdev->bus == BUS_USB) {
> @@ -2191,8 +2191,8 @@ static void dualshock4_output_worker(struct work_struct *work)
>
>         /* Bluetooth packets need additional flags as well as a CRC in the last 4 bytes. */
>         if (report.bt) {
> -               uint32_t crc;
> -               uint8_t seed = PS_OUTPUT_CRC32_SEED;
> +               u32 crc;
> +               u8 seed = PS_OUTPUT_CRC32_SEED;
>
>                 /* Hardware control flags need to set to let the device know
>                  * there is HID data as well as CRC.
> @@ -2220,9 +2220,9 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>         struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
>         struct dualshock4_input_report_common *ds4_report;
>         struct dualshock4_touch_report *touch_reports;
> -       uint8_t battery_capacity, num_touch_reports, value;
> +       u8 battery_capacity, num_touch_reports, value;
>         int battery_status, i, j;
> -       uint16_t sensor_timestamp;
> +       u16 sensor_timestamp;
>         bool is_minimal = false;
>
>         /*
> @@ -2240,7 +2240,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>         } else if (hdev->bus == BUS_BLUETOOTH && report->id == DS4_INPUT_REPORT_BT &&
>                         size == DS4_INPUT_REPORT_BT_SIZE) {
>                 struct dualshock4_input_report_bt *bt = (struct dualshock4_input_report_bt *)data;
> -               uint32_t report_crc = get_unaligned_le32(&bt->crc32);
> +               u32 report_crc = get_unaligned_le32(&bt->crc32);
>
>                 /* Last 4 bytes of input report contains CRC. */
>                 if (!ps_check_crc32(PS_INPUT_CRC32_SEED, data, size - 4, report_crc)) {
> @@ -2324,7 +2324,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>                 ds4->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp * 16, 3);
>                 ds4->sensor_timestamp_initialized = true;
>         } else {
> -               uint16_t delta;
> +               u16 delta;
>
>                 if (ds4->prev_sensor_timestamp > sensor_timestamp)
>                         delta = (U16_MAX - ds4->prev_sensor_timestamp + sensor_timestamp + 1);
> @@ -2369,7 +2369,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>          * - 15: charge error
>          */
>         if (ds4_report->status[0] & DS4_STATUS0_CABLE_STATE) {
> -               uint8_t battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
> +               u8 battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
>
>                 if (battery_data < 10) {
>                         /* Take the mid-point for each battery capacity value,
> @@ -2390,7 +2390,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>                         battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
>                 }
>         } else {
> -               uint8_t battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
> +               u8 battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
>
>                 if (battery_data < 10)
>                         battery_capacity = battery_data * 10 + 5;
> @@ -2499,7 +2499,7 @@ static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
>                 schedule_work(&ds4->output_worker);
>  }
>
> -static void dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, uint8_t interval)
> +static void dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, u8 interval)
>  {
>         ds4->bt_poll_interval = interval;
>         ds4->update_bt_poll_interval = true;
> @@ -2519,7 +2519,7 @@ static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4)
>                 { 0x20, 0x00, 0x20 }  /* Pink */
>         };
>
> -       uint8_t player_id = ds4->base.player_id % ARRAY_SIZE(player_colors);
> +       u8 player_id = ds4->base.player_id % ARRAY_SIZE(player_colors);
>
>         ds4->lightbar_enabled = true;
>         ds4->lightbar_red = player_colors[player_id][0];
> @@ -2534,7 +2534,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
>  {
>         struct dualshock4 *ds4;
>         struct ps_device *ps_dev;
> -       uint8_t max_output_report_size;
> +       u8 max_output_report_size;
>         int i, ret;
>
>         /* The DualShock4 has an RGB lightbar, which the original hid-sony driver
>
> --
> 2.49.0
>
>

Re: [PATCH 09/11] HID: playstation: Rename DualSense input report status field

[Roderick Colenbrander]

Hi Christian,

At the time I didn't document all parts of the HID report as not
everything was needed. When adding in the audio stuff, let's call this
one by its official name. There are officially 3 bytes of status, so
the correct thing is to technically make it a 3 byte array of status.
The official register names contain DS_STATUS0_.., DS_STATUS1_..,
etcetera in the name, so I would like to keep things aligned with our
datasheets.

Thanks,
Roderick

On Mon, May 26, 2025 at 5:54 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> The name of the 'status' member of struct dualsense_input_report is too
> generic, since it is only used to provide battery data.
>
> In preparation to support handling additional (non-battery) status
> information and avoid ambiguity, rename the field to status_battery.
> For consistency, also rename the related DS_STATUS_* bitfield macros.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/hid/hid-playstation.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 937b14679c8b17c5d3d245eae1cc4e0e56dadb71..b7777d3230b2fe277a4a2217ef6b1eff9cfad182 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -112,10 +112,10 @@ struct ps_led_info {
>  #define DS_BUTTONS2_TOUCHPAD   BIT(1)
>  #define DS_BUTTONS2_MIC_MUTE   BIT(2)
>
> -/* Status field of DualSense input report. */
> -#define DS_STATUS_BATTERY_CAPACITY     GENMASK(3, 0)
> -#define DS_STATUS_CHARGING             GENMASK(7, 4)
> -#define DS_STATUS_CHARGING_SHIFT       4
> +/* Battery status field of DualSense input report. */
> +#define DS_STATUS_BATTERY_CAPACITY             GENMASK(3, 0)
> +#define DS_STATUS_BATTERY_CHARGING             GENMASK(7, 4)
> +#define DS_STATUS_BATTERY_CHARGING_SHIFT       4
>
>  /* Feature version from DualSense Firmware Info report. */
>  #define DS_FEATURE_VERSION_MINOR               GENMASK(7, 0)
> @@ -236,7 +236,7 @@ struct dualsense_input_report {
>         struct dualsense_touch_point points[2];
>
>         u8 reserved3[12];
> -       u8 status;
> +       u8 status_battery;
>         u8 reserved4[10];
>  } __packed;
>  /* Common input report size shared equals the size of the USB report minus 1 byte for ReportID. */
> @@ -1462,8 +1462,9 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>         input_report_key(ds->touchpad, BTN_LEFT, ds_report->buttons[2] & DS_BUTTONS2_TOUCHPAD);
>         input_sync(ds->touchpad);
>
> -       battery_data = ds_report->status & DS_STATUS_BATTERY_CAPACITY;
> -       charging_status = (ds_report->status & DS_STATUS_CHARGING) >> DS_STATUS_CHARGING_SHIFT;
> +       battery_data = ds_report->status_battery & DS_STATUS_BATTERY_CAPACITY;
> +       charging_status = ds_report->status_battery & DS_STATUS_BATTERY_CHARGING;
> +       charging_status >>= DS_STATUS_BATTERY_CHARGING_SHIFT;
>
>         switch (charging_status) {
>         case 0x0:
>
> --
> 2.49.0
>
>

Re: [PATCH 10/11] HID: playstation: Support DualSense audio jack hotplug detection

[Roderick Colenbrander]

Hi Christian,

Thanks for sharing I need to have a closer look to see what we do on
the console as well (we want to keep behavior where possible in sync
also due to official PlayStation use cases on embedded devices).

At least use this logic only for USB, we should not register these
nodes or apply such settings for Bluetooth. Just setting these on
bluetooth without audio I could see affecting battery life a bit. But
in addition, I honestly don't really see a path to supporting
Bluetooth audio for the DualSense. It would add confusion for users.
The reason, I don't think there is a good way for adding audio is that
the DualSense is very, very complex. In Bluetooth mode we have our own
ways of doing audio (non-standard), which is already tricky. Above all
else the audio is compressed and not PCM. The only way to do it is
probably in userspace in something PulseAudio like. So before rushing
adding this also for Bluetooth, I want to limit this to USB.

I'm sharing some other feedback in line. It is small stuff, but in
particular I like to keep datasheets and code in sync, so of it is
around naming. It allows us internally for easier review as well.

There are some areas I need to study some more and get internal
documentation, so I don't have thorough feedback on the audio code
itself yet.

Thanks,
Roderick

On Mon, May 26, 2025 at 6:04 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> The default audio output path on DualSense controller hardware is set to
> headphones, regardless of whether they are actually inserted or not.
>
> Detect when the plugged state of the 3.5mm audio jack changes and toggle
> audio output between headphones and internal speaker, as required.  The
> latter is achieved by essentially routing the right channel of the audio
> source to the mono speaker.
>
> Additionally, adjust the speaker volume since its default level is too
> low and, therefore, cannot generate any audible sound.
>
> It's worth noting the audio functionality is currently not supported for
> Bluetooth, hence it's limited to USB connectivity.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/hid/hid-playstation.c | 86 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index b7777d3230b2fe277a4a2217ef6b1eff9cfad182..ed8e67c5bf5d3a9e9721da31a6bd84f0b6802b14 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -117,6 +117,12 @@ struct ps_led_info {
>  #define DS_STATUS_BATTERY_CHARGING             GENMASK(7, 4)
>  #define DS_STATUS_BATTERY_CHARGING_SHIFT       4
>
> +/* Plugged status field of DualSense input report. */
> +#define DS_STATUS_PLUGGED_HEADPHONES           BIT(0)
> +#define DS_STATUS_PLUGGED_MIC                  BIT(1)
> +#define DS_STATUS_PLUGGED_JACK                 (DS_STATUS_PLUGGED_HEADPHONES | \
> +                                                DS_STATUS_PLUGGED_MIC)

Like I mentioned in patch 10, there are 3 status bytes. So let's call
this DS_STATUS1_. The other 5 bits in this one are unrelated to audio.

Official datasheets call these 'HP_DETECT', 'MIC_DETECT' and
'MIC_MUTE'. I guess we should use those names.

> +
>  /* Feature version from DualSense Firmware Info report. */
>  #define DS_FEATURE_VERSION_MINOR               GENMASK(7, 0)
>  #define DS_FEATURE_VERSION_MAJOR               GENMASK(15, 8)
> @@ -144,13 +150,18 @@ struct ps_led_info {
>  /* Flags for DualSense output report. */
>  #define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION             BIT(0)
>  #define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT                   BIT(1)
> +#define DS_OUTPUT_VALID_FLAG0_SPEAKER_VOLUME_ENABLE            BIT(5)
Just for completeness also add BIT(6) which is MIC_VOLUME_ENABLE
> +#define DS_OUTPUT_VALID_FLAG0_AUDIO_CONTROL_ENABLE             BIT(7)
>  #define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE      BIT(0)
>  #define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE                BIT(1)
>  #define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE          BIT(2)
>  #define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS                     BIT(3)
>  #define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE  BIT(4)
> +#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_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)
>
> @@ -193,6 +204,11 @@ struct dualsense {
>         u8 lightbar_green;
>         u8 lightbar_blue;
>
> +       /* Audio Jack plugged state */
> +       u8 plugged_state;
> +       u8 prev_plugged_state;
> +       bool prev_plugged_state_valid;
> +
>         /* Microphone */
>         bool update_mic_mute;
>         bool mic_muted;
> @@ -237,7 +253,8 @@ struct dualsense_input_report {
>
>         u8 reserved3[12];
>         u8 status_battery;
> -       u8 reserved4[10];
> +       u8 status_plugged;
> +       u8 reserved4[9];
>  } __packed;
>  /* Common input report size shared equals the size of the USB report minus 1 byte for ReportID. */
>  static_assert(sizeof(struct dualsense_input_report) == DS_INPUT_REPORT_USB_SIZE - 1);
> @@ -252,11 +269,15 @@ struct dualsense_output_report_common {
>         u8 motor_left;
>
>         /* Audio controls */
> -       u8 reserved[4];
> +       u8 headphone_audio_volume;              /* 0-0x7f */
Just call this headphone_volume
> +       u8 speaker_audio_volume;                /* 0-0xff */
Just call this speaker_volume
> +       u8 internal_microphone_volume;  /* 0-0x40 */
We call this one mic_volume
> +       u8 audio_flags;
We call this one audio_control.

>         u8 mute_button_led;
>
>         u8 power_save_control;
> -       u8 reserved2[28];
> +       u8 reserved2[27];
> +       u8 audio_flags2;
This one we call audio_control2
>
>         /* LEDs and lightbar */
>         u8 valid_flag2;
> @@ -1304,6 +1325,46 @@ static void dualsense_output_worker(struct work_struct *work)
>                 ds->update_player_leds = false;
>         }
>
> +       if (ds->plugged_state != ds->prev_plugged_state) {
> +               u8 val = ds->plugged_state & DS_STATUS_PLUGGED_HEADPHONES;
> +
> +               if (val != (ds->prev_plugged_state & DS_STATUS_PLUGGED_HEADPHONES)) {
> +                       common->valid_flag0 = DS_OUTPUT_VALID_FLAG0_AUDIO_CONTROL_ENABLE;
> +                       /*
> +                        *  _--------> Output path setup in audio_flag0
> +                        * /  _------> Headphone (HP) Left channel sink
> +                        * | /  _----> Headphone (HP) Right channel sink
> +                        * | | /  _--> Internal Speaker (SP) sink
> +                        * | | | /
> +                        * | | | |     L/R - Left/Right channel source
> +                        * 0 L-R X       X - Unrouted (muted) channel source
> +                        * 1 L-L X
> +                        * 2 L-L R
> +                        * 3 X-X R
> +                        */
> +                       if (val) {
> +                               /* Mute SP and route L+R channels to HP */
> +                               common->audio_flags = 0;
> +                       } else {
> +                               /* Mute HP and route R channel to SP */
> +                               common->audio_flags =
> +                                       FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS_OUTPUT_PATH_SEL, 0x3);
> +                               /*
> +                                * Set SP hardware volume to 100%.
> +                                * Note the accepted range seems to be [0x3d..0x64]
> +                                */
> +                               common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_SPEAKER_VOLUME_ENABLE;
> +                               common->speaker_audio_volume = 0x64;
> +                               /* Set SP preamp gain to ~30% */
> +                               common->valid_flag1 = DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE;
> +                               common->audio_flags2 =
> +                                       FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN, 0x2);
> +                       }
> +               }
> +
> +               ds->prev_plugged_state = ds->plugged_state;
> +       }
> +
>         if (ds->update_mic_mute) {
>                 common->valid_flag1 |= DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE;
>                 common->mute_button_led = ds->mic_muted;
> @@ -1407,6 +1468,25 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>         }
>         ds->last_btn_mic_state = btn_mic_state;
>
> +       /* Parse HP/MIC plugged state data */
> +       value = ds_report->status_plugged & DS_STATUS_PLUGGED_JACK;
> +       if (!ds->prev_plugged_state_valid) {
> +               /* Initial handling of the plugged state report */
> +               scoped_guard(spinlock_irqsave, &ps_dev->lock) {
> +                       ds->plugged_state = (~value) & DS_STATUS_PLUGGED_JACK;
> +                       ds->prev_plugged_state_valid = true;
> +               }
> +       }
> +       if (value != ds->plugged_state) {
> +               scoped_guard(spinlock_irqsave, &ps_dev->lock) {
> +                       ds->prev_plugged_state = ds->plugged_state;
> +                       ds->plugged_state = value;
> +               }
> +
> +               /* Schedule audio routing towards active endpoint. */
> +               dualsense_schedule_work(ds);
> +       }
> +
>         /* Parse and calibrate gyroscope data. */
>         for (i = 0; i < ARRAY_SIZE(ds_report->gyro); i++) {
>                 int raw_data = (short)le16_to_cpu(ds_report->gyro[i]);
>
> --
> 2.49.0
>
>

Re: [PATCH 00/11] HID: playstation: Add support for audio jack handling on DualSense

[Cristian Ciocaltea]

Hi Roderick,

On 6/10/25 7:01 AM, Roderick Colenbrander wrote:
> Hi Cristian,
> 
> Thanks for sharing your patches around audio. I need to have a closer
> look at some of those and how the console also behaves (we try to keep
> things in-sync'ish when possible). I need to double check the
> datasheets as well.

No worries, take your time!

> The series does contain some other patches around style and stuff.
> Some of them for me are entering that slippery slope of what to
> change. There are some different styles in use around the kernel (e.g.
> uint32_t etcetera is fine). But then if you use super strict mode on
> checkpatch half the kernel almost needs to be touched. I'm a bit
> skeptical on those kind of patches.

While I can understand that some of these patches might be perceived as
unnecessary noise, I still think the sooner we do this type of cleanup,
the better.  And the rationale is that we should aim for consistency, at
least for the actively maintained code.  This should also encourage any
new contributor who might touch the code to comply with the recommended
style and/or best practices, instead of potentially falling into the
trap of taking as reference some obsolete or non-conformant constructs,
which would only bring additional mess.

As a matter of fact, I initially planned to just focus on fixing up the
patches introduced as part of this series, in order to make checkpatch
happy.  While doing it, I quickly realized some additional
inconsistencies, hence I extended a bit the scope of the cleanup.  That
highlighted even more issues and after a few iterations I eventually
ended up fixing them all.

I'm aware that some of these checkpatch reports could be silenced by
operating in non-strict mode, by I don't really like the idea.  The
reason is that some (or most?!) maintainers prefer or even demand using
the strict mode, hence it's hard or impossible to always have then right
switch set, particularly when touching multiple subsystems.

Thanks,
Cristian

Re: [PATCH 01/11] HID: playstation: Make use of bitfield macros

[Cristian Ciocaltea]

Hi Roderick,

On 6/10/25 7:07 AM, Roderick Colenbrander wrote:
> Hi Christian,
> 
> Thanks for the effort into this patch. Personally this is a patch I
> would rather drop. There is the trade-off between magical numbers and
> code readability. I think this patch makes things much less readable.
> Having to introduce more macros and FIELD_PREP vs a single bitshift
> here and there.

I think the usage of bitfield macros is a general recommendation of 
making code more robust, and this should also help improve overall 
readability - e.g. this reduced code duplication by replacing the open 
coded bit operations for computing touch point coordinates.

Sorry for the noise generated by the alignment correction!

Thanks,
Cristian

> 
> Thanks,
> Roderick
> 
> On Mon, May 26, 2025 at 5:52 AM Cristian Ciocaltea
> <cristian.ciocaltea@collabora.com> wrote:
>>
>> Improve code readability by replacing open coded bit operations with the
>> equivalent bitfield macros.
>>
>> While at it, vertically align all DS_OUTPUT_* bit constants.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/hid/hid-playstation.c | 68 ++++++++++++++++++++++++++-----------------
>>  1 file changed, 41 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
>> index 1468fb11e39dffc883181663a4ad44252e0a7ebb..538194ce8902fe1383b57ac59743f32838dcb0df 100644
>> --- a/drivers/hid/hid-playstation.c
>> +++ b/drivers/hid/hid-playstation.c
>> @@ -5,6 +5,7 @@
>>   *  Copyright (c) 2020-2022 Sony Interactive Entertainment
>>   */
>>
>> +#include <linux/bitfield.h>
>>  #include <linux/bits.h>
>>  #include <linux/crc32.h>
>>  #include <linux/device.h>
>> @@ -116,29 +117,41 @@ struct ps_led_info {
>>  #define DS_STATUS_CHARGING_SHIFT       4
>>
>>  /* Feature version from DualSense Firmware Info report. */
>> -#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
>> -
>> +#define DS_FEATURE_VERSION_MINOR               GENMASK(7, 0)
>> +#define DS_FEATURE_VERSION_MAJOR               GENMASK(15, 8)
>> +#define DS_FEATURE_VERSION(major, minor)       (FIELD_PREP(DS_FEATURE_VERSION_MAJOR, major) | \
>> +                                                FIELD_PREP(DS_FEATURE_VERSION_MINOR, minor))
>>  /*
>>   * Status of a DualSense touch point contact.
>>   * Contact IDs, with highest bit set are 'inactive'
>>   * and any associated data is then invalid.
>>   */
>> -#define DS_TOUCH_POINT_INACTIVE BIT(7)
>> +#define DS_TOUCH_POINT_INACTIVE                        BIT(7)
>> +#define DS_TOUCH_POINT_X_LO                    GENMASK(7, 0)
>> +#define DS_TOUCH_POINT_X_HI                    GENMASK(11, 8)
>> +#define DS_TOUCH_POINT_X(hi, lo)               (FIELD_PREP(DS_TOUCH_POINT_X_HI, hi) | \
>> +                                                FIELD_PREP(DS_TOUCH_POINT_X_LO, lo))
>> +#define DS_TOUCH_POINT_Y_LO                    GENMASK(3, 0)
>> +#define DS_TOUCH_POINT_Y_HI                    GENMASK(11, 4)
>> +#define DS_TOUCH_POINT_Y(hi, lo)               (FIELD_PREP(DS_TOUCH_POINT_Y_HI, hi) | \
>> +                                                FIELD_PREP(DS_TOUCH_POINT_Y_LO, lo))
>>
>>   /* Magic value required in tag field of Bluetooth output report. */
>> -#define DS_OUTPUT_TAG 0x10
>> +#define DS_OUTPUT_TAG                          0x10
>> +#define DS_OUTPUT_SEQ_TAG                      GENMASK(3, 0)
>> +#define DS_OUTPUT_SEQ_NO                       GENMASK(7, 4)
>>  /* Flags for DualSense output report. */
>> -#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION BIT(0)
>> -#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT BIT(1)
>> -#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE BIT(0)
>> -#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE BIT(1)
>> -#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE BIT(2)
>> -#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
>> -#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
>> -#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
>> -#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
>> -#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
>> -#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
>> +#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION             BIT(0)
>> +#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT                   BIT(1)
>> +#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE      BIT(0)
>> +#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE                BIT(1)
>> +#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE          BIT(2)
>> +#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS                     BIT(3)
>> +#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE  BIT(4)
>> +#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE    BIT(1)
>> +#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2            BIT(2)
>> +#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE                  BIT(4)
>> +#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT                     BIT(1)
>>
>>  /* DualSense hardware limits */
>>  #define DS_ACC_RES_PER_G       8192
>> @@ -315,7 +328,9 @@ struct dualsense_output_report {
>>   * Contact IDs, with highest bit set are 'inactive'
>>   * and any associated data is then invalid.
>>   */
>> -#define DS4_TOUCH_POINT_INACTIVE BIT(7)
>> +#define DS4_TOUCH_POINT_INACTIVE       BIT(7)
>> +#define DS4_TOUCH_POINT_X(hi, lo)      DS_TOUCH_POINT_X(hi, lo)
>> +#define DS4_TOUCH_POINT_Y(hi, lo)      DS_TOUCH_POINT_Y(hi, lo)
>>
>>  /* Status field of DualShock4 input report. */
>>  #define DS4_STATUS0_BATTERY_CAPACITY   GENMASK(3, 0)
>> @@ -1194,7 +1209,8 @@ static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_
>>                  * Highest 4-bit is a sequence number, which needs to be increased
>>                  * every report. Lowest 4-bit is tag and can be zero for now.
>>                  */
>> -               bt->seq_tag = (ds->output_seq << 4) | 0x0;
>> +               bt->seq_tag = FIELD_PREP(DS_OUTPUT_SEQ_NO, ds->output_seq) |
>> +                             FIELD_PREP(DS_OUTPUT_SEQ_TAG, 0x0);
>>                 if (++ds->output_seq == 16)
>>                         ds->output_seq = 0;
>>
>> @@ -1439,11 +1455,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>>                 input_mt_report_slot_state(ds->touchpad, MT_TOOL_FINGER, active);
>>
>>                 if (active) {
>> -                       int x = (point->x_hi << 8) | point->x_lo;
>> -                       int y = (point->y_hi << 4) | point->y_lo;
>> -
>> -                       input_report_abs(ds->touchpad, ABS_MT_POSITION_X, x);
>> -                       input_report_abs(ds->touchpad, ABS_MT_POSITION_Y, y);
>> +                       input_report_abs(ds->touchpad, ABS_MT_POSITION_X,
>> +                                        DS_TOUCH_POINT_X(point->x_hi, point->x_lo));
>> +                       input_report_abs(ds->touchpad, ABS_MT_POSITION_Y,
>> +                                        DS_TOUCH_POINT_Y(point->y_hi, point->y_lo));
>>                 }
>>         }
>>         input_mt_sync_frame(ds->touchpad);
>> @@ -2351,11 +2366,10 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>>                         input_mt_report_slot_state(ds4->touchpad, MT_TOOL_FINGER, active);
>>
>>                         if (active) {
>> -                               int x = (point->x_hi << 8) | point->x_lo;
>> -                               int y = (point->y_hi << 4) | point->y_lo;
>> -
>> -                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_X, x);
>> -                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y, y);
>> +                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_X,
>> +                                                DS4_TOUCH_POINT_X(point->x_hi, point->x_lo));
>> +                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y,
>> +                                                DS4_TOUCH_POINT_Y(point->y_hi, point->y_lo));
>>                         }
>>                 }
>>                 input_mt_sync_frame(ds4->touchpad);
>>
>> --
>> 2.49.0
>>
>>

Re: [PATCH 02/11] HID: playstation: Add spaces around arithmetic operators

[Cristian Ciocaltea]

Hi Roderick,

On 6/10/25 7:19 AM, Roderick Colenbrander wrote:
> Hi Christian,
> 
> We could change some of this. I guess checkpatch changed some output
> since original submission or is more strict in --strict mode. Though I
> have an opinion on these kind of silly patches.

As I've already mentioned earlier, disabling the strict mode would be
an error-prone workflow which I try to avoid as much as possible.

And I'd still prefer to do additional cleanup work and provide these 
kind of "silly" patches, rather then minimizing my diffs and grow the
overall inconsistency.

Thanks,
Cristian

Re: [PATCH 03/11] HID: playstation: Simplify locking with guard() and scoped_guard()

[Cristian Ciocaltea]

Hi Roderick,

On 6/10/25 7:28 AM, Roderick Colenbrander wrote:
> Hi Christian,
> 
> This patch look fine and does simplify some things. Though in terms of
> size the patch is a bit long, so it took some careful looking at the
> code.
Yeah, sorry for the long list of changes - I had a final round of
inspection before sending the patch out just to make sure I haven't
introduced a regression or something.  But that's not a guaranty, 
obviously, hence thanks a lot for checking this out!

Regards,
Cristian

Re: [PATCH 04/11] HID: playstation: Replace uint{32,16,8}_t with u{32,16,8}

[Cristian Ciocaltea]

Hi Roderick,

On 6/10/25 7:30 AM, Roderick Colenbrander wrote:
> Hi Christian,
> 
> I would rather skip this one. The use of uint32_t and family is fine
> and many drivers do the same thing.

That's again about ensuring consistency - even if it's not something
imposed by the maintainer(s), I'd very much prefer to submit patches
which comply with the recommended coding style and, consequently, 
do not generate any checkpatch complaints.

Thanks,
Cristian

Re: [PATCH 09/11] HID: playstation: Rename DualSense input report status field

[Cristian Ciocaltea]

Hi Roderick,

On 6/10/25 7:40 AM, Roderick Colenbrander wrote:
> Hi Christian,
> 
> At the time I didn't document all parts of the HID report as not
> everything was needed. When adding in the audio stuff, let's call this
> one by its official name. There are officially 3 bytes of status, so
> the correct thing is to technically make it a 3 byte array of status.
> The official register names contain DS_STATUS0_.., DS_STATUS1_..,
> etcetera in the name, so I would like to keep things aligned with our
> datasheets.

I don't have access to the datasheet, so thanks for pointing this out!
Will be handled v2.

Thanks,
Cristian

Re: [PATCH 10/11] HID: playstation: Support DualSense audio jack hotplug detection

[Cristian Ciocaltea]

Hi Roderick,

On 6/10/25 8:04 AM, Roderick Colenbrander wrote:
> Hi Christian,
> 
> Thanks for sharing I need to have a closer look to see what we do on
> the console as well (we want to keep behavior where possible in sync
> also due to official PlayStation use cases on embedded devices).
> 
> At least use this logic only for USB, we should not register these
> nodes or apply such settings for Bluetooth. Just setting these on
> bluetooth without audio I could see affecting battery life a bit. But
> in addition, I honestly don't really see a path to supporting
> Bluetooth audio for the DualSense. It would add confusion for users.
> The reason, I don't think there is a good way for adding audio is that
> the DualSense is very, very complex. In Bluetooth mode we have our own
> ways of doing audio (non-standard), which is already tricky. Above all
> else the audio is compressed and not PCM. The only way to do it is
> probably in userspace in something PulseAudio like. So before rushing
> adding this also for Bluetooth, I want to limit this to USB.

Thanks for shedding some light on the Bluetooth side of things - I'll
make sure the introduced changes will be further restricted to USB only.

> I'm sharing some other feedback in line. It is small stuff, but in
> particular I like to keep datasheets and code in sync, so of it is
> around naming. It allows us internally for easier review as well.

Yes, please highlight any DS inconsistencies you noticed and we'll
get them fixed in the next revision.

> There are some areas I need to study some more and get internal
> documentation, so I don't have thorough feedback on the audio code
> itself yet.

No worries, please let me know when I can start preparing v2.

> Thanks,
> Roderick
> 
> On Mon, May 26, 2025 at 6:04 AM Cristian Ciocaltea
> <cristian.ciocaltea@collabora.com> wrote:
>>
>> The default audio output path on DualSense controller hardware is set to
>> headphones, regardless of whether they are actually inserted or not.
>>
>> Detect when the plugged state of the 3.5mm audio jack changes and toggle
>> audio output between headphones and internal speaker, as required.  The
>> latter is achieved by essentially routing the right channel of the audio
>> source to the mono speaker.
>>
>> Additionally, adjust the speaker volume since its default level is too
>> low and, therefore, cannot generate any audible sound.
>>
>> It's worth noting the audio functionality is currently not supported for
>> Bluetooth, hence it's limited to USB connectivity.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/hid/hid-playstation.c | 86 +++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 83 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
>> index b7777d3230b2fe277a4a2217ef6b1eff9cfad182..ed8e67c5bf5d3a9e9721da31a6bd84f0b6802b14 100644
>> --- a/drivers/hid/hid-playstation.c
>> +++ b/drivers/hid/hid-playstation.c
>> @@ -117,6 +117,12 @@ struct ps_led_info {
>>  #define DS_STATUS_BATTERY_CHARGING             GENMASK(7, 4)
>>  #define DS_STATUS_BATTERY_CHARGING_SHIFT       4
>>
>> +/* Plugged status field of DualSense input report. */
>> +#define DS_STATUS_PLUGGED_HEADPHONES           BIT(0)
>> +#define DS_STATUS_PLUGGED_MIC                  BIT(1)
>> +#define DS_STATUS_PLUGGED_JACK                 (DS_STATUS_PLUGGED_HEADPHONES | \
>> +                                                DS_STATUS_PLUGGED_MIC)
> 
> Like I mentioned in patch 10, there are 3 status bytes. So let's call
> this DS_STATUS1_. The other 5 bits in this one are unrelated to audio.
> 
> Official datasheets call these 'HP_DETECT', 'MIC_DETECT' and
> 'MIC_MUTE'. I guess we should use those names.

Yep, will do.

>> +
>>  /* Feature version from DualSense Firmware Info report. */
>>  #define DS_FEATURE_VERSION_MINOR               GENMASK(7, 0)
>>  #define DS_FEATURE_VERSION_MAJOR               GENMASK(15, 8)
>> @@ -144,13 +150,18 @@ struct ps_led_info {
>>  /* Flags for DualSense output report. */
>>  #define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION             BIT(0)
>>  #define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT                   BIT(1)
>> +#define DS_OUTPUT_VALID_FLAG0_SPEAKER_VOLUME_ENABLE            BIT(5)
> Just for completeness also add BIT(6) which is MIC_VOLUME_ENABLE
>> +#define DS_OUTPUT_VALID_FLAG0_AUDIO_CONTROL_ENABLE             BIT(7)
>>  #define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE      BIT(0)
>>  #define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE                BIT(1)
>>  #define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE          BIT(2)
>>  #define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS                     BIT(3)
>>  #define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE  BIT(4)
>> +#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_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)
>>
>> @@ -193,6 +204,11 @@ struct dualsense {
>>         u8 lightbar_green;
>>         u8 lightbar_blue;
>>
>> +       /* Audio Jack plugged state */
>> +       u8 plugged_state;
>> +       u8 prev_plugged_state;
>> +       bool prev_plugged_state_valid;
>> +
>>         /* Microphone */
>>         bool update_mic_mute;
>>         bool mic_muted;
>> @@ -237,7 +253,8 @@ struct dualsense_input_report {
>>
>>         u8 reserved3[12];
>>         u8 status_battery;
>> -       u8 reserved4[10];
>> +       u8 status_plugged;
>> +       u8 reserved4[9];
>>  } __packed;
>>  /* Common input report size shared equals the size of the USB report minus 1 byte for ReportID. */
>>  static_assert(sizeof(struct dualsense_input_report) == DS_INPUT_REPORT_USB_SIZE - 1);
>> @@ -252,11 +269,15 @@ struct dualsense_output_report_common {
>>         u8 motor_left;
>>
>>         /* Audio controls */
>> -       u8 reserved[4];
>> +       u8 headphone_audio_volume;              /* 0-0x7f */
> Just call this headphone_volume

Ack.

>> +       u8 speaker_audio_volume;                /* 0-0xff */
> Just call this speaker_volume

Ack.

>> +       u8 internal_microphone_volume;  /* 0-0x40 */
> We call this one mic_volume

Ack.

>> +       u8 audio_flags;
> We call this one audio_control.

Ack.

> 
>>         u8 mute_button_led;
>>
>>         u8 power_save_control;
>> -       u8 reserved2[28];
>> +       u8 reserved2[27];
>> +       u8 audio_flags2;
> This one we call audio_control2

Ack.

>>
>>         /* LEDs and lightbar */
>>         u8 valid_flag2;

[...]

Thanks for the review,
Cristian