[PATCH v2 0/5] HID: playstation: DS4: LED bugfix, third-party gamepad support

[PATCH v2 0/5] HID: playstation: DS4: LED bugfix, third-party gamepad support

[Max Staudt]

Dear hid-playstation maintainers,

Here is v2 of my patch series, with the discussed changes.


Differences since v1:
 - Dropped patch for 7545:0104 (SZ-MYPOWER controllers)
 - Dropped patch for DS4 clones without a MAC address on USB
 - Changed hid_err() to hid_warn() where things are no longer fatal
 - Simplified goto as return in minimal report parsing


I've included the patch to simplify the PID/VID mapping to controller
types, since the previous discussion made it sound useful for future
support of second-party controllers. Please feel free to drop it if you
don't think it's relevant now.


Thanks for your feedback!

Max


Patches in this series:
  [PATCH v2 1/5] HID: playstation: DS4: Fix LED blinking
  [PATCH v2 2/5] HID: playstation: DS4: Don't fail on FW/HW version
  [PATCH v2 3/5] HID: playstation: DS4: Don't fail on calibration data
  [PATCH v2 4/5] HID: playstation: DS4: Parse minimal report 0x01
  [PATCH v2 5/5] HID: playstation: Simplify device type ID

[PATCH v2 1/5] HID: playstation: DS4: Fix LED blinking

[Max Staudt]

There was no way to disable blinking once enabled.
Disable it on brightness = 0, as per the Linux LED spec.

The driver reports back the values it sends to the controller, but they
need to be scaled back to milliseconds. Setting the LED blinking via
sysfs works as expected now.

Signed-off-by: Max Staudt <max@enpas.org>
---
 drivers/hid/hid-playstation.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 8ac8f7b8e317..7f50e13601f0 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -2037,8 +2037,9 @@ static int dualshock4_led_set_blink(struct led_classdev *led, unsigned long *del
 
 	dualshock4_schedule_work(ds4);
 
-	*delay_on = ds4->lightbar_blink_on;
-	*delay_off = ds4->lightbar_blink_off;
+	/* Report scaled values back to LED subsystem */
+	*delay_on = ds4->lightbar_blink_on * 10;
+	*delay_off = ds4->lightbar_blink_off * 10;
 
 	return 0;
 }
@@ -2065,6 +2066,13 @@ static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brig
 		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;
-- 
2.39.2

[PATCH v2 2/5] HID: playstation: DS4: Don't fail on FW/HW version request

[Max Staudt]

Some third-party controllers can't report firmware/hardware version.

Unlike for the DualSense, the driver does not use these values for
anything in the DualShock 4 case, but merely exposes them via sysfs.
They will simply be 0x0.

Signed-off-by: Max Staudt <max@enpas.org>
---
 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 7f50e13601f0..df50ca4dab90 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -2558,8 +2558,8 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 
 	ret = dualshock4_get_firmware_info(ds4);
 	if (ret) {
-		hid_err(hdev, "Failed to get firmware info from DualShock4\n");
-		return ERR_PTR(ret);
+		hid_warn(hdev, "Failed to get firmware info from DualShock4\n");
+		hid_warn(hdev, "HW/FW version data in sysfs will be invalid.\n");
 	}
 
 	ret = ps_devices_list_add(ps_dev);
-- 
2.39.2

[PATCH v2 3/5] HID: playstation: DS4: Don't fail on calibration data request

[Max Staudt]

Some third-party controllers can't report calibration data for the
gyro/accelerometer.

We can still use the gamepad as-is, so let's do that.

Signed-off-by: Max Staudt <max@enpas.org>
---
 drivers/hid/hid-playstation.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index df50ca4dab90..53bfc2828a61 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1778,8 +1778,10 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 		int retries;
 
 		buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
-		if (!buf)
-			return -ENOMEM;
+		if (!buf) {
+			ret = -ENOMEM;
+			goto no_buffer_tail_check;
+		}
 
 		/* We should normally receive the feature report data we asked
 		 * for, but hidraw applications such as Steam can issue feature
@@ -1796,26 +1798,27 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 					continue;
 				}
 
-				hid_err(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 err_free;
 			} else {
 				break;
 			}
 		}
 	} else { /* Bluetooth */
 		buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, GFP_KERNEL);
-		if (!buf)
-			return -ENOMEM;
+		if (!buf) {
+			ret = -ENOMEM;
+			goto no_buffer_tail_check;
+		}
 
 		ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION_BT, buf,
 				DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, true);
-		if (ret) {
-			hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
-			goto err_free;
-		}
+
+		if (ret)
+			hid_warn(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
 	}
 
+	/* Parse buffer. If the transfer failed, this safely copies zeroes. */
 	gyro_pitch_bias  = get_unaligned_le16(&buf[1]);
 	gyro_yaw_bias    = get_unaligned_le16(&buf[3]);
 	gyro_roll_bias   = get_unaligned_le16(&buf[5]);
@@ -1867,6 +1870,11 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	ds4->gyro_calib_data[2].sens_denom = abs(gyro_roll_plus - gyro_roll_bias) +
 			abs(gyro_roll_minus - gyro_roll_bias);
 
+	/* Done parsing the buffer, so let's free it. */
+	kfree(buf);
+
+no_buffer_tail_check:
+
 	/*
 	 * Sanity check gyro calibration data. This is needed to prevent crashes
 	 * during report handling of virtual, clone or broken devices not implementing
@@ -1919,8 +1927,6 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 		}
 	}
 
-err_free:
-	kfree(buf);
 	return ret;
 }
 
@@ -2568,8 +2574,8 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 
 	ret = dualshock4_get_calibration_data(ds4);
 	if (ret) {
-		hid_err(hdev, "Failed to get calibration data from DualShock4\n");
-		goto err;
+		hid_warn(hdev, "Failed to get calibration data from DualShock4\n");
+		hid_warn(hdev, "Gyroscope and accelerometer will be inaccurate.\n");
 	}
 
 	ds4->gamepad = ps_gamepad_create(hdev, dualshock4_play_effect);
-- 
2.39.2

[PATCH v2 4/5] HID: playstation: DS4: Parse minimal report 0x01

[Max Staudt]

Some third-party controllers never switch to the full 0x11 report.

They keep sending the short 0x01 report, so let's parse that instead.

Signed-off-by: Max Staudt <max@enpas.org>
---
 drivers/hid/hid-playstation.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 53bfc2828a61..6b0f25688657 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -287,6 +287,8 @@ struct dualsense_output_report {
 
 #define DS4_INPUT_REPORT_USB			0x01
 #define DS4_INPUT_REPORT_USB_SIZE		64
+#define DS4_INPUT_REPORT_BT_MINIMAL		0x01
+#define DS4_INPUT_REPORT_BT_MINIMAL_SIZE	10
 #define DS4_INPUT_REPORT_BT			0x11
 #define DS4_INPUT_REPORT_BT_SIZE		78
 #define DS4_OUTPUT_REPORT_USB			0x05
@@ -2196,6 +2198,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	int battery_status, i, j;
 	uint16_t sensor_timestamp;
 	unsigned long flags;
+	bool is_minimal = false;
 
 	/*
 	 * DualShock4 in USB uses the full HID report for reportID 1, but
@@ -2223,6 +2226,18 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 		ds4_report = &bt->common;
 		num_touch_reports = bt->num_touch_reports;
 		touch_reports = bt->touch_reports;
+	} else if (hdev->bus == BUS_BLUETOOTH &&
+		   report->id == DS4_INPUT_REPORT_BT_MINIMAL &&
+			 size == DS4_INPUT_REPORT_BT_MINIMAL_SIZE) {
+		/* Some third-party pads never switch to the full 0x11 report.
+		 * The short 0x01 report is 10 bytes long:
+		 *   u8 report_id == 0x01
+		 *   u8 first_bytes_of_full_report[9]
+		 * So let's reuse the full report parser, and stop it after
+		 * parsing the buttons.
+		 */
+		ds4_report = (struct dualshock4_input_report_common *)&data[1];
+		is_minimal = true;
 	} else {
 		hid_err(hdev, "Unhandled reportID=%d\n", report->id);
 		return -1;
@@ -2256,6 +2271,9 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	input_report_key(ds4->gamepad, BTN_MODE,   ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME);
 	input_sync(ds4->gamepad);
 
+	if (is_minimal)
+		return 0;
+
 	/* Parse and calibrate gyroscope data. */
 	for (i = 0; i < ARRAY_SIZE(ds4_report->gyro); i++) {
 		int raw_data = (short)le16_to_cpu(ds4_report->gyro[i]);
-- 
2.39.2

[PATCH v2 5/5] HID: playstation: Simplify device type ID

[Max Staudt]

Distinguish PS4/PS5 type controllers using .driver_data in
MODULE_DEVICE_TABLE rather than by VID/PID.

This allows adding compatible controllers with different VID/PID.

Signed-off-by: Max Staudt <max@enpas.org>
---
 drivers/hid/hid-playstation.c | 40 +++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 6b0f25688657..edc46fc02e9a 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -27,6 +27,11 @@ static DEFINE_IDA(ps_player_id_allocator);
 
 #define HID_PLAYSTATION_VERSION_PATCH 0x8000
 
+enum PS_TYPE {
+	PS_TYPE_PS4_DUALSHOCK4,
+	PS_TYPE_PS5_DUALSENSE,
+};
+
 /* Base class for playstation devices. */
 struct ps_device {
 	struct list_head list;
@@ -2687,17 +2692,14 @@ static int ps_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_stop;
 	}
 
-	if (hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER ||
-		hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_2 ||
-		hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) {
+	if (id->driver_data == PS_TYPE_PS4_DUALSHOCK4) {
 		dev = dualshock4_create(hdev);
 		if (IS_ERR(dev)) {
 			hid_err(hdev, "Failed to create dualshock4.\n");
 			ret = PTR_ERR(dev);
 			goto err_close;
 		}
-	} else if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER ||
-		hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) {
+	} else if (id->driver_data == PS_TYPE_PS5_DUALSENSE) {
 		dev = dualsense_create(hdev);
 		if (IS_ERR(dev)) {
 			hid_err(hdev, "Failed to create dualsense.\n");
@@ -2731,16 +2733,26 @@ static void ps_remove(struct hid_device *hdev)
 
 static const struct hid_device_id ps_devices[] = {
 	/* Sony DualShock 4 controllers for PS4 */
-	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) },
-	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER),
+		.driver_data = PS_TYPE_PS4_DUALSHOCK4 },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER),
+		.driver_data = PS_TYPE_PS4_DUALSHOCK4 },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2),
+		.driver_data = PS_TYPE_PS4_DUALSHOCK4 },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2),
+		.driver_data = PS_TYPE_PS4_DUALSHOCK4 },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE),
+		.driver_data = PS_TYPE_PS4_DUALSHOCK4 },
+
 	/* Sony DualSense controllers for PS5 */
-	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },
-	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER),
+		.driver_data = PS_TYPE_PS5_DUALSENSE },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER),
+		.driver_data = PS_TYPE_PS5_DUALSENSE },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER_2),
+		.driver_data = PS_TYPE_PS5_DUALSENSE },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER_2),
+		.driver_data = PS_TYPE_PS5_DUALSENSE },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, ps_devices);
-- 
2.39.2

Re: [PATCH v2 0/5] HID: playstation: DS4: LED bugfix, third-party gamepad support

[Jiri Kosina]

On Thu, 8 Feb 2024, Max Staudt wrote:

> Dear hid-playstation maintainers,
> 
> Here is v2 of my patch series, with the discussed changes.
> 
> 
> Differences since v1:
>  - Dropped patch for 7545:0104 (SZ-MYPOWER controllers)
>  - Dropped patch for DS4 clones without a MAC address on USB
>  - Changed hid_err() to hid_warn() where things are no longer fatal
>  - Simplified goto as return in minimal report parsing
> 
> 
> I've included the patch to simplify the PID/VID mapping to controller
> types, since the previous discussion made it sound useful for future
> support of second-party controllers. Please feel free to drop it if you
> don't think it's relevant now.
> 
> 
> Thanks for your feedback!
> 
> Max
> 
> 
> Patches in this series:
>   [PATCH v2 1/5] HID: playstation: DS4: Fix LED blinking
>   [PATCH v2 2/5] HID: playstation: DS4: Don't fail on FW/HW version
>   [PATCH v2 3/5] HID: playstation: DS4: Don't fail on calibration data
>   [PATCH v2 4/5] HID: playstation: DS4: Parse minimal report 0x01
>   [PATCH v2 5/5] HID: playstation: Simplify device type ID

Roderick, any word on this series please?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2 0/5] HID: playstation: DS4: LED bugfix, third-party gamepad support

[Jiri Kosina]

On Tue, 27 Feb 2024, Jiri Kosina wrote:

