Skip to contentSkip to conversation

hid: fix autocentering of PID devices

PATCH In review Anssi Hannula 3 participants 11 messages Jun 13, 2007 – Jun 21, 2007

Subsystems (3)
HID USB HID/hidbp drivers (USB keyboards, mice, remote controls, ...) USB HID pid drivers (USB wheelbases, joysticks, rudders, ...)
lore
Share & tools
lore

Anssi Hannula

author
#0 lore raw

PATCH 1/1 hid: fix autocentering of PID devices

From: Diogo Kastrup <[email protected]>

When setting the autocentering of PID devices, PID_DIRECTION_ENABLE is not being explicitely set to 1. This results in autocentering working only on the vertical axis when this field is preset to 0. This seems to happen since 2.6.21.

Fix that by setting it explicitely to 1 when preparing the set_effect report for autocentering spring effect.

Signed-off-by Diogo Kastrup <[email protected]>
Signed-off-by Anssi Hannula <[email protected]>

---

drivers/hid/usbhid/hid-pidff.c renamed from drivers/hid/usbhid/hid-pidff.bkp +1 -0
@@ -738,6 +738,7 @@
 	pidff->set_effect[PID_TRIGGER_BUTTON].value[0] = 0;
 	pidff->set_effect[PID_TRIGGER_REPEAT_INT].value[0] = 0;
 	pidff_set(&pidff->set_effect[PID_GAIN], magnitude);
