public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sony-laptop - remove private workqueue, use keventd instead
@ 2009-12-03  5:07 Dmitry Torokhov
  2009-12-03 22:01 ` Mattia Dongili
  0 siblings, 1 reply; 2+ messages in thread
From: Dmitry Torokhov @ 2009-12-03  5:07 UTC (permalink / raw)
  To: Mattia Dongili; +Cc: linux-acpi

If we reschedule work instead of having work function sleep for 10 msecs
between reads from kfifo we can safely use the main workqueue (keventd)
and not bother with creating driver-private one.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/platform/x86/sony-laptop.c |   57 ++++++++++++++++++++----------------
 1 files changed, 31 insertions(+), 26 deletions(-)


diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index a2a742c..b6d2961 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -144,7 +144,6 @@ struct sony_laptop_input_s {
 	struct input_dev	*key_dev;
 	struct kfifo		*fifo;
 	spinlock_t		fifo_lock;
-	struct workqueue_struct	*wq;
 };
 
 static struct sony_laptop_input_s sony_laptop_input = {
@@ -298,17 +297,28 @@ static int sony_laptop_input_keycode_map[] = {
 /* release buttons after a short delay if pressed */
 static void do_sony_laptop_release_key(struct work_struct *work)
 {
+	struct delayed_work *dwork =
+			container_of(work, struct delayed_work, work);
 	struct sony_laptop_keypress kp;
 
-	while (kfifo_get(sony_laptop_input.fifo, (unsigned char *)&kp,
-			 sizeof(kp)) == sizeof(kp)) {
-		msleep(10);
+	if (kfifo_get(sony_laptop_input.fifo,
+		      (unsigned char *)&kp, sizeof(kp)) == sizeof(kp)) {
 		input_report_key(kp.dev, kp.key, 0);
 		input_sync(kp.dev);
 	}
+
+	/*
+	 * If there is something in the fifo scnhedule nect release.
+	 * We don't care about locking, at worst we may schedule an
+	 * extra "empty" wakeup. This may be improved with the new
+	 * kfifo API.
+	 */
+	if (__kfifo_len(sony_laptop_input.fifo) != 0)
+		schedule_delayed_work(dwork, msecs_to_jiffies(10));
 }
-static DECLARE_WORK(sony_laptop_release_key_work,
-		do_sony_laptop_release_key);
+
+static DECLARE_DELAYED_WORK(sony_laptop_release_key_work,
+			    do_sony_laptop_release_key);
 
 /* forward event to the input subsystem */
 static void sony_laptop_report_input_event(u8 event)
@@ -362,12 +372,12 @@ static void sony_laptop_report_input_event(u8 event)
 		/* we emit the scancode so we can always remap the key */
 		input_event(kp.dev, EV_MSC, MSC_SCAN, event);
 		input_sync(kp.dev);
+
+		/* schedule key release */
 		kfifo_put(sony_laptop_input.fifo,
 			  (unsigned char *)&kp, sizeof(kp));
-
-		if (!work_pending(&sony_laptop_release_key_work))
-			queue_work(sony_laptop_input.wq,
-					&sony_laptop_release_key_work);
+		schedule_delayed_work(&sony_laptop_release_key_work,
+				      msecs_to_jiffies(10));
 	} else
 		dprintk("unknown input event %.2x\n", event);
 }
@@ -394,20 +404,11 @@ static int sony_laptop_setup_input(struct acpi_device *acpi_device)
 		goto err_dec_users;
 	}
 
-	/* init workqueue */
-	sony_laptop_input.wq = create_singlethread_workqueue("sony-laptop");
-	if (!sony_laptop_input.wq) {
-		printk(KERN_ERR DRV_PFX
-				"Unable to create workqueue.\n");
-		error = -ENXIO;
-		goto err_free_kfifo;
-	}
-
 	/* input keys */
 	key_dev = input_allocate_device();
 	if (!key_dev) {
 		error = -ENOMEM;
-		goto err_destroy_wq;
+		goto err_free_kfifo;
 	}
 
 	key_dev->name = "Sony Vaio Keys";
@@ -470,9 +471,6 @@ err_unregister_keydev:
 err_free_keydev:
 	input_free_device(key_dev);
 
-err_destroy_wq:
-	destroy_workqueue(sony_laptop_input.wq);
-
 err_free_kfifo:
 	kfifo_free(sony_laptop_input.fifo);
 
@@ -483,12 +481,20 @@ err_dec_users:
 
 static void sony_laptop_remove_input(void)
 {
+	struct sony_laptop_keypress kp = { NULL };
+
 	/* cleanup only after the last user has gone */
 	if (!atomic_dec_and_test(&sony_laptop_input.users))
 		return;
 
-	/* flush workqueue first */
-	flush_workqueue(sony_laptop_input.wq);
+	cancel_delayed_work_sync(&sony_laptop_release_key_work);
+
+	/* Generate key-up events for remaining keys */
+	while (kfifo_get(sony_laptop_input.fifo,
+			 (unsigned char *)&kp, sizeof(kp)) == sizeof(kp)) {
+		input_report_key(kp.dev, kp.key, 0);
+		input_sync(kp.dev);
+	}
 
 	/* destroy input devs */
 	input_unregister_device(sony_laptop_input.key_dev);
@@ -499,7 +505,6 @@ static void sony_laptop_remove_input(void)
 		sony_laptop_input.jog_dev = NULL;
 	}
 
-	destroy_workqueue(sony_laptop_input.wq);
 	kfifo_free(sony_laptop_input.fifo);
 }
 
-- 
Dmitry

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

* Re: [PATCH] sony-laptop - remove private workqueue, use keventd instead
  2009-12-03  5:07 [PATCH] sony-laptop - remove private workqueue, use keventd instead Dmitry Torokhov
@ 2009-12-03 22:01 ` Mattia Dongili
  0 siblings, 0 replies; 2+ messages in thread
