linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] HID:hid-lg4ff: Delay to allow wheels to center after plug-in
@ 2016-10-26  1:26 Simon Wood
  2016-10-26  7:36 ` Michal Malý
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Wood @ 2016-10-26  1:26 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Jiri Kosina, Edwin Velds, Elias Vanderstuyft,
	Michal Maly, Simon Wood

A number of wheels (G27/etc) do a little full right/full left 'dance'
when first plugged in. This patch inserts a delay so that this 'dance'
is completed before we disable (set to zero) the autocenter spring.

A side benefit is that the DFGT was confused without the delay, and is
now correctly being set to 900' rotation mode.

Side Effect - and the reason I am sending as RFC. This 8s delay seems
to have an effect on other wheels connected at the same time.

With 3 wheels on a hub, and then the hub connected to PC. The wheel
on the right in video below waits for G27 to complete this 8s, before
it will do it's 'dance' and register with the system.

https://www.youtube.com/watch?v=xCVpCw_yGgA

I don't know if this is a problem, or if someone here has suggestions
on a better way to implement the delay...
---
 drivers/hid/hid-lg4ff.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index af3a8ec..3eee920 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -1248,6 +1248,8 @@ int lg4ff_init(struct hid_device *hid)
 	/* Check if autocentering is available and
 	 * set the centering force to zero by default */
 	if (test_bit(FF_AUTOCENTER, dev->ffbit)) {
+		wait_queue_head_t wait;
+
 		/* Formula Force EX expects different autocentering command */
 		if ((bcdDevice >> 8) == LG4FF_FFEX_REV_MAJ &&
 		    (bcdDevice & 0xff) == LG4FF_FFEX_REV_MIN)
@@ -1255,6 +1257,14 @@ int lg4ff_init(struct hid_device *hid)
 		else
 			dev->ff->set_autocenter = lg4ff_set_autocenter_default;
 
+		/* insert a 8s delay to allow DFGT/G25/G27/G29 wheels to return to center position*/
+		if (lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_DFGT_WHEEL ||
+				lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_G25_WHEEL ||
+				lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_G27_WHEEL ||
+				lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_G29_WHEEL) {
+			init_waitqueue_head (&wait);
+			wait_event_interruptible_timeout(wait, 0, msecs_to_jiffies(8000));
+		}
 		dev->ff->set_autocenter(dev, 0);
 	}
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC] HID:hid-lg4ff: Delay to allow wheels to center after plug-in
  2016-10-26  1:26 [RFC] HID:hid-lg4ff: Delay to allow wheels to center after plug-in Simon Wood
@ 2016-10-26  7:36 ` Michal Malý
  2016-10-26 17:00   ` Dmitry Torokhov
  0 siblings, 1 reply; 4+ messages in thread
From: Michal Malý @ 2016-10-26  7:36 UTC (permalink / raw)
  To: Simon Wood
  Cc: linux-input, linux-kernel, Jiri Kosina, Edwin Velds,
	Elias Vanderstuyft, Simon Wood

Hi all,

maybe instead of stalling the entire init() function it'd be better to put a 
request to disable autocentering on a workqueue and start a delayed work once 
the init() function is done setting the wheel up? There'd be a bit more code 
to write though...

Michal

On Tuesday, October 25, 2016 7:26:12 PM CEST Simon Wood wrote:
> A number of wheels (G27/etc) do a little full right/full left 'dance'
> when first plugged in. This patch inserts a delay so that this 'dance'
> is completed before we disable (set to zero) the autocenter spring.
> 
> A side benefit is that the DFGT was confused without the delay, and is
> now correctly being set to 900' rotation mode.
> 
> Side Effect - and the reason I am sending as RFC. This 8s delay seems
> to have an effect on other wheels connected at the same time.
> 
> With 3 wheels on a hub, and then the hub connected to PC. The wheel
> on the right in video below waits for G27 to complete this 8s, before
> it will do it's 'dance' and register with the system.
> 
> https://www.youtube.com/watch?v=xCVpCw_yGgA
> 
> I don't know if this is a problem, or if someone here has suggestions
> on a better way to implement the delay...
> ---
>  drivers/hid/hid-lg4ff.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
> index af3a8ec..3eee920 100644
> --- a/drivers/hid/hid-lg4ff.c
> +++ b/drivers/hid/hid-lg4ff.c
> @@ -1248,6 +1248,8 @@ int lg4ff_init(struct hid_device *hid)
>  	/* Check if autocentering is available and
>  	 * set the centering force to zero by default */
>  	if (test_bit(FF_AUTOCENTER, dev->ffbit)) {
> +		wait_queue_head_t wait;
> +
>  		/* Formula Force EX expects different autocentering command */
>  		if ((bcdDevice >> 8) == LG4FF_FFEX_REV_MAJ &&
>  		    (bcdDevice & 0xff) == LG4FF_FFEX_REV_MIN)
> @@ -1255,6 +1257,14 @@ int lg4ff_init(struct hid_device *hid)
>  		else
>  			dev->ff->set_autocenter = lg4ff_set_autocenter_default;
> 
> +		/* insert a 8s delay to allow DFGT/G25/G27/G29 wheels to return to 
center
> position*/ +		if (lg4ff_devices[i].product_id ==
> USB_DEVICE_ID_LOGITECH_DFGT_WHEEL || +				
lg4ff_devices[i].product_id ==
> USB_DEVICE_ID_LOGITECH_G25_WHEEL || +				lg4ff_devices[i].product_id 
==
> USB_DEVICE_ID_LOGITECH_G27_WHEEL || +				lg4ff_devices[i].product_id 
==
> USB_DEVICE_ID_LOGITECH_G29_WHEEL) { +			init_waitqueue_head (&wait);
> +			wait_event_interruptible_timeout(wait, 0, msecs_to_jiffies(8000));
> +		}
>  		dev->ff->set_autocenter(dev, 0);
>  	}

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] HID:hid-lg4ff: Delay to allow wheels to center after plug-in
  2016-10-26  7:36 ` Michal Malý