+	pidff->set_effect[PID_DIRECTION_ENABLE].value[0] = 1;
 	pidff->set_effect[PID_START_DELAY].value[0] = 0;
 
 	usbhid_submit_report(pidff->hid, pidff->reports[PID_SET_EFFECT],
Signature · 1 line
Anssi Hannula
#1 lore raw
Quoted 4 lines · Anssi Hannula

When setting the autocentering of PID devices, PID_DIRECTION_ENABLE is not being explicitely set to 1. This results in autocentering working only on the vertical axis when this field is preset to 0. This seems to happen since 2.6.21.

Hi Anssi,

do you have any idea what caused this to happen in 2.6.21 and not in earlier versions?

Signature · 1 line
Jiri Kosina
author
#2 lore raw
Quoted 5 lines · Jiri Kosina
Quoted 1 line · Anssi Hannula

When setting the autocentering of PID devices, PID_DIRECTION_ENABLE is not being explicitely set to 1. This results in autocentering working only on the vertical axis when this field is preset to 0. This seems to happen since 2.6.21.

Hi Anssi,

Hi!

Quoted 1 line

do you have any idea what caused this to happen in 2.6.21 and not in earlier versions?

Nope. Do you think it would be useful for me to bisect 2.6.20-2.6.21?

Signature · 1 line
Anssi Hannula
#3 lore raw
Quoted 3 lines · Anssi Hannula
Quoted 2 lines

do you have any idea what caused this to happen in 2.6.21 and not in earlier versions?

Nope. Do you think it would be useful for me to bisect 2.6.20-2.6.21?

I'd of course happily merge the fix which initializes the value properly into my tree, but I would be curious what broke it, to double check that we didn't break anything else along with it.

Thanks,

Signature · 1 line
Jiri Kosina
author
#4 lore raw
Quoted 6 lines · Jiri Kosina
Quoted 2 lines · Anssi Hannula
Quoted 1 line

do you have any idea what caused this to happen in 2.6.21 and not in earlier versions?

Nope. Do you think it would be useful for me to bisect 2.6.20-2.6.21?

I'd of course happily merge the fix which initializes the value properly into my tree, but I would be curious what broke it, to double check that we didn't break anything else along with it.

Okay, I'll get back to you in a few days :)

Signature · 1 line
Anssi Hannula
#5 lore raw
Quoted 5 lines · Anssi Hannula
Quoted 4 lines · Jiri Kosina
Quoted 2 lines · Anssi Hannula
Quoted 2 lines

do you have any idea what caused this to happen in 2.6.21 and not in earlier versions?

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Nope. Do you think it would be useful for me to bisect 2.6.20-2.6.21?

I'd of course happily merge the fix which initializes the value properly into my tree, but I would be curious what broke it, to double check that we didn't break anything else along with it.

Okay, I'll get back to you in a few days :)

Hi,

Now that I was trying to remember the first time I saw this problem, I think it was randomly showing up long time ago, then stopped. I thought it was on the last kernel because recently I stepped on it again while writing a program to calibrate and configure my joystick forces. But I don't use my joystick on linux that much so I'm not sure, sorry.

Signature · 1 line
Diogo Kastrup <[email protected]>
#6 lore raw
In reply to Anssi Hannula
Quoted 4 lines · Anssi Hannula
Quoted 3 lines

I'd of course happily merge the fix which initializes the value properly into my tree, but I would be curious what broke it, to double check that we didn't break anything else along with it.

Okay, I'll get back to you in a few days :)

Hi Anssi,

did you make any progress here please? I am going to merge the patch, still I'd be understand what caused it (on the other hand from Diogo's last comment it doesn't seem 100% certain that we cased it somewhere around 2.6.20).

Thanks,

Signature · 1 line
Jiri Kosina
author
#7 lore raw
Quoted 6 lines · Jiri Kosina
Quoted 2 lines · Anssi Hannula
Quoted 1 line

I'd of course happily merge the fix which initializes the value properly into my tree, but I would be curious what broke it, to double check that we didn't break anything else along with it.

Okay, I'll get back to you in a few days :)

Hi Anssi,

Hi!

Quoted 1 line

did you make any progress here please? I am going to merge the patch, still I'd be understand what caused it (on the other hand from Diogo's last comment it doesn't seem 100% certain that we cased it somewhere around 2.6.20).

I finally had time to bisect 2.6.20 vs 2.6.21, and for me it seems to start happening after commit 4237081e573b99a48991aa71364b0682c444651c:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=4237081e573b99a48991aa71364b0682c444651c

Signature · 1 line
Anssi Hannula
#8 lore raw
Quoted 6 lines · Anssi Hannula
Quoted 4 lines

did you make any progress here please? I am going to merge the patch, still I'd be understand what caused it (on the other hand from Diogo's last comment it doesn't seem 100% certain that we cased it somewhere around 2.6.20).

I finally had time to bisect 2.6.20 vs 2.6.21, and for me it seems to start happening after commit 4237081e573b99a48991aa71364b0682c444651c:

Hi Anssi,

thanks a lot. Could you please check any kernel after the commit 46386b587086c8d2698222a031bf749688464032 (for example 2.6.22-rc1)? That should be the definitive fix for all issues with zeroing of unused bits in output reports.

Thanks,

Signature · 1 line
Jiri Kosina
author
#9 lore raw
Quoted 6 lines · Jiri Kosina
Quoted 2 lines · Anssi Hannula
Quoted 1 line

did you make any progress here please? I am going to merge the patch, still I'd be understand what caused it (on the other hand from Diogo's last comment it doesn't seem 100% certain that we cased it somewhere around 2.6.20).

I finally had time to bisect 2.6.20 vs 2.6.21, and for me it seems to start happening after commit 4237081e573b99a48991aa71364b0682c444651c:

Hi Anssi,

Hi!

Quoted 1 line

thanks a lot. Could you please check any kernel after the commit 46386b587086c8d2698222a031bf749688464032 (for example 2.6.22-rc1)? That should be the definitive fix for all issues with zeroing of unused bits in output reports.

Does not seem to work correctly with 2.6.22-rc1, either.

Signature · 1 line
Anssi Hannula
#10 lore raw
Quoted 5 lines · Anssi Hannula
Quoted 4 lines

thanks a lot. Could you please check any kernel after the commit 46386b587086c8d2698222a031bf749688464032 (for example 2.6.22-rc1)? That should be the definitive fix for all issues with zeroing of unused bits in output reports.

Does not seem to work correctly with 2.6.22-rc1, either.

OK, now I see. It worked due to luck previously, and Diego's fix is a proper one - the value needs to be set explicitly, we can't rely on bits in the output report being set previously to any known value.

Thanks,

Signature · 1 line
Jiri Kosina