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@xxxxxxx>
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!
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
