[patch] hid: fix autocentering of PID devices

[patch] hid: fix autocentering of PID devices

[Anssi Hannula]

From: Diogo Kastrup <dk@bighost.com.br>

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 <dk@bighost.com.br>
Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---

--- linux/drivers/hid/usbhid/hid-pidff.bkp	2007-06-12 20:39:23.000000000 
-0300
+++ linux/drivers/hid/usbhid/hid-pidff.c	2007-06-12 19:19:57.000000000 -0300
@@ -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],

-- 
Anssi Hannula

Re: [patch] hid: fix autocentering of PID devices

[Jiri Kosina]

On Wed, 13 Jun 2007, Anssi Hannula wrote:

> 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?

-- 
Jiri Kosina

Re: [patch] hid: fix autocentering of PID devices

[Anssi Hannula]

Jiri Kosina wrote:
> On Wed, 13 Jun 2007, Anssi Hannula wrote:
> 
>> 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!

> 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?

-- 
Anssi Hannula

Re: [patch] hid: fix autocentering of PID devices

[Jiri Kosina]

On Thu, 14 Jun 2007, Anssi Hannula wrote:

> > 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,

-- 
Jiri Kosina

Re: [patch] hid: fix autocentering of PID devices

[Anssi Hannula]

Jiri Kosina wrote:
> On Thu, 14 Jun 2007, Anssi Hannula wrote:
> 
>>> 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 :)

-- 
Anssi Hannula

Re: [patch] hid: fix autocentering of PID devices

[Diogo Kastrup]

Anssi Hannula wrote:
> Jiri Kosina wrote:
> > On Thu, 14 Jun 2007, Anssi Hannula wrote:
> > 
> >>> 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 :)

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.
-- 
Diogo Kastrup <dk@bighost.com.br>

Re: [patch] hid: fix autocentering of PID devices

[Jiri Kosina]

On Thu, 14 Jun 2007, Anssi Hannula wrote:

> > 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,

-- 
Jiri Kosina

Re: [patch] hid: fix autocentering of PID devices

[Anssi Hannula]

Jiri Kosina wrote:
> On Thu, 14 Jun 2007, Anssi Hannula wrote:
> 
>>> 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!

> 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

-- 
Anssi Hannula

Re: [patch] hid: fix autocentering of PID devices

[Jiri Kosina]

On Wed, 20 Jun 2007, Anssi Hannula wrote:

> > 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,

-- 
Jiri Kosina

Re: [patch] hid: fix autocentering of PID devices

[Anssi Hannula]

Jiri Kosina wrote:
> On Wed, 20 Jun 2007, Anssi Hannula wrote:
> 
>>> 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!

> 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.

-- 
Anssi Hannula

Re: [patch] hid: fix autocentering of PID devices

[Jiri Kosina]

On Thu, 21 Jun 2007, Anssi Hannula wrote:

> > 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,

-- 
Jiri Kosina