@ 2016-10-26 17:00   ` Dmitry Torokhov
  2016-10-27 23:30     ` Simon Wood
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2016-10-26 17:00 UTC (permalink / raw)
  To: Michal Malý
  Cc: Simon Wood, linux-input, lkml, Jiri Kosina, Edwin Velds,
	Elias Vanderstuyft, Simon Wood

On Wed, Oct 26, 2016 at 12:36 AM, Michal Malý <madcatxster@gmail.com> wrote:
> Hi all,
>
> maybe instead of stalling the entire init() function it'd be better to put a
> request to disable autocentering on a workqueue and start a delayed work once
> the init() function is done setting the wheel up? There'd be a bit more code
> to write though...
>

Yeah, if device happens to be plugged at boot time it will wait that
long before even getting to mount rootfs if driver is built into the
kernel.

This is also interesting way to say "msleep(8000)".

> Michal
>
> On Tuesday, October 25, 2016 7:26:12 PM CEST Simon Wood wrote:
>> A number of wheels (G27/etc) do a little full right/full left 'dance'
>> when first plugged in. This patch inserts a delay so that this 'dance'
>> is completed before we disable (set to zero) the autocenter spring.
>>
>> A side benefit is that the DFGT was confused without the delay, and is
>> now correctly being set to 900' rotation mode.
>>
>> Side Effect - and the reason I am sending as RFC. This 8s delay seems
>> to have an effect on other wheels connected at the same time.
>>
>> With 3 wheels on a hub, and then the hub connected to PC. The wheel
>> on the right in video below waits for G27 to complete this 8s, before
>> it will do it's 'dance' and register with the system.
>>
>> https://www.youtube.com/watch?v=xCVpCw_yGgA
>>
>> I don't know if this is a problem, or if someone here has suggestions
>> on a better way to implement the delay...
>> ---
>>  drivers/hid/hid-lg4ff.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
>> index af3a8ec..3eee920 100644
>> --- a/drivers/hid/hid-lg4ff.c
>> +++ b/drivers/hid/hid-lg4ff.c
>> @@ -1248,6 +1248,8 @@ int lg4ff_init(struct hid_device *hid)
>>       /* Check if autocentering is available and
>>        * set the centering force to zero by default */
>>       if (test_bit(FF_AUTOCENTER, dev->ffbit)) {
>> +             wait_queue_head_t wait;
>> +
>>               /* Formula Force EX expects different autocentering command */
>>               if ((bcdDevice >> 8) == LG4FF_FFEX_REV_MAJ &&
>>                   (bcdDevice & 0xff) == LG4FF_FFEX_REV_MIN)
>> @@ -1255,6 +1257,14 @@ int lg4ff_init(struct hid_device *hid)
>>               else
>>                       dev->ff->set_autocenter = lg4ff_set_autocenter_default;
>>
>> +             /* insert a 8s delay to allow DFGT/G25/G27/G29 wheels to return to
> center
>> position*/ +          if (lg4ff_devices[i].product_id ==
>> USB_DEVICE_ID_LOGITECH_DFGT_WHEEL || +
> lg4ff_devices[i].product_id ==
>> USB_DEVICE_ID_LOGITECH_G25_WHEEL || +                         lg4ff_devices[i].product_id
> ==
>> USB_DEVICE_ID_LOGITECH_G27_WHEEL || +                         lg4ff_devices[i].product_id
> ==
>> USB_DEVICE_ID_LOGITECH_G29_WHEEL) { +                 init_waitqueue_head (&wait);
>> +                     wait_event_interruptible_timeout(wait, 0, msecs_to_jiffies(8000));
>> +             }
>>               dev->ff->set_autocenter(dev, 0);
>>       }
>

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] HID:hid-lg4ff: Delay to allow wheels to center after plug-in
  2016-10-26 17:00   ` Dmitry Torokhov
@ 2016-10-27 23:30     ` Simon Wood
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Wood @ 2016-10-27 23:30 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: "Michal Malý",
	Simon Wood, linux-input, lkml, Jiri Kosina, Edwin Velds,
	Elias Vanderstuyft, Simon Wood

On Wed, October 26, 2016 11:00 am, Dmitry Torokhov wrote:
>
> Yeah, if device happens to be plugged at boot time it will wait that
> long before even getting to mount rootfs if driver is built into the
> kernel.

Not a case I had considered... that would probably be bad. I'll look at
coding up a work queue as Elias had suggested.

>
> This is also interesting way to say "msleep(8000)".

I'm a copy/paste guy, this was from the delay we have in hid-lg.c probe,
if one way or another is better I'll use that,
Simon

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-10-27 23:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-26  1:26 [RFC] HID:hid-lg4ff: Delay to allow wheels to center after plug-in Simon Wood
2016-10-26  7:36 ` Michal Malý
2016-10-26 17:00   ` Dmitry Torokhov
2016-10-27 23:30     ` Simon Wood

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).