From: Mattia Dongili @ 2009-12-03 22:01 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-acpi

On Wed, Dec 02, 2009 at 09:07:42PM -0800, Dmitry Torokhov wrote:
> If we reschedule work instead of having work function sleep for 10 msecs
> between reads from kfifo we can safely use the main workqueue (keventd)
> and not bother with creating driver-private one.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Hi Dmitry,

Looks reasonable to me. Let me take a deeper look and give it a go and
I'll submit it for inclusion together with another pending patch I have
for 2.6.33.

Thanks
mattia

> ---
> 
>  drivers/platform/x86/sony-laptop.c |   57 ++++++++++++++++++++----------------
>  1 files changed, 31 insertions(+), 26 deletions(-)
> 
> 
> diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
> index a2a742c..b6d2961 100644
> --- a/drivers/platform/x86/sony-laptop.c
> +++ b/drivers/platform/x86/sony-laptop.c
> @@ -144,7 +144,6 @@ struct sony_laptop_input_s {
>  	struct input_dev	*key_dev;
>  	struct kfifo		*fifo;
>  	spinlock_t		fifo_lock;
> -	struct workqueue_struct	*wq;
>  };
>  
>  static struct sony_laptop_input_s sony_laptop_input = {
> @@ -298,17 +297,28 @@ static int sony_laptop_input_keycode_map[] = {
>  /* release buttons after a short delay if pressed */
>  static void do_sony_laptop_release_key(struct work_struct *work)
>  {
> +	struct delayed_work *dwork =
> +			container_of(work, struct delayed_work, work);
>  	struct sony_laptop_keypress kp;
>  
> -	while (kfifo_get(sony_laptop_input.fifo, (unsigned char *)&kp,
> -			 sizeof(kp)) == sizeof(kp)) {
> -		msleep(10);
> +	if (kfifo_get(sony_laptop_input.fifo,
> +		      (unsigned char *)&kp, sizeof(kp)) == sizeof(kp)) {
>  		input_report_key(kp.dev, kp.key, 0);
>  		input_sync(kp.dev);
>  	}
> +
> +	/*
> +	 * If there is something in the fifo scnhedule nect release.
> +	 * We don't care about locking, at worst we may schedule an
> +	 * extra "empty" wakeup. This may be improved with the new
> +	 * kfifo API.
> +	 */
> +	if (__kfifo_len(sony_laptop_input.fifo) != 0)
> +		schedule_delayed_work(dwork, msecs_to_jiffies(10));
>  }
> -static DECLARE_WORK(sony_laptop_release_key_work,
> -		do_sony_laptop_release_key);
> +
> +static DECLARE_DELAYED_WORK(sony_laptop_release_key_work,
> +			    do_sony_laptop_release_key);
>  
>  /* forward event to the input subsystem */
>  static void sony_laptop_report_input_event(u8 event)
> @@ -362,12 +372,12 @@ static void sony_laptop_report_input_event(u8 event)
>  		/* we emit the scancode so we can always remap the key */
>  		input_event(kp.dev, EV_MSC, MSC_SCAN, event);
>  		input_sync(kp.dev);
> +
> +		/* schedule key release */
>  		kfifo_put(sony_laptop_input.fifo,
>  			  (unsigned char *)&kp, sizeof(kp));
> -
> -		if (!work_pending(&sony_laptop_release_key_work))
> -			queue_work(sony_laptop_input.wq,
> -					&sony_laptop_release_key_work);
> +		schedule_delayed_work(&sony_laptop_release_key_work,
> +				      msecs_to_jiffies(10));
>  	} else
>  		dprintk("unknown input event %.2x\n", event);
>  }
> @@ -394,20 +404,11 @@ static int sony_laptop_setup_input(struct acpi_device *acpi_device)
>  		goto err_dec_users;
>  	}
>  
> -	/* init workqueue */
> -	sony_laptop_input.wq = create_singlethread_workqueue("sony-laptop");
> -	if (!sony_laptop_input.wq) {
> -		printk(KERN_ERR DRV_PFX
> -				"Unable to create workqueue.\n");
> -		error = -ENXIO;
> -		goto err_free_kfifo;
> -	}
> -
>  	/* input keys */
>  	key_dev = input_allocate_device();
>  	if (!key_dev) {
>  		error = -ENOMEM;
> -		goto err_destroy_wq;
> +		goto err_free_kfifo;
>  	}
>  
>  	key_dev->name = "Sony Vaio Keys";
> @@ -470,9 +471,6 @@ err_unregister_keydev:
>  err_free_keydev:
>  	input_free_device(key_dev);
>  
> -err_destroy_wq:
> -	destroy_workqueue(sony_laptop_input.wq);
> -
>  err_free_kfifo:
>  	kfifo_free(sony_laptop_input.fifo);
>  
> @@ -483,12 +481,20 @@ err_dec_users:
>  
>  static void sony_laptop_remove_input(void)
>  {
> +	struct sony_laptop_keypress kp = { NULL };
> +
>  	/* cleanup only after the last user has gone */
>  	if (!atomic_dec_and_test(&sony_laptop_input.users))
>  		return;
>  
> -	/* flush workqueue first */
> -	flush_workqueue(sony_laptop_input.wq);
> +	cancel_delayed_work_sync(&sony_laptop_release_key_work);
> +
> +	/* Generate key-up events for remaining keys */
> +	while (kfifo_get(sony_laptop_input.fifo,
> +			 (unsigned char *)&kp, sizeof(kp)) == sizeof(kp)) {
> +		input_report_key(kp.dev, kp.key, 0);
> +		input_sync(kp.dev);
> +	}
>  
>  	/* destroy input devs */
>  	input_unregister_device(sony_laptop_input.key_dev);
> @@ -499,7 +505,6 @@ static void sony_laptop_remove_input(void)
>  		sony_laptop_input.jog_dev = NULL;
>  	}
>  
> -	destroy_workqueue(sony_laptop_input.wq);
>  	kfifo_free(sony_laptop_input.fifo);
>  }
>  
> -- 
> Dmitry
-- 
mattia
:wq!

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

end of thread, other threads:[~2009-12-03 22:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-03  5:07 [PATCH] sony-laptop - remove private workqueue, use keventd instead Dmitry Torokhov
2009-12-03 22:01 ` Mattia Dongili

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox