Re: [PATCH] HID: sony: Fix division by zero

[Roderick Colenbrander <thunderbird2k@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]

Hi Alain,

Give this patch a try. It is against hid-playstation, which as of 6.2
supports DS4. Let me know if it works. You can see the sensors working
through evdev on the motion sensors device.

In regards to your DS4. I am not sure if yours is an official one or
not. There are many official DS4 revisions (probably 10+ different
PCBs) and the clones go to great length. Considering the sensors do
seem to work, it does feel odd that the data is zero. Either something
cleared the calibration data (would be strange) or it might be a
clone. If you want to dig deeper, you can play around with
dualshock4_get_calibration_data in hid-playstation. The particular
report (though not fully documented in the driver) does contain a lot
of device info (firmware info, manufacturing dates, various strings).
You can probably find the details online. Some data there might be
weird or not populated.

Thanks,
Roderick

On Wed, Dec 28, 2022 at 1:58 PM Alain Carlucci <alain.carlucci@gmail.com> wrote:
>
> On Wed, Dec 28, 2022 at 11:38:03AM -0800, Roderick Colenbrander wrote:
> >Hi Alain,
> >
> >Thanks for the info. Out of curiosity are the reported sensor values in the
> >hid report non zero?
>
> Just wrote the print code and unexpectedly.. yes!
>
> Here's a report of the raw_data variable in dualshock4_parse_report()
> using printk:
>
> [10793.137066] rd0=4 rd1=1  rd2=17 rd3=707 rd4=-8063 rd5=-1536
> [10793.141121] rd0=4 rd1=0  rd2=17 rd3=698 rd4=-8061 rd5=-1546
> [10793.145101] rd0=4 rd1=-1 rd2=19 rd3=699 rd4=-8062 rd5=-1544
> [10793.149059] rd0=3 rd1=0  rd2=19 rd3=695 rd4=-8052 rd5=-1554
> [10793.153085] rd0=4 rd1=-1 rd2=19 rd3=691 rd4=-8052 rd5=-1547
> [10793.157059] rd0=4 rd1=0  rd2=18 rd3=709 rd4=-8057 rd5=-1551
>
> >As for testing there are no good apps. Evtest can at least do gamepad and
> >touchpad. Unfortunately we didn't make a proper desktop test app. Hopefully
> >in the future.
> Ok got it, thank you.
>
> Thanks,
> Alain

[-- Attachment #2: 0002-HID-playstation-sanity-check-DS4-calibration-data.patch --]
[-- Type: text/x-patch, Size: 3064 bytes --]

From 6e2d8f9fd243bcb8cc3de19212f64fcd8701fca0 Mon Sep 17 00:00:00 2001
From: Roderick Colenbrander <roderick.colenbrander@sony.com>
Date: Thu, 29 Dec 2022 08:19:28 -0800
Subject: [PATCH 2/2] HID: playstation: sanity check DS4 calibration data.

Some DualShock4 devices report invalid calibration data resulting
in kernel oopses due to division by zero during report handling.

The devices affected appear to be clone devices, which don't
implement all reports properly and don't populate proper calibration
data.

This patch prevents the crashes by essentially disabling calibration
when invalid values are detected.
---
 drivers/hid/hid-playstation.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 866cc4e94320..bdc32c818dc7 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1737,6 +1737,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	int speed_2x;
 	int range_2g;
 	int ret = 0;
+	int i;
 	uint8_t *buf;
 
 	if (ds4->base.hdev->bus == BUS_USB) {
@@ -1829,6 +1830,23 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	ds4->gyro_calib_data[2].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
 	ds4->gyro_calib_data[2].sens_denom = gyro_roll_plus - gyro_roll_minus;
 
+	/* Sanity check gyro calibration data. This is needed for clone devices,
+	 * which poorly implement support. Some devices appear to return zeros
+	 * for all data, while some hardcode all the same values and for some
+	 * use the wrong sign (the delta is then 0).
+	 * When we detect something fishy, just disable the calibration logic
+	 * altogether as nothing can be trusted for that axis.
+	 */
+	for (i = 0; ARRAY_SIZE(ds4->gyro_calib_data); i++) {
+		if (ds4->gyro_calib_data[i].sens_denom == 0) {
+			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_RES_PER_DEG_S;
+			ds4->gyro_calib_data[i].sens_denom = 1;
+		}
+	}
+
 	/*
 	 * Set accelerometer calibration and normalization parameters.
 	 * Data values will be normalized to 1/DS4_ACC_RES_PER_G g.
@@ -1851,6 +1869,19 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	ds4->accel_calib_data[2].sens_numer = 2*DS4_ACC_RES_PER_G;
 	ds4->accel_calib_data[2].sens_denom = range_2g;
 
+	/* Sanity check accelerometer calibration data. This is needed for clone devices,
+	 * which poorly implement support.
+	 */
+	for (i = 0; ARRAY_SIZE(ds4->accel_calib_data); i++) {
+		if (ds4->accel_calib_data[i].sens_denom == 0) {
+			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_RES_PER_G;
+			ds4->accel_calib_data[i].sens_denom = 1;
+		}
+	}
+
 err_free:
 	kfree(buf);
 	return ret;
-- 
2.38.1