| Message ID | 20221226000722.2xgbvsnrl6k7f7tk@ananas (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: sony: Fix division by zero | expand |
Hi Alain, Thanks for sharing your patch. Others have encountered similar issues. This is the case when the calibration coefficients are incorrect. These are hard programmed into devices from the factory. It are typically clone devices, which don't implement all DS4 functionality properly. Can you try printing all the variables (gyro_speed_plus,.. acc_z_minus) for your device as decimal numbers from the get_calibration_data function? I have been considering to add a little bit of clone support code if possible to the new hid-playstation as long as it doesn't pollute the code too much or causes issues. Thanks, Roderick On Sun, Dec 25, 2022 at 4:08 PM Alain Carlucci <alain.carlucci@gmail.com> wrote: > > Hello, > > Today I connected a partially broken DS4 via USB and got a kernel > panic with a division by zero in the hid-sony driver. > > The issue is caused by sens_denom=0 in the sensor calibration data, > which triggers a division by zero when dualshock4_parse_report() is > invoked, the division happens in the mult_frac() macro. > > This patch applies a workaround that allows the DS4 to be used even > with a broken sensor: if the denominator sent by the device is zero, > it is replaced with 1 and a warning is emitted. > > Signed-off-by: Alain Carlucci <alain.carlucci@gmail.com> > --- > drivers/hid/hid-sony.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c > index 13125997a..f8b05cb29 100644 > --- a/drivers/hid/hid-sony.c > +++ b/drivers/hid/hid-sony.c > @@ -1714,6 +1714,7 @@ static int dualshock4_get_calibration_data(struct sony_sc *sc) > short acc_z_plus, acc_z_minus; > int speed_2x; > int range_2g; > + int calib_id; > > /* For Bluetooth we use a different request, which supports CRC. > * Note: in Bluetooth mode feature report 0x02 also changes the state > @@ -1858,6 +1859,23 @@ static int dualshock4_get_calibration_data(struct sony_sc *sc) > sc->ds4_calib_data[5].sens_numer = 2*DS4_ACC_RES_PER_G; > sc->ds4_calib_data[5].sens_denom = range_2g; > > + for (calib_id = 0; calib_id < 6; calib_id++) { > + /* Ensure there are no denominators equal to zero to prevent > + * crashes while dividing by that number. > + */ > + > + if (sc->ds4_calib_data[calib_id].sens_denom != 0) { > + /* Denominator OK, skip this */ > + continue; > + } > + > + sc->ds4_calib_data[calib_id].sens_denom = 1; > + > + hid_warn(sc->hdev, > + "DualShock 4 USB dongle: invalid calibration for sensor %d\n", > + calib_id); > + } > + > err_stop: > kfree(buf); > return ret; > -- > 2.39.0
Hi Roderick, On Tue, Dec 27, 2022 at 08:17:15AM -0800, Roderick Colenbrander wrote: >Thanks for sharing your patch. Others have encountered similar issues. >This is the case when the calibration coefficients are incorrect. >These are hard programmed into devices from the factory. It are >typically clone devices, which don't implement all DS4 functionality >properly. I cannot ensure it's an original DS4 but it really seems so. The board is a JDM-055 with a MT3610N, but instead of a BST-BMI270 (as reported on a reverse-engineered schematic[1]) it has a "N339(CCF)". I can't find any datasheet or specification of that chip. I'll provide few images of the board for reference[2]. Do you think it's a clone? >Can you try printing all the variables (gyro_speed_plus,.. >acc_z_minus) for your device as decimal numbers from the >get_calibration_data function? Sure, here is the output: gyro_pitch_plus=0 gyro_pitch_minus=0 gyro_yaw_plus=0 gyro_yaw_minus=0 gyro_roll_plus=0 gyro_roll_minus=0 gyro_speed_plus=0 gyro_speed_minus=0 acc_x_plus=0 acc_x_minus=0 acc_y_plus=0 acc_y_minus=0 acc_z_plus=0 acc_z_minus=0 Probably it has communication issues with the IMU. This one is a joypad with multiple issues I'm trying to solve one by one. By the way, do you know if there are tools for linux to test all the functionalities of the DS4? I would be interested to read IMU values, test audio, vibration and touchpad. Thanks, Alain [1] https://www.acidmods.com/RDC/DS4/JDM-055/1-982-707-31%20small/JDM-055__(31)_SOME_VALUES.pdf [2] https://imgur.com/a/tckVWKR
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
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
Hi Roderick, >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. Thank you for the patch, just tried. I think there is a typo in the condition of the for-loop that sanitizes the input. Instead of `; i < ARRAY_SIZE()` it's written `; ARRAY_SIZE()`, which always evaluates to true. The loop then overflows the array and crashes the kernel. By the way, the DualSense code has similar calibration code and also there the input is not sanitized. I think it's quite easy to create a fake DualSense device with an Arduino that emulates the protocol up to calib_denom=0, just to exploit that and crash every linux machine is plugged in. Or even worst, exploit that via bluetooth. >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. Thank you! Just discovered that this strange DS4 (CUH-ZCT2E) replies all zeros both to HID request 0x02 (get calibration data) and as the BT address (request 0x12, Pairing Info), which explains why BT is not working. I tried to request version info (0xa3), the response seems the same as another fully-working and original CUH-ZCT2E: """ Build Date: Sep 21 2018 04:50:51 HW Version: 0100.b008 SW Version: 00000001.a00a SW Series: 2010 Code Size: 0002a000 """ Anyway, I have seen that request 0xA2 puts the DS4 in an alternate boot mode, probably DFU mode, where the device reboots as 054c:0856 and waits for data, which seems totally undocumented online. Do you know anything about this mode? It would be amazing to be able to reflash the original firmware, Thanks, Alain
Hi Alain, On Thu, Dec 29, 2022 at 11:21 AM Alain Carlucci <alain.carlucci@gmail.com> wrote: > > Hi Roderick, > > >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. > > Thank you for the patch, just tried. I think there is a typo in the > condition of the for-loop that sanitizes the input. > Instead of `; i < ARRAY_SIZE()` it's written `; ARRAY_SIZE()`, > which always evaluates to true. The loop then overflows the array and > crashes the kernel. Ooh oops. It was a quick patch I wrote without testing. > By the way, the DualSense code has similar calibration code and also > there the input is not sanitized. > I think it's quite easy to create a fake DualSense device with > an Arduino that emulates the protocol up to calib_denom=0, just to > exploit that and crash every linux machine is plugged in. Or even > worst, exploit that via bluetooth. You are right it probably won't hurt to duplicate the logic there. > >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. > > Thank you! Just discovered that this strange DS4 (CUH-ZCT2E) replies > all zeros both to HID request 0x02 (get calibration data) and as the > BT address (request 0x12, Pairing Info), which explains why BT is not > working. There is definitely something weird with your device. Something must have gotten wiped somehow. > I tried to request version info (0xa3), the response seems the same as > another fully-working and original CUH-ZCT2E: > """ > Build Date: Sep 21 2018 04:50:51 > HW Version: 0100.b008 > SW Version: 00000001.a00a > SW Series: 2010 > Code Size: 0002a000 > """ > > Anyway, I have seen that request 0xA2 puts the DS4 in an alternate > boot mode, probably DFU mode, where the device reboots as 054c:0856 > and waits for data, which seems totally undocumented online. > Do you know anything about this mode? > It would be amazing to be able to reflash the original firmware, Unfortunately I can't disclose any of that information. I can say that on DS4 it wasn't common to update firmware (as opposed to DualSense) while out in the wild. Some of these requests and others are probably used to initiate firmware programming and programming of MAC address, calibration data and other settings. It is probably quite involved and hard to get right without bricking your device. > Thanks, > Alain Thanks, Roderick
Hi Roderick, Hi Daniel, Thank you for all the suggestions, Roderick. I fixed the typo in your patch, backported to hid-sony and tested both of them. They fix the issue and the DS4 can be used even without calibration data. I cannot test if everything works well with a DualSense because I do not own one. Hi Daniel, I've seen that in the hid-nintendo driver, the calibration denominator is not sanitized if it's zero. If I'm not mistaken, a device that disguises itself as a joycon with denominator zero can crash the kernel when this is used in the mult_frac(). I had this behaviour with the hid-sony driver. You can find attached the patch that should solve the problem. Thanks, Alain
Hi Alain, Thanks for testing. I added a similar patch to my hid-playstation tree this morning (hid-sony will go away soon). I'm not entirely happy with the patches yet and need to do some thinking. The issue is the value range, which is not correct. For the accelerometer the numerator and denominator need to be 1 to match the proper range. (It just happens that the scaling factors are the way they are I think.) The gyroscope is the issue and I'm not entirely sure what the numerator needs to be. If I print the coefficients using one of my dualshocks, the ratio of numerator to denominator is around 60. And then I'm hunted by some comment from someone in the community.. https://dsremap.readthedocs.io/en/latest/reverse.html#use-calibration "Note: There’s a 2 factor in the Linux driver, i.e. max_speed and min_speed are added instead of averaged. Either there’s something I don’t get, or the factor is taken care of in the resolution constant, or it’s a bug. " I need to do some thinking on whether the current code is right (even if it isn't, it can't be changed as it would break software). Then what the factor needs to be. Thanks, Roderick On Fri, Dec 30, 2022 at 10:12 AM Alain <alain.carlucci@gmail.com> wrote: > > Hi Roderick, > Hi Daniel, > > Thank you for all the suggestions, Roderick. > I fixed the typo in your patch, backported to hid-sony and tested both of them. > They fix the issue and the DS4 can be used even without calibration data. > I cannot test if everything works well with a DualSense because I do not own one. > > Hi Daniel, I've seen that in the hid-nintendo driver, the calibration denominator is not sanitized if it's zero. > If I'm not mistaken, a device that disguises itself as a joycon with denominator zero can crash the kernel > when this is used in the mult_frac(). I had this behaviour with the hid-sony driver. > You can find attached the patch that should solve the problem. > > Thanks, > Alain > > > Il giorno gio 29 dic 2022 alle ore 21:56 Roderick Colenbrander <thunderbird2k@gmail.com> ha scritto: >> >> Hi Alain, >> >> On Thu, Dec 29, 2022 at 11:21 AM Alain Carlucci >> <alain.carlucci@gmail.com> wrote: >> > >> > Hi Roderick, >> > >> > >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. >> > >> > Thank you for the patch, just tried. I think there is a typo in the >> > condition of the for-loop that sanitizes the input. >> > Instead of `; i < ARRAY_SIZE()` it's written `; ARRAY_SIZE()`, >> > which always evaluates to true. The loop then overflows the array and >> > crashes the kernel. >> >> Ooh oops. It was a quick patch I wrote without testing. >> >> > By the way, the DualSense code has similar calibration code and also >> > there the input is not sanitized. >> > I think it's quite easy to create a fake DualSense device with >> > an Arduino that emulates the protocol up to calib_denom=0, just to >> > exploit that and crash every linux machine is plugged in. Or even >> > worst, exploit that via bluetooth. >> >> You are right it probably won't hurt to duplicate the logic there. >> >> > >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. >> > >> > Thank you! Just discovered that this strange DS4 (CUH-ZCT2E) replies >> > all zeros both to HID request 0x02 (get calibration data) and as the >> > BT address (request 0x12, Pairing Info), which explains why BT is not >> > working. >> >> There is definitely something weird with your device. Something must >> have gotten wiped somehow. >> >> > I tried to request version info (0xa3), the response seems the same as >> > another fully-working and original CUH-ZCT2E: >> > """ >> > Build Date: Sep 21 2018 04:50:51 >> > HW Version: 0100.b008 >> > SW Version: 00000001.a00a >> > SW Series: 2010 >> > Code Size: 0002a000 >> > """ >> > >> > Anyway, I have seen that request 0xA2 puts the DS4 in an alternate >> > boot mode, probably DFU mode, where the device reboots as 054c:0856 >> > and waits for data, which seems totally undocumented online. >> > Do you know anything about this mode? >> > It would be amazing to be able to reflash the original firmware, >> >> Unfortunately I can't disclose any of that information. I can say that >> on DS4 it wasn't common to update firmware (as opposed to DualSense) >> while out in the wild. Some of these requests and others are probably >> used to initiate firmware programming and programming of MAC address, >> calibration data and other settings. It is probably quite involved and >> hard to get right without bricking your device. >> >> > Thanks, >> > Alain >> >> Thanks, >> Roderick
Hi Alain, Just a brief update. I'm still looking at this and I'm not yet sure what to do. It looks like there is something wrong with the value ranges of the gyroscope. I need to dig deep in my memory how this code came around. Some parts of the sensors code were developed and tested by a team member, who left. For testing he used some internal test 3D application in which you center some object. There is some validation there on the values. However, there is just something strange. The uncalibrated gyroscope values are 16-bit signed, but the hardware limits are +/- 2000 degree per second. To have enough precision during calibration the value range is extended (resolution per axis is set to 1024, so 1024 corresponds to 1 degree per second). When I heavily shake a DS4 in evtest, I can easily reach values like 15 million or more. However, the maximum value we have set on the evdev node to about 2M. In other words there is something wrong. I need to look closely at other internal code. I really hate this as if true, the DS4 unit tests in hid-tools (and Android) are broken too. Thanks, Roderick On Fri, Dec 30, 2022 at 1:53 PM Roderick Colenbrander <thunderbird2k@gmail.com> wrote: > > Hi Alain, > > Thanks for testing. I added a similar patch to my hid-playstation tree > this morning (hid-sony will go away soon). > > I'm not entirely happy with the patches yet and need to do some > thinking. The issue is the value range, which is not correct. For the > accelerometer the numerator and denominator need to be 1 to match the > proper range. (It just happens that the scaling factors are the way > they are I think.) > > The gyroscope is the issue and I'm not entirely sure what the > numerator needs to be. If I print the coefficients using one of my > dualshocks, the ratio of numerator to denominator is around 60. And > then I'm hunted by some comment from someone in the community.. > https://dsremap.readthedocs.io/en/latest/reverse.html#use-calibration > > "Note: > There’s a 2 factor in the Linux driver, i.e. max_speed and min_speed > are added instead of averaged. Either there’s something I don’t get, > or the factor is taken care of in the resolution constant, or it’s a > bug. > " > > I need to do some thinking on whether the current code is right (even > if it isn't, it can't be changed as it would break software). Then > what the factor needs to be. > > Thanks, > Roderick > > On Fri, Dec 30, 2022 at 10:12 AM Alain <alain.carlucci@gmail.com> wrote: > > > > Hi Roderick, > > Hi Daniel, > > > > Thank you for all the suggestions, Roderick. > > I fixed the typo in your patch, backported to hid-sony and tested both of them. > > They fix the issue and the DS4 can be used even without calibration data. > > I cannot test if everything works well with a DualSense because I do not own one. > > > > Hi Daniel, I've seen that in the hid-nintendo driver, the calibration denominator is not sanitized if it's zero. > > If I'm not mistaken, a device that disguises itself as a joycon with denominator zero can crash the kernel > > when this is used in the mult_frac(). I had this behaviour with the hid-sony driver. > > You can find attached the patch that should solve the problem. > > > > Thanks, > > Alain > > > > > > Il giorno gio 29 dic 2022 alle ore 21:56 Roderick Colenbrander <thunderbird2k@gmail.com> ha scritto: > >> > >> Hi Alain, > >> > >> On Thu, Dec 29, 2022 at 11:21 AM Alain Carlucci > >> <alain.carlucci@gmail.com> wrote: > >> > > >> > Hi Roderick, > >> > > >> > >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. > >> > > >> > Thank you for the patch, just tried. I think there is a typo in the > >> > condition of the for-loop that sanitizes the input. > >> > Instead of `; i < ARRAY_SIZE()` it's written `; ARRAY_SIZE()`, > >> > which always evaluates to true. The loop then overflows the array and > >> > crashes the kernel. > >> > >> Ooh oops. It was a quick patch I wrote without testing. > >> > >> > By the way, the DualSense code has similar calibration code and also > >> > there the input is not sanitized. > >> > I think it's quite easy to create a fake DualSense device with > >> > an Arduino that emulates the protocol up to calib_denom=0, just to > >> > exploit that and crash every linux machine is plugged in. Or even > >> > worst, exploit that via bluetooth. > >> > >> You are right it probably won't hurt to duplicate the logic there. > >> > >> > >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. > >> > > >> > Thank you! Just discovered that this strange DS4 (CUH-ZCT2E) replies > >> > all zeros both to HID request 0x02 (get calibration data) and as the > >> > BT address (request 0x12, Pairing Info), which explains why BT is not > >> > working. > >> > >> There is definitely something weird with your device. Something must > >> have gotten wiped somehow. > >> > >> > I tried to request version info (0xa3), the response seems the same as > >> > another fully-working and original CUH-ZCT2E: > >> > """ > >> > Build Date: Sep 21 2018 04:50:51 > >> > HW Version: 0100.b008 > >> > SW Version: 00000001.a00a > >> > SW Series: 2010 > >> > Code Size: 0002a000 > >> > """ > >> > > >> > Anyway, I have seen that request 0xA2 puts the DS4 in an alternate > >> > boot mode, probably DFU mode, where the device reboots as 054c:0856 > >> > and waits for data, which seems totally undocumented online. > >> > Do you know anything about this mode? > >> > It would be amazing to be able to reflash the original firmware, > >> > >> Unfortunately I can't disclose any of that information. I can say that > >> on DS4 it wasn't common to update firmware (as opposed to DualSense) > >> while out in the wild. Some of these requests and others are probably > >> used to initiate firmware programming and programming of MAC address, > >> calibration data and other settings. It is probably quite involved and > >> hard to get right without bricking your device. > >> > >> > Thanks, > >> > Alain > >> > >> Thanks, > >> Roderick
Hi Roderick, sorry for the late response, I've been busy in these days. I'll reply to both emails in this one. >Thanks for testing. I added a similar patch to my hid-playstation tree >this morning (hid-sony will go away soon). Ok perfect! Does hid-sony will continue to support old Dualshock controllers or they will be all merged into hid-playstation? >I'm not entirely happy with the patches yet and need to do some >thinking. The issue is the value range, which is not correct. For the >accelerometer the numerator and denominator need to be 1 to match the >proper range. (It just happens that the scaling factors are the way >they are I think.) >The uncalibrated gyroscope values are 16-bit signed, but the hardware >limits are +/- 2000 degree per second. To have enough precision during >calibration the value range is extended (resolution per axis is set to >1024, so 1024 corresponds to 1 degree per second). Can you tell me a bit more about what is the expected calibrated output for both gyro and accelerometer? Like unit of measurement (I hope deg/s and m/s^2) and data type? Is "1024 -> 1dg/s" a mapping used only while calibrating or is the expected conversion during normal usage? >When I heavily shake a DS4 in evtest, I can easily reach values like >15 million or more. However, the maximum value we have set on the >evdev node to about 2M. In other words there is something wrong. I >need to look closely at other internal code. Definitely seems there is an issue. Can you suggest me a tool to test the gyro/accel using linux? Thanks, Alain
Hi Roderick, I just tried to see with evtest the values of gyro/accel after changing the driver so that follows the suggestion on the dsremap website: dividing by two speed_2x: speed_2x = (gyro_speed_plus + gyro_speed_minus) >> 1; The DS4 shows values no higher than 600000 (post-calibration) while heavily shaking the joystick. For the record, the calibration is: gyro_pitch_plus: 8848 gyro_pitch_minus: -8853 gyro_yaw_plus: 8833 gyro_yaw_minus: -8827 gyro_roll_plus: 8856 gyro_roll_minus: -8841 gyro_speed_plus: 540 gyro_speed_minus: 540 acc_x_plus: 8107 acc_x_minus: -8107 acc_y_plus: 8259 acc_y_minus: -8259 acc_z_plus: 8187 acc_z_minus: -8186 This is an example of the output: Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 128610 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 95747 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 61321 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 28864 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 874 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -27802 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -54949 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -82064 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -110398 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -138107 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -170345 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -205239 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -242320 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -281525 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -318043 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -356748 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -394453 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -430628 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -465428 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -496105 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -526469 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -551897 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -554865 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -522127 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -450933 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -323041 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -180404 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -44859 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 71006 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 148353 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 202209 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 242757 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 274183 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 298456 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 316106 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 331569 Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 338942 Thanks, Alain
Hi Alain,
I'm technically still on vacation, but this issue annoyed me a lot. I
had a look at other internal code and I'm relieved the code is
correct. (Well there is one tiny little issue around calibration,
which I will fix, but in practice only causes a tiny, tiny error).
I don't know why I thought it was wrong. Likely I had worked on a
patched kernel with some logic changed which caused these high values.
In any case I mentioned how the ratio was around 60 on my DS4. It
didn't make sense to me, but it is basically: 2000 / 32768 * 1024
(62.5). The 2000 is the range of the sensor, 32768 max value of 16-bit
signed, 1024 the scaling factor we use in the driver.
So basically:+ for (i = 0; i < 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_RANGE;
+ ds4->gyro_calib_data[i].sens_denom = S16_MAX;
+ }
+ }
The same for accelerometer.
I will clean up the patch, add some other patches and send them out shortly.
Thanks,
Roderick
On Thu, Jan 5, 2023 at 10:24 AM Alain Carlucci <alain.carlucci@gmail.com> wrote:
>
> Hi Roderick,
>
> I just tried to see with evtest the values of gyro/accel after
> changing the driver so that follows the suggestion on the dsremap
> website: dividing by two speed_2x:
> speed_2x = (gyro_speed_plus + gyro_speed_minus) >> 1;
>
> The DS4 shows values no higher than 600000 (post-calibration) while
> heavily shaking the joystick. For the record, the calibration is:
>
> gyro_pitch_plus: 8848 gyro_pitch_minus: -8853
> gyro_yaw_plus: 8833 gyro_yaw_minus: -8827
> gyro_roll_plus: 8856 gyro_roll_minus: -8841
> gyro_speed_plus: 540 gyro_speed_minus: 540
> acc_x_plus: 8107 acc_x_minus: -8107
> acc_y_plus: 8259 acc_y_minus: -8259
> acc_z_plus: 8187 acc_z_minus: -8186
>
> This is an example of the output:
>
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 128610
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 95747
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 61321
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 28864
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 874
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -27802
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -54949
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -82064
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -110398
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -138107
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -170345
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -205239
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -242320
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -281525
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -318043
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -356748
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -394453
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -430628
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -465428
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -496105
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -526469
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -551897
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -554865
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -522127
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -450933
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -323041
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -180404
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value -44859
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 71006
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 148353
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 202209
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 242757
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 274183
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 298456
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 316106
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 331569
> Event: time [...], type 3 (EV_ABS), code 3 (ABS_RX), value 338942
>
> Thanks,
> Alain
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c index 13125997a..f8b05cb29 100644 --- a/drivers/hid/hid-sony.c +++ b/drivers/hid/hid-sony.c @@ -1714,6 +1714,7 @@ static int dualshock4_get_calibration_data(struct sony_sc *sc) short acc_z_plus, acc_z_minus; int speed_2x; int range_2g; + int calib_id; /* For Bluetooth we use a different request, which supports CRC. * Note: in Bluetooth mode feature report 0x02 also changes the state @@ -1858,6 +1859,23 @@ static int dualshock4_get_calibration_data(struct sony_sc *sc) sc->ds4_calib_data[5].sens_numer = 2*DS4_ACC_RES_PER_G; sc->ds4_calib_data[5].sens_denom = range_2g; + for (calib_id = 0; calib_id < 6; calib_id++) { + /* Ensure there are no denominators equal to zero to prevent + * crashes while dividing by that number. + */ + + if (sc->ds4_calib_data[calib_id].sens_denom != 0) { + /* Denominator OK, skip this */ + continue; + } + + sc->ds4_calib_data[calib_id].sens_denom = 1; + + hid_warn(sc->hdev, + "DualShock 4 USB dongle: invalid calibration for sensor %d\n", + calib_id); + } + err_stop: kfree(buf); return ret;
Hello, Today I connected a partially broken DS4 via USB and got a kernel panic with a division by zero in the hid-sony driver. The issue is caused by sens_denom=0 in the sensor calibration data, which triggers a division by zero when dualshock4_parse_report() is invoked, the division happens in the mult_frac() macro. This patch applies a workaround that allows the DS4 to be used even with a broken sensor: if the denominator sent by the device is zero, it is replaced with 1 and a warning is emitted. Signed-off-by: Alain Carlucci <alain.carlucci@gmail.com> --- drivers/hid/hid-sony.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)