[PATCH 1/2] Input: wacom_w8001 - implement open and close

[PATCH 1/2] Input: wacom_w8001 - implement open and close

[Dmitry Torokhov]

Implement open() and close() methods for the input device so that we
do not start the device unless there are users listening to the events.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/wacom_w8001.c |   21 ++++++++++++++++++++-
 1 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index c14412e..5ece6c1 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -367,6 +367,20 @@ static int w8001_command(struct w8001 *w8001, unsigned char command,
 	return rc;
 }
 
+static int w8001_open(struct input_dev *dev)
+{
+	struct w8001 *w8001 = input_get_drvdata(dev);
+
+	return w8001_command(w8001, W8001_CMD_START, false);
+}
+
+static void w8001_close(struct input_dev *dev)
+{
+	struct w8001 *w8001 = input_get_drvdata(dev);
+
+	w8001_command(w8001, W8001_CMD_STOP, false);
+}
+
 static int w8001_setup(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->dev;
@@ -474,7 +488,7 @@ static int w8001_setup(struct w8001 *w8001)
 
 	strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
 
-	return w8001_command(w8001, W8001_CMD_START, false);
+	return 0;
 }
 
 /*
@@ -534,6 +548,11 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	input_dev->id.version = 0x0100;
 	input_dev->dev.parent = &serio->dev;
 
+	input_dev->open = w8001_open;
+	input_dev->close = w8001_close;
+
+	input_set_drvdata(input_dev, w8001);
+
 	err = input_register_device(w8001->dev);
 	if (err)
 		goto fail3;
-- 
1.7.6

[PATCH 2/2] Input: wacom_w8001 - simplify w8001_remove

[Dmitry Torokhov]

Since touchscreen driver does not handle any events to be sent to the
device we can close serio port first and then unregister the input device.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/wacom_w8001.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 5ece6c1..1f42d91 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -499,12 +499,12 @@ static void w8001_disconnect(struct serio *serio)
 {
 	struct w8001 *w8001 = serio_get_drvdata(serio);
 
-	input_get_device(w8001->dev);
-	input_unregister_device(w8001->dev);
 	serio_close(serio);
-	serio_set_drvdata(serio, NULL);
-	input_put_device(w8001->dev);
+
+	input_unregister_device(w8001->dev);
 	kfree(w8001);
+
+	serio_set_drvdata(serio, NULL);
 }
 
 /*
-- 
1.7.6

Re: [PATCH 1/2] Input: wacom_w8001 - implement open and close

[Chris Bagwell]

I don't have one of these to test but anyways:

Acked-by: Chris Bagwell <chris@cnpbagwell.com>

I've been trying to slowing document the protocols for ISDV4 and USB
Wacom devices (http://sourceforge.net/apps/mediawiki/linuxwacom/index.php?title=ISDV4_Protocol
for these ISDV4 ones if your interested).  During this, I was
comparing this version against the user land version of this driver
inside xf86-input-wacom.

xf86-input-wacom was doing a version of this patch so good to see
here.  The only other difference remaining is it also discards any
data on the line during startup to flush old data from after initial
STOP command and so that its sure of command/response sequence.

I do not know the serio well enough to know if it discards old data.
Even if it does not, the driver would handle old motion events OK and
the tiniest possibility an old touch query response on line when it
sends initial pen query.

Chris

On Sun, Aug 21, 2011 at 3:15 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Implement open() and close() methods for the input device so that we
> do not start the device unless there are users listening to the events.
>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>  drivers/input/touchscreen/wacom_w8001.c |   21 ++++++++++++++++++++-
>  1 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index c14412e..5ece6c1 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -367,6 +367,20 @@ static int w8001_command(struct w8001 *w8001, unsigned char command,
>        return rc;
>  }
>
> +static int w8001_open(struct input_dev *dev)
> +{
> +       struct w8001 *w8001 = input_get_drvdata(dev);
> +
> +       return w8001_command(w8001, W8001_CMD_START, false);
> +}
> +
> +static void w8001_close(struct input_dev *dev)
> +{
> +       struct w8001 *w8001 = input_get_drvdata(dev);
> +
> +       w8001_command(w8001, W8001_CMD_STOP, false);
> +}
> +
>  static int w8001_setup(struct w8001 *w8001)
>  {
>        struct input_dev *dev = w8001->dev;
> @@ -474,7 +488,7 @@ static int w8001_setup(struct w8001 *w8001)
>
>        strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
>
> -       return w8001_command(w8001, W8001_CMD_START, false);
> +       return 0;
>  }
>
>  /*
> @@ -534,6 +548,11 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
>        input_dev->id.version = 0x0100;
>        input_dev->dev.parent = &serio->dev;
>
> +       input_dev->open = w8001_open;
> +       input_dev->close = w8001_close;
> +
> +       input_set_drvdata(input_dev, w8001);
> +
>        err = input_register_device(w8001->dev);
>        if (err)
>                goto fail3;
> --
> 1.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Input: wacom_w8001 - implement open and close

[Dmitry Torokhov]

On Tue, Aug 23, 2011 at 12:22:06PM -0500, Chris Bagwell wrote:
> I don't have one of these to test but anyways:
> 
> Acked-by: Chris Bagwell <chris@cnpbagwell.com>

Thanks for looking at this. I don't have the hardware either so it would
be nice if someone who has it tried the patches...

> 
> I've been trying to slowing document the protocols for ISDV4 and USB
> Wacom devices (http://sourceforge.net/apps/mediawiki/linuxwacom/index.php?title=ISDV4_Protocol
> for these ISDV4 ones if your interested).  During this, I was
> comparing this version against the user land version of this driver
> inside xf86-input-wacom.
> 
> xf86-input-wacom was doing a version of this patch so good to see
> here.  The only other difference remaining is it also discards any
> data on the line during startup to flush old data from after initial
> STOP command and so that its sure of command/response sequence.

Hmm, I guess it indeed would be a good idea to reset w8001->idx in
w8001_open() to make sure we do not restart with buffer half-filled with
old data...

Thanks.

-- 
Dmitry

[PATCH 1/2] Input: wacom_w8001 - implement open and close

[Ping Cheng]

On Sun, Aug 21, 2011 at 1:15 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Implement open() and close() methods for the input device so that we
> do not start the device unless there are users listening to the events.
>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>


Tested-by: Ping Cheng <pingc@wacom.com> for the series.

I can not tell if the driver doing what it claimed in the commit
comment or not since I didn't trace into the code. As far as my
testing goes, the driver works as it was; it works as usual after
suspend and resume.

Ping

>
> ---
>  drivers/input/touchscreen/wacom_w8001.c |   21 ++++++++++++++++++++-
>  1 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index c14412e..5ece6c1 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -367,6 +367,20 @@ static int w8001_command(struct w8001 *w8001, unsigned char command,
>        return rc;
>  }
>
> +static int w8001_open(struct input_dev *dev)
> +{
> +       struct w8001 *w8001 = input_get_drvdata(dev);
> +
> +       return w8001_command(w8001, W8001_CMD_START, false);
> +}
> +
> +static void w8001_close(struct input_dev *dev)
> +{
> +       struct w8001 *w8001 = input_get_drvdata(dev);
> +
> +       w8001_command(w8001, W8001_CMD_STOP, false);
> +}
> +
>  static int w8001_setup(struct w8001 *w8001)
>  {
>        struct input_dev *dev = w8001->dev;
> @@ -474,7 +488,7 @@ static int w8001_setup(struct w8001 *w8001)
>
>        strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
>
> -       return w8001_command(w8001, W8001_CMD_START, false);
> +       return 0;
>  }
>
>  /*
> @@ -534,6 +548,11 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
>        input_dev->id.version = 0x0100;
>        input_dev->dev.parent = &serio->dev;
>
> +       input_dev->open = w8001_open;
> +       input_dev->close = w8001_close;
> +
> +       input_set_drvdata(input_dev, w8001);
> +
>        err = input_register_device(w8001->dev);
>        if (err)
>                goto fail3;
> --
> 1.7.6
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Input: wacom_w8001 - implement open and close

[Dmitry Torokhov]

On Tue, Aug 23, 2011 at 02:21:46PM -0700, Ping Cheng wrote:
> On Sun, Aug 21, 2011 at 1:15 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com>wrote:
> 
> > Implement open() and close() methods for the input device so that we
> > do not start the device unless there are users listening to the events.
> >
> > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> 
> 
> Tested-by: Ping Cheng <pingc@wacom.com> for the series.
> 
> I can not tell if the driver doing what it claimed in the commit comment or
> not since I didn't trace into the code. As far as my testing goes, the
> driver works as it was; it works as usual after suspend and resume.
> 

Thanks Ping, much appreciated.

-- 
Dmitry