> > Dear hid-playstation maintainers,
> > 
> > Here is v2 of my patch series, with the discussed changes.
> > 
> > 
> > Differences since v1:
> >  - Dropped patch for 7545:0104 (SZ-MYPOWER controllers)
> >  - Dropped patch for DS4 clones without a MAC address on USB
> >  - Changed hid_err() to hid_warn() where things are no longer fatal
> >  - Simplified goto as return in minimal report parsing
> > 
> > 
> > I've included the patch to simplify the PID/VID mapping to controller
> > types, since the previous discussion made it sound useful for future
> > support of second-party controllers. Please feel free to drop it if you
> > don't think it's relevant now.
> > 
> > 
> > Thanks for your feedback!
> > 
> > Max
> > 
> > 
> > Patches in this series:
> >   [PATCH v2 1/5] HID: playstation: DS4: Fix LED blinking
> >   [PATCH v2 2/5] HID: playstation: DS4: Don't fail on FW/HW version
> >   [PATCH v2 3/5] HID: playstation: DS4: Don't fail on calibration data
> >   [PATCH v2 4/5] HID: playstation: DS4: Parse minimal report 0x01
> >   [PATCH v2 5/5] HID: playstation: Simplify device type ID
> 
> Roderick, any word on this series please?

Roderick, please speak up now, or I'll queue this as-is for 6.10 in the 
coming few days. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2 0/5] HID: playstation: DS4: LED bugfix, third-party gamepad support

[Roderick Colenbrander]

On Wed, Apr 3, 2024 at 11:11 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Tue, 27 Feb 2024, Jiri Kosina wrote:
>
> > > Dear hid-playstation maintainers,
> > >
> > > Here is v2 of my patch series, with the discussed changes.
> > >
> > >
> > > Differences since v1:
> > >  - Dropped patch for 7545:0104 (SZ-MYPOWER controllers)
> > >  - Dropped patch for DS4 clones without a MAC address on USB
> > >  - Changed hid_err() to hid_warn() where things are no longer fatal
> > >  - Simplified goto as return in minimal report parsing
> > >
> > >
> > > I've included the patch to simplify the PID/VID mapping to controller
> > > types, since the previous discussion made it sound useful for future
> > > support of second-party controllers. Please feel free to drop it if you
> > > don't think it's relevant now.
> > >
> > >
> > > Thanks for your feedback!
> > >
> > > Max
> > >
> > >
> > > Patches in this series:
> > >   [PATCH v2 1/5] HID: playstation: DS4: Fix LED blinking
> > >   [PATCH v2 2/5] HID: playstation: DS4: Don't fail on FW/HW version
> > >   [PATCH v2 3/5] HID: playstation: DS4: Don't fail on calibration data
> > >   [PATCH v2 4/5] HID: playstation: DS4: Parse minimal report 0x01
> > >   [PATCH v2 5/5] HID: playstation: Simplify device type ID
> >
> > Roderick, any word on this series please?
>
> Roderick, please speak up now, or I'll queue this as-is for 6.10 in the
> coming few days. Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

Hi Jiri,

Sorry for late reply, let's merge it. There are maybe 1-2 nitpicky
things I forgot to email about, but I can deal with those later if I
feel it is needed down the road.

Thanks,
Roderick

Re: [PATCH v2 0/5] HID: playstation: DS4: LED bugfix, third-party gamepad support

[Jiri Kosina]

On Wed, 3 Apr 2024, Roderick Colenbrander wrote:

> > Roderick, please speak up now, or I'll queue this as-is for 6.10 in the
> > coming few days. Thanks,
> 
> Hi Jiri,
> 
> Sorry for late reply, let's merge it. There are maybe 1-2 nitpicky
> things I forgot to email about, but I can deal with those later if I
> feel it is needed down the road.

Welcome back! :-)

Now queued in hid.git#for-6.10/playstation. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2 0/5] HID: playstation: DS4: LED bugfix, third-party gamepad support

[Max Staudt]

On 4/3/24 21:54, Jiri Kosina wrote:
> On Wed, 3 Apr 2024, Roderick Colenbrander wrote:
> 
>>> Roderick, please speak up now, or I'll queue this as-is for 6.10 in the
>>> coming few days. Thanks,
>>
>> Hi Jiri,
>>
>> Sorry for late reply, let's merge it. There are maybe 1-2 nitpicky
>> things I forgot to email about, but I can deal with those later if I
>> feel it is needed down the road.
> 
> Welcome back! :-)
> 
> Now queued in hid.git#for-6.10/playstation. Thanks,


Thank you both!


Max