* [PATCH v2 01/11] HID: playstation: Make use of bitfield macros
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 02/11] HID: playstation: Add spaces around arithmetic operators Cristian Ciocaltea
` (10 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
Improve code readability and make it more robust by replacing open coded
bit operations with the equivalent bitfield macros.
While at it, also fix the vertical alignment for some of the bitfield
constants.
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/hid/hid-playstation.c | 77 +++++++++++++++++++++++++------------------
1 file changed, 45 insertions(+), 32 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 1468fb11e39dffc883181663a4ad44252e0a7ebb..79190147fa00b90a4416dd2723c68bcae811684b 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>
@@ -111,34 +112,45 @@ struct ps_led_info {
#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
+#define DS_STATUS_BATTERY_CAPACITY GENMASK(3, 0)
+#define DS_STATUS_CHARGING GENMASK(7, 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 +327,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 +1208,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,19 +1454,18 @@ 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);
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 = FIELD_GET(DS_STATUS_BATTERY_CAPACITY, ds_report->status);
+ charging_status = FIELD_GET(DS_STATUS_CHARGING, ds_report->status);
switch (charging_status) {
case 0x0:
@@ -2351,11 +2365,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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 02/11] HID: playstation: Add spaces around arithmetic operators
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 01/11] HID: playstation: Make use of bitfield macros Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 03/11] HID: playstation: Simplify locking with guard() and scoped_guard() Cristian Ciocaltea
` (9 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
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 79190147fa00b90a4416dd2723c68bcae811684b..799b47cdfe034c2b78ec589ac19e3c7a764dc784 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -154,9 +154,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
@@ -363,9 +363,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
@@ -1015,19 +1015,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);
@@ -1053,19 +1053,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;
/*
@@ -1881,19 +1881,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);
@@ -1904,19 +1904,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:
@@ -2058,8 +2058,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;
@@ -2339,7 +2341,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;
@@ -2348,7 +2350,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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 03/11] HID: playstation: Simplify locking with guard() and scoped_guard()
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 01/11] HID: playstation: Make use of bitfield macros Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 02/11] HID: playstation: Add spaces around arithmetic operators Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 04/11] HID: playstation: Replace uint{32,16,8}_t with u{32,16,8} Cristian Ciocaltea
` (8 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
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 799b47cdfe034c2b78ec589ac19e3c7a764dc784..ab3a0c505c4db9110ae4d528ba70b32d9f90b81b 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>
@@ -566,26 +567,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;
}
@@ -649,13 +649,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:
@@ -1173,19 +1172,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);
@@ -1234,12 +1231,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);
}
/*
@@ -1337,7 +1331,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;
/*
@@ -1399,10 +1392,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);
@@ -1495,10 +1488,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;
}
@@ -1507,16 +1500,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;
@@ -1525,11 +1517,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);
}
@@ -1561,14 +1551,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);
}
@@ -1755,7 +1743,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;
@@ -1774,9 +1761,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)
@@ -2048,26 +2034,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 */
@@ -2081,36 +2064,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);
@@ -2242,7 +2222,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;
/*
@@ -2420,10 +2399,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;
}
@@ -2441,7 +2420,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;
@@ -2450,9 +2428,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);
@@ -2464,9 +2441,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;
@@ -2488,16 +2464,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;
@@ -2506,11 +2481,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);
@@ -2520,12 +2493,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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 04/11] HID: playstation: Replace uint{32,16,8}_t with u{32,16,8}
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
` (2 preceding siblings ...)
2025-06-24 21:56 ` [PATCH v2 03/11] HID: playstation: Simplify locking with guard() and scoped_guard() Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 05/11] HID: playstation: Correct spelling in comment sections Cristian Ciocaltea
` (7 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
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 ab3a0c505c4db9110ae4d528ba70b32d9f90b81b..78d28a0eb697ecb3d0ad7601c89383b67f9a53d8 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);
@@ -168,7 +168,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];
@@ -176,21 +176,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;
@@ -199,90 +199,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);
@@ -292,8 +292,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;
@@ -395,26 +395,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;
@@ -423,88 +423,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);
@@ -515,8 +515,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;
@@ -555,7 +555,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);
/*
@@ -647,7 +647,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;
@@ -710,9 +710,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);
@@ -758,7 +758,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;
@@ -782,8 +782,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);
@@ -976,7 +976,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)
@@ -1090,7 +1090,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);
@@ -1123,7 +1123,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);
@@ -1149,7 +1149,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;
@@ -1247,8 +1247,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);
@@ -1327,9 +1327,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;
@@ -1344,7 +1344,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");
@@ -1427,7 +1427,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);
@@ -1527,7 +1527,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)
@@ -1549,7 +1549,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;
@@ -1577,7 +1577,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];
@@ -1588,7 +1588,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[] = {
@@ -1779,7 +1779,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;
@@ -1943,7 +1943,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);
@@ -1971,7 +1971,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) {
@@ -2190,8 +2190,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.
@@ -2219,9 +2219,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;
/*
@@ -2239,7 +2239,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)) {
@@ -2323,7 +2323,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);
@@ -2368,7 +2368,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,
@@ -2389,7 +2389,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;
@@ -2498,7 +2498,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;
@@ -2518,7 +2518,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];
@@ -2533,7 +2533,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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 05/11] HID: playstation: Correct spelling in comment sections
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
` (3 preceding siblings ...)
2025-06-24 21:56 ` [PATCH v2 04/11] HID: playstation: Replace uint{32,16,8}_t with u{32,16,8} Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 06/11] HID: playstation: Fix all alignment and line length issues Cristian Ciocaltea
` (6 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
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 78d28a0eb697ecb3d0ad7601c89383b67f9a53d8..349d6cac964a5b4fb1a311498b9573e03a3b7a01 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -338,7 +338,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)
@@ -2169,7 +2169,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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 06/11] HID: playstation: Fix all alignment and line length issues
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
` (4 preceding siblings ...)
2025-06-24 21:56 ` [PATCH v2 05/11] HID: playstation: Correct spelling in comment sections Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 07/11] HID: playstation: Document spinlock_t usage Cristian Ciocaltea
` (5 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
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 349d6cac964a5b4fb1a311498b9573e03a3b7a01..811b0b90c0e277fe6a511a9a766790125d9602c5 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -572,7 +572,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;
}
}
@@ -607,7 +607,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;
@@ -622,8 +623,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 {
@@ -643,8 +644,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;
@@ -688,7 +689,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;
@@ -720,8 +721,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;
@@ -758,8 +760,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;
@@ -795,17 +797,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)
@@ -829,7 +834,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;
@@ -850,7 +855,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;
@@ -866,8 +871,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;
@@ -903,8 +908,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;
@@ -932,8 +937,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);
@@ -944,8 +948,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);
@@ -983,7 +986,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;
@@ -1037,8 +1040,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;
@@ -1074,8 +1078,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;
@@ -1087,7 +1092,6 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
return ret;
}
-
static int dualsense_get_firmware_info(struct dualsense *ds)
{
u8 *buf;
@@ -1098,7 +1102,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;
@@ -1131,7 +1135,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;
@@ -1145,7 +1149,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);
@@ -1189,8 +1193,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;
@@ -1241,7 +1245,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;
@@ -1322,7 +1326,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);
@@ -1339,10 +1343,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]);
@@ -1677,7 +1681,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;
@@ -1731,7 +1735,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;
@@ -1754,7 +1758,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");
@@ -1798,14 +1803,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 {
@@ -1820,7 +1828,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);
@@ -1914,8 +1922,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;
@@ -1930,8 +1939,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;
@@ -1954,7 +1964,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;
@@ -1980,7 +1990,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;
@@ -1993,9 +2003,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;
@@ -2030,7 +2040,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);
@@ -2098,7 +2108,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;
@@ -2213,7 +2223,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);
@@ -2230,14 +2240,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);
@@ -2408,7 +2419,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;
@@ -2419,7 +2430,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;
@@ -2546,11 +2558,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);
@@ -2620,7 +2635,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;
@@ -2659,7 +2674,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:
@@ -2668,7 +2683,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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 07/11] HID: playstation: Document spinlock_t usage
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
` (5 preceding siblings ...)
2025-06-24 21:56 ` [PATCH v2 06/11] HID: playstation: Fix all alignment and line length issues Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 08/11] HID: playstation: Prefer kzalloc(sizeof(*buf)...) Cristian Ciocaltea
` (4 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
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 811b0b90c0e277fe6a511a9a766790125d9602c5..37fc1a74517c1f14a7bae9a1fa90f7d64115a605 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 08/11] HID: playstation: Prefer kzalloc(sizeof(*buf)...)
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
` (6 preceding siblings ...)
2025-06-24 21:56 ` [PATCH v2 07/11] HID: playstation: Document spinlock_t usage Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 09/11] HID: playstation: Redefine DualSense input report status field Cristian Ciocaltea
` (3 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
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 37fc1a74517c1f14a7bae9a1fa90f7d64115a605..6e3132418f2ef88f49d7ea55eb1f52ddf53680c1 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1531,9 +1531,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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 09/11] HID: playstation: Redefine DualSense input report status field
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
` (7 preceding siblings ...)
2025-06-24 21:56 ` [PATCH v2 08/11] HID: playstation: Prefer kzalloc(sizeof(*buf)...) Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 10/11] HID: playstation: Support DualSense audio jack hotplug detection Cristian Ciocaltea
` (2 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
The 'status' member of struct dualsense_input_report is currently used
to store just the battery data, despite the fact that hardware is
capable to report two extra bytes of status information.
In preparation to make use of some of the additional data, redefine the
field type as a 3-byte array.
Moreover, to preserve consistency with the related DS_STATUS[0..2]_*
registers in datasheet, rename DS_STATUS_* bitfield macros accordingly.
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/hid/hid-playstation.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 6e3132418f2ef88f49d7ea55eb1f52ddf53680c1..f523ca989a2f1d085a460c4e05db00599d19492d 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -112,9 +112,9 @@ 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)
+/* Battery status field of DualSense input report. */
+#define DS_STATUS0_BATTERY_CAPACITY GENMASK(3, 0)
+#define DS_STATUS0_CHARGING GENMASK(7, 4)
/* Feature version from DualSense Firmware Info report. */
#define DS_FEATURE_VERSION_MINOR GENMASK(7, 0)
@@ -235,8 +235,8 @@ struct dualsense_input_report {
struct dualsense_touch_point points[2];
u8 reserved3[12];
- u8 status;
- u8 reserved4[10];
+ u8 status[3];
+ u8 reserved4[8];
} __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);
@@ -1461,8 +1461,8 @@ 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 = FIELD_GET(DS_STATUS_BATTERY_CAPACITY, ds_report->status);
- charging_status = FIELD_GET(DS_STATUS_CHARGING, ds_report->status);
+ battery_data = FIELD_GET(DS_STATUS0_BATTERY_CAPACITY, ds_report->status[0]);
+ charging_status = FIELD_GET(DS_STATUS0_CHARGING, ds_report->status[0]);
switch (charging_status) {
case 0x0:
--
2.49.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 10/11] HID: playstation: Support DualSense audio jack hotplug detection
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
` (8 preceding siblings ...)
2025-06-24 21:56 ` [PATCH v2 09/11] HID: playstation: Redefine DualSense input report status field Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-06-24 21:56 ` [PATCH v2 11/11] HID: playstation: Support DualSense audio jack event reporting Cristian Ciocaltea
2025-07-03 7:48 ` [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Jiri Kosina
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
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 | 91 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 88 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index f523ca989a2f1d085a460c4e05db00599d19492d..4285260c7e22bc4097bc755f31a968f57ba16db8 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -112,9 +112,13 @@ struct ps_led_info {
#define DS_BUTTONS2_TOUCHPAD BIT(1)
#define DS_BUTTONS2_MIC_MUTE BIT(2)
-/* Battery status field of DualSense input report. */
+/* Status fields of DualSense input report. */
#define DS_STATUS0_BATTERY_CAPACITY GENMASK(3, 0)
#define DS_STATUS0_CHARGING GENMASK(7, 4)
+#define DS_STATUS1_HP_DETECT BIT(0)
+#define DS_STATUS1_MIC_DETECT BIT(1)
+#define DS_STATUS1_JACK_DETECT (DS_STATUS1_HP_DETECT | DS_STATUS1_MIC_DETECT)
+#define DS_STATUS1_MIC_MUTE BIT(2)
/* Feature version from DualSense Firmware Info report. */
#define DS_FEATURE_VERSION_MINOR GENMASK(7, 0)
@@ -143,13 +147,19 @@ 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_MIC_VOLUME_ENABLE BIT(6)
+#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)
@@ -192,6 +202,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;
@@ -251,11 +266,15 @@ struct dualsense_output_report_common {
u8 motor_left;
/* Audio controls */
- u8 reserved[4];
+ u8 headphone_volume; /* 0x0 - 0x7f */
+ u8 speaker_volume; /* 0x0 - 0xff */
+ u8 mic_volume; /* 0x0 - 0x40 */
+ u8 audio_control;
u8 mute_button_led;
u8 power_save_control;
- u8 reserved2[28];
+ u8 reserved2[27];
+ u8 audio_control2;
/* LEDs and lightbar */
u8 valid_flag2;
@@ -1303,6 +1322,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_STATUS1_HP_DETECT;
+
+ if (val != (ds->prev_plugged_state & DS_STATUS1_HP_DETECT)) {
+ 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_control = 0;
+ } else {
+ /* Mute HP and route R channel to SP */
+ common->audio_control =
+ 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_volume = 0x64;
+ /* Set SP preamp gain to ~30% */
+ common->valid_flag1 = DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE;
+ common->audio_control2 =
+ 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;
@@ -1406,6 +1465,32 @@ 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 for USB use case, since Bluetooth
+ * audio is currently not supported.
+ */
+ if (hdev->bus == BUS_USB) {
+ value = ds_report->status[1] & DS_STATUS1_JACK_DETECT;
+
+ 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_STATUS1_JACK_DETECT;
+ 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 11/11] HID: playstation: Support DualSense audio jack event reporting
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
` (9 preceding siblings ...)
2025-06-24 21:56 ` [PATCH v2 10/11] HID: playstation: Support DualSense audio jack hotplug detection Cristian Ciocaltea
@ 2025-06-24 21:56 ` Cristian Ciocaltea
2025-07-03 7:48 ` [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Jiri Kosina
11 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-06-24 21:56 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg
Cc: kernel, linux-input, linux-kernel
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.
When operating in USB mode, 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 | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 4285260c7e22bc4097bc755f31a968f57ba16db8..d2bee1a314b1b4697378d8472d3f65df5d2cf790 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -176,6 +176,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;
@@ -955,6 +956,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)
{
@@ -1357,8 +1377,15 @@ static void dualsense_output_worker(struct work_struct *work)
common->audio_control2 =
FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN, 0x2);
}
+
+ input_report_switch(ds->jack, SW_HEADPHONE_INSERT, val);
}
+ val = ds->plugged_state & DS_STATUS1_MIC_DETECT;
+ if (val != (ds->prev_plugged_state & DS_STATUS1_MIC_DETECT))
+ input_report_switch(ds->jack, SW_MICROPHONE_INSERT, val);
+
+ input_sync(ds->jack);
ds->prev_plugged_state = ds->plugged_state;
}
@@ -1778,6 +1805,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
goto err;
}
+ /* Bluetooth audio is currently not supported. */
+ if (hdev->bus == BUS_USB) {
+ 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-06-24 21:56 [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Cristian Ciocaltea
` (10 preceding siblings ...)
2025-06-24 21:56 ` [PATCH v2 11/11] HID: playstation: Support DualSense audio jack event reporting Cristian Ciocaltea
@ 2025-07-03 7:48 ` Jiri Kosina
2025-07-10 5:24 ` Cristian Ciocaltea
11 siblings, 1 reply; 23+ messages in thread
From: Jiri Kosina @ 2025-07-03 7:48 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Roderick Colenbrander, Benjamin Tissoires, Henrik Rydberg, kernel,
linux-input, linux-kernel
On Wed, 25 Jun 2025, Cristian Ciocaltea 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 when operating in
> USB mode, 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. That's exactly what has been accomplished
> via the "ALSA: usb-audio: Support jack detection on Sony DualSense"
> patchset [1], which has been already merged and should be available in
> v6.17.
>
> Unrelated to the above, also provide a few driver cleanup patches, e.g.
> make use of bitfields macros, simplify locking, fix coding style.
>
> [1] https://lore.kernel.org/all/20250526-dualsense-alsa-jack-v1-0-1a821463b632@collabora.com/
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
> Changes in v2:
> - Updated cover letter including a reference to the usb-audio patch series
> - Updated 'HID: playstation: Make use of bitfield macros' patch to drop
> DS_STATUS_CHARGING_SHIFT and use FIELD_GET() for battery status ops
> - Replaced 'HID: playstation: Rename DualSense input report status
> field' with 'HID: playstation: Redefine DualSense input report status
> field' changing data type to a 3-byte array instead of renaming the
> struct member (Roderick)
> - Updated 'HID: playstation: Support DualSense audio jack hotplug
> detection' according to Roderick's feedback:
> * Used DS_STATUS1_ prefixes for the plugged status register and rename
> its bits to match the datasheet
> * Defined MIC_VOLUME_ENABLE bit of DS_OUTPUT_VALID_FLAG0 register
> * Renamed the newly introduced audio controls members in struct
> dualsense_output_report_common: headphone_volume, speaker_volume,
> mic_volume, audio_control, audio_control2
> - Restricted audio jack hotplug detection and event reporting to USB
> operation mode only, since Bluetooth audio is currently not supported
> and it might have a negative impact on the battery life (Roderick)
> - Rebased series onto next-20250624
> - Link to v1: https://lore.kernel.org/r/20250526-dualsense-hid-jack-v1-0-a65fee4a60cc@collabora.com
Just for the record -- I like the v2, and am inclined to merge it, but
would prefer doing that with Roderick's Ack, so I am waiting for a bit
here.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-03 7:48 ` [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense Jiri Kosina
@ 2025-07-10 5:24 ` Cristian Ciocaltea
2025-07-10 21:31 ` Roderick Colenbrander
0 siblings, 1 reply; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-07-10 5:24 UTC (permalink / raw)
To: Roderick Colenbrander, Roderick Colenbrander
Cc: Jiri Kosina, Benjamin Tissoires, Henrik Rydberg, kernel,
linux-input, linux-kernel
Hi Roderick,
On 7/3/25 10:48 AM, Jiri Kosina wrote:
> On Wed, 25 Jun 2025, Cristian Ciocaltea 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 when operating in
>> USB mode, 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. That's exactly what has been accomplished
>> via the "ALSA: usb-audio: Support jack detection on Sony DualSense"
>> patchset [1], which has been already merged and should be available in
>> v6.17.
>>
>> Unrelated to the above, also provide a few driver cleanup patches, e.g.
>> make use of bitfields macros, simplify locking, fix coding style.
>>
>> [1] https://lore.kernel.org/all/20250526-dualsense-alsa-jack-v1-0-1a821463b632@collabora.com/
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>> Changes in v2:
>> - Updated cover letter including a reference to the usb-audio patch series
>> - Updated 'HID: playstation: Make use of bitfield macros' patch to drop
>> DS_STATUS_CHARGING_SHIFT and use FIELD_GET() for battery status ops
>> - Replaced 'HID: playstation: Rename DualSense input report status
>> field' with 'HID: playstation: Redefine DualSense input report status
>> field' changing data type to a 3-byte array instead of renaming the
>> struct member (Roderick)
>> - Updated 'HID: playstation: Support DualSense audio jack hotplug
>> detection' according to Roderick's feedback:
>> * Used DS_STATUS1_ prefixes for the plugged status register and rename
>> its bits to match the datasheet
>> * Defined MIC_VOLUME_ENABLE bit of DS_OUTPUT_VALID_FLAG0 register
>> * Renamed the newly introduced audio controls members in struct
>> dualsense_output_report_common: headphone_volume, speaker_volume,
>> mic_volume, audio_control, audio_control2
>> - Restricted audio jack hotplug detection and event reporting to USB
>> operation mode only, since Bluetooth audio is currently not supported
>> and it might have a negative impact on the battery life (Roderick)
>> - Rebased series onto next-20250624
>> - Link to v1: https://lore.kernel.org/r/20250526-dualsense-hid-jack-v1-0-a65fee4a60cc@collabora.com
>
> Just for the record -- I like the v2, and am inclined to merge it, but
> would prefer doing that with Roderick's Ack, so I am waiting for a bit
> here.
Could you please confirm you are fine with the latest changes so that Jiri
is able to merge the series?
If you cannot find the time to look into every detail right now, we can
still take care of any non-essential matters afterwards.
Thanks,
Cristian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-10 5:24 ` Cristian Ciocaltea
@ 2025-07-10 21:31 ` Roderick Colenbrander
2025-07-22 5:47 ` Roderick Colenbrander
0 siblings, 1 reply; 23+ messages in thread
From: Roderick Colenbrander @ 2025-07-10 21:31 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
Hi Cristian,
I'm on a business trip to Japan, I started looking during my flight
last weekend. But due to meetings haven't had a lot of opportunity. I
may have a little bit of time in the coming days in between meetings.
Thanks,
Roderick
On Wed, Jul 9, 2025 at 10:24 PM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> Hi Roderick,
>
> On 7/3/25 10:48 AM, Jiri Kosina wrote:
> > On Wed, 25 Jun 2025, Cristian Ciocaltea 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 when operating in
> >> USB mode, 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. That's exactly what has been accomplished
> >> via the "ALSA: usb-audio: Support jack detection on Sony DualSense"
> >> patchset [1], which has been already merged and should be available in
> >> v6.17.
> >>
> >> Unrelated to the above, also provide a few driver cleanup patches, e.g.
> >> make use of bitfields macros, simplify locking, fix coding style.
> >>
> >> [1] https://lore.kernel.org/all/20250526-dualsense-alsa-jack-v1-0-1a821463b632@collabora.com/
> >>
> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >> ---
> >> Changes in v2:
> >> - Updated cover letter including a reference to the usb-audio patch series
> >> - Updated 'HID: playstation: Make use of bitfield macros' patch to drop
> >> DS_STATUS_CHARGING_SHIFT and use FIELD_GET() for battery status ops
> >> - Replaced 'HID: playstation: Rename DualSense input report status
> >> field' with 'HID: playstation: Redefine DualSense input report status
> >> field' changing data type to a 3-byte array instead of renaming the
> >> struct member (Roderick)
> >> - Updated 'HID: playstation: Support DualSense audio jack hotplug
> >> detection' according to Roderick's feedback:
> >> * Used DS_STATUS1_ prefixes for the plugged status register and rename
> >> its bits to match the datasheet
> >> * Defined MIC_VOLUME_ENABLE bit of DS_OUTPUT_VALID_FLAG0 register
> >> * Renamed the newly introduced audio controls members in struct
> >> dualsense_output_report_common: headphone_volume, speaker_volume,
> >> mic_volume, audio_control, audio_control2
> >> - Restricted audio jack hotplug detection and event reporting to USB
> >> operation mode only, since Bluetooth audio is currently not supported
> >> and it might have a negative impact on the battery life (Roderick)
> >> - Rebased series onto next-20250624
> >> - Link to v1: https://lore.kernel.org/r/20250526-dualsense-hid-jack-v1-0-a65fee4a60cc@collabora.com
> >
> > Just for the record -- I like the v2, and am inclined to merge it, but
> > would prefer doing that with Roderick's Ack, so I am waiting for a bit
> > here.
>
> Could you please confirm you are fine with the latest changes so that Jiri
> is able to merge the series?
>
> If you cannot find the time to look into every detail right now, we can
> still take care of any non-essential matters afterwards.
>
> Thanks,
> Cristian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-10 21:31 ` Roderick Colenbrander
@ 2025-07-22 5:47 ` Roderick Colenbrander
2025-07-22 6:18 ` Roderick Colenbrander
2025-07-22 7:24 ` Cristian Ciocaltea
0 siblings, 2 replies; 23+ messages in thread
From: Roderick Colenbrander @ 2025-07-22 5:47 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
Hi Christian,
I just got back from Japan (trip was a bit extended). In the meantime
I had some of employees had a look as well.
The audio patches towards the end seem to be okay. We tried to dig for
the official volume numbers, but they were too hard to find (too many
layers, too many repositories). When we use a PS5, the default volume
for the headset and speaker are both close to 70% (just eyeballing).
At the hardware level the volume is quite non-linear and internally we
use a mapping table (not sure what the curve is based on). For the
speaker this starts at 0x3d as you found out already. The 70% volume
for the speaker seems to correspond to a value of 93 and headphones
83.
The set pre-amp gain of 0x2 is a common value we seem to set and means
+6dB, so change comment around to mean that I guess.
As for the other patches I'm not entirely sure yet. I know they were
well intended, but let me just say, they rubbed some of my team
members quite the wrong way resulting in some heavy discussion. I have
somewhat similar feelings about the ultra strict checkpatch toggle as
well.
We had to move mountains to be allowed to even upstream controller
code among our limited time (it is closer to a hobby thing, even
though many products nowadays use it as well). So that's a factor
which adds up a bit as well.
I think some of the patches we could live with if it came to it. There
is no real agreed up full kernel standard (as it is contentious). So
for example we tend to prefer more uint8_t family, where older kernel
style was more u8 and the kernel allows for both. I think we would
probably lean towards keeping it at the modern form.
Some of the macros also felt a little too magical. Our feeling tends
to be if you have to go many layers deep to understand what a macro or
line of code does (and it is easier to then printk the value),
something feels off...
Thanks,
Roderick Colenbrander
Sr. Director - Hardware & Systems Engineering
Sony Interactive Entertainment, LLC
On Thu, Jul 10, 2025 at 2:31 PM Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
>
> Hi Cristian,
>
> I'm on a business trip to Japan, I started looking during my flight
> last weekend. But due to meetings haven't had a lot of opportunity. I
> may have a little bit of time in the coming days in between meetings.
>
> Thanks,
> Roderick
>
> On Wed, Jul 9, 2025 at 10:24 PM Cristian Ciocaltea
> <cristian.ciocaltea@collabora.com> wrote:
> >
> > Hi Roderick,
> >
> > On 7/3/25 10:48 AM, Jiri Kosina wrote:
> > > On Wed, 25 Jun 2025, Cristian Ciocaltea 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 when operating in
> > >> USB mode, 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. That's exactly what has been accomplished
> > >> via the "ALSA: usb-audio: Support jack detection on Sony DualSense"
> > >> patchset [1], which has been already merged and should be available in
> > >> v6.17.
> > >>
> > >> Unrelated to the above, also provide a few driver cleanup patches, e.g.
> > >> make use of bitfields macros, simplify locking, fix coding style.
> > >>
> > >> [1] https://lore.kernel.org/all/20250526-dualsense-alsa-jack-v1-0-1a821463b632@collabora.com/
> > >>
> > >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > >> ---
> > >> Changes in v2:
> > >> - Updated cover letter including a reference to the usb-audio patch series
> > >> - Updated 'HID: playstation: Make use of bitfield macros' patch to drop
> > >> DS_STATUS_CHARGING_SHIFT and use FIELD_GET() for battery status ops
> > >> - Replaced 'HID: playstation: Rename DualSense input report status
> > >> field' with 'HID: playstation: Redefine DualSense input report status
> > >> field' changing data type to a 3-byte array instead of renaming the
> > >> struct member (Roderick)
> > >> - Updated 'HID: playstation: Support DualSense audio jack hotplug
> > >> detection' according to Roderick's feedback:
> > >> * Used DS_STATUS1_ prefixes for the plugged status register and rename
> > >> its bits to match the datasheet
> > >> * Defined MIC_VOLUME_ENABLE bit of DS_OUTPUT_VALID_FLAG0 register
> > >> * Renamed the newly introduced audio controls members in struct
> > >> dualsense_output_report_common: headphone_volume, speaker_volume,
> > >> mic_volume, audio_control, audio_control2
> > >> - Restricted audio jack hotplug detection and event reporting to USB
> > >> operation mode only, since Bluetooth audio is currently not supported
> > >> and it might have a negative impact on the battery life (Roderick)
> > >> - Rebased series onto next-20250624
> > >> - Link to v1: https://lore.kernel.org/r/20250526-dualsense-hid-jack-v1-0-a65fee4a60cc@collabora.com
> > >
> > > Just for the record -- I like the v2, and am inclined to merge it, but
> > > would prefer doing that with Roderick's Ack, so I am waiting for a bit
> > > here.
> >
> > Could you please confirm you are fine with the latest changes so that Jiri
> > is able to merge the series?
> >
> > If you cannot find the time to look into every detail right now, we can
> > still take care of any non-essential matters afterwards.
> >
> > Thanks,
> > Cristian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-22 5:47 ` Roderick Colenbrander
@ 2025-07-22 6:18 ` Roderick Colenbrander
2025-07-22 8:03 ` Cristian Ciocaltea
2025-07-22 7:24 ` Cristian Ciocaltea
1 sibling, 1 reply; 23+ messages in thread
From: Roderick Colenbrander @ 2025-07-22 6:18 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
Hi Cristian and Jiri,
One thing I forgot to bring up is whether it is best to have the audio
plug logic have its separate input device or have it be part of an
existing (e.g. main gamepad). The patch currently creates a separate
input device. Originally we added multiple input devices (gamepad,
touchpad and sensors) due to axes and button collisions really.
For this feature there is no collision. There are not many devices in
the kernel, which support these audio EV_SW. I see for example the
Switch 2 controller has a mini jack port as well. Some xbox
controllers too (though audio not supported in the kernel from a quick
glance or at least no HID or xpad driver features for them).
I don't have a strong opinion yet. Initial feeling was perhaps have it
on the 'main' input device. But on the other hand, I'm not sure what
software is normally listening for these kinds of EV_SW events. What
would be listening for this like a pipewire?
Thanks,
Roderick
On Mon, Jul 21, 2025 at 10:47 PM Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
>
> Hi Christian,
>
> I just got back from Japan (trip was a bit extended). In the meantime
> I had some of employees had a look as well.
>
> The audio patches towards the end seem to be okay. We tried to dig for
> the official volume numbers, but they were too hard to find (too many
> layers, too many repositories). When we use a PS5, the default volume
> for the headset and speaker are both close to 70% (just eyeballing).
> At the hardware level the volume is quite non-linear and internally we
> use a mapping table (not sure what the curve is based on). For the
> speaker this starts at 0x3d as you found out already. The 70% volume
> for the speaker seems to correspond to a value of 93 and headphones
> 83.
> The set pre-amp gain of 0x2 is a common value we seem to set and means
> +6dB, so change comment around to mean that I guess.
>
> As for the other patches I'm not entirely sure yet. I know they were
> well intended, but let me just say, they rubbed some of my team
> members quite the wrong way resulting in some heavy discussion. I have
> somewhat similar feelings about the ultra strict checkpatch toggle as
> well.
>
> We had to move mountains to be allowed to even upstream controller
> code among our limited time (it is closer to a hobby thing, even
> though many products nowadays use it as well). So that's a factor
> which adds up a bit as well.
>
> I think some of the patches we could live with if it came to it. There
> is no real agreed up full kernel standard (as it is contentious). So
> for example we tend to prefer more uint8_t family, where older kernel
> style was more u8 and the kernel allows for both. I think we would
> probably lean towards keeping it at the modern form.
>
> Some of the macros also felt a little too magical. Our feeling tends
> to be if you have to go many layers deep to understand what a macro or
> line of code does (and it is easier to then printk the value),
> something feels off...
>
> Thanks,
>
> Roderick Colenbrander
> Sr. Director - Hardware & Systems Engineering
> Sony Interactive Entertainment, LLC
>
> On Thu, Jul 10, 2025 at 2:31 PM Roderick Colenbrander
> <thunderbird2k@gmail.com> wrote:
> >
> > Hi Cristian,
> >
> > I'm on a business trip to Japan, I started looking during my flight
> > last weekend. But due to meetings haven't had a lot of opportunity. I
> > may have a little bit of time in the coming days in between meetings.
> >
> > Thanks,
> > Roderick
> >
> > On Wed, Jul 9, 2025 at 10:24 PM Cristian Ciocaltea
> > <cristian.ciocaltea@collabora.com> wrote:
> > >
> > > Hi Roderick,
> > >
> > > On 7/3/25 10:48 AM, Jiri Kosina wrote:
> > > > On Wed, 25 Jun 2025, Cristian Ciocaltea 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 when operating in
> > > >> USB mode, 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. That's exactly what has been accomplished
> > > >> via the "ALSA: usb-audio: Support jack detection on Sony DualSense"
> > > >> patchset [1], which has been already merged and should be available in
> > > >> v6.17.
> > > >>
> > > >> Unrelated to the above, also provide a few driver cleanup patches, e.g.
> > > >> make use of bitfields macros, simplify locking, fix coding style.
> > > >>
> > > >> [1] https://lore.kernel.org/all/20250526-dualsense-alsa-jack-v1-0-1a821463b632@collabora.com/
> > > >>
> > > >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > > >> ---
> > > >> Changes in v2:
> > > >> - Updated cover letter including a reference to the usb-audio patch series
> > > >> - Updated 'HID: playstation: Make use of bitfield macros' patch to drop
> > > >> DS_STATUS_CHARGING_SHIFT and use FIELD_GET() for battery status ops
> > > >> - Replaced 'HID: playstation: Rename DualSense input report status
> > > >> field' with 'HID: playstation: Redefine DualSense input report status
> > > >> field' changing data type to a 3-byte array instead of renaming the
> > > >> struct member (Roderick)
> > > >> - Updated 'HID: playstation: Support DualSense audio jack hotplug
> > > >> detection' according to Roderick's feedback:
> > > >> * Used DS_STATUS1_ prefixes for the plugged status register and rename
> > > >> its bits to match the datasheet
> > > >> * Defined MIC_VOLUME_ENABLE bit of DS_OUTPUT_VALID_FLAG0 register
> > > >> * Renamed the newly introduced audio controls members in struct
> > > >> dualsense_output_report_common: headphone_volume, speaker_volume,
> > > >> mic_volume, audio_control, audio_control2
> > > >> - Restricted audio jack hotplug detection and event reporting to USB
> > > >> operation mode only, since Bluetooth audio is currently not supported
> > > >> and it might have a negative impact on the battery life (Roderick)
> > > >> - Rebased series onto next-20250624
> > > >> - Link to v1: https://lore.kernel.org/r/20250526-dualsense-hid-jack-v1-0-a65fee4a60cc@collabora.com
> > > >
> > > > Just for the record -- I like the v2, and am inclined to merge it, but
> > > > would prefer doing that with Roderick's Ack, so I am waiting for a bit
> > > > here.
> > >
> > > Could you please confirm you are fine with the latest changes so that Jiri
> > > is able to merge the series?
> > >
> > > If you cannot find the time to look into every detail right now, we can
> > > still take care of any non-essential matters afterwards.
> > >
> > > Thanks,
> > > Cristian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-22 6:18 ` Roderick Colenbrander
@ 2025-07-22 8:03 ` Cristian Ciocaltea
2025-07-23 4:04 ` Roderick Colenbrander
0 siblings, 1 reply; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-07-22 8:03 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
Hi Roderick,
On 7/22/25 9:18 AM, Roderick Colenbrander wrote:
> Hi Cristian and Jiri,
>
> One thing I forgot to bring up is whether it is best to have the audio
> plug logic have its separate input device or have it be part of an
> existing (e.g. main gamepad). The patch currently creates a separate
> input device. Originally we added multiple input devices (gamepad,
> touchpad and sensors) due to axes and button collisions really.
>
> For this feature there is no collision. There are not many devices in
> the kernel, which support these audio EV_SW. I see for example the
> Switch 2 controller has a mini jack port as well. Some xbox
> controllers too (though audio not supported in the kernel from a quick
> glance or at least no HID or xpad driver features for them).
>
> I don't have a strong opinion yet. Initial feeling was perhaps have it
> on the 'main' input device. But on the other hand, I'm not sure what
> software is normally listening for these kinds of EV_SW events. What
> would be listening for this like a pipewire?
For now this is going to be used by the usb-audio driver which contains a
quirk [1] creating the jack controls for headphone and headset mic,
respectively. This will further setup an input handler for each of them in
order to intercept the related hotplug events.
But it can be also used directly from ALSA/pipewire, e.g. for monitoring,
hence it think it's best to keep it as an audio dedicated input device.
Regards,
Cristian
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/usb/mixer_quirks.c#n540
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-22 8:03 ` Cristian Ciocaltea
@ 2025-07-23 4:04 ` Roderick Colenbrander
2025-07-23 4:20 ` Roderick Colenbrander
2025-07-23 7:17 ` Cristian Ciocaltea
0 siblings, 2 replies; 23+ messages in thread
From: Roderick Colenbrander @ 2025-07-23 4:04 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
Hi Cristian,
Thanks for the information on the audio patches in the sound tree. We
weren't familiar with that part.
I talked a bit with my team members as well. In general audio is
getting some bigger attention (will see where that goes). I'm getting
a bit worried that the HID and usb driver need much closer coupling,
the current coupling not being enough.
I don't know the USB audio spec too well, but it is more on the
digital interface and a DAC. I'm not sure on the exact circuitry on
the DualSense, but there is a lot of logic in the console drivers for
volume handling where adjustment of the volume talks to the HID layer
to send a new output report. I suspect they had very good reasons for
it (e.g. for headphone also dealing with different impedances).
So I'm not sure how the volume control is really supposed to work, but
I would think to do it properly it requires some interaction between
the audio and HID drivers. Just letting the audio side do it right
now, is more about leveraging the range of the DAC I guess versus a
proper audio amplification stage.
Just thinking of things from the user perspective, they should have a
unified volume control. I don't know how other devices are doing it,
but I think we need to think a bit further and we need to reconsider
how things work....
Thanks,
Roderick
On Tue, Jul 22, 2025 at 1:03 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> Hi Roderick,
>
> On 7/22/25 9:18 AM, Roderick Colenbrander wrote:
> > Hi Cristian and Jiri,
> >
> > One thing I forgot to bring up is whether it is best to have the audio
> > plug logic have its separate input device or have it be part of an
> > existing (e.g. main gamepad). The patch currently creates a separate
> > input device. Originally we added multiple input devices (gamepad,
> > touchpad and sensors) due to axes and button collisions really.
> >
> > For this feature there is no collision. There are not many devices in
> > the kernel, which support these audio EV_SW. I see for example the
> > Switch 2 controller has a mini jack port as well. Some xbox
> > controllers too (though audio not supported in the kernel from a quick
> > glance or at least no HID or xpad driver features for them).
> >
> > I don't have a strong opinion yet. Initial feeling was perhaps have it
> > on the 'main' input device. But on the other hand, I'm not sure what
> > software is normally listening for these kinds of EV_SW events. What
> > would be listening for this like a pipewire?
>
> For now this is going to be used by the usb-audio driver which contains a
> quirk [1] creating the jack controls for headphone and headset mic,
> respectively. This will further setup an input handler for each of them in
> order to intercept the related hotplug events.
>
> But it can be also used directly from ALSA/pipewire, e.g. for monitoring,
> hence it think it's best to keep it as an audio dedicated input device.
>
> Regards,
> Cristian
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/usb/mixer_quirks.c#n540
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-23 4:04 ` Roderick Colenbrander
@ 2025-07-23 4:20 ` Roderick Colenbrander
2025-07-23 7:17 ` Cristian Ciocaltea
1 sibling, 0 replies; 23+ messages in thread
From: Roderick Colenbrander @ 2025-07-23 4:20 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
I have asked someone in Tokyo for some more clarification as well. As
I said my worry is that it really isn't this simple...
Separately we are now starting to formally study Bluetooth audio. That
one is difficult, but at least less messy on the plumbing side as it
is all HID based.
Thanks,
Roderick
On Tue, Jul 22, 2025 at 9:04 PM Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
>
> Hi Cristian,
>
> Thanks for the information on the audio patches in the sound tree. We
> weren't familiar with that part.
>
> I talked a bit with my team members as well. In general audio is
> getting some bigger attention (will see where that goes). I'm getting
> a bit worried that the HID and usb driver need much closer coupling,
> the current coupling not being enough.
>
> I don't know the USB audio spec too well, but it is more on the
> digital interface and a DAC. I'm not sure on the exact circuitry on
> the DualSense, but there is a lot of logic in the console drivers for
> volume handling where adjustment of the volume talks to the HID layer
> to send a new output report. I suspect they had very good reasons for
> it (e.g. for headphone also dealing with different impedances).
>
> So I'm not sure how the volume control is really supposed to work, but
> I would think to do it properly it requires some interaction between
> the audio and HID drivers. Just letting the audio side do it right
> now, is more about leveraging the range of the DAC I guess versus a
> proper audio amplification stage.
>
> Just thinking of things from the user perspective, they should have a
> unified volume control. I don't know how other devices are doing it,
> but I think we need to think a bit further and we need to reconsider
> how things work....
>
> Thanks,
> Roderick
>
> On Tue, Jul 22, 2025 at 1:03 AM Cristian Ciocaltea
> <cristian.ciocaltea@collabora.com> wrote:
> >
> > Hi Roderick,
> >
> > On 7/22/25 9:18 AM, Roderick Colenbrander wrote:
> > > Hi Cristian and Jiri,
> > >
> > > One thing I forgot to bring up is whether it is best to have the audio
> > > plug logic have its separate input device or have it be part of an
> > > existing (e.g. main gamepad). The patch currently creates a separate
> > > input device. Originally we added multiple input devices (gamepad,
> > > touchpad and sensors) due to axes and button collisions really.
> > >
> > > For this feature there is no collision. There are not many devices in
> > > the kernel, which support these audio EV_SW. I see for example the
> > > Switch 2 controller has a mini jack port as well. Some xbox
> > > controllers too (though audio not supported in the kernel from a quick
> > > glance or at least no HID or xpad driver features for them).
> > >
> > > I don't have a strong opinion yet. Initial feeling was perhaps have it
> > > on the 'main' input device. But on the other hand, I'm not sure what
> > > software is normally listening for these kinds of EV_SW events. What
> > > would be listening for this like a pipewire?
> >
> > For now this is going to be used by the usb-audio driver which contains a
> > quirk [1] creating the jack controls for headphone and headset mic,
> > respectively. This will further setup an input handler for each of them in
> > order to intercept the related hotplug events.
> >
> > But it can be also used directly from ALSA/pipewire, e.g. for monitoring,
> > hence it think it's best to keep it as an audio dedicated input device.
> >
> > Regards,
> > Cristian
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/usb/mixer_quirks.c#n540
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-23 4:04 ` Roderick Colenbrander
2025-07-23 4:20 ` Roderick Colenbrander
@ 2025-07-23 7:17 ` Cristian Ciocaltea
2025-08-27 6:36 ` Cristian Ciocaltea
1 sibling, 1 reply; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-07-23 7:17 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
Hi Roderick,
On 7/23/25 7:04 AM, Roderick Colenbrander wrote:
> Hi Cristian,
>
> Thanks for the information on the audio patches in the sound tree. We
> weren't familiar with that part.
I've actually mentioned that in the cover letter and changelog of this
revision. Couldn't do this previously because I submitted the HID series
before the USB audio one.
> I talked a bit with my team members as well. In general audio is
> getting some bigger attention (will see where that goes). I'm getting
> a bit worried that the HID and usb driver need much closer coupling,
> the current coupling not being enough.
I think we should keep things simple for now, at least until we land the
basic support, also considering the USB part has been already merged.
> I don't know the USB audio spec too well, but it is more on the
> digital interface and a DAC. I'm not sure on the exact circuitry on
> the DualSense, but there is a lot of logic in the console drivers for
> volume handling where adjustment of the volume talks to the HID layer
> to send a new output report. I suspect they had very good reasons for
> it (e.g. for headphone also dealing with different impedances).
>
> So I'm not sure how the volume control is really supposed to work, but
> I would think to do it properly it requires some interaction between
> the audio and HID drivers. Just letting the audio side do it right
> now, is more about leveraging the range of the DAC I guess versus a
> proper audio amplification stage.
Indeed, it's not possible to support hardware volume control from
ALSA/usb-audio without involving HID. This could be done, or at least it's
worth investigating further, but it's not mandatory and definitely beyond
the scope of the current work.
> Just thinking of things from the user perspective, they should have a
> unified volume control. I don't know how other devices are doing it,
> but I think we need to think a bit further and we need to reconsider
> how things work....
There's a bit more complexity in here than initially anticipated, but the
(software) volume control is not really a problem. It's worth noting I am
going to provide some UCM changes, part of the ALSA project:
https://github.com/alsa-project/alsa-ucm-conf
This is to ensure proper support for audio profile switching between
headphones/headset and internal speaker/microphone, which also addresses
a few volume control related issues. Those are mainly caused by the haptic
feedback functionality, which is controlled by a pair of dedicated channels
in a quadraphonic audio stream. One of the UCM's main jobs is to split the
4.0 PCM stream into 4 mono channels or a pair of stereo (FL+FR) channels,
depending on the active output device/profile.
The only blocker now is this HID series, which prevents us moving further.
Therefore, unless there is anything else remaining which requires urgent
attention, could you please provide an ack for Jiri to be able to pick this
up?
Thanks,
Cristian
> Thanks,
> Roderick
>
> On Tue, Jul 22, 2025 at 1:03 AM Cristian Ciocaltea
> <cristian.ciocaltea@collabora.com> wrote:
>>
>> Hi Roderick,
>>
>> On 7/22/25 9:18 AM, Roderick Colenbrander wrote:
>>> Hi Cristian and Jiri,
>>>
>>> One thing I forgot to bring up is whether it is best to have the audio
>>> plug logic have its separate input device or have it be part of an
>>> existing (e.g. main gamepad). The patch currently creates a separate
>>> input device. Originally we added multiple input devices (gamepad,
>>> touchpad and sensors) due to axes and button collisions really.
>>>
>>> For this feature there is no collision. There are not many devices in
>>> the kernel, which support these audio EV_SW. I see for example the
>>> Switch 2 controller has a mini jack port as well. Some xbox
>>> controllers too (though audio not supported in the kernel from a quick
>>> glance or at least no HID or xpad driver features for them).
>>>
>>> I don't have a strong opinion yet. Initial feeling was perhaps have it
>>> on the 'main' input device. But on the other hand, I'm not sure what
>>> software is normally listening for these kinds of EV_SW events. What
>>> would be listening for this like a pipewire?
>>
>> For now this is going to be used by the usb-audio driver which contains a
>> quirk [1] creating the jack controls for headphone and headset mic,
>> respectively. This will further setup an input handler for each of them in
>> order to intercept the related hotplug events.
>>
>> But it can be also used directly from ALSA/pipewire, e.g. for monitoring,
>> hence it think it's best to keep it as an audio dedicated input device.
>>
>> Regards,
>> Cristian
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/usb/mixer_quirks.c#n540
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-23 7:17 ` Cristian Ciocaltea
@ 2025-08-27 6:36 ` Cristian Ciocaltea
0 siblings, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-08-27 6:36 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
On 7/23/25 10:17 AM, Cristian Ciocaltea wrote:
> Hi Roderick,
>
> On 7/23/25 7:04 AM, Roderick Colenbrander wrote:
>> Hi Cristian,
>>
>> Thanks for the information on the audio patches in the sound tree. We
>> weren't familiar with that part.
>
> I've actually mentioned that in the cover letter and changelog of this
> revision. Couldn't do this previously because I submitted the HID series
> before the USB audio one.
>
>> I talked a bit with my team members as well. In general audio is
>> getting some bigger attention (will see where that goes). I'm getting
>> a bit worried that the HID and usb driver need much closer coupling,
>> the current coupling not being enough.
>
> I think we should keep things simple for now, at least until we land the
> basic support, also considering the USB part has been already merged.
>
>> I don't know the USB audio spec too well, but it is more on the
>> digital interface and a DAC. I'm not sure on the exact circuitry on
>> the DualSense, but there is a lot of logic in the console drivers for
>> volume handling where adjustment of the volume talks to the HID layer
>> to send a new output report. I suspect they had very good reasons for
>> it (e.g. for headphone also dealing with different impedances).
>>
>> So I'm not sure how the volume control is really supposed to work, but
>> I would think to do it properly it requires some interaction between
>> the audio and HID drivers. Just letting the audio side do it right
>> now, is more about leveraging the range of the DAC I guess versus a
>> proper audio amplification stage.
>
> Indeed, it's not possible to support hardware volume control from
> ALSA/usb-audio without involving HID. This could be done, or at least it's
> worth investigating further, but it's not mandatory and definitely beyond
> the scope of the current work.
>
>> Just thinking of things from the user perspective, they should have a
>> unified volume control. I don't know how other devices are doing it,
>> but I think we need to think a bit further and we need to reconsider
>> how things work....
>
> There's a bit more complexity in here than initially anticipated, but the
> (software) volume control is not really a problem. It's worth noting I am
> going to provide some UCM changes, part of the ALSA project:
>
> https://github.com/alsa-project/alsa-ucm-conf
>
> This is to ensure proper support for audio profile switching between
> headphones/headset and internal speaker/microphone, which also addresses
> a few volume control related issues. Those are mainly caused by the haptic
> feedback functionality, which is controlled by a pair of dedicated channels
> in a quadraphonic audio stream. One of the UCM's main jobs is to split the
> 4.0 PCM stream into 4 mono channels or a pair of stereo (FL+FR) channels,
> depending on the active output device/profile.
>
> The only blocker now is this HID series, which prevents us moving further.
>
> Therefore, unless there is anything else remaining which requires urgent
> attention, could you please provide an ack for Jiri to be able to pick this
> up?
It's been over a month now since this was kind of blocked without any clear
reason, and by the end of next week I'll be on leave, which means we're
close to missing the merge window once again.
Considering the counterpart quirk in the generic USB audio driver has been
already merged since v6.17, I kindly ask for your support in getting this
into v6.18.
Thanks,
Cristian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
2025-07-22 5:47 ` Roderick Colenbrander
2025-07-22 6:18 ` Roderick Colenbrander
@ 2025-07-22 7:24 ` Cristian Ciocaltea
1 sibling, 0 replies; 23+ messages in thread
From: Cristian Ciocaltea @ 2025-07-22 7:24 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
Hi Roderick,
On 7/22/25 8:47 AM, Roderick Colenbrander wrote:
> Hi Christian,
>
> I just got back from Japan (trip was a bit extended). In the meantime
> I had some of employees had a look as well.
>
> The audio patches towards the end seem to be okay. We tried to dig for
> the official volume numbers, but they were too hard to find (too many
> layers, too many repositories). When we use a PS5, the default volume
> for the headset and speaker are both close to 70% (just eyeballing).
> At the hardware level the volume is quite non-linear and internally we
> use a mapping table (not sure what the curve is based on). For the
> speaker this starts at 0x3d as you found out already. The 70% volume
> for the speaker seems to correspond to a value of 93 and headphones
> 83.
> The set pre-amp gain of 0x2 is a common value we seem to set and means
> +6dB, so change comment around to mean that I guess.
Thanks for the additional clarifications. I added a fixup below, if Jiri is
fine applying that before merging, just to avoid respinning the whole
series:
Subject: [PATCH] fixup! HID: playstation: Support DualSense audio jack
hotplug detection
---
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 4285260c7e22..641c6337ff63 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1352,7 +1352,7 @@ static void dualsense_output_worker(struct work_struct *work)
*/
common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_SPEAKER_VOLUME_ENABLE;
common->speaker_volume = 0x64;
- /* Set SP preamp gain to ~30% */
+ /* Set SP preamp gain to +6dB */
common->valid_flag1 = DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE;
common->audio_control2 =
FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN, 0x2);
> As for the other patches I'm not entirely sure yet. I know they were
> well intended, but let me just say, they rubbed some of my team
> members quite the wrong way resulting in some heavy discussion. I have
> somewhat similar feelings about the ultra strict checkpatch toggle as
> well.
>
> We had to move mountains to be allowed to even upstream controller
> code among our limited time (it is closer to a hobby thing, even
> though many products nowadays use it as well). So that's a factor
> which adds up a bit as well.
>
> I think some of the patches we could live with if it came to it. There
> is no real agreed up full kernel standard (as it is contentious). So
> for example we tend to prefer more uint8_t family, where older kernel
> style was more u8 and the kernel allows for both. I think we would
> probably lean towards keeping it at the modern form.
>
> Some of the macros also felt a little too magical. Our feeling tends
> to be if you have to go many layers deep to understand what a macro or
> line of code does (and it is easier to then printk the value),
> something feels off...
I'm sorry for all the troubles introduced with the additional patches! My
intention was not to highlight deficiencies with the current implementation,
but to bring the driver as close as possible to the coding standard agreed
by the kernel community, to avoid dealing with the kind of problems that I
tried to explain a while ago.
Thanks again for your support,
Cristian
^ permalink raw reply related [flat|nested] 23+ messages in thread